From 8bb0235c923673fdaf48526224bc12c4cc970bd3 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 10 May 2023 14:49:59 +0200 Subject: [PATCH] Remove discover.Config These changes remove the use of discover.Config which was used to pass the driver root and the nvidiaCTK path in some cases. Instead, the nvidiaCTKPath is resolved at the begining of runtime invocation to ensure that this is valid at all points where it is used. Signed-off-by: Evan Lezar --- internal/discover/discover.go | 6 ------ internal/discover/graphics.go | 16 +++++++--------- internal/discover/ldconfig.go | 4 ++-- internal/discover/ldconfig_test.go | 7 +------ internal/discover/symlinks.go | 4 ++-- internal/modifier/csv.go | 11 +++-------- internal/modifier/graphics.go | 7 ++----- internal/runtime/runtime.go | 12 ++++++++---- pkg/nvcdi/driver-nvml.go | 6 +----- pkg/nvcdi/driver-wsl.go | 6 +----- 10 files changed, 27 insertions(+), 52 deletions(-) diff --git a/internal/discover/discover.go b/internal/discover/discover.go index 0687055f..7ff9f042 100644 --- a/internal/discover/discover.go +++ b/internal/discover/discover.go @@ -16,12 +16,6 @@ package discover -// Config represents the configuration options for discovery -type Config struct { - DriverRoot string - NvidiaCTKPath string -} - // Device represents a discovered character device. type Device struct { HostPath string diff --git a/internal/discover/graphics.go b/internal/discover/graphics.go index dedbfbe7..527f7c1d 100644 --- a/internal/discover/graphics.go +++ b/internal/discover/graphics.go @@ -31,9 +31,7 @@ import ( ) // NewGraphicsDiscoverer returns the discoverer for graphics tools such as Vulkan. -func NewGraphicsDiscoverer(logger *logrus.Logger, devices image.VisibleDevices, cfg *Config) (Discover, error) { - driverRoot := cfg.DriverRoot - +func NewGraphicsDiscoverer(logger *logrus.Logger, devices image.VisibleDevices, driverRoot string, nvidiaCTKPath string) (Discover, error) { mounts, err := NewGraphicsMountsDiscoverer(logger, driverRoot) if err != nil { return nil, fmt.Errorf("failed to create mounts discoverer: %v", err) @@ -44,9 +42,9 @@ func NewGraphicsDiscoverer(logger *logrus.Logger, devices image.VisibleDevices, return nil, fmt.Errorf("failed to create DRM device discoverer: %v", err) } - drmByPathSymlinks := newCreateDRMByPathSymlinks(logger, drmDeviceNodes, cfg) + drmByPathSymlinks := newCreateDRMByPathSymlinks(logger, drmDeviceNodes, driverRoot, nvidiaCTKPath) - xorg := optionalXorgDiscoverer(logger, driverRoot, cfg.NvidiaCTKPath) + xorg := optionalXorgDiscoverer(logger, driverRoot, nvidiaCTKPath) discover := Merge( Merge(drmDeviceNodes, drmByPathSymlinks), @@ -106,11 +104,11 @@ type drmDevicesByPath struct { } // 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 { +func newCreateDRMByPathSymlinks(logger *logrus.Logger, devices Discover, driverRoot string, nvidiaCTKPath string) Discover { d := drmDevicesByPath{ logger: logger, - nvidiaCTKPath: FindNvidiaCTK(logger, cfg.NvidiaCTKPath), - driverRoot: cfg.DriverRoot, + nvidiaCTKPath: nvidiaCTKPath, + driverRoot: driverRoot, devicesFrom: devices, } @@ -300,7 +298,7 @@ func newXorgDiscoverer(logger *logrus.Logger, driverRoot string, nvidiaCTKPath s xorgHooks := xorgHooks{ libraries: xorgLibs, driverVersion: version, - nvidiaCTKPath: FindNvidiaCTK(logger, nvidiaCTKPath), + nvidiaCTKPath: nvidiaCTKPath, } xorgConfg := NewMounts( diff --git a/internal/discover/ldconfig.go b/internal/discover/ldconfig.go index e56f605a..a1c237e0 100644 --- a/internal/discover/ldconfig.go +++ b/internal/discover/ldconfig.go @@ -25,10 +25,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) { +func NewLDCacheUpdateHook(logger *logrus.Logger, mounts Discover, nvidiaCTKPath string) (Discover, error) { d := ldconfig{ logger: logger, - nvidiaCTKPath: FindNvidiaCTK(logger, cfg.NvidiaCTKPath), + nvidiaCTKPath: nvidiaCTKPath, mountsFrom: mounts, } diff --git a/internal/discover/ldconfig_test.go b/internal/discover/ldconfig_test.go index 4b3c11a1..8d72dde6 100644 --- a/internal/discover/ldconfig_test.go +++ b/internal/discover/ldconfig_test.go @@ -31,11 +31,6 @@ const ( func TestLDCacheUpdateHook(t *testing.T) { logger, _ := testlog.NewNullLogger() - cfg := Config{ - DriverRoot: "/", - NvidiaCTKPath: testNvidiaCTKPath, - } - testCases := []struct { description string mounts []Mount @@ -95,7 +90,7 @@ func TestLDCacheUpdateHook(t *testing.T) { Lifecycle: "createContainer", } - d, err := NewLDCacheUpdateHook(logger, mountMock, &cfg) + d, err := NewLDCacheUpdateHook(logger, mountMock, testNvidiaCTKPath) require.NoError(t, err) hooks, err := d.Hooks() diff --git a/internal/discover/symlinks.go b/internal/discover/symlinks.go index b5d344e3..31e8e64e 100644 --- a/internal/discover/symlinks.go +++ b/internal/discover/symlinks.go @@ -33,10 +33,10 @@ type symlinks struct { } // 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) { +func NewCreateSymlinksHook(logger *logrus.Logger, csvFiles []string, mounts Discover, nvidiaCTKPath string) (Discover, error) { d := symlinks{ logger: logger, - nvidiaCTKPath: FindNvidiaCTK(logger, cfg.NvidiaCTKPath), + nvidiaCTKPath: nvidiaCTKPath, csvFiles: csvFiles, mountsFrom: mounts, } diff --git a/internal/modifier/csv.go b/internal/modifier/csv.go index e0f8582e..0f59b2f4 100644 --- a/internal/modifier/csv.go +++ b/internal/modifier/csv.go @@ -61,11 +61,6 @@ func NewCSVModifier(logger *logrus.Logger, cfg *config.Config, ociSpec oci.Spec) } logger.Infof("Constructing modifier from config: %+v", *cfg) - config := &discover.Config{ - DriverRoot: cfg.NVIDIAContainerCLIConfig.Root, - NvidiaCTKPath: cfg.NVIDIACTKConfig.Path, - } - if err := checkRequirements(logger, image); err != nil { return nil, fmt.Errorf("requirements not met: %v", err) } @@ -79,17 +74,17 @@ func NewCSVModifier(logger *logrus.Logger, cfg *config.Config, ociSpec oci.Spec) csvFiles = csv.BaseFilesOnly(csvFiles) } - csvDiscoverer, err := discover.NewFromCSVFiles(logger, csvFiles, config.DriverRoot) + csvDiscoverer, err := discover.NewFromCSVFiles(logger, csvFiles, cfg.NVIDIAContainerCLIConfig.Root) if err != nil { return nil, fmt.Errorf("failed to create CSV discoverer: %v", err) } - createSymlinksHook, err := discover.NewCreateSymlinksHook(logger, csvFiles, csvDiscoverer, config) + createSymlinksHook, err := discover.NewCreateSymlinksHook(logger, csvFiles, csvDiscoverer, cfg.NVIDIACTKConfig.Path) if err != nil { return nil, fmt.Errorf("failed to create symlink hook discoverer: %v", err) } - ldcacheUpdateHook, err := discover.NewLDCacheUpdateHook(logger, csvDiscoverer, config) + ldcacheUpdateHook, err := discover.NewLDCacheUpdateHook(logger, csvDiscoverer, cfg.NVIDIACTKConfig.Path) if err != nil { return nil, fmt.Errorf("failed to create ldcach update hook discoverer: %v", err) } diff --git a/internal/modifier/graphics.go b/internal/modifier/graphics.go index 7e4fa8d1..ec4750d0 100644 --- a/internal/modifier/graphics.go +++ b/internal/modifier/graphics.go @@ -44,14 +44,11 @@ func NewGraphicsModifier(logger *logrus.Logger, cfg *config.Config, ociSpec oci. return nil, nil } - config := &discover.Config{ - DriverRoot: cfg.NVIDIAContainerCLIConfig.Root, - NvidiaCTKPath: cfg.NVIDIACTKConfig.Path, - } d, err := discover.NewGraphicsDiscoverer( logger, image.DevicesFromEnvvars(visibleDevicesEnvvar), - config, + cfg.NVIDIAContainerCLIConfig.Root, + cfg.NVIDIACTKConfig.Path, ) if err != nil { return nil, fmt.Errorf("failed to construct discoverer: %v", err) diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index 5634e75a..4e30f8b9 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/NVIDIA/nvidia-container-toolkit/internal/config" + "github.com/NVIDIA/nvidia-container-toolkit/internal/discover" "github.com/NVIDIA/nvidia-container-toolkit/internal/info" "github.com/opencontainers/runtime-spec/specs-go" ) @@ -44,10 +45,6 @@ func (r rt) Run(argv []string) (rerr error) { if err != nil { return fmt.Errorf("error loading config: %v", err) } - if r.modeOverride != "" { - cfg.NVIDIAContainerRuntimeConfig.Mode = r.modeOverride - } - err = r.logger.Update( cfg.NVIDIAContainerRuntimeConfig.DebugFilePath, cfg.NVIDIAContainerRuntimeConfig.LogLevel, @@ -63,6 +60,13 @@ func (r rt) Run(argv []string) (rerr error) { r.logger.Reset() }() + // We apply some config updates here to ensure that the config is valid in + // all cases. + if r.modeOverride != "" { + cfg.NVIDIAContainerRuntimeConfig.Mode = r.modeOverride + } + cfg.NVIDIACTKConfig.Path = discover.FindNvidiaCTK(r.logger.Logger, cfg.NVIDIACTKConfig.Path) + // Print the config to the output. configJSON, err := json.MarshalIndent(cfg, "", " ") if err == nil { diff --git a/pkg/nvcdi/driver-nvml.go b/pkg/nvcdi/driver-nvml.go index 408da55a..c0dc1445 100644 --- a/pkg/nvcdi/driver-nvml.go +++ b/pkg/nvcdi/driver-nvml.go @@ -86,11 +86,7 @@ func NewDriverLibraryDiscoverer(logger *logrus.Logger, driverRoot string, nvidia libraryPaths, ) - cfg := &discover.Config{ - DriverRoot: driverRoot, - NvidiaCTKPath: nvidiaCTKPath, - } - hooks, _ := discover.NewLDCacheUpdateHook(logger, libraries, cfg) + hooks, _ := discover.NewLDCacheUpdateHook(logger, libraries, nvidiaCTKPath) d := discover.Merge( libraries, diff --git a/pkg/nvcdi/driver-wsl.go b/pkg/nvcdi/driver-wsl.go index cca4d52c..fbe0572f 100644 --- a/pkg/nvcdi/driver-wsl.go +++ b/pkg/nvcdi/driver-wsl.go @@ -90,11 +90,7 @@ func newWSLDriverStoreDiscoverer(logger *logrus.Logger, driverRoot string, nvidi links := []string{fmt.Sprintf("%s::%s", target, link)} symlinkHook := discover.CreateCreateSymlinkHook(nvidiaCTKPath, links) - cfg := &discover.Config{ - DriverRoot: driverRoot, - NvidiaCTKPath: nvidiaCTKPath, - } - ldcacheHook, _ := discover.NewLDCacheUpdateHook(logger, libraries, cfg) + ldcacheHook, _ := discover.NewLDCacheUpdateHook(logger, libraries, nvidiaCTKPath) d := discover.Merge( libraries,