Sandbox API: Add a new mode config for sandbox controller impls #7590
Conversation
a15d40b to
764932c
Compare
metadata/sandbox_test.go
Outdated
| }, | ||
| Runtime: api.RuntimeOpts{Name: "test"}, | ||
| Runtime: api.RuntimeOpts{Name: "test"}, | ||
| Controller: "remote", |
There was a problem hiding this comment.
Sandboxes are implemented by shims (side by side with task service), so we can reuse Runtime field to find right shim for tasks/sandboxing. There is no need in dedicated Controller field.
eb14f1d to
c8b6ea5
Compare
|
CC @mxpv I've pushed again. Use |
pkg/cri/config/config.go
Outdated
| // while using default snapshotters for operational simplicity. | ||
| // See https://github.com/containerd/containerd/issues/6657 for details. | ||
| Snapshotter string `toml:"snapshotter" json:"snapshotter"` | ||
| // SandboxMode setting sandbox controller mode of sandbox at runtime level like Snapshotter |
There was a problem hiding this comment.
| // SandboxMode setting sandbox controller mode of sandbox at runtime level like Snapshotter | |
| // SandboxMode defines which sandbox runtime to use when scheduling pods | |
| // This features requires experimental CRI server to be enabled (use ENABLE_CRI_SANDBOXES=1) |
sandbox/controller.go
Outdated
| "github.com/containerd/containerd/api/services/sandbox/v1" | ||
| ) | ||
|
|
||
| const DefaultSandboxController = "podsandbox" |
There was a problem hiding this comment.
If the mode is type, I think we should use const type and add validation for this.
type Mode string
const (
ModeDefault Mode = "default"
ModeRemote = "remote"
)
func ValidateMode() error {
...
}Something like that.
There was a problem hiding this comment.
I want make "SandboxMode" similar to "Snapshotter", they're all string type. If we change it to Mode type, there wolud be some conversion. Is it necessary?
pkg/cri/sbserver/sandbox_run.go
Outdated
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get sandbox runtime: %w", err) | ||
| } | ||
| // Absent case |
There was a problem hiding this comment.
we can verify the Runtime.Config when setup CRI plugin. And update it to default mode if it is empty.
09d0dcf to
8ca5390
Compare
6727116 to
5ccbc87
Compare
|
@Burning1020: The
Use
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
sandbox/controller.go
Outdated
| type Mode string | ||
|
|
||
| const ( | ||
| // ModePodSandbox means use Controller implementation from sbserver podsandbox package. |
There was a problem hiding this comment.
I feel like this is CRI specific and should be maintained in cri package.
572a591 to
233399c
Compare
pkg/cri/config/config.go
Outdated
| "github.com/containerd/containerd/plugin" | ||
| ) | ||
|
|
||
| type Mode string |
There was a problem hiding this comment.
updated to SandboxControllerMode
Add a new config as sandbox controller mod, which can be either "podsandbox" or "shim". If empty, set it to default "podsandbox" when CRI plugin inits. Signed-off-by: Zhang Tianyang <burning9699@gmail.com>
233399c to
c953eec
Compare
|
/test pull-containerd-sandboxed-node-e2e |
|
/retest |
Works for #7312:
Add a new config as sandbox controller mod, which can be either
"podsandbox" or "shim". If empty, set it to default "podsandbox"
when CRI plugin inits.
Signed-off-by: Zhang Tianyang burning9699@gmail.com