Remove confusing warning in cri runtime config migration

djdongjin · k8s-infra-cherrypick-robot · commit 468079c5c4a8 · 2025-01-13T15:26:45.000Z
Signed-off-by: Jin Dong &lt;djdongjin95@gmail.com&gt;
diff --git a/plugins/cri/runtime/plugin.go b/plugins/cri/runtime/plugin.go
@@ -51,20 +51,8 @@ func init() {
 		Requires: []plugin.Type{
 			plugins.WarningPlugin,
 		},
-		ConfigMigration: func(ctx context.Context, configVersion int, pluginConfigs map[string]interface{}) error {
-			if configVersion >= version.ConfigVersion {
-				return nil
-			}
-			c, ok := pluginConfigs[string(plugins.GRPCPlugin)+".cri"]
-			if !ok {
-				return nil
-			}
-			conf := c.(map[string]interface{})
-			migrateConfig(conf)
-			pluginConfigs[string(plugins.CRIServicePlugin)+".runtime"] = conf
-			return nil
-		},
-		InitFn: initCRIRuntime,
+		ConfigMigration: configMigration,
+		InitFn:          initCRIRuntime,
 	})
 }
 
@@ -198,12 +186,79 @@ func setGLogLevel() error {
 	return nil
 }
 
-func migrateConfig(conf map[string]interface{}) {
-	containerdConf, ok := conf["containerd"]
+func configMigration(ctx context.Context, configVersion int, pluginConfigs map[string]interface{}) error {
+	if configVersion >= version.ConfigVersion {
+		return nil
+	}
+	src, ok := pluginConfigs[string(plugins.GRPCPlugin)+".cri"].(map[string]interface{})
+	if !ok {
+		return nil
+	}
+	dst, ok := pluginConfigs[string(plugins.CRIServicePlugin)+".runtime"].(map[string]interface{})
+	if !ok {
+		dst = make(map[string]interface{})
+	}
+	migrateConfig(dst, src)
+	pluginConfigs[string(plugins.CRIServicePlugin)+".runtime"] = dst
+	return nil
+}
+
+func migrateConfig(dst, src map[string]interface{}) {
+	for k, v := range src {
+		switch k {
+		case "containerd":
+			// skip (handled separately below)
+			continue
+		case
+			"sandbox_image",
+			"registry",
+			"image_decryption",
+			"max_concurrent_downloads",
+			"image_pull_progress_timeout",
+			"image_pull_with_sync_fs",
+			"stats_collect_period":
+			// skip (moved to cri image service plugin)
+			continue
+		case
+			"disable_tcp_service",
+			"stream_server_address",
+			"stream_server_port",
+			"stream_idle_timeout",
+			"enable_tls_streaming",
+			"x509_key_pair_streaming":
+			// skip (moved to cri ServerConfig)
+			continue
+		default:
+			if _, ok := dst[k]; !ok {
+				dst[k] = v
+			}
+		}
+	}
+
+	// migrate cri containerd configs
+	containerdConf, ok := src["containerd"].(map[string]interface{})
 	if !ok {
 		return
 	}
-	runtimesConf, ok := containerdConf.(map[string]interface{})["runtimes"]
+	newContainerdConf, ok := dst["containerd"].(map[string]interface{})
+	if !ok {
+		newContainerdConf = map[string]interface{}{}
+	}
+	for k, v := range containerdConf {
+		switch k {
+		case "snapshotter", "disable_snapshot_annotations", "discard_unpacked_layers":
+			// skip (moved to cri image service plugin)
+			continue
+		default:
+			if _, ok := newContainerdConf[k]; !ok {
+				newContainerdConf[k] = v
+			}
+		}
+	}
+	dst["containerd"] = newContainerdConf
+
+	// migrate runtimes configs
+	runtimesConf, ok := newContainerdConf["runtimes"]
 	if !ok {
 		return
 	}
@@ -212,6 +267,7 @@ func migrateConfig(conf map[string]interface{}) {
 		if sandboxMode, ok := runtimeConf["sandbox_mode"]; ok {
 			if _, ok := runtimeConf["sandboxer"]; !ok {
 				runtimeConf["sandboxer"] = sandboxMode
+				delete(runtimeConf, "sandbox_mode")
 			}
 		}
 	}
diff --git a/plugins/cri/runtime/plugin_test.go b/plugins/cri/runtime/plugin_test.go
@@ -0,0 +1,69 @@
+/*
+   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 runtime
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/containerd/containerd/v2/plugins"
+)
+
+func TestCRIRuntimePluginConfigMigration(t *testing.T) {
+	runcSandboxer := "podsandbox"
+
+	grpcCri := map[string]interface{}{
+		"enable_selinux":              true,
+		"max_container_log_line_size": 100,
+		"max_concurrent_downloads":    3,    // removed since it's moved to cri image service
+		"disable_tcp_service":         true, // removed since it's moved to cri grpc service
+		"containerd": map[string]interface{}{
+			"runtimes": map[string]interface{}{
+				"runc": map[string]interface{}{
+					"sandbox_mode": runcSandboxer,
+				},
+			},
+		},
+	}
+
+	pluginConfigs := map[string]interface{}{
+		string(plugins.GRPCPlugin) + ".cri": grpcCri,
+	}
+	configMigration(context.Background(), 2, pluginConfigs)
+
+	runtimeConf, ok := pluginConfigs[string(plugins.CRIServicePlugin)+".runtime"].(map[string]interface{})
+	require.True(t, ok)
+	require.NotNil(t, runtimeConf)
+	assert.Equal(t, grpcCri["enable_selinux"], runtimeConf["enable_selinux"])
+	assert.Equal(t, grpcCri["max_container_log_line_size"], runtimeConf["max_container_log_line_size"])
+	assert.NotContains(t, runtimeConf, "max_concurrent_downloads")
+	assert.NotContains(t, runtimeConf, "disable_tcp_service")
+
+	ctd, ok := runtimeConf["containerd"].(map[string]interface{})
+	require.True(t, ok)
+	require.NotNil(t, ctd)
+
+	runtimes := ctd["runtimes"].(map[string]interface{})
+	runc, ok := runtimes["runc"].(map[string]interface{})
+	require.True(t, ok)
+	require.NotNil(t, runc)
+	assert.Equal(t, runcSandboxer, runc["sandboxer"])
+	assert.NotContains(t, runc, "sandbox_mode")
+}
