sandbox: make podsandbox controller plugin type of PodSandboxPlugin #9882
Conversation
|
Hi @abel-von. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
There is no legacy podsandbox controller. Sandbox api was introduced in 1.7 as experimental API with no backward compatibility guaranties until its stable. It's perfectly legal to break things until 2.0 release. |
|
It is just the podsandbox controller is a little bit special because it manage sandbox by Task plugin. and in #9884 the sandboxed Task may have dependency on the sandbox controller to update the sandbox before/after task manangement, this makes a cyclic dependency. We add a v2 sandbox controller to distinguish it with the podsandbox controller, so we can exclude it from the sandboxed Task dependency. In the last community meeting I proposed another solution, we can add a ReqiureExcludes in the plugin definition to exclude the podsandbox controller from the Requires I am not sure which is a better solution, may you have a better suggestion, please? @mxpv |
30c5593 to
000a367
Compare
|
/ok-to-test |
|
/retest |
000a367 to
b2bfed6
Compare
|
/retest |
|
@mxpv could you please take a look? |
b2bfed6 to
21aa8a3
Compare
21aa8a3 to
4bc02d6
Compare
4bc02d6 to
ae8c725
Compare
|
@mxpv Could you please take a look at this PR again? I realized that the change of plugin name may related to the change of config and config migration. Maybe we don't have to consider the compatibility issue, if we can merge it before 2.0. |
|
ping @abel-von @Burning1020 |
Signed-off-by: Abel Feng <fshb1988@gmail.com>
cri will call sandbox controller from the sandboxService, remove the dependency of client. Signed-off-by: Abel Feng <fshb1988@gmail.com>
ae8c725 to
fc5086a
Compare
|
Done resoved the confliction, but the some of the ci tests failed which seems not related to the current PR. @fuweid |
We can not handle the task in the pod sandbox as a sandboxed Task because podsandbox controller rely on task plugin to create pause container, If the task handle rely on podsandbox controller, it will make a cyclic dependency of plugins.
So In this PR, We add a new PodSandboxPlugin type for the podsandbox controller, Shim sandbox controller and other user defined sandbox controllers will be kind of SandboxControllerPlugin. and the subsequent PR of sandboxed Task will rely on this PR to handle sandboxed Task in sandbox of SandboxControllerPlugin.