Skip to content

Commit edc5041

Browse files
klueskaklueska
committed
Merge branch 'upstream-update-devices-from-volume-mounts-semantics' into 'master'
Refactor accepting device lists from volume mounts as a boolean See merge request nvidia/container-toolkit/container-toolkit!20
2 parents 322006c + 2c18094 commit edc5041

File tree

8 files changed

+77
-40
lines changed

8 files changed

+77
-40
lines changed

‎config/config.toml.amzn‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
disable-require = false
22
#swarm-resource = "DOCKER_RESOURCE_GPU"
33
#accept-nvidia-visible-devices-envvar-when-unprivileged = true
4-
#look-for-nvidia-visible-devices-as-volume-mounts-under = "/var/run/nvidia-container-devices"
4+
#accept-nvidia-visible-devices-as-volume-mounts = false
55

66
[nvidia-container-cli]
77
#root = "/run/nvidia/driver"

‎config/config.toml.centos‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
disable-require = false
22
#swarm-resource = "DOCKER_RESOURCE_GPU"
33
#accept-nvidia-visible-devices-envvar-when-unprivileged = true
4-
#look-for-nvidia-visible-devices-as-volume-mounts-under = "/var/run/nvidia-container-devices"
4+
#accept-nvidia-visible-devices-as-volume-mounts = false
55

66
[nvidia-container-cli]
77
#root = "/run/nvidia/driver"

‎config/config.toml.debian‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
disable-require = false
22
#swarm-resource = "DOCKER_RESOURCE_GPU"
33
#accept-nvidia-visible-devices-envvar-when-unprivileged = true
4-
#look-for-nvidia-visible-devices-as-volume-mounts-under = "/var/run/nvidia-container-devices"
4+
#accept-nvidia-visible-devices-as-volume-mounts = false
55

66
[nvidia-container-cli]
77
#root = "/run/nvidia/driver"

‎config/config.toml.opensuse-leap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
disable-require = false
22
#swarm-resource = "DOCKER_RESOURCE_GPU"
33
#accept-nvidia-visible-devices-envvar-when-unprivileged = true
4-
#look-for-nvidia-visible-devices-as-volume-mounts-under = "/var/run/nvidia-container-devices"
4+
#accept-nvidia-visible-devices-as-volume-mounts = false
55

66
[nvidia-container-cli]
77
#root = "/run/nvidia/driver"

‎config/config.toml.ubuntu‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
disable-require = false
22
#swarm-resource = "DOCKER_RESOURCE_GPU"
33
#accept-nvidia-visible-devices-envvar-when-unprivileged = true
4-
#look-for-nvidia-visible-devices-as-volume-mounts-under = "/var/run/nvidia-container-devices"
4+
#accept-nvidia-visible-devices-as-volume-mounts = false
55

66
[nvidia-container-cli]
77
#root = "/run/nvidia/driver"

‎pkg/container_config.go‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ const (
3535
capSysAdmin = "CAP_SYS_ADMIN"
3636
)
3737

38+
const (
39+
deviceListAsVolumeMountsRoot = "/var/run/nvidia-container-devices"
40+
)
41+
3842
type nvidiaConfig struct {
3943
Devices string
4044
MigConfigDevices string
@@ -236,10 +240,10 @@ func getDevicesFromEnvvar(env map[string]string, legacyImage bool) *string {
236240
return devices
237241
}
238242

239-
func getDevicesFromMounts(root string, mounts []Mount) *string {
243+
func getDevicesFromMounts(mounts []Mount) *string {
240244
var devices []string
241245
for _, m := range mounts {
242-
root := filepath.Clean(root)
246+
root := filepath.Clean(deviceListAsVolumeMountsRoot)
243247
source := filepath.Clean(m.Source)
244248
destination := filepath.Clean(m.Destination)
245249

@@ -274,14 +278,16 @@ func getDevicesFromMounts(root string, mounts []Mount) *string {
274278
}
275279

276280
func getDevices(hookConfig *HookConfig, env map[string]string, mounts []Mount, privileged bool, legacyImage bool) *string {
277-
// Try and get the device list from mount volumes first
278-
devices := getDevicesFromMounts(*hookConfig.DeviceListVolumeMount, mounts)
279-
if devices != nil {
280-
return devices
281+
// If enabled, try and get the device list from volume mounts first
282+
if hookConfig.AcceptDeviceListAsVolumeMounts {
283+
devices := getDevicesFromMounts(mounts)
284+
if devices != nil {
285+
return devices
286+
}
281287
}
282288

283289
// Fallback to reading from the environment variable if privileges are correct
284-
devices = getDevicesFromEnvvar(env, legacyImage)
290+
devices := getDevicesFromEnvvar(env, legacyImage)
285291
if devices == nil {
286292
return nil
287293
}

‎pkg/container_test.go‎

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -454,30 +454,26 @@ func TestGetNvidiaConfig(t *testing.T) {
454454
func TestGetDevicesFromMounts(t *testing.T) {
455455
var tests = []struct {
456456
description string
457-
root string
458457
mounts []Mount
459458
expectedDevices *string
460459
}{
461460
{
462461
description: "No mounts",
463-
root: defaultDeviceListVolumeMount,
464462
mounts: nil,
465463
expectedDevices: nil,
466464
},
467465
{
468466
description: "Host path is not /dev/null",
469-
root: defaultDeviceListVolumeMount,
470467
mounts: []Mount{
471468
{
472469
Source: "/not/dev/null",
473-
Destination: filepath.Join(defaultDeviceListVolumeMount, "GPU0"),
470+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU0"),
474471
},
475472
},
476473
expectedDevices: nil,
477474
},
478475
{
479476
description: "Container path is not prefixed by 'root'",
480-
root: defaultDeviceListVolumeMount,
481477
mounts: []Mount{
482478
{
483479
Source: "/dev/null",
@@ -488,49 +484,46 @@ func TestGetDevicesFromMounts(t *testing.T) {
488484
},
489485
{
490486
description: "Container path is only 'root'",
491-
root: defaultDeviceListVolumeMount,
492487
mounts: []Mount{
493488
{
494489
Source: "/dev/null",
495-
Destination: defaultDeviceListVolumeMount,
490+
Destination: deviceListAsVolumeMountsRoot,
496491
},
497492
},
498493
expectedDevices: nil,
499494
},
500495
{
501496
description: "Discover 2 devices",
502-
root: defaultDeviceListVolumeMount,
503497
mounts: []Mount{
504498
{
505499
Source: "/dev/null",
506-
Destination: filepath.Join(defaultDeviceListVolumeMount, "GPU0"),
500+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU0"),
507501
},
508502
{
509503
Source: "/dev/null",
510-
Destination: filepath.Join(defaultDeviceListVolumeMount, "GPU1"),
504+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU1"),
511505
},
512506
},
513507
expectedDevices: &[]string{"GPU0,GPU1"}[0],
514508
},
515509
{
516510
description: "Discover 2 devices with slashes in the name",
517-
root: defaultDeviceListVolumeMount,
518511
mounts: []Mount{
519512
{
520513
Source: "/dev/null",
521-
Destination: filepath.Join(defaultDeviceListVolumeMount, "GPU0-MIG0/0/1"),
514+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU0-MIG0/0/1"),
522515
},
523516
{
524517
Source: "/dev/null",
525-
Destination: filepath.Join(defaultDeviceListVolumeMount, "GPU1-MIG0/0/1"),
518+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU1-MIG0/0/1"),
526519
},
527520
},
528521
expectedDevices: &[]string{"GPU0-MIG0/0/1,GPU1-MIG0/0/1"}[0],
529522
},
530523
}
531524
for _, tc := range tests {
532525
t.Run(tc.description, func(t *testing.T) {
533-
devices := getDevicesFromMounts(tc.root, tc.mounts)
526+
devices := getDevicesFromMounts(tc.mounts)
534527
if !reflect.DeepEqual(devices, tc.expectedDevices) {
535528
t.Errorf("Unexpected devices (got: %v, wanted: %v)", *devices, *tc.expectedDevices)
536529
}
@@ -545,6 +538,7 @@ func TestDeviceListSourcePriority(t *testing.T) {
545538
envvarDevices string
546539
privileged bool
547540
acceptUnprivileged bool
541+
acceptMounts bool
548542
expectedDevices *string
549543
expectedPanic bool
550544
}{
@@ -553,16 +547,17 @@ func TestDeviceListSourcePriority(t *testing.T) {
553547
mountDevices: []Mount{
554548
{
555549
Source: "/dev/null",
556-
Destination: filepath.Join(defaultDeviceListVolumeMount, "GPU0"),
550+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU0"),
557551
},
558552
{
559553
Source: "/dev/null",
560-
Destination: filepath.Join(defaultDeviceListVolumeMount, "GPU1"),
554+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU1"),
561555
},
562556
},
563557
envvarDevices: "GPU2,GPU3",
564558
privileged: false,
565559
acceptUnprivileged: false,
560+
acceptMounts: true,
566561
expectedDevices: &[]string{"GPU0,GPU1"}[0],
567562
},
568563
{
@@ -571,6 +566,7 @@ func TestDeviceListSourcePriority(t *testing.T) {
571566
envvarDevices: "GPU0,GPU1",
572567
privileged: false,
573568
acceptUnprivileged: false,
569+
acceptMounts: true,
574570
expectedPanic: true,
575571
},
576572
{
@@ -579,6 +575,7 @@ func TestDeviceListSourcePriority(t *testing.T) {
579575
envvarDevices: "GPU0,GPU1",
580576
privileged: true,
581577
acceptUnprivileged: false,
578+
acceptMounts: true,
582579
expectedDevices: &[]string{"GPU0,GPU1"}[0],
583580
},
584581
{
@@ -587,8 +584,45 @@ func TestDeviceListSourcePriority(t *testing.T) {
587584
envvarDevices: "GPU0,GPU1",
588585
privileged: false,
589586
acceptUnprivileged: true,
587+
acceptMounts: true,
590588
expectedDevices: &[]string{"GPU0,GPU1"}[0],
591589
},
590+
{
591+
description: "Mount devices, unprivileged, accept unprivileged, no accept mounts",
592+
mountDevices: []Mount{
593+
{
594+
Source: "/dev/null",
595+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU0"),
596+
},
597+
{
598+
Source: "/dev/null",
599+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU1"),
600+
},
601+
},
602+
envvarDevices: "GPU2,GPU3",
603+
privileged: false,
604+
acceptUnprivileged: true,
605+
acceptMounts: false,
606+
expectedDevices: &[]string{"GPU2,GPU3"}[0],
607+
},
608+
{
609+
description: "Mount devices, unprivileged, no accept unprivileged, no accept mounts",
610+
mountDevices: []Mount{
611+
{
612+
Source: "/dev/null",
613+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU0"),
614+
},
615+
{
616+
Source: "/dev/null",
617+
Destination: filepath.Join(deviceListAsVolumeMountsRoot, "GPU1"),
618+
},
619+
},
620+
envvarDevices: "GPU2,GPU3",
621+
privileged: false,
622+
acceptUnprivileged: false,
623+
acceptMounts: false,
624+
expectedPanic: true,
625+
},
592626
}
593627
for _, tc := range tests {
594628
t.Run(tc.description, func(t *testing.T) {
@@ -600,6 +634,7 @@ func TestDeviceListSourcePriority(t *testing.T) {
600634
}
601635
hookConfig := getDefaultHookConfig()
602636
hookConfig.AcceptEnvvarUnprivileged = tc.acceptUnprivileged
637+
hookConfig.AcceptDeviceListAsVolumeMounts = tc.acceptMounts
603638
devices = getDevices(&hookConfig, env, tc.mountDevices, tc.privileged, false)
604639
}
605640

‎pkg/hook_config.go‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,6 @@ const (
1313
driverPath = "/run/nvidia/driver"
1414
)
1515

16-
const (
17-
defaultDeviceListVolumeMount = "/var/run/nvidia-container-devices"
18-
)
19-
2016
var defaultPaths = [...]string{
2117
path.Join(driverPath, configPath),
2218
configPath,
@@ -38,20 +34,20 @@ type CLIConfig struct {
3834

3935
// HookConfig : options for the nvidia-container-toolkit.
4036
type HookConfig struct {
41-
DisableRequire bool `toml:"disable-require"`
42-
SwarmResource *string `toml:"swarm-resource"`
43-
AcceptEnvvarUnprivileged bool `toml:"accept-nvidia-visible-devices-envvar-when-unprivileged"`
44-
DeviceListVolumeMount *string `toml:"look-for-nvidia-visible-devices-as-volume-mounts-under"`
37+
DisableRequire bool `toml:"disable-require"`
38+
SwarmResource *string `toml:"swarm-resource"`
39+
AcceptEnvvarUnprivileged bool `toml:"accept-nvidia-visible-devices-envvar-when-unprivileged"`
40+
AcceptDeviceListAsVolumeMounts bool `toml:"accept-nvidia-visible-devices-as-volume-mounts"`
4541

4642
NvidiaContainerCLI CLIConfig `toml:"nvidia-container-cli"`
4743
}
4844

4945
func getDefaultHookConfig() (config HookConfig) {
5046
return HookConfig{
51-
DisableRequire: false,
52-
SwarmResource: nil,
53-
AcceptEnvvarUnprivileged: true,
54-
DeviceListVolumeMount: &[]string{defaultDeviceListVolumeMount}[0],
47+
DisableRequire: false,
48+
SwarmResource: nil,
49+
AcceptEnvvarUnprivileged: true,
50+
AcceptDeviceListAsVolumeMounts: false,
5551
NvidiaContainerCLI: CLIConfig{
5652
Root: nil,
5753
Path: nil,

0 commit comments

Comments
 (0)