Add option to specify containerd runtime #4141

slonopotamus · 2023-08-13T12:37:04Z

Thoughts: it could just be runtime or default runtime, hinting that in the future specific build run can override it.

cmd/buildkitd/config/config.go

cmd/buildkitd/main_containerd_worker.go

tonistiigi · 2023-08-24T00:39:14Z

5.865 # github.com/moby/buildkit/cmd/buildkitd
5.865 cmd/buildkitd/main_containerd_worker.go:54:45: undefined: defaults

slonopotamus · 2023-08-24T07:17:20Z

Added missing import.

slonopotamus · 2023-08-24T13:07:37Z

I believe that TestRuncWorkerExec failure on CI is unrelated to this PR: #4146

AkihiroSuda · 2023-08-25T18:15:17Z

cmd/buildkitd/config/config.go

@@ -112,6 +112,7 @@ type ContainerdConfig struct {
 	Labels    map[string]string `toml:"labels"`
 	Platforms []string          `toml:"platforms"`
 	Namespace string            `toml:"namespace"`
+	Runtime   string            `toml:"runtime"`


Shouldn't this support options for runc binary path, cgroup driver, etc?
I also guess this should be map[string]interface{} to support multiple runtime classes in the future.

i.e., The config structure should be similar to the config structure of containerd/CRI.

I am not sure how to pass that to containerd. What I just wanted to achieve is to be able to override client.defaultRuntime. Because there are cough, cough platforms where runtime isn't mature enough to be declared The Default.

The lack of such configuration option leaded to FreeBSD runtime being hardcoded in buildkit. I'm in the same boat with macOS.

So, my idea is: with these changes, one can configure runtime options in containerd config, while using buildkit to select needed runtime. Or, option added here can just be used to specify path path to runtime binary. Exposing the full runtime configuration through buildkit config is too much extending the scope of the task.

Some related work in moby;

daemon: allow shimv2 runtimes to be configured moby#45032

I need to refresh my memory (but perhaps the code describes it), but ISTR, we only allowed paths etc to be configured for the legacy bits, and otherwise only allow a (fully qualified?) reference to be specified, or at least from the client side (to prevent a arbitrary paths to be passed that are not in the server's PATH).

I think I missed this originally. We should probably make sure to cover this is in a first pass - I wrote up a possible implementation: jedevc@22d9bb1. @AkihiroSuda @tonistiigi wdyt? (I took inspiration from how CRI does it today). @slonopotamus you happy to pull that change into your branch (or I can force-push)?

Without a way to configure the runtime, I think the feature is quite limited (e.g. I have a use case for wanting to override the runc binary used by the io.containerd.runc.v2 runtime, which is possible by using the binary_name option). Containerd doesn't seem to have any other way to configure this, so we need to allow this configuration in buildkit if we want to allow users this kind of freedom.

Even if we don't take this now, we need to provide a place in config fields to allow this kind of configuration. Previously we had:

[worker.containerd] runtime = "io.containerd.runc.v2"

I think something like this should work easily:

[worker.containerd.runtime] name = "io.containerd.runc.v2" options = { binary_name = "foobar" }

@jedevc I'm perfectly fine if you create a separate PR. I just need a way to specify which runtime to use, it isn't really important what lines one would need to write into config file, it is easy to adapt.

Opened in #4279.

ktock

LGTM
maybe we'll need to use containerd's WithRuntime if we want to allow users fully configuring the shim behaviour (as commented in #4141 (comment))

https://github.com/containerd/containerd/blob/d015c99b2ec990c914a4b4546ec10d61cd947ab0/container_opts.go#L58

slonopotamus · 2023-09-11T06:32:26Z

User already can fully configure runtime in containerd config. I don't see much value adding a second place with exactly the same settings. And then the third, to allow configuration of all that through Moby config?

AkihiroSuda · 2023-09-11T07:01:17Z

User already can fully configure runtime in containerd config. I don't see much value adding a second place with exactly the same settings. And then the third, to allow configuration of all that through Moby config?

The first one only applies to CRI, not to BuildKit and Moby.
It would be nice to have a unified configuration registry, but that may take a time.

slonopotamus · 2023-09-17T13:19:34Z

BTW, Moby already has map[string,Runtime]. So, both containerd and Moby can use different runtimes. It's only buildctl that doesn't have an option to specify which runtime to use.

docs/buildkitd.toml.md

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>

crazy-max · 2023-09-24T15:00:48Z

cmd/buildkitd/main_containerd_worker.go

@@ -46,6 +48,14 @@ func init() {
 		defaultConf.Workers.Containerd.Namespace = defaultContainerdNamespace
 	}

+	if defaultConf.Workers.Containerd.Runtime == "" {
+		if runtime.GOOS == "freebsd" {
+			defaultConf.Workers.Containerd.Runtime = "wtf.sbk.runj.v1"


Maybe add a TODO saying it should be removed once containerd/containerd#8964 comes with a containerd release?

tonistiigi · 2023-09-26T17:08:20Z

#4279

slonopotamus · 2023-09-26T17:25:06Z

@jedevc thanks for pushing this to the end!

slonopotamus mentioned this pull request Aug 13, 2023

Add macOS support #4059

Closed

4 tasks

jedevc reviewed Aug 13, 2023

View reviewed changes

cmd/buildkitd/config/config.go Show resolved Hide resolved

cmd/buildkitd/main_containerd_worker.go Outdated Show resolved Hide resolved

slonopotamus force-pushed the containerd-runtime-option branch from aca0577 to d9b22ac Compare August 15, 2023 08:38

slonopotamus force-pushed the containerd-runtime-option branch from d9b22ac to d1ad968 Compare August 24, 2023 07:17

jedevc approved these changes Aug 24, 2023

View reviewed changes

AkihiroSuda reviewed Aug 25, 2023

View reviewed changes

slonopotamus force-pushed the containerd-runtime-option branch from d1ad968 to e9f1eab Compare September 10, 2023 13:44

AkihiroSuda requested review from tonistiigi and ktock September 11, 2023 03:36

tonistiigi approved these changes Sep 11, 2023

View reviewed changes

ktock approved these changes Sep 11, 2023

View reviewed changes

slonopotamus force-pushed the containerd-runtime-option branch from e9f1eab to b79ce6d Compare September 17, 2023 13:16

slonopotamus force-pushed the containerd-runtime-option branch from b79ce6d to a96327d Compare September 24, 2023 12:39

AkihiroSuda reviewed Sep 24, 2023

View reviewed changes

docs/buildkitd.toml.md Outdated Show resolved Hide resolved

Add option to specify containerd runtime

ae8e604

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>

slonopotamus force-pushed the containerd-runtime-option branch from a96327d to ae8e604 Compare September 24, 2023 13:08

AkihiroSuda approved these changes Sep 24, 2023

View reviewed changes

crazy-max reviewed Sep 24, 2023

View reviewed changes

jedevc mentioned this pull request Sep 26, 2023

Add options to specify containerd runtime (alternative) #4279

Merged

tonistiigi closed this Sep 26, 2023

slonopotamus deleted the containerd-runtime-option branch September 27, 2023 19:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to specify containerd runtime #4141

Add option to specify containerd runtime #4141

slonopotamus commented Aug 13, 2023 •

edited

Loading

tonistiigi commented Aug 24, 2023

slonopotamus commented Aug 24, 2023

slonopotamus commented Aug 24, 2023

AkihiroSuda Aug 25, 2023

slonopotamus Aug 25, 2023

slonopotamus Aug 26, 2023

thaJeztah Sep 17, 2023

jedevc Sep 26, 2023

slonopotamus Sep 26, 2023

jedevc Sep 26, 2023

ktock left a comment •

edited

Loading

slonopotamus commented Sep 11, 2023

AkihiroSuda commented Sep 11, 2023

slonopotamus commented Sep 17, 2023 •

edited

Loading

crazy-max Sep 24, 2023

tonistiigi commented Sep 26, 2023

slonopotamus commented Sep 26, 2023

Add option to specify containerd runtime #4141

Add option to specify containerd runtime #4141

Conversation

slonopotamus commented Aug 13, 2023 • edited Loading

tonistiigi commented Aug 24, 2023

slonopotamus commented Aug 24, 2023

slonopotamus commented Aug 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktock left a comment • edited Loading

Choose a reason for hiding this comment

slonopotamus commented Sep 11, 2023

AkihiroSuda commented Sep 11, 2023

slonopotamus commented Sep 17, 2023 • edited Loading

Choose a reason for hiding this comment

tonistiigi commented Sep 26, 2023

slonopotamus commented Sep 26, 2023

slonopotamus commented Aug 13, 2023 •

edited

Loading

ktock left a comment •

edited

Loading

slonopotamus commented Sep 17, 2023 •

edited

Loading