Merge pull request #9982 from rayburgemeestre/merge-toml-configurations-for-plugins

Allow sections of Plugins to be merged, and not overwritten as entire sections.

cmd/containerd/server/config/config.go

@@ -412,10 +412,6 @@ func mergeConfig(to, from *Config) error {
 	}
 
 	// Replace entire sections instead of merging map's values.
-	for k, v := range from.Plugins {
-		to.Plugins[k] = v
-	}
-
 	for k, v := range from.StreamProcessors {
 		to.StreamProcessors[k] = v
 	}

cmd/containerd/server/config/config_test.go

@@ -17,12 +17,17 @@
 package config
 
 import (
+	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 	"sort"
+	"strings"
 	"testing"
 
+	"github.com/pelletier/go-toml/v2"
+
 	"github.com/stretchr/testify/assert"
 
 	"github.com/containerd/containerd/v2/version"
@@ -260,3 +265,165 @@ func TestDecodePluginInV1Config(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, true, pluginConfig["shim_debug"])
 }
+
+func TestMergingPluginsWithTwoCriDropInConfigs(t *testing.T) {
+	data1 := `
+[plugins."io.containerd.grpc.v1.cri".cni]
+    bin_dir = "/cm/local/apps/kubernetes/current/bin/cni"
+`
+	data2 := `
+[plugins."io.containerd.grpc.v1.cri".registry]
+    config_path = "/cm/local/apps/containerd/var/etc/certs.d"
+`
+	expected := `
+[cni]
+  bin_dir = '/cm/local/apps/kubernetes/current/bin/cni'
+
+[registry]
+  config_path = '/cm/local/apps/containerd/var/etc/certs.d'
+`
+
+	testMergeConfig(t, []string{data1, data2}, expected, "io.containerd.grpc.v1.cri")
+	testMergeConfig(t, []string{data2, data1}, expected, "io.containerd.grpc.v1.cri")
+}
+
+func TestMergingPluginsWithTwoCriCniDropInConfigs(t *testing.T) {
+	data1 := `
+[plugins."io.containerd.grpc.v1.cri".cni]
+    bin_dir = "/cm/local/apps/kubernetes/current/bin/cni"
+`
+	data2 := `
+[plugins."io.containerd.grpc.v1.cri".cni]
+    conf_dir = "/tmp"
+`
+	expected := `
+[cni]
+  bin_dir = '/cm/local/apps/kubernetes/current/bin/cni'
+  conf_dir = '/tmp'
+`
+	testMergeConfig(t, []string{data1, data2}, expected, "io.containerd.grpc.v1.cri")
+}
+
+func TestMergingPluginsWithTwoCriRuntimeDropInConfigs(t *testing.T) {
+	runcRuntime := `
+[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
+    runtime_type = "io.containerd.runc.v2"
+    [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
+      SystemdCgroup = true
+`
+	nvidiaRuntime := `
+[plugins]
+  [plugins."io.containerd.grpc.v1.cri"]
+    [plugins."io.containerd.grpc.v1.cri".containerd]
+      default_runtime_name = "nvidia"
+      [plugins."io.containerd.grpc.v1.cri".containerd.runtimes]
+        [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.nvidia]
+          privileged_without_host_devices = false
+          runtime_engine = ""
+          runtime_root = ""
+          runtime_type = "io.containerd.runc.v2"
+          [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.nvidia.options]
+            BinaryName = "/usr/bin/nvidia-container-runtime"
+            SystemdCgroup = true
+`
+	expected := `
+[containerd]
+  default_runtime_name = 'nvidia'
+
+  [containerd.runtimes]
+    [containerd.runtimes.nvidia]
+      privileged_without_host_devices = false
+      runtime_engine = ''
+      runtime_root = ''
+      runtime_type = 'io.containerd.runc.v2'
+
+      [containerd.runtimes.nvidia.options]
+        BinaryName = '/usr/bin/nvidia-container-runtime'
+        SystemdCgroup = true
+
+    [containerd.runtimes.runc]
+      runtime_type = 'io.containerd.runc.v2'
+
+      [containerd.runtimes.runc.options]
+        SystemdCgroup = true
+`
+	testMergeConfig(t, []string{runcRuntime, nvidiaRuntime}, expected, "io.containerd.grpc.v1.cri")
+
+	// Merging a third config that customizes only the default_runtime_name should result in mostly identical result
+	runcDefault := `
+    [plugins."io.containerd.grpc.v1.cri".containerd]
+      default_runtime_name = "runc"
+`
+	// This will then be the only difference in our expected TOML
+	expected2 := strings.Replace(expected, "default_runtime_name = 'nvidia'", "default_runtime_name = 'runc'", 1)
+
+	testMergeConfig(t, []string{runcRuntime, nvidiaRuntime, runcDefault}, expected2, "io.containerd.grpc.v1.cri")
+
+	// Mixing up the order will again result in 'nvidia' being the default runtime
+	testMergeConfig(t, []string{runcRuntime, runcDefault, nvidiaRuntime}, expected, "io.containerd.grpc.v1.cri")
+}
+
+func TestMergingPluginsWithTwoCriRuntimeWithPodAnnotationsDropInConfigs(t *testing.T) {
+	runc1 := `
+[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
+    runtime_type = "io.containerd.runc.v2"
+    cni_conf_dir = "/foo"
+    pod_annotations = ["a", "b", "c"]
+`
+	runc2 := `
+[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
+    runtime_type = "io.containerd.runc.v2"
+    cni_conf_dir = "/bar"
+    pod_annotations = ["d", "e", "f"]
+`
+	expected := `
+[containerd]
+  [containerd.runtimes]
+    [containerd.runtimes.runc]
+      cni_conf_dir = '/bar'
+      pod_annotations = ['d', 'e', 'f']
+      runtime_type = 'io.containerd.runc.v2'
+`
+	testMergeConfig(t, []string{runc1, runc2}, expected, "io.containerd.grpc.v1.cri")
+
+	// The other way around: runc1 over runc2
+	expected = `
+[containerd]
+  [containerd.runtimes]
+    [containerd.runtimes.runc]
+      cni_conf_dir = '/foo'
+      pod_annotations = ['a', 'b', 'c']
+      runtime_type = 'io.containerd.runc.v2'
+`
+	testMergeConfig(t, []string{runc2, runc1}, expected, "io.containerd.grpc.v1.cri")
+}
+
+func testMergeConfig(t *testing.T, inputs []string, expected string, comparePlugin string) {
+	tempDir := t.TempDir()
+	var result Config
+
+	for i, data := range inputs {
+		filename := fmt.Sprintf("data%d.toml", i+1)
+		filepath := filepath.Join(tempDir, filename)
+		err := os.WriteFile(filepath, []byte(data), 0600)
+		assert.NoError(t, err)
+
+		var tempOut Config
+		err = LoadConfig(context.Background(), filepath, &tempOut)
+		assert.NoError(t, err)
+
+		if i == 0 {
+			result = tempOut
+		} else {
+			err = mergeConfig(&result, &tempOut)
+			assert.NoError(t, err)
+		}
+	}
+
+	criPlugin := result.Plugins[comparePlugin]
+	var buf bytes.Buffer
+	if err := toml.NewEncoder(&buf).SetIndentTables(true).Encode(criPlugin); err != nil {
+		panic(err)
+	}
+	assert.Equal(t, strings.TrimLeft(expected, "\n"), buf.String())
+}