Merge branch 'fix-symlink-discovery' into 'main'

Fix bug in creating symlinks in containers on Tegra-based systems

See merge request nvidia/container-toolkit/container-toolkit!479

CHANGELOG.md

@@ -1,5 +1,9 @@
 # NVIDIA Container Toolkit Changelog
 
+## v1.14.2
+* Fix bug on Tegra-based systems where symlinks were not created in containers.
+* Add --csv.ignore-pattern command line option to nvidia-ctk cdi generate command.
+
 ## v1.14.1
 * Fixed bug where contents of `/etc/nvidia-container-runtime/config.toml` is ignored by the NVIDIA Container Runtime Hook.
 

cmd/nvidia-ctk/cdi/generate/generate.go

@@ -53,7 +53,8 @@ type options struct {
 	librarySearchPaths cli.StringSlice
 
 	csv struct {
-		files cli.StringSlice
+		files          cli.StringSlice
+		ignorePatterns cli.StringSlice
 	}
 }
 
@@ -141,6 +142,11 @@ func (m command) build() *cli.Command {
 			Value:       cli.NewStringSlice(csv.DefaultFileList()...),
 			Destination: &opts.csv.files,
 		},
+		&cli.StringSliceFlag{
+			Name:        "csv.ignore-pattern",
+			Usage:       "Specify a pattern the CSV mount specifications.",
+			Destination: &opts.csv.ignorePatterns,
+		},
 	}
 
 	return &c
@@ -233,8 +239,9 @@ func (m command) generateSpec(opts *options) (spec.Interface, error) {
 		nvcdi.WithNVIDIACTKPath(opts.nvidiaCTKPath),
 		nvcdi.WithDeviceNamer(deviceNamer),
 		nvcdi.WithMode(string(opts.mode)),
-		nvcdi.WithCSVFiles(opts.csv.files.Value()),
 		nvcdi.WithLibrarySearchPaths(opts.librarySearchPaths.Value()),
+		nvcdi.WithCSVFiles(opts.csv.files.Value()),
+		nvcdi.WithCSVIgnorePatterns(opts.csv.ignorePatterns.Value()),
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create CDI library: %v", err)

internal/platform-support/tegra/csv.go

@@ -28,51 +28,45 @@ import (
 // newDiscovererFromCSVFiles creates a discoverer for the specified CSV files. A logger is also supplied.
 // The constructed discoverer is comprised of a list, with each element in the list being associated with a
 // single CSV files.
-func newDiscovererFromCSVFiles(logger logger.Interface, files []string, driverRoot string, nvidiaCTKPath string, librarySearchPaths []string) (discover.Discover, error) {
-	if len(files) == 0 {
-		logger.Warningf("No CSV files specified")
+func (o tegraOptions) newDiscovererFromCSVFiles() (discover.Discover, error) {
+	if len(o.csvFiles) == 0 {
+		o.logger.Warningf("No CSV files specified")
 		return discover.None{}, nil
 	}
 
-	targetsByType := getTargetsFromCSVFiles(logger, files)
+	targetsByType := getTargetsFromCSVFiles(o.logger, o.csvFiles)
 
 	devices := discover.NewDeviceDiscoverer(
-		logger,
-		lookup.NewCharDeviceLocator(lookup.WithLogger(logger), lookup.WithRoot(driverRoot)),
-		driverRoot,
+		o.logger,
+		lookup.NewCharDeviceLocator(lookup.WithLogger(o.logger), lookup.WithRoot(o.driverRoot)),
+		o.driverRoot,
 		targetsByType[csv.MountSpecDev],
 	)
 
 	directories := discover.NewMounts(
-		logger,
-		lookup.NewDirectoryLocator(lookup.WithLogger(logger), lookup.WithRoot(driverRoot)),
-		driverRoot,
+		o.logger,
+		lookup.NewDirectoryLocator(lookup.WithLogger(o.logger), lookup.WithRoot(o.driverRoot)),
+		o.driverRoot,
 		targetsByType[csv.MountSpecDir],
 	)
 
 	// Libraries and symlinks use the same locator.
-	searchPaths := append(librarySearchPaths, "/")
-	symlinkLocator := lookup.NewSymlinkLocator(
-		lookup.WithLogger(logger),
-		lookup.WithRoot(driverRoot),
-		lookup.WithSearchPaths(searchPaths...),
-	)
 	libraries := discover.NewMounts(
-		logger,
-		symlinkLocator,
-		driverRoot,
+		o.logger,
+		o.symlinkLocator,
+		o.driverRoot,
 		targetsByType[csv.MountSpecLib],
 	)
 
-	nonLibSymlinks := ignoreFilenamePatterns{"*.so", "*.so.[0-9]"}.Apply(targetsByType[csv.MountSpecSym]...)
-	logger.Debugf("Non-lib symlinks: %v", nonLibSymlinks)
+	symlinkTargets := o.ignorePatterns.Apply(targetsByType[csv.MountSpecSym]...)
+	o.logger.Debugf("Filtered symlink targets: %v", symlinkTargets)
 	symlinks := discover.NewMounts(
-		logger,
-		symlinkLocator,
-		driverRoot,
-		nonLibSymlinks,
+		o.logger,
+		o.symlinkLocator,
+		o.driverRoot,
+		symlinkTargets,
 	)
-	createSymlinks := createCSVSymlinkHooks(logger, nonLibSymlinks, libraries, nvidiaCTKPath)
+	createSymlinks := o.createCSVSymlinkHooks(symlinkTargets, libraries)
 
 	d := discover.Merge(
 		devices,
@@ -87,7 +81,9 @@ func newDiscovererFromCSVFiles(logger logger.Interface, files []string, driverRo
 
 // getTargetsFromCSVFiles returns the list of mount specs from the specified CSV files.
 // These are aggregated by mount spec type.
-func getTargetsFromCSVFiles(logger logger.Interface, files []string) map[csv.MountSpecType][]string {
+// TODO: We use a function variable here to allow this to be overridden for testing.
+// This should be properly mocked.
+var getTargetsFromCSVFiles = func(logger logger.Interface, files []string) map[csv.MountSpecType][]string {
 	targetsByType := make(map[csv.MountSpecType][]string)
 	for _, filename := range files {
 		targets, err := loadCSVFile(logger, filename)

internal/platform-support/tegra/csv_test.go

@@ -15,3 +15,209 @@
 **/
 
 package tegra
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup"
+	testlog "github.com/sirupsen/logrus/hooks/test"
+	"github.com/stretchr/testify/require"
+
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/platform-support/tegra/csv"
+)
+
+func TestDiscovererFromCSVFiles(t *testing.T) {
+	logger, _ := testlog.NewNullLogger()
+	testCases := []struct {
+		description         string
+		moutSpecs           map[csv.MountSpecType][]string
+		ignorePatterns      []string
+		symlinkLocator      lookup.Locator
+		symlinkChainLocator lookup.Locator
+		symlinkResolver     func(string) (string, error)
+		expectedError       error
+		expectedMounts      []discover.Mount
+		expectedMountsError error
+		expectedHooks       []discover.Hook
+		expectedHooksError  error
+	}{
+		{
+			// TODO: This current resolves to two mounts that are the same.
+			// These are deduplicated at a later stage. We could consider deduplicating earlier in the pipeline.
+			description: "symlink is resolved to target; mounts and symlink are created",
+			moutSpecs: map[csv.MountSpecType][]string{
+				"lib": {"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"},
+				"sym": {"/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so"},
+			},
+			symlinkLocator: &lookup.LocatorMock{
+				LocateFunc: func(path string) ([]string, error) {
+					if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
+						return []string{"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"}, nil
+					}
+					return []string{path}, nil
+				},
+			},
+			symlinkChainLocator: &lookup.LocatorMock{
+				LocateFunc: func(path string) ([]string, error) {
+					if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
+						return []string{"/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so", "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"}, nil
+					}
+					return nil, fmt.Errorf("Unexpected path: %v", path)
+				},
+			},
+			symlinkResolver: func(path string) (string, error) {
+				if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
+					return "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so", nil
+				}
+				return path, nil
+			},
+			expectedMounts: []discover.Mount{
+				{
+					Path:     "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
+					HostPath: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
+					Options:  []string{"ro", "nosuid", "nodev", "bind"},
+				},
+				{
+					Path:     "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
+					HostPath: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
+					Options:  []string{"ro", "nosuid", "nodev", "bind"},
+				},
+			},
+			expectedHooks: []discover.Hook{
+				{
+					Lifecycle: "createContainer",
+					Path:      "/usr/bin/nvidia-ctk",
+					Args: []string{
+						"nvidia-ctk",
+						"hook",
+						"create-symlinks",
+						"--link",
+						"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so::/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so",
+					},
+				},
+			},
+		},
+		{
+			// TODO: This current resolves to two mounts that are the same.
+			// These are deduplicated at a later stage. We could consider deduplicating earlier in the pipeline.
+			description: "single glob filter does not remove symlink mounts",
+			moutSpecs: map[csv.MountSpecType][]string{
+				"lib": {"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"},
+				"sym": {"/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so"},
+			},
+			ignorePatterns: []string{"*.so"},
+			symlinkLocator: &lookup.LocatorMock{
+				LocateFunc: func(path string) ([]string, error) {
+					if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
+						return []string{"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"}, nil
+					}
+					return []string{path}, nil
+				},
+			},
+			symlinkChainLocator: &lookup.LocatorMock{
+				LocateFunc: func(path string) ([]string, error) {
+					if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
+						return []string{"/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so", "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"}, nil
+					}
+					return nil, fmt.Errorf("Unexpected path: %v", path)
+				},
+			},
+			symlinkResolver: func(path string) (string, error) {
+				if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
+					return "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so", nil
+				}
+				return path, nil
+			},
+			expectedMounts: []discover.Mount{
+				{
+					Path:     "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
+					HostPath: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
+					Options:  []string{"ro", "nosuid", "nodev", "bind"},
+				},
+				{
+					Path:     "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
+					HostPath: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
+					Options:  []string{"ro", "nosuid", "nodev", "bind"},
+				},
+			},
+			expectedHooks: []discover.Hook{
+				{
+					Lifecycle: "createContainer",
+					Path:      "/usr/bin/nvidia-ctk",
+					Args: []string{
+						"nvidia-ctk",
+						"hook",
+						"create-symlinks",
+						"--link",
+						"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so::/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so",
+					},
+				},
+			},
+		},
+		{
+			description: "** filter removes symlink mounts",
+			moutSpecs: map[csv.MountSpecType][]string{
+				"lib": {"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"},
+				"sym": {"/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so"},
+			},
+			symlinkLocator: &lookup.LocatorMock{
+				LocateFunc: func(path string) ([]string, error) {
+					if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
+						return []string{"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"}, nil
+					}
+					return []string{path}, nil
+				},
+			},
+			ignorePatterns: []string{"**/*.so"},
+			expectedMounts: []discover.Mount{
+				{
+					Path:     "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
+					HostPath: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
+					Options:  []string{"ro", "nosuid", "nodev", "bind"},
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			defer setGetTargetsFromCSVFiles(tc.moutSpecs)()
+
+			o := tegraOptions{
+				logger:              logger,
+				nvidiaCTKPath:       "/usr/bin/nvidia-ctk",
+				csvFiles:            []string{"dummy"},
+				ignorePatterns:      tc.ignorePatterns,
+				symlinkLocator:      tc.symlinkLocator,
+				symlinkChainLocator: tc.symlinkChainLocator,
+				resolveSymlink:      tc.symlinkResolver,
+			}
+
+			d, err := o.newDiscovererFromCSVFiles()
+			require.ErrorIs(t, err, tc.expectedError)
+
+			hooks, err := d.Hooks()
+			require.ErrorIs(t, err, tc.expectedHooksError)
+			require.EqualValues(t, tc.expectedHooks, hooks)
+
+			mounts, err := d.Mounts()
+			require.ErrorIs(t, err, tc.expectedMountsError)
+			require.EqualValues(t, tc.expectedMounts, mounts)
+
+		})
+	}
+}
+
+func setGetTargetsFromCSVFiles(ovverride map[csv.MountSpecType][]string) func() {
+	original := getTargetsFromCSVFiles
+	getTargetsFromCSVFiles = func(logger logger.Interface, files []string) map[csv.MountSpecType][]string {
+		return ovverride
+	}
+
+	return func() {
+		getTargetsFromCSVFiles = original
+	}
+}

internal/platform-support/tegra/filter.go

@@ -16,20 +16,28 @@
 
 package tegra
 
-import "path/filepath"
+import (
+	"path/filepath"
+	"strings"
+)
 
-type ignoreFilenamePatterns []string
+type ignoreMountSpecPatterns []string
 
-func (d ignoreFilenamePatterns) Match(name string) bool {
+func (d ignoreMountSpecPatterns) Match(name string) bool {
 	for _, pattern := range d {
-		if match, _ := filepath.Match(pattern, filepath.Base(name)); match {
+		target := name
+		if strings.HasPrefix(pattern, "**/") {
+			target = filepath.Base(name)
+			pattern = strings.TrimPrefix(pattern, "**/")
+		}
+		if match, _ := filepath.Match(pattern, target); match {
 			return true
 		}
 	}
 	return false
 }
 
-func (d ignoreFilenamePatterns) Apply(input ...string) []string {
+func (d ignoreMountSpecPatterns) Apply(input ...string) []string {
 	var filtered []string
 	for _, name := range input {
 		if d.Match(name) {