From e4ff3327cf756f9f2e23887548b2ada3c51b2fcd Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 29 Nov 2024 16:26:51 +0100 Subject: [PATCH 1/4] Improve the implementation for UseLegacyConfig Signed-off-by: Evan Lezar --- pkg/config/engine/containerd/config_v1.go | 24 +++++++------ pkg/config/engine/containerd/containerd.go | 30 +++++++++------- .../runtime/containerd/config_v1_test.go | 34 +++++++++---------- .../runtime/containerd/containerd.go | 7 ++-- 4 files changed, 53 insertions(+), 42 deletions(-) diff --git a/pkg/config/engine/containerd/config_v1.go b/pkg/config/engine/containerd/config_v1.go index db6cf2dc..10b6d087 100644 --- a/pkg/config/engine/containerd/config_v1.go +++ b/pkg/config/engine/containerd/config_v1.go @@ -70,18 +70,20 @@ func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool) error config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "options", "BinaryName"}, path) config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "options", "Runtime"}, path) - if setAsDefault && c.UseDefaultRuntimeName { - config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime_name"}, name) - } else if setAsDefault { - // Note: This is deprecated in containerd 1.4.0 and will be removed in 1.5.0 - if config.GetPath([]string{"plugins", "cri", "containerd", "default_runtime"}) == nil { - config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_type"}, c.RuntimeType) - config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_root"}, "") - config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_engine"}, "") - config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "privileged_without_host_devices"}, false) + if setAsDefault { + if !c.UseLegacyConfig { + config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime_name"}, name) + } else { + // Note: This is deprecated in containerd 1.4.0 and will be removed in 1.5.0 + if config.GetPath([]string{"plugins", "cri", "containerd", "default_runtime"}) == nil { + config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_type"}, c.RuntimeType) + config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_root"}, "") + config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_engine"}, "") + config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "privileged_without_host_devices"}, false) + } + config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "BinaryName"}, path) + config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "Runtime"}, path) } - config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "BinaryName"}, path) - config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "Runtime"}, path) } *c.Tree = config diff --git a/pkg/config/engine/containerd/containerd.go b/pkg/config/engine/containerd/containerd.go index a5b08810..128befb3 100644 --- a/pkg/config/engine/containerd/containerd.go +++ b/pkg/config/engine/containerd/containerd.go @@ -27,10 +27,15 @@ import ( // Config represents the containerd config type Config struct { *toml.Tree - Logger logger.Interface - RuntimeType string - UseDefaultRuntimeName bool - ContainerAnnotations []string + Logger logger.Interface + RuntimeType string + ContainerAnnotations []string + // UseLegacyConfig indicates whether a config file pre v1.3 should be generated. + // For version 1 config prior to containerd v1.4 the default runtime was + // specified in a containerd.runtimes.default_runtime section. + // This was deprecated in v1.4 in favour of containerd.default_runtime_name. + // Support for this section has been removed in v2.0. + UseLegacyConfig bool } var _ engine.Interface = (*Config)(nil) @@ -73,14 +78,14 @@ func New(opts ...Option) (engine.Interface, error) { } cfg := &Config{ - Tree: tomlConfig, - Logger: b.logger, - RuntimeType: b.runtimeType, - UseDefaultRuntimeName: b.useLegacyConfig, - ContainerAnnotations: b.containerAnnotations, + Tree: tomlConfig, + Logger: b.logger, + RuntimeType: b.runtimeType, + ContainerAnnotations: b.containerAnnotations, + UseLegacyConfig: b.useLegacyConfig, } - version, err := cfg.parseVersion(b.useLegacyConfig) + version, err := cfg.parseVersion() if err != nil { return nil, fmt.Errorf("failed to parse config version: %v", err) } @@ -95,9 +100,10 @@ func New(opts ...Option) (engine.Interface, error) { } // parseVersion returns the version of the config -func (c *Config) parseVersion(useLegacyConfig bool) (int, error) { +func (c *Config) parseVersion() (int, error) { defaultVersion := 2 - if useLegacyConfig { + // For legacy configs, we default to v1 configs. + if c.UseLegacyConfig { defaultVersion = 1 } diff --git a/tools/container/runtime/containerd/config_v1_test.go b/tools/container/runtime/containerd/config_v1_test.go index 26b673e4..a3967833 100644 --- a/tools/container/runtime/containerd/config_v1_test.go +++ b/tools/container/runtime/containerd/config_v1_test.go @@ -92,10 +92,10 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) { require.NoError(t, err, "%d: %v", i, tc) v1 := &containerd.ConfigV1{ - Logger: logger, - Tree: cfg, - UseDefaultRuntimeName: !tc.legacyConfig, - RuntimeType: runtimeType, + Logger: logger, + Tree: cfg, + UseLegacyConfig: tc.legacyConfig, + RuntimeType: runtimeType, } err = o.UpdateConfig(v1) @@ -238,11 +238,11 @@ func TestUpdateV1Config(t *testing.T) { require.NoError(t, err) v1 := &containerd.ConfigV1{ - Logger: logger, - Tree: cfg, - UseDefaultRuntimeName: true, - RuntimeType: runtimeType, - ContainerAnnotations: []string{"cdi.k8s.io/*"}, + Logger: logger, + Tree: cfg, + UseLegacyConfig: true, + RuntimeType: runtimeType, + ContainerAnnotations: []string{"cdi.k8s.io/*"}, } err = o.UpdateConfig(v1) @@ -397,11 +397,11 @@ func TestUpdateV1ConfigWithRuncPresent(t *testing.T) { require.NoError(t, err) v1 := &containerd.ConfigV1{ - Logger: logger, - Tree: cfg, - UseDefaultRuntimeName: true, - RuntimeType: runtimeType, - ContainerAnnotations: []string{"cdi.k8s.io/*"}, + Logger: logger, + Tree: cfg, + UseLegacyConfig: true, + RuntimeType: runtimeType, + ContainerAnnotations: []string{"cdi.k8s.io/*"}, } err = o.UpdateConfig(v1) @@ -476,9 +476,9 @@ func TestRevertV1Config(t *testing.T) { require.NoError(t, err, "%d: %v", i, tc) v1 := &containerd.ConfigV1{ - Tree: cfg, - UseDefaultRuntimeName: true, - RuntimeType: runtimeType, + Tree: cfg, + UseLegacyConfig: true, + RuntimeType: runtimeType, } err = o.RevertConfig(v1) diff --git a/tools/container/runtime/containerd/containerd.go b/tools/container/runtime/containerd/containerd.go index a53a0655..31fd2fea 100644 --- a/tools/container/runtime/containerd/containerd.go +++ b/tools/container/runtime/containerd/containerd.go @@ -52,8 +52,11 @@ type Options struct { func Flags(opts *Options) []cli.Flag { flags := []cli.Flag{ &cli.BoolFlag{ - Name: "use-legacy-config", - Usage: "Specify whether a legacy (pre v1.3) config should be used", + Name: "use-legacy-config", + Usage: "Specify whether a legacy (pre v1.3) config should be used. " + + "This ensures that a version 1 container config is created by default and that the " + + "containerd.runtimes.default_runtime config section is used to define the default " + + "runtime instead of container.default_runtime_name.", Destination: &opts.useLegacyConfig, EnvVars: []string{"CONTAINERD_USE_LEGACY_CONFIG"}, }, From 1dd66d009dfae9d2c148a1492429aeb1d253fb14 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 29 Nov 2024 15:20:14 +0100 Subject: [PATCH 2/4] Add string TOML source Signed-off-by: Evan Lezar --- pkg/config/engine/api.go | 7 ++--- .../engine/containerd/config_v1_test.go | 15 ++++++----- .../engine/containerd/config_v2_test.go | 25 +++++++++--------- pkg/config/engine/docker/docker.go | 10 +++++++ pkg/config/toml/source-string.go | 26 +++++++++++++++++++ pkg/config/toml/source.go | 9 +++++++ 6 files changed, 69 insertions(+), 23 deletions(-) create mode 100644 pkg/config/toml/source-string.go diff --git a/pkg/config/engine/api.go b/pkg/config/engine/api.go index bc6c4a68..8c7d1b50 100644 --- a/pkg/config/engine/api.go +++ b/pkg/config/engine/api.go @@ -18,12 +18,13 @@ package engine // Interface defines the API for a runtime config updater. type Interface interface { - DefaultRuntime() string AddRuntime(string, string, bool) error - Set(string, interface{}) + DefaultRuntime() string + GetRuntimeConfig(string) (RuntimeConfig, error) RemoveRuntime(string) error Save(string) (int64, error) - GetRuntimeConfig(string) (RuntimeConfig, error) + Set(string, interface{}) + String() string } // RuntimeConfig defines the interface to query container runtime handler configuration diff --git a/pkg/config/engine/containerd/config_v1_test.go b/pkg/config/engine/containerd/config_v1_test.go index 787ab30b..ca82032a 100644 --- a/pkg/config/engine/containerd/config_v1_test.go +++ b/pkg/config/engine/containerd/config_v1_test.go @@ -200,20 +200,21 @@ func TestAddRuntimeV1(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - cfg, err := toml.Load(tc.config) - require.NoError(t, err) expectedConfig, err := toml.Load(tc.expectedConfig) require.NoError(t, err) - c := &ConfigV1{ - Logger: logger, - Tree: cfg, - } + c, err := New( + WithLogger(logger), + WithConfigSource(toml.FromString(tc.config)), + WithUseLegacyConfig(true), + WithRuntimeType(""), + ) + require.NoError(t, err) err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault) require.NoError(t, err) - require.EqualValues(t, expectedConfig.String(), cfg.String()) + require.EqualValues(t, expectedConfig.String(), c.String()) }) } } diff --git a/pkg/config/engine/containerd/config_v2_test.go b/pkg/config/engine/containerd/config_v2_test.go index a9e0c3be..6304e210 100644 --- a/pkg/config/engine/containerd/config_v2_test.go +++ b/pkg/config/engine/containerd/config_v2_test.go @@ -46,7 +46,7 @@ func TestAddRuntime(t *testing.T) { privileged_without_host_devices = false runtime_engine = "" runtime_root = "" - runtime_type = "" + runtime_type = "io.containerd.runc.v2" [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test.options] BinaryName = "/usr/bin/test" `, @@ -199,20 +199,19 @@ func TestAddRuntime(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - cfg, err := toml.Load(tc.config) - require.NoError(t, err) expectedConfig, err := toml.Load(tc.expectedConfig) require.NoError(t, err) - c := &Config{ - Logger: logger, - Tree: cfg, - } + c, err := New( + WithLogger(logger), + WithConfigSource(toml.FromString(tc.config)), + ) + require.NoError(t, err) err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault) require.NoError(t, err) - require.EqualValues(t, expectedConfig.String(), cfg.String()) + require.EqualValues(t, expectedConfig.String(), c.String()) }) } } @@ -299,13 +298,13 @@ func TestGetRuntimeConfig(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - cfg, err := toml.Load(config) + + c, err := New( + WithLogger(logger), + WithConfigSource(toml.FromString(config)), + ) require.NoError(t, err) - c := &Config{ - Logger: logger, - Tree: cfg, - } rc, err := c.GetRuntimeConfig(tc.runtime) require.Equal(t, tc.expectedError, err) require.Equal(t, tc.expected, rc.GetBinaryPath()) diff --git a/pkg/config/engine/docker/docker.go b/pkg/config/engine/docker/docker.go index 6ea64f06..eda700d0 100644 --- a/pkg/config/engine/docker/docker.go +++ b/pkg/config/engine/docker/docker.go @@ -166,3 +166,13 @@ func (c *Config) GetRuntimeConfig(name string) (engine.RuntimeConfig, error) { } return &dockerRuntime{}, nil } + +// String returns the string representation of the JSON config. +func (c Config) String() string { + output, err := json.MarshalIndent(c, "", " ") + if err != nil { + return fmt.Sprintf("invalid JSON: %v", err) + } + + return string(output) +} diff --git a/pkg/config/toml/source-string.go b/pkg/config/toml/source-string.go new file mode 100644 index 00000000..72d4cc6d --- /dev/null +++ b/pkg/config/toml/source-string.go @@ -0,0 +1,26 @@ +/** +# Copyright 2024 NVIDIA CORPORATION +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package toml + +type tomlString string + +var _ Loader = (*tomlFile)(nil) + +// Load loads the contents of the specified TOML file as a map. +func (l tomlString) Load() (*Tree, error) { + return Load(string(l)) +} diff --git a/pkg/config/toml/source.go b/pkg/config/toml/source.go index 08907764..90cae913 100644 --- a/pkg/config/toml/source.go +++ b/pkg/config/toml/source.go @@ -45,3 +45,12 @@ func FromCommandLine(cmds ...string) Loader { args: cmds[1:], } } + +// FromString creates a TOML source for the specified contents. +// If an empty string is passed an empty toml config is used. +func FromString(contents string) Loader { + if contents == "" { + return Empty + } + return tomlString(contents) +} From 5d073fbdc37a125e674c3f7ca16960a2035380f1 Mon Sep 17 00:00:00 2001 From: Sam Lockart Date: Mon, 18 Nov 2024 16:20:55 +1100 Subject: [PATCH 3/4] Add support for containerd version 3 config This change adds support for containerd configs with version=3. From the perspective of the runtime configuration the contents of the config are the same. This means that we just have to load the new version and ensure that this is propagated to the generated config. Note that we still use a default config of version=2 since we need to ensure compatibility with older containerd versions (1.6.x and 1.7.x). Signed-off-by: Sam Lockart Signed-off-by: Evan Lezar --- .../containerd/{config_v2.go => config.go} | 2 +- .../engine/containerd/config_v2_test.go | 62 +++++++++++++ pkg/config/engine/containerd/containerd.go | 51 +++++----- pkg/config/engine/containerd/option.go | 16 ++-- pkg/config/toml/source-map.go | 26 ++++++ pkg/config/toml/source.go | 25 +++-- .../runtime/containerd/config_v1_test.go | 93 +++++++++---------- .../runtime/containerd/config_v2_test.go | 69 +++++++------- 8 files changed, 221 insertions(+), 123 deletions(-) rename pkg/config/engine/containerd/{config_v2.go => config.go} (99%) create mode 100644 pkg/config/toml/source-map.go diff --git a/pkg/config/engine/containerd/config_v2.go b/pkg/config/engine/containerd/config.go similarity index 99% rename from pkg/config/engine/containerd/config_v2.go rename to pkg/config/engine/containerd/config.go index 562ccdf8..8b76edbb 100644 --- a/pkg/config/engine/containerd/config_v2.go +++ b/pkg/config/engine/containerd/config.go @@ -30,7 +30,7 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error { } config := *c.Tree - config.Set("version", int64(2)) + config.Set("version", c.Version) runtimeNamesForConfig := engine.GetLowLevelRuntimes(c) for _, r := range runtimeNamesForConfig { diff --git a/pkg/config/engine/containerd/config_v2_test.go b/pkg/config/engine/containerd/config_v2_test.go index 6304e210..80c2675f 100644 --- a/pkg/config/engine/containerd/config_v2_test.go +++ b/pkg/config/engine/containerd/config_v2_test.go @@ -195,6 +195,68 @@ func TestAddRuntime(t *testing.T) { SystemdCgroup = false `, }, + { + description: "empty v3 spec is supported", + config: ` + version = 3 + `, + expectedConfig: ` + version = 3 + [plugins] + [plugins."io.containerd.grpc.v1.cri"] + [plugins."io.containerd.grpc.v1.cri".containerd] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test] + privileged_without_host_devices = false + runtime_engine = "" + runtime_root = "" + runtime_type = "io.containerd.runc.v2" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test.options] + BinaryName = "/usr/bin/test" + `, + expectedError: nil, + }, + { + description: "v3 spec is supported", + config: ` + version = 3 + [plugins] + [plugins."io.containerd.grpc.v1.cri"] + [plugins."io.containerd.grpc.v1.cri".containerd] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options] + BinaryName = "/usr/bin/runc" + SystemdCgroup = true + `, + expectedConfig: ` + version = 3 + [plugins] + [plugins."io.containerd.grpc.v1.cri"] + [plugins."io.containerd.grpc.v1.cri".containerd] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options] + BinaryName = "/usr/bin/runc" + SystemdCgroup = true + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test.options] + BinaryName = "/usr/bin/test" + SystemdCgroup = true + `, + }, } for _, tc := range testCases { diff --git a/pkg/config/engine/containerd/containerd.go b/pkg/config/engine/containerd/containerd.go index 128befb3..3e171018 100644 --- a/pkg/config/engine/containerd/containerd.go +++ b/pkg/config/engine/containerd/containerd.go @@ -24,9 +24,15 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/toml" ) +const ( + defaultConfigVersion = 2 + defaultRuntimeType = "io.containerd.runc.v2" +) + // Config represents the containerd config type Config struct { *toml.Tree + Version int64 Logger logger.Interface RuntimeType string ContainerAnnotations []string @@ -60,7 +66,8 @@ func (c *containerdCfgRuntime) GetBinaryPath() string { // New creates a containerd config with the specified options func New(opts ...Option) (engine.Interface, error) { b := &builder{ - runtimeType: defaultRuntimeType, + configVersion: defaultConfigVersion, + runtimeType: defaultRuntimeType, } for _, opt := range opts { opt(b) @@ -77,46 +84,46 @@ func New(opts ...Option) (engine.Interface, error) { return nil, fmt.Errorf("failed to load config: %v", err) } + configVersion, err := b.parseVersion(tomlConfig) + if err != nil { + return nil, fmt.Errorf("failed to parse config version: %w", err) + } + cfg := &Config{ Tree: tomlConfig, + Version: configVersion, Logger: b.logger, RuntimeType: b.runtimeType, - ContainerAnnotations: b.containerAnnotations, UseLegacyConfig: b.useLegacyConfig, + ContainerAnnotations: b.containerAnnotations, } - version, err := cfg.parseVersion() - if err != nil { - return nil, fmt.Errorf("failed to parse config version: %v", err) - } - switch version { + switch configVersion { case 1: return (*ConfigV1)(cfg), nil - case 2: + case 2, 3: return cfg, nil } - - return nil, fmt.Errorf("unsupported config version: %v", version) + return nil, fmt.Errorf("unsupported config version: %v", configVersion) } // parseVersion returns the version of the config -func (c *Config) parseVersion() (int, error) { - defaultVersion := 2 - // For legacy configs, we default to v1 configs. - if c.UseLegacyConfig { - defaultVersion = 1 +func (b *builder) parseVersion(c *toml.Tree) (int64, error) { + if c == nil || len(c.Keys()) == 0 { + // No config exists, or the config file is empty. + if b.useLegacyConfig { + // If a legacy config is explicitly requested, we default to a v1 config. + return 1, nil + } + // Use the requested version. + return int64(b.configVersion), nil } switch v := c.Get("version").(type) { case nil: - switch len(c.Keys()) { - case 0: // No config exists, or the config file is empty, use version inferred from containerd - return defaultVersion, nil - default: // A config file exists, has content, and no version is set - return 1, nil - } + return 1, nil case int64: - return int(v), nil + return v, nil default: return -1, fmt.Errorf("unsupported type for version field: %v", v) } diff --git a/pkg/config/engine/containerd/option.go b/pkg/config/engine/containerd/option.go index 6174a9ca..2dec62ef 100644 --- a/pkg/config/engine/containerd/option.go +++ b/pkg/config/engine/containerd/option.go @@ -21,16 +21,13 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/toml" ) -const ( - defaultRuntimeType = "io.containerd.runc.v2" -) - type builder struct { logger logger.Interface configSource toml.Loader + configVersion int + useLegacyConfig bool path string runtimeType string - useLegacyConfig bool containerAnnotations []string } @@ -65,13 +62,20 @@ func WithRuntimeType(runtimeType string) Option { } } -// WithUseLegacyConfig sets the useLegacyConfig flag for the config builder +// WithUseLegacyConfig sets the useLegacyConfig flag for the config builder. func WithUseLegacyConfig(useLegacyConfig bool) Option { return func(b *builder) { b.useLegacyConfig = useLegacyConfig } } +// WithConfigVersion sets the config version for the config builder +func WithConfigVersion(configVersion int) Option { + return func(b *builder) { + b.configVersion = configVersion + } +} + // WithContainerAnnotations sets the container annotations for the config builder func WithContainerAnnotations(containerAnnotations ...string) Option { return func(b *builder) { diff --git a/pkg/config/toml/source-map.go b/pkg/config/toml/source-map.go new file mode 100644 index 00000000..077bbb39 --- /dev/null +++ b/pkg/config/toml/source-map.go @@ -0,0 +1,26 @@ +/** +# Copyright 2024 NVIDIA CORPORATION +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package toml + +type tomlMap map[string]interface{} + +var _ Loader = (*tomlFile)(nil) + +// Load loads the contents of the specified TOML file as a map. +func (l tomlMap) Load() (*Tree, error) { + return LoadMap(l) +} diff --git a/pkg/config/toml/source.go b/pkg/config/toml/source.go index 90cae913..105a9743 100644 --- a/pkg/config/toml/source.go +++ b/pkg/config/toml/source.go @@ -25,6 +25,18 @@ type Loader interface { Load() (*Tree, error) } +// FromCommandLine creates a TOML source from the output of a shell command and its corresponding args. +// If the command is empty, an empty config is returned. +func FromCommandLine(cmds ...string) Loader { + if len(cmds) == 0 { + return Empty + } + return &tomlCliSource{ + command: cmds[0], + args: cmds[1:], + } +} + // FromFile creates a TOML source from the specified file. // If an empty string is passed an empty toml config is used. func FromFile(path string) Loader { @@ -34,16 +46,13 @@ func FromFile(path string) Loader { return tomlFile(path) } -// FromCommandLine creates a TOML source from the output of a shell command and its corresponding args. -// If the command is empty, an empty config is returned. -func FromCommandLine(cmds ...string) Loader { - if len(cmds) == 0 { +// FromMap creates a TOML source for the specified map. +// If an empty map is passed and empty tomly config is used. +func FromMap(m map[string]interface{}) Loader { + if m == nil { return Empty } - return &tomlCliSource{ - command: cmds[0], - args: cmds[1:], - } + return tomlMap(m) } // FromString creates a TOML source for the specified contents. diff --git a/tools/container/runtime/containerd/config_v1_test.go b/tools/container/runtime/containerd/config_v1_test.go index a3967833..7042744f 100644 --- a/tools/container/runtime/containerd/config_v1_test.go +++ b/tools/container/runtime/containerd/config_v1_test.go @@ -88,37 +88,36 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) { SetAsDefault: tc.setAsDefault, } - cfg, err := toml.Empty.Load() - require.NoError(t, err, "%d: %v", i, tc) - - v1 := &containerd.ConfigV1{ - Logger: logger, - Tree: cfg, - UseLegacyConfig: tc.legacyConfig, - RuntimeType: runtimeType, - } + v1, err := containerd.New( + containerd.WithLogger(logger), + containerd.WithConfigSource(toml.Empty), + containerd.WithRuntimeType(runtimeType), + containerd.WithUseLegacyConfig(tc.legacyConfig), + containerd.WithConfigVersion(1), + ) + require.NoError(t, err) err = o.UpdateConfig(v1) - require.NoError(t, err, "%d: %v", i, tc) + require.NoError(t, err) - defaultRuntimeName := v1.GetPath([]string{"plugins", "cri", "containerd", "default_runtime_name"}) - require.EqualValues(t, tc.expectedDefaultRuntimeName, defaultRuntimeName, "%d: %v", i, tc) + cfg := v1.(*containerd.ConfigV1) + defaultRuntimeName := cfg.GetPath([]string{"plugins", "cri", "containerd", "default_runtime_name"}) + require.EqualValues(t, tc.expectedDefaultRuntimeName, defaultRuntimeName) - defaultRuntime := v1.GetPath([]string{"plugins", "cri", "containerd", "default_runtime"}) + defaultRuntime := cfg.GetPath([]string{"plugins", "cri", "containerd", "default_runtime"}) if tc.expectedDefaultRuntimeBinary == nil { - require.Nil(t, defaultRuntime, "%d: %v", i, tc) + require.Nil(t, defaultRuntime) } else { require.NotNil(t, defaultRuntime) expected, err := defaultRuntimeTomlConfigV1(tc.expectedDefaultRuntimeBinary.(string)) - require.NoError(t, err, "%d: %v", i, tc) + require.NoError(t, err) configContents, _ := toml.Marshal(defaultRuntime.(*toml.Tree)) expectedContents, _ := toml.Marshal(expected) require.Equal(t, string(expectedContents), string(configContents), "%d: %v: %v", i, tc) } - }) } } @@ -234,24 +233,22 @@ func TestUpdateV1Config(t *testing.T) { RuntimeDir: runtimeDir, } - cfg, err := toml.Empty.Load() + v1, err := containerd.New( + containerd.WithLogger(logger), + containerd.WithConfigSource(toml.Empty), + containerd.WithRuntimeType(runtimeType), + containerd.WithConfigVersion(1), + containerd.WithContainerAnnotations("cdi.k8s.io/*"), + ) require.NoError(t, err) - v1 := &containerd.ConfigV1{ - Logger: logger, - Tree: cfg, - UseLegacyConfig: true, - RuntimeType: runtimeType, - ContainerAnnotations: []string{"cdi.k8s.io/*"}, - } - err = o.UpdateConfig(v1) require.NoError(t, err) expected, err := toml.TreeFromMap(tc.expectedConfig) require.NoError(t, err) - require.Equal(t, expected.String(), cfg.String()) + require.Equal(t, expected.String(), v1.String()) }) } } @@ -393,29 +390,28 @@ func TestUpdateV1ConfigWithRuncPresent(t *testing.T) { RuntimeDir: runtimeDir, } - cfg, err := toml.TreeFromMap(runcConfigMapV1("/runc-binary")) + v1, err := containerd.New( + containerd.WithLogger(logger), + containerd.WithConfigSource(toml.FromMap(runcConfigMapV1("/runc-binary"))), + containerd.WithRuntimeType(runtimeType), + containerd.WithConfigVersion(1), + containerd.WithContainerAnnotations("cdi.k8s.io/*"), + ) require.NoError(t, err) - v1 := &containerd.ConfigV1{ - Logger: logger, - Tree: cfg, - UseLegacyConfig: true, - RuntimeType: runtimeType, - ContainerAnnotations: []string{"cdi.k8s.io/*"}, - } - err = o.UpdateConfig(v1) require.NoError(t, err) expected, err := toml.TreeFromMap(tc.expectedConfig) require.NoError(t, err) - require.Equal(t, expected.String(), cfg.String()) + require.Equal(t, expected.String(), v1.String()) }) } } func TestRevertV1Config(t *testing.T) { + logger, _ := testlog.NewNullLogger() testCases := []struct { config map[string]interface { } @@ -469,25 +465,22 @@ func TestRevertV1Config(t *testing.T) { RuntimeName: "nvidia", } - cfg, err := toml.LoadMap(tc.config) - require.NoError(t, err, "%d: %v", i, tc) - expected, err := toml.TreeFromMap(tc.expected) - require.NoError(t, err, "%d: %v", i, tc) + require.NoError(t, err) - v1 := &containerd.ConfigV1{ - Tree: cfg, - UseLegacyConfig: true, - RuntimeType: runtimeType, - } + v1, err := containerd.New( + containerd.WithLogger(logger), + containerd.WithConfigSource(toml.FromMap(tc.config)), + containerd.WithRuntimeType(runtimeType), - err = o.RevertConfig(v1) - require.NoError(t, err, "%d: %v", i, tc) + containerd.WithContainerAnnotations("cdi.k8s.io/*"), + ) + require.NoError(t, err) - configContents, _ := toml.Marshal(cfg) - expectedContents, _ := toml.Marshal(expected) + err = o.RevertConfig(v1) + require.NoError(t, err) - require.Equal(t, string(expectedContents), string(configContents), "%d: %v", i, tc) + require.Equal(t, expected.String(), v1.String()) }) } } diff --git a/tools/container/runtime/containerd/config_v2_test.go b/tools/container/runtime/containerd/config_v2_test.go index 8c4620eb..92eea1fa 100644 --- a/tools/container/runtime/containerd/config_v2_test.go +++ b/tools/container/runtime/containerd/config_v2_test.go @@ -72,18 +72,19 @@ func TestUpdateV2ConfigDefaultRuntime(t *testing.T) { SetAsDefault: tc.setAsDefault, } - cfg, err := toml.LoadMap(map[string]interface{}{}) + v2, err := containerd.New( + containerd.WithLogger(logger), + containerd.WithConfigSource(toml.Empty), + containerd.WithRuntimeType(runtimeType), + containerd.WithContainerAnnotations("cdi.k8s.io/*"), + ) require.NoError(t, err) - v2 := &containerd.Config{ - Logger: logger, - Tree: cfg, - RuntimeType: runtimeType, - } - err = o.UpdateConfig(v2) require.NoError(t, err) + cfg := v2.(*containerd.Config) + defaultRuntimeName := cfg.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "default_runtime_name"}) require.EqualValues(t, tc.expectedDefaultRuntimeName, defaultRuntimeName) }) @@ -195,23 +196,21 @@ func TestUpdateV2Config(t *testing.T) { RuntimeDir: runtimeDir, } - cfg, err := toml.LoadMap(map[string]interface{}{}) + v2, err := containerd.New( + containerd.WithLogger(logger), + containerd.WithConfigSource(toml.Empty), + containerd.WithRuntimeType(runtimeType), + containerd.WithContainerAnnotations("cdi.k8s.io/*"), + ) require.NoError(t, err) - v2 := &containerd.Config{ - Logger: logger, - Tree: cfg, - RuntimeType: runtimeType, - ContainerAnnotations: []string{"cdi.k8s.io/*"}, - } - err = o.UpdateConfig(v2) require.NoError(t, err) expected, err := toml.TreeFromMap(tc.expectedConfig) require.NoError(t, err) - require.Equal(t, expected.String(), cfg.String()) + require.Equal(t, expected.String(), v2.String()) }) } @@ -348,28 +347,28 @@ func TestUpdateV2ConfigWithRuncPresent(t *testing.T) { RuntimeDir: runtimeDir, } - cfg, err := toml.LoadMap(runcConfigMapV2("/runc-binary")) + v2, err := containerd.New( + containerd.WithLogger(logger), + containerd.WithConfigSource(toml.FromMap(runcConfigMapV2("/runc-binary"))), + containerd.WithRuntimeType(runtimeType), + containerd.WithContainerAnnotations("cdi.k8s.io/*"), + ) require.NoError(t, err) - v2 := &containerd.Config{ - Logger: logger, - Tree: cfg, - RuntimeType: runtimeType, - ContainerAnnotations: []string{"cdi.k8s.io/*"}, - } - err = o.UpdateConfig(v2) require.NoError(t, err) expected, err := toml.TreeFromMap(tc.expectedConfig) require.NoError(t, err) - require.Equal(t, expected.String(), cfg.String()) + require.Equal(t, expected.String(), v2.String()) }) } } func TestRevertV2Config(t *testing.T) { + logger, _ := testlog.NewNullLogger() + testCases := []struct { config map[string]interface { } @@ -418,24 +417,21 @@ func TestRevertV2Config(t *testing.T) { RuntimeName: "nvidia", } - cfg, err := toml.LoadMap(tc.config) - require.NoError(t, err) - expected, err := toml.TreeFromMap(tc.expected) require.NoError(t, err) - v2 := &containerd.Config{ - Tree: cfg, - RuntimeType: runtimeType, - } + v2, err := containerd.New( + containerd.WithLogger(logger), + containerd.WithConfigSource(toml.FromMap(tc.config)), + containerd.WithRuntimeType(runtimeType), + containerd.WithContainerAnnotations("cdi.k8s.io/*"), + ) + require.NoError(t, err) err = o.RevertConfig(v2) require.NoError(t, err) - configContents, _ := toml.Marshal(cfg) - expectedContents, _ := toml.Marshal(expected) - - require.Equal(t, string(expectedContents), string(configContents)) + require.Equal(t, expected.String(), v2.String()) }) } } @@ -454,6 +450,7 @@ func runtimeMapV2(binary string) map[string]interface{} { func runcConfigMapV2(binary string) map[string]interface{} { return map[string]interface{}{ + "version": 2, "plugins": map[string]interface{}{ "io.containerd.grpc.v1.cri": map[string]interface{}{ "containerd": map[string]interface{}{ From 16b5376a907c35a32f012afd4ba371b327a70fc6 Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Wed, 18 Dec 2024 17:36:34 -0800 Subject: [PATCH 4/4] Use 'io.containerd.cri.v1.runtime' instead of 'io.containerd.grpc.v1.cri' in v3 containerd configs The fully qualified name of the containerd plugin for the CRI runtime service was changed to io.containerd.cri.v1.runtime in the v3 configuration. References: https://github.com/containerd/containerd/blob/v2.0.0/docs/PLUGINS.md https://github.com/containerd/containerd/issues/10132 Signed-off-by: Christopher Desiniotis --- pkg/config/engine/containerd/config.go | 34 +++++++++---------- .../engine/containerd/config_v2_test.go | 34 +++++++++---------- pkg/config/engine/containerd/containerd.go | 12 +++++-- 3 files changed, 44 insertions(+), 36 deletions(-) diff --git a/pkg/config/engine/containerd/config.go b/pkg/config/engine/containerd/config.go index 8b76edbb..52a336ba 100644 --- a/pkg/config/engine/containerd/config.go +++ b/pkg/config/engine/containerd/config.go @@ -34,36 +34,36 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error { runtimeNamesForConfig := engine.GetLowLevelRuntimes(c) for _, r := range runtimeNamesForConfig { - options := config.GetSubtreeByPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", r}) + options := config.GetSubtreeByPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", r}) if options == nil { continue } c.Logger.Debugf("using options from runtime %v: %v", r, options) - config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}, options.Copy()) + config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name}, options.Copy()) break } - if config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}) == nil { + if config.GetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name}) == nil { c.Logger.Warningf("could not infer options from runtimes %v; using defaults", runtimeNamesForConfig) - config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name, "runtime_type"}, c.RuntimeType) - config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name, "runtime_root"}, "") - config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name, "runtime_engine"}, "") - config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name, "privileged_without_host_devices"}, false) + config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "runtime_type"}, c.RuntimeType) + config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "runtime_root"}, "") + config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "runtime_engine"}, "") + config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "privileged_without_host_devices"}, false) } if len(c.ContainerAnnotations) > 0 { - annotations, err := c.getRuntimeAnnotations([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name, "container_annotations"}) + annotations, err := c.getRuntimeAnnotations([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "container_annotations"}) if err != nil { return err } annotations = append(c.ContainerAnnotations, annotations...) - config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name, "container_annotations"}, annotations) + config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "container_annotations"}, annotations) } - config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name, "options", "BinaryName"}, path) + config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "options", "BinaryName"}, path) if setAsDefault { - config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "default_runtime_name"}, name) + config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "default_runtime_name"}, name) } *c.Tree = config @@ -99,13 +99,13 @@ func (c *Config) getRuntimeAnnotations(path []string) ([]string, error) { // Set sets the specified containerd option. func (c *Config) Set(key string, value interface{}) { config := *c.Tree - config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", key}, value) + config.SetPath([]string{"plugins", c.CRIRuntimePluginName, key}, value) *c.Tree = config } // DefaultRuntime returns the default runtime for the cri-o config func (c Config) DefaultRuntime() string { - if runtime, ok := c.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "default_runtime_name"}).(string); ok { + if runtime, ok := c.GetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "default_runtime_name"}).(string); ok { return runtime } return "" @@ -119,14 +119,14 @@ func (c *Config) RemoveRuntime(name string) error { config := *c.Tree - config.DeletePath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}) - if runtime, ok := config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "default_runtime_name"}).(string); ok { + config.DeletePath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name}) + if runtime, ok := config.GetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "default_runtime_name"}).(string); ok { if runtime == name { - config.DeletePath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "default_runtime_name"}) + config.DeletePath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "default_runtime_name"}) } } - runtimePath := []string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name} + runtimePath := []string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name} for i := 0; i < len(runtimePath); i++ { if runtimes, ok := config.GetPath(runtimePath[:len(runtimePath)-i]).(*toml.Tree); ok { if len(runtimes.Keys()) == 0 { diff --git a/pkg/config/engine/containerd/config_v2_test.go b/pkg/config/engine/containerd/config_v2_test.go index 80c2675f..836d2b1d 100644 --- a/pkg/config/engine/containerd/config_v2_test.go +++ b/pkg/config/engine/containerd/config_v2_test.go @@ -203,15 +203,15 @@ func TestAddRuntime(t *testing.T) { expectedConfig: ` version = 3 [plugins] - [plugins."io.containerd.grpc.v1.cri"] - [plugins."io.containerd.grpc.v1.cri".containerd] - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test] + [plugins."io.containerd.cri.v1.runtime"] + [plugins."io.containerd.cri.v1.runtime".containerd] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes.test] privileged_without_host_devices = false runtime_engine = "" runtime_root = "" runtime_type = "io.containerd.runc.v2" - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test.options] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes.test.options] BinaryName = "/usr/bin/test" `, expectedError: nil, @@ -221,38 +221,38 @@ func TestAddRuntime(t *testing.T) { config: ` version = 3 [plugins] - [plugins."io.containerd.grpc.v1.cri"] - [plugins."io.containerd.grpc.v1.cri".containerd] - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc] + [plugins."io.containerd.cri.v1.runtime"] + [plugins."io.containerd.cri.v1.runtime".containerd] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes.runc] privileged_without_host_devices = true runtime_engine = "engine" runtime_root = "root" runtime_type = "type" - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes.runc.options] BinaryName = "/usr/bin/runc" SystemdCgroup = true `, expectedConfig: ` version = 3 [plugins] - [plugins."io.containerd.grpc.v1.cri"] - [plugins."io.containerd.grpc.v1.cri".containerd] - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc] + [plugins."io.containerd.cri.v1.runtime"] + [plugins."io.containerd.cri.v1.runtime".containerd] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes.runc] privileged_without_host_devices = true runtime_engine = "engine" runtime_root = "root" runtime_type = "type" - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes.runc.options] BinaryName = "/usr/bin/runc" SystemdCgroup = true - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes.test] privileged_without_host_devices = true runtime_engine = "engine" runtime_root = "root" runtime_type = "type" - [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test.options] + [plugins."io.containerd.cri.v1.runtime".containerd.runtimes.test.options] BinaryName = "/usr/bin/test" SystemdCgroup = true `, diff --git a/pkg/config/engine/containerd/containerd.go b/pkg/config/engine/containerd/containerd.go index 3e171018..767f0588 100644 --- a/pkg/config/engine/containerd/containerd.go +++ b/pkg/config/engine/containerd/containerd.go @@ -42,6 +42,10 @@ type Config struct { // This was deprecated in v1.4 in favour of containerd.default_runtime_name. // Support for this section has been removed in v2.0. UseLegacyConfig bool + // CRIRuntimePluginName represents the fully qualified name of the containerd plugin + // for the CRI runtime service. The name of this plugin was changed in v3 of the + // containerd configuration file. + CRIRuntimePluginName string } var _ engine.Interface = (*Config)(nil) @@ -101,7 +105,11 @@ func New(opts ...Option) (engine.Interface, error) { switch configVersion { case 1: return (*ConfigV1)(cfg), nil - case 2, 3: + case 2: + cfg.CRIRuntimePluginName = "io.containerd.grpc.v1.cri" + return cfg, nil + case 3: + cfg.CRIRuntimePluginName = "io.containerd.cri.v1.runtime" return cfg, nil } return nil, fmt.Errorf("unsupported config version: %v", configVersion) @@ -133,7 +141,7 @@ func (c *Config) GetRuntimeConfig(name string) (engine.RuntimeConfig, error) { if c == nil || c.Tree == nil { return nil, fmt.Errorf("config is nil") } - runtimeData := c.GetSubtreeByPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}) + runtimeData := c.GetSubtreeByPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name}) return &containerdCfgRuntime{ tree: runtimeData, }, nil