Repository navigation
Add runc API for building CLI extensions - #1788
crosbymichael wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Should we call API as ContainerOperations?
There was a problem hiding this comment.
Ya, we can work on naming and how big this interface should be for general OCI compatibility.
There was a problem hiding this comment.
What do you think about adding context.Context to these methods? It's not really important with the implementation side, but if we want to build clients the context is useful for exec.CommandContext to support timeouts and such.
There was a problem hiding this comment.
I am not sure at the moment. Is it something that we can revisit later, if required? Right now we are able to manage with context at higher layers.
|
Pushed a change moving checkpoint/restore to a separate interface blocked by an |
| NoPivot: context.Bool("no-pivot"), | ||
| NoNewKeyring: context.Bool("no-new-keyring"), | ||
| PreserveFDs: context.Int("preserve-fds"), | ||
| Detach: context.Bool("detach"), |
There was a problem hiding this comment.
There doesn't seem to be detach flag for runc create.
There was a problem hiding this comment.
Create detaches by default so this value shouldn't matter either way but Ill remove it for correctness
dqminh
left a comment
dqminh
left a comment
There was a problem hiding this comment.
I think this looks great overall. Very nice !
| Run(id string, opts CreateOpts) (*CreateResult, error) | ||
| Start(id string) error | ||
| State(id string) (*Container, error) | ||
| Exec(id string, opts ExecOpts) (*CreateResult, error) |
There was a problem hiding this comment.
I think Run and Exec returns CreateResult is not very intuitive, probably CommandResult is a better name.
| type ContainerOperations interface { | ||
| Create(id string, opts CreateOpts) (*CreateResult, error) | ||
| Delete(id string, opts DeleteOpts) error | ||
| Kill(id string, sig syscall.Signal, opts KillOpts) error |
There was a problem hiding this comment.
not sure if this is intended to be consumed cross-platform. If yes, probably this can be replaced with os.Signal instead of current linux-specific syscall.Signal
This packages allow new CLI apis to be created easily Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
There is still issues of backwards compat when running runc via the cli alone. It still needs to create pipes in the non-detached state so a lot of functionality is keyd off the `detach` flag for runc. The other option is to move this to the CLI but that would be unfail to other consumers. runc will handle this internally for now to keep its backwards compat. Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
|
This looks fine overall. We should probably put it in CI for containerd and cri-o to make sure there are no regressions. |
|
I will open a PR in cri-o to test this out. |
|
ping @opencontainers/runc-maintainers maintainers, this came up on the OCI dev call today (as towards moving to a v1.0 release) |
5 participants
This change adds an
apiandcommandspackage that abstracts out the libcontainer implementation from what CLI consumers see. It allows other producers of runtimes, that atleast write the runtime in Go to implement theAPIinterface and get the CLI code for free. The command package also allows for extensions and I have placed things like update, events, and the runc init command there as it should be out of the API spec.This allows us to solidify the CLI as a spec and things like runtime-tools and other runtimes to depend on a consistent API for OCI containers and runtime-spec. This also allows us to write consistent client code for interacting with OCI runtimes based on the API interface.
Please review and give your thoughts. I think we will probably want to make the API interface smaller, such as removing checkpoint/restore into an extension API as not all OCI runtimes will implement this.