From f63ad3d9e7a2146391d4b479914d4b7aae60c717 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 22 Sep 2023 16:30:20 +0200 Subject: [PATCH] Refactor symlink filter This change refactors the use of the symlink filter to make it extendible. A blocked filter can be set on the Tegra CSV discoverer to ensure that the correct symlink libraries are filtered out. Here, globs can be used to select mulitple libraries, and a **/ prefix on the globs indicates that the pattern that follows is only applied to the filename of the symlink entry in the CSV file. A --csv.ignore-pattern command line argument is added to the nvidia-ctk cdi generate command that allows this to be set. Signed-off-by: Evan Lezar --- CHANGELOG.md | 1 + cmd/nvidia-ctk/cdi/generate/generate.go | 11 ++- internal/platform-support/tegra/csv.go | 3 +- internal/platform-support/tegra/csv_test.go | 82 +++++++++++++++++++ internal/platform-support/tegra/filter.go | 18 ++-- .../platform-support/tegra/filter_test.go | 32 +++++++- internal/platform-support/tegra/tegra.go | 8 ++ pkg/nvcdi/lib-csv.go | 1 + pkg/nvcdi/lib.go | 3 +- pkg/nvcdi/options.go | 7 ++ 10 files changed, 155 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92d5d455..74460852 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 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. diff --git a/cmd/nvidia-ctk/cdi/generate/generate.go b/cmd/nvidia-ctk/cdi/generate/generate.go index 1afb7550..6adb0936 100644 --- a/cmd/nvidia-ctk/cdi/generate/generate.go +++ b/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) diff --git a/internal/platform-support/tegra/csv.go b/internal/platform-support/tegra/csv.go index f511d327..ff2f4932 100644 --- a/internal/platform-support/tegra/csv.go +++ b/internal/platform-support/tegra/csv.go @@ -58,7 +58,8 @@ func (o tegraOptions) newDiscovererFromCSVFiles() (discover.Discover, error) { targetsByType[csv.MountSpecLib], ) - symlinkTargets := targetsByType[csv.MountSpecSym] + symlinkTargets := o.ignorePatterns.Apply(targetsByType[csv.MountSpecSym]...) + o.logger.Debugf("Filtered symlink targets: %v", symlinkTargets) symlinks := discover.NewMounts( o.logger, o.symlinkLocator, diff --git a/internal/platform-support/tegra/csv_test.go b/internal/platform-support/tegra/csv_test.go index 1f1cbd12..a40acf3c 100644 --- a/internal/platform-support/tegra/csv_test.go +++ b/internal/platform-support/tegra/csv_test.go @@ -34,6 +34,7 @@ func TestDiscovererFromCSVFiles(t *testing.T) { testCases := []struct { description string moutSpecs map[csv.MountSpecType][]string + ignorePatterns []string symlinkLocator lookup.Locator symlinkChainLocator lookup.Locator symlinkResolver func(string) (string, error) @@ -99,6 +100,86 @@ func TestDiscovererFromCSVFiles(t *testing.T) { }, }, }, + { + // 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 { @@ -109,6 +190,7 @@ func TestDiscovererFromCSVFiles(t *testing.T) { logger: logger, nvidiaCTKPath: "/usr/bin/nvidia-ctk", csvFiles: []string{"dummy"}, + ignorePatterns: tc.ignorePatterns, symlinkLocator: tc.symlinkLocator, symlinkChainLocator: tc.symlinkChainLocator, resolveSymlink: tc.symlinkResolver, diff --git a/internal/platform-support/tegra/filter.go b/internal/platform-support/tegra/filter.go index 7d6e8e15..03b18bf7 100644 --- a/internal/platform-support/tegra/filter.go +++ b/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) { diff --git a/internal/platform-support/tegra/filter_test.go b/internal/platform-support/tegra/filter_test.go index 5cca505d..a3b1a8f7 100644 --- a/internal/platform-support/tegra/filter_test.go +++ b/internal/platform-support/tegra/filter_test.go @@ -23,7 +23,35 @@ import ( ) func TestIgnorePatterns(t *testing.T) { - filtered := ignoreFilenamePatterns{"*.so", "*.so.[0-9]"}.Apply("/foo/bar/libsomething.so", "libsometing.so", "libsometing.so.1", "libsometing.so.1.2.3") + testCases := []struct { + description string + blockedFilter []string + input []string + expected []string + }{ + { + description: "nil slice", + input: []string{"something", "somethingelse"}, + expected: []string{"something", "somethingelse"}, + }, + { + description: "match libraries full path and so symlinks using globs", + blockedFilter: []string{"*.so", "*.so.[0-9]"}, + input: []string{"/foo/bar/libsomething.so", "libsometing.so", "libsometing.so.1", "libsometing.so.1.2.3"}, + expected: []string{"/foo/bar/libsomething.so", "libsometing.so.1.2.3"}, + }, + { + description: "match libraries full path and so symlinks using globs with any path prefix", + blockedFilter: []string{"**/*.so", "**/*.so.[0-9]"}, + input: []string{"/foo/bar/libsomething.so", "libsometing.so", "libsometing.so.1", "libsometing.so.1.2.3"}, + expected: []string{"libsometing.so.1.2.3"}, + }, + } - require.ElementsMatch(t, []string{"libsometing.so.1.2.3"}, filtered) + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + filtered := ignoreMountSpecPatterns(tc.blockedFilter).Apply(tc.input...) + require.ElementsMatch(t, tc.expected, filtered) + }) + } } diff --git a/internal/platform-support/tegra/tegra.go b/internal/platform-support/tegra/tegra.go index 695f8414..ad5c5b6a 100644 --- a/internal/platform-support/tegra/tegra.go +++ b/internal/platform-support/tegra/tegra.go @@ -31,6 +31,7 @@ type tegraOptions struct { driverRoot string nvidiaCTKPath string librarySearchPaths []string + ignorePatterns ignoreMountSpecPatterns // The following can be overridden for testing symlinkLocator lookup.Locator @@ -132,3 +133,10 @@ func WithLibrarySearchPaths(librarySearchPaths ...string) Option { o.librarySearchPaths = librarySearchPaths } } + +// WithIngorePatterns sets patterns to ignore in the CSV files +func WithIngorePatterns(ignorePatterns ...string) Option { + return func(o *tegraOptions) { + o.ignorePatterns = ignoreMountSpecPatterns(ignorePatterns) + } +} diff --git a/pkg/nvcdi/lib-csv.go b/pkg/nvcdi/lib-csv.go index 5ae17964..22b54b64 100644 --- a/pkg/nvcdi/lib-csv.go +++ b/pkg/nvcdi/lib-csv.go @@ -45,6 +45,7 @@ func (l *csvlib) GetAllDeviceSpecs() ([]specs.Device, error) { tegra.WithNVIDIACTKPath(l.nvidiaCTKPath), tegra.WithCSVFiles(l.csvFiles), tegra.WithLibrarySearchPaths(l.librarySearchPaths...), + tegra.WithIngorePatterns(l.csvIgnorePatterns...), ) if err != nil { return nil, fmt.Errorf("failed to create discoverer for CSV files: %v", err) diff --git a/pkg/nvcdi/lib.go b/pkg/nvcdi/lib.go index 30d12aae..f8a52c1b 100644 --- a/pkg/nvcdi/lib.go +++ b/pkg/nvcdi/lib.go @@ -47,7 +47,8 @@ type nvcdilib struct { nvidiaCTKPath string librarySearchPaths []string - csvFiles []string + csvFiles []string + csvIgnorePatterns []string vendor string class string diff --git a/pkg/nvcdi/options.go b/pkg/nvcdi/options.go index f354e6df..61c4c956 100644 --- a/pkg/nvcdi/options.go +++ b/pkg/nvcdi/options.go @@ -104,6 +104,13 @@ func WithCSVFiles(csvFiles []string) Option { } } +// WithCSVIgnorePatterns sets the ignore patterns for entries in the CSV files. +func WithCSVIgnorePatterns(csvIgnorePatterns []string) Option { + return func(o *nvcdilib) { + o.csvIgnorePatterns = csvIgnorePatterns + } +} + // WithLibrarySearchPaths sets the library search paths. // This is currently only used for CSV-mode. func WithLibrarySearchPaths(paths []string) Option {