Skip to content

Commit bf2bdfd

Browse files
elezarelezar
committed
Refactor Toml config handling
This change refactors the toml config file handlig for runtimes such as containerd or crio. A toml.Loader is introduced that encapsulates loading the required file. This can be extended to allow other mechanisms for loading loading the current config. Signed-off-by: Evan Lezar <[email protected]>
1 parent 6c5f4ee commit bf2bdfd

File tree

20 files changed

+428
-229
lines changed

20 files changed

+428
-229
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/crio"
2929
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/docker"
3030
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/ocihook"
31+
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/toml"
3132
)
3233

3334
const (
@@ -226,11 +227,13 @@ func (m command) configureConfigFile(c *cli.Context, config *config) error {
226227
cfg, err = containerd.New(
227228
containerd.WithLogger(m.logger),
228229
containerd.WithPath(configFilePath),
230+
containerd.WithConfigSource(toml.FromFile(configFilePath)),
229231
)
230232
case "crio":
231233
cfg, err = crio.New(
232234
crio.WithLogger(m.logger),
233235
crio.WithPath(configFilePath),
236+
crio.WithConfigSource(toml.FromFile(configFilePath)),
234237
)
235238
case "docker":
236239
cfg, err = docker.New(

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,14 @@ func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool) error
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 {

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ 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) {
@@ -199,20 +200,20 @@ func TestAddRuntimeV1(t *testing.T) {
199200

200201
for _, tc := range testCases {
201202
t.Run(tc.description, func(t *testing.T) {
202-
config, err := toml.Load(tc.config)
203+
cfg, err := toml.Load(tc.config)
203204
require.NoError(t, err)
204205
expectedConfig, err := toml.Load(tc.expectedConfig)
205206
require.NoError(t, err)
206207

207208
c := &ConfigV1{
208209
Logger: logger,
209-
Tree: config,
210+
Tree: cfg,
210211
}
211212

212213
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault)
213214
require.NoError(t, err)
214215

215-
require.EqualValues(t, expectedConfig.String(), config.String())
216+
require.EqualValues(t, expectedConfig.String(), cfg.String())
216217
})
217218
}
218219
}

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

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@ 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
@@ -39,12 +37,13 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error {
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 {
@@ -146,15 +145,3 @@ func (c *Config) RemoveRuntime(name string) error {
146145
*c.Tree = config
147146
return nil
148147
}
149-
150-
// Save writes the config to the specified path
151-
func (c Config) Save(path string) (int64, error) {
152-
config := c.Tree
153-
output, err := config.Marshal()
154-
if err != nil {
155-
return 0, fmt.Errorf("unable to convert to TOML: %v", err)
156-
}
157-
158-
n, err := engine.Config(path).Write(output)
159-
return int64(n), err
160-
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ 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) {
@@ -198,20 +199,20 @@ func TestAddRuntime(t *testing.T) {
198199

199200
for _, tc := range testCases {
200201
t.Run(tc.description, func(t *testing.T) {
201-
config, err := toml.Load(tc.config)
202+
cfg, err := toml.Load(tc.config)
202203
require.NoError(t, err)
203204
expectedConfig, err := toml.Load(tc.expectedConfig)
204205
require.NoError(t, err)
205206

206207
c := &Config{
207208
Logger: logger,
208-
Tree: config,
209+
Tree: cfg,
209210
}
210211

211212
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault)
212213
require.NoError(t, err)
213214

214-
require.EqualValues(t, expectedConfig.String(), config.String())
215+
require.EqualValues(t, expectedConfig.String(), cfg.String())
215216
})
216217
}
217218
}

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

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@
1717
package containerd
1818

1919
import (
20-
"github.com/pelletier/go-toml"
20+
"fmt"
2121

2222
"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
2323
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine"
24+
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/toml"
2425
)
2526

2627
// Config represents the containerd config
@@ -36,14 +37,64 @@ var _ engine.Interface = (*Config)(nil)
3637

3738
// New creates a containerd config with the specified options
3839
func New(opts ...Option) (engine.Interface, error) {
39-
b := &builder{}
40+
b := &builder{
41+
runtimeType: defaultRuntimeType,
42+
}
4043
for _, opt := range opts {
4144
opt(b)
4245
}
43-
4446
if b.logger == nil {
4547
b.logger = logger.New()
4648
}
49+
if b.configSource == nil {
50+
b.configSource = toml.FromFile(b.path)
51+
}
52+
53+
tomlConfig, err := b.configSource.Load()
54+
if err != nil {
55+
return nil, fmt.Errorf("failed to load config: %v", err)
56+
}
57+
58+
cfg := &Config{
59+
Tree: tomlConfig,
60+
Logger: b.logger,
61+
RuntimeType: b.runtimeType,
62+
UseDefaultRuntimeName: b.useLegacyConfig,
63+
ContainerAnnotations: b.containerAnnotations,
64+
}
4765

48-
return b.build()
66+
version, err := cfg.parseVersion(b.useLegacyConfig)
67+
if err != nil {
68+
return nil, fmt.Errorf("failed to parse config version: %v", err)
69+
}
70+
switch version {
71+
case 1:
72+
return (*ConfigV1)(cfg), nil
73+
case 2:
74+
return cfg, nil
75+
}
76+
77+
return nil, fmt.Errorf("unsupported config version: %v", version)
78+
}
79+
80+
// parseVersion returns the version of the config
81+
func (c *Config) parseVersion(useLegacyConfig bool) (int, error) {
82+
defaultVersion := 2
83+
if useLegacyConfig {
84+
defaultVersion = 1
85+
}
86+
87+
switch v := c.Get("version").(type) {
88+
case nil:
89+
switch len(c.Keys()) {
90+
case 0: // No config exists, or the config file is empty, use version inferred from containerd
91+
return defaultVersion, nil
92+
default: // A config file exists, has content, and no version is set
93+
return 1, nil
94+
}
95+
case int64:
96+
return int(v), nil
97+
default:
98+
return -1, fmt.Errorf("unsupported type for version field: %v", v)
99+
}
49100
}

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

Lines changed: 9 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,8 @@
1717
package containerd
1818

1919
import (
20-
"fmt"
21-
"os"
22-
23-
"github.com/pelletier/go-toml"
24-
2520
"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
26-
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine"
21+
"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/toml"
2722
)
2823

2924
const (
@@ -32,6 +27,7 @@ const (
3227

3328
type builder struct {
3429
logger logger.Interface
30+
configSource toml.Loader
3531
path string
3632
runtimeType string
3733
useLegacyConfig bool
@@ -55,6 +51,13 @@ func WithPath(path string) Option {
5551
}
5652
}
5753

54+
// WithConfigSource sets the source for the config.
55+
func WithConfigSource(configSource toml.Loader) Option {
56+
return func(b *builder) {
57+
b.configSource = configSource
58+
}
59+
}
60+
5861
// WithRuntimeType sets the runtime type for the config builder
5962
func WithRuntimeType(runtimeType string) Option {
6063
return func(b *builder) {
@@ -75,82 +78,3 @@ func WithContainerAnnotations(containerAnnotations ...string) Option {
7578
b.containerAnnotations = containerAnnotations
7679
}
7780
}
78-
79-
func (b *builder) build() (engine.Interface, error) {
80-
if b.path == "" {
81-
return nil, fmt.Errorf("config path is empty")
82-
}
83-
84-
if b.runtimeType == "" {
85-
b.runtimeType = defaultRuntimeType
86-
}
87-
88-
config, err := b.loadConfig(b.path)
89-
if err != nil {
90-
return nil, fmt.Errorf("failed to load config: %v", err)
91-
}
92-
config.Logger = b.logger
93-
config.RuntimeType = b.runtimeType
94-
config.UseDefaultRuntimeName = !b.useLegacyConfig
95-
config.ContainerAnnotations = b.containerAnnotations
96-
97-
version, err := config.parseVersion(b.useLegacyConfig)
98-
if err != nil {
99-
return nil, fmt.Errorf("failed to parse config version: %v", err)
100-
}
101-
switch version {
102-
case 1:
103-
return (*ConfigV1)(config), nil
104-
case 2:
105-
return config, nil
106-
}
107-
108-
return nil, fmt.Errorf("unsupported config version: %v", version)
109-
}
110-
111-
// loadConfig loads the containerd config from disk
112-
func (b *builder) loadConfig(config string) (*Config, error) {
113-
info, err := os.Stat(config)
114-
if os.IsExist(err) && info.IsDir() {
115-
return nil, fmt.Errorf("config file is a directory")
116-
}
117-
118-
if os.IsNotExist(err) {
119-
b.logger.Infof("Config file does not exist; using empty config")
120-
config = "/dev/null"
121-
} else {
122-
b.logger.Infof("Loading config from %v", config)
123-
}
124-
125-
tomlConfig, err := toml.LoadFile(config)
126-
if err != nil {
127-
return nil, err
128-
}
129-
130-
cfg := Config{
131-
Tree: tomlConfig,
132-
}
133-
return &cfg, nil
134-
}
135-
136-
// parseVersion returns the version of the config
137-
func (c *Config) parseVersion(useLegacyConfig bool) (int, error) {
138-
defaultVersion := 2
139-
if useLegacyConfig {
140-
defaultVersion = 1
141-
}
142-
143-
switch v := c.Get("version").(type) {
144-
case nil:
145-
switch len(c.Keys()) {
146-
case 0: // No config exists, or the config file is empty, use version inferred from containerd
147-
return defaultVersion, nil
148-
default: // A config file exists, has content, and no version is set
149-
return 1, nil
150-
}
151-
case int64:
152-
return int(v), nil
153-
default:
154-
return -1, fmt.Errorf("unsupported type for version field: %v", v)
155-
}
156-
}

0 commit comments

Comments
 (0)