Initial sandbox API CRI integration (implement Controller.Start) #7228
Conversation
This file contains 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
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
mxpv
force-pushed
the
sb2
branch
2 times, most recently
from
9dc253a
to
d1e6777
Compare
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
|
/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) |
There was a problem hiding this comment.
Just wondering that we should handle network here or not. 👀
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.
|
@fuweid thanks for looking into this. I've added a few TODOs to follow up. |
fuweid
approved these changes
Aug 2, 2022
cpuguy83
approved these changes
Aug 2, 2022
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
mikebrow
approved these changes
Aug 4, 2022
mxpv
changed the title
This was referenced Feb 25, 2023
This was referenced Mar 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.