Merge pull request #11140 from k8s-infra-cherrypick-robot/cherry-pick-11061-to-release/2.0

[release/2.0] Fix cri grpc plugin config migration

integration/client/migration_test.go

@@ -18,9 +18,11 @@ package client
 
 import (
 	"bytes"
+	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"runtime"
 	"strings"
 	"testing"
@@ -62,6 +64,17 @@ func TestMigration(t *testing.T) {
 				File:     "testdata/default-1.7.toml",
 				Migrated: defaultContent,
 			},
+			{
+				Name: "1.7-Custom",
+				File: "testdata/custom-1.7.toml",
+				Migrated: replaceAllValues(defaultContent, map[string]string{
+					"sandbox":               "'custom.io/pause:3.10'",
+					"stream_idle_timeout":   "'2h0m0s'",
+					"stream_server_address": "'127.0.1.1'",
+					"stream_server_port":    "'15000'",
+					"enable_tls_streaming":  "true",
+				}),
+			},
 		}...)
 	}
 
@@ -74,7 +87,7 @@ func TestMigration(t *testing.T) {
 			cmd.Stderr = os.Stderr
 			require.NoError(t, cmd.Run())
 			actual := buf.String()
-			assert.Equal(t, tc.Migrated, actual)
+			assert.Equal(t, tc.Migrated, actual, "Actual (full)\n%s", tc.Migrated, actual)
 		})
 	}
 
@@ -90,3 +103,15 @@ func currentDefaultConfig() (string, error) {
 	}
 	return buf.String(), nil
 }
+
+func replaceAllValues(src string, values map[string]string) string {
+	for k, v := range values {
+		src = replaceValue(src, k, v)
+	}
+	return src
+}
+
+func replaceValue(src, key, value string) string {
+	re := regexp.MustCompile(fmt.Sprintf(`(\s*)%s = [^\n]*`, key))
+	return re.ReplaceAllString(src, fmt.Sprintf(`${1}%s = %s`, key, value))
+}

integration/client/testdata/custom-1.7.toml

@@ -0,0 +1,308 @@
+disabled_plugins = []
+imports = []
+oom_score = 0
+plugin_dir = ""
+required_plugins = []
+root = "/var/lib/containerd"
+state = "/run/containerd"
+temp = ""
+version = 2
+
+[cgroup]
+  path = ""
+
+[debug]
+  address = ""
+  format = ""
+  gid = 0
+  level = ""
+  uid = 0
+
+[grpc]
+  address = "/run/containerd/containerd.sock"
+  gid = 0
+  max_recv_message_size = 16777216
+  max_send_message_size = 16777216
+  tcp_address = ""
+  tcp_tls_ca = ""
+  tcp_tls_cert = ""
+  tcp_tls_key = ""
+  uid = 0
+
+[metrics]
+  address = ""
+  grpc_histogram = false
+
+[plugins]
+
+  [plugins."io.containerd.gc.v1.scheduler"]
+    deletion_threshold = 0
+    mutation_threshold = 100
+    pause_threshold = 0.02
+    schedule_delay = "0s"
+    startup_delay = "100ms"
+
+  [plugins."io.containerd.grpc.v1.cri"]
+    cdi_spec_dirs = ["/etc/cdi", "/var/run/cdi"]
+    device_ownership_from_security_context = false
+    disable_apparmor = false
+    disable_cgroup = false
+    disable_hugetlb_controller = true
+    disable_proc_mount = false
+    disable_tcp_service = true
+    drain_exec_sync_io_timeout = "0s"
+    # Updated in latest release
+    #enable_cdi = false
+    enable_cdi = true
+    enable_selinux = false
+    enable_tls_streaming = true
+    # Updated in latest release
+    #enable_unprivileged_icmp = false
+    #enable_unprivileged_ports = false
+    enable_unprivileged_icmp = true
+    enable_unprivileged_ports = true
+    ignore_image_defined_volumes = false
+    image_pull_progress_timeout = "5m0s"
+    max_concurrent_downloads = 3
+    max_container_log_line_size = 16384
+    netns_mounts_under_state_dir = false
+    restrict_oom_score_adj = false
+    sandbox_image = "custom.io/pause:3.10"
+    selinux_category_range = 1024
+    stats_collect_period = 10
+    stream_idle_timeout = "2h0m0s"
+    stream_server_address = "127.0.1.1"
+    stream_server_port = "15000"
+    systemd_cgroup = false
+    tolerate_missing_hugetlb_controller = true
+    unset_seccomp_profile = ""
+
+    [plugins."io.containerd.grpc.v1.cri".cni]
+      bin_dir = "/opt/cni/bin"
+      conf_dir = "/etc/cni/net.d"
+      conf_template = ""
+      ip_pref = ""
+      max_conf_num = 1
+      setup_serially = false
+
+    [plugins."io.containerd.grpc.v1.cri".containerd]
+      default_runtime_name = "runc"
+      disable_snapshot_annotations = true
+      discard_unpacked_layers = false
+      ignore_blockio_not_enabled_errors = false
+      ignore_rdt_not_enabled_errors = false
+      no_pivot = false
+      snapshotter = "overlayfs"
+
+      [plugins."io.containerd.grpc.v1.cri".containerd.default_runtime]
+        base_runtime_spec = ""
+        cni_conf_dir = ""
+        cni_max_conf_num = 0
+        container_annotations = []
+        pod_annotations = []
+        privileged_without_host_devices = false
+        privileged_without_host_devices_all_devices_allowed = false
+        runtime_engine = ""
+        runtime_path = ""
+        runtime_root = ""
+        runtime_type = ""
+        sandbox_mode = ""
+        snapshotter = ""
+
+        [plugins."io.containerd.grpc.v1.cri".containerd.default_runtime.options]
+
+      [plugins."io.containerd.grpc.v1.cri".containerd.runtimes]
+
+        [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
+          base_runtime_spec = ""
+          cni_conf_dir = ""
+          cni_max_conf_num = 0
+          container_annotations = []
+          pod_annotations = []
+          privileged_without_host_devices = false
+          privileged_without_host_devices_all_devices_allowed = false
+          runtime_engine = ""
+          runtime_path = ""
+          runtime_root = ""
+          runtime_type = "io.containerd.runc.v2"
+          sandbox_mode = "podsandbox"
+          snapshotter = ""
+
+          [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
+            BinaryName = ""
+            CriuImagePath = ""
+            CriuWorkPath = ""
+            IoGid = 0
+            IoUid = 0
+            NoNewKeyring = false
+            Root = ""
+            ShimCgroup = ""
+            # Removed in latest
+            #CriuPath = ""
+            #NoPivotRoot = false
+            #SystemdCgroup = false
+
+      [plugins."io.containerd.grpc.v1.cri".containerd.untrusted_workload_runtime]
+        base_runtime_spec = ""
+        cni_conf_dir = ""
+        cni_max_conf_num = 0
+        container_annotations = []
+        pod_annotations = []
+        privileged_without_host_devices = false
+        privileged_without_host_devices_all_devices_allowed = false
+        runtime_engine = ""
+        runtime_path = ""
+        runtime_root = ""
+        runtime_type = ""
+        sandbox_mode = ""
+        snapshotter = ""
+
+        [plugins."io.containerd.grpc.v1.cri".containerd.untrusted_workload_runtime.options]
+
+    [plugins."io.containerd.grpc.v1.cri".image_decryption]
+      key_model = "node"
+
+    [plugins."io.containerd.grpc.v1.cri".registry]
+      config_path = ""
+
+      [plugins."io.containerd.grpc.v1.cri".registry.auths]
+
+      [plugins."io.containerd.grpc.v1.cri".registry.configs]
+
+      [plugins."io.containerd.grpc.v1.cri".registry.headers]
+
+      [plugins."io.containerd.grpc.v1.cri".registry.mirrors]
+
+    [plugins."io.containerd.grpc.v1.cri".x509_key_pair_streaming]
+      tls_cert_file = ""
+      tls_key_file = ""
+
+  [plugins."io.containerd.internal.v1.opt"]
+    path = "/opt/containerd"
+
+  [plugins."io.containerd.internal.v1.restart"]
+    interval = "10s"
+
+  # Removed in latest
+  #[plugins."io.containerd.internal.v1.tracing"]
+  #  sampling_ratio = 1.0
+  #  service_name = "containerd"
+
+  [plugins."io.containerd.metadata.v1.bolt"]
+    content_sharing_policy = "shared"
+
+  [plugins."io.containerd.monitor.v1.cgroups"]
+    no_prometheus = false
+
+  [plugins."io.containerd.nri.v1.nri"]
+    # Updated in latest
+    #disable = true
+    disable = false
+    disable_connections = false
+    plugin_config_path = "/etc/nri/conf.d"
+    plugin_path = "/opt/nri/plugins"
+    plugin_registration_timeout = "5s"
+    plugin_request_timeout = "2s"
+    socket_path = "/var/run/nri/nri.sock"
+
+  # Removed in latest
+  #[plugins."io.containerd.runtime.v1.linux"]
+  #  no_shim = false
+  #  runtime = "runc"
+  #  runtime_root = ""
+  #  shim = "containerd-shim"
+  #  shim_debug = false
+
+  [plugins."io.containerd.runtime.v2.task"]
+    platforms = ["linux/amd64"]
+    sched_core = false
+
+  [plugins."io.containerd.service.v1.diff-service"]
+    default = ["walking"]
+
+  [plugins."io.containerd.service.v1.tasks-service"]
+    blockio_config_file = ""
+    rdt_config_file = ""
+
+  # Removed in latest
+  #[plugins."io.containerd.snapshotter.v1.aufs"]
+  #  root_path = ""
+
+  [plugins."io.containerd.snapshotter.v1.blockfile"]
+    fs_type = ""
+    mount_options = []
+    root_path = ""
+    scratch_file = ""
+
+  [plugins."io.containerd.snapshotter.v1.btrfs"]
+    root_path = ""
+
+  [plugins."io.containerd.snapshotter.v1.devmapper"]
+    async_remove = false
+    base_image_size = ""
+    discard_blocks = false
+    fs_options = ""
+    fs_type = ""
+    pool_name = ""
+    root_path = ""
+
+  [plugins."io.containerd.snapshotter.v1.native"]
+    root_path = ""
+
+  [plugins."io.containerd.snapshotter.v1.overlayfs"]
+    mount_options = []
+    root_path = ""
+    sync_remove = false
+    upperdir_label = false
+
+  # Removed in latest
+  #[plugins."io.containerd.snapshotter.v1.zfs"]
+  #  root_path = ""
+
+  # Removed in latest
+  #[plugins."io.containerd.tracing.processor.v1.otlp"]
+  #  endpoint = ""
+  #  insecure = false
+  #  protocol = ""
+
+  [plugins."io.containerd.transfer.v1.local"]
+    config_path = ""
+    max_concurrent_downloads = 3
+    max_concurrent_uploaded_layers = 3
+
+    # In 2.0, the default unpack config is applied when not specified
+    #[[plugins."io.containerd.transfer.v1.local".unpack_config]]
+    #  differ = ""
+    #  platform = "linux/amd64"
+    #  snapshotter = "overlayfs"
+
+[proxy_plugins]
+
+[stream_processors]
+
+  [stream_processors."io.containerd.ocicrypt.decoder.v1.tar"]
+    accepts = ["application/vnd.oci.image.layer.v1.tar+encrypted"]
+    args = ["--decryption-keys-path", "/etc/containerd/ocicrypt/keys"]
+    env = ["OCICRYPT_KEYPROVIDER_CONFIG=/etc/containerd/ocicrypt/ocicrypt_keyprovider.conf"]
+    path = "ctd-decoder"
+    returns = "application/vnd.oci.image.layer.v1.tar"
+
+  [stream_processors."io.containerd.ocicrypt.decoder.v1.tar.gzip"]
+    accepts = ["application/vnd.oci.image.layer.v1.tar+gzip+encrypted"]
+    args = ["--decryption-keys-path", "/etc/containerd/ocicrypt/keys"]
+    env = ["OCICRYPT_KEYPROVIDER_CONFIG=/etc/containerd/ocicrypt/ocicrypt_keyprovider.conf"]
+    path = "ctd-decoder"
+    returns = "application/vnd.oci.image.layer.v1.tar+gzip"
+
+[timeouts]
+  "io.containerd.timeout.bolt.open" = "0s"
+  "io.containerd.timeout.metrics.shimstats" = "2s"
+  "io.containerd.timeout.shim.cleanup" = "5s"
+  "io.containerd.timeout.shim.load" = "5s"
+  "io.containerd.timeout.shim.shutdown" = "3s"
+  "io.containerd.timeout.task.state" = "2s"
+
+[ttrpc]
+  address = ""
+  gid = 0
+  uid = 0

plugins/cri/cri.go

@@ -59,27 +59,7 @@ func init() {
 			plugins.WarningPlugin,
 		},
 		Config:          &defaultConfig,
-		ConfigMigration: func(ctx context.Context, configVersion int, pluginConfigs map[string]interface{}) error {
+		ConfigMigration: configMigration,
-			if configVersion >= version.ConfigVersion {
-				return nil
-			}
-			const pluginName = string(plugins.GRPCPlugin) + ".cri"
-			original, ok := pluginConfigs[pluginName]
-			if !ok {
-				return nil
-			}
-			src := original.(map[string]interface{})
-
-			// Currently only a single key migrated
-			if val, ok := src["disable_tcp_service"]; ok {
-				pluginConfigs[pluginName] = map[string]interface{}{
-					"disable_tcp_service": val,
-				}
-			} else {
-				delete(pluginConfigs, pluginName)
-			}
-			return nil
-		},
 		InitFn:          initCRIService,
 	})
 }
@@ -255,3 +235,31 @@ func getSandboxControllers(ic *plugin.InitContext) (map[string]sandbox.Controlle
 	}
 	return sc, nil
 }
+
+func configMigration(ctx context.Context, configVersion int, pluginConfigs map[string]interface{}) error {
+	if configVersion >= version.ConfigVersion {
+		return nil
+	}
+	const pluginName = string(plugins.GRPCPlugin) + ".cri"
+	src, ok := pluginConfigs[pluginName].(map[string]interface{})
+	if !ok {
+		return nil
+	}
+
+	dst := map[string]interface{}{}
+	for _, k := range []string{
+		"disable_tcp_service",
+		"stream_server_address",
+		"stream_server_port",
+		"stream_idle_timeout",
+		"enable_tls_streaming",
+		"x509_key_pair_streaming",
+	} {
+		if val, ok := src[k]; ok {
+			dst[k] = val
+		}
+	}
+
+	pluginConfigs[pluginName] = dst
+	return nil
+}

plugins/cri/cri_test.go

@@ -0,0 +1,68 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package cri
+
+import (
+	"context"
+	"testing"
+
+	"github.com/containerd/containerd/v2/plugins"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCRIGRPCServerConfigMigration(t *testing.T) {
+	pluginName := string(plugins.GRPCPlugin) + ".cri"
+
+	src := map[string]interface{}{
+		// these should be removed in the new cri grpc config
+		"registry": map[string]interface{}{
+			"config_path": "/etc/containerd/certs.d",
+		},
+		"containerd": map[string]interface{}{
+			"default_runtime_name": "runc",
+		},
+
+		// these should be kept in the new cri grpc config
+		"disable_tcp_service":   false,
+		"stream_server_address": "127.0.0.2",
+		"stream_server_port":    "10000",
+		"stream_idle_timeout":   "3h0m0s",
+		"enable_tls_streaming":  true,
+		"x509_key_pair_streaming": map[string]interface{}{
+			"cert_file": "/etc/containerd/certs.d/server.crt",
+			"key_file":  "/etc/containerd/certs.d/server.key",
+		},
+	}
+	pluginConfigs := map[string]interface{}{
+		pluginName: src,
+	}
+	configMigration(context.Background(), 2, pluginConfigs)
+
+	dst, ok := pluginConfigs[pluginName].(map[string]interface{})
+	assert.True(t, ok)
+	assert.NotNil(t, dst)
+
+	for _, k := range []string{"registry", "containerd"} {
+		_, ok := dst[k]
+		assert.False(t, ok)
+		delete(src, k)
+	}
+
+	for k, v := range src {
+		assert.Equal(t, v, dst[k])
+	}
+}