diff --git a/pkg/config/engine/containerd/config_v1.go b/pkg/config/engine/containerd/config_v1.go index 10e80b58..5522a227 100644 --- a/pkg/config/engine/containerd/config_v1.go +++ b/pkg/config/engine/containerd/config_v1.go @@ -39,12 +39,22 @@ func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool, confi config.Set("version", int64(1)) - if runc, ok := config.GetPath([]string{"plugins", "cri", "containerd", "runtimes", "runc"}).(*toml.Tree); ok { - runc, _ = toml.Load(runc.String()) - config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name}, runc) + // By default we extract the runtime options from the runc settings; if this does not exist we get the options from the default runtime specified in the config. + runtimeNamesForConfig := []string{"runc"} + if name, ok := config.GetPath([]string{"plugins", "cri", "containerd", "default_runtime_name"}).(string); ok && name != "" { + runtimeNamesForConfig = append(runtimeNamesForConfig, name) + } + for _, r := range runtimeNamesForConfig { + if options, ok := config.GetPath([]string{"plugins", "cri", "containerd", "runtimes", r}).(*toml.Tree); ok { + c.Logger.Debugf("using options from runtime %v: %v", r, options.String()) + options, _ = toml.Load(options.String()) + config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name}, options) + break + } } if config.GetPath([]string{"plugins", "cri", "containerd", "runtimes", name}) == nil { + c.Logger.Warningf("could not infer options from runtimes %v; using defaults", runtimeNamesForConfig) config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "runtime_type"}, c.RuntimeType) config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "runtime_root"}, "") config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "runtime_engine"}, "") diff --git a/pkg/config/engine/containerd/config_v1_test.go b/pkg/config/engine/containerd/config_v1_test.go new file mode 100644 index 00000000..774500d5 --- /dev/null +++ b/pkg/config/engine/containerd/config_v1_test.go @@ -0,0 +1,245 @@ +/** +# 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 containerd + +import ( + "testing" + + "github.com/pelletier/go-toml" + testlog "github.com/sirupsen/logrus/hooks/test" + "github.com/stretchr/testify/require" +) + +func TestAddRuntimeV1(t *testing.T) { + logger, _ := testlog.NewNullLogger() + testCases := []struct { + description string + config string + setAsDefault bool + configOverrides []map[string]interface{} + expectedConfig string + expectedError error + }{ + { + description: "empty config not default runtime", + expectedConfig: ` + version = 1 + [plugins] + [plugins.cri] + [plugins.cri.containerd] + [plugins.cri.containerd.runtimes] + [plugins.cri.containerd.runtimes.test] + privileged_without_host_devices = false + runtime_engine = "" + runtime_root = "" + runtime_type = "" + [plugins.cri.containerd.runtimes.test.options] + BinaryName = "/usr/bin/test" + Runtime = "/usr/bin/test" + `, + expectedError: nil, + }, + { + description: "empty config not default runtime with overrides", + configOverrides: []map[string]interface{}{ + { + "options": map[string]interface{}{ + "SystemdCgroup": true, + }, + }, + }, + expectedConfig: ` + version = 1 + [plugins] + [plugins.cri] + [plugins.cri.containerd] + [plugins.cri.containerd.runtimes] + [plugins.cri.containerd.runtimes.test] + privileged_without_host_devices = false + runtime_engine = "" + runtime_root = "" + runtime_type = "" + [plugins.cri.containerd.runtimes.test.options] + BinaryName = "/usr/bin/test" + Runtime = "/usr/bin/test" + SystemdCgroup = true + `, + }, + { + description: "options from runc are imported", + config: ` + [plugins] + [plugins.cri] + [plugins.cri.containerd] + [plugins.cri.containerd.runtimes] + [plugins.cri.containerd.runtimes.runc] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins.cri.containerd.runtimes.runc.options] + BinaryName = "/usr/bin/runc" + SystemdCgroup = true + `, + expectedConfig: ` + version = 1 + [plugins] + [plugins.cri] + [plugins.cri.containerd] + [plugins.cri.containerd.runtimes] + [plugins.cri.containerd.runtimes.runc] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins.cri.containerd.runtimes.runc.options] + BinaryName = "/usr/bin/runc" + SystemdCgroup = true + [plugins.cri.containerd.runtimes.test] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins.cri.containerd.runtimes.test.options] + BinaryName = "/usr/bin/test" + Runtime = "/usr/bin/test" + SystemdCgroup = true + `, + }, + { + description: "options from default runtime are imported", + config: ` + [plugins] + [plugins.cri] + [plugins.cri.containerd] + default_runtime_name = "default" + [plugins.cri.containerd.runtimes] + [plugins.cri.containerd.runtimes.default] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins.cri.containerd.runtimes.default.options] + BinaryName = "/usr/bin/default" + SystemdCgroup = true + `, + expectedConfig: ` + version = 1 + [plugins] + [plugins.cri] + [plugins.cri.containerd] + default_runtime_name = "default" + [plugins.cri.containerd.runtimes] + [plugins.cri.containerd.runtimes.default] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins.cri.containerd.runtimes.default.options] + BinaryName = "/usr/bin/default" + SystemdCgroup = true + [plugins.cri.containerd.runtimes.test] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins.cri.containerd.runtimes.test.options] + BinaryName = "/usr/bin/test" + Runtime = "/usr/bin/test" + SystemdCgroup = true + `, + }, + { + description: "options from runc take precedence over default runtime", + config: ` + [plugins] + [plugins.cri] + [plugins.cri.containerd] + default_runtime_name = "default" + [plugins.cri.containerd.runtimes] + [plugins.cri.containerd.runtimes.runc] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins.cri.containerd.runtimes.runc.options] + BinaryName = "/usr/bin/runc" + SystemdCgroup = true + [plugins.cri.containerd.runtimes.default] + privileged_without_host_devices = false + runtime_engine = "defaultengine" + runtime_root = "defaultroot" + runtime_type = "defaulttype" + [plugins.cri.containerd.runtimes.default.options] + BinaryName = "/usr/bin/default" + SystemdCgroup = false + `, + expectedConfig: ` + version = 1 + [plugins] + [plugins.cri] + [plugins.cri.containerd] + default_runtime_name = "default" + [plugins.cri.containerd.runtimes] + [plugins.cri.containerd.runtimes.runc] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins.cri.containerd.runtimes.runc.options] + BinaryName = "/usr/bin/runc" + SystemdCgroup = true + [plugins.cri.containerd.runtimes.default] + privileged_without_host_devices = false + runtime_engine = "defaultengine" + runtime_root = "defaultroot" + runtime_type = "defaulttype" + [plugins.cri.containerd.runtimes.default.options] + BinaryName = "/usr/bin/default" + SystemdCgroup = false + [plugins.cri.containerd.runtimes.test] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins.cri.containerd.runtimes.test.options] + BinaryName = "/usr/bin/test" + Runtime = "/usr/bin/test" + SystemdCgroup = true + `, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + config, err := toml.Load(tc.config) + require.NoError(t, err) + expectedConfig, err := toml.Load(tc.expectedConfig) + require.NoError(t, err) + + c := &ConfigV1{ + Logger: logger, + Tree: config, + } + + err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault, tc.configOverrides...) + require.NoError(t, err) + + require.EqualValues(t, expectedConfig.String(), config.String()) + }) + } +} diff --git a/pkg/config/engine/containerd/config_v2.go b/pkg/config/engine/containerd/config_v2.go index e8818988..ee9d33f7 100644 --- a/pkg/config/engine/containerd/config_v2.go +++ b/pkg/config/engine/containerd/config_v2.go @@ -33,12 +33,22 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool, configO config.Set("version", int64(2)) - if runc, ok := config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", "runc"}).(*toml.Tree); ok { - runc, _ = toml.Load(runc.String()) - config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}, runc) + // By default we extract the runtime options from the runc settings; if this does not exist we get the options from the default runtime specified in the config. + runtimeNamesForConfig := []string{"runc"} + if name, ok := config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "default_runtime_name"}).(string); ok && name != "" { + runtimeNamesForConfig = append(runtimeNamesForConfig, name) + } + for _, r := range runtimeNamesForConfig { + if options, ok := config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", r}).(*toml.Tree); ok { + c.Logger.Debugf("using options from runtime %v: %v", r, options.String()) + options, _ = toml.Load(options.String()) + config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}, options) + break + } } if config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "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"}, "") diff --git a/pkg/config/engine/containerd/config_v2_test.go b/pkg/config/engine/containerd/config_v2_test.go index 1ab6cc1e..598222f8 100644 --- a/pkg/config/engine/containerd/config_v2_test.go +++ b/pkg/config/engine/containerd/config_v2_test.go @@ -20,10 +20,12 @@ import ( "testing" "github.com/pelletier/go-toml" + testlog "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" ) func TestAddRuntime(t *testing.T) { + logger, _ := testlog.NewNullLogger() testCases := []struct { description string config string @@ -75,6 +77,149 @@ func TestAddRuntime(t *testing.T) { SystemdCgroup = true `, }, + { + description: "options from runc are imported", + config: ` + version = 2 + [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 = 2 + [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 + `, + }, + { + description: "options from default runtime are imported", + config: ` + version = 2 + [plugins] + [plugins."io.containerd.grpc.v1.cri"] + [plugins."io.containerd.grpc.v1.cri".containerd] + default_runtime_name = "default" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.default] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.default.options] + BinaryName = "/usr/bin/default" + SystemdCgroup = true + `, + expectedConfig: ` + version = 2 + [plugins] + [plugins."io.containerd.grpc.v1.cri"] + [plugins."io.containerd.grpc.v1.cri".containerd] + default_runtime_name = "default" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.default] + privileged_without_host_devices = true + runtime_engine = "engine" + runtime_root = "root" + runtime_type = "type" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.default.options] + BinaryName = "/usr/bin/default" + 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 + `, + }, + { + description: "options from runc take precedence over default runtime", + config: ` + version = 2 + [plugins] + [plugins."io.containerd.grpc.v1.cri"] + [plugins."io.containerd.grpc.v1.cri".containerd] + default_runtime_name = "default" + [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.default] + privileged_without_host_devices = false + runtime_engine = "defaultengine" + runtime_root = "defaultroot" + runtime_type = "defaulttype" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.default.options] + BinaryName = "/usr/bin/default" + SystemdCgroup = false + `, + expectedConfig: ` + version = 2 + [plugins] + [plugins."io.containerd.grpc.v1.cri"] + [plugins."io.containerd.grpc.v1.cri".containerd] + default_runtime_name = "default" + [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.default] + privileged_without_host_devices = false + runtime_engine = "defaultengine" + runtime_root = "defaultroot" + runtime_type = "defaulttype" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.default.options] + BinaryName = "/usr/bin/default" + SystemdCgroup = false + [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 { @@ -85,7 +230,8 @@ func TestAddRuntime(t *testing.T) { require.NoError(t, err) c := &Config{ - Tree: config, + Logger: logger, + Tree: config, } err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault, tc.configOverrides...) diff --git a/pkg/config/engine/containerd/containerd.go b/pkg/config/engine/containerd/containerd.go index 26ae7746..3bdc27ae 100644 --- a/pkg/config/engine/containerd/containerd.go +++ b/pkg/config/engine/containerd/containerd.go @@ -26,6 +26,7 @@ import ( // Config represents the containerd config type Config struct { *toml.Tree + Logger logger.Interface RuntimeType string UseDefaultRuntimeName bool ContainerAnnotations []string diff --git a/pkg/config/engine/containerd/option.go b/pkg/config/engine/containerd/option.go index 5a3ba0e1..ae1d9944 100644 --- a/pkg/config/engine/containerd/option.go +++ b/pkg/config/engine/containerd/option.go @@ -89,6 +89,7 @@ func (b *builder) build() (engine.Interface, error) { if err != nil { return nil, fmt.Errorf("failed to load config: %v", err) } + config.Logger = b.logger config.RuntimeType = b.runtimeType config.UseDefaultRuntimeName = !b.useLegacyConfig config.ContainerAnnotations = b.containerAnnotations diff --git a/pkg/config/engine/crio/crio.go b/pkg/config/engine/crio/crio.go index 4fa9795e..47320d15 100644 --- a/pkg/config/engine/crio/crio.go +++ b/pkg/config/engine/crio/crio.go @@ -21,11 +21,15 @@ import ( "github.com/pelletier/go-toml" + "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine" ) // Config represents the cri-o config -type Config toml.Tree +type Config struct { + *toml.Tree + Logger logger.Interface +} var _ engine.Interface = (*Config)(nil) @@ -45,11 +49,20 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool, _ ...ma return fmt.Errorf("config is nil") } - config := (toml.Tree)(*c) + config := *c.Tree - if runc, ok := config.Get("crio.runtime.runtimes.runc").(*toml.Tree); ok { - runc, _ = toml.Load(runc.String()) - config.SetPath([]string{"crio", "runtime", "runtimes", name}, runc) + // By default we extract the runtime options from the runc settings; if this does not exist we get the options from the default runtime specified in the config. + runtimeNamesForConfig := []string{"runc"} + if name, ok := config.GetPath([]string{"crio", "runtime", "default_runtime"}).(string); ok && name != "" { + runtimeNamesForConfig = append(runtimeNamesForConfig, name) + } + for _, r := range runtimeNamesForConfig { + if options, ok := config.GetPath([]string{"crio", "runtime", "runtimes", r}).(*toml.Tree); ok { + c.Logger.Debugf("using options from runtime %v: %v", r, options.String()) + options, _ = toml.Load(options.String()) + config.SetPath([]string{"crio", "runtime", "runtimes", name}, options) + break + } } config.SetPath([]string{"crio", "runtime", "runtimes", name, "runtime_path"}, path) @@ -59,14 +72,16 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool, _ ...ma config.SetPath([]string{"crio", "runtime", "default_runtime"}, name) } - *c = (Config)(config) + *c.Tree = config return nil } // DefaultRuntime returns the default runtime for the cri-o config -func (c Config) DefaultRuntime() string { - config := (toml.Tree)(c) - if runtime, ok := config.GetPath([]string{"crio", "runtime", "default_runtime"}).(string); ok { +func (c *Config) DefaultRuntime() string { + if c == nil || c.Tree == nil { + return "" + } + if runtime, ok := c.GetPath([]string{"crio", "runtime", "default_runtime"}).(string); ok { return runtime } return "" @@ -78,7 +93,7 @@ func (c *Config) RemoveRuntime(name string) error { return nil } - config := (toml.Tree)(*c) + config := *c.Tree if runtime, ok := config.GetPath([]string{"crio", "runtime", "default_runtime"}).(string); ok { if runtime == name { config.DeletePath([]string{"crio", "runtime", "default_runtime"}) @@ -97,20 +112,20 @@ func (c *Config) RemoveRuntime(name string) error { } } - *c = (Config)(config) + *c.Tree = config return nil } // Set sets the specified cri-o option. func (c *Config) Set(key string, value interface{}) { - config := (toml.Tree)(*c) + config := *c.Tree config.Set(key, value) - *c = (Config)(config) + *c.Tree = config } // Save writes the config to the specified path func (c Config) Save(path string) (int64, error) { - config := (toml.Tree)(c) + config := c.Tree output, err := config.Marshal() if err != nil { return 0, fmt.Errorf("unable to convert to TOML: %v", err) diff --git a/pkg/config/engine/crio/crio_test.go b/pkg/config/engine/crio/crio_test.go new file mode 100644 index 00000000..a43d3a94 --- /dev/null +++ b/pkg/config/engine/crio/crio_test.go @@ -0,0 +1,146 @@ +/** +# 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 crio + +import ( + "testing" + + "github.com/pelletier/go-toml" + testlog "github.com/sirupsen/logrus/hooks/test" + "github.com/stretchr/testify/require" +) + +func TestAddRuntime(t *testing.T) { + logger, _ := testlog.NewNullLogger() + testCases := []struct { + description string + config string + setAsDefault bool + configOverrides []map[string]interface{} + expectedConfig string + expectedError error + }{ + { + description: "empty config not default runtime", + expectedConfig: ` + [crio] + [crio.runtime.runtimes.test] + runtime_path = "/usr/bin/test" + runtime_type = "oci" + `, + expectedError: nil, + }, + { + description: "options from runc are imported", + config: ` + [crio] + [crio.runtime.runtimes.runc] + runtime_path = "/usr/bin/runc" + runtime_type = "runcoci" + runc_option = "option" + `, + expectedConfig: ` + [crio] + [crio.runtime.runtimes.runc] + runtime_path = "/usr/bin/runc" + runtime_type = "runcoci" + runc_option = "option" + [crio.runtime.runtimes.test] + runtime_path = "/usr/bin/test" + runtime_type = "oci" + runc_option = "option" + `, + }, + { + description: "options from default runtime are imported", + config: ` + [crio] + [crio.runtime] + default_runtime = "default" + [crio.runtime.runtimes.default] + runtime_path = "/usr/bin/default" + runtime_type = "defaultoci" + default_option = "option" + `, + expectedConfig: ` + [crio] + [crio.runtime] + default_runtime = "default" + [crio.runtime.runtimes.default] + runtime_path = "/usr/bin/default" + runtime_type = "defaultoci" + default_option = "option" + [crio.runtime.runtimes.test] + runtime_path = "/usr/bin/test" + runtime_type = "oci" + default_option = "option" + `, + }, + { + description: "options from runc take precedence over default runtime", + config: ` + [crio] + [crio.runtime] + default_runtime = "default" + [crio.runtime.runtimes.default] + runtime_path = "/usr/bin/default" + runtime_type = "defaultoci" + default_option = "option" + [crio.runtime.runtimes.runc] + runtime_path = "/usr/bin/runc" + runtime_type = "runcoci" + runc_option = "option" + `, + expectedConfig: ` + [crio] + [crio.runtime] + default_runtime = "default" + [crio.runtime.runtimes.default] + runtime_path = "/usr/bin/default" + runtime_type = "defaultoci" + default_option = "option" + [crio.runtime.runtimes.runc] + runtime_path = "/usr/bin/runc" + runtime_type = "runcoci" + runc_option = "option" + [crio.runtime.runtimes.test] + runtime_path = "/usr/bin/test" + runtime_type = "oci" + runc_option = "option" + `, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + config, err := toml.Load(tc.config) + require.NoError(t, err) + expectedConfig, err := toml.Load(tc.expectedConfig) + require.NoError(t, err) + + c := &Config{ + Logger: logger, + Tree: config, + } + + err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault, tc.configOverrides...) + require.NoError(t, err) + + require.EqualValues(t, expectedConfig.String(), config.String()) + }) + } +} diff --git a/pkg/config/engine/crio/option.go b/pkg/config/engine/crio/option.go index b457ffa8..9f4d10fc 100644 --- a/pkg/config/engine/crio/option.go +++ b/pkg/config/engine/crio/option.go @@ -48,13 +48,17 @@ func WithPath(path string) Option { } func (b *builder) build() (*Config, error) { - if b.path == "" { - empty := toml.Tree{} - return (*Config)(&empty), nil - } if b.logger == nil { b.logger = logger.New() } + if b.path == "" { + empty := toml.Tree{} + c := Config{ + Tree: &empty, + Logger: b.logger, + } + return &c, nil + } return b.loadConfig(b.path) } @@ -82,5 +86,9 @@ func (b *builder) loadConfig(config string) (*Config, error) { b.logger.Infof("Successfully loaded config") - return (*Config)(cfg), nil + c := Config{ + Tree: cfg, + Logger: b.logger, + } + return &c, nil } diff --git a/tools/container/containerd/config_v1_test.go b/tools/container/containerd/config_v1_test.go index 81c4ff59..49a4bc2f 100644 --- a/tools/container/containerd/config_v1_test.go +++ b/tools/container/containerd/config_v1_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/pelletier/go-toml" + testlog "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/containerd" @@ -28,6 +29,7 @@ import ( ) func TestUpdateV1ConfigDefaultRuntime(t *testing.T) { + logger, _ := testlog.NewNullLogger() const runtimeDir = "/test/runtime/dir" testCases := []struct { @@ -94,6 +96,7 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) { require.NoError(t, err, "%d: %v", i, tc) v1 := &containerd.ConfigV1{ + Logger: logger, Tree: config, UseDefaultRuntimeName: !tc.legacyConfig, RuntimeType: runtimeType, @@ -125,6 +128,7 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) { } func TestUpdateV1Config(t *testing.T) { + logger, _ := testlog.NewNullLogger() const runtimeDir = "/test/runtime/dir" testCases := []struct { @@ -240,6 +244,7 @@ func TestUpdateV1Config(t *testing.T) { require.NoError(t, err) v1 := &containerd.ConfigV1{ + Logger: logger, Tree: config, UseDefaultRuntimeName: true, RuntimeType: runtimeType, @@ -258,6 +263,7 @@ func TestUpdateV1Config(t *testing.T) { } func TestUpdateV1ConfigWithRuncPresent(t *testing.T) { + logger, _ := testlog.NewNullLogger() const runtimeDir = "/test/runtime/dir" testCases := []struct { @@ -399,6 +405,7 @@ func TestUpdateV1ConfigWithRuncPresent(t *testing.T) { require.NoError(t, err) v1 := &containerd.ConfigV1{ + Logger: logger, Tree: config, UseDefaultRuntimeName: true, RuntimeType: runtimeType, diff --git a/tools/container/containerd/config_v2_test.go b/tools/container/containerd/config_v2_test.go index dd65f253..ce4a58be 100644 --- a/tools/container/containerd/config_v2_test.go +++ b/tools/container/containerd/config_v2_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/pelletier/go-toml" + testlog "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/containerd" @@ -32,6 +33,7 @@ const ( ) func TestUpdateV2ConfigDefaultRuntime(t *testing.T) { + logger, _ := testlog.NewNullLogger() const runtimeDir = "/test/runtime/dir" testCases := []struct { @@ -76,6 +78,7 @@ func TestUpdateV2ConfigDefaultRuntime(t *testing.T) { require.NoError(t, err) v2 := &containerd.Config{ + Logger: logger, Tree: config, RuntimeType: runtimeType, } @@ -90,6 +93,7 @@ func TestUpdateV2ConfigDefaultRuntime(t *testing.T) { } func TestUpdateV2Config(t *testing.T) { + logger, _ := testlog.NewNullLogger() const runtimeDir = "/test/runtime/dir" testCases := []struct { @@ -200,8 +204,9 @@ func TestUpdateV2Config(t *testing.T) { require.NoError(t, err) v2 := &containerd.Config{ + Logger: logger, Tree: config, - RuntimeType: runtimeType, + RuntimeType: o.runtimeType, ContainerAnnotations: []string{"cdi.k8s.io/*"}, } @@ -218,6 +223,7 @@ func TestUpdateV2Config(t *testing.T) { } func TestUpdateV2ConfigWithRuncPresent(t *testing.T) { + logger, _ := testlog.NewNullLogger() const runtimeDir = "/test/runtime/dir" testCases := []struct { @@ -353,6 +359,7 @@ func TestUpdateV2ConfigWithRuncPresent(t *testing.T) { require.NoError(t, err) v2 := &containerd.Config{ + Logger: logger, Tree: config, RuntimeType: runtimeType, ContainerAnnotations: []string{"cdi.k8s.io/*"},