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{}{