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 <rostislavg@vmware.com>

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",

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")
 	}
 

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",

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
 	}
 

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",

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
 		}
 

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")
 	}
 

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",

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

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