[nvidia-ctk-installer] remove default_runtime from cri-o config on cleanup

Signed-off-by: Christopher Desiniotis <[email protected]>

Author: cdesiniotis

cmd/nvidia-ctk-installer/container/container.go

@@ -65,7 +65,7 @@ func (o Options) Configure(cfg engine.Interface) error {
 	if err != nil {
 		return fmt.Errorf("unable to update config: %v", err)
 	}
-	return o.flush(cfg)
+	return o.Flush(cfg)
 }
 
 // Unconfigure removes the options from the specified config
@@ -75,7 +75,7 @@ func (o Options) Unconfigure(cfg engine.Interface) error {
 		return fmt.Errorf("unable to update config: %v", err)
 	}
 
-	if err := o.flush(cfg); err != nil {
+	if err := o.Flush(cfg); err != nil {
 		return err
 	}
 
@@ -93,8 +93,8 @@ func (o Options) Unconfigure(cfg engine.Interface) error {
 	return nil
 }
 
-// flush flushes the specified config to disk
-func (o Options) flush(cfg engine.Interface) error {
+// Flush flushes the specified config to disk
+func (o Options) Flush(cfg engine.Interface) error {
 	filepath := o.DropInConfig
 	if filepath == "" {
 		filepath = o.TopLevelConfigPath

cmd/nvidia-ctk-installer/container/runtime/crio/config_test.go

@@ -88,6 +88,30 @@ func TestCrioConfigLifecycle(t *testing.T) {
 			},
 			assertCleanupPostConditions: func(t *testing.T, co *container.Options, _ *Options) error {
 				require.NoFileExists(t, co.TopLevelConfigPath)
+				// drop-in file not removed on cleanup
+				actual, err := os.ReadFile(co.DropInConfig)
+				require.NoError(t, err)
+
+				expected := `
+[crio]
+
+  [crio.runtime]
+
+    [crio.runtime.runtimes]
+
+      [crio.runtime.runtimes.nvidia]
+        runtime_path = "/usr/bin/nvidia-container-runtime"
+        runtime_type = "oci"
+
+      [crio.runtime.runtimes.nvidia-cdi]
+        runtime_path = "/usr/bin/nvidia-container-runtime.cdi"
+        runtime_type = "oci"
+
+      [crio.runtime.runtimes.nvidia-legacy]
+        runtime_path = "/usr/bin/nvidia-container-runtime.legacy"
+        runtime_type = "oci"
+`
+				require.Equal(t, expected, string(actual))
 				return nil
 			},
 		},
@@ -200,6 +224,37 @@ signature_policy = "/etc/crio/policy.json"
 `
 				require.Equal(t, expectedTopLevel, string(actualTopLevel))
 
+				// drop-in file not removed on cleanup
+				require.FileExists(t, co.DropInConfig)
+				actual, err := os.ReadFile(co.DropInConfig)
+				require.NoError(t, err)
+
+				expected := `
+[crio]
+
+  [crio.runtime]
+
+    [crio.runtime.runtimes]
+
+      [crio.runtime.runtimes.nvidia]
+        monitor_path = "/usr/libexec/crio/conmon"
+        runtime_path = "/usr/bin/nvidia-container-runtime"
+        runtime_root = "/run/crun"
+        runtime_type = "oci"
+
+      [crio.runtime.runtimes.nvidia-cdi]
+        monitor_path = "/usr/libexec/crio/conmon"
+        runtime_path = "/usr/bin/nvidia-container-runtime.cdi"
+        runtime_root = "/run/crun"
+        runtime_type = "oci"
+
+      [crio.runtime.runtimes.nvidia-legacy]
+        monitor_path = "/usr/libexec/crio/conmon"
+        runtime_path = "/usr/bin/nvidia-container-runtime.legacy"
+        runtime_root = "/run/crun"
+        runtime_type = "oci"
+`
+				require.Equal(t, expected, string(actual))
 				return nil
 			},
 		},
@@ -309,8 +364,33 @@ runtime_type = "oci"
 
 				require.Equal(t, expectedTopLevel, string(actualTopLevel))
 
-				require.NoFileExists(t, co.DropInConfig)
+				// drop-in file not removed on cleanup
+				// default_runtime setting removed from drop-in
+				require.FileExists(t, co.DropInConfig)
+				actual, err := os.ReadFile(co.DropInConfig)
+				require.NoError(t, err)
+
+				expected := `
+[crio]
+
+  [crio.runtime]
+
+    [crio.runtime.runtimes]
 
+      [crio.runtime.runtimes.nvidia]
+        runtime_path = "/usr/bin/nvidia-container-runtime"
+        runtime_type = "oci"
+
+      [crio.runtime.runtimes.nvidia-cdi]
+        runtime_path = "/usr/bin/nvidia-container-runtime.cdi"
+        runtime_type = "oci"
+
+      [crio.runtime.runtimes.nvidia-legacy]
+        runtime_path = "/usr/bin/nvidia-container-runtime.legacy"
+        runtime_type = "oci"
+`
+
+				require.Equal(t, expected, string(actual))
 				return nil
 			},
 		},
@@ -477,6 +557,38 @@ plugin_dirs = [
 `
 				require.Equal(t, expected, string(actual))
 
+				// drop-in file not removed on cleanup
+				require.FileExists(t, co.DropInConfig)
+				actualDropIn, err := os.ReadFile(co.DropInConfig)
+				require.NoError(t, err)
+
+				expectedDropIn := `
+[crio]
+
+  [crio.runtime]
+
+    [crio.runtime.runtimes]
+
+      [crio.runtime.runtimes.nvidia]
+        monitor_path = "/usr/libexec/crio/conmon"
+        runtime_path = "/usr/bin/nvidia-container-runtime"
+        runtime_root = "/run/crun"
+        runtime_type = "oci"
+
+      [crio.runtime.runtimes.nvidia-cdi]
+        monitor_path = "/usr/libexec/crio/conmon"
+        runtime_path = "/usr/bin/nvidia-container-runtime.cdi"
+        runtime_root = "/run/crun"
+        runtime_type = "oci"
+
+      [crio.runtime.runtimes.nvidia-legacy]
+        monitor_path = "/usr/libexec/crio/conmon"
+        runtime_path = "/usr/bin/nvidia-container-runtime.legacy"
+        runtime_root = "/run/crun"
+        runtime_type = "oci"
+`
+				require.Equal(t, expectedDropIn, string(actualDropIn))
+
 				return nil
 			},
 		},

cmd/nvidia-ctk-installer/container/runtime/crio/crio.go

@@ -133,7 +133,7 @@ func setupHook(o *container.Options, co *Options) error {
 func setupConfig(o *container.Options) error {
 	log.Infof("Updating config file")
 
-	cfg, err := getRuntimeConfig(o)
+	cfg, err := getRuntimeConfig(o, false)
 	if err != nil {
 		return fmt.Errorf("unable to load config: %v", err)
 	}
@@ -186,14 +186,15 @@ func cleanupConfig(o *container.Options) error {
 
 	log.Infof("Reverting config file modifications")
 
-	cfg, err := getRuntimeConfig(o)
+	cfg, err := getRuntimeConfig(o, true)
 	if err != nil {
 		return fmt.Errorf("unable to load config: %v", err)
 	}
 
-	err = o.Unconfigure(cfg)
-	if err != nil {
-		return fmt.Errorf("unable to unconfigure cri-o: %v", err)
+	cfg.UnsetDefaultRuntime(o.RuntimeName)
+
+	if err := o.Flush(cfg); err != nil {
+		return err
 	}
 
 	err = RestartCrio(o)
@@ -210,24 +211,35 @@ func RestartCrio(o *container.Options) error {
 }
 
 func GetLowlevelRuntimePaths(o *container.Options) ([]string, error) {
-	cfg, err := getRuntimeConfig(o)
+	cfg, err := getRuntimeConfig(o, false)
 	if err != nil {
 		return nil, fmt.Errorf("unable to load crio config: %w", err)
 	}
 	return engine.GetBinaryPathsForRuntimes(cfg), nil
 }
 
-func getRuntimeConfig(o *container.Options) (engine.Interface, error) {
+func getRuntimeConfig(o *container.Options, loadDestinationConfig bool) (engine.Interface, error) {
 	loaders, err := o.GetConfigLoaders(crio.CommandLineSource)
 	if err != nil {
 		return nil, err
 	}
-	return crio.New(
+
+	options := []crio.Option{
 		crio.WithTopLevelConfigPath(o.TopLevelConfigPath),
 		crio.WithConfigSource(
 			toml.LoadFirst(
 				loaders...,
 			),
 		),
-	)
+	}
+
+	if loadDestinationConfig {
+		destinationConfigPath := o.TopLevelConfigPath
+		if o.DropInConfig != "" {
+			destinationConfigPath = o.DropInConfig
+		}
+		options = append(options, crio.WithConfigDestination(toml.FromFile(destinationConfigPath)))
+	}
+
+	return crio.New(options...)
 }

pkg/config/engine/api.go

@@ -29,6 +29,7 @@ type Interface interface {
 	EnableCDI()
 	GetRuntimeConfig(string) (RuntimeConfig, error)
 	RemoveRuntime(string) error
+	UnsetDefaultRuntime(string)
 	Save(string) (int64, error)
 	String() string
 }

pkg/config/engine/config.go

@@ -43,6 +43,7 @@ type RuntimeConfigDestination interface {
 	AddRuntimeWithOptions(string, string, bool, interface{}) error
 	EnableCDI()
 	RemoveRuntime(string) error
+	UnsetDefaultRuntime(string)
 	Save(string) (int64, error)
 	String() string
 }
@@ -60,6 +61,11 @@ func (c *Config) RemoveRuntime(runtime string) error {
 	return c.Destination.RemoveRuntime(runtime)
 }
 
+// UnsetDefaultRuntime removes the default runtime setting from the destination config.
+func (c *Config) UnsetDefaultRuntime(runtime string) {
+	c.Destination.UnsetDefaultRuntime(runtime)
+}
+
 // EnableCDI enables CDI in the destination config.
 func (c *Config) EnableCDI() {
 	c.Destination.EnableCDI()

pkg/config/engine/containerd/config.go

@@ -154,3 +154,17 @@ func (c *Config) RemoveRuntime(name string) error {
 	*c.Tree = config
 	return nil
 }
+
+func (c *Config) UnsetDefaultRuntime(name string) {
+	if c == nil || c.Tree == nil {
+		return
+	}
+
+	config := *c.Tree
+	if runtime, ok := config.GetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "default_runtime_name"}).(string); ok {
+		if runtime == name {
+			config.DeletePath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "default_runtime_name"})
+		}
+	}
+	*c.Tree = config
+}

pkg/config/engine/containerd/config_drop_in.go

@@ -96,6 +96,10 @@ func (c *ConfigWithDropIn) RemoveRuntime(name string) error {
 	return c.Interface.RemoveRuntime(name)
 }
 
+func (c *ConfigWithDropIn) UnsetDefaultRuntime(name string) {
+	c.Interface.UnsetDefaultRuntime(name)
+}
+
 // flush saves the top-level config to its path.
 // If the config is empty, the file will be deleted.
 func (c *topLevelConfig) Save(dropInPath string) (int64, error) {

pkg/config/engine/containerd/config_v1.go

@@ -120,6 +120,28 @@ func (c *ConfigV1) RemoveRuntime(name string) error {
 	return nil
 }
 
+func (c *ConfigV1) UnsetDefaultRuntime(name string) {
+	if c == nil || c.Tree == nil {
+		return
+	}
+
+	config := *c.Tree
+
+	runtimePath, ok := config.GetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "options", "BinaryName"}).(string)
+	if !ok || runtimePath == "" {
+		runtimePath, _ = config.GetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "options", "Runtime"}).(string)
+	}
+	defaultRuntimePath, ok := config.GetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "BinaryName"}).(string)
+	if !ok || defaultRuntimePath == "" {
+		defaultRuntimePath, _ = config.GetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "Runtime"}).(string)
+	}
+	if runtimePath != "" && defaultRuntimePath != "" && runtimePath == defaultRuntimePath {
+		config.DeletePath([]string{"plugins", "cri", "containerd", "default_runtime"})
+	}
+
+	*c.Tree = config
+}
+
 // Save writes the config to a file
 func (c ConfigV1) Save(path string) (int64, error) {
 	return (Config)(c).Save(path)

pkg/config/engine/crio/crio.go

@@ -67,13 +67,23 @@ func New(opts ...Option) (engine.Interface, error) {
 		return nil, err
 	}
 
+	var destinationConfig *toml.Tree
+	if b.configDestination != nil {
+		destinationConfig, err = b.configDestination.Load()
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		destinationConfig = toml.NewEmpty()
+	}
+
 	cfg := &engine.Config{
 		Source: &Config{
 			Tree:   sourceConfig,
 			Logger: b.logger,
 		},
 		Destination: &Config{
-			Tree:   toml.NewEmpty(),
+			Tree:   destinationConfig,
 			Logger: b.logger,
 		},
 	}
@@ -167,6 +177,22 @@ func (c *Config) RemoveRuntime(name string) error {
 	return nil
 }
 
+// UnsetDefaultRuntime ensures that the provided runtime name is not
+// configured as the default runtime in the config
+func (c *Config) UnsetDefaultRuntime(name string) {
+	if c == nil || c.Tree == nil {
+		return
+	}
+
+	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"})
+		}
+	}
+	*c.Tree = config
+}
+
 func (c *Config) GetRuntimeConfig(name string) (engine.RuntimeConfig, error) {
 	if c == nil || c.Tree == nil {
 		return nil, fmt.Errorf("config is nil")

pkg/config/engine/crio/option.go

@@ -24,6 +24,7 @@ import (
 type builder struct {
 	logger             logger.Interface
 	configSource       toml.Loader
+	configDestination  toml.Loader
 	topLevelConfigPath string
 }
 
@@ -50,3 +51,10 @@ func WithConfigSource(configSource toml.Loader) Option {
 		b.configSource = configSource
 	}
 }
+
+// WithConfigDestination sets the TOML destination for the config.
+func WithConfigDestination(configDestination toml.Loader) Option {
+	return func(b *builder) {
+		b.configDestination = configDestination
+	}
+}