Skip to content

Commit 3626a13

Browse files
author
Evan Lezar
committed
Merge branch 'fix-disable-require' into 'main'
Return empty requirements if NVIDIA_DISABLE_REQUIRE is true See merge request nvidia/container-toolkit/container-toolkit!438
2 parents 7451e6e + 6750ce1 commit 3626a13

File tree

7 files changed

+107
-54
lines changed

7 files changed

+107
-54
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* Create ouput folders if required when running `nvidia-ctk runtime configure`
1717
* Generate default config as post-install step.
1818
* Added support for detecting GSP firmware at custom paths when generating CDI specifications.
19+
* Added logic to skip the extraction of image requirements if NVIDIA_DISABLE_REQUIRES is set to true.
1920

2021
* [libnvidia-container] Support OpenSSL 3 with the Encrypt/Decrypt library
2122

cmd/nvidia-container-runtime-hook/container_config.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,10 @@ type nvidiaConfig struct {
3838
MigConfigDevices string
3939
MigMonitorDevices string
4040
DriverCapabilities string
41-
Requirements []string
42-
DisableRequire bool
41+
// Requirements defines the requirements DSL for the container to run.
42+
// This is empty if no specific requirements are needed, or if requirements are
43+
// explicitly disabled.
44+
Requirements []string
4345
}
4446

4547
type containerConfig struct {
@@ -327,15 +329,12 @@ func getNvidiaConfig(hookConfig *HookConfig, image image.CUDA, mounts []Mount, p
327329
log.Panicln("failed to get requirements", err)
328330
}
329331

330-
disableRequire := image.HasDisableRequire()
331-
332332
return &nvidiaConfig{
333333
Devices: devices,
334334
MigConfigDevices: migConfigDevices,
335335
MigMonitorDevices: migMonitorDevices,
336336
DriverCapabilities: driverCapabilities,
337337
Requirements: requirements,
338-
DisableRequire: disableRequire,
339338
}
340339
}
341340

@@ -353,7 +352,10 @@ func getContainerConfig(hook HookConfig) (config containerConfig) {
353352

354353
s := loadSpec(path.Join(b, "config.json"))
355354

356-
image, err := image.NewCUDAImageFromEnv(s.Process.Env)
355+
image, err := image.New(
356+
image.WithEnv(s.Process.Env),
357+
image.WithDisableRequire(hook.DisableRequire),
358+
)
357359
if err != nil {
358360
log.Panicln(err)
359361
}

cmd/nvidia-container-runtime-hook/container_config_test.go

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ func TestGetNvidiaConfig(t *testing.T) {
4040
Devices: "all",
4141
DriverCapabilities: allDriverCapabilities.String(),
4242
Requirements: []string{"cuda>=9.0"},
43-
DisableRequire: false,
4443
},
4544
},
4645
{
@@ -54,7 +53,6 @@ func TestGetNvidiaConfig(t *testing.T) {
5453
Devices: "all",
5554
DriverCapabilities: allDriverCapabilities.String(),
5655
Requirements: []string{"cuda>=9.0"},
57-
DisableRequire: false,
5856
},
5957
},
6058
{
@@ -86,7 +84,6 @@ func TestGetNvidiaConfig(t *testing.T) {
8684
Devices: "",
8785
DriverCapabilities: allDriverCapabilities.String(),
8886
Requirements: []string{"cuda>=9.0"},
89-
DisableRequire: false,
9087
},
9188
},
9289
{
@@ -100,7 +97,6 @@ func TestGetNvidiaConfig(t *testing.T) {
10097
Devices: "gpu0,gpu1",
10198
DriverCapabilities: allDriverCapabilities.String(),
10299
Requirements: []string{"cuda>=9.0"},
103-
DisableRequire: false,
104100
},
105101
},
106102
{
@@ -115,7 +111,6 @@ func TestGetNvidiaConfig(t *testing.T) {
115111
Devices: "gpu0,gpu1",
116112
DriverCapabilities: defaultDriverCapabilities.String(),
117113
Requirements: []string{"cuda>=9.0"},
118-
DisableRequire: false,
119114
},
120115
},
121116
{
@@ -130,7 +125,6 @@ func TestGetNvidiaConfig(t *testing.T) {
130125
Devices: "gpu0,gpu1",
131126
DriverCapabilities: allDriverCapabilities.String(),
132127
Requirements: []string{"cuda>=9.0"},
133-
DisableRequire: false,
134128
},
135129
},
136130
{
@@ -145,7 +139,6 @@ func TestGetNvidiaConfig(t *testing.T) {
145139
Devices: "gpu0,gpu1",
146140
DriverCapabilities: "video,display",
147141
Requirements: []string{"cuda>=9.0"},
148-
DisableRequire: false,
149142
},
150143
},
151144
{
@@ -162,7 +155,6 @@ func TestGetNvidiaConfig(t *testing.T) {
162155
Devices: "gpu0,gpu1",
163156
DriverCapabilities: "video,display",
164157
Requirements: []string{"cuda>=9.0", "req0=true", "req1=false"},
165-
DisableRequire: false,
166158
},
167159
},
168160
{
@@ -179,8 +171,7 @@ func TestGetNvidiaConfig(t *testing.T) {
179171
expectedConfig: &nvidiaConfig{
180172
Devices: "gpu0,gpu1",
181173
DriverCapabilities: "video,display",
182-
Requirements: []string{"cuda>=9.0", "req0=true", "req1=false"},
183-
DisableRequire: true,
174+
Requirements: []string{},
184175
},
185176
},
186177
{
@@ -211,7 +202,6 @@ func TestGetNvidiaConfig(t *testing.T) {
211202
Devices: "all",
212203
DriverCapabilities: defaultDriverCapabilities.String(),
213204
Requirements: []string{"cuda>=9.0"},
214-
DisableRequire: false,
215205
},
216206
},
217207
{
@@ -243,7 +233,6 @@ func TestGetNvidiaConfig(t *testing.T) {
243233
Devices: "",
244234
DriverCapabilities: defaultDriverCapabilities.String(),
245235
Requirements: []string{"cuda>=9.0"},
246-
DisableRequire: false,
247236
},
248237
},
249238
{
@@ -257,7 +246,6 @@ func TestGetNvidiaConfig(t *testing.T) {
257246
Devices: "gpu0,gpu1",
258247
DriverCapabilities: defaultDriverCapabilities.String(),
259248
Requirements: []string{"cuda>=9.0"},
260-
DisableRequire: false,
261249
},
262250
},
263251
{
@@ -272,7 +260,6 @@ func TestGetNvidiaConfig(t *testing.T) {
272260
Devices: "gpu0,gpu1",
273261
DriverCapabilities: defaultDriverCapabilities.String(),
274262
Requirements: []string{"cuda>=9.0"},
275-
DisableRequire: false,
276263
},
277264
},
278265
{
@@ -287,7 +274,6 @@ func TestGetNvidiaConfig(t *testing.T) {
287274
Devices: "gpu0,gpu1",
288275
DriverCapabilities: allDriverCapabilities.String(),
289276
Requirements: []string{"cuda>=9.0"},
290-
DisableRequire: false,
291277
},
292278
},
293279
{
@@ -302,7 +288,6 @@ func TestGetNvidiaConfig(t *testing.T) {
302288
Devices: "gpu0,gpu1",
303289
DriverCapabilities: "video,display",
304290
Requirements: []string{"cuda>=9.0"},
305-
DisableRequire: false,
306291
},
307292
},
308293
{
@@ -319,7 +304,6 @@ func TestGetNvidiaConfig(t *testing.T) {
319304
Devices: "gpu0,gpu1",
320305
DriverCapabilities: "video,display",
321306
Requirements: []string{"cuda>=9.0", "req0=true", "req1=false"},
322-
DisableRequire: false,
323307
},
324308
},
325309
{
@@ -336,8 +320,7 @@ func TestGetNvidiaConfig(t *testing.T) {
336320
expectedConfig: &nvidiaConfig{
337321
Devices: "gpu0,gpu1",
338322
DriverCapabilities: "video,display",
339-
Requirements: []string{"cuda>=9.0", "req0=true", "req1=false"},
340-
DisableRequire: true,
323+
Requirements: []string{},
341324
},
342325
},
343326
{
@@ -351,7 +334,6 @@ func TestGetNvidiaConfig(t *testing.T) {
351334
Devices: "all",
352335
DriverCapabilities: defaultDriverCapabilities.String(),
353336
Requirements: []string{},
354-
DisableRequire: false,
355337
},
356338
},
357339
{
@@ -367,7 +349,6 @@ func TestGetNvidiaConfig(t *testing.T) {
367349
MigConfigDevices: "mig0,mig1",
368350
DriverCapabilities: defaultDriverCapabilities.String(),
369351
Requirements: []string{"cuda>=9.0"},
370-
DisableRequire: false,
371352
},
372353
},
373354
{
@@ -393,7 +374,6 @@ func TestGetNvidiaConfig(t *testing.T) {
393374
MigMonitorDevices: "mig0,mig1",
394375
DriverCapabilities: defaultDriverCapabilities.String(),
395376
Requirements: []string{"cuda>=9.0"},
396-
DisableRequire: false,
397377
},
398378
},
399379
{
@@ -525,7 +505,6 @@ func TestGetNvidiaConfig(t *testing.T) {
525505
require.Equal(t, tc.expectedConfig.DriverCapabilities, config.DriverCapabilities)
526506

527507
require.ElementsMatch(t, tc.expectedConfig.Requirements, config.Requirements)
528-
require.Equal(t, tc.expectedConfig.DisableRequire, config.DisableRequire)
529508
})
530509
}
531510
}

cmd/nvidia-container-runtime-hook/main.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,8 @@ func doPrestart() {
137137
args = append(args, capabilityToCLI(cap))
138138
}
139139

140-
if !hook.DisableRequire && !nvidia.DisableRequire {
141-
for _, req := range nvidia.Requirements {
142-
args = append(args, fmt.Sprintf("--require=%s", req))
143-
}
140+
for _, req := range nvidia.Requirements {
141+
args = append(args, fmt.Sprintf("--require=%s", req))
144142
}
145143

146144
args = append(args, fmt.Sprintf("--pid=%s", strconv.FormatUint(uint64(container.Pid), 10)))

internal/config/image/builder.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/**
2+
# Copyright (c) NVIDIA CORPORATION. All rights reserved.
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
**/
16+
17+
package image
18+
19+
import (
20+
"fmt"
21+
"strings"
22+
)
23+
24+
type builder struct {
25+
env []string
26+
disableRequire bool
27+
}
28+
29+
// New creates a new CUDA image from the input options.
30+
func New(opt ...Option) (CUDA, error) {
31+
b := &builder{}
32+
for _, o := range opt {
33+
o(b)
34+
}
35+
36+
return b.build()
37+
}
38+
39+
// build creates a CUDA image from the builder.
40+
func (b builder) build() (CUDA, error) {
41+
c := make(CUDA)
42+
43+
for _, e := range b.env {
44+
parts := strings.SplitN(e, "=", 2)
45+
if len(parts) != 2 {
46+
return nil, fmt.Errorf("invalid environment variable: %v", e)
47+
}
48+
c[parts[0]] = parts[1]
49+
}
50+
51+
if b.disableRequire {
52+
c[envNVDisableRequire] = "true"
53+
}
54+
55+
return c, nil
56+
}
57+
58+
// Option is a functional option for creating a CUDA image.
59+
type Option func(*builder)
60+
61+
// WithDisableRequire sets the disable require option.
62+
func WithDisableRequire(disableRequire bool) Option {
63+
return func(b *builder) {
64+
b.disableRequire = disableRequire
65+
}
66+
}
67+
68+
// WithEnv sets the environment variables to use when creating the CUDA image.
69+
func WithEnv(env []string) Option {
70+
return func(b *builder) {
71+
b.env = env
72+
}
73+
}

internal/config/image/cuda_image.go

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -42,27 +42,18 @@ type CUDA map[string]string
4242
// NewCUDAImageFromSpec creates a CUDA image from the input OCI runtime spec.
4343
// The process environment is read (if present) to construc the CUDA Image.
4444
func NewCUDAImageFromSpec(spec *specs.Spec) (CUDA, error) {
45-
if spec == nil || spec.Process == nil {
46-
return NewCUDAImageFromEnv(nil)
45+
var env []string
46+
if spec != nil && spec.Process != nil {
47+
env = spec.Process.Env
4748
}
4849

49-
return NewCUDAImageFromEnv(spec.Process.Env)
50+
return New(WithEnv(env))
5051
}
5152

5253
// NewCUDAImageFromEnv creates a CUDA image from the input environment. The environment
5354
// is a list of strings of the form ENVAR=VALUE.
5455
func NewCUDAImageFromEnv(env []string) (CUDA, error) {
55-
c := make(CUDA)
56-
57-
for _, e := range env {
58-
parts := strings.SplitN(e, "=", 2)
59-
if len(parts) != 2 {
60-
return nil, fmt.Errorf("invalid environment variable: %v", e)
61-
}
62-
c[parts[0]] = parts[1]
63-
}
64-
65-
return c, nil
56+
return New(WithEnv(env))
6657
}
6758

6859
// IsLegacy returns whether the associated CUDA image is a "legacy" image. An
@@ -77,11 +68,9 @@ func (i CUDA) IsLegacy() bool {
7768
// GetRequirements returns the requirements from all NVIDIA_REQUIRE_ environment
7869
// variables.
7970
func (i CUDA) GetRequirements() ([]string, error) {
80-
// TODO: We need not process this if disable require is set, but this will be done
81-
// in a single follow-up to ensure that the behavioural change is accurately captured.
82-
// if i.HasDisableRequire() {
83-
// return nil, nil
84-
// }
71+
if i.HasDisableRequire() {
72+
return nil, nil
73+
}
8574

8675
// All variables with the "NVIDIA_REQUIRE_" prefix are passed to nvidia-container-cli
8776
var requirements []string
@@ -159,9 +148,10 @@ func (i CUDA) GetDriverCapabilities() DriverCapabilities {
159148
}
160149

161150
func (i CUDA) legacyVersion() (string, error) {
162-
majorMinor, err := parseMajorMinorVersion(i[envCUDAVersion])
151+
cudaVersion := i[envCUDAVersion]
152+
majorMinor, err := parseMajorMinorVersion(cudaVersion)
163153
if err != nil {
164-
return "", fmt.Errorf("invalid CUDA version: %v", err)
154+
return "", fmt.Errorf("invalid CUDA version %v: %v", cudaVersion, err)
165155
}
166156

167157
return majorMinor, nil

internal/config/image/cuda_image_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,16 @@ func TestGetRequirements(t *testing.T) {
106106
env: []string{"CUDA_VERSION=11.6", "NVIDIA_REQUIRE_BRAND=brand=tesla"},
107107
requirements: []string{"cuda>=11.6", "brand=tesla"},
108108
},
109+
{
110+
description: "NVIDIA_DISABLE_REQUIRE ignores requirements",
111+
env: []string{"NVIDIA_REQUIRE_CUDA=cuda>=11.6", "NVIDIA_REQUIRE_BRAND=brand=tesla", "NVIDIA_DISABLE_REQUIRE=true"},
112+
requirements: []string{},
113+
},
114+
{
115+
description: "NVIDIA_DISABLE_REQUIRE ignores legacy image requirements",
116+
env: []string{"CUDA_VERSION=11.6", "NVIDIA_REQUIRE_BRAND=brand=tesla", "NVIDIA_DISABLE_REQUIRE=true"},
117+
requirements: []string{},
118+
},
109119
}
110120

111121
for _, tc := range testCases {

0 commit comments

Comments
 (0)