From 543f29be4eafc43c6d8aeda389cf684922a14306 Mon Sep 17 00:00:00 2001 From: "Rostislav M. Georgiev" Date: Thu, 28 Nov 2019 14:47:53 +0200 Subject: [PATCH] kubeadm: Reduce kubelet.DownloadConfig usage kubelet.DownloadConfig is an old utility function which takes a client set and a kubelet version, uses them to fetch the kubelet component config from a config map, and places it in a local file. This function is simple to use, but it is dangerous and unnecessary. Practically, in all cases the kubelet configuration is present locally and does not need to be fetched from a config map on the cluster (it just needs to be stored in a file). Furthermore, kubelet.DownloadConfig does not use the kubeadm component configs module in any way. Hence, a kubelet configuration fetched using it may not be patched, validated, or otherwise, processed in any way by kubeadm other than piping it to a file. This patch replaces all but a single kubelet.DownloadConfig invocation with equivalents that get the local copy of the kubelet component config and just store it in a file. The sole remaining invocation covers the `kubeadm upgrade node --kubelet-version` case. In addition to that, a possible panic is fixed in kubelet.DownloadConfig and it now takes the kubelet version parameter as string. Signed-off-by: Rostislav M. Georgiev --- cmd/kubeadm/app/cmd/phases/init/BUILD | 1 - cmd/kubeadm/app/cmd/phases/init/kubelet.go | 8 +---- cmd/kubeadm/app/cmd/phases/join/BUILD | 1 - cmd/kubeadm/app/cmd/phases/join/kubelet.go | 8 +---- cmd/kubeadm/app/cmd/phases/upgrade/node/BUILD | 1 - .../cmd/phases/upgrade/node/kubeletconfig.go | 32 ++++++++----------- cmd/kubeadm/app/cmd/upgrade/apply.go | 2 +- cmd/kubeadm/app/phases/kubelet/BUILD | 1 - cmd/kubeadm/app/phases/kubelet/config.go | 32 ++++++++++++------- cmd/kubeadm/app/phases/upgrade/postupgrade.go | 16 +++------- 10 files changed, 42 insertions(+), 60 deletions(-) diff --git a/cmd/kubeadm/app/cmd/phases/init/BUILD b/cmd/kubeadm/app/cmd/phases/init/BUILD index 98fcbdf23a1..d79f0cecef7 100644 --- a/cmd/kubeadm/app/cmd/phases/init/BUILD +++ b/cmd/kubeadm/app/cmd/phases/init/BUILD @@ -27,7 +27,6 @@ go_library( "//cmd/kubeadm/app/cmd/options:go_default_library", "//cmd/kubeadm/app/cmd/phases/workflow:go_default_library", "//cmd/kubeadm/app/cmd/util:go_default_library", - "//cmd/kubeadm/app/componentconfigs:go_default_library", "//cmd/kubeadm/app/constants:go_default_library", "//cmd/kubeadm/app/phases/addons/dns:go_default_library", "//cmd/kubeadm/app/phases/addons/proxy:go_default_library", diff --git a/cmd/kubeadm/app/cmd/phases/init/kubelet.go b/cmd/kubeadm/app/cmd/phases/init/kubelet.go index 311384f4ab0..0db4fa16fe5 100644 --- a/cmd/kubeadm/app/cmd/phases/init/kubelet.go +++ b/cmd/kubeadm/app/cmd/phases/init/kubelet.go @@ -24,7 +24,6 @@ import ( "k8s.io/kubernetes/cmd/kubeadm/app/cmd/options" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow" cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util" - "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" kubeletphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubelet" ) @@ -72,13 +71,8 @@ func runKubeletStart(c workflow.RunData) error { return errors.Wrap(err, "error writing a dynamic environment file for the kubelet") } - kubeletCfg, ok := data.Cfg().ComponentConfigs[componentconfigs.KubeletGroup] - if !ok { - return errors.New("no kubelet component config found in the active component config set") - } - // Write the kubelet configuration file to disk. - if err := kubeletphase.WriteConfigToDisk(kubeletCfg, data.KubeletDir()); err != nil { + if err := kubeletphase.WriteConfigToDisk(&data.Cfg().ClusterConfiguration, data.KubeletDir()); err != nil { return errors.Wrap(err, "error writing kubelet configuration to disk") } diff --git a/cmd/kubeadm/app/cmd/phases/join/BUILD b/cmd/kubeadm/app/cmd/phases/join/BUILD index 80000374edc..c4492d4cce3 100644 --- a/cmd/kubeadm/app/cmd/phases/join/BUILD +++ b/cmd/kubeadm/app/cmd/phases/join/BUILD @@ -34,7 +34,6 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/version:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/tools/clientcmd/api:go_default_library", diff --git a/cmd/kubeadm/app/cmd/phases/join/kubelet.go b/cmd/kubeadm/app/cmd/phases/join/kubelet.go index a119f10e460..4abb2d6ba4e 100644 --- a/cmd/kubeadm/app/cmd/phases/join/kubelet.go +++ b/cmd/kubeadm/app/cmd/phases/join/kubelet.go @@ -26,7 +26,6 @@ import ( v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/util/wait" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" certutil "k8s.io/client-go/util/cert" @@ -122,11 +121,6 @@ func runKubeletStartJoinPhase(c workflow.RunData) (returnErr error) { } } - kubeletVersion, err := version.ParseSemantic(initCfg.ClusterConfiguration.KubernetesVersion) - if err != nil { - return err - } - bootstrapClient, err := kubeconfigutil.ClientSetFromFile(bootstrapKubeConfigFile) if err != nil { return errors.Errorf("couldn't create client from kubeconfig file %q", bootstrapKubeConfigFile) @@ -160,7 +154,7 @@ func runKubeletStartJoinPhase(c workflow.RunData) (returnErr error) { kubeletphase.TryStopKubelet() // Write the configuration for the kubelet (using the bootstrap token credentials) to disk so the kubelet can start - if err := kubeletphase.DownloadConfig(bootstrapClient, kubeletVersion, kubeadmconstants.KubeletRunDirectory); err != nil { + if err := kubeletphase.WriteConfigToDisk(&initCfg.ClusterConfiguration, kubeadmconstants.KubeletRunDirectory); err != nil { return err } diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/BUILD b/cmd/kubeadm/app/cmd/phases/upgrade/node/BUILD index f5d74dcac4a..2e1080798cc 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/BUILD +++ b/cmd/kubeadm/app/cmd/phases/upgrade/node/BUILD @@ -22,7 +22,6 @@ go_library( "//cmd/kubeadm/app/util/apiclient:go_default_library", "//cmd/kubeadm/app/util/dryrun:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/version:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//vendor/github.com/pkg/errors:go_default_library", "//vendor/k8s.io/utils/exec:go_default_library", diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go index 9d147e17719..e6a50d7d8c8 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go +++ b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go @@ -23,7 +23,6 @@ import ( "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/util/version" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/options" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow" cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util" @@ -66,7 +65,6 @@ func runKubeletConfigPhase() func(c workflow.RunData) error { // otherwise, retrieve all the info required for kubelet config upgrade cfg := data.Cfg() - client := data.Client() dryRun := data.DryRun() // Set up the kubelet directory to use. If dry-running, this will return a fake directory @@ -75,24 +73,20 @@ func runKubeletConfigPhase() func(c workflow.RunData) error { return err } - // Gets the target kubelet version. - // by default kubelet version is expected to be equal to ClusterConfiguration.KubernetesVersion, but - // users can specify a different kubelet version (this is a legacy of the original implementation - // of `kubeam upgrade node config` which we are preserving in order to don't break GA contract) - kubeletVersionStr := cfg.ClusterConfiguration.KubernetesVersion - if data.KubeletVersion() != "" && data.KubeletVersion() != kubeletVersionStr { - kubeletVersionStr = data.KubeletVersion() - fmt.Printf("[upgrade] Using kubelet config version %s, while kubernetes-version is %s\n", kubeletVersionStr, cfg.ClusterConfiguration.KubernetesVersion) - } - - // Parse the desired kubelet version - kubeletVersion, err := version.ParseSemantic(kubeletVersionStr) - if err != nil { - return err - } - // TODO: Checkpoint the current configuration first so that if something goes wrong it can be recovered - if err := kubeletphase.DownloadConfig(client, kubeletVersion, kubeletDir); err != nil { + + // Store the kubelet component configuration. + // By default the kubelet version is expected to be equal to cfg.ClusterConfiguration.KubernetesVersion, but + // users can specify a different kubelet version (this is a legacy of the original implementation + // of `kubeadm upgrade node config` which we are preserving in order to not break the GA contract) + if data.KubeletVersion() != "" && data.KubeletVersion() != cfg.ClusterConfiguration.KubernetesVersion { + fmt.Printf("[upgrade] Using kubelet config version %s, while kubernetes-version is %s\n", data.KubeletVersion(), cfg.ClusterConfiguration.KubernetesVersion) + if err := kubeletphase.DownloadConfig(data.Client(), data.KubeletVersion(), kubeletDir); err != nil { + return err + } + + // WriteConfigToDisk is what we should be calling since we already have the correct component config loaded + } else if err = kubeletphase.WriteConfigToDisk(&cfg.ClusterConfiguration, kubeletDir); err != nil { return err } diff --git a/cmd/kubeadm/app/cmd/upgrade/apply.go b/cmd/kubeadm/app/cmd/upgrade/apply.go index 8b59e6a2c59..9ca4f5c515e 100644 --- a/cmd/kubeadm/app/cmd/upgrade/apply.go +++ b/cmd/kubeadm/app/cmd/upgrade/apply.go @@ -170,7 +170,7 @@ func runApply(flags *applyFlags, userVersion string) error { // Upgrade RBAC rules and addons. klog.V(1).Infoln("[upgrade/postupgrade] upgrading RBAC rules and addons") - if err := upgrade.PerformPostUpgradeTasks(client, cfg, newK8sVersion, flags.dryRun); err != nil { + if err := upgrade.PerformPostUpgradeTasks(client, cfg, flags.dryRun); err != nil { return errors.Wrap(err, "[upgrade/postupgrade] FATAL post-upgrade error") } diff --git a/cmd/kubeadm/app/phases/kubelet/BUILD b/cmd/kubeadm/app/phases/kubelet/BUILD index 276b76dbf17..4f9a0050655 100644 --- a/cmd/kubeadm/app/phases/kubelet/BUILD +++ b/cmd/kubeadm/app/phases/kubelet/BUILD @@ -20,7 +20,6 @@ go_library( "//cmd/kubeadm/app/util/initsystem:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/rbac/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/version:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", diff --git a/cmd/kubeadm/app/phases/kubelet/config.go b/cmd/kubeadm/app/phases/kubelet/config.go index 6e873528101..1aeb39b6ebd 100644 --- a/cmd/kubeadm/app/phases/kubelet/config.go +++ b/cmd/kubeadm/app/phases/kubelet/config.go @@ -26,7 +26,6 @@ import ( v1 "k8s.io/api/core/v1" rbac "k8s.io/api/rbac/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/version" clientset "k8s.io/client-go/kubernetes" @@ -38,12 +37,17 @@ import ( // WriteConfigToDisk writes the kubelet config object down to a file // Used at "kubeadm init" and "kubeadm upgrade" time -func WriteConfigToDisk(kubeletCfg kubeadmapi.ComponentConfig, kubeletDir string) error { +func WriteConfigToDisk(cfg *kubeadmapi.ClusterConfiguration, kubeletDir string) error { + kubeletCfg, ok := cfg.ComponentConfigs[componentconfigs.KubeletGroup] + if !ok { + return errors.New("no kubelet component config found in the active component config set") + } kubeletBytes, err := kubeletCfg.Marshal() if err != nil { return err } + return writeConfigBytesToDisk(kubeletBytes, kubeletDir) } @@ -130,8 +134,13 @@ func createConfigMapRBACRules(client clientset.Interface, k8sVersion *version.Ve } // DownloadConfig downloads the kubelet configuration from a ConfigMap and writes it to disk. -// Used at "kubeadm join" time -func DownloadConfig(client clientset.Interface, kubeletVersion *version.Version, kubeletDir string) error { +// DEPRECATED: Do not use in new code! +func DownloadConfig(client clientset.Interface, kubeletVersionStr string, kubeletDir string) error { + // Parse the desired kubelet version + kubeletVersion, err := version.ParseSemantic(kubeletVersionStr) + if err != nil { + return err + } // Download the ConfigMap from the cluster based on what version the kubelet is configMapName := kubeadmconstants.GetKubeletConfigMapName(kubeletVersion) @@ -139,17 +148,18 @@ func DownloadConfig(client clientset.Interface, kubeletVersion *version.Version, fmt.Printf("[kubelet-start] Downloading configuration for the kubelet from the %q ConfigMap in the %s namespace\n", configMapName, metav1.NamespaceSystem) - kubeletCfg, err := apiclient.GetConfigMapWithRetry(client, metav1.NamespaceSystem, configMapName) - // If the ConfigMap wasn't found and the kubelet version is v1.10.x, where we didn't support the config file yet - // just return, don't error out - if apierrors.IsNotFound(err) && kubeletVersion.Minor() == 10 { - return nil - } + kubeletCfgMap, err := apiclient.GetConfigMapWithRetry(client, metav1.NamespaceSystem, configMapName) if err != nil { return err } - return writeConfigBytesToDisk([]byte(kubeletCfg.Data[kubeadmconstants.KubeletBaseConfigurationConfigMapKey]), kubeletDir) + // Check for the key existence, otherwise we'll panic here + kubeletCfg, ok := kubeletCfgMap.Data[kubeadmconstants.KubeletBaseConfigurationConfigMapKey] + if !ok { + return errors.Errorf("no key %q found in config map %s", kubeadmconstants.KubeletBaseConfigurationConfigMapKey, configMapName) + } + + return writeConfigBytesToDisk([]byte(kubeletCfg), kubeletDir) } // configMapRBACName returns the name for the Role/RoleBinding for the kubelet config configmap for the right branch of k8s diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade.go b/cmd/kubeadm/app/phases/upgrade/postupgrade.go index 9daece5e00d..886a9da1fca 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade.go +++ b/cmd/kubeadm/app/phases/upgrade/postupgrade.go @@ -26,7 +26,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" errorsutil "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/apimachinery/pkg/util/version" clientset "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" @@ -44,7 +43,7 @@ import ( // PerformPostUpgradeTasks runs nearly the same functions as 'kubeadm init' would do // Note that the mark-control-plane phase is left out, not needed, and no token is created as that doesn't belong to the upgrade -func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.InitConfiguration, newK8sVer *version.Version, dryRun bool) error { +func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.InitConfiguration, dryRun bool) error { errs := []error{} // Upload currently used configuration to the cluster @@ -60,7 +59,7 @@ func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.InitCon } // Write the new kubelet config down to disk and the env file if needed - if err := writeKubeletConfigFiles(client, cfg, newK8sVer, dryRun); err != nil { + if err := writeKubeletConfigFiles(client, cfg, dryRun); err != nil { errs = append(errs, err) } @@ -204,7 +203,7 @@ func removeOldDNSDeploymentIfAnotherDNSIsUsed(cfg *kubeadmapi.ClusterConfigurati }, 10) } -func writeKubeletConfigFiles(client clientset.Interface, cfg *kubeadmapi.InitConfiguration, newK8sVer *version.Version, dryRun bool) error { +func writeKubeletConfigFiles(client clientset.Interface, cfg *kubeadmapi.InitConfiguration, dryRun bool) error { kubeletDir, err := GetKubeletDir(dryRun) if err != nil { // The error here should never occur in reality, would only be thrown if /tmp doesn't exist on the machine. @@ -212,13 +211,8 @@ func writeKubeletConfigFiles(client clientset.Interface, cfg *kubeadmapi.InitCon } errs := []error{} // Write the configuration for the kubelet down to disk so the upgraded kubelet can start with fresh config - if err := kubeletphase.DownloadConfig(client, newK8sVer, kubeletDir); err != nil { - // Tolerate the error being NotFound when dryrunning, as there is a pretty common scenario: the dryrun process - // *would* post the new kubelet-config-1.X configmap that doesn't exist now when we're trying to download it - // again. - if !(apierrors.IsNotFound(err) && dryRun) { - errs = append(errs, errors.Wrap(err, "error downloading kubelet configuration from the ConfigMap")) - } + if err := kubeletphase.WriteConfigToDisk(&cfg.ClusterConfiguration, kubeletDir); err != nil { + errs = append(errs, errors.Wrap(err, "error writing kubelet configuration to file")) } if dryRun { // Print what contents would be written