Skip to content

Commit f63ad3d

Browse files
elezarelezar
committed
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 <[email protected]>
1 parent c4b4478 commit f63ad3d

File tree

10 files changed

+155
-11
lines changed

10 files changed

+155
-11
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## v1.14.2
44
* Fix bug on Tegra-based systems where symlinks were not created in containers.
5+
* Add --csv.ignore-pattern command line option to nvidia-ctk cdi generate command.
56

67
## v1.14.1
78
* 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‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ type options struct {
5353
librarySearchPaths cli.StringSlice
5454

5555
csv struct {
56-
files cli.StringSlice
56+
files cli.StringSlice
57+
ignorePatterns cli.StringSlice
5758
}
5859
}
5960

@@ -141,6 +142,11 @@ func (m command) build() *cli.Command {
141142
Value: cli.NewStringSlice(csv.DefaultFileList()...),
142143
Destination: &opts.csv.files,
143144
},
145+
&cli.StringSliceFlag{
146+
Name: "csv.ignore-pattern",
147+
Usage: "Specify a pattern the CSV mount specifications.",
148+
Destination: &opts.csv.ignorePatterns,
149+
},
144150
}
145151

146152
return &c
@@ -233,8 +239,9 @@ func (m command) generateSpec(opts *options) (spec.Interface, error) {
233239
nvcdi.WithNVIDIACTKPath(opts.nvidiaCTKPath),
234240
nvcdi.WithDeviceNamer(deviceNamer),
235241
nvcdi.WithMode(string(opts.mode)),
236-
nvcdi.WithCSVFiles(opts.csv.files.Value()),
237242
nvcdi.WithLibrarySearchPaths(opts.librarySearchPaths.Value()),
243+
nvcdi.WithCSVFiles(opts.csv.files.Value()),
244+
nvcdi.WithCSVIgnorePatterns(opts.csv.ignorePatterns.Value()),
238245
)
239246
if err != nil {
240247
return nil, fmt.Errorf("failed to create CDI library: %v", err)

‎internal/platform-support/tegra/csv.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ func (o tegraOptions) newDiscovererFromCSVFiles() (discover.Discover, error) {
5858
targetsByType[csv.MountSpecLib],
5959
)
6060

61-
symlinkTargets := targetsByType[csv.MountSpecSym]
61+
symlinkTargets := o.ignorePatterns.Apply(targetsByType[csv.MountSpecSym]...)
62+
o.logger.Debugf("Filtered symlink targets: %v", symlinkTargets)
6263
symlinks := discover.NewMounts(
6364
o.logger,
6465
o.symlinkLocator,

‎internal/platform-support/tegra/csv_test.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ func TestDiscovererFromCSVFiles(t *testing.T) {
3434
testCases := []struct {
3535
description string
3636
moutSpecs map[csv.MountSpecType][]string
37+
ignorePatterns []string
3738
symlinkLocator lookup.Locator
3839
symlinkChainLocator lookup.Locator
3940
symlinkResolver func(string) (string, error)
@@ -99,6 +100,86 @@ func TestDiscovererFromCSVFiles(t *testing.T) {
99100
},
100101
},
101102
},
103+
{
104+
// TODO: This current resolves to two mounts that are the same.
105+
// These are deduplicated at a later stage. We could consider deduplicating earlier in the pipeline.
106+
description: "single glob filter does not remove symlink mounts",
107+
moutSpecs: map[csv.MountSpecType][]string{
108+
"lib": {"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"},
109+
"sym": {"/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so"},
110+
},
111+
ignorePatterns: []string{"*.so"},
112+
symlinkLocator: &lookup.LocatorMock{
113+
LocateFunc: func(path string) ([]string, error) {
114+
if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
115+
return []string{"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"}, nil
116+
}
117+
return []string{path}, nil
118+
},
119+
},
120+
symlinkChainLocator: &lookup.LocatorMock{
121+
LocateFunc: func(path string) ([]string, error) {
122+
if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
123+
return []string{"/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so", "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"}, nil
124+
}
125+
return nil, fmt.Errorf("Unexpected path: %v", path)
126+
},
127+
},
128+
symlinkResolver: func(path string) (string, error) {
129+
if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
130+
return "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so", nil
131+
}
132+
return path, nil
133+
},
134+
expectedMounts: []discover.Mount{
135+
{
136+
Path: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
137+
HostPath: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
138+
Options: []string{"ro", "nosuid", "nodev", "bind"},
139+
},
140+
{
141+
Path: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
142+
HostPath: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
143+
Options: []string{"ro", "nosuid", "nodev", "bind"},
144+
},
145+
},
146+
expectedHooks: []discover.Hook{
147+
{
148+
Lifecycle: "createContainer",
149+
Path: "/usr/bin/nvidia-ctk",
150+
Args: []string{
151+
"nvidia-ctk",
152+
"hook",
153+
"create-symlinks",
154+
"--link",
155+
"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so::/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so",
156+
},
157+
},
158+
},
159+
},
160+
{
161+
description: "** filter removes symlink mounts",
162+
moutSpecs: map[csv.MountSpecType][]string{
163+
"lib": {"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"},
164+
"sym": {"/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so"},
165+
},
166+
symlinkLocator: &lookup.LocatorMock{
167+
LocateFunc: func(path string) ([]string, error) {
168+
if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
169+
return []string{"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"}, nil
170+
}
171+
return []string{path}, nil
172+
},
173+
},
174+
ignorePatterns: []string{"**/*.so"},
175+
expectedMounts: []discover.Mount{
176+
{
177+
Path: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
178+
HostPath: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
179+
Options: []string{"ro", "nosuid", "nodev", "bind"},
180+
},
181+
},
182+
},
102183
}
103184

104185
for _, tc := range testCases {
@@ -109,6 +190,7 @@ func TestDiscovererFromCSVFiles(t *testing.T) {
109190
logger: logger,
110191
nvidiaCTKPath: "/usr/bin/nvidia-ctk",
111192
csvFiles: []string{"dummy"},
193+
ignorePatterns: tc.ignorePatterns,
112194
symlinkLocator: tc.symlinkLocator,
113195
symlinkChainLocator: tc.symlinkChainLocator,
114196
resolveSymlink: tc.symlinkResolver,

‎internal/platform-support/tegra/filter.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,28 @@
1616

1717
package tegra
1818

19-
import "path/filepath"
19+
import (
20+
"path/filepath"
21+
"strings"
22+
)
2023

21-
type ignoreFilenamePatterns []string
24+
type ignoreMountSpecPatterns []string
2225

23-
func (d ignoreFilenamePatterns) Match(name string) bool {
26+
func (d ignoreMountSpecPatterns) Match(name string) bool {
2427
for _, pattern := range d {
25-
if match, _ := filepath.Match(pattern, filepath.Base(name)); match {
28+
target := name
29+
if strings.HasPrefix(pattern, "**/") {
30+
target = filepath.Base(name)
31+
pattern = strings.TrimPrefix(pattern, "**/")
32+
}
33+
if match, _ := filepath.Match(pattern, target); match {
2634
return true
2735
}
2836
}
2937
return false
3038
}
3139

32-
func (d ignoreFilenamePatterns) Apply(input ...string) []string {
40+
func (d ignoreMountSpecPatterns) Apply(input ...string) []string {
3341
var filtered []string
3442
for _, name := range input {
3543
if d.Match(name) {

‎internal/platform-support/tegra/filter_test.go‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,35 @@ import (
2323
)
2424

2525
func TestIgnorePatterns(t *testing.T) {
26-
filtered := ignoreFilenamePatterns{"*.so", "*.so.[0-9]"}.Apply("/foo/bar/libsomething.so", "libsometing.so", "libsometing.so.1", "libsometing.so.1.2.3")
26+
testCases := []struct {
27+
description string
28+
blockedFilter []string
29+
input []string
30+
expected []string
31+
}{
32+
{
33+
description: "nil slice",
34+
input: []string{"something", "somethingelse"},
35+
expected: []string{"something", "somethingelse"},
36+
},
37+
{
38+
description: "match libraries full path and so symlinks using globs",
39+
blockedFilter: []string{"*.so", "*.so.[0-9]"},
40+
input: []string{"/foo/bar/libsomething.so", "libsometing.so", "libsometing.so.1", "libsometing.so.1.2.3"},
41+
expected: []string{"/foo/bar/libsomething.so", "libsometing.so.1.2.3"},
42+
},
43+
{
44+
description: "match libraries full path and so symlinks using globs with any path prefix",
45+
blockedFilter: []string{"**/*.so", "**/*.so.[0-9]"},
46+
input: []string{"/foo/bar/libsomething.so", "libsometing.so", "libsometing.so.1", "libsometing.so.1.2.3"},
47+
expected: []string{"libsometing.so.1.2.3"},
48+
},
49+
}
2750

28-
require.ElementsMatch(t, []string{"libsometing.so.1.2.3"}, filtered)
51+
for _, tc := range testCases {
52+
t.Run(tc.description, func(t *testing.T) {
53+
filtered := ignoreMountSpecPatterns(tc.blockedFilter).Apply(tc.input...)
54+
require.ElementsMatch(t, tc.expected, filtered)
55+
})
56+
}
2957
}

‎internal/platform-support/tegra/tegra.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ type tegraOptions struct {
3131
driverRoot string
3232
nvidiaCTKPath string
3333
librarySearchPaths []string
34+
ignorePatterns ignoreMountSpecPatterns
3435

3536
// The following can be overridden for testing
3637
symlinkLocator lookup.Locator
@@ -132,3 +133,10 @@ func WithLibrarySearchPaths(librarySearchPaths ...string) Option {
132133
o.librarySearchPaths = librarySearchPaths
133134
}
134135
}
136+
137+
// WithIngorePatterns sets patterns to ignore in the CSV files
138+
func WithIngorePatterns(ignorePatterns ...string) Option {
139+
return func(o *tegraOptions) {
140+
o.ignorePatterns = ignoreMountSpecPatterns(ignorePatterns)
141+
}
142+
}

‎pkg/nvcdi/lib-csv.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func (l *csvlib) GetAllDeviceSpecs() ([]specs.Device, error) {
4545
tegra.WithNVIDIACTKPath(l.nvidiaCTKPath),
4646
tegra.WithCSVFiles(l.csvFiles),
4747
tegra.WithLibrarySearchPaths(l.librarySearchPaths...),
48+
tegra.WithIngorePatterns(l.csvIgnorePatterns...),
4849
)
4950
if err != nil {
5051
return nil, fmt.Errorf("failed to create discoverer for CSV files: %v", err)

‎pkg/nvcdi/lib.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ type nvcdilib struct {
4747
nvidiaCTKPath string
4848
librarySearchPaths []string
4949

50-
csvFiles []string
50+
csvFiles []string
51+
csvIgnorePatterns []string
5152

5253
vendor string
5354
class string

‎pkg/nvcdi/options.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,13 @@ func WithCSVFiles(csvFiles []string) Option {
104104
}
105105
}
106106

107+
// WithCSVIgnorePatterns sets the ignore patterns for entries in the CSV files.
108+
func WithCSVIgnorePatterns(csvIgnorePatterns []string) Option {
109+
return func(o *nvcdilib) {
110+
o.csvIgnorePatterns = csvIgnorePatterns
111+
}
112+
}
113+
107114
// WithLibrarySearchPaths sets the library search paths.
108115
// This is currently only used for CSV-mode.
109116
func WithLibrarySearchPaths(paths []string) Option {

0 commit comments

Comments
 (0)