From 89b99ff7867550fada54e610f172a267060f8e80 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 13 Mar 2025 16:20:37 +0200 Subject: [PATCH 1/2] Remove deprecated --runtime-args from nvidia-ctk-installer The GPU Operator no longer sets the RUNTIME_ARGS environment variable to control the runtime and instead sets general environment variables. This change removes support for setting RUNTIME_ARGS in the CLI as these settings have no effect. Signed-off-by: Evan Lezar --- cmd/nvidia-ctk-installer/main.go | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/cmd/nvidia-ctk-installer/main.go b/cmd/nvidia-ctk-installer/main.go index b89eb81e..26ace1ed 100644 --- a/cmd/nvidia-ctk-installer/main.go +++ b/cmd/nvidia-ctk-installer/main.go @@ -22,8 +22,7 @@ const ( defaultPidFile = "/run/nvidia/toolkit/" + toolkitPidFilename toolkitSubDir = "toolkit" - defaultRuntime = "docker" - defaultRuntimeArgs = "" + defaultRuntime = "docker" ) var availableRuntimes = map[string]struct{}{"docker": {}, "crio": {}, "containerd": {}} @@ -34,12 +33,11 @@ var signalReceived = make(chan bool, 1) // options stores the command line arguments type options struct { - noDaemon bool - runtime string - runtimeArgs string - root string - pidFile string - sourceRoot string + noDaemon bool + runtime string + root string + pidFile string + sourceRoot string toolkitOptions toolkit.Options runtimeOptions runtime.Options @@ -124,15 +122,6 @@ func (a app) build() *cli.App { Destination: &options.runtime, EnvVars: []string{"RUNTIME"}, }, - // TODO: Remove runtime-args - &cli.StringFlag{ - Name: "runtime-args", - Aliases: []string{"u"}, - Usage: "arguments to pass to 'docker', 'crio', or 'containerd' setup command", - Value: defaultRuntimeArgs, - Destination: &options.runtimeArgs, - EnvVars: []string{"RUNTIME_ARGS"}, - }, &cli.StringFlag{ Name: "root", Value: a.defaultRoot, From 75a30af36aaf3cf3bddc77754aec96674c74960a Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 13 Mar 2025 16:33:19 +0200 Subject: [PATCH 2/2] Remove positional arguments from nvidia-ctk-installer Parsing positional arguments require additional processing instead of relying on named flags. This change switches to using a named flag for specifying the toolkit installation directory. Signed-off-by: Evan Lezar --- cmd/nvidia-ctk-installer/main.go | 79 ++++++++------------------- cmd/nvidia-ctk-installer/main_test.go | 65 +--------------------- 2 files changed, 24 insertions(+), 120 deletions(-) diff --git a/cmd/nvidia-ctk-installer/main.go b/cmd/nvidia-ctk-installer/main.go index 26ace1ed..fce109aa 100644 --- a/cmd/nvidia-ctk-installer/main.go +++ b/cmd/nvidia-ctk-installer/main.go @@ -5,7 +5,6 @@ import ( "os" "os/signal" "path/filepath" - "strings" "syscall" "github.com/urfave/cli/v2" @@ -20,7 +19,9 @@ import ( const ( toolkitPidFilename = "toolkit.pid" defaultPidFile = "/run/nvidia/toolkit/" + toolkitPidFilename - toolkitSubDir = "toolkit" + + defaultToolkitInstallDir = "/usr/local/nvidia" + toolkitSubDir = "toolkit" defaultRuntime = "docker" ) @@ -33,9 +34,10 @@ var signalReceived = make(chan bool, 1) // options stores the command line arguments type options struct { + toolkitInstallDir string + noDaemon bool runtime string - root string pidFile string sourceRoot string @@ -44,24 +46,17 @@ type options struct { } func (o options) toolkitRoot() string { - return filepath.Join(o.root, toolkitSubDir) + return filepath.Join(o.toolkitInstallDir, toolkitSubDir) } func main() { logger := logger.New() - - remainingArgs, root, err := ParseArgs(logger, os.Args) - if err != nil { - logger.Errorf("Error: unable to parse arguments: %v", err) - os.Exit(1) - } - - c := NewApp(logger, root) + c := NewApp(logger) // Run the CLI logger.Infof("Starting %v", c.Name) - if err := c.Run(remainingArgs); err != nil { - logger.Errorf("error running nvidia-toolkit: %v", err) + if err := c.Run(os.Args); err != nil { + logger.Errorf("error running %v: %v", c.Name, err) os.Exit(1) } @@ -71,18 +66,14 @@ func main() { // An app represents the nvidia-ctk-installer. type app struct { logger logger.Interface - // defaultRoot stores the root to use if the --root flag is not specified. - defaultRoot string toolkit *toolkit.Installer } // NewApp creates the CLI app fro the specified options. -// defaultRoot is used as the root if not specified via the --root flag. -func NewApp(logger logger.Interface, defaultRoot string) *cli.App { +func NewApp(logger logger.Interface) *cli.App { a := app{ - logger: logger, - defaultRoot: defaultRoot, + logger: logger, } return a.build() } @@ -94,9 +85,7 @@ func (a app) build() *cli.App { // Create the top-level CLI c := cli.NewApp() c.Name = "nvidia-ctk-installer" - c.Usage = "Install the nvidia-container-toolkit for use by a given runtime" - c.UsageText = "[DESTINATION] [-n | --no-daemon] [-r | --runtime] [-u | --runtime-args]" - c.Description = "DESTINATION points to the host path underneath which the nvidia-container-toolkit should be installed.\nIt will be installed at ${DESTINATION}/toolkit" + c.Usage = "Install the NVIDIA Container Toolkit and configure the specified runtime to use the `nvidia` runtime." c.Version = info.GetVersionString() c.Before = func(ctx *cli.Context) error { return a.Before(ctx, &options) @@ -123,11 +112,15 @@ func (a app) build() *cli.App { EnvVars: []string{"RUNTIME"}, }, &cli.StringFlag{ - Name: "root", - Value: a.defaultRoot, - Usage: "the folder where the NVIDIA Container Toolkit is to be installed. It will be installed to `ROOT`/toolkit", - Destination: &options.root, - EnvVars: []string{"ROOT"}, + Name: "toolkit-install-dir", + Aliases: []string{"root"}, + Usage: "The directory where the NVIDIA Container Toolkit is to be installed. " + + "The components of the toolkit will be installed to `ROOT`/toolkit. " + + "Note that in the case of a containerized installer, this is the path in the container and it is " + + "recommended that this match the path on the host.", + Value: defaultToolkitInstallDir, + Destination: &options.toolkitInstallDir, + EnvVars: []string{"TOOLKIT_INSTALL_DIR", "ROOT"}, }, &cli.StringFlag{ Name: "source-root", @@ -161,7 +154,7 @@ func (a *app) Before(c *cli.Context, o *options) error { } func (a *app) validateFlags(c *cli.Context, o *options) error { - if o.root == "" { + if o.toolkitInstallDir == "" { return fmt.Errorf("the install root must be specified") } if _, exists := availableRuntimes[o.runtime]; !exists { @@ -225,34 +218,6 @@ func (a *app) Run(c *cli.Context, o *options) error { return nil } -// ParseArgs checks if a single positional argument was defined and extracts this the root. -// If no positional arguments are defined, it is assumed that the root is specified as a flag. -func ParseArgs(logger logger.Interface, args []string) ([]string, string, error) { - logger.Infof("Parsing arguments") - - if len(args) < 2 { - return args, "", nil - } - - var lastPositionalArg int - for i, arg := range args { - if strings.HasPrefix(arg, "-") { - break - } - lastPositionalArg = i - } - - if lastPositionalArg == 0 { - return args, "", nil - } - - if lastPositionalArg == 1 { - return append([]string{args[0]}, args[2:]...), args[1], nil - } - - return nil, "", fmt.Errorf("unexpected positional argument(s) %v", args[2:lastPositionalArg+1]) -} - func (a *app) initialize(pidFile string) error { a.logger.Infof("Initializing") diff --git a/cmd/nvidia-ctk-installer/main_test.go b/cmd/nvidia-ctk-installer/main_test.go index 759ae8c1..b98bf3ea 100644 --- a/cmd/nvidia-ctk-installer/main_test.go +++ b/cmd/nvidia-ctk-installer/main_test.go @@ -17,7 +17,6 @@ package main import ( - "fmt" "os" "path/filepath" "strings" @@ -29,67 +28,6 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/test" ) -func TestParseArgs(t *testing.T) { - logger, _ := testlog.NewNullLogger() - testCases := []struct { - args []string - expectedRemaining []string - expectedRoot string - expectedError error - }{ - { - args: []string{}, - expectedRemaining: []string{}, - expectedRoot: "", - expectedError: nil, - }, - { - args: []string{"app"}, - expectedRemaining: []string{"app"}, - }, - { - args: []string{"app", "root"}, - expectedRemaining: []string{"app"}, - expectedRoot: "root", - }, - { - args: []string{"app", "--flag"}, - expectedRemaining: []string{"app", "--flag"}, - }, - { - args: []string{"app", "root", "--flag"}, - expectedRemaining: []string{"app", "--flag"}, - expectedRoot: "root", - }, - { - args: []string{"app", "root", "not-root", "--flag"}, - expectedError: fmt.Errorf("unexpected positional argument(s) [not-root]"), - }, - { - args: []string{"app", "root", "not-root"}, - expectedError: fmt.Errorf("unexpected positional argument(s) [not-root]"), - }, - { - args: []string{"app", "root", "not-root", "also"}, - expectedError: fmt.Errorf("unexpected positional argument(s) [not-root also]"), - }, - } - - for i, tc := range testCases { - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - remaining, root, err := ParseArgs(logger, tc.args) - if tc.expectedError != nil { - require.EqualError(t, err, tc.expectedError.Error()) - } else { - require.NoError(t, err) - } - - require.ElementsMatch(t, tc.expectedRemaining, remaining) - require.Equal(t, tc.expectedRoot, root) - }) - } -} - func TestApp(t *testing.T) { t.Setenv("__NVCT_TESTING_DEVICES_ARE_FILES", "true") logger, _ := testlog.NewNullLogger() @@ -468,10 +406,11 @@ swarm-resource = "" toolkitRoot := filepath.Join(testRoot, "toolkit-test") toolkitConfigFile := filepath.Join(toolkitRoot, "toolkit/.config/nvidia-container-runtime/config.toml") - app := NewApp(logger, toolkitRoot) + app := NewApp(logger) testArgs := []string{ "nvidia-ctk-installer", + "--toolkit-install-dir=" + toolkitRoot, "--no-daemon", "--cdi-output-dir=" + cdiOutputDir, "--config=" + runtimeConfigFile,