Skip to content

Commit f882eb5

Browse files
ArangoGutierrezArangoGutierrez
committed
Load settings from config.toml file during CDI generation
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
1 parent fdcd250 commit f882eb5

File tree

7 files changed

+698
-6
lines changed

7 files changed

+698
-6
lines changed

‎cmd/nvidia-ctk/cdi/generate/generate.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,14 @@ type options struct {
6868
nvmllib nvml.Interface
6969
}
7070

71+
// Add setter methods for csv.files and csv.ignorePatterns
72+
func (o *options) SetCSVFiles(files []string) {
73+
o.csv.files = *cli.NewStringSlice(files...)
74+
}
75+
func (o *options) SetCSVIgnorePatterns(patterns []string) {
76+
o.csv.ignorePatterns = *cli.NewStringSlice(patterns...)
77+
}
78+
7179
// NewCommand constructs a generate-cdi command with the specified logger
7280
func NewCommand(logger logger.Interface) *cli.Command {
7381
c := command{
@@ -192,6 +200,16 @@ func (m command) build() *cli.Command {
192200
}
193201

194202
func (m command) validateFlags(c *cli.Context, opts *options) error {
203+
// Load config file as base configuration
204+
cfg, err := config.GetConfig()
205+
if err != nil {
206+
return fmt.Errorf("failed to load config: %v", err)
207+
}
208+
209+
// Use centralized flag resolution (CLI > config file > default)
210+
config.ResolveCDIGenerateOptions(c, cfg, opts)
211+
212+
// Additional validation (format, mode, etc.) can remain here if needed
195213
opts.format = strings.ToLower(opts.format)
196214
switch opts.format {
197215
case spec.FormatJSON:

‎cmd/nvidia-ctk/cdi/generate/generate_test.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package generate
1818

1919
import (
2020
"bytes"
21+
"os"
2122
"path/filepath"
2223
"strings"
2324
"testing"
@@ -32,6 +33,29 @@ import (
3233
)
3334

3435
func TestGenerateSpec(t *testing.T) {
36+
// Create a temporary directory for config
37+
tmpDir, err := os.MkdirTemp("", "nvidia-container-toolkit-test-*")
38+
require.NoError(t, err)
39+
defer os.RemoveAll(tmpDir)
40+
41+
// Create a temporary config file
42+
configContent := `
43+
[nvidia-container-runtime]
44+
mode = "nvml"
45+
[[nvidia-container-runtime.modes.cdi]]
46+
spec-dirs = ["/etc/cdi", "/usr/local/cdi"]
47+
[nvidia-container-runtime.modes.csv]
48+
mount-spec-path = "/etc/nvidia-container-runtime/host-files-for-container.d"
49+
`
50+
configPath := filepath.Join(tmpDir, "config.toml")
51+
err = os.WriteFile(configPath, []byte(configContent), 0600)
52+
require.NoError(t, err)
53+
54+
// Set XDG_CONFIG_HOME to point to our temporary directory
55+
oldXDGConfigHome := os.Getenv("XDG_CONFIG_HOME")
56+
os.Setenv("XDG_CONFIG_HOME", tmpDir)
57+
defer os.Setenv("XDG_CONFIG_HOME", oldXDGConfigHome)
58+
3559
t.Setenv("__NVCT_TESTING_DEVICES_ARE_FILES", "true")
3660
moduleRoot, err := test.GetModuleRoot()
3761
require.NoError(t, err)
@@ -63,6 +87,13 @@ func TestGenerateSpec(t *testing.T) {
6387
class: "device",
6488
nvidiaCDIHookPath: "/usr/bin/nvidia-cdi-hook",
6589
driverRoot: driverRoot,
90+
csv: struct {
91+
files cli.StringSlice
92+
ignorePatterns cli.StringSlice
93+
}{
94+
files: *cli.NewStringSlice("/etc/nvidia-container-runtime/host-files-for-container.d"),
95+
ignorePatterns: *cli.NewStringSlice(),
96+
},
6697
},
6798
expectedSpec: `---
6899
cdiVersion: 0.5.0
@@ -140,6 +171,13 @@ containerEdits:
140171
nvidiaCDIHookPath: "/usr/bin/nvidia-cdi-hook",
141172
driverRoot: driverRoot,
142173
disabledHooks: valueOf(cli.NewStringSlice("enable-cuda-compat")),
174+
csv: struct {
175+
files cli.StringSlice
176+
ignorePatterns cli.StringSlice
177+
}{
178+
files: *cli.NewStringSlice("/etc/nvidia-container-runtime/host-files-for-container.d"),
179+
ignorePatterns: *cli.NewStringSlice(),
180+
},
143181
},
144182
expectedSpec: `---
145183
cdiVersion: 0.5.0
@@ -209,6 +247,13 @@ containerEdits:
209247
nvidiaCDIHookPath: "/usr/bin/nvidia-cdi-hook",
210248
driverRoot: driverRoot,
211249
disabledHooks: valueOf(cli.NewStringSlice("enable-cuda-compat", "update-ldcache")),
250+
csv: struct {
251+
files cli.StringSlice
252+
ignorePatterns cli.StringSlice
253+
}{
254+
files: *cli.NewStringSlice("/etc/nvidia-container-runtime/host-files-for-container.d"),
255+
ignorePatterns: *cli.NewStringSlice(),
256+
},
212257
},
213258
expectedSpec: `---
214259
cdiVersion: 0.5.0
@@ -269,6 +314,13 @@ containerEdits:
269314
nvidiaCDIHookPath: "/usr/bin/nvidia-cdi-hook",
270315
driverRoot: driverRoot,
271316
disabledHooks: valueOf(cli.NewStringSlice("all")),
317+
csv: struct {
318+
files cli.StringSlice
319+
ignorePatterns cli.StringSlice
320+
}{
321+
files: *cli.NewStringSlice("/etc/nvidia-container-runtime/host-files-for-container.d"),
322+
ignorePatterns: *cli.NewStringSlice(),
323+
},
272324
},
273325
expectedSpec: `---
274326
cdiVersion: 0.5.0
@@ -311,6 +363,10 @@ containerEdits:
311363

312364
err := c.validateFlags(nil, &tc.options)
313365
require.ErrorIs(t, err, tc.expectedValidateError)
366+
// Set the ldconfig path to empty.
367+
// This is required during test because config.GetConfig() returns
368+
// the default ldconfig path, even if it is not set in the config file.
369+
tc.options.ldconfigPath = ""
314370
require.EqualValues(t, tc.expectedOptions, tc.options)
315371

316372
// Set up a mock server, reusing the DGX A100 mock.

‎cmd/nvidia-ctk/cdi/list/list.go‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/urfave/cli/v2"
2424
"tags.cncf.io/container-device-interface/pkg/cdi"
2525

26+
ctkconfig "github.com/NVIDIA/nvidia-container-toolkit/internal/config"
2627
"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
2728
)
2829

@@ -34,6 +35,11 @@ type config struct {
3435
cdiSpecDirs cli.StringSlice
3536
}
3637

38+
// SetCDISpecDirs sets the cdiSpecDirs field from a []string
39+
func (c *config) SetCDISpecDirs(dirs []string) {
40+
c.cdiSpecDirs = *cli.NewStringSlice(dirs...)
41+
}
42+
3743
// NewCommand constructs a cdi list command with the specified logger
3844
func NewCommand(logger logger.Interface) *cli.Command {
3945
c := command{
@@ -64,16 +70,27 @@ func (m command) build() *cli.Command {
6470
Usage: "specify the directories to scan for CDI specifications",
6571
Value: cli.NewStringSlice(cdi.DefaultSpecDirs...),
6672
Destination: &cfg.cdiSpecDirs,
73+
EnvVars: []string{"NVIDIA_CTK_CDI_SPEC_DIRS"},
6774
},
6875
}
6976

7077
return &c
7178
}
7279

73-
func (m command) validateFlags(c *cli.Context, cfg *config) error {
80+
func (m command) validateFlags(ctx *cli.Context, cfg *config) error {
81+
// Load config file as base configuration
82+
c, err := ctkconfig.GetConfig()
83+
if err != nil {
84+
return fmt.Errorf("failed to load config: %v", err)
85+
}
86+
87+
// Use centralized normalization
88+
ctkconfig.ResolveCDIListConfig(ctx, c, cfg)
89+
7490
if len(cfg.cdiSpecDirs.Value()) == 0 {
7591
return errors.New("at least one CDI specification directory must be specified")
7692
}
93+
7794
return nil
7895
}
7996

‎cmd/nvidia-ctk/cdi/list/list_test.go‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package list
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
8+
testlog "github.com/sirupsen/logrus/hooks/test"
9+
"github.com/stretchr/testify/require"
10+
"github.com/urfave/cli/v2"
11+
)
12+
13+
func TestValidateFlags(t *testing.T) {
14+
logger, _ := testlog.NewNullLogger()
15+
// Create a temporary directory for config
16+
tmpDir, err := os.MkdirTemp("", "nvidia-container-toolkit-test-*")
17+
require.NoError(t, err)
18+
defer os.RemoveAll(tmpDir)
19+
20+
// Create a temporary config file
21+
configContent := `
22+
[nvidia-container-runtime]
23+
mode = "cdi"
24+
[[nvidia-container-runtime.modes.cdi]]
25+
spec-dirs = ["/etc/cdi", "/usr/local/cdi"]
26+
`
27+
configPath := filepath.Join(tmpDir, "config.toml")
28+
err = os.WriteFile(configPath, []byte(configContent), 0600)
29+
require.NoError(t, err)
30+
31+
// Set XDG_CONFIG_HOME to point to our temporary directory
32+
oldXDGConfigHome := os.Getenv("XDG_CONFIG_HOME")
33+
os.Setenv("XDG_CONFIG_HOME", tmpDir)
34+
defer os.Setenv("XDG_CONFIG_HOME", oldXDGConfigHome)
35+
36+
tests := []struct {
37+
name string
38+
cliArgs []string
39+
envVars map[string]string
40+
expectedDirs []string
41+
expectError bool
42+
errorContains string
43+
}{
44+
{
45+
name: "command line takes precedence",
46+
cliArgs: []string{"--spec-dir=/custom/dir1", "--spec-dir=/custom/dir2"},
47+
expectedDirs: []string{"/custom/dir1", "/custom/dir2"},
48+
},
49+
{
50+
name: "environment variable takes precedence over config",
51+
envVars: map[string]string{"NVIDIA_CTK_CDI_SPEC_DIRS": "/env/dir1:/env/dir2"},
52+
expectedDirs: []string{"/env/dir1", "/env/dir2"},
53+
},
54+
{
55+
name: "config file used as fallback",
56+
expectedDirs: []string{"/etc/cdi", "/usr/local/cdi"},
57+
},
58+
}
59+
60+
for _, tt := range tests {
61+
t.Run(tt.name, func(t *testing.T) {
62+
// Set up environment variables
63+
for k, v := range tt.envVars {
64+
old := os.Getenv(k)
65+
os.Setenv(k, v)
66+
defer os.Setenv(k, old)
67+
}
68+
69+
// Create command
70+
cmd := NewCommand(logger)
71+
72+
// Create a new context with the command
73+
app := &cli.App{
74+
Commands: []*cli.Command{
75+
{
76+
Name: "cdi",
77+
Subcommands: []*cli.Command{cmd},
78+
},
79+
},
80+
}
81+
82+
// Run command
83+
args := append([]string{"nvidia-ctk", "cdi", "list"}, tt.cliArgs...)
84+
err := app.Run(args)
85+
86+
if tt.expectError {
87+
require.Error(t, err)
88+
require.Contains(t, err.Error(), tt.errorContains)
89+
return
90+
}
91+
92+
require.NoError(t, err)
93+
})
94+
}
95+
}

‎internal/config/config_test.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -445,11 +445,6 @@ func setGetDistIDLikeForTest(ids []string) func() {
445445
}
446446
}
447447

448-
// prt returns a reference to whatever type is passed into it
449-
func ptr[T any](x T) *T {
450-
return &x
451-
}
452-
453448
func setGetLdConfigPathForTest() func() {
454449
previous := getLdConfigPath
455450
getLdConfigPath = func() ldconfigPath {

0 commit comments

Comments
 (0)