From a9b01a43bc12a15d11315ec14c8385f6fdde24a6 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 31 Oct 2023 15:24:27 +0100 Subject: [PATCH 1/2] Add cdi.enabled option to runtime configure Signed-off-by: Evan Lezar --- CHANGELOG.md | 1 + cmd/nvidia-ctk/runtime/configure/configure.go | 23 +++++++++++++++++++ pkg/config/engine/api.go | 1 + pkg/config/engine/containerd/config_v1.go | 8 +++++++ pkg/config/engine/containerd/config_v2.go | 8 +++++++ pkg/config/engine/crio/crio.go | 5 ++++ pkg/config/engine/docker/docker.go | 5 ++++ 7 files changed, 51 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a5fab66..6212856c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Added detection of libnvdxgdmal.so.1 on WSL2. * Fix bug in determining default nvidia-container-runtime.user config value on SUSE-based systems. * Add `crun` to the list of configured low-level runtimes. +* Add `--cdi.enabled` option to `nvidia-ctk runtime configure` command to enable CDI in containerd. * [toolkit-container] Bump CUDA base image version to 12.3.1. * [libnvidia-container] Added detection of libnvdxgdmal.so.1 on WSL2. diff --git a/cmd/nvidia-ctk/runtime/configure/configure.go b/cmd/nvidia-ctk/runtime/configure/configure.go index 90af0cb6..11f37387 100644 --- a/cmd/nvidia-ctk/runtime/configure/configure.go +++ b/cmd/nvidia-ctk/runtime/configure/configure.go @@ -71,6 +71,11 @@ type config struct { hookPath string setAsDefault bool } + + // cdi-specific options + cdi struct { + enabled bool + } } func (m command) build() *cli.Command { @@ -141,6 +146,11 @@ func (m command) build() *cli.Command { Usage: "set the NVIDIA runtime as the default runtime", Destination: &config.nvidiaRuntime.setAsDefault, }, + &cli.BoolFlag{ + Name: "cdi.enabled", + Usage: "Enable CDI in the configured runtime", + Destination: &config.cdi.enabled, + }, } return &configure @@ -175,6 +185,13 @@ func (m command) validateFlags(c *cli.Context, config *config) error { } } + if config.runtime != "containerd" { + if config.cdi.enabled { + m.logger.Warningf("Ignoring cdi.enabled flag for %v", config.runtime) + } + config.cdi.enabled = false + } + return nil } @@ -227,6 +244,12 @@ func (m command) configureConfigFile(c *cli.Context, config *config) error { return fmt.Errorf("unable to update config: %v", err) } + if config.cdi.enabled { + if err := cfg.Set("enable_cdi", true); err != nil { + return fmt.Errorf("failed enable CDI in containerd: %w", err) + } + } + outputPath := config.getOuputConfigPath() n, err := cfg.Save(outputPath) if err != nil { diff --git a/pkg/config/engine/api.go b/pkg/config/engine/api.go index 5cd5fb52..3e6d1035 100644 --- a/pkg/config/engine/api.go +++ b/pkg/config/engine/api.go @@ -20,6 +20,7 @@ package engine type Interface interface { DefaultRuntime() string AddRuntime(string, string, bool) error + Set(string, interface{}) error RemoveRuntime(string) error Save(string) (int64, error) } diff --git a/pkg/config/engine/containerd/config_v1.go b/pkg/config/engine/containerd/config_v1.go index b432f60b..5e4099df 100644 --- a/pkg/config/engine/containerd/config_v1.go +++ b/pkg/config/engine/containerd/config_v1.go @@ -134,6 +134,14 @@ func (c *ConfigV1) RemoveRuntime(name string) error { return nil } +// SetOption sets the specified containerd option. +func (c *ConfigV1) Set(key string, value interface{}) error { + config := *c.Tree + config.SetPath([]string{"plugins", "cri", "containerd", key}, value) + *c.Tree = config + return nil +} + // Save wrotes the config to a file func (c ConfigV1) Save(path string) (int64, error) { return (Config)(c).Save(path) diff --git a/pkg/config/engine/containerd/config_v2.go b/pkg/config/engine/containerd/config_v2.go index 3fcdb9c5..a6cbc2e8 100644 --- a/pkg/config/engine/containerd/config_v2.go +++ b/pkg/config/engine/containerd/config_v2.go @@ -90,6 +90,14 @@ func (c *Config) getRuntimeAnnotations(path []string) ([]string, error) { return annotations, nil } +// Set sets the specified containerd option. +func (c *Config) Set(key string, value interface{}) error { + config := *c.Tree + config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", key}, value) + *c.Tree = config + return nil +} + // 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 { diff --git a/pkg/config/engine/crio/crio.go b/pkg/config/engine/crio/crio.go index 03db8bff..8be6ccba 100644 --- a/pkg/config/engine/crio/crio.go +++ b/pkg/config/engine/crio/crio.go @@ -99,6 +99,11 @@ func (c *Config) RemoveRuntime(name string) error { return nil } +// Set is not supported for cri-o configs. +func (c *Config) Set(key string, value interface{}) error { + return fmt.Errorf("Set is not supported for crio configs") +} + // Save writes the config to the specified path func (c Config) Save(path string) (int64, error) { config := (toml.Tree)(c) diff --git a/pkg/config/engine/docker/docker.go b/pkg/config/engine/docker/docker.go index 92594cc5..965dc2e4 100644 --- a/pkg/config/engine/docker/docker.go +++ b/pkg/config/engine/docker/docker.go @@ -114,6 +114,11 @@ func (c *Config) RemoveRuntime(name string) error { return nil } +// Set is not supported for docker configs. +func (c *Config) Set(key string, value interface{}) error { + return fmt.Errorf("Set is not supported for crio configs") +} + // Save writes the config to the specified path func (c Config) Save(path string) (int64, error) { output, err := json.MarshalIndent(c, "", " ") From 7ff23999e87db57a38165e0e234982b734e92b8c Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Mon, 20 Nov 2023 14:55:21 -0800 Subject: [PATCH 2/2] Add option to nvidia-ctk to enable CDI in docker Signed-off-by: Christopher Desiniotis Signed-off-by: Evan Lezar --- CHANGELOG.md | 1 + cmd/nvidia-ctk/runtime/configure/configure.go | 26 ++++++++++++++----- pkg/config/engine/docker/docker.go | 5 ++-- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6212856c..7d182aa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ * Fix bug in determining default nvidia-container-runtime.user config value on SUSE-based systems. * Add `crun` to the list of configured low-level runtimes. * Add `--cdi.enabled` option to `nvidia-ctk runtime configure` command to enable CDI in containerd. +* Added support for `nvidia-ctk runtime configure --enable-cdi` for the `docker` runtime. Note that this requires Docker >= 25. * [toolkit-container] Bump CUDA base image version to 12.3.1. * [libnvidia-container] Added detection of libnvdxgdmal.so.1 on WSL2. diff --git a/cmd/nvidia-ctk/runtime/configure/configure.go b/cmd/nvidia-ctk/runtime/configure/configure.go index 11f37387..145fef0e 100644 --- a/cmd/nvidia-ctk/runtime/configure/configure.go +++ b/cmd/nvidia-ctk/runtime/configure/configure.go @@ -20,13 +20,14 @@ import ( "fmt" "path/filepath" + "github.com/urfave/cli/v2" + "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine" "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/containerd" "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/crio" "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/docker" "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/ocihook" - "github.com/urfave/cli/v2" ) const ( @@ -185,7 +186,7 @@ func (m command) validateFlags(c *cli.Context, config *config) error { } } - if config.runtime != "containerd" { + if config.runtime != "containerd" && config.runtime != "docker" { if config.cdi.enabled { m.logger.Warningf("Ignoring cdi.enabled flag for %v", config.runtime) } @@ -244,10 +245,9 @@ func (m command) configureConfigFile(c *cli.Context, config *config) error { return fmt.Errorf("unable to update config: %v", err) } - if config.cdi.enabled { - if err := cfg.Set("enable_cdi", true); err != nil { - return fmt.Errorf("failed enable CDI in containerd: %w", err) - } + err = enableCDI(config, cfg) + if err != nil { + return fmt.Errorf("failed to enable CDI in %s: %w", config.runtime, err) } outputPath := config.getOuputConfigPath() @@ -300,3 +300,17 @@ func (m *command) configureOCIHook(c *cli.Context, config *config) error { } return nil } + +// enableCDI enables the use of CDI in the corresponding container engine +func enableCDI(config *config, cfg engine.Interface) error { + if !config.cdi.enabled { + return nil + } + switch config.runtime { + case "containerd": + return cfg.Set("enable_cdi", true) + case "docker": + return cfg.Set("experimental", true) + } + return fmt.Errorf("enabling CDI in %s is not supported", config.runtime) +} diff --git a/pkg/config/engine/docker/docker.go b/pkg/config/engine/docker/docker.go index 965dc2e4..a6828528 100644 --- a/pkg/config/engine/docker/docker.go +++ b/pkg/config/engine/docker/docker.go @@ -114,9 +114,10 @@ func (c *Config) RemoveRuntime(name string) error { return nil } -// Set is not supported for docker configs. +// Set sets the specified docker option func (c *Config) Set(key string, value interface{}) error { - return fmt.Errorf("Set is not supported for crio configs") + (*c)[key] = value + return nil } // Save writes the config to the specified path