From 6cf3e36c3706669f5f98ed7b6c083daee7d64512 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Thu, 8 Jul 2021 02:35:30 +0300 Subject: [PATCH] kubeadm: statically default the "from cluster" InitConfiguration During operations such as "upgrade", kubeadm fetches the ClusterConfiguration object from the kubeadm ConfigMap. However, due to requiring node specifics it wraps it in an InitConfiguration object. The function responsible for that is: app/util/config#FetchInitConfigurationFromCluster(). A problem with this function (and sub-calls) is that it ignores the static defaults applied from versioned types (e.g. v1beta3/defaults.go) and only applies dynamic defaults for: - API endpoints - node registration - etc... The introduction of Init|JoinConfiguration.ImagePullPolicy now has static defaulting of the NodeRegistration object with a default policy of "PullIfNotPresent". Respect this defaulting by constructing a defaulted internal InitConfiguration from FetchInitConfigurationFromCluster() and only then apply the dynamic defaults over it. This fixes a bug where "kubeadm upgrade ..." fails when pulling images due to an empty ("") ImagePullPolicy. We could assume that empty string means default policy on runtime in: cmd/kubeadm/app/preflight/checks.go#ImagePullCheck() but that might actually not be the user intent during "init" and "join", due to e.g. a typo. Similarly, we don't allow empty tokens on runtime and error out. --- cmd/kubeadm/app/util/config/cluster.go | 8 +++++++- cmd/kubeadm/app/util/config/cluster_test.go | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/cmd/kubeadm/app/util/config/cluster.go b/cmd/kubeadm/app/util/config/cluster.go index 32927d70489..12fce577e84 100644 --- a/cmd/kubeadm/app/util/config/cluster.go +++ b/cmd/kubeadm/app/util/config/cluster.go @@ -26,6 +26,7 @@ import ( kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmscheme "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme" + kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3" "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" "k8s.io/kubernetes/cmd/kubeadm/app/constants" "k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" @@ -68,8 +69,13 @@ func getInitConfigurationFromCluster(kubeconfigDir string, client clientset.Inte return nil, errors.Wrap(err, "failed to get config map") } - // InitConfiguration is composed with data from different places + // Take an empty versioned InitConfiguration, statically default it and convert it to the internal type + versionedInitcfg := &kubeadmapiv1.InitConfiguration{} + kubeadmscheme.Scheme.Default(versionedInitcfg) initcfg := &kubeadmapi.InitConfiguration{} + if err := kubeadmscheme.Scheme.Convert(versionedInitcfg, initcfg, nil); err != nil { + return nil, errors.Wrap(err, "could not prepare a defaulted InitConfiguration") + } // gets ClusterConfiguration from kubeadm-config clusterConfigurationData, ok := configMap.Data[constants.ClusterConfigurationConfigMapKey] diff --git a/cmd/kubeadm/app/util/config/cluster_test.go b/cmd/kubeadm/app/util/config/cluster_test.go index 70b83d93f44..8aa9490a351 100644 --- a/cmd/kubeadm/app/util/config/cluster_test.go +++ b/cmd/kubeadm/app/util/config/cluster_test.go @@ -730,6 +730,9 @@ func TestGetInitConfigurationFromCluster(t *testing.T) { if cfg.ClusterConfiguration.KubernetesVersion != k8sVersionString { t.Errorf("invalid ClusterConfiguration.KubernetesVersion") } + if cfg.NodeRegistration.ImagePullPolicy != kubeadmapiv1.DefaultImagePullPolicy { + t.Errorf("invalid cfg.NodeRegistration.ImagePullPolicy %v", cfg.NodeRegistration.ImagePullPolicy) + } if !rt.newControlPlane && (cfg.LocalAPIEndpoint.AdvertiseAddress != "1.2.3.4" || cfg.LocalAPIEndpoint.BindPort != 1234) { t.Errorf("invalid cfg.LocalAPIEndpoint: %v", cfg.LocalAPIEndpoint) }