From 35df24d63af8df33ca4f287cd30dc989108fcabd Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 12 Dec 2022 15:26:21 +0100 Subject: [PATCH 1/5] Make handling of nvidia-ctk path consistent This change adds an --nvidia-ctk-path to the nvidia-ctk cdi generate command. This ensures that the executable path for the generated hooks can be specified consistently. Since the NVIDIA Container Runtime already allows for the executable path to be specified in the config the utility code to update the LDCache and create other nvidia-ctk hooks are also updated. Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/cdi/generate/common.go | 4 +- .../cdi/generate/device-folder-permissions.go | 23 ++++---- cmd/nvidia-ctk/cdi/generate/driver.go | 27 +++++----- cmd/nvidia-ctk/cdi/generate/full-gpu.go | 33 ++++++------ cmd/nvidia-ctk/cdi/generate/generate.go | 53 +++++++++++++++---- internal/discover/ldconfig.go | 26 ++------- internal/discover/ldconfig_test.go | 8 ++- test/release/Makefile | 3 +- 8 files changed, 96 insertions(+), 81 deletions(-) diff --git a/cmd/nvidia-ctk/cdi/generate/common.go b/cmd/nvidia-ctk/cdi/generate/common.go index 3e6cf944..7775acfb 100644 --- a/cmd/nvidia-ctk/cdi/generate/common.go +++ b/cmd/nvidia-ctk/cdi/generate/common.go @@ -27,7 +27,7 @@ import ( // NewCommonDiscoverer returns a discoverer for entities that are not associated with a specific CDI device. // This includes driver libraries and meta devices, for example. -func NewCommonDiscoverer(logger *logrus.Logger, root string, nvmllib nvml.Interface) (discover.Discover, error) { +func NewCommonDiscoverer(logger *logrus.Logger, root string, nvidiaCTKPath string, nvmllib nvml.Interface) (discover.Discover, error) { metaDevices := discover.NewDeviceDiscoverer( logger, lookup.NewCharDeviceLocator(logger, root), @@ -45,7 +45,7 @@ func NewCommonDiscoverer(logger *logrus.Logger, root string, nvmllib nvml.Interf return nil, fmt.Errorf("error constructing discoverer for graphics mounts: %v", err) } - driverFiles, err := NewDriverDiscoverer(logger, root, nvmllib) + driverFiles, err := NewDriverDiscoverer(logger, root, nvidiaCTKPath, nvmllib) if err != nil { return nil, fmt.Errorf("failed to create discoverer for driver files: %v", err) } diff --git a/cmd/nvidia-ctk/cdi/generate/device-folder-permissions.go b/cmd/nvidia-ctk/cdi/generate/device-folder-permissions.go index 4d180c80..d4602544 100644 --- a/cmd/nvidia-ctk/cdi/generate/device-folder-permissions.go +++ b/cmd/nvidia-ctk/cdi/generate/device-folder-permissions.go @@ -20,15 +20,15 @@ import ( "path/filepath" "github.com/NVIDIA/nvidia-container-toolkit/internal/discover" - "github.com/NVIDIA/nvidia-container-toolkit/internal/lookup" "github.com/container-orchestrated-devices/container-device-interface/specs-go" "github.com/sirupsen/logrus" ) type deviceFolderPermissions struct { - logger *logrus.Logger - root string - folders []string + logger *logrus.Logger + root string + nvidiaCTKPath string + folders []string } var _ discover.Discover = (*deviceFolderPermissions)(nil) @@ -39,7 +39,7 @@ var _ discover.Discover = (*deviceFolderPermissions)(nil) // The nested devices that are applicable to the NVIDIA GPU devices are: // - DRM devices at /dev/dri/* // - NVIDIA Caps devices at /dev/nvidia-caps/* -func NewDeviceFolderPermissionHookDiscoverer(logger *logrus.Logger, root string, deviceSpecs []specs.Device) (discover.Discover, error) { +func NewDeviceFolderPermissionHookDiscoverer(logger *logrus.Logger, root string, nvidiaCTKPath string, deviceSpecs []specs.Device) (discover.Discover, error) { var folders []string seen := make(map[string]bool) for _, device := range deviceSpecs { @@ -65,9 +65,10 @@ func NewDeviceFolderPermissionHookDiscoverer(logger *logrus.Logger, root string, } d := &deviceFolderPermissions{ - logger: logger, - root: root, - folders: folders, + logger: logger, + root: root, + nvidiaCTKPath: nvidiaCTKPath, + folders: folders, } return d, nil @@ -88,11 +89,9 @@ func (d *deviceFolderPermissions) Hooks() ([]discover.Hook, error) { for _, folder := range d.folders { args = append(args, "--path", folder) } + hook := discover.CreateNvidiaCTKHook( - d.logger, - lookup.NewExecutableLocator(d.logger, d.root), - nvidiaCTKExecutable, - nvidiaCTKDefaultFilePath, + d.nvidiaCTKPath, "chmod", args..., ) diff --git a/cmd/nvidia-ctk/cdi/generate/driver.go b/cmd/nvidia-ctk/cdi/generate/driver.go index 0408514c..c89718cb 100644 --- a/cmd/nvidia-ctk/cdi/generate/driver.go +++ b/cmd/nvidia-ctk/cdi/generate/driver.go @@ -29,22 +29,23 @@ import ( ) type driverLibraries struct { - logger *logrus.Logger - root string - libraries []string + logger *logrus.Logger + root string + nvidiaCTKPath string + libraries []string } var _ discover.Discover = (*driverLibraries)(nil) // NewDriverDiscoverer creates a discoverer for the libraries and binaries associated with a driver installation. // The supplied NVML Library is used to query the expected driver version. -func NewDriverDiscoverer(logger *logrus.Logger, root string, nvmllib nvml.Interface) (discover.Discover, error) { +func NewDriverDiscoverer(logger *logrus.Logger, root string, nvidiaCTKPath string, nvmllib nvml.Interface) (discover.Discover, error) { version, r := nvmllib.SystemGetDriverVersion() if r != nvml.SUCCESS { return nil, fmt.Errorf("failed to determine driver version: %v", r) } - libraries, err := NewDriverLibraryDiscoverer(logger, root, version) + libraries, err := NewDriverLibraryDiscoverer(logger, root, nvidiaCTKPath, version) if err != nil { return nil, fmt.Errorf("failed to create discoverer for driver libraries: %v", err) } @@ -63,16 +64,17 @@ func NewDriverDiscoverer(logger *logrus.Logger, root string, nvmllib nvml.Interf } // NewDriverLibraryDiscoverer creates a discoverer for the libraries associated with the specified driver version. -func NewDriverLibraryDiscoverer(logger *logrus.Logger, root string, version string) (discover.Discover, error) { +func NewDriverLibraryDiscoverer(logger *logrus.Logger, root string, nvidiaCTKPath, version string) (discover.Discover, error) { libraries, err := findVersionLibs(logger, root, version) if err != nil { return nil, fmt.Errorf("failed to get libraries for driver version: %v", err) } d := driverLibraries{ - logger: logger, - root: root, - libraries: libraries, + logger: logger, + root: root, + nvidiaCTKPath: nvidiaCTKPath, + libraries: libraries, } return &d, nil @@ -129,13 +131,8 @@ func (d *driverLibraries) Mounts() ([]discover.Mount, error) { // Hooks returns a hook that updates the LDCache for the specified driver library paths. func (d *driverLibraries) Hooks() ([]discover.Hook, error) { - locator := lookup.NewExecutableLocator(d.logger, d.root) - hook := discover.CreateLDCacheUpdateHook( - d.logger, - locator, - nvidiaCTKExecutable, - nvidiaCTKDefaultFilePath, + d.nvidiaCTKPath, d.libraries, ) diff --git a/cmd/nvidia-ctk/cdi/generate/full-gpu.go b/cmd/nvidia-ctk/cdi/generate/full-gpu.go index 71fc7ce1..7b19f246 100644 --- a/cmd/nvidia-ctk/cdi/generate/full-gpu.go +++ b/cmd/nvidia-ctk/cdi/generate/full-gpu.go @@ -31,15 +31,16 @@ import ( // byPathHookDiscoverer discovers the entities required for injecting by-path DRM device links type byPathHookDiscoverer struct { - logger *logrus.Logger - root string - pciBusID string + logger *logrus.Logger + root string + nvidiaCTKPath string + pciBusID string } var _ discover.Discover = (*byPathHookDiscoverer)(nil) // NewFullGPUDiscoverer creates a discoverer for the full GPU defined by the specified device. -func NewFullGPUDiscoverer(logger *logrus.Logger, root string, d device.Device) (discover.Discover, error) { +func NewFullGPUDiscoverer(logger *logrus.Logger, root string, nvidiaCTKPath string, d device.Device) (discover.Discover, error) { // TODO: The functionality to get device paths should be integrated into the go-nvlib/pkg/device.Device interface. // This will allow reuse here and in other code where the paths are queried such as the NVIDIA device plugin. minor, ret := d.GetMinorNumber() @@ -68,9 +69,10 @@ func NewFullGPUDiscoverer(logger *logrus.Logger, root string, d device.Device) ( ) byPathHooks := &byPathHookDiscoverer{ - logger: logger, - root: root, - pciBusID: pciBusID, + logger: logger, + root: root, + nvidiaCTKPath: nvidiaCTKPath, + pciBusID: pciBusID, } dd := discover.Merge( @@ -98,21 +100,18 @@ func (d *byPathHookDiscoverer) Hooks() ([]discover.Hook, error) { return nil, nil } - hookPath := "nvidia-ctk" - args := []string{hookPath, "hook", "create-symlinks"} + var args []string for _, l := range links { args = append(args, "--link", l) } - var hooks []discover.Hook - hook := discover.Hook{ - Lifecycle: "createContainer", - Path: hookPath, - Args: args, - } - hooks = append(hooks, hook) + hook := discover.CreateNvidiaCTKHook( + d.nvidiaCTKPath, + "create-symlinks", + args..., + ) - return hooks, nil + return []discover.Hook{hook}, nil } // Mounts returns an empty slice for a full GPU diff --git a/cmd/nvidia-ctk/cdi/generate/generate.go b/cmd/nvidia-ctk/cdi/generate/generate.go index 23922ba7..cb427b3d 100644 --- a/cmd/nvidia-ctk/cdi/generate/generate.go +++ b/cmd/nvidia-ctk/cdi/generate/generate.go @@ -25,6 +25,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/discover" "github.com/NVIDIA/nvidia-container-toolkit/internal/edits" + "github.com/NVIDIA/nvidia-container-toolkit/internal/lookup" "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" specs "github.com/container-orchestrated-devices/container-device-interface/specs-go" "github.com/sirupsen/logrus" @@ -47,9 +48,10 @@ type command struct { } type config struct { - output string - format string - root string + output string + format string + root string + nvidiaCTKPath string } // NewCommand constructs a generate-cdi command with the specified logger @@ -93,6 +95,11 @@ func (m command) build() *cli.Command { Usage: "Specify the root to use when discovering the entities that should be included in the CDI specification.", Destination: &cfg.root, }, + &cli.StringFlag{ + Name: "nvidia-ctk-path", + Usage: "Specify the path to use for the nvidia-ctk in the generated CDI specification. If this is left empty, the path will be searched.", + Destination: &cfg.nvidiaCTKPath, + }, } return &c @@ -111,7 +118,7 @@ func (m command) validateFlags(r *cli.Context, cfg *config) error { } func (m command) run(c *cli.Context, cfg *config) error { - spec, err := m.generateSpec(cfg.root) + spec, err := m.generateSpec(cfg.root, cfg.nvidiaCTKPath) if err != nil { return fmt.Errorf("failed to generate CDI spec: %v", err) } @@ -161,6 +168,30 @@ func (m command) run(c *cli.Context, cfg *config) error { return nil } +// findNvidiaCTK locates the nvidia-ctk executable to be used in hooks. +// If an override is specified, this is used instead. +func (m command) findNvidiaCTK(override string) string { + if override != "" { + m.logger.Debugf("Using specified NVIDIA Container Toolkit CLI path %v", override) + return override + } + + lookup := lookup.NewExecutableLocator(m.logger, "") + hookPath := nvidiaCTKDefaultFilePath + targets, err := lookup.Locate(nvidiaCTKExecutable) + if err != nil { + m.logger.Warnf("Failed to locate %v: %v", nvidiaCTKExecutable, err) + } else if len(targets) == 0 { + m.logger.Warnf("%v not found", nvidiaCTKExecutable) + } else { + m.logger.Debugf("Found %v candidates: %v", nvidiaCTKExecutable, targets) + hookPath = targets[0] + } + m.logger.Debugf("Using NVIDIA Container Toolkit CLI path %v", hookPath) + + return hookPath +} + func formatFromFilename(filename string) string { ext := filepath.Ext(filename) switch strings.ToLower(ext) { @@ -190,7 +221,7 @@ func writeToOutput(format string, data []byte, output io.Writer) error { return nil } -func (m command) generateSpec(root string) (*specs.Spec, error) { +func (m command) generateSpec(root string, nvidiaCTKPath string) (*specs.Spec, error) { nvmllib := nvml.New() if r := nvmllib.Init(); r != nvml.SUCCESS { return nil, r @@ -199,7 +230,9 @@ func (m command) generateSpec(root string) (*specs.Spec, error) { devicelib := device.New(device.WithNvml(nvmllib)) - deviceSpecs, err := m.generateDeviceSpecs(devicelib, root) + useNvidiaCTKPath := m.findNvidiaCTK(nvidiaCTKPath) + + deviceSpecs, err := m.generateDeviceSpecs(devicelib, root, useNvidiaCTKPath) if err != nil { return nil, fmt.Errorf("failed to create device CDI specs: %v", err) } @@ -226,12 +259,12 @@ func (m command) generateSpec(root string) (*specs.Spec, error) { allEdits.Append(ipcEdits) - common, err := NewCommonDiscoverer(m.logger, root, nvmllib) + common, err := NewCommonDiscoverer(m.logger, root, nvidiaCTKPath, nvmllib) if err != nil { return nil, fmt.Errorf("failed to create discoverer for common entities: %v", err) } - deviceFolderPermissionHooks, err := NewDeviceFolderPermissionHookDiscoverer(m.logger, root, deviceSpecs) + deviceFolderPermissionHooks, err := NewDeviceFolderPermissionHookDiscoverer(m.logger, root, nvidiaCTKPath, deviceSpecs) if err != nil { return nil, fmt.Errorf("failed to generated permission hooks for device nodes: %v", err) } @@ -255,7 +288,7 @@ func (m command) generateSpec(root string) (*specs.Spec, error) { return &spec, nil } -func (m command) generateDeviceSpecs(devicelib device.Interface, root string) ([]specs.Device, error) { +func (m command) generateDeviceSpecs(devicelib device.Interface, root string, nvidiaCTKPath string) ([]specs.Device, error) { var deviceSpecs []specs.Device err := devicelib.VisitDevices(func(i int, d device.Device) error { @@ -266,7 +299,7 @@ func (m command) generateDeviceSpecs(devicelib device.Interface, root string) ([ if isMigEnabled { return nil } - device, err := NewFullGPUDiscoverer(m.logger, root, d) + device, err := NewFullGPUDiscoverer(m.logger, root, nvidiaCTKPath, d) if err != nil { return fmt.Errorf("failed to create device: %v", err) } diff --git a/internal/discover/ldconfig.go b/internal/discover/ldconfig.go index d42fa4bf..19430041 100644 --- a/internal/discover/ldconfig.go +++ b/internal/discover/ldconfig.go @@ -57,27 +57,21 @@ func (d ldconfig) Hooks() ([]Hook, error) { return nil, fmt.Errorf("failed to discover mounts for ldcache update: %v", err) } h := CreateLDCacheUpdateHook( - d.logger, - d.lookup, d.nvidiaCTKExecutablePath, - nvidiaCTKDefaultFilePath, getLibraryPaths(mounts), ) return []Hook{h}, nil } // CreateLDCacheUpdateHook locates the NVIDIA Container Toolkit CLI and creates a hook for updating the LD Cache -func CreateLDCacheUpdateHook(logger *logrus.Logger, lookup lookup.Locator, executable string, defaultPath string, libraries []string) Hook { +func CreateLDCacheUpdateHook(executable string, libraries []string) Hook { var args []string for _, f := range uniqueFolders(libraries) { args = append(args, "--folder", f) } hook := CreateNvidiaCTKHook( - logger, - lookup, executable, - defaultPath, "update-ldcache", args..., ) @@ -87,23 +81,11 @@ func CreateLDCacheUpdateHook(logger *logrus.Logger, lookup lookup.Locator, execu } // CreateNvidiaCTKHook creates a hook which invokes the NVIDIA Container CLI hook subcommand. -func CreateNvidiaCTKHook(logger *logrus.Logger, lookup lookup.Locator, executable string, defaultPath string, hookName string, additionalArgs ...string) Hook { - hookPath := defaultPath - targets, err := lookup.Locate(executable) - if err != nil { - logger.Warnf("Failed to locate %v: %v", executable, err) - } else if len(targets) == 0 { - logger.Warnf("%v not found", executable) - } else { - logger.Debugf("Found %v candidates: %v", executable, targets) - hookPath = targets[0] - } - logger.Debugf("Using NVIDIA Container Toolkit CLI path %v", hookPath) - +func CreateNvidiaCTKHook(executable string, hookName string, additionalArgs ...string) Hook { return Hook{ Lifecycle: cdi.CreateContainerHook, - Path: hookPath, - Args: append([]string{filepath.Base(hookPath), "hook", hookName}, additionalArgs...), + Path: executable, + Args: append([]string{filepath.Base(executable), "hook", hookName}, additionalArgs...), } } diff --git a/internal/discover/ldconfig_test.go b/internal/discover/ldconfig_test.go index 763aa420..e160e437 100644 --- a/internal/discover/ldconfig_test.go +++ b/internal/discover/ldconfig_test.go @@ -24,12 +24,16 @@ import ( "github.com/stretchr/testify/require" ) +const ( + testNvidiaCTKPath = "/foo/bar/nvidia-ctk" +) + func TestLDCacheUpdateHook(t *testing.T) { logger, _ := testlog.NewNullLogger() cfg := Config{ Root: "/", - NVIDIAContainerToolkitCLIExecutablePath: "/foo/bar/nvidia-ctk", + NVIDIAContainerToolkitCLIExecutablePath: testNvidiaCTKPath, } testCases := []struct { @@ -86,7 +90,7 @@ func TestLDCacheUpdateHook(t *testing.T) { }, } expectedHook := Hook{ - Path: "/usr/bin/nvidia-ctk", + Path: testNvidiaCTKPath, Args: tc.expectedArgs, Lifecycle: "createContainer", } diff --git a/test/release/Makefile b/test/release/Makefile index 386468c1..18f0b156 100644 --- a/test/release/Makefile +++ b/test/release/Makefile @@ -14,7 +14,7 @@ WORKFLOW ?= nvidia-docker -DISTRIBUTIONS := ubuntu18.04 centos8 fedora35 +DISTRIBUTIONS := ubuntu20.04 ubuntu18.04 centos8 fedora35 IMAGE_TARGETS := $(patsubst %,image-%, $(DISTRIBUTIONS)) RUN_TARGETS := $(patsubst %,run-%, $(DISTRIBUTIONS)) @@ -33,6 +33,7 @@ $(IMAGE_TARGETS): image-%: $(DOCKERFILE) $(shell dirname $(DOCKERFILE)) +%-ubuntu20.04: ARCH ?= amd64 %-ubuntu18.04: ARCH ?= amd64 %-centos8: ARCH ?= x86_64 %-fedora35: ARCH ?= x86_64 From 09d42f0ad94f66b1a41ff8ed3f0600c97f84881f Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 18 Jan 2023 11:41:29 +0100 Subject: [PATCH 2/5] Remove 'Executable' from config struct member Signed-off-by: Evan Lezar --- internal/discover/discover.go | 7 ++++--- internal/discover/graphics.go | 2 +- internal/discover/ldconfig.go | 2 +- internal/discover/ldconfig_test.go | 4 ++-- internal/discover/symlinks.go | 2 +- internal/modifier/csv.go | 4 ++-- internal/modifier/graphics.go | 4 ++-- 7 files changed, 13 insertions(+), 12 deletions(-) diff --git a/internal/discover/discover.go b/internal/discover/discover.go index e65b360e..23dc4c13 100644 --- a/internal/discover/discover.go +++ b/internal/discover/discover.go @@ -18,8 +18,8 @@ package discover // Config represents the configuration options for discovery type Config struct { - Root string - NVIDIAContainerToolkitCLIExecutablePath string + Root string + NvidiaCTKPath string } // Device represents a discovered character device. @@ -41,8 +41,9 @@ type Hook struct { Args []string } -//go:generate moq -stub -out discover_mock.go . Discover // Discover defines an interface for discovering the devices, mounts, and hooks available on a system +// +//go:generate moq -stub -out discover_mock.go . Discover type Discover interface { Devices() ([]Device, error) Mounts() ([]Mount, error) diff --git a/internal/discover/graphics.go b/internal/discover/graphics.go index ab858405..fca66579 100644 --- a/internal/discover/graphics.go +++ b/internal/discover/graphics.go @@ -107,7 +107,7 @@ func newCreateDRMByPathSymlinks(logger *logrus.Logger, devices Discover, cfg *Co d := drmDevicesByPath{ logger: logger, lookup: lookup.NewExecutableLocator(logger, cfg.Root), - nvidiaCTKExecutablePath: cfg.NVIDIAContainerToolkitCLIExecutablePath, + nvidiaCTKExecutablePath: cfg.NvidiaCTKPath, root: cfg.Root, devicesFrom: devices, } diff --git a/internal/discover/ldconfig.go b/internal/discover/ldconfig.go index 19430041..48ad6611 100644 --- a/internal/discover/ldconfig.go +++ b/internal/discover/ldconfig.go @@ -32,7 +32,7 @@ func NewLDCacheUpdateHook(logger *logrus.Logger, mounts Discover, cfg *Config) ( logger: logger, mountsFrom: mounts, lookup: lookup.NewExecutableLocator(logger, cfg.Root), - nvidiaCTKExecutablePath: cfg.NVIDIAContainerToolkitCLIExecutablePath, + nvidiaCTKExecutablePath: cfg.NvidiaCTKPath, } return &d, nil diff --git a/internal/discover/ldconfig_test.go b/internal/discover/ldconfig_test.go index e160e437..daf5465c 100644 --- a/internal/discover/ldconfig_test.go +++ b/internal/discover/ldconfig_test.go @@ -32,8 +32,8 @@ func TestLDCacheUpdateHook(t *testing.T) { logger, _ := testlog.NewNullLogger() cfg := Config{ - Root: "/", - NVIDIAContainerToolkitCLIExecutablePath: testNvidiaCTKPath, + Root: "/", + NvidiaCTKPath: testNvidiaCTKPath, } testCases := []struct { diff --git a/internal/discover/symlinks.go b/internal/discover/symlinks.go index b758100b..15f1b0a4 100644 --- a/internal/discover/symlinks.go +++ b/internal/discover/symlinks.go @@ -40,7 +40,7 @@ func NewCreateSymlinksHook(logger *logrus.Logger, csvFiles []string, mounts Disc d := symlinks{ logger: logger, lookup: lookup.NewExecutableLocator(logger, cfg.Root), - nvidiaCTKExecutablePath: cfg.NVIDIAContainerToolkitCLIExecutablePath, + nvidiaCTKExecutablePath: cfg.NvidiaCTKPath, csvFiles: csvFiles, mountsFrom: mounts, } diff --git a/internal/modifier/csv.go b/internal/modifier/csv.go index 57f8deea..b8217def 100644 --- a/internal/modifier/csv.go +++ b/internal/modifier/csv.go @@ -62,8 +62,8 @@ func NewCSVModifier(logger *logrus.Logger, cfg *config.Config, ociSpec oci.Spec) logger.Infof("Constructing modifier from config: %+v", *cfg) config := &discover.Config{ - Root: cfg.NVIDIAContainerCLIConfig.Root, - NVIDIAContainerToolkitCLIExecutablePath: cfg.NVIDIACTKConfig.Path, + Root: cfg.NVIDIAContainerCLIConfig.Root, + NvidiaCTKPath: cfg.NVIDIACTKConfig.Path, } if err := checkRequirements(logger, image); err != nil { diff --git a/internal/modifier/graphics.go b/internal/modifier/graphics.go index 76fec386..22e236de 100644 --- a/internal/modifier/graphics.go +++ b/internal/modifier/graphics.go @@ -45,8 +45,8 @@ func NewGraphicsModifier(logger *logrus.Logger, cfg *config.Config, ociSpec oci. } config := &discover.Config{ - Root: cfg.NVIDIAContainerCLIConfig.Root, - NVIDIAContainerToolkitCLIExecutablePath: cfg.NVIDIACTKConfig.Path, + Root: cfg.NVIDIAContainerCLIConfig.Root, + NvidiaCTKPath: cfg.NVIDIACTKConfig.Path, } d, err := discover.NewGraphicsDiscoverer( logger, From ebbc47702d831b0ac8a67f8060a0a7aee2d7a585 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 18 Jan 2023 11:43:05 +0100 Subject: [PATCH 3/5] Remove 'Executable' from private struct member names Signed-off-by: Evan Lezar --- internal/discover/graphics.go | 28 ++++++++++++++-------------- internal/discover/ldconfig.go | 18 +++++++++--------- internal/discover/symlinks.go | 28 ++++++++++++++-------------- 3 files changed, 37 insertions(+), 37 deletions(-) diff --git a/internal/discover/graphics.go b/internal/discover/graphics.go index fca66579..2a9f3d72 100644 --- a/internal/discover/graphics.go +++ b/internal/discover/graphics.go @@ -95,21 +95,21 @@ func NewGraphicsMountsDiscoverer(logger *logrus.Logger, root string) (Discover, type drmDevicesByPath struct { None - logger *logrus.Logger - lookup lookup.Locator - nvidiaCTKExecutablePath string - root string - devicesFrom Discover + logger *logrus.Logger + lookup lookup.Locator + nvidiaCTKPath string + root string + devicesFrom Discover } // newCreateDRMByPathSymlinks creates a discoverer for a hook to create the by-path symlinks for DRM devices discovered by the specified devices discoverer func newCreateDRMByPathSymlinks(logger *logrus.Logger, devices Discover, cfg *Config) Discover { d := drmDevicesByPath{ - logger: logger, - lookup: lookup.NewExecutableLocator(logger, cfg.Root), - nvidiaCTKExecutablePath: cfg.NvidiaCTKPath, - root: cfg.Root, - devicesFrom: devices, + logger: logger, + lookup: lookup.NewExecutableLocator(logger, cfg.Root), + nvidiaCTKPath: cfg.NvidiaCTKPath, + root: cfg.Root, + devicesFrom: devices, } return &d @@ -133,13 +133,13 @@ func (d drmDevicesByPath) Hooks() ([]Hook, error) { } hookPath := nvidiaCTKDefaultFilePath - targets, err := d.lookup.Locate(d.nvidiaCTKExecutablePath) + targets, err := d.lookup.Locate(d.nvidiaCTKPath) if err != nil { - d.logger.Warnf("Failed to locate %v: %v", d.nvidiaCTKExecutablePath, err) + d.logger.Warnf("Failed to locate %v: %v", d.nvidiaCTKPath, err) } else if len(targets) == 0 { - d.logger.Warnf("%v not found", d.nvidiaCTKExecutablePath) + d.logger.Warnf("%v not found", d.nvidiaCTKPath) } else { - d.logger.Debugf("Found %v candidates: %v", d.nvidiaCTKExecutablePath, targets) + d.logger.Debugf("Found %v candidates: %v", d.nvidiaCTKPath, targets) hookPath = targets[0] } d.logger.Debugf("Using NVIDIA Container Toolkit CLI path %v", hookPath) diff --git a/internal/discover/ldconfig.go b/internal/discover/ldconfig.go index 48ad6611..308f6371 100644 --- a/internal/discover/ldconfig.go +++ b/internal/discover/ldconfig.go @@ -29,10 +29,10 @@ import ( // NewLDCacheUpdateHook creates a discoverer that updates the ldcache for the specified mounts. A logger can also be specified func NewLDCacheUpdateHook(logger *logrus.Logger, mounts Discover, cfg *Config) (Discover, error) { d := ldconfig{ - logger: logger, - mountsFrom: mounts, - lookup: lookup.NewExecutableLocator(logger, cfg.Root), - nvidiaCTKExecutablePath: cfg.NvidiaCTKPath, + logger: logger, + mountsFrom: mounts, + lookup: lookup.NewExecutableLocator(logger, cfg.Root), + nvidiaCTKPath: cfg.NvidiaCTKPath, } return &d, nil @@ -44,10 +44,10 @@ const ( type ldconfig struct { None - logger *logrus.Logger - mountsFrom Discover - lookup lookup.Locator - nvidiaCTKExecutablePath string + logger *logrus.Logger + mountsFrom Discover + lookup lookup.Locator + nvidiaCTKPath string } // Hooks checks the required mounts for libraries and returns a hook to update the LDcache for the discovered paths. @@ -57,7 +57,7 @@ func (d ldconfig) Hooks() ([]Hook, error) { return nil, fmt.Errorf("failed to discover mounts for ldcache update: %v", err) } h := CreateLDCacheUpdateHook( - d.nvidiaCTKExecutablePath, + d.nvidiaCTKPath, getLibraryPaths(mounts), ) return []Hook{h}, nil diff --git a/internal/discover/symlinks.go b/internal/discover/symlinks.go index 15f1b0a4..0507f699 100644 --- a/internal/discover/symlinks.go +++ b/internal/discover/symlinks.go @@ -28,21 +28,21 @@ import ( type symlinks struct { None - logger *logrus.Logger - lookup lookup.Locator - nvidiaCTKExecutablePath string - csvFiles []string - mountsFrom Discover + logger *logrus.Logger + lookup lookup.Locator + nvidiaCTKPath string + csvFiles []string + mountsFrom Discover } // NewCreateSymlinksHook creates a discoverer for a hook that creates required symlinks in the container func NewCreateSymlinksHook(logger *logrus.Logger, csvFiles []string, mounts Discover, cfg *Config) (Discover, error) { d := symlinks{ - logger: logger, - lookup: lookup.NewExecutableLocator(logger, cfg.Root), - nvidiaCTKExecutablePath: cfg.NvidiaCTKPath, - csvFiles: csvFiles, - mountsFrom: mounts, + logger: logger, + lookup: lookup.NewExecutableLocator(logger, cfg.Root), + nvidiaCTKPath: cfg.NvidiaCTKPath, + csvFiles: csvFiles, + mountsFrom: mounts, } return &d, nil @@ -51,13 +51,13 @@ func NewCreateSymlinksHook(logger *logrus.Logger, csvFiles []string, mounts Disc // Hooks returns a hook to create the symlinks from the required CSV files func (d symlinks) Hooks() ([]Hook, error) { hookPath := nvidiaCTKDefaultFilePath - targets, err := d.lookup.Locate(d.nvidiaCTKExecutablePath) + targets, err := d.lookup.Locate(d.nvidiaCTKPath) if err != nil { - d.logger.Warnf("Failed to locate %v: %v", d.nvidiaCTKExecutablePath, err) + d.logger.Warnf("Failed to locate %v: %v", d.nvidiaCTKPath, err) } else if len(targets) == 0 { - d.logger.Warnf("%v not found", d.nvidiaCTKExecutablePath) + d.logger.Warnf("%v not found", d.nvidiaCTKPath) } else { - d.logger.Debugf("Found %v candidates: %v", d.nvidiaCTKExecutablePath, targets) + d.logger.Debugf("Found %v candidates: %v", d.nvidiaCTKPath, targets) hookPath = targets[0] } d.logger.Debugf("Using NVIDIA Container Toolkit CLI path %v", hookPath) From 27347c98d99b92ab48b1c6f028dd023d25cfea74 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 19 Jan 2023 10:31:42 +0100 Subject: [PATCH 4/5] Consolidate code to find nvidia-ctk Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/cdi/generate/generate.go | 30 +----------- internal/discover/graphics.go | 20 ++------ internal/discover/hooks.go | 63 +++++++++++++++++++++++++ internal/discover/ldconfig.go | 14 ------ internal/discover/symlinks.go | 21 ++------- 5 files changed, 70 insertions(+), 78 deletions(-) create mode 100644 internal/discover/hooks.go diff --git a/cmd/nvidia-ctk/cdi/generate/generate.go b/cmd/nvidia-ctk/cdi/generate/generate.go index cb427b3d..bb7e021f 100644 --- a/cmd/nvidia-ctk/cdi/generate/generate.go +++ b/cmd/nvidia-ctk/cdi/generate/generate.go @@ -25,7 +25,6 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/discover" "github.com/NVIDIA/nvidia-container-toolkit/internal/edits" - "github.com/NVIDIA/nvidia-container-toolkit/internal/lookup" "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" specs "github.com/container-orchestrated-devices/container-device-interface/specs-go" "github.com/sirupsen/logrus" @@ -36,9 +35,6 @@ import ( ) const ( - nvidiaCTKExecutable = "nvidia-ctk" - nvidiaCTKDefaultFilePath = "/usr/bin/" + nvidiaCTKExecutable - formatJSON = "json" formatYAML = "yaml" ) @@ -168,30 +164,6 @@ func (m command) run(c *cli.Context, cfg *config) error { return nil } -// findNvidiaCTK locates the nvidia-ctk executable to be used in hooks. -// If an override is specified, this is used instead. -func (m command) findNvidiaCTK(override string) string { - if override != "" { - m.logger.Debugf("Using specified NVIDIA Container Toolkit CLI path %v", override) - return override - } - - lookup := lookup.NewExecutableLocator(m.logger, "") - hookPath := nvidiaCTKDefaultFilePath - targets, err := lookup.Locate(nvidiaCTKExecutable) - if err != nil { - m.logger.Warnf("Failed to locate %v: %v", nvidiaCTKExecutable, err) - } else if len(targets) == 0 { - m.logger.Warnf("%v not found", nvidiaCTKExecutable) - } else { - m.logger.Debugf("Found %v candidates: %v", nvidiaCTKExecutable, targets) - hookPath = targets[0] - } - m.logger.Debugf("Using NVIDIA Container Toolkit CLI path %v", hookPath) - - return hookPath -} - func formatFromFilename(filename string) string { ext := filepath.Ext(filename) switch strings.ToLower(ext) { @@ -230,7 +202,7 @@ func (m command) generateSpec(root string, nvidiaCTKPath string) (*specs.Spec, e devicelib := device.New(device.WithNvml(nvmllib)) - useNvidiaCTKPath := m.findNvidiaCTK(nvidiaCTKPath) + useNvidiaCTKPath := discover.FindNvidiaCTK(m.logger, nvidiaCTKPath) deviceSpecs, err := m.generateDeviceSpecs(devicelib, root, useNvidiaCTKPath) if err != nil { diff --git a/internal/discover/graphics.go b/internal/discover/graphics.go index 2a9f3d72..c0f058da 100644 --- a/internal/discover/graphics.go +++ b/internal/discover/graphics.go @@ -96,7 +96,6 @@ func NewGraphicsMountsDiscoverer(logger *logrus.Logger, root string) (Discover, type drmDevicesByPath struct { None logger *logrus.Logger - lookup lookup.Locator nvidiaCTKPath string root string devicesFrom Discover @@ -106,8 +105,7 @@ type drmDevicesByPath struct { func newCreateDRMByPathSymlinks(logger *logrus.Logger, devices Discover, cfg *Config) Discover { d := drmDevicesByPath{ logger: logger, - lookup: lookup.NewExecutableLocator(logger, cfg.Root), - nvidiaCTKPath: cfg.NvidiaCTKPath, + nvidiaCTKPath: FindNvidiaCTK(logger, cfg.NvidiaCTKPath), root: cfg.Root, devicesFrom: devices, } @@ -132,26 +130,14 @@ func (d drmDevicesByPath) Hooks() ([]Hook, error) { return nil, nil } - hookPath := nvidiaCTKDefaultFilePath - targets, err := d.lookup.Locate(d.nvidiaCTKPath) - if err != nil { - d.logger.Warnf("Failed to locate %v: %v", d.nvidiaCTKPath, err) - } else if len(targets) == 0 { - d.logger.Warnf("%v not found", d.nvidiaCTKPath) - } else { - d.logger.Debugf("Found %v candidates: %v", d.nvidiaCTKPath, targets) - hookPath = targets[0] - } - d.logger.Debugf("Using NVIDIA Container Toolkit CLI path %v", hookPath) - - args := []string{hookPath, "hook", "create-symlinks"} + args := []string{d.nvidiaCTKPath, "hook", "create-symlinks"} for _, l := range links { args = append(args, "--link", l) } h := Hook{ Lifecycle: cdi.CreateContainerHook, - Path: hookPath, + Path: d.nvidiaCTKPath, Args: args, } diff --git a/internal/discover/hooks.go b/internal/discover/hooks.go new file mode 100644 index 00000000..f8aeea44 --- /dev/null +++ b/internal/discover/hooks.go @@ -0,0 +1,63 @@ +/** +# 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 discover + +import ( + "path/filepath" + + "github.com/NVIDIA/nvidia-container-toolkit/internal/lookup" + "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" + "github.com/sirupsen/logrus" +) + +const ( + nvidiaCTKExecutable = "nvidia-ctk" + nvidiaCTKDefaultFilePath = "/usr/bin/nvidia-ctk" +) + +// CreateNvidiaCTKHook creates a hook which invokes the NVIDIA Container CLI hook subcommand. +func CreateNvidiaCTKHook(executable string, hookName string, additionalArgs ...string) Hook { + return Hook{ + Lifecycle: cdi.CreateContainerHook, + Path: executable, + Args: append([]string{filepath.Base(executable), "hook", hookName}, additionalArgs...), + } +} + +// FindNvidiaCTK locates the nvidia-ctk executable to be used in hooks. +// If an override is specified, this is used instead. +func FindNvidiaCTK(logger *logrus.Logger, override string) string { + if override != "" { + logger.Debugf("Using specified NVIDIA Container Toolkit CLI path %v", override) + return override + } + + lookup := lookup.NewExecutableLocator(logger, "") + hookPath := nvidiaCTKDefaultFilePath + targets, err := lookup.Locate(nvidiaCTKExecutable) + if err != nil { + logger.Warnf("Failed to locate %v: %v", nvidiaCTKExecutable, err) + } else if len(targets) == 0 { + logger.Warnf("%v not found", nvidiaCTKExecutable) + } else { + logger.Debugf("Found %v candidates: %v", nvidiaCTKExecutable, targets) + hookPath = targets[0] + } + logger.Debugf("Using NVIDIA Container Toolkit CLI path %v", hookPath) + + return hookPath +} diff --git a/internal/discover/ldconfig.go b/internal/discover/ldconfig.go index 308f6371..83f26b36 100644 --- a/internal/discover/ldconfig.go +++ b/internal/discover/ldconfig.go @@ -22,7 +22,6 @@ import ( "strings" "github.com/NVIDIA/nvidia-container-toolkit/internal/lookup" - "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" "github.com/sirupsen/logrus" ) @@ -38,10 +37,6 @@ func NewLDCacheUpdateHook(logger *logrus.Logger, mounts Discover, cfg *Config) ( return &d, nil } -const ( - nvidiaCTKDefaultFilePath = "/usr/bin/nvidia-ctk" -) - type ldconfig struct { None logger *logrus.Logger @@ -80,15 +75,6 @@ func CreateLDCacheUpdateHook(executable string, libraries []string) Hook { } -// CreateNvidiaCTKHook creates a hook which invokes the NVIDIA Container CLI hook subcommand. -func CreateNvidiaCTKHook(executable string, hookName string, additionalArgs ...string) Hook { - return Hook{ - Lifecycle: cdi.CreateContainerHook, - Path: executable, - Args: append([]string{filepath.Base(executable), "hook", hookName}, additionalArgs...), - } -} - // getLibraryPaths extracts the library dirs from the specified mounts func getLibraryPaths(mounts []Mount) []string { var paths []string diff --git a/internal/discover/symlinks.go b/internal/discover/symlinks.go index 0507f699..a8f904c1 100644 --- a/internal/discover/symlinks.go +++ b/internal/discover/symlinks.go @@ -21,7 +21,6 @@ import ( "path/filepath" "strings" - "github.com/NVIDIA/nvidia-container-toolkit/internal/lookup" "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" "github.com/sirupsen/logrus" ) @@ -29,7 +28,6 @@ import ( type symlinks struct { None logger *logrus.Logger - lookup lookup.Locator nvidiaCTKPath string csvFiles []string mountsFrom Discover @@ -39,8 +37,7 @@ type symlinks struct { func NewCreateSymlinksHook(logger *logrus.Logger, csvFiles []string, mounts Discover, cfg *Config) (Discover, error) { d := symlinks{ logger: logger, - lookup: lookup.NewExecutableLocator(logger, cfg.Root), - nvidiaCTKPath: cfg.NvidiaCTKPath, + nvidiaCTKPath: FindNvidiaCTK(logger, cfg.NvidiaCTKPath), csvFiles: csvFiles, mountsFrom: mounts, } @@ -50,19 +47,7 @@ func NewCreateSymlinksHook(logger *logrus.Logger, csvFiles []string, mounts Disc // Hooks returns a hook to create the symlinks from the required CSV files func (d symlinks) Hooks() ([]Hook, error) { - hookPath := nvidiaCTKDefaultFilePath - targets, err := d.lookup.Locate(d.nvidiaCTKPath) - if err != nil { - d.logger.Warnf("Failed to locate %v: %v", d.nvidiaCTKPath, err) - } else if len(targets) == 0 { - d.logger.Warnf("%v not found", d.nvidiaCTKPath) - } else { - d.logger.Debugf("Found %v candidates: %v", d.nvidiaCTKPath, targets) - hookPath = targets[0] - } - d.logger.Debugf("Using NVIDIA Container Toolkit CLI path %v", hookPath) - - args := []string{hookPath, "hook", "create-symlinks"} + args := []string{d.nvidiaCTKPath, "hook", "create-symlinks"} for _, f := range d.csvFiles { args = append(args, "--csv-filename", f) } @@ -75,7 +60,7 @@ func (d symlinks) Hooks() ([]Hook, error) { h := Hook{ Lifecycle: cdi.CreateContainerHook, - Path: hookPath, + Path: d.nvidiaCTKPath, Args: args, } From 19cfb2774dbe5127586ecc41355e5ca2e8371ec8 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 19 Jan 2023 10:37:10 +0100 Subject: [PATCH 5/5] Use common code to construct nvidia-ctk hooks Signed-off-by: Evan Lezar --- internal/discover/graphics.go | 15 +++++++-------- internal/discover/symlinks.go | 15 +++++++-------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/internal/discover/graphics.go b/internal/discover/graphics.go index c0f058da..7847314c 100644 --- a/internal/discover/graphics.go +++ b/internal/discover/graphics.go @@ -25,7 +25,6 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/info/drm" "github.com/NVIDIA/nvidia-container-toolkit/internal/info/proc" "github.com/NVIDIA/nvidia-container-toolkit/internal/lookup" - "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" "github.com/sirupsen/logrus" ) @@ -130,18 +129,18 @@ func (d drmDevicesByPath) Hooks() ([]Hook, error) { return nil, nil } - args := []string{d.nvidiaCTKPath, "hook", "create-symlinks"} + var args []string for _, l := range links { args = append(args, "--link", l) } - h := Hook{ - Lifecycle: cdi.CreateContainerHook, - Path: d.nvidiaCTKPath, - Args: args, - } + hook := CreateNvidiaCTKHook( + d.nvidiaCTKPath, + "create-symlinks", + args..., + ) - return []Hook{h}, nil + return []Hook{hook}, nil } // getSpecificLinkArgs returns the required specic links that need to be created diff --git a/internal/discover/symlinks.go b/internal/discover/symlinks.go index a8f904c1..b5d344e3 100644 --- a/internal/discover/symlinks.go +++ b/internal/discover/symlinks.go @@ -21,7 +21,6 @@ import ( "path/filepath" "strings" - "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" "github.com/sirupsen/logrus" ) @@ -47,7 +46,7 @@ func NewCreateSymlinksHook(logger *logrus.Logger, csvFiles []string, mounts Disc // Hooks returns a hook to create the symlinks from the required CSV files func (d symlinks) Hooks() ([]Hook, error) { - args := []string{d.nvidiaCTKPath, "hook", "create-symlinks"} + var args []string for _, f := range d.csvFiles { args = append(args, "--csv-filename", f) } @@ -58,13 +57,13 @@ func (d symlinks) Hooks() ([]Hook, error) { } args = append(args, links...) - h := Hook{ - Lifecycle: cdi.CreateContainerHook, - Path: d.nvidiaCTKPath, - Args: args, - } + hook := CreateNvidiaCTKHook( + d.nvidiaCTKPath, + "create-symlinks", + args..., + ) - return []Hook{h}, nil + return []Hook{hook}, nil } // getSpecificLinkArgs returns the required specic links that need to be created