Merge pull request #112000 from pacoxu/kubeadm-cleanup

kubeadm: remove container-runtime=remote

cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go

@@ -86,6 +86,12 @@ func runKubeletConfigPhase() func(c workflow.RunData) error {
 			return nil
 		}
 
+		// TODO: Temporary workaround. Remove in 1.27:
+		// https://github.com/kubernetes/kubeadm/issues/2626
+		if err := upgrade.CleanupKubeletDynamicEnvFileContainerRuntime(dryRun); err != nil {
+			return err
+		}
+
 		fmt.Println("[upgrade] The configuration for this node was successfully updated!")
 		fmt.Println("[upgrade] Now you should go ahead and upgrade the kubelet package using your package manager.")
 		return nil

cmd/kubeadm/app/phases/kubelet/flags.go

@@ -76,8 +76,6 @@ func WriteKubeletDynamicEnvFile(cfg *kubeadmapi.ClusterConfiguration, nodeReg *k
 func buildKubeletArgMapCommon(opts kubeletFlagsOpts) map[string]string {
 	kubeletFlags := map[string]string{}
 	kubeletFlags["container-runtime-endpoint"] = opts.nodeRegOpts.CRISocket
-	// container runtime is by default docker in kubelet v1.23, so it can be removed in v1.26
-	kubeletFlags["container-runtime"] = "remote"
 
 	// This flag passes the pod infra container image (e.g. "pause" image) to the kubelet
 	// and prevents its garbage collection

cmd/kubeadm/app/phases/kubelet/flags_test.go

@@ -42,7 +42,6 @@ func TestBuildKubeletArgMap(t *testing.T) {
 				},
 			},
 			expected: map[string]string{
-				"container-runtime":          "remote",
 				"container-runtime-endpoint": "unix:///var/run/containerd/containerd.sock",
 				"hostname-override":          "override-name",
 			},
@@ -68,7 +67,6 @@ func TestBuildKubeletArgMap(t *testing.T) {
 				registerTaintsUsingFlags: true,
 			},
 			expected: map[string]string{
-				"container-runtime":          "remote",
 				"container-runtime-endpoint": "unix:///var/run/containerd/containerd.sock",
 				"register-with-taints":       "foo=bar:baz,key=val:eff",
 			},
@@ -82,7 +80,6 @@ func TestBuildKubeletArgMap(t *testing.T) {
 				pauseImage: "registry.k8s.io/pause:3.8",
 			},
 			expected: map[string]string{
-				"container-runtime":          "remote",
 				"container-runtime-endpoint": "unix:///var/run/containerd/containerd.sock",
 				"pod-infra-container-image":  "registry.k8s.io/pause:3.8",
 			},

cmd/kubeadm/app/phases/upgrade/postupgrade.go

@@ -20,7 +20,9 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -34,7 +36,6 @@ import (
 	"k8s.io/klog/v2"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/dns"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/proxy"
@@ -69,6 +70,12 @@ func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.InitCon
 		errs = append(errs, err)
 	}
 
+	// TODO: Temporary workaround. Remove in 1.27:
+	// https://github.com/kubernetes/kubeadm/issues/2626
+	if err := CleanupKubeletDynamicEnvFileContainerRuntime(dryRun); err != nil {
+		return err
+	}
+
 	// Annotate the node with the crisocket information, sourced either from the InitConfiguration struct or
 	// --cri-socket.
 	// TODO: In the future we want to use something more official like NodeStatus or similar for detecting this properly
@@ -247,10 +254,32 @@ func RemoveOldControlPlaneTaint(client clientset.Interface) error {
 	return nil
 }
 
-func updateKubeletDynamicEnvFileWithURLScheme(str string) string {
+// CleanupKubeletDynamicEnvFileContainerRuntime reads the kubelet dynamic environment file
+// from disk, ensure that the container runtime flag is removed.
+// TODO: Temporary workaround. Remove in 1.27:
+// https://github.com/kubernetes/kubeadm/issues/2626
+func CleanupKubeletDynamicEnvFileContainerRuntime(dryRun bool) error {
+	filePath := filepath.Join(kubeadmconstants.KubeletRunDirectory, kubeadmconstants.KubeletEnvFileName)
+	if dryRun {
+		fmt.Printf("[dryrun] Would ensure that %q does not include a --container-runtime flag\n", filePath)
+		return nil
+	}
+	klog.V(2).Infof("Ensuring that %q does not include a --container-runtime flag", filePath)
+	bytes, err := ioutil.ReadFile(filePath)
+	if err != nil {
+		return errors.Wrapf(err, "failed to read kubelet configuration from file %q", filePath)
+	}
+	updated := cleanupKubeletDynamicEnvFileContainerRuntime(string(bytes))
+	if err := ioutil.WriteFile(filePath, []byte(updated), 0644); err != nil {
+		return errors.Wrapf(err, "failed to write kubelet configuration to the file %q", filePath)
+	}
+	return nil
+}
+
+func cleanupKubeletDynamicEnvFileContainerRuntime(str string) string {
 	const (
-		flag   = "container-runtime-endpoint"
-		scheme = kubeadmapiv1.DefaultContainerRuntimeURLScheme + "://"
+		// `remote` is the only possible value
+		flag = "container-runtime"
 	)
 	// Trim the prefix
 	str = strings.TrimLeft(str, fmt.Sprintf("%s=\"", kubeadmconstants.KubeletEnvFileVariableName))
@@ -267,22 +296,21 @@ func updateKubeletDynamicEnvFileWithURLScheme(str string) string {
 		if len(keyValue) < 2 {
 			// Post init/join, the user may have edited the file and has flags that are not
 			// followed by "=". If that is the case the next argument must be the value
-			// of the endpoint flag and if its not a flag itself. Update that argument with
-			// the scheme instead.
+			// of the endpoint flag and if its not a flag itself.
 			if i+1 < len(split) {
 				nextArg := split[i+1]
-				if !strings.HasPrefix(nextArg, "-") && !strings.HasPrefix(nextArg, scheme) {
-					split[i+1] = scheme + nextArg
+				if strings.HasPrefix(nextArg, "-") {
+					// remove the flag only
+					split = append(split[:i], split[i+1:]...)
+				} else {
+					// remove the flag and value
+					split = append(split[:i], split[i+2:]...)
 				}
 			}
 			continue
 		}
-		if len(keyValue[1]) == 0 || strings.HasPrefix(keyValue[1], scheme) {
-			continue // The flag value already has the URL scheme prefix or is empty
-		}
-		// Missing prefix. Add it and update the key=value pair
-		keyValue[1] = scheme + keyValue[1]
-		split[i] = strings.Join(keyValue, "=")
+		// remove the flag and value in one
+		split = append(split[:i], split[i+1:]...)
 	}
 	str = strings.Join(split, " ")
 	return fmt.Sprintf("%s=\"%s", kubeadmconstants.KubeletEnvFileVariableName, str)

cmd/kubeadm/app/phases/upgrade/postupgrade_test.go

@@ -25,7 +25,6 @@ import (
 
 	"github.com/pkg/errors"
 
-	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	testutil "k8s.io/kubernetes/cmd/kubeadm/test"
 )
@@ -104,7 +103,7 @@ func TestRollbackFiles(t *testing.T) {
 	}
 }
 
-func TestUpdateKubeletDynamicEnvFileWithURLScheme(t *testing.T) {
+func TestCleanupKubeletDynamicEnvFileContainerRuntime(t *testing.T) {
 	tcases := []struct {
 		name     string
 		input    string
@@ -117,28 +116,28 @@ func TestUpdateKubeletDynamicEnvFileWithURLScheme(t *testing.T) {
 		},
 		{
 			name:     "add missing URL scheme",
-			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint=/some/endpoint --bar=def\"", constants.KubeletEnvFileVariableName),
-			expected: fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint=%s:///some/endpoint --bar=def\"", constants.KubeletEnvFileVariableName, kubeadmapiv1.DefaultContainerRuntimeURLScheme),
+			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime=remote --bar=def\"", constants.KubeletEnvFileVariableName),
+			expected: fmt.Sprintf("%s=\"--foo=abc --bar=def\"", constants.KubeletEnvFileVariableName),
 		},
 		{
 			name:     "add missing URL scheme if there is no '=' after the flag name",
-			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint /some/endpoint --bar=def\"", constants.KubeletEnvFileVariableName),
-			expected: fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint %s:///some/endpoint --bar=def\"", constants.KubeletEnvFileVariableName, kubeadmapiv1.DefaultContainerRuntimeURLScheme),
+			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime remote --bar=def\"", constants.KubeletEnvFileVariableName),
+			expected: fmt.Sprintf("%s=\"--foo=abc --bar=def\"", constants.KubeletEnvFileVariableName),
 		},
 		{
 			name:     "empty flag of interest value following '='",
-			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint= --bar=def\"", constants.KubeletEnvFileVariableName),
-			expected: fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint= --bar=def\"", constants.KubeletEnvFileVariableName),
+			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime= --bar=def\"", constants.KubeletEnvFileVariableName),
+			expected: fmt.Sprintf("%s=\"--foo=abc --bar=def\"", constants.KubeletEnvFileVariableName),
 		},
 		{
 			name:     "empty flag of interest value without '='",
-			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint --bar=def\"", constants.KubeletEnvFileVariableName),
-			expected: fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint --bar=def\"", constants.KubeletEnvFileVariableName),
+			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime --bar=def\"", constants.KubeletEnvFileVariableName),
+			expected: fmt.Sprintf("%s=\"--foo=abc --bar=def\"", constants.KubeletEnvFileVariableName),
 		},
 	}
 	for _, tt := range tcases {
 		t.Run(tt.name, func(t *testing.T) {
-			output := updateKubeletDynamicEnvFileWithURLScheme(tt.input)
+			output := cleanupKubeletDynamicEnvFileContainerRuntime(tt.input)
 			if output != tt.expected {
 				t.Errorf("expected output: %q, got: %q", tt.expected, output)
 			}