Skip to content

Commit 10bafd1

Browse files
tariq1890tariq1890
authored
Merge pull request #643 from elezar/refactor-toml-source
Refactor handling of TOML config files for runtimes
2 parents f126877 + bf2bdfd commit 10bafd1

File tree

24 files changed

+456
-500
lines changed

24 files changed

+456
-500
lines changed

‎cmd/nvidia-ctk/runtime/configure/configure.go‎

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package configure
1818

1919
import (
20-
"encoding/json"
2120
"fmt"
2221
"path/filepath"
2322

@@ -29,6 +28,7 @@ import (
2928
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/crio"
3029
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/docker"
3130
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/ocihook"
31+
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/toml"
3232
)
3333

3434
const (
@@ -156,13 +156,6 @@ func (m command) build() *cli.Command {
156156
Usage: "Enable CDI in the configured runtime",
157157
Destination: &config.cdi.enabled,
158158
},
159-
&cli.StringFlag{
160-
Name: "runtime-config-override",
161-
Destination: &config.runtimeConfigOverrideJSON,
162-
Usage: "specify additional runtime options as a JSON string. The paths are relative to the runtime config.",
163-
Value: "",
164-
EnvVars: []string{"RUNTIME_CONFIG_OVERRIDE"},
165-
},
166159
}
167160

168161
return &configure
@@ -234,11 +227,13 @@ func (m command) configureConfigFile(c *cli.Context, config *config) error {
234227
cfg, err = containerd.New(
235228
containerd.WithLogger(m.logger),
236229
containerd.WithPath(configFilePath),
230+
containerd.WithConfigSource(toml.FromFile(configFilePath)),
237231
)
238232
case "crio":
239233
cfg, err = crio.New(
240234
crio.WithLogger(m.logger),
241235
crio.WithPath(configFilePath),
236+
crio.WithConfigSource(toml.FromFile(configFilePath)),
242237
)
243238
case "docker":
244239
cfg, err = docker.New(
@@ -252,16 +247,10 @@ func (m command) configureConfigFile(c *cli.Context, config *config) error {
252247
return fmt.Errorf("unable to load config for runtime %v: %v", config.runtime, err)
253248
}
254249

255-
runtimeConfigOverride, err := config.runtimeConfigOverride()
256-
if err != nil {
257-
return fmt.Errorf("unable to parse config overrides: %w", err)
258-
}
259-
260250
err = cfg.AddRuntime(
261251
config.nvidiaRuntime.name,
262252
config.nvidiaRuntime.path,
263253
config.nvidiaRuntime.setAsDefault,
264-
runtimeConfigOverride,
265254
)
266255
if err != nil {
267256
return fmt.Errorf("unable to update config: %v", err)
@@ -314,20 +303,6 @@ func (c *config) getOuputConfigPath() string {
314303
return c.resolveConfigFilePath()
315304
}
316305

317-
// runtimeConfigOverride converts the specified runtimeConfigOverride JSON string to a map.
318-
func (o *config) runtimeConfigOverride() (map[string]interface{}, error) {
319-
if o.runtimeConfigOverrideJSON == "" {
320-
return nil, nil
321-
}
322-
323-
runtimeOptions := make(map[string]interface{})
324-
if err := json.Unmarshal([]byte(o.runtimeConfigOverrideJSON), &runtimeOptions); err != nil {
325-
return nil, fmt.Errorf("failed to read %v as JSON: %w", o.runtimeConfigOverrideJSON, err)
326-
}
327-
328-
return runtimeOptions, nil
329-
}
330-
331306
// configureOCIHook creates and configures the OCI hook for the NVIDIA runtime
332307
func (m *command) configureOCIHook(c *cli.Context, config *config) error {
333308
err := ocihook.CreateHook(config.hookFilePath, config.nvidiaRuntime.hookPath)

‎pkg/config/engine/api.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ package engine
1919
// Interface defines the API for a runtime config updater.
2020
type Interface interface {
2121
DefaultRuntime() string
22-
AddRuntime(string, string, bool, ...map[string]interface{}) error
22+
AddRuntime(string, string, bool) error
2323
Set(string, interface{})
2424
RemoveRuntime(string) error
2525
Save(string) (int64, error)

‎pkg/config/engine/containerd/config_v1.go‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type ConfigV1 Config
3030
var _ engine.Interface = (*ConfigV1)(nil)
3131

3232
// AddRuntime adds a runtime to the containerd config
33-
func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool, configOverrides ...map[string]interface{}) error {
33+
func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool) error {
3434
if c == nil || c.Tree == nil {
3535
return fmt.Errorf("config is nil")
3636
}
@@ -45,12 +45,14 @@ func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool, confi
4545
runtimeNamesForConfig = append(runtimeNamesForConfig, name)
4646
}
4747
for _, r := range runtimeNamesForConfig {
48-
if options, ok := config.GetPath([]string{"plugins", "cri", "containerd", "runtimes", r}).(*toml.Tree); ok {
49-
c.Logger.Debugf("using options from runtime %v: %v", r, options.String())
50-
options, _ = toml.Load(options.String())
51-
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name}, options)
52-
break
48+
options := config.GetSubtreeByPath([]string{"plugins", "cri", "containerd", "runtimes", r})
49+
if options == nil {
50+
continue
5351
}
52+
c.Logger.Debugf("using options from runtime %v: %v", r, options)
53+
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name}, options.Copy())
54+
break
55+
5456
}
5557

5658
if config.GetPath([]string{"plugins", "cri", "containerd", "runtimes", name}) == nil {
@@ -85,16 +87,6 @@ func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool, confi
8587
}
8688
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "BinaryName"}, path)
8789
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "Runtime"}, path)
88-
89-
defaultRuntimeSubtree := subtreeAtPath(config, "plugins", "cri", "containerd", "default_runtime")
90-
if err := defaultRuntimeSubtree.applyOverrides(configOverrides...); err != nil {
91-
return fmt.Errorf("failed to apply config overrides to default_runtime: %w", err)
92-
}
93-
}
94-
95-
runtimeSubtree := subtreeAtPath(config, "plugins", "cri", "containerd", "runtimes", name)
96-
if err := runtimeSubtree.applyOverrides(configOverrides...); err != nil {
97-
return fmt.Errorf("failed to apply config overrides: %w", err)
9890
}
9991

10092
*c.Tree = config

‎pkg/config/engine/containerd/config_v1_test.go‎

Lines changed: 11 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,20 @@ package containerd
1919
import (
2020
"testing"
2121

22-
"github.com/pelletier/go-toml"
2322
testlog "github.com/sirupsen/logrus/hooks/test"
2423
"github.com/stretchr/testify/require"
24+
25+
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/toml"
2526
)
2627

2728
func TestAddRuntimeV1(t *testing.T) {
2829
logger, _ := testlog.NewNullLogger()
2930
testCases := []struct {
30-
description string
31-
config string
32-
setAsDefault bool
33-
configOverrides []map[string]interface{}
34-
expectedConfig string
35-
expectedError error
31+
description string
32+
config string
33+
setAsDefault bool
34+
expectedConfig string
35+
expectedError error
3636
}{
3737
{
3838
description: "empty config not default runtime",
@@ -53,32 +53,6 @@ func TestAddRuntimeV1(t *testing.T) {
5353
`,
5454
expectedError: nil,
5555
},
56-
{
57-
description: "empty config not default runtime with overrides",
58-
configOverrides: []map[string]interface{}{
59-
{
60-
"options": map[string]interface{}{
61-
"SystemdCgroup": true,
62-
},
63-
},
64-
},
65-
expectedConfig: `
66-
version = 1
67-
[plugins]
68-
[plugins.cri]
69-
[plugins.cri.containerd]
70-
[plugins.cri.containerd.runtimes]
71-
[plugins.cri.containerd.runtimes.test]
72-
privileged_without_host_devices = false
73-
runtime_engine = ""
74-
runtime_root = ""
75-
runtime_type = ""
76-
[plugins.cri.containerd.runtimes.test.options]
77-
BinaryName = "/usr/bin/test"
78-
Runtime = "/usr/bin/test"
79-
SystemdCgroup = true
80-
`,
81-
},
8256
{
8357
description: "options from runc are imported",
8458
config: `
@@ -226,20 +200,20 @@ func TestAddRuntimeV1(t *testing.T) {
226200

227201
for _, tc := range testCases {
228202
t.Run(tc.description, func(t *testing.T) {
229-
config, err := toml.Load(tc.config)
203+
cfg, err := toml.Load(tc.config)
230204
require.NoError(t, err)
231205
expectedConfig, err := toml.Load(tc.expectedConfig)
232206
require.NoError(t, err)
233207

234208
c := &ConfigV1{
235209
Logger: logger,
236-
Tree: config,
210+
Tree: cfg,
237211
}
238212

239-
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault, tc.configOverrides...)
213+
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault)
240214
require.NoError(t, err)
241215

242-
require.EqualValues(t, expectedConfig.String(), config.String())
216+
require.EqualValues(t, expectedConfig.String(), cfg.String())
243217
})
244218
}
245219
}

‎pkg/config/engine/containerd/config_v2.go‎

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,11 @@ package containerd
1919
import (
2020
"fmt"
2121

22-
"github.com/pelletier/go-toml"
23-
24-
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine"
22+
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/toml"
2523
)
2624

2725
// AddRuntime adds a runtime to the containerd config
28-
func (c *Config) AddRuntime(name string, path string, setAsDefault bool, configOverrides ...map[string]interface{}) error {
26+
func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error {
2927
if c == nil || c.Tree == nil {
3028
return fmt.Errorf("config is nil")
3129
}
@@ -39,12 +37,13 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool, configO
3937
runtimeNamesForConfig = append(runtimeNamesForConfig, name)
4038
}
4139
for _, r := range runtimeNamesForConfig {
42-
if options, ok := config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", r}).(*toml.Tree); ok {
43-
c.Logger.Debugf("using options from runtime %v: %v", r, options.String())
44-
options, _ = toml.Load(options.String())
45-
config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}, options)
46-
break
40+
options := config.GetSubtreeByPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", r})
41+
if options == nil {
42+
continue
4743
}
44+
c.Logger.Debugf("using options from runtime %v: %v", r, options)
45+
config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}, options.Copy())
46+
break
4847
}
4948

5049
if config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}) == nil {
@@ -70,11 +69,6 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool, configO
7069
config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "default_runtime_name"}, name)
7170
}
7271

73-
runtimeSubtree := subtreeAtPath(config, "plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name)
74-
if err := runtimeSubtree.applyOverrides(configOverrides...); err != nil {
75-
return fmt.Errorf("failed to apply config overrides: %w", err)
76-
}
77-
7872
*c.Tree = config
7973
return nil
8074
}
@@ -151,15 +145,3 @@ func (c *Config) RemoveRuntime(name string) error {
151145
*c.Tree = config
152146
return nil
153147
}
154-
155-
// Save writes the config to the specified path
156-
func (c Config) Save(path string) (int64, error) {
157-
config := c.Tree
158-
output, err := config.Marshal()
159-
if err != nil {
160-
return 0, fmt.Errorf("unable to convert to TOML: %v", err)
161-
}
162-
163-
n, err := engine.Config(path).Write(output)
164-
return int64(n), err
165-
}

‎pkg/config/engine/containerd/config_v2_test.go‎

Lines changed: 11 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,20 @@ package containerd
1919
import (
2020
"testing"
2121

22-
"github.com/pelletier/go-toml"
2322
testlog "github.com/sirupsen/logrus/hooks/test"
2423
"github.com/stretchr/testify/require"
24+
25+
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/toml"
2526
)
2627

2728
func TestAddRuntime(t *testing.T) {
2829
logger, _ := testlog.NewNullLogger()
2930
testCases := []struct {
30-
description string
31-
config string
32-
setAsDefault bool
33-
configOverrides []map[string]interface{}
34-
expectedConfig string
35-
expectedError error
31+
description string
32+
config string
33+
setAsDefault bool
34+
expectedConfig string
35+
expectedError error
3636
}{
3737
{
3838
description: "empty config not default runtime",
@@ -52,31 +52,6 @@ func TestAddRuntime(t *testing.T) {
5252
`,
5353
expectedError: nil,
5454
},
55-
{
56-
description: "empty config not default runtime with overrides",
57-
configOverrides: []map[string]interface{}{
58-
{
59-
"options": map[string]interface{}{
60-
"SystemdCgroup": true,
61-
},
62-
},
63-
},
64-
expectedConfig: `
65-
version = 2
66-
[plugins]
67-
[plugins."io.containerd.grpc.v1.cri"]
68-
[plugins."io.containerd.grpc.v1.cri".containerd]
69-
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes]
70-
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test]
71-
privileged_without_host_devices = false
72-
runtime_engine = ""
73-
runtime_root = ""
74-
runtime_type = ""
75-
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test.options]
76-
BinaryName = "/usr/bin/test"
77-
SystemdCgroup = true
78-
`,
79-
},
8055
{
8156
description: "options from runc are imported",
8257
config: `
@@ -224,20 +199,20 @@ func TestAddRuntime(t *testing.T) {
224199

225200
for _, tc := range testCases {
226201
t.Run(tc.description, func(t *testing.T) {
227-
config, err := toml.Load(tc.config)
202+
cfg, err := toml.Load(tc.config)
228203
require.NoError(t, err)
229204
expectedConfig, err := toml.Load(tc.expectedConfig)
230205
require.NoError(t, err)
231206

232207
c := &Config{
233208
Logger: logger,
234-
Tree: config,
209+
Tree: cfg,
235210
}
236211

237-
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault, tc.configOverrides...)
212+
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault)
238213
require.NoError(t, err)
239214

240-
require.EqualValues(t, expectedConfig.String(), config.String())
215+
require.EqualValues(t, expectedConfig.String(), cfg.String())
241216
})
242217
}
243218
}

0 commit comments

Comments
 (0)