Initial sandbox API CRI integration (implement Controller.Start)#7228
Merged
mxpv merged 7 commits intocontainerd:mainfrom Aug 5, 2022
Merged
Initial sandbox API CRI integration (implement Controller.Start)#7228mxpv merged 7 commits intocontainerd:mainfrom
mxpv merged 7 commits intocontainerd:mainfrom
Conversation
Signed-off-by: Maksym Pavlenko <[email protected]>
mxpv
force-pushed
the
sb2
branch
2 times, most recently
from
9dc253a to
d1e6777
Compare
Signed-off-by: Maksym Pavlenko <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]>
Member
Author
|
/test pull-containerd-sandboxed-node-e2e |
fuweid
reviewed
Aug 2, 2022
| } | ||
|
|
||
| func (c *Controller) Start(ctx context.Context, id string) (_ *api.ControllerStartResponse, retErr error) { | ||
| sandboxInfo, err := c.client.SandboxStore().Get(ctx, id) |
Member
There was a problem hiding this comment.
Just wondering that we should handle network here or not. 👀
Member
Author
There was a problem hiding this comment.
I don't have strong opinion on this. Since we can build on all platforms and turn it off, it's ok to keep it in CRI for now. We can brainstorm more on this.
Member
Author
|
@fuweid thanks for looking into this. I've added a few TODOs to follow up. |
fuweid
approved these changes
Aug 2, 2022
Member
fuweid
left a comment
fuweid
left a comment
There was a problem hiding this comment.
LGTM
It is milestone.
Let's move it forward!
cpuguy83
approved these changes
Aug 2, 2022
Signed-off-by: Maksym Pavlenko <[email protected]>
mikebrow
approved these changes
Aug 4, 2022
Member
mikebrow
left a comment
mikebrow
left a comment
There was a problem hiding this comment.
just a small nit .. progress! let's move forward
mxpv
changed the title
This was referenced Feb 25, 2023
This was referenced Mar 7, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sandbox API has Controller interface used to manage a sandbox instances lifecycle (kin of
TaskServiceinterface to manage tasks lifecycle). containerd shims that support sandbox API supposed to implement this interface at shim level. This way we can expose different sandbox implementations in containerd (like podsandbox aka pause container or microVM).In case of CRI/Kube, primary user of the
Controllerinterface is going to be our CRI plugin (which we forked in #7164 for sandbox work). Depending on CRI request, it'll call appropriate controller func (e.g.RunPodSandboxinvokesController.Start,StopPodSandboxinvokesController.Stop, etc).So the end-goal is to have CRI rely on
Controllerinterface instead of managing pod-sandbox directly. This PR starts refactoring and handlesController.Startimplementation. All relevant CRI code is now moved topodsandboxpackage and implements Start func to setup container spec and launch a task.For pod-sandbox, we'll have an in-memory implementation. Once migrated,
Controllerinterface will getttrpcRemoteController, which will call-in shims instead.