kubeadm: ensure CRI URL scheme is present in the kubelet env file

During "upgrade node" and "upgrade apply" read the
kubelet env file from /var/lib/kubelet/kubeadm-flags.env
patch the --container-runtime-endpoint flag value to
have the appropriate URL scheme prefix (e.g. unix:// on Linux)
and write the file back to disk.

This is a temporary workaround that should be kept only for 1 release
cycle - i.e. remove this in 1.25.

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

@@ -122,6 +122,12 @@ func runKubeletConfigPhase() func(c workflow.RunData) error {
 			}
 		}
 
+		// TODO: Temporary workaround. Remove in 1.25:
+		// https://github.com/kubernetes/kubeadm/issues/2426
+		if err := upgrade.UpdateKubeletDynamicEnvFileWithURLScheme(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/upgrade/postupgrade.go

@@ -18,7 +18,11 @@ package upgrade
 
 import (
 	"context"
+	"fmt"
+	"io/ioutil"
 	"os"
+	"path/filepath"
+	"strings"
 
 	"github.com/pkg/errors"
 
@@ -31,6 +35,7 @@ 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"
@@ -65,6 +70,12 @@ func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.InitCon
 		errs = append(errs, err)
 	}
 
+	// TODO: Temporary workaround. Remove in 1.25:
+	// https://github.com/kubernetes/kubeadm/issues/2426
+	if err := UpdateKubeletDynamicEnvFileWithURLScheme(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
@@ -231,3 +242,67 @@ func LabelOldControlPlaneNodes(client clientset.Interface) error {
 	}
 	return nil
 }
+
+// UpdateKubeletDynamicEnvFileWithURLScheme reads the kubelet dynamic environment file
+// from disk, ensure that the CRI endpoint flag has a scheme prefix and writes it
+// back to disk.
+// TODO: Temporary workaround. Remove in 1.25:
+// https://github.com/kubernetes/kubeadm/issues/2426
+func UpdateKubeletDynamicEnvFileWithURLScheme(dryRun bool) error {
+	filePath := filepath.Join(kubeadmconstants.KubeletRunDirectory, kubeadmconstants.KubeletEnvFileName)
+	if dryRun {
+		fmt.Printf("[dryrun] Would ensure that %q includes a CRI endpoint URL scheme\n", filePath)
+		return nil
+	}
+	klog.V(2).Infof("Ensuring that %q includes a CRI endpoint URL scheme", filePath)
+	bytes, err := ioutil.ReadFile(filePath)
+	if err != nil {
+		return errors.Wrapf(err, "failed to read kubelet configuration from file %q", filePath)
+	}
+	updated := updateKubeletDynamicEnvFileWithURLScheme(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 updateKubeletDynamicEnvFileWithURLScheme(str string) string {
+	const (
+		flag   = "container-runtime-endpoint"
+		scheme = kubeadmapiv1.DefaultContainerRuntimeURLScheme + "://"
+	)
+	// Trim the prefix
+	str = strings.TrimLeft(str, fmt.Sprintf("%s=\"", kubeadmconstants.KubeletEnvFileVariableName))
+
+	// Flags are managed by kubeadm as pairs of key=value separated by space.
+	// Split them, find the one containing the flag of interest and update
+	// its value to have the scheme prefix.
+	split := strings.Split(str, " ")
+	for i, s := range split {
+		if !strings.Contains(s, flag) {
+			continue
+		}
+		keyValue := strings.Split(s, "=")
+		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.
+			if i+1 < len(split) {
+				nextArg := split[i+1]
+				if !strings.HasPrefix(nextArg, "-") && !strings.HasPrefix(nextArg, scheme) {
+					split[i+1] = scheme + nextArg
+				}
+			}
+			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, "=")
+	}
+	str = strings.Join(split, " ")
+	return fmt.Sprintf("%s=\"%s", kubeadmconstants.KubeletEnvFileVariableName, str)
+}

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

@@ -17,6 +17,7 @@ limitations under the License.
 package upgrade
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -24,6 +25,7 @@ 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"
 )
@@ -101,3 +103,45 @@ func TestRollbackFiles(t *testing.T) {
 		t.Fatalf("Expected error contains %q, got %v", errString, err)
 	}
 }
+
+func TestUpdateKubeletDynamicEnvFileWithURLScheme(t *testing.T) {
+	tcases := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "missing flag of interest",
+			input:    fmt.Sprintf("%s=\"--foo=abc --bar=def\"", constants.KubeletEnvFileVariableName),
+			expected: fmt.Sprintf("%s=\"--foo=abc --bar=def\"", constants.KubeletEnvFileVariableName),
+		},
+		{
+			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),
+		},
+		{
+			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),
+		},
+		{
+			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),
+		},
+		{
+			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),
+		},
+	}
+	for _, tt := range tcases {
+		t.Run(tt.name, func(t *testing.T) {
+			output := updateKubeletDynamicEnvFileWithURLScheme(tt.input)
+			if output != tt.expected {
+				t.Errorf("expected output: %q, got: %q", tt.expected, output)
+			}
+		})
+	}
+}