Skip to content

Commit 807c87e

Browse files
author
Evan Lezar
committed
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
2 parents 6094eff + f63ad3d commit 807c87e

File tree

12 files changed

+345
-53
lines changed

12 files changed

+345
-53
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# NVIDIA Container Toolkit Changelog
22

3+
## v1.14.2
4+
* 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.
6+
37
## v1.14.1
48
* Fixed bug where contents of `/etc/nvidia-container-runtime/config.toml` is ignored by the NVIDIA Container Runtime Hook.
59

‎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: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,51 +28,45 @@ import (
2828
// newDiscovererFromCSVFiles creates a discoverer for the specified CSV files. A logger is also supplied.
2929
// The constructed discoverer is comprised of a list, with each element in the list being associated with a
3030
// single CSV files.
31-
func newDiscovererFromCSVFiles(logger logger.Interface, files []string, driverRoot string, nvidiaCTKPath string, librarySearchPaths []string) (discover.Discover, error) {
32-
if len(files) == 0 {
33-
logger.Warningf("No CSV files specified")
31+
func (o tegraOptions) newDiscovererFromCSVFiles() (discover.Discover, error) {
32+
if len(o.csvFiles) == 0 {
33+
o.logger.Warningf("No CSV files specified")
3434
return discover.None{}, nil
3535
}
3636

37-
targetsByType := getTargetsFromCSVFiles(logger, files)
37+
targetsByType := getTargetsFromCSVFiles(o.logger, o.csvFiles)
3838

3939
devices := discover.NewDeviceDiscoverer(
40-
logger,
41-
lookup.NewCharDeviceLocator(lookup.WithLogger(logger), lookup.WithRoot(driverRoot)),
42-
driverRoot,
40+
o.logger,
41+
lookup.NewCharDeviceLocator(lookup.WithLogger(o.logger), lookup.WithRoot(o.driverRoot)),
42+
o.driverRoot,
4343
targetsByType[csv.MountSpecDev],
4444
)
4545

4646
directories := discover.NewMounts(
47-
logger,
48-
lookup.NewDirectoryLocator(lookup.WithLogger(logger), lookup.WithRoot(driverRoot)),
49-
driverRoot,
47+
o.logger,
48+
lookup.NewDirectoryLocator(lookup.WithLogger(o.logger), lookup.WithRoot(o.driverRoot)),
49+
o.driverRoot,
5050
targetsByType[csv.MountSpecDir],
5151
)
5252

5353
// Libraries and symlinks use the same locator.
54-
searchPaths := append(librarySearchPaths, "/")
55-
symlinkLocator := lookup.NewSymlinkLocator(
56-
lookup.WithLogger(logger),
57-
lookup.WithRoot(driverRoot),
58-
lookup.WithSearchPaths(searchPaths...),
59-
)
6054
libraries := discover.NewMounts(
61-
logger,
62-
symlinkLocator,
63-
driverRoot,
55+
o.logger,
56+
o.symlinkLocator,
57+
o.driverRoot,
6458
targetsByType[csv.MountSpecLib],
6559
)
6660

67-
nonLibSymlinks := ignoreFilenamePatterns{"*.so", "*.so.[0-9]"}.Apply(targetsByType[csv.MountSpecSym]...)
68-
logger.Debugf("Non-lib symlinks: %v", nonLibSymlinks)
61+
symlinkTargets := o.ignorePatterns.Apply(targetsByType[csv.MountSpecSym]...)
62+
o.logger.Debugf("Filtered symlink targets: %v", symlinkTargets)
6963
symlinks := discover.NewMounts(
70-
logger,
71-
symlinkLocator,
72-
driverRoot,
73-
nonLibSymlinks,
64+
o.logger,
65+
o.symlinkLocator,
66+
o.driverRoot,
67+
symlinkTargets,
7468
)
75-
createSymlinks := createCSVSymlinkHooks(logger, nonLibSymlinks, libraries, nvidiaCTKPath)
69+
createSymlinks := o.createCSVSymlinkHooks(symlinkTargets, libraries)
7670

7771
d := discover.Merge(
7872
devices,
@@ -87,7 +81,9 @@ func newDiscovererFromCSVFiles(logger logger.Interface, files []string, driverRo
8781

8882
// getTargetsFromCSVFiles returns the list of mount specs from the specified CSV files.
8983
// These are aggregated by mount spec type.
90-
func getTargetsFromCSVFiles(logger logger.Interface, files []string) map[csv.MountSpecType][]string {
84+
// TODO: We use a function variable here to allow this to be overridden for testing.
85+
// This should be properly mocked.
86+
var getTargetsFromCSVFiles = func(logger logger.Interface, files []string) map[csv.MountSpecType][]string {
9187
targetsByType := make(map[csv.MountSpecType][]string)
9288
for _, filename := range files {
9389
targets, err := loadCSVFile(logger, filename)

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

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,209 @@
1515
**/
1616

1717
package tegra
18+
19+
import (
20+
"fmt"
21+
"testing"
22+
23+
"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
24+
"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
25+
"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup"
26+
testlog "github.com/sirupsen/logrus/hooks/test"
27+
"github.com/stretchr/testify/require"
28+
29+
"github.com/NVIDIA/nvidia-container-toolkit/internal/platform-support/tegra/csv"
30+
)
31+
32+
func TestDiscovererFromCSVFiles(t *testing.T) {
33+
logger, _ := testlog.NewNullLogger()
34+
testCases := []struct {
35+
description string
36+
moutSpecs map[csv.MountSpecType][]string
37+
ignorePatterns []string
38+
symlinkLocator lookup.Locator
39+
symlinkChainLocator lookup.Locator
40+
symlinkResolver func(string) (string, error)
41+
expectedError error
42+
expectedMounts []discover.Mount
43+
expectedMountsError error
44+
expectedHooks []discover.Hook
45+
expectedHooksError error
46+
}{
47+
{
48+
// TODO: This current resolves to two mounts that are the same.
49+
// These are deduplicated at a later stage. We could consider deduplicating earlier in the pipeline.
50+
description: "symlink is resolved to target; mounts and symlink are created",
51+
moutSpecs: map[csv.MountSpecType][]string{
52+
"lib": {"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"},
53+
"sym": {"/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so"},
54+
},
55+
symlinkLocator: &lookup.LocatorMock{
56+
LocateFunc: func(path string) ([]string, error) {
57+
if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
58+
return []string{"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"}, nil
59+
}
60+
return []string{path}, nil
61+
},
62+
},
63+
symlinkChainLocator: &lookup.LocatorMock{
64+
LocateFunc: func(path string) ([]string, error) {
65+
if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
66+
return []string{"/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so", "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so"}, nil
67+
}
68+
return nil, fmt.Errorf("Unexpected path: %v", path)
69+
},
70+
},
71+
symlinkResolver: func(path string) (string, error) {
72+
if path == "/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so" {
73+
return "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so", nil
74+
}
75+
return path, nil
76+
},
77+
expectedMounts: []discover.Mount{
78+
{
79+
Path: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
80+
HostPath: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
81+
Options: []string{"ro", "nosuid", "nodev", "bind"},
82+
},
83+
{
84+
Path: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
85+
HostPath: "/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so",
86+
Options: []string{"ro", "nosuid", "nodev", "bind"},
87+
},
88+
},
89+
expectedHooks: []discover.Hook{
90+
{
91+
Lifecycle: "createContainer",
92+
Path: "/usr/bin/nvidia-ctk",
93+
Args: []string{
94+
"nvidia-ctk",
95+
"hook",
96+
"create-symlinks",
97+
"--link",
98+
"/usr/lib/aarch64-linux-gnu/tegra/libv4l2_nvargus.so::/usr/lib/aarch64-linux-gnu/libv4l/plugins/nv/libv4l2_nvargus.so",
99+
},
100+
},
101+
},
102+
},
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+
},
183+
}
184+
185+
for _, tc := range testCases {
186+
t.Run(tc.description, func(t *testing.T) {
187+
defer setGetTargetsFromCSVFiles(tc.moutSpecs)()
188+
189+
o := tegraOptions{
190+
logger: logger,
191+
nvidiaCTKPath: "/usr/bin/nvidia-ctk",
192+
csvFiles: []string{"dummy"},
193+
ignorePatterns: tc.ignorePatterns,
194+
symlinkLocator: tc.symlinkLocator,
195+
symlinkChainLocator: tc.symlinkChainLocator,
196+
resolveSymlink: tc.symlinkResolver,
197+
}
198+
199+
d, err := o.newDiscovererFromCSVFiles()
200+
require.ErrorIs(t, err, tc.expectedError)
201+
202+
hooks, err := d.Hooks()
203+
require.ErrorIs(t, err, tc.expectedHooksError)
204+
require.EqualValues(t, tc.expectedHooks, hooks)
205+
206+
mounts, err := d.Mounts()
207+
require.ErrorIs(t, err, tc.expectedMountsError)
208+
require.EqualValues(t, tc.expectedMounts, mounts)
209+
210+
})
211+
}
212+
}
213+
214+
func setGetTargetsFromCSVFiles(ovverride map[csv.MountSpecType][]string) func() {
215+
original := getTargetsFromCSVFiles
216+
getTargetsFromCSVFiles = func(logger logger.Interface, files []string) map[csv.MountSpecType][]string {
217+
return ovverride
218+
}
219+
220+
return func() {
221+
getTargetsFromCSVFiles = original
222+
}
223+
}

‎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) {

0 commit comments

Comments
 (0)