RFC: runtime: enable kata as optional runtime by jongwu · Pull Request #42986 · moby/moby

jongwu · 2021-11-03T02:09:19Z

After kata move to 2.x, "--add-runtime" is no longer working as kata runtime changed into a containerd-shim. People enjoy starting kata using docker, but it seems there is no way to go. Here is just a proposal to enable kata as an optional runtime. It's not good enough but deserve a try.

Currently, containerd-shim-v2 is adopt by dockerd, thus we can easily
add containerd-shim-kata-v2 as docker runtime.
we can get it by just exporting shim config to user and people can add containerd kata shim through the config.

Signed-off-by: Jianyong Wu jianyong.wu@arm.com

- What I did
enable containerd-shim based runtime like kata as docker optional runtime

- How I did it
export shim config to user
- How to verify it
add kata runtime in dockerd config like:
{
"runtimes":{
"io.containerd.kata.v2":{
"shim":{
"binary":"io.containerd.kata.v2"
}
}
}
}
and specify runtime using "--runtime io.containerd.kata.v2" when start kata using docker

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

🐭

AkihiroSuda · 2021-11-03T10:15:59Z

daemon/runtime_unix.go

+func kataV2ShimConfig(conf *config.Config, runtimePath string) *types.ShimConfig {
+	return &types.ShimConfig{
+		Binary: linuxKataShimV2,
+		Opts: &v2runcoptions.Options{


Does kata really support this struct?

thanks @AkihiroSuda -, I think that kata doesn't support the opts part. I have removed these lines.

thaJeztah · 2021-11-03T13:23:15Z

What would be needed to allow this to be configured in the daemon configuration without having to add special handling / code in this repository?

Basically, my ideal would be that new runtimes could be configured by the user (without us having to maintain special code for each of them)

jongwu · 2021-11-06T08:08:41Z

What would be needed to allow this to be configured in the daemon configuration without having to add special handling / code in this repository?

Basically, my ideal would be that new runtimes could be configured by the user (without us having to maintain special code for each of them)

Thanks @thaJeztah -, Agree. docker should not maintain runtime list. I have changed the way to support kata that just exporting the shim config to user.

cpuguy83 · 2021-11-06T14:55:23Z

api/types/types.go

 type ShimConfig struct {
-	Binary string
+	Binary string      `json:"binary,omitempty`
 	Opts   interface{}


Maybe a *ptypes.Any here.
Or perhaps define our own equivalent w/o importing proto libs which we can convert internally.

Hello @cpuguy83 -, I'm not sure why we need proto type here? How we used it for? Is it enough to transfer "shim" info through grpc by just seting the binary type to "ptypes.Any"?

Right, we don't want to handle the type in dockerd, especially since it's just passed off to shim anyway.

The *Any type is just a proper encoding of interface{}.

Hi @cpuguy83 -, I try to set binary to *ptypes.Any but I don't know how to test it using local json file to specify the runtime (I'm not familiar with the proto language) thus I don't know if the code is correctly.

Hello @cpuguy83 -, I twist the code again, now it works well.

I'm not sure I understand the change you added.

What we should be able to do is:

Tell containerd which shim to use

Note that containerd only currently supports resolving a reference name like io.containerd.runc.v2 to containerd-shim-runc-v2, it does not support custom paths

Pass along a config to the shim on create.

This is specific to each shim, eg runctypes.RuncOptions

To do 2, we either need to have explicit knowledge of every shim we ever want to support OR we should just be passing through configs.
In order to pass through those configs we need a place for users to encode them.
In this case we are looking to do that in the daemon config that gets stored in /etc/docker/daemon.json.
An interface{} does allow us to do this, but then we are passing around untyped data, and the containerd API is expecting to fill a types.Any (from the protobuf lib).
A types.Any has a type URL (i.e. full path of type... e.g. github.com/my/shim.Options) and some data.
https://github.com/gogo/protobuf/blob/master/protobuf/google/protobuf/any.proto

To understand how this is consumed, check out https://github.com/containerd/typeurl

These options get passed along to shims when containerd calls Create against the shim.

Sorry for my poor code. I have never touched these proto things before. Thanks for your guidance, I begin to aware how to deal with these. Really appreciated @cpuguy83

Hello @cpuguy83 -, I set shim.opts to types.Any and keep binary as string, thus let opts to convey the config. Is it right?

jongwu · 2021-11-09T02:49:37Z

Hello @AkihiroSuda @thaJeztah -, help review this PR, thanks.

thaJeztah · 2021-11-15T12:32:45Z

Looks like something isn't happy in CI; I see various tests fail with a similar error, like:

=== RUN   TestAuthZPluginV2Disable
    authz_plugin_v2_test.go:73: assertion failed: error is not nil: Error response from daemon: invalid task create option for io.containerd.runc.v2: unknown
--- FAIL: TestAuthZPluginV2Disable (1.67s)

Seems to come from https://github.com/containerd/containerd/blob/b5b83e0512458afd1ef75e778dfdbde2f88d424c/services/tasks/local.go#L771-L791

jongwu · 2021-11-16T06:06:54Z

Looks like something isn't happy in CI; I see various tests fail with a similar error, like:
=== RUN   TestAuthZPluginV2Disable
    authz_plugin_v2_test.go:73: assertion failed: error is not nil: Error response from daemon: invalid task create option for io.containerd.runc.v2: unknown
--- FAIL: TestAuthZPluginV2Disable (1.67s)
Seems to come from https://github.com/containerd/containerd/blob/b5b83e0512458afd1ef75e778dfdbde2f88d424c/services/tasks/local.go#L771-L791

Hi @thaJeztah -, I'm not sure if I should do something here to save the CI or it's no business of this PR.

thaJeztah · 2021-11-19T14:27:50Z

Hi @thaJeztah -, I'm not sure if I should do something here to save the CI or it's no business of this PR.

From the looks of it, I suspect it's related to the changes in this PR (invalid task create option for io.containerd.runc.v2: unknown). I must admit I'm not too familiar with this part of the code though. Hoping @cpuguy83 or @AkihiroSuda may be able to assist if needed 😅

cpuguy83

I'm guessing we are passing the wrong thing to containerd on the create call.

cpuguy83 · 2021-11-22T17:49:01Z

daemon/runtime_unix.go

 		if rt.Shim == nil {
 			rt.Shim = defaultV2ShimConfig(daemon.configStore, rt.Path)
 		}
+		if rt.Shim.Opts == nil {


Only the runc shims takes these default options, if the options are nil then we need to leave it nil unless we are using a runc shim.

cpuguy83 · 2021-11-22T17:49:17Z

daemon/start_unix.go

 package daemon // import "github.com/docker/docker/daemon"

 import (
+	types1 "github.com/containerd/typeurl"


Can we keep all of these as typeurl?

cpuguy83 · 2021-11-22T17:49:34Z

api/types/types.go

 	"github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/go-connections/nat"
+	types1 "github.com/gogo/protobuf/types"


nit ptypes

cpuguy83 · 2021-11-22T18:12:25Z

I think one more issue with this is, we likely don't want those shim options to be protobuf in our json config.
I'm not exactly sure what to do here. Maybe instead of embedding the options in the daemon config like this we need to have a specific file, like /etc/docker/runtimes/<name>/shimopts. Maybe type url can go into the daemon config, though?

I can't really think of a way to do this that's not a pain to deal with as a user.
Open to suggestions here.

struanb · 2021-12-17T18:49:38Z

Hi @jongwu @cpuguy83. I've been trying to get Docker to launch Kata v2 containers too. I'm unsure what the shim options are you're referring to, and so don't know if it helps, but Kata proposes using a wrapper executable, allowing any needed options to be added to the wrapper, like this:

/usr/local/bin/containerd-shim-katafc-v2 (on Ubuntu, with kata-containers 2.3.0 installed via Snap):

#!/bin/bash
KATA_CONF_FILE=/snap/kata-containers/current/usr/share/defaults/kata-containers/configuration.toml /snap/kata-containers/current/usr/bin/containerd-shim-kata-v2 "$@"

Is this a workable approach that would allow the daemon.json to contain only a path to the /usr/local/bin/containerd-shim-katafc-v2 wrapper?

N.B. In tests, if I copy this /usr/local/bin/containerd-shim-katafc-v2 wrapper over /usr/bin/containerd-shim-runc-v2 then unconfigured docker succeeds in launching kata v2 containers. So it really doesn't seem like much is needed to enable this.

jongwu · 2021-12-20T03:23:38Z

Hi @jongwu @cpuguy83. I've been trying to get Docker to launch Kata v2 containers too. I'm unsure what the shim options are you're referring to, and so don't know if it helps, but Kata proposes using a wrapper executable, allowing any needed options to be added to the wrapper, like this:

/usr/local/bin/containerd-shim-katafc-v2 (on Ubuntu, with kata-containers 2.3.0 installed via Snap):
#!/bin/bash
KATA_CONF_FILE=/snap/kata-containers/current/usr/share/defaults/kata-containers/configuration.toml /snap/kata-containers/current/usr/bin/containerd-shim-kata-v2 "$@"
Is this a workable approach that would allow the daemon.json to contain only a path to the /usr/local/bin/containerd-shim-katafc-v2 wrapper?

N.B. In tests, if I copy this /usr/local/bin/containerd-shim-katafc-v2 wrapper over /usr/bin/containerd-shim-runc-v2 then unconfigured docker succeeds in launching kata v2 containers. So it really doesn't seem like much is needed to enable this.

Hi, @struanb -, what we want is let docker recognize and launch different runtimes. eg. using "--runtime " to specify runtime. IMO, docker has not prepared to accept other runtimes than "containerd-shim-runc-*" now. Replacing "shim" is not the normal way to launch a docker.

Some runtimes, e.g. Kata Containers, has no OCI runtime binary but a containerd-shim based runtime. To support these kind of things, shim config must be export to user config. For example, if user want to add kata as its runtime, there are 2 steps to go: First, add kata runtime to daemon config file like: { "runtimes":{ "io.containerd.kata.v2":{ "shim":{ "binary": "io.containerd.kata.v2" } } } } Second, specify kata runtime using "--runtime io.containerd.kata.v2" in command line. Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu · 2021-12-20T05:36:18Z

I think one more issue with this is, we likely don't want those shim options to be protobuf in our json config. I'm not exactly sure what to do here. Maybe instead of embedding the options in the daemon config like this we need to have a specific file, like /etc/docker/runtimes/<name>/shimopts. Maybe type url can go into the daemon config, though?

Sorry to replay so late. IMO adding a new config file is a little complicated for user and users are used to configurate dockerd in daemon config.

I can't really think of a way to do this that's not a pain to deal with as a user. Open to suggestions here.

We can do it here this way and may improve it later if there is a better way composed.

cpuguy83 · 2022-01-27T19:44:22Z

Discussed this on the maintainers meeting today with @thaJeztah, @tonistiigi, @samuelkarp, and @chris-crone.
The approach to use a separate config for shim specific options is what we'd like to do.
This means the user shove the options in protobuf or whatever encoding is easiest that the shim supports into the file on disk if they need to configure the shim.

If we ask the user to add the type url in the daemon.json, then docker can construct the ptypes.Any itself.
Later on we could add support for inline config if needed, e.g. maybe the shim supports json or... base64 encoded toml(?)... which may be friendly to put into a json document.

Should this config file be implicit based on the name of the runtime or explicit in the runtime config?

jongwu · 2022-01-29T06:35:57Z

Discussed this on the maintainers meeting today with @thaJeztah, @tonistiigi, @samuelkarp, and @chris-crone. The approach to use a separate config for shim specific options is what we'd like to do. This means the user shove the options in protobuf or whatever encoding is easiest that the shim supports into the file on disk if they need to configure the shim.

OK.

If we ask the user to add the type url in the daemon.json, then docker can construct the ptypes.Any itself. Later on we could add support for inline config if needed, e.g. maybe the shim supports json or... base64 encoded toml(?)... which may be friendly to put into a json document.

Er, what's the format of the shim config file in the initial plan?

Should this config file be implicit based on the name of the runtime or explicit in the runtime config?

So, the config file for kata can be "/etc/docker/runtimes/kata-containers/shimopts.json"? In this commit, the value of "--runtime" is "io.containerd.kata.v2". Base on this separated shim config file, shall we start a kata using "--runtime kata-containers"?
what do you mean by "runtime config" ? I'm not sure there is a config file for "kata-shim".

AkihiroSuda reviewed Nov 3, 2021

View reviewed changes

AkihiroSuda added area/runtime Runtime kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny labels Nov 3, 2021

jongwu force-pushed the kata_enable branch from 7bf7dfd to 0da3f19 Compare November 6, 2021 08:02

cpuguy83 reviewed Nov 6, 2021

View reviewed changes

jongwu force-pushed the kata_enable branch from 0da3f19 to 6f83748 Compare November 8, 2021 09:44

jongwu force-pushed the kata_enable branch 2 times, most recently from 64175f5 to f81601b Compare November 10, 2021 13:26

jongwu requested a review from cpuguy83 November 11, 2021 02:53

jongwu force-pushed the kata_enable branch from f81601b to 8e5f611 Compare November 14, 2021 06:12

fidencio mentioned this pull request Nov 15, 2021

shimv2 support for sandboxed runtimes in Moby kata-containers/kata-containers#488

Open

cpuguy83 requested changes Nov 22, 2021

View reviewed changes

jongwu force-pushed the kata_enable branch from 8e5f611 to fdde9e5 Compare December 20, 2021 05:30

bergwolf mentioned this pull request Jan 11, 2022

Why is docker no longer supported on kata2.X? kata-containers/kata-containers#3417

Closed

Jakob-Naucke mentioned this pull request Jan 26, 2022

Running GUI applications within Kata Containers accelerated with iGLX kata-containers/kata-containers#3524

Open

This was referenced Jul 14, 2022

daemon: support alternative runtimes MVP #43800

Merged

daemon: let shim binary of runtime configurable #43736

Closed

cpuguy83 closed this in #43800 Jul 29, 2022

thaJeztah mentioned this pull request Jul 29, 2022

[22.06 backport] daemon: support other containerd runtimes (MVP) #43887

Merged

Conversation

jongwu commented Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Nov 3, 2021

Uh oh!

jongwu commented Nov 6, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jongwu commented Nov 9, 2021

Uh oh!

thaJeztah commented Nov 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jongwu commented Nov 16, 2021

Uh oh!

thaJeztah commented Nov 19, 2021

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpuguy83 commented Nov 22, 2021

Uh oh!

struanb commented Dec 17, 2021

Uh oh!

jongwu commented Dec 20, 2021

Uh oh!

jongwu commented Dec 20, 2021

Uh oh!

cpuguy83 commented Jan 27, 2022

Uh oh!

jongwu commented Jan 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jongwu commented Nov 3, 2021 •

edited

Loading

thaJeztah commented Nov 15, 2021 •

edited

Loading

jongwu commented Jan 29, 2022 •

edited

Loading