From f1e201d3685fc494f25cf71329752e4c31768f96 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 23 Mar 2023 14:50:48 +0200 Subject: [PATCH 1/2] Refactor runtime configure cli Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/runtime/configure/configure.go | 183 +++++++++--------- cmd/nvidia-ctk/runtime/nvidia/nvidia.go | 75 ------- .../config/engine/containerd/config_v2.go | 5 + internal/config/engine/crio/crio.go | 5 + internal/config/engine/docker/docker.go | 5 + 5 files changed, 107 insertions(+), 166 deletions(-) delete mode 100644 cmd/nvidia-ctk/runtime/nvidia/nvidia.go diff --git a/cmd/nvidia-ctk/runtime/configure/configure.go b/cmd/nvidia-ctk/runtime/configure/configure.go index 21954b30..a25d4b2d 100644 --- a/cmd/nvidia-ctk/runtime/configure/configure.go +++ b/cmd/nvidia-ctk/runtime/configure/configure.go @@ -17,14 +17,12 @@ package configure import ( - "encoding/json" "fmt" - "os" + "path/filepath" - "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-ctk/runtime/nvidia" + "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine/crio" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine/docker" - "github.com/pelletier/go-toml" "github.com/sirupsen/logrus" "github.com/urfave/cli/v2" ) @@ -32,8 +30,14 @@ import ( const ( defaultRuntime = "docker" - defaultDockerConfigFilePath = "/etc/docker/daemon.json" + // defaultNVIDIARuntimeName is the default name to use in configs for the NVIDIA Container Runtime + defaultNVIDIARuntimeName = "nvidia" + // defaultNVIDIARuntimeExecutable is the default NVIDIA Container Runtime executable file name + defaultNVIDIARuntimeExecutable = "nvidia-container-runtime" + defailtNVIDIARuntimeExpecutablePath = "/usr/bin/nvidia-container-runtime" + defaultCrioConfigFilePath = "/etc/crio/crio.conf" + defaultDockerConfigFilePath = "/etc/docker/daemon.json" ) type command struct { @@ -54,7 +58,12 @@ type config struct { dryRun bool runtime string configFilePath string - nvidiaOptions nvidia.Options + + nvidiaRuntime struct { + name string + path string + setAsDefault bool + } } func (m command) build() *cli.Command { @@ -65,6 +74,9 @@ func (m command) build() *cli.Command { configure := cli.Command{ Name: "configure", Usage: "Add a runtime to the specified container engine", + Before: func(c *cli.Context) error { + return validateFlags(c, &config) + }, Action: func(c *cli.Context) error { return m.configureWrapper(c, &config) }, @@ -78,7 +90,7 @@ func (m command) build() *cli.Command { }, &cli.StringFlag{ Name: "runtime", - Usage: "the target runtime engine. One of [crio, docker]", + Usage: "the target runtime engine; one of [crio, docker]", Value: defaultRuntime, Destination: &config.runtime, }, @@ -90,124 +102,113 @@ func (m command) build() *cli.Command { &cli.StringFlag{ Name: "nvidia-runtime-name", Usage: "specify the name of the NVIDIA runtime that will be added", - Value: nvidia.RuntimeName, - Destination: &config.nvidiaOptions.RuntimeName, + Value: defaultNVIDIARuntimeName, + Destination: &config.nvidiaRuntime.name, }, &cli.StringFlag{ - Name: "runtime-path", + Name: "nvidia-runtime-path", + Aliases: []string{"runtime-path"}, Usage: "specify the path to the NVIDIA runtime executable", - Value: nvidia.RuntimeExecutable, - Destination: &config.nvidiaOptions.RuntimePath, + Value: defaultNVIDIARuntimeExecutable, + Destination: &config.nvidiaRuntime.path, }, &cli.BoolFlag{ - Name: "set-as-default", - Usage: "set the specified runtime as the default runtime", - Destination: &config.nvidiaOptions.SetAsDefault, + Name: "nvidia-set-as-default", + Aliases: []string{"set-as-default"}, + Usage: "set the NVIDIA runtime as the default runtime", + Destination: &config.nvidiaRuntime.setAsDefault, }, } return &configure } -func (m command) configureWrapper(c *cli.Context, config *config) error { +func validateFlags(c *cli.Context, config *config) error { + switch config.runtime { + case "crio", "docker": + break + default: + return fmt.Errorf("unrecognized runtime '%v'", config.runtime) + } + switch config.runtime { case "crio": - return m.configureCrio(c, config) + if config.nvidiaRuntime.path == defaultNVIDIARuntimeExecutable { + config.nvidiaRuntime.path = defailtNVIDIARuntimeExpecutablePath + } + if !filepath.IsAbs(config.nvidiaRuntime.path) { + return fmt.Errorf("the NVIDIA runtime path %q is not an absolute path", config.nvidiaRuntime.path) + } + } + + return nil +} + +// configureWrapper updates the specified container engine config to enable the NVIDIA runtime +func (m command) configureWrapper(c *cli.Context, config *config) error { + configFilePath := config.resolveConfigFilePath() + + var cfg engine.Interface + var err error + switch config.runtime { + case "crio": + cfg, err = crio.New( + crio.WithPath(configFilePath), + ) case "docker": - return m.configureDocker(c, config) + cfg, err = docker.New( + docker.WithPath(configFilePath), + ) + default: + err = fmt.Errorf("unrecognized runtime '%v'", config.runtime) } - - return fmt.Errorf("unrecognized runtime '%v'", config.runtime) -} - -// configureDocker updates the docker config to enable the NVIDIA Container Runtime -func (m command) configureDocker(c *cli.Context, config *config) error { - configFilePath := config.configFilePath - if configFilePath == "" { - configFilePath = defaultDockerConfigFilePath - } - - cfg, err := docker.New( - docker.WithPath(configFilePath), - ) - if err != nil { - return fmt.Errorf("unable to load config: %v", err) + if err != nil || cfg == nil { + return fmt.Errorf("unable to load config for runtime %v: %v", config.runtime, err) } err = cfg.AddRuntime( - config.nvidiaOptions.RuntimeName, - config.nvidiaOptions.RuntimePath, - config.nvidiaOptions.SetAsDefault, + config.nvidiaRuntime.name, + config.nvidiaRuntime.path, + config.nvidiaRuntime.setAsDefault, ) if err != nil { return fmt.Errorf("unable to update config: %v", err) } - if config.dryRun { - output, err := json.MarshalIndent(cfg, "", " ") - if err != nil { - return fmt.Errorf("unable to convert to JSON: %v", err) - } - os.Stdout.WriteString(fmt.Sprintf("%s\n", output)) - return nil - } - n, err := cfg.Save(configFilePath) + outputPath := config.getOuputConfigPath() + n, err := cfg.Save(outputPath) if err != nil { return fmt.Errorf("unable to flush config: %v", err) } if n == 0 { - m.logger.Infof("Removed empty config from %v", configFilePath) + m.logger.Infof("Removed empty config from %v", outputPath) } else { - m.logger.Infof("Wrote updated config to %v", configFilePath) + m.logger.Infof("Wrote updated config to %v", outputPath) } - m.logger.Infof("It is recommended that the docker daemon be restarted.") + m.logger.Infof("It is recommended that %v daemon be restarted.", config.runtime) return nil } -// configureCrio updates the crio config to enable the NVIDIA Container Runtime -func (m command) configureCrio(c *cli.Context, config *config) error { - configFilePath := config.configFilePath - if configFilePath == "" { - configFilePath = defaultCrioConfigFilePath +// resolveConfigFilePath returns the default config file path for the configured container engine +func (c *config) resolveConfigFilePath() string { + if c.configFilePath != "" { + return c.configFilePath } - - cfg, err := crio.New( - crio.WithPath(configFilePath), - ) - if err != nil { - return fmt.Errorf("unable to load config: %v", err) + switch c.runtime { + case "crio": + return defaultCrioConfigFilePath + case "docker": + return defaultDockerConfigFilePath } - - err = cfg.AddRuntime( - config.nvidiaOptions.RuntimeName, - config.nvidiaOptions.RuntimePath, - config.nvidiaOptions.SetAsDefault, - ) - if err != nil { - return fmt.Errorf("unable to update config: %v", err) - } - - if config.dryRun { - output, err := toml.Marshal(cfg) - if err != nil { - return fmt.Errorf("unable to convert to TOML: %v", err) - } - os.Stdout.WriteString(fmt.Sprintf("%s\n", output)) - return nil - } - n, err := cfg.Save(configFilePath) - if err != nil { - return fmt.Errorf("unable to flush config: %v", err) - } - - if n == 0 { - m.logger.Infof("Removed empty config from %v", configFilePath) - } else { - m.logger.Infof("Wrote updated config to %v", configFilePath) - } - m.logger.Infof("It is recommended that the cri-o daemon be restarted.") - - return nil + return "" +} + +// getOuputConfigPath returns the configured config path or "" if dry-run is enabled +func (c *config) getOuputConfigPath() string { + if c.dryRun { + return "" + } + return c.resolveConfigFilePath() } diff --git a/cmd/nvidia-ctk/runtime/nvidia/nvidia.go b/cmd/nvidia-ctk/runtime/nvidia/nvidia.go deleted file mode 100644 index 1f425146..00000000 --- a/cmd/nvidia-ctk/runtime/nvidia/nvidia.go +++ /dev/null @@ -1,75 +0,0 @@ -/* -# Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved. -# -# 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 nvidia - -const ( - // RuntimeName is the default name to use in configs for the NVIDIA Container Runtime - RuntimeName = "nvidia" - // RuntimeExecutable is the default NVIDIA Container Runtime executable file name - RuntimeExecutable = "nvidia-container-runtime" -) - -// Options specifies the options for the NVIDIA Container Runtime w.r.t a container engine such as docker. -type Options struct { - SetAsDefault bool - RuntimeName string - RuntimePath string -} - -// Runtime defines an NVIDIA runtime with a name and a executable -type Runtime struct { - Name string - Path string -} - -// DefaultRuntime returns the default runtime for the configured options. -// If the configuration is invalid or the default runtimes should not be set -// the empty string is returned. -func (o Options) DefaultRuntime() string { - if !o.SetAsDefault { - return "" - } - - return o.RuntimeName -} - -// Runtime creates a runtime struct based on the options. -func (o Options) Runtime() Runtime { - path := o.RuntimePath - - if o.RuntimePath == "" { - path = RuntimeExecutable - } - - r := Runtime{ - Name: o.RuntimeName, - Path: path, - } - - return r -} - -// DockerRuntimesConfig generatest the expected docker config for the specified runtime -func (r Runtime) DockerRuntimesConfig() map[string]interface{} { - runtimes := make(map[string]interface{}) - runtimes[r.Name] = map[string]interface{}{ - "path": r.Path, - "args": []string{}, - } - - return runtimes -} diff --git a/internal/config/engine/containerd/config_v2.go b/internal/config/engine/containerd/config_v2.go index fa1287fa..bd9abada 100644 --- a/internal/config/engine/containerd/config_v2.go +++ b/internal/config/engine/containerd/config_v2.go @@ -138,6 +138,11 @@ func (c Config) Save(path string) (int64, error) { return 0, fmt.Errorf("unable to convert to TOML: %v", err) } + if path == "" { + os.Stdout.WriteString(fmt.Sprintf("%s\n", output)) + return int64(len(output)), nil + } + if len(output) == 0 { err := os.Remove(path) if err != nil { diff --git a/internal/config/engine/crio/crio.go b/internal/config/engine/crio/crio.go index 59b066a0..c9b72af7 100644 --- a/internal/config/engine/crio/crio.go +++ b/internal/config/engine/crio/crio.go @@ -108,6 +108,11 @@ func (c Config) Save(path string) (int64, error) { return 0, fmt.Errorf("unable to convert to TOML: %v", err) } + if path == "" { + os.Stdout.WriteString(fmt.Sprintf("%s\n", output)) + return int64(len(output)), nil + } + if len(output) == 0 { err := os.Remove(path) if err != nil { diff --git a/internal/config/engine/docker/docker.go b/internal/config/engine/docker/docker.go index 3c998375..12130f53 100644 --- a/internal/config/engine/docker/docker.go +++ b/internal/config/engine/docker/docker.go @@ -117,6 +117,11 @@ func (c Config) Save(path string) (int64, error) { return 0, fmt.Errorf("unable to convert to JSON: %v", err) } + if path == "" { + os.Stdout.WriteString(fmt.Sprintf("%s\n", output)) + return int64(len(output)), nil + } + if len(output) == 0 { err := os.Remove(path) if err != nil { From 70920d7a04817199fa9f9703d9c24606ee688662 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 23 Mar 2023 14:57:18 +0200 Subject: [PATCH 2/2] Add support for containerd to the runtime configure CLI Signed-off-by: Evan Lezar --- CHANGELOG.md | 2 ++ cmd/nvidia-ctk/runtime/configure/configure.go | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 472a74e1..f5b7dbff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## v1.14.0-rc.1 +* Add support for updating containerd configs to the `nvidia-ctk runtime configure` command. + ## v1.13.1 * Update `update-ldcache` hook to only update ldcache if it exists. diff --git a/cmd/nvidia-ctk/runtime/configure/configure.go b/cmd/nvidia-ctk/runtime/configure/configure.go index a25d4b2d..bcc41953 100644 --- a/cmd/nvidia-ctk/runtime/configure/configure.go +++ b/cmd/nvidia-ctk/runtime/configure/configure.go @@ -21,6 +21,7 @@ import ( "path/filepath" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine" + "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine/containerd" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine/crio" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine/docker" "github.com/sirupsen/logrus" @@ -36,8 +37,9 @@ const ( defaultNVIDIARuntimeExecutable = "nvidia-container-runtime" defailtNVIDIARuntimeExpecutablePath = "/usr/bin/nvidia-container-runtime" - defaultCrioConfigFilePath = "/etc/crio/crio.conf" - defaultDockerConfigFilePath = "/etc/docker/daemon.json" + defaultContainerdConfigFilePath = "/etc/containerd/config.toml" + defaultCrioConfigFilePath = "/etc/crio/crio.conf" + defaultDockerConfigFilePath = "/etc/docker/daemon.json" ) type command struct { @@ -90,7 +92,7 @@ func (m command) build() *cli.Command { }, &cli.StringFlag{ Name: "runtime", - Usage: "the target runtime engine; one of [crio, docker]", + Usage: "the target runtime engine; one of [containerd, crio, docker]", Value: defaultRuntime, Destination: &config.runtime, }, @@ -125,14 +127,14 @@ func (m command) build() *cli.Command { func validateFlags(c *cli.Context, config *config) error { switch config.runtime { - case "crio", "docker": + case "containerd", "crio", "docker": break default: return fmt.Errorf("unrecognized runtime '%v'", config.runtime) } switch config.runtime { - case "crio": + case "containerd", "crio": if config.nvidiaRuntime.path == defaultNVIDIARuntimeExecutable { config.nvidiaRuntime.path = defailtNVIDIARuntimeExpecutablePath } @@ -151,6 +153,10 @@ func (m command) configureWrapper(c *cli.Context, config *config) error { var cfg engine.Interface var err error switch config.runtime { + case "containerd": + cfg, err = containerd.New( + containerd.WithPath(configFilePath), + ) case "crio": cfg, err = crio.New( crio.WithPath(configFilePath), @@ -197,6 +203,8 @@ func (c *config) resolveConfigFilePath() string { return c.configFilePath } switch c.runtime { + case "containerd": + return defaultContainerdConfigFilePath case "crio": return defaultCrioConfigFilePath case "docker":