Merge pull request #576 from elezar/get-options-from-default

Extract options from default runtime if runc does not exist

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 {
+	// 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.
-		runc, _ = toml.Load(runc.String())
+	runtimeNamesForConfig := []string{"runc"}
-		config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name}, 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"}, "")

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())
+		})
+	}
+}

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 {
+	// 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.
-		runc, _ = toml.Load(runc.String())
+	runtimeNamesForConfig := []string{"runc"}
-		config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}, 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"}, "")

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...)

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

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

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 {
+	// 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.
-		runc, _ = toml.Load(runc.String())
+	runtimeNamesForConfig := []string{"runc"}
-		config.SetPath([]string{"crio", "runtime", "runtimes", name}, 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 {
+func (c *Config) DefaultRuntime() string {
-	config := (toml.Tree)(c)
+	if c == nil || c.Tree == nil {
-	if runtime, ok := config.GetPath([]string{"crio", "runtime", "default_runtime"}).(string); ok {
+		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)

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())
+		})
+	}
+}

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
 }

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,

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/*"},