From 47b6b01f48468cfebc6e3cc595cef54e1b730bd4 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 24 Mar 2023 11:54:58 +0200 Subject: [PATCH 1/9] Allow same envars for all runtime configs Signed-off-by: Evan Lezar --- tools/container/containerd/containerd.go | 56 ++++++++-------- tools/container/crio/crio.go | 85 +++++++++++++----------- tools/container/docker/docker.go | 70 ++++++++++--------- 3 files changed, 112 insertions(+), 99 deletions(-) diff --git a/tools/container/containerd/containerd.go b/tools/container/containerd/containerd.go index 011a27d9..34387e9a 100644 --- a/tools/container/containerd/containerd.go +++ b/tools/container/containerd/containerd.go @@ -116,51 +116,40 @@ func main() { commonFlags := []cli.Flag{ &cli.StringFlag{ Name: "config", - Aliases: []string{"c"}, Usage: "Path to the containerd config file", Value: defaultConfig, Destination: &options.config, - EnvVars: []string{"CONTAINERD_CONFIG"}, + EnvVars: []string{"CONTAINERD_CONFIG", "RUNTIME_CONFIG"}, }, &cli.StringFlag{ Name: "socket", - Aliases: []string{"s"}, Usage: "Path to the containerd socket file", Value: defaultSocket, Destination: &options.socket, - EnvVars: []string{"CONTAINERD_SOCKET"}, - }, - &cli.StringFlag{ - Name: "runtime-class", - Aliases: []string{"r"}, - Usage: "The name of the runtime class to set for the nvidia-container-runtime", - Value: defaultRuntimeClass, - Destination: &options.runtimeClass, - EnvVars: []string{"CONTAINERD_RUNTIME_CLASS"}, - }, - &cli.StringFlag{ - Name: "runtime-type", - Usage: "The runtime_type to use for the configured runtime classes", - Value: defaultRuntmeType, - Destination: &options.runtimeType, - EnvVars: []string{"CONTAINERD_RUNTIME_TYPE"}, - }, - // The flags below are only used by the 'setup' command. - &cli.BoolFlag{ - Name: "set-as-default", - Aliases: []string{"d"}, - Usage: "Set nvidia-container-runtime as the default runtime", - Value: defaultSetAsDefault, - Destination: &options.setAsDefault, - EnvVars: []string{"CONTAINERD_SET_AS_DEFAULT"}, - Hidden: true, + EnvVars: []string{"CONTAINERD_SOCKET", "RUNTIME_SOCKET"}, }, &cli.StringFlag{ Name: "restart-mode", Usage: "Specify how containerd should be restarted; If 'none' is selected, it will not be restarted [signal | systemd | none]", Value: defaultRestartMode, Destination: &options.restartMode, - EnvVars: []string{"CONTAINERD_RESTART_MODE"}, + EnvVars: []string{"CONTAINERD_RESTART_MODE", "RUNTIME_RESTART_MODE"}, + }, + &cli.StringFlag{ + Name: "runtime-class", + Aliases: []string{"runtime-name", "nvidia-runtime-name"}, + Usage: "The name of the runtime class to set for the nvidia-container-runtime", + Value: defaultRuntimeClass, + Destination: &options.runtimeClass, + EnvVars: []string{"CONTAINERD_RUNTIME_CLASS", "NVIDIA_RUNTIME_NAME"}, + }, + &cli.BoolFlag{ + Name: "set-as-default", + Usage: "Set nvidia-container-runtime as the default runtime", + Value: defaultSetAsDefault, + Destination: &options.setAsDefault, + EnvVars: []string{"CONTAINERD_SET_AS_DEFAULT", "NVIDIA_RUNTIME_SET_AS_DEFAULT"}, + Hidden: true, }, &cli.StringFlag{ Name: "host-root", @@ -175,6 +164,13 @@ func main() { Destination: &options.useLegacyConfig, EnvVars: []string{"CONTAINERD_USE_LEGACY_CONFIG"}, }, + &cli.StringFlag{ + Name: "runtime-type", + Usage: "The runtime_type to use for the configured runtime classes", + Value: defaultRuntmeType, + Destination: &options.runtimeType, + EnvVars: []string{"CONTAINERD_RUNTIME_TYPE"}, + }, &cli.StringSliceFlag{ Name: "nvidia-container-runtime-modes.cdi.annotation-prefixes", Destination: &options.ContainerRuntimeModesCDIAnnotationPrefixes, diff --git a/tools/container/crio/crio.go b/tools/container/crio/crio.go index 233a26fc..2525dc44 100644 --- a/tools/container/crio/crio.go +++ b/tools/container/crio/crio.go @@ -43,6 +43,7 @@ const ( // Config-based settings defaultConfig = "/etc/crio/crio.conf" + defaultSocket = "/var/run/crio/crio.sock" defaultRuntimeClass = "nvidia" defaultSetAsDefault = true defaultRestartMode = restartModeSystemd @@ -58,6 +59,7 @@ type options struct { runtimeDir string config string + socket string runtimeClass string setAsDefault bool restartMode string @@ -104,9 +106,53 @@ func main() { // only require the user to specify one set of flags for both 'startup' // and 'cleanup' to simplify things. commonFlags := []cli.Flag{ + &cli.StringFlag{ + Name: "config", + Usage: "Path to the cri-o config file", + Value: defaultConfig, + Destination: &options.config, + EnvVars: []string{"CRIO_CONFIG", "RUNTIME_CONFIG"}, + }, + &cli.StringFlag{ + Name: "socket", + Usage: "Path to the crio socket file", + Value: "", + Destination: &options.socket, + EnvVars: []string{"CRIO_SOCKET", "RUNTIME_SOCKET"}, + Hidden: true, + }, + &cli.StringFlag{ + Name: "restart-mode", + Usage: "Specify how cri-o should be restarted; If 'none' is selected, it will not be restarted [systemd | none]", + Value: defaultRestartMode, + Destination: &options.restartMode, + EnvVars: []string{"CRIO_RESTART_MODE", "RUNTIME_RESTART_MODE"}, + }, + &cli.StringFlag{ + Name: "runtime-class", + Aliases: []string{"runtime-name", "nvidia-runtime-name"}, + Usage: "The name of the runtime class to set for the nvidia-container-runtime", + Value: defaultRuntimeClass, + Destination: &options.runtimeClass, + EnvVars: []string{"CRIO_RUNTIME_CLASS", "NVIDIA_RUNTIME_NAME"}, + }, + &cli.BoolFlag{ + Name: "set-as-default", + Usage: "Set nvidia-container-runtime as the default runtime", + Value: defaultSetAsDefault, + Destination: &options.setAsDefault, + EnvVars: []string{"CRIO_SET_AS_DEFAULT", "NVIDIA_RUNTIME_SET_AS_DEFAULT"}, + Hidden: true, + }, + &cli.StringFlag{ + Name: "host-root", + Usage: "Specify the path to the host root to be used when restarting crio using systemd", + Value: defaultHostRootMount, + Destination: &options.hostRootMount, + EnvVars: []string{"HOST_ROOT_MOUNT"}, + }, &cli.StringFlag{ Name: "hooks-dir", - Aliases: []string{"d"}, Usage: "path to the cri-o hooks directory", Value: defaultHooksDir, Destination: &options.hooksDir, @@ -115,7 +161,6 @@ func main() { }, &cli.StringFlag{ Name: "hook-filename", - Aliases: []string{"f"}, Usage: "filename of the cri-o hook that will be created / removed in the hooks directory", Value: defaultHookFilename, Destination: &options.hookFilename, @@ -129,43 +174,7 @@ func main() { Destination: &options.configMode, EnvVars: []string{"CRIO_CONFIG_MODE"}, }, - &cli.StringFlag{ - Name: "config", - Usage: "Path to the cri-o config file", - Value: defaultConfig, - Destination: &options.config, - EnvVars: []string{"CRIO_CONFIG"}, - }, - &cli.StringFlag{ - Name: "runtime-class", - Usage: "The name of the runtime class to set for the nvidia-container-runtime", - Value: defaultRuntimeClass, - Destination: &options.runtimeClass, - EnvVars: []string{"CRIO_RUNTIME_CLASS"}, - }, // The flags below are only used by the 'setup' command. - &cli.BoolFlag{ - Name: "set-as-default", - Usage: "Set nvidia-container-runtime as the default runtime", - Value: defaultSetAsDefault, - Destination: &options.setAsDefault, - EnvVars: []string{"CRIO_SET_AS_DEFAULT"}, - Hidden: true, - }, - &cli.StringFlag{ - Name: "restart-mode", - Usage: "Specify how cri-o should be restarted; If 'none' is selected, it will not be restarted [systemd | none]", - Value: defaultRestartMode, - Destination: &options.restartMode, - EnvVars: []string{"CRIO_RESTART_MODE"}, - }, - &cli.StringFlag{ - Name: "host-root", - Usage: "Specify the path to the host root to be used when restarting crio using systemd", - Value: defaultHostRootMount, - Destination: &options.hostRootMount, - EnvVars: []string{"HOST_ROOT_MOUNT"}, - }, } // Update the subcommand flags with the common subcommand flags diff --git a/tools/container/docker/docker.go b/tools/container/docker/docker.go index 57704e3e..899d85b9 100644 --- a/tools/container/docker/docker.go +++ b/tools/container/docker/docker.go @@ -43,8 +43,9 @@ const ( defaultSocket = "/var/run/docker.sock" defaultSetAsDefault = true // defaultRuntimeName specifies the NVIDIA runtime to be use as the default runtime if setting the default runtime is enabled - defaultRuntimeName = nvidiaRuntimeName - defaultRestartMode = restartModeSignal + defaultRuntimeName = nvidiaRuntimeName + defaultRestartMode = restartModeSignal + defaultHostRootMount = "/host" reloadBackoff = 5 * time.Second maxReloadAttempts = 6 @@ -61,12 +62,13 @@ var nvidiaRuntimeBinaries = map[string]string{ // options stores the configuration from the command line or environment variables type options struct { - config string - socket string - runtimeName string - setAsDefault bool - runtimeDir string - restartMode string + config string + socket string + runtimeName string + setAsDefault bool + runtimeDir string + restartMode string + hostRootMount string } func main() { @@ -109,44 +111,50 @@ func main() { commonFlags := []cli.Flag{ &cli.StringFlag{ Name: "config", - Aliases: []string{"c"}, Usage: "Path to docker config file", Value: defaultConfig, Destination: &options.config, - EnvVars: []string{"DOCKER_CONFIG"}, + EnvVars: []string{"DOCKER_CONFIG", "RUNTIME_CONFIG"}, }, &cli.StringFlag{ Name: "socket", - Aliases: []string{"s"}, Usage: "Path to the docker socket file", Value: defaultSocket, Destination: &options.socket, - EnvVars: []string{"DOCKER_SOCKET"}, - }, - // The flags below are only used by the 'setup' command. - &cli.StringFlag{ - Name: "runtime-name", - Aliases: []string{"r"}, - Usage: "Specify the name of the `nvidia` runtime. If set-as-default is selected, the runtime is used as the default runtime.", - Value: defaultRuntimeName, - Destination: &options.runtimeName, - EnvVars: []string{"DOCKER_RUNTIME_NAME"}, - }, - &cli.BoolFlag{ - Name: "set-as-default", - Aliases: []string{"d"}, - Usage: "Set the `nvidia` runtime as the default runtime. If --runtime-name is specified as `nvidia-experimental` the experimental runtime is set as the default runtime instead", - Value: defaultSetAsDefault, - Destination: &options.setAsDefault, - EnvVars: []string{"DOCKER_SET_AS_DEFAULT"}, - Hidden: true, + EnvVars: []string{"DOCKER_SOCKET", "RUNTIME_SOCKET"}, }, &cli.StringFlag{ Name: "restart-mode", Usage: "Specify how docker should be restarted; If 'none' is selected it will not be restarted [signal | none]", Value: defaultRestartMode, Destination: &options.restartMode, - EnvVars: []string{"DOCKER_RESTART_MODE"}, + EnvVars: []string{"DOCKER_RESTART_MODE", "RUNTIME_RESTART_MODE"}, + }, + &cli.StringFlag{ + Name: "host-root", + Usage: "Specify the path to the host root to be used when restarting docker using systemd", + Value: defaultHostRootMount, + Destination: &options.hostRootMount, + EnvVars: []string{"HOST_ROOT_MOUNT"}, + // Restart using systemd is currently not supported. + // We hide this option for the time being. + Hidden: true, + }, + &cli.StringFlag{ + Name: "runtime-name", + Aliases: []string{"runtime-class", "nvidia-runtime-name"}, + Usage: "Specify the name of the `nvidia` runtime. If set-as-default is selected, the runtime is used as the default runtime.", + Value: defaultRuntimeName, + Destination: &options.runtimeName, + EnvVars: []string{"DOCKER_RUNTIME_NAME", "NVIDIA_RUNTIME_NAME"}, + }, + &cli.BoolFlag{ + Name: "set-as-default", + Usage: "Set the `nvidia` runtime as the default runtime. If --runtime-name is specified as `nvidia-experimental` the experimental runtime is set as the default runtime instead", + Value: defaultSetAsDefault, + Destination: &options.setAsDefault, + EnvVars: []string{"DOCKER_SET_AS_DEFAULT", "NVIDIA_RUNTIME_SET_AS_DEFAULT"}, + Hidden: true, }, } From b9b19494d0dfa45a2b370b6e4623a6f4dd11f13e Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 9 Jun 2023 16:50:51 +0200 Subject: [PATCH 2/9] Add runtimeDir as argument Thsi change adds the --nvidia-runtime-dir as a command line argument when configuring container runtimes in the toolkit container. This removes the need to set it via the command line. Signed-off-by: Evan Lezar --- tools/container/containerd/containerd.go | 59 ++++++++++++++---------- tools/container/crio/crio.go | 31 ++++++++++--- tools/container/docker/docker.go | 49 +++++++++++--------- 3 files changed, 86 insertions(+), 53 deletions(-) diff --git a/tools/container/containerd/containerd.go b/tools/container/containerd/containerd.go index 34387e9a..eb04ed71 100644 --- a/tools/container/containerd/containerd.go +++ b/tools/container/containerd/containerd.go @@ -63,15 +63,17 @@ var nvidiaRuntimeBinaries = map[string]string{ // options stores the configuration from the command line or environment variables type options struct { - config string - socket string - runtimeClass string - runtimeType string - setAsDefault bool - restartMode string - hostRootMount string - runtimeDir string + config string + socket string + runtimeClass string + runtimeDir string + setAsDefault bool + restartMode string + hostRootMount string + + // containerd-specific options useLegacyConfig bool + runtimeType string ContainerRuntimeModesCDIAnnotationPrefixes cli.StringSlice } @@ -93,6 +95,9 @@ func main() { setup.Action = func(c *cli.Context) error { return Setup(c, &options) } + setup.Before = func(c *cli.Context) error { + return ParseArgs(c, &options) + } // Create the 'cleanup' subcommand cleanup := cli.Command{} @@ -102,6 +107,9 @@ func main() { cleanup.Action = func(c *cli.Context) error { return Cleanup(c, &options) } + cleanup.Before = func(c *cli.Context) error { + return ParseArgs(c, &options) + } // Register the subcommands with the top-level CLI c.Commands = []*cli.Command{ @@ -143,6 +151,13 @@ func main() { Destination: &options.runtimeClass, EnvVars: []string{"CONTAINERD_RUNTIME_CLASS", "NVIDIA_RUNTIME_NAME"}, }, + &cli.StringFlag{ + Name: "nvidia-runtime-dir", + Aliases: []string{"runtime-dir"}, + Usage: "The path where the nvidia-container-runtime binaries are located. If this is not specified, the first argument will be used instead", + Destination: &options.runtimeDir, + EnvVars: []string{"NVIDIA_RUNTIME_DIR"}, + }, &cli.BoolFlag{ Name: "set-as-default", Usage: "Set nvidia-container-runtime as the default runtime", @@ -192,12 +207,6 @@ func main() { func Setup(c *cli.Context, o *options) error { log.Infof("Starting 'setup' for %v", c.App.Name) - runtimeDir, err := ParseArgs(c) - if err != nil { - return fmt.Errorf("unable to parse args: %v", err) - } - o.runtimeDir = runtimeDir - cfg, err := containerd.New( containerd.WithPath(o.config), containerd.WithRuntimeType(o.runtimeType), @@ -236,11 +245,6 @@ func Setup(c *cli.Context, o *options) error { func Cleanup(c *cli.Context, o *options) error { log.Infof("Starting 'cleanup' for %v", c.App.Name) - _, err := ParseArgs(c) - if err != nil { - return fmt.Errorf("unable to parse args: %v", err) - } - cfg, err := containerd.New( containerd.WithPath(o.config), containerd.WithRuntimeType(o.runtimeType), @@ -276,17 +280,24 @@ func Cleanup(c *cli.Context, o *options) error { } // ParseArgs parses the command line arguments to the CLI -func ParseArgs(c *cli.Context) (string, error) { +func ParseArgs(c *cli.Context, o *options) error { + if o.runtimeDir != "" { + log.Debug("Runtime directory already set") + return nil + } + args := c.Args() log.Infof("Parsing arguments: %v", args.Slice()) - if args.Len() != 1 { - return "", fmt.Errorf("incorrect number of arguments") + if c.NArg() != 1 { + return fmt.Errorf("incorrect number of arguments") } - runtimeDir := args.Get(0) + + o.runtimeDir = args.Get(0) + log.Infof("Successfully parsed arguments") - return runtimeDir, nil + return nil } // UpdateConfig updates the containerd config to include the nvidia-container-runtime diff --git a/tools/container/crio/crio.go b/tools/container/crio/crio.go index 2525dc44..de919b95 100644 --- a/tools/container/crio/crio.go +++ b/tools/container/crio/crio.go @@ -52,18 +52,19 @@ const ( // options stores the configuration from the command linek or environment variables type options struct { - configMode string - - hooksDir string - hookFilename string - runtimeDir string - config string socket string runtimeClass string + runtimeDir string setAsDefault bool restartMode string hostRootMount string + + configMode string + + // hook-specific options + hooksDir string + hookFilename string } func main() { @@ -95,6 +96,10 @@ func main() { cleanup.Action = func(c *cli.Context) error { return Cleanup(c, &options) } + cleanup.Before = func(c *cli.Context) error { + return ParseArgs(c, &options) + } + // Register the subcommands with the top-level CLI c.Commands = []*cli.Command{ &setup, @@ -136,6 +141,13 @@ func main() { Destination: &options.runtimeClass, EnvVars: []string{"CRIO_RUNTIME_CLASS", "NVIDIA_RUNTIME_NAME"}, }, + &cli.StringFlag{ + Name: "nvidia-runtime-dir", + Aliases: []string{"runtime-dir"}, + Usage: "The path where the nvidia-container-runtime binaries are located. If this is not specified, the first argument will be used instead", + Destination: &options.runtimeDir, + EnvVars: []string{"NVIDIA_RUNTIME_DIR"}, + }, &cli.BoolFlag{ Name: "set-as-default", Usage: "Set nvidia-container-runtime as the default runtime", @@ -314,13 +326,20 @@ func cleanupConfig(o *options) error { // ParseArgs parses the command line arguments to the CLI func ParseArgs(c *cli.Context, o *options) error { + if o.runtimeDir != "" { + log.Debug("Runtime directory already set") + return nil + } + args := c.Args() log.Infof("Parsing arguments: %v", args.Slice()) if c.NArg() != 1 { return fmt.Errorf("incorrect number of arguments") } + o.runtimeDir = args.Get(0) + log.Infof("Successfully parsed arguments") return nil diff --git a/tools/container/docker/docker.go b/tools/container/docker/docker.go index 899d85b9..d6531766 100644 --- a/tools/container/docker/docker.go +++ b/tools/container/docker/docker.go @@ -54,19 +54,13 @@ const ( socketMessageToGetPID = "GET /info HTTP/1.0\r\n\r\n" ) -// nvidiaRuntimeBinaries defines a map of runtime names to binary names -var nvidiaRuntimeBinaries = map[string]string{ - nvidiaRuntimeName: nvidiaRuntimeBinary, - nvidiaExperimentalRuntimeName: nvidiaExperimentalRuntimeBinary, -} - // options stores the configuration from the command line or environment variables type options struct { config string socket string runtimeName string - setAsDefault bool runtimeDir string + setAsDefault bool restartMode string hostRootMount string } @@ -88,6 +82,9 @@ func main() { setup.Action = func(c *cli.Context) error { return Setup(c, &options) } + setup.Before = func(c *cli.Context) error { + return ParseArgs(c, &options) + } // Create the 'cleanup' subcommand cleanup := cli.Command{} @@ -97,6 +94,9 @@ func main() { cleanup.Action = func(c *cli.Context) error { return Cleanup(c, &options) } + cleanup.Before = func(c *cli.Context) error { + return ParseArgs(c, &options) + } // Register the subcommands with the top-level CLI c.Commands = []*cli.Command{ @@ -148,6 +148,13 @@ func main() { Destination: &options.runtimeName, EnvVars: []string{"DOCKER_RUNTIME_NAME", "NVIDIA_RUNTIME_NAME"}, }, + &cli.StringFlag{ + Name: "nvidia-runtime-dir", + Aliases: []string{"runtime-dir"}, + Usage: "The path where the nvidia-container-runtime binaries are located. If this is not specified, the first argument will be used instead", + Destination: &options.runtimeDir, + EnvVars: []string{"NVIDIA_RUNTIME_DIR"}, + }, &cli.BoolFlag{ Name: "set-as-default", Usage: "Set the `nvidia` runtime as the default runtime. If --runtime-name is specified as `nvidia-experimental` the experimental runtime is set as the default runtime instead", @@ -173,12 +180,6 @@ func main() { func Setup(c *cli.Context, o *options) error { log.Infof("Starting 'setup' for %v", c.App.Name) - runtimeDir, err := ParseArgs(c) - if err != nil { - return fmt.Errorf("unable to parse args: %v", err) - } - o.runtimeDir = runtimeDir - cfg, err := docker.New( docker.WithPath(o.config), ) @@ -211,11 +212,6 @@ func Setup(c *cli.Context, o *options) error { func Cleanup(c *cli.Context, o *options) error { log.Infof("Starting 'cleanup' for %v", c.App.Name) - _, err := ParseArgs(c) - if err != nil { - return fmt.Errorf("unable to parse args: %v", err) - } - cfg, err := docker.New( docker.WithPath(o.config), ) @@ -248,17 +244,24 @@ func Cleanup(c *cli.Context, o *options) error { } // ParseArgs parses the command line arguments to the CLI -func ParseArgs(c *cli.Context) (string, error) { +func ParseArgs(c *cli.Context, o *options) error { + if o.runtimeDir != "" { + log.Debug("Runtime directory already set") + return nil + } + args := c.Args() log.Infof("Parsing arguments: %v", args.Slice()) - if args.Len() != 1 { - return "", fmt.Errorf("incorrect number of arguments") + if c.NArg() != 1 { + return fmt.Errorf("incorrect number of arguments") } - runtimeDir := args.Get(0) + + o.runtimeDir = args.Get(0) + log.Infof("Successfully parsed arguments") - return runtimeDir, nil + return nil } // UpdateConfig updates the docker config to include the nvidia runtimes From 56c533d7d4d87131f20df589be62fafec9b96b68 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 9 Jun 2023 17:17:54 +0200 Subject: [PATCH 3/9] Refactor toolking to setup and cleanup configs Signed-off-by: Evan Lezar --- tools/container/container.go | 123 +++++++++++++++++++ tools/container/containerd/config_v1_test.go | 63 +++++----- tools/container/containerd/config_v2_test.go | 66 +++++----- tools/container/containerd/containerd.go | 123 ++++--------------- tools/container/crio/crio.go | 123 ++++--------------- tools/container/docker/docker.go | 119 ++++-------------- tools/container/docker/docker_test.go | 26 ++-- 7 files changed, 276 insertions(+), 367 deletions(-) create mode 100644 tools/container/container.go diff --git a/tools/container/container.go b/tools/container/container.go new file mode 100644 index 00000000..1d1ea8a5 --- /dev/null +++ b/tools/container/container.go @@ -0,0 +1,123 @@ +/** +# Copyright (c) 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 container + +import ( + "fmt" + + "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine" + "github.com/NVIDIA/nvidia-container-toolkit/tools/container/operator" + "github.com/sirupsen/logrus" + "github.com/urfave/cli/v2" +) + +// Options defines the shared options for the CLIs to configure containers runtimes. +type Options struct { + Config string + Socket string + RuntimeName string + RuntimeDir string + SetAsDefault bool + RestartMode string + HostRootMount string +} + +// ParseArgs parses the command line arguments to the CLI +func ParseArgs(c *cli.Context, o *Options) error { + if o.RuntimeDir != "" { + logrus.Debug("Runtime directory already set; ignoring arguments") + return nil + } + + args := c.Args() + + logrus.Infof("Parsing arguments: %v", args.Slice()) + if c.NArg() != 1 { + return fmt.Errorf("incorrect number of arguments") + } + + o.RuntimeDir = args.Get(0) + + logrus.Infof("Successfully parsed arguments") + + return nil +} + +// Configure applies the options to the specified config +func (o Options) Configure(cfg engine.Interface) error { + err := o.UpdateConfig(cfg) + if err != nil { + return fmt.Errorf("unable to update config: %v", err) + } + return o.flush(cfg) +} + +// Unconfigure removes the options from the specified config +func (o Options) Unconfigure(cfg engine.Interface) error { + err := o.RevertConfig(cfg) + if err != nil { + return fmt.Errorf("unable to update config: %v", err) + } + return o.flush(cfg) +} + +// flush flushes the specified config to disk +func (o Options) flush(cfg engine.Interface) error { + logrus.Infof("Flushing config to %v", o.Config) + n, err := cfg.Save(o.Config) + if err != nil { + return fmt.Errorf("unable to flush config: %v", err) + } + if n == 0 { + logrus.Infof("Config file is empty, removed") + } + return nil +} + +// UpdateConfig updates the specified config to include the nvidia runtimes +func (o Options) UpdateConfig(cfg engine.Interface) error { + runtimes := operator.GetRuntimes( + operator.WithNvidiaRuntimeName(o.RuntimeName), + operator.WithSetAsDefault(o.SetAsDefault), + operator.WithRoot(o.RuntimeDir), + ) + for name, runtime := range runtimes { + err := cfg.AddRuntime(name, runtime.Path, runtime.SetAsDefault) + if err != nil { + return fmt.Errorf("failed to update runtime %q: %v", name, err) + } + } + + return nil +} + +// RevertConfig reverts the specified config to remove the nvidia runtimes +func (o Options) RevertConfig(cfg engine.Interface) error { + runtimes := operator.GetRuntimes( + operator.WithNvidiaRuntimeName(o.RuntimeName), + operator.WithSetAsDefault(o.SetAsDefault), + operator.WithRoot(o.RuntimeDir), + ) + for name := range runtimes { + err := cfg.RemoveRuntime(name) + if err != nil { + return fmt.Errorf("failed to remove runtime %q: %v", name, err) + } + } + + return nil +} diff --git a/tools/container/containerd/config_v1_test.go b/tools/container/containerd/config_v1_test.go index 9625d90b..9aaec149 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/NVIDIA/nvidia-container-toolkit/internal/config/engine/containerd" + "github.com/NVIDIA/nvidia-container-toolkit/tools/container" "github.com/pelletier/go-toml" "github.com/stretchr/testify/require" ) @@ -31,7 +32,7 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) { testCases := []struct { legacyConfig bool setAsDefault bool - runtimeClass string + runtimeName string expectedDefaultRuntimeName interface{} expectedDefaultRuntimeBinary interface{} }{ @@ -51,14 +52,14 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) { { legacyConfig: true, setAsDefault: true, - runtimeClass: "NAME", + runtimeName: "NAME", expectedDefaultRuntimeName: nil, expectedDefaultRuntimeBinary: "/test/runtime/dir/nvidia-container-runtime", }, { legacyConfig: true, setAsDefault: true, - runtimeClass: "nvidia-experimental", + runtimeName: "nvidia-experimental", expectedDefaultRuntimeName: nil, expectedDefaultRuntimeBinary: "/test/runtime/dir/nvidia-container-runtime.experimental", }, @@ -77,14 +78,14 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) { { legacyConfig: false, setAsDefault: true, - runtimeClass: "NAME", + runtimeName: "NAME", expectedDefaultRuntimeName: "NAME", expectedDefaultRuntimeBinary: nil, }, { legacyConfig: false, setAsDefault: true, - runtimeClass: "nvidia-experimental", + runtimeName: "nvidia-experimental", expectedDefaultRuntimeName: "nvidia-experimental", expectedDefaultRuntimeBinary: nil, }, @@ -93,11 +94,13 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { o := &options{ - useLegacyConfig: tc.legacyConfig, - setAsDefault: tc.setAsDefault, - runtimeClass: tc.runtimeClass, + Options: container.Options{ + RuntimeName: tc.runtimeName, + RuntimeDir: runtimeDir, + SetAsDefault: tc.setAsDefault, + }, runtimeType: runtimeType, - runtimeDir: runtimeDir, + useLegacyConfig: tc.legacyConfig, } config, err := toml.TreeFromMap(map[string]interface{}{}) @@ -109,7 +112,7 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) { RuntimeType: runtimeType, } - err = UpdateConfig(v1, o) + err = o.UpdateConfig(v1) require.NoError(t, err, "%d: %v", i, tc) defaultRuntimeName := v1.GetPath([]string{"plugins", "cri", "containerd", "default_runtime_name"}) @@ -138,11 +141,11 @@ func TestUpdateV1Config(t *testing.T) { const runtimeDir = "/test/runtime/dir" testCases := []struct { - runtimeClass string + runtimeName string expectedConfig map[string]interface{} }{ { - runtimeClass: "nvidia", + runtimeName: "nvidia", expectedConfig: map[string]interface{}{ "version": int64(1), "plugins": map[string]interface{}{ @@ -200,7 +203,7 @@ func TestUpdateV1Config(t *testing.T) { }, }, { - runtimeClass: "NAME", + runtimeName: "NAME", expectedConfig: map[string]interface{}{ "version": int64(1), "plugins": map[string]interface{}{ @@ -258,7 +261,7 @@ func TestUpdateV1Config(t *testing.T) { }, }, { - runtimeClass: "nvidia-experimental", + runtimeName: "nvidia-experimental", expectedConfig: map[string]interface{}{ "version": int64(1), "plugins": map[string]interface{}{ @@ -320,9 +323,10 @@ func TestUpdateV1Config(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { o := &options{ - runtimeClass: tc.runtimeClass, - runtimeType: runtimeType, - runtimeDir: runtimeDir, + Options: container.Options{ + RuntimeName: tc.runtimeName, + RuntimeDir: runtimeDir, + }, } config, err := toml.TreeFromMap(map[string]interface{}{}) @@ -335,7 +339,7 @@ func TestUpdateV1Config(t *testing.T) { ContainerAnnotations: []string{"cdi.k8s.io/*"}, } - err = UpdateConfig(v1, o) + err = o.UpdateConfig(v1) require.NoError(t, err) expected, err := toml.TreeFromMap(tc.expectedConfig) @@ -350,11 +354,11 @@ func TestUpdateV1ConfigWithRuncPresent(t *testing.T) { const runtimeDir = "/test/runtime/dir" testCases := []struct { - runtimeClass string + runtimeName string expectedConfig map[string]interface{} }{ { - runtimeClass: "nvidia", + runtimeName: "nvidia", expectedConfig: map[string]interface{}{ "version": int64(1), "plugins": map[string]interface{}{ @@ -426,7 +430,7 @@ func TestUpdateV1ConfigWithRuncPresent(t *testing.T) { }, }, { - runtimeClass: "NAME", + runtimeName: "NAME", expectedConfig: map[string]interface{}{ "version": int64(1), "plugins": map[string]interface{}{ @@ -498,7 +502,7 @@ func TestUpdateV1ConfigWithRuncPresent(t *testing.T) { }, }, { - runtimeClass: "nvidia-experimental", + runtimeName: "nvidia-experimental", expectedConfig: map[string]interface{}{ "version": int64(1), "plugins": map[string]interface{}{ @@ -574,9 +578,10 @@ func TestUpdateV1ConfigWithRuncPresent(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { o := &options{ - runtimeClass: tc.runtimeClass, - runtimeType: runtimeType, - runtimeDir: runtimeDir, + Options: container.Options{ + RuntimeName: tc.runtimeName, + RuntimeDir: runtimeDir, + }, } config, err := toml.TreeFromMap(runcConfigMapV1("/runc-binary")) @@ -589,7 +594,7 @@ func TestUpdateV1ConfigWithRuncPresent(t *testing.T) { ContainerAnnotations: []string{"cdi.k8s.io/*"}, } - err = UpdateConfig(v1, o) + err = o.UpdateConfig(v1) require.NoError(t, err) expected, err := toml.TreeFromMap(tc.expectedConfig) @@ -653,7 +658,9 @@ func TestRevertV1Config(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { o := &options{ - runtimeClass: "nvidia", + Options: container.Options{ + RuntimeName: "nvidia", + }, } config, err := toml.TreeFromMap(tc.config) @@ -668,7 +675,7 @@ func TestRevertV1Config(t *testing.T) { RuntimeType: runtimeType, } - err = RevertConfig(v1, o) + err = o.RevertConfig(v1) require.NoError(t, err, "%d: %v", i, tc) configContents, _ := toml.Marshal(config) diff --git a/tools/container/containerd/config_v2_test.go b/tools/container/containerd/config_v2_test.go index c14fb58f..85732aed 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/NVIDIA/nvidia-container-toolkit/internal/config/engine/containerd" + "github.com/NVIDIA/nvidia-container-toolkit/tools/container" "github.com/pelletier/go-toml" "github.com/stretchr/testify/require" ) @@ -34,38 +35,38 @@ func TestUpdateV2ConfigDefaultRuntime(t *testing.T) { testCases := []struct { setAsDefault bool - runtimeClass string + runtimeName string expectedDefaultRuntimeName interface{} }{ {}, { setAsDefault: false, - runtimeClass: "nvidia", + runtimeName: "nvidia", expectedDefaultRuntimeName: nil, }, { setAsDefault: false, - runtimeClass: "NAME", + runtimeName: "NAME", expectedDefaultRuntimeName: nil, }, { setAsDefault: false, - runtimeClass: "nvidia-experimental", + runtimeName: "nvidia-experimental", expectedDefaultRuntimeName: nil, }, { setAsDefault: true, - runtimeClass: "nvidia", + runtimeName: "nvidia", expectedDefaultRuntimeName: "nvidia", }, { setAsDefault: true, - runtimeClass: "NAME", + runtimeName: "NAME", expectedDefaultRuntimeName: "NAME", }, { setAsDefault: true, - runtimeClass: "nvidia-experimental", + runtimeName: "nvidia-experimental", expectedDefaultRuntimeName: "nvidia-experimental", }, } @@ -73,9 +74,11 @@ func TestUpdateV2ConfigDefaultRuntime(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { o := &options{ - setAsDefault: tc.setAsDefault, - runtimeClass: tc.runtimeClass, - runtimeDir: runtimeDir, + Options: container.Options{ + RuntimeName: tc.runtimeName, + RuntimeDir: runtimeDir, + SetAsDefault: tc.setAsDefault, + }, } config, err := toml.TreeFromMap(map[string]interface{}{}) @@ -86,7 +89,7 @@ func TestUpdateV2ConfigDefaultRuntime(t *testing.T) { RuntimeType: runtimeType, } - err = UpdateConfig(v2, o) + err = o.UpdateConfig(v2) require.NoError(t, err) defaultRuntimeName := config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "default_runtime_name"}) @@ -100,11 +103,11 @@ func TestUpdateV2Config(t *testing.T) { const expectedVersion = int64(2) testCases := []struct { - runtimeClass string + runtimeName string expectedConfig map[string]interface{} }{ { - runtimeClass: "nvidia", + runtimeName: "nvidia", expectedConfig: map[string]interface{}{ "version": int64(2), "plugins": map[string]interface{}{ @@ -158,7 +161,7 @@ func TestUpdateV2Config(t *testing.T) { }, }, { - runtimeClass: "NAME", + runtimeName: "NAME", expectedConfig: map[string]interface{}{ "version": int64(2), "plugins": map[string]interface{}{ @@ -212,7 +215,7 @@ func TestUpdateV2Config(t *testing.T) { }, }, { - runtimeClass: "nvidia-experimental", + runtimeName: "nvidia-experimental", expectedConfig: map[string]interface{}{ "version": int64(2), "plugins": map[string]interface{}{ @@ -270,9 +273,11 @@ func TestUpdateV2Config(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { o := &options{ - runtimeClass: tc.runtimeClass, - runtimeType: runtimeType, - runtimeDir: runtimeDir, + Options: container.Options{ + RuntimeName: tc.runtimeName, + RuntimeDir: runtimeDir, + }, + runtimeType: runtimeType, } config, err := toml.TreeFromMap(map[string]interface{}{}) @@ -284,7 +289,7 @@ func TestUpdateV2Config(t *testing.T) { ContainerAnnotations: []string{"cdi.k8s.io/*"}, } - err = UpdateConfig(v2, o) + err = o.UpdateConfig(v2) require.NoError(t, err) expected, err := toml.TreeFromMap(tc.expectedConfig) @@ -300,11 +305,11 @@ func TestUpdateV2ConfigWithRuncPresent(t *testing.T) { const runtimeDir = "/test/runtime/dir" testCases := []struct { - runtimeClass string + runtimeName string expectedConfig map[string]interface{} }{ { - runtimeClass: "nvidia", + runtimeName: "nvidia", expectedConfig: map[string]interface{}{ "version": int64(2), "plugins": map[string]interface{}{ @@ -372,7 +377,7 @@ func TestUpdateV2ConfigWithRuncPresent(t *testing.T) { }, }, { - runtimeClass: "NAME", + runtimeName: "NAME", expectedConfig: map[string]interface{}{ "version": int64(2), "plugins": map[string]interface{}{ @@ -440,7 +445,7 @@ func TestUpdateV2ConfigWithRuncPresent(t *testing.T) { }, }, { - runtimeClass: "nvidia-experimental", + runtimeName: "nvidia-experimental", expectedConfig: map[string]interface{}{ "version": int64(2), "plugins": map[string]interface{}{ @@ -512,9 +517,10 @@ func TestUpdateV2ConfigWithRuncPresent(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { o := &options{ - runtimeClass: tc.runtimeClass, - runtimeType: runtimeType, - runtimeDir: runtimeDir, + Options: container.Options{ + RuntimeName: tc.runtimeName, + RuntimeDir: runtimeDir, + }, } config, err := toml.TreeFromMap(runcConfigMapV2("/runc-binary")) @@ -526,7 +532,7 @@ func TestUpdateV2ConfigWithRuncPresent(t *testing.T) { ContainerAnnotations: []string{"cdi.k8s.io/*"}, } - err = UpdateConfig(v2, o) + err = o.UpdateConfig(v2) require.NoError(t, err) expected, err := toml.TreeFromMap(tc.expectedConfig) @@ -585,7 +591,9 @@ func TestRevertV2Config(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { o := &options{ - runtimeClass: "nvidia", + Options: container.Options{ + RuntimeName: "nvidia", + }, } config, err := toml.TreeFromMap(tc.config) @@ -599,7 +607,7 @@ func TestRevertV2Config(t *testing.T) { RuntimeType: runtimeType, } - err = RevertConfig(v2, o) + err = o.RevertConfig(v2) require.NoError(t, err) configContents, _ := toml.Marshal(config) diff --git a/tools/container/containerd/containerd.go b/tools/container/containerd/containerd.go index eb04ed71..a1e86d04 100644 --- a/tools/container/containerd/containerd.go +++ b/tools/container/containerd/containerd.go @@ -24,9 +24,8 @@ import ( "syscall" "time" - "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/tools/container/operator" + "github.com/NVIDIA/nvidia-container-toolkit/tools/container" log "github.com/sirupsen/logrus" cli "github.com/urfave/cli/v2" ) @@ -63,13 +62,7 @@ var nvidiaRuntimeBinaries = map[string]string{ // options stores the configuration from the command line or environment variables type options struct { - config string - socket string - runtimeClass string - runtimeDir string - setAsDefault bool - restartMode string - hostRootMount string + container.Options // containerd-specific options useLegacyConfig bool @@ -96,7 +89,7 @@ func main() { return Setup(c, &options) } setup.Before = func(c *cli.Context) error { - return ParseArgs(c, &options) + return container.ParseArgs(c, &options.Options) } // Create the 'cleanup' subcommand @@ -108,7 +101,7 @@ func main() { return Cleanup(c, &options) } cleanup.Before = func(c *cli.Context) error { - return ParseArgs(c, &options) + return container.ParseArgs(c, &options.Options) } // Register the subcommands with the top-level CLI @@ -126,21 +119,21 @@ func main() { Name: "config", Usage: "Path to the containerd config file", Value: defaultConfig, - Destination: &options.config, + Destination: &options.Config, EnvVars: []string{"CONTAINERD_CONFIG", "RUNTIME_CONFIG"}, }, &cli.StringFlag{ Name: "socket", Usage: "Path to the containerd socket file", Value: defaultSocket, - Destination: &options.socket, + Destination: &options.Socket, EnvVars: []string{"CONTAINERD_SOCKET", "RUNTIME_SOCKET"}, }, &cli.StringFlag{ Name: "restart-mode", Usage: "Specify how containerd should be restarted; If 'none' is selected, it will not be restarted [signal | systemd | none]", Value: defaultRestartMode, - Destination: &options.restartMode, + Destination: &options.RestartMode, EnvVars: []string{"CONTAINERD_RESTART_MODE", "RUNTIME_RESTART_MODE"}, }, &cli.StringFlag{ @@ -148,21 +141,21 @@ func main() { Aliases: []string{"runtime-name", "nvidia-runtime-name"}, Usage: "The name of the runtime class to set for the nvidia-container-runtime", Value: defaultRuntimeClass, - Destination: &options.runtimeClass, + Destination: &options.RuntimeName, EnvVars: []string{"CONTAINERD_RUNTIME_CLASS", "NVIDIA_RUNTIME_NAME"}, }, &cli.StringFlag{ Name: "nvidia-runtime-dir", Aliases: []string{"runtime-dir"}, Usage: "The path where the nvidia-container-runtime binaries are located. If this is not specified, the first argument will be used instead", - Destination: &options.runtimeDir, + Destination: &options.RuntimeDir, EnvVars: []string{"NVIDIA_RUNTIME_DIR"}, }, &cli.BoolFlag{ Name: "set-as-default", Usage: "Set nvidia-container-runtime as the default runtime", Value: defaultSetAsDefault, - Destination: &options.setAsDefault, + Destination: &options.SetAsDefault, EnvVars: []string{"CONTAINERD_SET_AS_DEFAULT", "NVIDIA_RUNTIME_SET_AS_DEFAULT"}, Hidden: true, }, @@ -170,7 +163,7 @@ func main() { Name: "host-root", Usage: "Specify the path to the host root to be used when restarting containerd using systemd", Value: defaultHostRootMount, - Destination: &options.hostRootMount, + Destination: &options.HostRootMount, EnvVars: []string{"HOST_ROOT_MOUNT"}, }, &cli.BoolFlag{ @@ -208,7 +201,7 @@ func Setup(c *cli.Context, o *options) error { log.Infof("Starting 'setup' for %v", c.App.Name) cfg, err := containerd.New( - containerd.WithPath(o.config), + containerd.WithPath(o.Config), containerd.WithRuntimeType(o.runtimeType), containerd.WithUseLegacyConfig(o.useLegacyConfig), containerd.WithContainerAnnotations(o.containerAnnotationsFromCDIPrefixes()...), @@ -217,18 +210,9 @@ func Setup(c *cli.Context, o *options) error { return fmt.Errorf("unable to load config: %v", err) } - err = UpdateConfig(cfg, o) + err = o.Configure(cfg) if err != nil { - return fmt.Errorf("unable to update config: %v", err) - } - - log.Infof("Flushing containerd config to %v", o.config) - n, err := cfg.Save(o.config) - if err != nil { - return fmt.Errorf("unable to flush config: %v", err) - } - if n == 0 { - log.Infof("Config file is empty, removed") + return fmt.Errorf("unable to configure containerd: %v", err) } err = RestartContainerd(o) @@ -246,7 +230,7 @@ func Cleanup(c *cli.Context, o *options) error { log.Infof("Starting 'cleanup' for %v", c.App.Name) cfg, err := containerd.New( - containerd.WithPath(o.config), + containerd.WithPath(o.Config), containerd.WithRuntimeType(o.runtimeType), containerd.WithUseLegacyConfig(o.useLegacyConfig), containerd.WithContainerAnnotations(o.containerAnnotationsFromCDIPrefixes()...), @@ -255,18 +239,9 @@ func Cleanup(c *cli.Context, o *options) error { return fmt.Errorf("unable to load config: %v", err) } - err = RevertConfig(cfg, o) + err = o.Unconfigure(cfg) if err != nil { - return fmt.Errorf("unable to update config: %v", err) - } - - log.Infof("Flushing containerd config to %v", o.config) - n, err := cfg.Save(o.config) - if err != nil { - return fmt.Errorf("unable to flush config: %v", err) - } - if n == 0 { - log.Infof("Config file is empty, removed") + return fmt.Errorf("unable to unconfigure containerd: %v", err) } err = RestartContainerd(o) @@ -279,65 +254,11 @@ func Cleanup(c *cli.Context, o *options) error { return nil } -// ParseArgs parses the command line arguments to the CLI -func ParseArgs(c *cli.Context, o *options) error { - if o.runtimeDir != "" { - log.Debug("Runtime directory already set") - return nil - } - - args := c.Args() - - log.Infof("Parsing arguments: %v", args.Slice()) - if c.NArg() != 1 { - return fmt.Errorf("incorrect number of arguments") - } - - o.runtimeDir = args.Get(0) - - log.Infof("Successfully parsed arguments") - - return nil -} - -// UpdateConfig updates the containerd config to include the nvidia-container-runtime -func UpdateConfig(cfg engine.Interface, o *options) error { - runtimes := operator.GetRuntimes( - operator.WithNvidiaRuntimeName(o.runtimeClass), - operator.WithSetAsDefault(o.setAsDefault), - operator.WithRoot(o.runtimeDir), - ) - for class, runtime := range runtimes { - err := cfg.AddRuntime(class, runtime.Path, runtime.SetAsDefault) - if err != nil { - return fmt.Errorf("unable to update config for runtime class '%v': %v", class, err) - } - } - - return nil -} - -// RevertConfig reverts the containerd config to remove the nvidia-container-runtime -func RevertConfig(cfg engine.Interface, o *options) error { - runtimes := operator.GetRuntimes( - operator.WithNvidiaRuntimeName(o.runtimeClass), - operator.WithSetAsDefault(o.setAsDefault), - operator.WithRoot(o.runtimeDir), - ) - for class := range runtimes { - err := cfg.RemoveRuntime(class) - if err != nil { - return fmt.Errorf("unable to revert config for runtime class '%v': %v", class, err) - } - } - return nil -} - // RestartContainerd restarts containerd depending on the value of restartModeFlag func RestartContainerd(o *options) error { - switch o.restartMode { + switch o.RestartMode { case restartModeNone: - log.Warnf("Skipping sending signal to containerd due to --restart-mode=%v", o.restartMode) + log.Warnf("Skipping sending signal to containerd due to --restart-mode=%v", o.RestartMode) return nil case restartModeSignal: err := SignalContainerd(o) @@ -345,9 +266,9 @@ func RestartContainerd(o *options) error { return fmt.Errorf("unable to signal containerd: %v", err) } case restartModeSystemd: - return RestartContainerdSystemd(o.hostRootMount) + return RestartContainerdSystemd(o.HostRootMount) default: - return fmt.Errorf("Invalid restart mode specified: %v", o.restartMode) + return fmt.Errorf("Invalid restart mode specified: %v", o.RestartMode) } return nil @@ -359,7 +280,7 @@ func SignalContainerd(o *options) error { // Wrap the logic to perform the SIGHUP in a function so we can retry it on failure retriable := func() error { - conn, err := net.Dial("unix", o.socket) + conn, err := net.Dial("unix", o.Socket) if err != nil { return fmt.Errorf("unable to dial: %v", err) } diff --git a/tools/container/crio/crio.go b/tools/container/crio/crio.go index de919b95..f44efc17 100644 --- a/tools/container/crio/crio.go +++ b/tools/container/crio/crio.go @@ -24,9 +24,8 @@ import ( "path/filepath" "github.com/NVIDIA/nvidia-container-toolkit/internal/config" - "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/tools/container/operator" + "github.com/NVIDIA/nvidia-container-toolkit/tools/container" log "github.com/sirupsen/logrus" cli "github.com/urfave/cli/v2" ) @@ -52,13 +51,7 @@ const ( // options stores the configuration from the command linek or environment variables type options struct { - config string - socket string - runtimeClass string - runtimeDir string - setAsDefault bool - restartMode string - hostRootMount string + container.Options configMode string @@ -86,7 +79,7 @@ func main() { return Setup(c, &options) } setup.Before = func(c *cli.Context) error { - return ParseArgs(c, &options) + return container.ParseArgs(c, &options.Options) } // Create the 'cleanup' subcommand @@ -97,7 +90,7 @@ func main() { return Cleanup(c, &options) } cleanup.Before = func(c *cli.Context) error { - return ParseArgs(c, &options) + return container.ParseArgs(c, &options.Options) } // Register the subcommands with the top-level CLI @@ -115,14 +108,14 @@ func main() { Name: "config", Usage: "Path to the cri-o config file", Value: defaultConfig, - Destination: &options.config, + Destination: &options.Config, EnvVars: []string{"CRIO_CONFIG", "RUNTIME_CONFIG"}, }, &cli.StringFlag{ Name: "socket", Usage: "Path to the crio socket file", Value: "", - Destination: &options.socket, + Destination: &options.Socket, EnvVars: []string{"CRIO_SOCKET", "RUNTIME_SOCKET"}, Hidden: true, }, @@ -130,7 +123,7 @@ func main() { Name: "restart-mode", Usage: "Specify how cri-o should be restarted; If 'none' is selected, it will not be restarted [systemd | none]", Value: defaultRestartMode, - Destination: &options.restartMode, + Destination: &options.RestartMode, EnvVars: []string{"CRIO_RESTART_MODE", "RUNTIME_RESTART_MODE"}, }, &cli.StringFlag{ @@ -138,21 +131,21 @@ func main() { Aliases: []string{"runtime-name", "nvidia-runtime-name"}, Usage: "The name of the runtime class to set for the nvidia-container-runtime", Value: defaultRuntimeClass, - Destination: &options.runtimeClass, + Destination: &options.RuntimeName, EnvVars: []string{"CRIO_RUNTIME_CLASS", "NVIDIA_RUNTIME_NAME"}, }, &cli.StringFlag{ Name: "nvidia-runtime-dir", Aliases: []string{"runtime-dir"}, Usage: "The path where the nvidia-container-runtime binaries are located. If this is not specified, the first argument will be used instead", - Destination: &options.runtimeDir, + Destination: &options.RuntimeDir, EnvVars: []string{"NVIDIA_RUNTIME_DIR"}, }, &cli.BoolFlag{ Name: "set-as-default", Usage: "Set nvidia-container-runtime as the default runtime", Value: defaultSetAsDefault, - Destination: &options.setAsDefault, + Destination: &options.SetAsDefault, EnvVars: []string{"CRIO_SET_AS_DEFAULT", "NVIDIA_RUNTIME_SET_AS_DEFAULT"}, Hidden: true, }, @@ -160,7 +153,7 @@ func main() { Name: "host-root", Usage: "Specify the path to the host root to be used when restarting crio using systemd", Value: defaultHostRootMount, - Destination: &options.hostRootMount, + Destination: &options.HostRootMount, EnvVars: []string{"HOST_ROOT_MOUNT"}, }, &cli.StringFlag{ @@ -223,7 +216,7 @@ func setupHook(o *options) error { } hookPath := getHookPath(o.hooksDir, o.hookFilename) - err = createHook(o.runtimeDir, hookPath) + err = createHook(o.RuntimeDir, hookPath) if err != nil { return fmt.Errorf("error creating hook: %v", err) } @@ -236,24 +229,15 @@ func setupConfig(o *options) error { log.Infof("Updating config file") cfg, err := crio.New( - crio.WithPath(o.config), + crio.WithPath(o.Config), ) if err != nil { return fmt.Errorf("unable to load config: %v", err) } - err = UpdateConfig(cfg, o) + err = o.Configure(cfg) if err != nil { - return fmt.Errorf("unable to update config: %v", err) - } - - log.Infof("Flushing cri-o config to %v", o.config) - n, err := cfg.Save(o.config) - if err != nil { - return fmt.Errorf("unable to flush config: %v", err) - } - if n == 0 { - log.Infof("Config file is empty, removed") + return fmt.Errorf("unable to configure cri-o: %v", err) } err = RestartCrio(o) @@ -296,24 +280,15 @@ func cleanupConfig(o *options) error { log.Infof("Reverting config file modifications") cfg, err := crio.New( - crio.WithPath(o.config), + crio.WithPath(o.Config), ) if err != nil { return fmt.Errorf("unable to load config: %v", err) } - err = RevertConfig(cfg, o) + err = o.Unconfigure(cfg) if err != nil { - return fmt.Errorf("unable to update config: %v", err) - } - - log.Infof("Flushing cri-o config to %v", o.config) - n, err := cfg.Save(o.config) - if err != nil { - return fmt.Errorf("unable to flush config: %v", err) - } - if n == 0 { - log.Infof("Config file is empty, removed") + return fmt.Errorf("unable to unconfigure cri-o: %v", err) } err = RestartCrio(o) @@ -324,27 +299,6 @@ func cleanupConfig(o *options) error { return nil } -// ParseArgs parses the command line arguments to the CLI -func ParseArgs(c *cli.Context, o *options) error { - if o.runtimeDir != "" { - log.Debug("Runtime directory already set") - return nil - } - - args := c.Args() - - log.Infof("Parsing arguments: %v", args.Slice()) - if c.NArg() != 1 { - return fmt.Errorf("incorrect number of arguments") - } - - o.runtimeDir = args.Get(0) - - log.Infof("Successfully parsed arguments") - - return nil -} - func createHook(toolkitDir string, hookPath string) error { hook, err := os.Create(hookPath) if err != nil { @@ -385,49 +339,16 @@ func generateOciHook(toolkitDir string) podmanHook { return hook } -// UpdateConfig updates the cri-o config to include the NVIDIA Container Runtime -func UpdateConfig(cfg engine.Interface, o *options) error { - runtimes := operator.GetRuntimes( - operator.WithNvidiaRuntimeName(o.runtimeClass), - operator.WithSetAsDefault(o.setAsDefault), - operator.WithRoot(o.runtimeDir), - ) - for class, runtime := range runtimes { - err := cfg.AddRuntime(class, runtime.Path, runtime.SetAsDefault) - if err != nil { - return fmt.Errorf("unable to update config for runtime class '%v': %v", class, err) - } - } - - return nil -} - -// RevertConfig reverts the cri-o config to remove the NVIDIA Container Runtime -func RevertConfig(cfg engine.Interface, o *options) error { - runtimes := operator.GetRuntimes( - operator.WithNvidiaRuntimeName(o.runtimeClass), - operator.WithSetAsDefault(o.setAsDefault), - operator.WithRoot(o.runtimeDir), - ) - for class := range runtimes { - err := cfg.RemoveRuntime(class) - if err != nil { - return fmt.Errorf("unable to revert config for runtime class '%v': %v", class, err) - } - } - return nil -} - // RestartCrio restarts crio depending on the value of restartModeFlag func RestartCrio(o *options) error { - switch o.restartMode { + switch o.RestartMode { case restartModeNone: - log.Warnf("Skipping restart of crio due to --restart-mode=%v", o.restartMode) + log.Warnf("Skipping restart of crio due to --restart-mode=%v", o.RestartMode) return nil case restartModeSystemd: - return RestartCrioSystemd(o.hostRootMount) + return RestartCrioSystemd(o.HostRootMount) default: - return fmt.Errorf("invalid restart mode specified: %v", o.restartMode) + return fmt.Errorf("invalid restart mode specified: %v", o.RestartMode) } } diff --git a/tools/container/docker/docker.go b/tools/container/docker/docker.go index d6531766..1d2be6ef 100644 --- a/tools/container/docker/docker.go +++ b/tools/container/docker/docker.go @@ -23,9 +23,8 @@ import ( "syscall" "time" - "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine/docker" - "github.com/NVIDIA/nvidia-container-toolkit/tools/container/operator" + "github.com/NVIDIA/nvidia-container-toolkit/tools/container" log "github.com/sirupsen/logrus" cli "github.com/urfave/cli/v2" ) @@ -56,13 +55,7 @@ const ( // options stores the configuration from the command line or environment variables type options struct { - config string - socket string - runtimeName string - runtimeDir string - setAsDefault bool - restartMode string - hostRootMount string + container.Options } func main() { @@ -83,7 +76,7 @@ func main() { return Setup(c, &options) } setup.Before = func(c *cli.Context) error { - return ParseArgs(c, &options) + return container.ParseArgs(c, &options.Options) } // Create the 'cleanup' subcommand @@ -95,7 +88,7 @@ func main() { return Cleanup(c, &options) } cleanup.Before = func(c *cli.Context) error { - return ParseArgs(c, &options) + return container.ParseArgs(c, &options.Options) } // Register the subcommands with the top-level CLI @@ -113,28 +106,28 @@ func main() { Name: "config", Usage: "Path to docker config file", Value: defaultConfig, - Destination: &options.config, + Destination: &options.Config, EnvVars: []string{"DOCKER_CONFIG", "RUNTIME_CONFIG"}, }, &cli.StringFlag{ Name: "socket", Usage: "Path to the docker socket file", Value: defaultSocket, - Destination: &options.socket, + Destination: &options.Socket, EnvVars: []string{"DOCKER_SOCKET", "RUNTIME_SOCKET"}, }, &cli.StringFlag{ Name: "restart-mode", Usage: "Specify how docker should be restarted; If 'none' is selected it will not be restarted [signal | none]", Value: defaultRestartMode, - Destination: &options.restartMode, + Destination: &options.RestartMode, EnvVars: []string{"DOCKER_RESTART_MODE", "RUNTIME_RESTART_MODE"}, }, &cli.StringFlag{ Name: "host-root", Usage: "Specify the path to the host root to be used when restarting docker using systemd", Value: defaultHostRootMount, - Destination: &options.hostRootMount, + Destination: &options.HostRootMount, EnvVars: []string{"HOST_ROOT_MOUNT"}, // Restart using systemd is currently not supported. // We hide this option for the time being. @@ -145,21 +138,21 @@ func main() { Aliases: []string{"runtime-class", "nvidia-runtime-name"}, Usage: "Specify the name of the `nvidia` runtime. If set-as-default is selected, the runtime is used as the default runtime.", Value: defaultRuntimeName, - Destination: &options.runtimeName, + Destination: &options.RuntimeName, EnvVars: []string{"DOCKER_RUNTIME_NAME", "NVIDIA_RUNTIME_NAME"}, }, &cli.StringFlag{ Name: "nvidia-runtime-dir", Aliases: []string{"runtime-dir"}, Usage: "The path where the nvidia-container-runtime binaries are located. If this is not specified, the first argument will be used instead", - Destination: &options.runtimeDir, + Destination: &options.RuntimeDir, EnvVars: []string{"NVIDIA_RUNTIME_DIR"}, }, &cli.BoolFlag{ Name: "set-as-default", Usage: "Set the `nvidia` runtime as the default runtime. If --runtime-name is specified as `nvidia-experimental` the experimental runtime is set as the default runtime instead", Value: defaultSetAsDefault, - Destination: &options.setAsDefault, + Destination: &options.SetAsDefault, EnvVars: []string{"DOCKER_SET_AS_DEFAULT", "NVIDIA_RUNTIME_SET_AS_DEFAULT"}, Hidden: true, }, @@ -181,21 +174,15 @@ func Setup(c *cli.Context, o *options) error { log.Infof("Starting 'setup' for %v", c.App.Name) cfg, err := docker.New( - docker.WithPath(o.config), + docker.WithPath(o.Config), ) if err != nil { return fmt.Errorf("unable to load config: %v", err) } - err = UpdateConfig(cfg, o) + err = o.Configure(cfg) if err != nil { - return fmt.Errorf("unable to update config: %v", err) - } - - log.Infof("Flushing docker config to %v", o.config) - _, err = cfg.Save(o.config) - if err != nil { - return fmt.Errorf("unable to flush config: %v", err) + return fmt.Errorf("unable to configure docker: %v", err) } err = RestartDocker(o) @@ -213,24 +200,15 @@ func Cleanup(c *cli.Context, o *options) error { log.Infof("Starting 'cleanup' for %v", c.App.Name) cfg, err := docker.New( - docker.WithPath(o.config), + docker.WithPath(o.Config), ) if err != nil { return fmt.Errorf("unable to load config: %v", err) } - err = RevertConfig(cfg, o) + err = o.Unconfigure(cfg) if err != nil { - return fmt.Errorf("unable to update config: %v", err) - } - - log.Infof("Flushing docker config to %v", o.config) - n, err := cfg.Save(o.config) - if err != nil { - return fmt.Errorf("unable to flush config: %v", err) - } - if n == 0 { - log.Infof("Config file is empty, removed") + return fmt.Errorf("unable to unconfigure docker: %v", err) } err = RestartDocker(o) @@ -243,73 +221,18 @@ func Cleanup(c *cli.Context, o *options) error { return nil } -// ParseArgs parses the command line arguments to the CLI -func ParseArgs(c *cli.Context, o *options) error { - if o.runtimeDir != "" { - log.Debug("Runtime directory already set") - return nil - } - - args := c.Args() - - log.Infof("Parsing arguments: %v", args.Slice()) - if c.NArg() != 1 { - return fmt.Errorf("incorrect number of arguments") - } - - o.runtimeDir = args.Get(0) - - log.Infof("Successfully parsed arguments") - - return nil -} - -// UpdateConfig updates the docker config to include the nvidia runtimes -func UpdateConfig(cfg engine.Interface, o *options) error { - runtimes := operator.GetRuntimes( - operator.WithNvidiaRuntimeName(o.runtimeName), - operator.WithSetAsDefault(o.setAsDefault), - operator.WithRoot(o.runtimeDir), - ) - for name, runtime := range runtimes { - err := cfg.AddRuntime(name, runtime.Path, runtime.SetAsDefault) - if err != nil { - return fmt.Errorf("failed to update runtime %q: %v", name, err) - } - } - - return nil -} - -// RevertConfig reverts the docker config to remove the nvidia runtime -func RevertConfig(cfg engine.Interface, o *options) error { - runtimes := operator.GetRuntimes( - operator.WithNvidiaRuntimeName(o.runtimeName), - operator.WithSetAsDefault(o.setAsDefault), - operator.WithRoot(o.runtimeDir), - ) - for name := range runtimes { - err := cfg.RemoveRuntime(name) - if err != nil { - return fmt.Errorf("failed to remove runtime %q: %v", name, err) - } - } - - return nil -} - // RestartDocker restarts docker depending on the value of restartModeFlag func RestartDocker(o *options) error { - switch o.restartMode { + switch o.RestartMode { case restartModeNone: - log.Warnf("Skipping sending signal to docker due to --restart-mode=%v", o.restartMode) + log.Warnf("Skipping sending signal to docker due to --restart-mode=%v", o.RestartMode) case restartModeSignal: - err := SignalDocker(o.socket) + err := SignalDocker(o.Socket) if err != nil { return fmt.Errorf("unable to signal docker: %v", err) } default: - return fmt.Errorf("invalid restart mode specified: %v", o.restartMode) + return fmt.Errorf("invalid restart mode specified: %v", o.RestartMode) } return nil diff --git a/tools/container/docker/docker_test.go b/tools/container/docker/docker_test.go index 2bba2116..3174e377 100644 --- a/tools/container/docker/docker_test.go +++ b/tools/container/docker/docker_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine/docker" + "github.com/NVIDIA/nvidia-container-toolkit/tools/container" "github.com/stretchr/testify/require" ) @@ -56,14 +57,16 @@ func TestUpdateConfigDefaultRuntime(t *testing.T) { for i, tc := range testCases { o := &options{ - setAsDefault: tc.setAsDefault, - runtimeName: tc.runtimeName, - runtimeDir: runtimeDir, + Options: container.Options{ + RuntimeName: tc.runtimeName, + RuntimeDir: runtimeDir, + SetAsDefault: tc.setAsDefault, + }, } config := docker.Config(map[string]interface{}{}) - err := UpdateConfig(&config, o) + err := o.UpdateConfig(&config) require.NoError(t, err, "%d: %v", i, tc) defaultRuntimeName := config["default-runtime"] @@ -314,13 +317,15 @@ func TestUpdateConfig(t *testing.T) { } for i, tc := range testCases { - options := &options{ - setAsDefault: tc.setAsDefault, - runtimeName: tc.runtimeName, - runtimeDir: runtimeDir, + o := &options{ + Options: container.Options{ + RuntimeName: tc.runtimeName, + RuntimeDir: runtimeDir, + SetAsDefault: tc.setAsDefault, + }, } - err := UpdateConfig(&tc.config, options) + err := o.UpdateConfig(&tc.config) require.NoError(t, err, "%d: %v", i, tc) configContent, err := json.MarshalIndent(tc.config, "", " ") @@ -457,7 +462,8 @@ func TestRevertConfig(t *testing.T) { } for i, tc := range testCases { - err := RevertConfig(&tc.config, &options{}) + o := &options{} + err := o.RevertConfig(&tc.config) require.NoError(t, err, "%d: %v", i, tc) From 19ca4338f2925d2e9bddad9469b4223781ea8dee Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 9 Jun 2023 17:28:42 +0200 Subject: [PATCH 4/9] Add version info to config CLIs Signed-off-by: Evan Lezar --- tools/container/containerd/containerd.go | 3 ++- tools/container/crio/crio.go | 4 ++-- tools/container/docker/docker.go | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/container/containerd/containerd.go b/tools/container/containerd/containerd.go index a1e86d04..dc64f348 100644 --- a/tools/container/containerd/containerd.go +++ b/tools/container/containerd/containerd.go @@ -25,6 +25,7 @@ import ( "time" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine/containerd" + "github.com/NVIDIA/nvidia-container-toolkit/internal/info" "github.com/NVIDIA/nvidia-container-toolkit/tools/container" log "github.com/sirupsen/logrus" cli "github.com/urfave/cli/v2" @@ -78,7 +79,7 @@ func main() { c := cli.NewApp() c.Name = "containerd" c.Usage = "Update a containerd config with the nvidia-container-runtime" - c.Version = "0.1.0" + c.Version = info.GetVersionString() // Create the 'setup' subcommand setup := cli.Command{} diff --git a/tools/container/crio/crio.go b/tools/container/crio/crio.go index f44efc17..9bc12eca 100644 --- a/tools/container/crio/crio.go +++ b/tools/container/crio/crio.go @@ -25,6 +25,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/config" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine/crio" + "github.com/NVIDIA/nvidia-container-toolkit/internal/info" "github.com/NVIDIA/nvidia-container-toolkit/tools/container" log "github.com/sirupsen/logrus" cli "github.com/urfave/cli/v2" @@ -67,8 +68,7 @@ func main() { c := cli.NewApp() c.Name = "crio" c.Usage = "Update cri-o hooks to include the NVIDIA runtime hook" - c.ArgsUsage = "" - c.Version = "0.1.0" + c.Version = info.GetVersionString() // Create the 'setup' subcommand setup := cli.Command{} diff --git a/tools/container/docker/docker.go b/tools/container/docker/docker.go index 1d2be6ef..6bdcac74 100644 --- a/tools/container/docker/docker.go +++ b/tools/container/docker/docker.go @@ -24,6 +24,7 @@ import ( "time" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine/docker" + "github.com/NVIDIA/nvidia-container-toolkit/internal/info" "github.com/NVIDIA/nvidia-container-toolkit/tools/container" log "github.com/sirupsen/logrus" cli "github.com/urfave/cli/v2" @@ -65,7 +66,7 @@ func main() { c := cli.NewApp() c.Name = "docker" c.Usage = "Update docker config with the nvidia runtime" - c.Version = "0.1.0" + c.Version = info.GetVersionString() // Create the 'setup' subcommand setup := cli.Command{} From 524802df2bc392b326b940b116c575c1a88b4f49 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sat, 10 Jun 2023 12:41:53 +0200 Subject: [PATCH 5/9] Rework restart logic Signed-off-by: Evan Lezar --- tools/container/container.go | 46 ++++++++++++++++++++++++ tools/container/containerd/containerd.go | 46 +++--------------------- tools/container/crio/crio.go | 38 +++----------------- tools/container/docker/docker.go | 21 ++--------- 4 files changed, 57 insertions(+), 94 deletions(-) diff --git a/tools/container/container.go b/tools/container/container.go index 1d1ea8a5..23289e10 100644 --- a/tools/container/container.go +++ b/tools/container/container.go @@ -18,6 +18,8 @@ package container import ( "fmt" + "os" + "os/exec" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/engine" "github.com/NVIDIA/nvidia-container-toolkit/tools/container/operator" @@ -25,6 +27,12 @@ import ( "github.com/urfave/cli/v2" ) +const ( + restartModeNone = "none" + restartModeSignal = "signal" + restartModeSystemd = "systemd" +) + // Options defines the shared options for the CLIs to configure containers runtimes. type Options struct { Config string @@ -121,3 +129,41 @@ func (o Options) RevertConfig(cfg engine.Interface) error { return nil } + +// Restart restarts the specified service +func (o Options) Restart(service string, withSignal func(string) error) error { + switch o.RestartMode { + case restartModeNone: + logrus.Warnf("Skipping restart of %v due to --restart-mode=%v", service, o.RestartMode) + return nil + case restartModeSignal: + return withSignal(o.Socket) + case restartModeSystemd: + return o.SystemdRestart(service) + } + + return fmt.Errorf("invalid restart mode specified: %v", o.RestartMode) +} + +// SystemdRestart restarts the specified service using systemd +func (o Options) SystemdRestart(service string) error { + var args []string + var msg string + if o.HostRootMount != "" { + msg = " on host" + args = append(args, "chroot", o.HostRootMount) + } + args = append(args, "systemctl", "restart", service) + + logrus.Infof("Restarting %v%v using systemd: %v", service, msg, args) + + cmd := exec.Command(args[0], args[1:]...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + err := cmd.Run() + if err != nil { + return fmt.Errorf("error restarting %v using systemd: %v", service, err) + } + + return nil +} diff --git a/tools/container/containerd/containerd.go b/tools/container/containerd/containerd.go index dc64f348..e864b6d3 100644 --- a/tools/container/containerd/containerd.go +++ b/tools/container/containerd/containerd.go @@ -20,7 +20,6 @@ import ( "fmt" "net" "os" - "os/exec" "syscall" "time" @@ -32,10 +31,6 @@ import ( ) const ( - restartModeSignal = "signal" - restartModeSystemd = "systemd" - restartModeNone = "none" - nvidiaRuntimeName = "nvidia" nvidiaRuntimeBinary = "nvidia-container-runtime" nvidiaExperimentalRuntimeName = "nvidia-experimental" @@ -46,7 +41,7 @@ const ( defaultRuntimeClass = "nvidia" defaultRuntmeType = "io.containerd.runc.v2" defaultSetAsDefault = true - defaultRestartMode = restartModeSignal + defaultRestartMode = "signal" defaultHostRootMount = "/host" reloadBackoff = 5 * time.Second @@ -257,31 +252,16 @@ func Cleanup(c *cli.Context, o *options) error { // RestartContainerd restarts containerd depending on the value of restartModeFlag func RestartContainerd(o *options) error { - switch o.RestartMode { - case restartModeNone: - log.Warnf("Skipping sending signal to containerd due to --restart-mode=%v", o.RestartMode) - return nil - case restartModeSignal: - err := SignalContainerd(o) - if err != nil { - return fmt.Errorf("unable to signal containerd: %v", err) - } - case restartModeSystemd: - return RestartContainerdSystemd(o.HostRootMount) - default: - return fmt.Errorf("Invalid restart mode specified: %v", o.RestartMode) - } - - return nil + return o.Restart("containerd", SignalContainerd) } // SignalContainerd sends a SIGHUP signal to the containerd daemon -func SignalContainerd(o *options) error { +func SignalContainerd(socket string) error { log.Infof("Sending SIGHUP signal to containerd") // Wrap the logic to perform the SIGHUP in a function so we can retry it on failure retriable := func() error { - conn, err := net.Dial("unix", o.Socket) + conn, err := net.Dial("unix", socket) if err != nil { return fmt.Errorf("unable to dial: %v", err) } @@ -355,24 +335,6 @@ func SignalContainerd(o *options) error { return nil } -// RestartContainerdSystemd restarts containerd using systemctl -func RestartContainerdSystemd(hostRootMount string) error { - log.Infof("Restarting containerd using systemd and host root mounted at %v", hostRootMount) - - command := "chroot" - args := []string{hostRootMount, "systemctl", "restart", "containerd"} - - cmd := exec.Command(command, args...) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - err := cmd.Run() - if err != nil { - return fmt.Errorf("error restarting containerd using systemd: %v", err) - } - - return nil -} - // containerAnnotationsFromCDIPrefixes returns the container annotations to set for the given CDI prefixes. func (o *options) containerAnnotationsFromCDIPrefixes() []string { var annotations []string diff --git a/tools/container/crio/crio.go b/tools/container/crio/crio.go index 9bc12eca..6b0cf6de 100644 --- a/tools/container/crio/crio.go +++ b/tools/container/crio/crio.go @@ -20,7 +20,6 @@ import ( "encoding/json" "fmt" "os" - "os/exec" "path/filepath" "github.com/NVIDIA/nvidia-container-toolkit/internal/config" @@ -32,9 +31,6 @@ import ( ) const ( - restartModeSystemd = "systemd" - restartModeNone = "none" - defaultConfigMode = "hook" // Hook-based settings @@ -46,7 +42,7 @@ const ( defaultSocket = "/var/run/crio/crio.sock" defaultRuntimeClass = "nvidia" defaultSetAsDefault = true - defaultRestartMode = restartModeSystemd + defaultRestartMode = "systemd" defaultHostRootMount = "/host" ) @@ -117,7 +113,8 @@ func main() { Value: "", Destination: &options.Socket, EnvVars: []string{"CRIO_SOCKET", "RUNTIME_SOCKET"}, - Hidden: true, + // Note: We hide this option since restarting cri-o via a socket is not supported. + Hidden: true, }, &cli.StringFlag{ Name: "restart-mode", @@ -179,7 +176,6 @@ func main() { Destination: &options.configMode, EnvVars: []string{"CRIO_CONFIG_MODE"}, }, - // The flags below are only used by the 'setup' command. } // Update the subcommand flags with the common subcommand flags @@ -341,31 +337,5 @@ func generateOciHook(toolkitDir string) podmanHook { // RestartCrio restarts crio depending on the value of restartModeFlag func RestartCrio(o *options) error { - switch o.RestartMode { - case restartModeNone: - log.Warnf("Skipping restart of crio due to --restart-mode=%v", o.RestartMode) - return nil - case restartModeSystemd: - return RestartCrioSystemd(o.HostRootMount) - default: - return fmt.Errorf("invalid restart mode specified: %v", o.RestartMode) - } -} - -// RestartCrioSystemd restarts cri-o using systemctl -func RestartCrioSystemd(hostRootMount string) error { - log.Infof("Restarting cri-o using systemd and host root mounted at %v", hostRootMount) - - command := "chroot" - args := []string{hostRootMount, "systemctl", "restart", "crio"} - - cmd := exec.Command(command, args...) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - err := cmd.Run() - if err != nil { - return fmt.Errorf("error restarting crio using systemd: %v", err) - } - - return nil + return o.Restart("crio", func(string) error { return fmt.Errorf("supporting crio via signal is unsupported") }) } diff --git a/tools/container/docker/docker.go b/tools/container/docker/docker.go index 6bdcac74..7d2c671d 100644 --- a/tools/container/docker/docker.go +++ b/tools/container/docker/docker.go @@ -31,9 +31,6 @@ import ( ) const ( - restartModeSignal = "signal" - restartModeNone = "none" - nvidiaRuntimeName = "nvidia" nvidiaRuntimeBinary = "nvidia-container-runtime" nvidiaExperimentalRuntimeName = "nvidia-experimental" @@ -44,7 +41,7 @@ const ( defaultSetAsDefault = true // defaultRuntimeName specifies the NVIDIA runtime to be use as the default runtime if setting the default runtime is enabled defaultRuntimeName = nvidiaRuntimeName - defaultRestartMode = restartModeSignal + defaultRestartMode = "signal" defaultHostRootMount = "/host" reloadBackoff = 5 * time.Second @@ -119,7 +116,7 @@ func main() { }, &cli.StringFlag{ Name: "restart-mode", - Usage: "Specify how docker should be restarted; If 'none' is selected it will not be restarted [signal | none]", + Usage: "Specify how docker should be restarted; If 'none' is selected it will not be restarted [signal | systemd | none ]", Value: defaultRestartMode, Destination: &options.RestartMode, EnvVars: []string{"DOCKER_RESTART_MODE", "RUNTIME_RESTART_MODE"}, @@ -224,19 +221,7 @@ func Cleanup(c *cli.Context, o *options) error { // RestartDocker restarts docker depending on the value of restartModeFlag func RestartDocker(o *options) error { - switch o.RestartMode { - case restartModeNone: - log.Warnf("Skipping sending signal to docker due to --restart-mode=%v", o.RestartMode) - case restartModeSignal: - err := SignalDocker(o.Socket) - if err != nil { - return fmt.Errorf("unable to signal docker: %v", err) - } - default: - return fmt.Errorf("invalid restart mode specified: %v", o.RestartMode) - } - - return nil + return o.Restart("docker", SignalDocker) } // SignalDocker sends a SIGHUP signal to docker daemon From 591e6109056834d75be8799c971b78b11e292a31 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 11 Jun 2023 11:38:22 +0200 Subject: [PATCH 6/9] Remove unused constants and variables Signed-off-by: Evan Lezar --- tools/container/containerd/containerd.go | 11 ----------- tools/container/docker/docker.go | 8 +------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/tools/container/containerd/containerd.go b/tools/container/containerd/containerd.go index e864b6d3..e4f941ef 100644 --- a/tools/container/containerd/containerd.go +++ b/tools/container/containerd/containerd.go @@ -31,11 +31,6 @@ import ( ) const ( - nvidiaRuntimeName = "nvidia" - nvidiaRuntimeBinary = "nvidia-container-runtime" - nvidiaExperimentalRuntimeName = "nvidia-experimental" - nvidiaExperimentalRuntimeBinary = "nvidia-container-runtime.experimental" - defaultConfig = "/etc/containerd/config.toml" defaultSocket = "/run/containerd/containerd.sock" defaultRuntimeClass = "nvidia" @@ -50,12 +45,6 @@ const ( socketMessageToGetPID = "" ) -// nvidiaRuntimeBinaries defines a map of runtime names to binary names -var nvidiaRuntimeBinaries = map[string]string{ - nvidiaRuntimeName: nvidiaRuntimeBinary, - nvidiaExperimentalRuntimeName: nvidiaExperimentalRuntimeBinary, -} - // options stores the configuration from the command line or environment variables type options struct { container.Options diff --git a/tools/container/docker/docker.go b/tools/container/docker/docker.go index 7d2c671d..90b5bc05 100644 --- a/tools/container/docker/docker.go +++ b/tools/container/docker/docker.go @@ -31,23 +31,17 @@ import ( ) const ( - nvidiaRuntimeName = "nvidia" - nvidiaRuntimeBinary = "nvidia-container-runtime" - nvidiaExperimentalRuntimeName = "nvidia-experimental" - nvidiaExperimentalRuntimeBinary = "nvidia-container-runtime.experimental" - defaultConfig = "/etc/docker/daemon.json" defaultSocket = "/var/run/docker.sock" defaultSetAsDefault = true // defaultRuntimeName specifies the NVIDIA runtime to be use as the default runtime if setting the default runtime is enabled - defaultRuntimeName = nvidiaRuntimeName + defaultRuntimeName = "nvidia" defaultRestartMode = "signal" defaultHostRootMount = "/host" reloadBackoff = 5 * time.Second maxReloadAttempts = 6 - defaultDockerRuntime = "runc" socketMessageToGetPID = "GET /info HTTP/1.0\r\n\r\n" ) From c9a8b7f335f226c8483373d8846c42d40e7ac42e Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 11 Jun 2023 12:46:08 +0200 Subject: [PATCH 7/9] Ensure runtime dir is set for crio cleanup Signed-off-by: Evan Lezar --- test/container/crio_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/container/crio_test.sh b/test/container/crio_test.sh index a883debb..90c3a691 100644 --- a/test/container/crio_test.sh +++ b/test/container/crio_test.sh @@ -27,7 +27,7 @@ testing::crio::hook_created() { } testing::crio::hook_cleanup() { - testing::docker_run::toolkit::shell 'crio cleanup' + testing::docker_run::toolkit::shell 'crio cleanup --nvidia-runtime-dir /run/nvidia/toolkit/' test -z "$(ls -A "${shared_dir}${CRIO_HOOKS_DIR}")" } From a4e13c5197570cbf10ef41147212d5136253ed4d Mon Sep 17 00:00:00 2001 From: Carlos Eduardo Arango Gutierrez Date: Mon, 26 Jun 2023 12:29:34 +0200 Subject: [PATCH 8/9] Add entry to changelog Signed-off-by: Carlos Eduardo Arango Gutierrez --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52af4444..9bdf7452 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## v1.13.3 +* [toolkit-container] Allow same envars for all runtime configs + ## v1.13.2 * Add `nvidia-container-runtime-hook.path` config option to specify NVIDIA Container Runtime Hook path explicitly. From a9ccef609068d3ea90cd18a1e34dc41b935e001f Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 27 Jun 2023 14:13:00 +0200 Subject: [PATCH 9/9] Ensure common envvars have higher precedence Signed-off-by: Evan Lezar --- tools/container/containerd/containerd.go | 14 +++++++------- tools/container/crio/crio.go | 14 +++++++------- tools/container/docker/docker.go | 12 ++++++------ 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/tools/container/containerd/containerd.go b/tools/container/containerd/containerd.go index e4f941ef..8bbcf20c 100644 --- a/tools/container/containerd/containerd.go +++ b/tools/container/containerd/containerd.go @@ -105,29 +105,29 @@ func main() { Usage: "Path to the containerd config file", Value: defaultConfig, Destination: &options.Config, - EnvVars: []string{"CONTAINERD_CONFIG", "RUNTIME_CONFIG"}, + EnvVars: []string{"RUNTIME_CONFIG", "CONTAINERD_CONFIG"}, }, &cli.StringFlag{ Name: "socket", Usage: "Path to the containerd socket file", Value: defaultSocket, Destination: &options.Socket, - EnvVars: []string{"CONTAINERD_SOCKET", "RUNTIME_SOCKET"}, + EnvVars: []string{"RUNTIME_SOCKET", "CONTAINERD_SOCKET"}, }, &cli.StringFlag{ Name: "restart-mode", Usage: "Specify how containerd should be restarted; If 'none' is selected, it will not be restarted [signal | systemd | none]", Value: defaultRestartMode, Destination: &options.RestartMode, - EnvVars: []string{"CONTAINERD_RESTART_MODE", "RUNTIME_RESTART_MODE"}, + EnvVars: []string{"RUNTIME_RESTART_MODE", "CONTAINERD_RESTART_MODE"}, }, &cli.StringFlag{ - Name: "runtime-class", - Aliases: []string{"runtime-name", "nvidia-runtime-name"}, + Name: "runtime-name", + Aliases: []string{"nvidia-runtime-name", "runtime-class"}, Usage: "The name of the runtime class to set for the nvidia-container-runtime", Value: defaultRuntimeClass, Destination: &options.RuntimeName, - EnvVars: []string{"CONTAINERD_RUNTIME_CLASS", "NVIDIA_RUNTIME_NAME"}, + EnvVars: []string{"NVIDIA_RUNTIME_NAME", "CONTAINERD_RUNTIME_CLASS"}, }, &cli.StringFlag{ Name: "nvidia-runtime-dir", @@ -141,7 +141,7 @@ func main() { Usage: "Set nvidia-container-runtime as the default runtime", Value: defaultSetAsDefault, Destination: &options.SetAsDefault, - EnvVars: []string{"CONTAINERD_SET_AS_DEFAULT", "NVIDIA_RUNTIME_SET_AS_DEFAULT"}, + EnvVars: []string{"NVIDIA_RUNTIME_SET_AS_DEFAULT", "CONTAINERD_SET_AS_DEFAULT"}, Hidden: true, }, &cli.StringFlag{ diff --git a/tools/container/crio/crio.go b/tools/container/crio/crio.go index 6b0cf6de..53065a1f 100644 --- a/tools/container/crio/crio.go +++ b/tools/container/crio/crio.go @@ -105,14 +105,14 @@ func main() { Usage: "Path to the cri-o config file", Value: defaultConfig, Destination: &options.Config, - EnvVars: []string{"CRIO_CONFIG", "RUNTIME_CONFIG"}, + EnvVars: []string{"RUNTIME_CONFIG", "CRIO_CONFIG"}, }, &cli.StringFlag{ Name: "socket", Usage: "Path to the crio socket file", Value: "", Destination: &options.Socket, - EnvVars: []string{"CRIO_SOCKET", "RUNTIME_SOCKET"}, + EnvVars: []string{"RUNTIME_SOCKET", "CRIO_SOCKET"}, // Note: We hide this option since restarting cri-o via a socket is not supported. Hidden: true, }, @@ -121,15 +121,15 @@ func main() { Usage: "Specify how cri-o should be restarted; If 'none' is selected, it will not be restarted [systemd | none]", Value: defaultRestartMode, Destination: &options.RestartMode, - EnvVars: []string{"CRIO_RESTART_MODE", "RUNTIME_RESTART_MODE"}, + EnvVars: []string{"RUNTIME_RESTART_MODE", "CRIO_RESTART_MODE"}, }, &cli.StringFlag{ - Name: "runtime-class", - Aliases: []string{"runtime-name", "nvidia-runtime-name"}, + Name: "runtime-name", + Aliases: []string{"nvidia-runtime-name", "runtime-class"}, Usage: "The name of the runtime class to set for the nvidia-container-runtime", Value: defaultRuntimeClass, Destination: &options.RuntimeName, - EnvVars: []string{"CRIO_RUNTIME_CLASS", "NVIDIA_RUNTIME_NAME"}, + EnvVars: []string{"NVIDIA_RUNTIME_NAME", "CRIO_RUNTIME_CLASS"}, }, &cli.StringFlag{ Name: "nvidia-runtime-dir", @@ -143,7 +143,7 @@ func main() { Usage: "Set nvidia-container-runtime as the default runtime", Value: defaultSetAsDefault, Destination: &options.SetAsDefault, - EnvVars: []string{"CRIO_SET_AS_DEFAULT", "NVIDIA_RUNTIME_SET_AS_DEFAULT"}, + EnvVars: []string{"NVIDIA_RUNTIME_SET_AS_DEFAULT", "CRIO_SET_AS_DEFAULT"}, Hidden: true, }, &cli.StringFlag{ diff --git a/tools/container/docker/docker.go b/tools/container/docker/docker.go index 90b5bc05..02e1b96f 100644 --- a/tools/container/docker/docker.go +++ b/tools/container/docker/docker.go @@ -99,21 +99,21 @@ func main() { Usage: "Path to docker config file", Value: defaultConfig, Destination: &options.Config, - EnvVars: []string{"DOCKER_CONFIG", "RUNTIME_CONFIG"}, + EnvVars: []string{"RUNTIME_CONFIG", "DOCKER_CONFIG"}, }, &cli.StringFlag{ Name: "socket", Usage: "Path to the docker socket file", Value: defaultSocket, Destination: &options.Socket, - EnvVars: []string{"DOCKER_SOCKET", "RUNTIME_SOCKET"}, + EnvVars: []string{"RUNTIME_SOCKET", "DOCKER_SOCKET"}, }, &cli.StringFlag{ Name: "restart-mode", Usage: "Specify how docker should be restarted; If 'none' is selected it will not be restarted [signal | systemd | none ]", Value: defaultRestartMode, Destination: &options.RestartMode, - EnvVars: []string{"DOCKER_RESTART_MODE", "RUNTIME_RESTART_MODE"}, + EnvVars: []string{"RUNTIME_RESTART_MODE", "DOCKER_RESTART_MODE"}, }, &cli.StringFlag{ Name: "host-root", @@ -127,11 +127,11 @@ func main() { }, &cli.StringFlag{ Name: "runtime-name", - Aliases: []string{"runtime-class", "nvidia-runtime-name"}, + Aliases: []string{"nvidia-runtime-name", "runtime-class"}, Usage: "Specify the name of the `nvidia` runtime. If set-as-default is selected, the runtime is used as the default runtime.", Value: defaultRuntimeName, Destination: &options.RuntimeName, - EnvVars: []string{"DOCKER_RUNTIME_NAME", "NVIDIA_RUNTIME_NAME"}, + EnvVars: []string{"NVIDIA_RUNTIME_NAME", "DOCKER_RUNTIME_NAME"}, }, &cli.StringFlag{ Name: "nvidia-runtime-dir", @@ -145,7 +145,7 @@ func main() { Usage: "Set the `nvidia` runtime as the default runtime. If --runtime-name is specified as `nvidia-experimental` the experimental runtime is set as the default runtime instead", Value: defaultSetAsDefault, Destination: &options.SetAsDefault, - EnvVars: []string{"DOCKER_SET_AS_DEFAULT", "NVIDIA_RUNTIME_SET_AS_DEFAULT"}, + EnvVars: []string{"NVIDIA_RUNTIME_SET_AS_DEFAULT", "DOCKER_SET_AS_DEFAULT"}, Hidden: true, }, }