From f07d110e85729216c7af80fe84c3ef3e93b846de Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 8 Apr 2022 07:18:49 +0200 Subject: [PATCH 1/3] Use DefaultExecutableDir to determine default paths This change adds a DefaultExecutableDir = /usr/bin constant that is used to construct default paths for executables instead of specifying these explicitly. Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime/modifier/stable.go | 10 +++++----- internal/config/config.go | 3 +++ internal/discover/legacy.go | 8 +++++--- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/cmd/nvidia-container-runtime/modifier/stable.go b/cmd/nvidia-container-runtime/modifier/stable.go index fa0fb8f1..194a337f 100644 --- a/cmd/nvidia-container-runtime/modifier/stable.go +++ b/cmd/nvidia-container-runtime/modifier/stable.go @@ -19,18 +19,18 @@ package modifier import ( "os" "os/exec" + "path/filepath" "strings" + "github.com/NVIDIA/nvidia-container-toolkit/internal/config" "github.com/NVIDIA/nvidia-container-toolkit/internal/oci" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) const ( - nvidiaContainerRuntimeHookExecutable = "nvidia-container-runtime-hook" - nvidiaContainerRuntimeHookDefaultPath = "/usr/bin/nvidia-container-runtime-hook" - - nvidiaContainerToolkitExecutable = "nvidia-container-toolkit" + nvidiaContainerRuntimeHookExecutable = "nvidia-container-runtime-hook" + nvidiaContainerToolkitExecutable = "nvidia-container-toolkit" ) // NewStableRuntimeModifier creates an OCI spec modifier that inserts the NVIDIA Container Runtime Hook into an OCI @@ -52,7 +52,7 @@ type stableRuntimeModifier struct { func (m stableRuntimeModifier) Modify(spec *specs.Spec) error { path, err := exec.LookPath(nvidiaContainerRuntimeHookExecutable) if err != nil { - path = nvidiaContainerRuntimeHookDefaultPath + path = filepath.Join(config.DefaultExecutableDir, nvidiaContainerRuntimeHookExecutable) _, err = os.Stat(path) if err != nil { return err diff --git a/internal/config/config.go b/internal/config/config.go index 9f4f16f3..da6de2de 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -31,6 +31,9 @@ const ( ) var ( + // DefaultExecutableDir specifies the default path to use for executables if they cannot be located in the path. + DefaultExecutableDir = "/usr/bin" + configDir = "/etc/" ) diff --git a/internal/discover/legacy.go b/internal/discover/legacy.go index adc4c0c5..d95d6833 100644 --- a/internal/discover/legacy.go +++ b/internal/discover/legacy.go @@ -17,6 +17,9 @@ package discover import ( + "path/filepath" + + "github.com/NVIDIA/nvidia-container-toolkit/internal/config" "github.com/NVIDIA/nvidia-container-toolkit/internal/lookup" "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" "github.com/sirupsen/logrus" @@ -41,14 +44,13 @@ type legacy struct { var _ Discover = (*legacy)(nil) const ( - nvidiaContainerRuntimeHookExecutable = "nvidia-container-runtime-hook" - nvidiaContainerRuntimeHookDefaultFilePath = "/usr/bin/nvidia-container-runtime-hook" + nvidiaContainerRuntimeHookExecutable = "nvidia-container-runtime-hook" ) // Hooks returns the "legacy" NVIDIA Container Runtime hook. This mirrors the behaviour of the stable // modifier. func (d legacy) Hooks() ([]Hook, error) { - hookPath := nvidiaContainerRuntimeHookDefaultFilePath + hookPath := filepath.Join(config.DefaultExecutableDir, nvidiaContainerRuntimeHookExecutable) targets, err := d.lookup.Locate(nvidiaContainerRuntimeHookExecutable) if err != nil { d.logger.Warnf("Failed to locate %v: %v", nvidiaContainerRuntimeHookExecutable, err) From 196d5c5461bf7e28159b38b0862fd41f496b0ceb Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 8 Apr 2022 07:24:39 +0200 Subject: [PATCH 2/3] Move NVIDIA Container Runtime Hook executable name to shared constant Signed-off-by: Evan Lezar --- .../modifier/hook_remover.go | 5 +++-- .../modifier/stable.go | 11 +++-------- internal/config/config.go | 5 +++++ internal/discover/legacy.go | 19 ++++++++----------- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/cmd/nvidia-container-runtime/modifier/hook_remover.go b/cmd/nvidia-container-runtime/modifier/hook_remover.go index 7df4b2f1..797de81f 100644 --- a/cmd/nvidia-container-runtime/modifier/hook_remover.go +++ b/cmd/nvidia-container-runtime/modifier/hook_remover.go @@ -20,6 +20,7 @@ import ( "fmt" "path/filepath" + "github.com/NVIDIA/nvidia-container-toolkit/internal/config" "github.com/NVIDIA/nvidia-container-toolkit/internal/oci" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" @@ -60,8 +61,8 @@ func (m nvidiaContainerRuntimeHookRemover) Modify(spec *specs.Spec) error { // nvidia-container-runtime or docker when specifying the --gpus flag. func isNVIDIAContainerRuntimeHook(hook *specs.Hook) bool { bins := map[string]struct{}{ - nvidiaContainerRuntimeHookExecutable: {}, - nvidiaContainerToolkitExecutable: {}, + config.NVIDIAContainerRuntimeHookExecutable: {}, + config.NVIDIAContainerToolkitExecutable: {}, } _, exists := bins[filepath.Base(hook.Path)] diff --git a/cmd/nvidia-container-runtime/modifier/stable.go b/cmd/nvidia-container-runtime/modifier/stable.go index 194a337f..57bacf0b 100644 --- a/cmd/nvidia-container-runtime/modifier/stable.go +++ b/cmd/nvidia-container-runtime/modifier/stable.go @@ -28,11 +28,6 @@ import ( "github.com/sirupsen/logrus" ) -const ( - nvidiaContainerRuntimeHookExecutable = "nvidia-container-runtime-hook" - nvidiaContainerToolkitExecutable = "nvidia-container-toolkit" -) - // NewStableRuntimeModifier creates an OCI spec modifier that inserts the NVIDIA Container Runtime Hook into an OCI // spec. The specified logger is used to capture log output. func NewStableRuntimeModifier(logger *logrus.Logger) oci.SpecModifier { @@ -50,9 +45,9 @@ type stableRuntimeModifier struct { // Modify applies the required modification to the incoming OCI spec, inserting the nvidia-container-runtime-hook // as a prestart hook. func (m stableRuntimeModifier) Modify(spec *specs.Spec) error { - path, err := exec.LookPath(nvidiaContainerRuntimeHookExecutable) + path, err := exec.LookPath(config.NVIDIAContainerRuntimeHookExecutable) if err != nil { - path = filepath.Join(config.DefaultExecutableDir, nvidiaContainerRuntimeHookExecutable) + path = filepath.Join(config.DefaultExecutableDir, config.NVIDIAContainerRuntimeHookExecutable) _, err = os.Stat(path) if err != nil { return err @@ -66,7 +61,7 @@ func (m stableRuntimeModifier) Modify(spec *specs.Spec) error { spec.Hooks = &specs.Hooks{} } else if len(spec.Hooks.Prestart) != 0 { for _, hook := range spec.Hooks.Prestart { - if strings.Contains(hook.Path, nvidiaContainerRuntimeHookExecutable) { + if strings.Contains(hook.Path, config.NVIDIAContainerRuntimeHookExecutable) { m.logger.Infof("existing nvidia prestart hook found in OCI spec") return nil } diff --git a/internal/config/config.go b/internal/config/config.go index da6de2de..19f41386 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -34,6 +34,11 @@ var ( // DefaultExecutableDir specifies the default path to use for executables if they cannot be located in the path. DefaultExecutableDir = "/usr/bin" + // NVIDIAContainerRuntimeHookExecutable is the executable name for the NVIDIA Container Runtime Hook + NVIDIAContainerRuntimeHookExecutable = "nvidia-container-runtime-hook" + // NVIDIAContainerToolkitExecutable is the executable name for the NVIDIA Container Toolkit (an alias for the NVIDIA Container Runtime Hook) + NVIDIAContainerToolkitExecutable = "nvidia-container-toolkit" + configDir = "/etc/" ) diff --git a/internal/discover/legacy.go b/internal/discover/legacy.go index d95d6833..4a2d8270 100644 --- a/internal/discover/legacy.go +++ b/internal/discover/legacy.go @@ -43,21 +43,18 @@ type legacy struct { var _ Discover = (*legacy)(nil) -const ( - nvidiaContainerRuntimeHookExecutable = "nvidia-container-runtime-hook" -) - -// Hooks returns the "legacy" NVIDIA Container Runtime hook. This mirrors the behaviour of the stable -// modifier. +// Hooks returns the "legacy" NVIDIA Container Runtime hook. This hook calls out +// to the nvidia-container-cli to make modifications to the container as defined +// in libnvidia-container. func (d legacy) Hooks() ([]Hook, error) { - hookPath := filepath.Join(config.DefaultExecutableDir, nvidiaContainerRuntimeHookExecutable) - targets, err := d.lookup.Locate(nvidiaContainerRuntimeHookExecutable) + hookPath := filepath.Join(config.DefaultExecutableDir, config.NVIDIAContainerRuntimeHookExecutable) + targets, err := d.lookup.Locate(config.NVIDIAContainerRuntimeHookExecutable) if err != nil { - d.logger.Warnf("Failed to locate %v: %v", nvidiaContainerRuntimeHookExecutable, err) + d.logger.Warnf("Failed to locate %v: %v", config.NVIDIAContainerRuntimeHookExecutable, err) } else if len(targets) == 0 { - d.logger.Warnf("%v not found", nvidiaContainerRuntimeHookExecutable) + d.logger.Warnf("%v not found", config.NVIDIAContainerRuntimeHookExecutable) } else { - d.logger.Debugf("Found %v candidates: %v", nvidiaContainerRuntimeHookExecutable, targets) + d.logger.Debugf("Found %v candidates: %v", config.NVIDIAContainerRuntimeHookExecutable, targets) hookPath = targets[0] } d.logger.Debugf("Using NVIDIA Container Runtime Hook path %v", hookPath) From ab7f25500fc2bd88007a9021977f950a5109de9d Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 8 Apr 2022 11:36:48 +0200 Subject: [PATCH 3/3] Fix creation of CSV parser in create-symlinks Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/hook/create-symlinks/create-symlinks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/nvidia-ctk/hook/create-symlinks/create-symlinks.go b/cmd/nvidia-ctk/hook/create-symlinks/create-symlinks.go index f8e6e835..aaf45e76 100644 --- a/cmd/nvidia-ctk/hook/create-symlinks/create-symlinks.go +++ b/cmd/nvidia-ctk/hook/create-symlinks/create-symlinks.go @@ -103,7 +103,7 @@ func (m command) run(c *cli.Context, cfg *config) error { var candidates []string for _, file := range csvFiles { - mountSpecs, err := csv.ParseFile(m.logger, file) + mountSpecs, err := csv.NewCSVFileParser(m.logger, file).Parse() if err != nil { m.logger.Debugf("Skipping CSV file %v: %v", file, err) continue