From f9643b69f329abada2ac01210e6e516bd1c99171 Mon Sep 17 00:00:00 2001 From: Paco Xu Date: Thu, 25 Aug 2022 16:57:22 +0800 Subject: [PATCH] cleanup kubelet config file: kubeadm-flags.env to remove container-runtime flag --- .../cmd/phases/upgrade/node/kubeletconfig.go | 6 ++ cmd/kubeadm/app/phases/upgrade/postupgrade.go | 56 ++++++++++++++----- .../app/phases/upgrade/postupgrade_test.go | 21 ++++--- 3 files changed, 58 insertions(+), 25 deletions(-) diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go index 5ec99c4e260..b2652dac2da 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go +++ b/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 diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade.go b/cmd/kubeadm/app/phases/upgrade/postupgrade.go index d6a5394ccde..313bcd1bf3e 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade.go +++ b/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) diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go b/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go index c5c9472f408..f23fa7d8937 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go +++ b/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) }