From f91791b4d1e61c873323a3039a3620126fe57af7 Mon Sep 17 00:00:00 2001
From: Evan Lezar <elezar@nvidia.com>
Date: Fri, 29 Nov 2024 16:26:51 +0100
Subject: [PATCH] Improve the implementation for UseLegacyConfig
Signed-off-by: Evan Lezar <elezar@nvidia.com>
---
pkg/config/engine/containerd/config_v1.go | 24 +++++++------
pkg/config/engine/containerd/containerd.go | 30 +++++++++-------
.../runtime/containerd/config_v1_test.go | 34 +++++++++----------
.../runtime/containerd/containerd.go | 7 ++--
4 files changed, 53 insertions(+), 42 deletions(-)
diff --git a/pkg/config/engine/containerd/config_v1.go b/pkg/config/engine/containerd/config_v1.go
index db6cf2dc..10b6d087 100644
--- a/pkg/config/engine/containerd/config_v1.go
+++ b/pkg/config/engine/containerd/config_v1.go
@@ -70,18 +70,20 @@ func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool) error
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "options", "BinaryName"}, path)
config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name, "options", "Runtime"}, path)
- if setAsDefault && c.UseDefaultRuntimeName {
- config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime_name"}, name)
- } else if setAsDefault {
- // Note: This is deprecated in containerd 1.4.0 and will be removed in 1.5.0
- if config.GetPath([]string{"plugins", "cri", "containerd", "default_runtime"}) == nil {
- config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_type"}, c.RuntimeType)
- config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_root"}, "")
- config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_engine"}, "")
- config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "privileged_without_host_devices"}, false)
+ if setAsDefault {
+ if !c.UseLegacyConfig {
+ config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime_name"}, name)
+ } else {
+ // Note: This is deprecated in containerd 1.4.0 and will be removed in 1.5.0
+ if config.GetPath([]string{"plugins", "cri", "containerd", "default_runtime"}) == nil {
+ config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_type"}, c.RuntimeType)
+ config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_root"}, "")
+ config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "runtime_engine"}, "")
+ config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "privileged_without_host_devices"}, false)
+ }
+ config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "BinaryName"}, path)
+ config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "Runtime"}, path)
}
- config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "BinaryName"}, path)
- config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "Runtime"}, path)
}
*c.Tree = config
diff --git a/pkg/config/engine/containerd/containerd.go b/pkg/config/engine/containerd/containerd.go
index a5b08810..128befb3 100644
--- a/pkg/config/engine/containerd/containerd.go
+++ b/pkg/config/engine/containerd/containerd.go
@@ -27,10 +27,15 @@ import (
// Config represents the containerd config
type Config struct {
*toml.Tree
- Logger logger.Interface
- RuntimeType string
- UseDefaultRuntimeName bool
- ContainerAnnotations []string
+ Logger logger.Interface
+ RuntimeType string
+ ContainerAnnotations []string
+ // UseLegacyConfig indicates whether a config file pre v1.3 should be generated.
+ // For version 1 config prior to containerd v1.4 the default runtime was
+ // specified in a containerd.runtimes.default_runtime section.
+ // This was deprecated in v1.4 in favour of containerd.default_runtime_name.
+ // Support for this section has been removed in v2.0.
+ UseLegacyConfig bool
}
var _ engine.Interface = (*Config)(nil)
@@ -73,14 +78,14 @@ func New(opts ...Option) (engine.Interface, error) {
}
cfg := &Config{
- Tree: tomlConfig,
- Logger: b.logger,
- RuntimeType: b.runtimeType,
- UseDefaultRuntimeName: b.useLegacyConfig,
- ContainerAnnotations: b.containerAnnotations,
+ Tree: tomlConfig,
+ Logger: b.logger,
+ RuntimeType: b.runtimeType,
+ ContainerAnnotations: b.containerAnnotations,
+ UseLegacyConfig: b.useLegacyConfig,
}
- version, err := cfg.parseVersion(b.useLegacyConfig)
+ version, err := cfg.parseVersion()
if err != nil {
return nil, fmt.Errorf("failed to parse config version: %v", err)
}
@@ -95,9 +100,10 @@ func New(opts ...Option) (engine.Interface, error) {
}
// parseVersion returns the version of the config
-func (c *Config) parseVersion(useLegacyConfig bool) (int, error) {
+func (c *Config) parseVersion() (int, error) {
defaultVersion := 2
- if useLegacyConfig {
+ // For legacy configs, we default to v1 configs.
+ if c.UseLegacyConfig {
defaultVersion = 1
}
diff --git a/tools/container/runtime/containerd/config_v1_test.go b/tools/container/runtime/containerd/config_v1_test.go
index 26b673e4..a3967833 100644
--- a/tools/container/runtime/containerd/config_v1_test.go
+++ b/tools/container/runtime/containerd/config_v1_test.go
@@ -92,10 +92,10 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) {
require.NoError(t, err, "%d: %v", i, tc)
v1 := &containerd.ConfigV1{
- Logger: logger,
- Tree: cfg,
- UseDefaultRuntimeName: !tc.legacyConfig,
- RuntimeType: runtimeType,
+ Logger: logger,
+ Tree: cfg,
+ UseLegacyConfig: tc.legacyConfig,
+ RuntimeType: runtimeType,
}
err = o.UpdateConfig(v1)
@@ -238,11 +238,11 @@ func TestUpdateV1Config(t *testing.T) {
require.NoError(t, err)
v1 := &containerd.ConfigV1{
- Logger: logger,
- Tree: cfg,
- UseDefaultRuntimeName: true,
- RuntimeType: runtimeType,
- ContainerAnnotations: []string{"cdi.k8s.io/*"},
+ Logger: logger,
+ Tree: cfg,
+ UseLegacyConfig: true,
+ RuntimeType: runtimeType,
+ ContainerAnnotations: []string{"cdi.k8s.io/*"},
}
err = o.UpdateConfig(v1)
@@ -397,11 +397,11 @@ func TestUpdateV1ConfigWithRuncPresent(t *testing.T) {
require.NoError(t, err)
v1 := &containerd.ConfigV1{
- Logger: logger,
- Tree: cfg,
- UseDefaultRuntimeName: true,
- RuntimeType: runtimeType,
- ContainerAnnotations: []string{"cdi.k8s.io/*"},
+ Logger: logger,
+ Tree: cfg,
+ UseLegacyConfig: true,
+ RuntimeType: runtimeType,
+ ContainerAnnotations: []string{"cdi.k8s.io/*"},
}
err = o.UpdateConfig(v1)
@@ -476,9 +476,9 @@ func TestRevertV1Config(t *testing.T) {
require.NoError(t, err, "%d: %v", i, tc)
v1 := &containerd.ConfigV1{
- Tree: cfg,
- UseDefaultRuntimeName: true,
- RuntimeType: runtimeType,
+ Tree: cfg,
+ UseLegacyConfig: true,
+ RuntimeType: runtimeType,
}
err = o.RevertConfig(v1)
diff --git a/tools/container/runtime/containerd/containerd.go b/tools/container/runtime/containerd/containerd.go
index a53a0655..31fd2fea 100644
--- a/tools/container/runtime/containerd/containerd.go
+++ b/tools/container/runtime/containerd/containerd.go
@@ -52,8 +52,11 @@ type Options struct {
func Flags(opts *Options) []cli.Flag {
flags := []cli.Flag{
&cli.BoolFlag{
- Name: "use-legacy-config",
- Usage: "Specify whether a legacy (pre v1.3) config should be used",
+ Name: "use-legacy-config",
+ Usage: "Specify whether a legacy (pre v1.3) config should be used. " +
+ "This ensures that a version 1 container config is created by default and that the " +
+ "containerd.runtimes.default_runtime config section is used to define the default " +
+ "runtime instead of container.default_runtime_name.",
Destination: &opts.useLegacyConfig,
EnvVars: []string{"CONTAINERD_USE_LEGACY_CONFIG"},
},