Skip to content

Commit 537f53b

Browse files
elezarelezar
authored
Merge pull request #1279 from elezar/clean-runtime-config-api
[no-relnote] Refactor AddRuntime
2 parents 2e1d8f8 + 5d37aba commit 537f53b

File tree

4 files changed

+78
-86
lines changed

4 files changed

+78
-86
lines changed

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

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,29 +28,37 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error {
2828
if c == nil || c.Tree == nil {
2929
return fmt.Errorf("config is nil")
3030
}
31-
config := *c.Tree
32-
33-
config.Set("version", c.Version)
31+
defaultRuntimeOptions := c.GetDefaultRuntimeOptions()
32+
return c.AddRuntimeWithOptions(name, path, setAsDefault, defaultRuntimeOptions)
33+
}
3434

35+
func (c *Config) GetDefaultRuntimeOptions() interface{} {
3536
runtimeNamesForConfig := engine.GetLowLevelRuntimes(c)
3637
for _, r := range runtimeNamesForConfig {
37-
options := config.GetSubtreeByPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", r})
38-
if options == nil {
39-
continue
38+
options := c.GetSubtreeByPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", r})
39+
if options != nil {
40+
c.Logger.Debugf("Using options from runtime %v: %v", r, options)
41+
return options.Copy()
4042
}
41-
c.Logger.Debugf("using options from runtime %v: %v", r, options)
42-
config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name}, options.Copy())
43-
break
4443
}
44+
c.Logger.Warningf("Could not infer options from runtimes %v", runtimeNamesForConfig)
45+
options, _ := toml.TreeFromMap(map[string]interface{}{
46+
"runtime_type": c.RuntimeType,
47+
"runtime_root": "",
48+
"runtime_engine": "",
49+
"privileged_without_host_devices": false,
50+
})
51+
return options
52+
}
4553

46-
if config.GetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name}) == nil {
47-
c.Logger.Warningf("could not infer options from runtimes %v; using defaults", runtimeNamesForConfig)
48-
config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "runtime_type"}, c.RuntimeType)
49-
config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "runtime_root"}, "")
50-
config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "runtime_engine"}, "")
51-
config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "privileged_without_host_devices"}, false)
52-
}
54+
func (c *Config) AddRuntimeWithOptions(name string, path string, setAsDefault bool, options interface{}) error {
55+
config := *c.Tree
56+
57+
config.Set("version", c.Version)
5358

59+
if options != nil {
60+
config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name}, options)
61+
}
5462
if len(c.ContainerAnnotations) > 0 {
5563
annotations, err := c.getRuntimeAnnotations([]string{"plugins", c.CRIRuntimePluginName, "containerd", "runtimes", name, "container_annotations"})
5664
if err != nil {
@@ -70,7 +78,6 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error {
7078
config.DeletePath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "default_runtime_name"})
7179
}
7280
}
73-
7481
*c.Tree = config
7582
return nil
7683
}

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

Lines changed: 23 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -33,59 +33,36 @@ func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool) error
3333
if c == nil || c.Tree == nil {
3434
return fmt.Errorf("config is nil")
3535
}
36+
defaultRuntimeOptions := c.GetDefaultRuntimeOptions()
37+
return c.AddRuntimeWithOptions(name, path, setAsDefault, defaultRuntimeOptions)
38+
}
3639

37-
config := *c.Tree
38-
39-
config.Set("version", int64(1))
40-
41-
runtimeNamesForConfig := engine.GetLowLevelRuntimes(c)
42-
for _, r := range runtimeNamesForConfig {
43-
options := config.GetSubtreeByPath([]string{"plugins", "cri", "containerd", "runtimes", r})
44-
if options == nil {
45-
continue
46-
}
47-
c.Logger.Debugf("using options from runtime %v: %v", r, options)
48-
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name}, options.Copy())
49-
break
50-
51-
}
52-
53-
if config.GetPath([]string{"plugins", "cri", "containerd", "runtimes", name}) == nil {
54-
c.Logger.Warningf("could not infer options from runtimes %v; using defaults", runtimeNamesForConfig)
55-
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "runtime_type"}, c.RuntimeType)
56-
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "runtime_root"}, "")
57-
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "runtime_engine"}, "")
58-
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "privileged_without_host_devices"}, false)
59-
}
40+
func (c *ConfigV1) GetDefaultRuntimeOptions() interface{} {
41+
return (*Config)(c).GetDefaultRuntimeOptions()
42+
}
6043

61-
if len(c.ContainerAnnotations) > 0 {
62-
annotations, err := (*Config)(c).getRuntimeAnnotations([]string{"plugins", "cri", "containerd", "runtimes", name, "container_annotations"})
63-
if err != nil {
64-
return err
65-
}
66-
annotations = append(c.ContainerAnnotations, annotations...)
67-
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "container_annotations"}, annotations)
44+
func (c *ConfigV1) AddRuntimeWithOptions(name string, path string, setAsDefault bool, options interface{}) error {
45+
if err := (*Config)(c).AddRuntimeWithOptions(name, path, setAsDefault && !c.UseLegacyConfig, options); err != nil {
46+
return err
6847
}
69-
70-
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "options", "BinaryName"}, path)
48+
config := *c.Tree
7149
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "options", "Runtime"}, path)
50+
*c.Tree = config
7251

73-
if setAsDefault {
74-
if !c.UseLegacyConfig {
75-
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime_name"}, name)
76-
} else {
77-
// Note: This is deprecated in containerd 1.4.0 and will be removed in 1.5.0
78-
if config.GetPath([]string{"plugins", "cri", "containerd", "default_runtime"}) == nil {
79-
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_type"}, c.RuntimeType)
80-
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_root"}, "")
81-
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_engine"}, "")
82-
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "privileged_without_host_devices"}, false)
83-
}
84-
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "BinaryName"}, path)
85-
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "Runtime"}, path)
86-
}
52+
if !c.UseLegacyConfig || !setAsDefault {
53+
return nil
8754
}
8855

56+
config = *c.Tree
57+
// Note: This is deprecated in containerd 1.4.0 and will be removed in 1.5.0
58+
if config.GetPath([]string{"plugins", "cri", "containerd", "default_runtime"}) == nil {
59+
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_type"}, c.RuntimeType)
60+
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_root"}, "")
61+
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_engine"}, "")
62+
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "privileged_without_host_devices"}, false)
63+
}
64+
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "BinaryName"}, path)
65+
config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "Runtime"}, path)
8966
*c.Tree = config
9067
return nil
9168
}

‎pkg/config/engine/crio/crio.go‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,24 +74,36 @@ func New(opts ...Option) (engine.Interface, error) {
7474
return &cfg, nil
7575
}
7676

77-
// AddRuntime adds a new runtime to the crio config
77+
// AddRuntime adds a new runtime to the crio config.
78+
// The runtime options are extracted from the default runtime and the applicable
79+
// settings are overridden.
7880
func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error {
79-
if c == nil {
81+
if c == nil || c.Tree == nil {
8082
return fmt.Errorf("config is nil")
8183
}
84+
defaultRuntimeOptions := c.GetDefaultRuntimeOptions()
85+
return c.AddRuntimeWithOptions(name, path, setAsDefault, defaultRuntimeOptions)
86+
}
8287

83-
config := *c.Tree
84-
88+
func (c *Config) GetDefaultRuntimeOptions() interface{} {
8589
runtimeNamesForConfig := engine.GetLowLevelRuntimes(c)
8690
for _, r := range runtimeNamesForConfig {
87-
if options, ok := config.GetPath([]string{"crio", "runtime", "runtimes", r}).(*toml.Tree); ok {
88-
c.Logger.Debugf("using options from runtime %v: %v", r, options.String())
89-
options, _ = toml.Load(options.String())
90-
config.SetPath([]string{"crio", "runtime", "runtimes", name}, options)
91-
break
91+
options := c.GetSubtreeByPath([]string{"crio", "runtime", "runtimes", r})
92+
if options != nil {
93+
c.Logger.Debugf("Using options from runtime %v: %v", r, options)
94+
return options.Copy()
9295
}
9396
}
97+
c.Logger.Warningf("Could not infer options from runtimes %v", runtimeNamesForConfig)
98+
return nil
99+
}
100+
101+
func (c *Config) AddRuntimeWithOptions(name string, path string, setAsDefault bool, options interface{}) error {
102+
config := *c.Tree
94103

104+
if options != nil {
105+
config.SetPath([]string{"crio", "runtime", "runtimes", name}, options)
106+
}
95107
config.SetPath([]string{"crio", "runtime", "runtimes", name, "runtime_path"}, path)
96108
config.SetPath([]string{"crio", "runtime", "runtimes", name, "runtime_type"}, "oci")
97109

@@ -104,7 +116,6 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error {
104116
}
105117
}
106118
}
107-
108119
*c.Tree = config
109120
return nil
110121
}

‎pkg/config/engine/crio/crio_test.go‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -181,20 +181,19 @@ func TestAddRuntime(t *testing.T) {
181181

182182
for _, tc := range testCases {
183183
t.Run(tc.description, func(t *testing.T) {
184-
cfg, err := toml.Load(tc.config)
185-
require.NoError(t, err)
186184
expectedConfig, err := toml.Load(tc.expectedConfig)
187185
require.NoError(t, err)
188186

189-
c := &Config{
190-
Logger: logger,
191-
Tree: cfg,
192-
}
187+
c, err := New(
188+
WithLogger(logger),
189+
WithConfigSource(toml.FromString(tc.config)),
190+
)
191+
require.NoError(t, err)
193192

194193
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault)
195194
require.NoError(t, err)
196195

197-
require.EqualValues(t, expectedConfig.String(), cfg.String())
196+
require.EqualValues(t, expectedConfig.String(), c.String())
198197
})
199198
}
200199
}
@@ -242,14 +241,12 @@ monitor_path = "/usr/libexec/crio/conmon"
242241
}
243242
for _, tc := range testCases {
244243
t.Run(tc.description, func(t *testing.T) {
245-
cfg, err := toml.Load(config)
244+
c, err := New(
245+
WithLogger(logger),
246+
WithConfigSource(toml.FromString(config)),
247+
)
246248
require.NoError(t, err)
247249

248-
c := &Config{
249-
Logger: logger,
250-
Tree: cfg,
251-
}
252-
253250
rc, err := c.GetRuntimeConfig(tc.runtime)
254251
require.Equal(t, tc.expectedError, err)
255252
require.Equal(t, tc.expected, rc.GetBinaryPath())

0 commit comments

Comments
 (0)