diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go index 04ddeff3ff0..a31e2c55173 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go +++ b/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 diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade.go b/cmd/kubeadm/app/phases/upgrade/postupgrade.go index d388194c276..c3e169489de 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade.go +++ b/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) +} diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go b/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go index 04ede0a0e3e..c5c9472f408 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go +++ b/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) + } + }) + } +}