kubeadm upgrade: Allow supplying hand migrated component configs

Currently, kubeadm would refuse to perfom an upgrade (or even planing for one)
if it detects a user supplied unsupported component config version. Hence,
users are required to manually upgrade their component configs and store them
in the config maps prior to executing `kubeadm upgrade plan` or
`kubeadm upgrade apply`.

This change introduces the ability to use the `--config` option of the
`kubeadm upgrade plan` and `kubeadm upgrade apply` commands to supply a YAML
file containing component configs to be used in place of the existing ones in
the cluster upon upgrade.

The old behavior where `--config` is used to reconfigure a cluster is still
supported. kubeadm automatically detects which behavior to use based on the
presence (or absense) of kubeadm config types (API group
`kubeadm.kubernetes.io`).

Signed-off-by: Rostislav M. Georgiev <rostislavg@vmware.com>

cmd/kubeadm/app/cmd/upgrade/BUILD

@@ -20,6 +20,7 @@ go_library(
         "//cmd/kubeadm/app/cmd/phases/upgrade/node: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/features:go_default_library",
         "//cmd/kubeadm/app/phases/controlplane:go_default_library",

cmd/kubeadm/app/cmd/upgrade/common.go

@@ -21,6 +21,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"os"
 	"strings"
 	"time"
@@ -36,16 +37,89 @@ import (
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation"
 	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
+	"k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/upgrade"
 	"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
+	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	"k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient"
 	configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config"
 	dryrunutil "k8s.io/kubernetes/cmd/kubeadm/app/util/dryrun"
 	kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
 )
 
+// isKubeadmConfigPresent checks if a kubeadm config type is found in the provided document map
+func isKubeadmConfigPresent(docmap kubeadmapi.DocumentMap) bool {
+	for gvk := range docmap {
+		if gvk.Group == kubeadmapi.GroupName {
+			return true
+		}
+	}
+	return false
+}
+
+// loadConfig loads configuration from a file and/or the cluster. InitConfiguration, ClusterConfiguration and (optionally) component configs
+// are loaded. This function allows the component configs to be loaded from a file that contains only them. If the file contains any kubeadm types
+// in it (API group "kubeadm.kubernetes.io" present), then the supplied file is treaded as a legacy reconfiguration style "--config" use and the
+// returned bool value is set to true (the only case to be done so).
+func loadConfig(cfgPath string, client clientset.Interface, skipComponentConfigs bool) (*kubeadmapi.InitConfiguration, bool, error) {
+	// Used for info logs here
+	const logPrefix = "upgrade/config"
+
+	// The usual case here is to not have a config file, but rather load the config from the cluster.
+	// This is probably 90% of the time. So we handle it first.
+	if cfgPath == "" {
+		cfg, err := configutil.FetchInitConfigurationFromCluster(client, os.Stdout, logPrefix, false, skipComponentConfigs)
+		return cfg, false, err
+	}
+
+	// Otherwise, we have a config file. Let's load it.
+	configBytes, err := ioutil.ReadFile(cfgPath)
+	if err != nil {
+		return nil, false, errors.Wrapf(err, "unable to load config from file %q", cfgPath)
+	}
+
+	// Split the YAML documents in the file into a DocumentMap
+	docmap, err := kubeadmutil.SplitYAMLDocuments(configBytes)
+	if err != nil {
+		return nil, false, err
+	}
+
+	// If there are kubeadm types (API group kubeadm.kubernetes.io) present, we need to keep the existing behavior
+	// here. Basically, we have to load all of the configs from the file and none from the cluster. Configs that are
+	// missing from the file will be automatically regenerated by kubeadm even if they are present in the cluster.
+	// The resulting configs overwrite the existing cluster ones at the end of a successful upgrade apply operation.
+	if isKubeadmConfigPresent(docmap) {
+		klog.Warning("WARNING: Usage of the --config flag with kubeadm config types for reconfiguring the cluster during upgrade is not recommended!")
+		cfg, err := configutil.BytesToInitConfiguration(configBytes)
+		return cfg, true, err
+	}
+
+	// If no kubeadm config types are present, we assume that there are manually upgraded component configs in the file.
+	// Hence, we load the kubeadm types from the cluster.
+	initCfg, err := configutil.FetchInitConfigurationFromCluster(client, os.Stdout, logPrefix, false, true)
+	if err != nil {
+		return nil, false, err
+	}
+
+	// Stop here if the caller does not want us to load the component configs
+	if !skipComponentConfigs {
+		// Load the component configs with upgrades
+		if err := componentconfigs.FetchFromClusterWithLocalOverwrites(&initCfg.ClusterConfiguration, client, docmap); err != nil {
+			return nil, false, err
+		}
+
+		// Now default and validate the configs
+		componentconfigs.Default(&initCfg.ClusterConfiguration, &initCfg.LocalAPIEndpoint, &initCfg.NodeRegistration)
+		if errs := componentconfigs.Validate(&initCfg.ClusterConfiguration); len(errs) != 0 {
+			return nil, false, errs.ToAggregate()
+		}
+	}
+
+	return initCfg, false, nil
+}
+
 // enforceRequirements verifies that it's okay to upgrade and then returns the variables needed for the rest of the procedure
 func enforceRequirements(flags *applyPlanFlags, args []string, dryRun bool, upgradeApply bool) (clientset.Interface, upgrade.VersionGetter, *kubeadmapi.InitConfiguration, error) {
 	client, err := getClient(flags.kubeConfigPath, dryRun)
@@ -62,21 +136,7 @@ func enforceRequirements(flags *applyPlanFlags, args []string, dryRun bool, upgr
 	fmt.Println("[upgrade/config] Making sure the configuration is correct:")
 
 	var newK8sVersion string
-	var cfg *kubeadmapi.InitConfiguration
-
-	if flags.cfgPath != "" {
-		klog.Warning("WARNING: Usage of the --config flag for reconfiguring the cluster during upgrade is not recommended!")
-		cfg, err = configutil.LoadInitConfigurationFromFile(flags.cfgPath)
-
-		// Initialize newK8sVersion to the value in the ClusterConfiguration. This is done, so that users who use the --config option
-		// don't have to specify the Kubernetes version twice if they don't want to upgrade, but just change a setting.
-		if err != nil {
-			newK8sVersion = cfg.KubernetesVersion
-		}
-	} else {
-		cfg, err = configutil.FetchInitConfigurationFromCluster(client, os.Stdout, "upgrade/config", false, !upgradeApply)
-	}
-
+	cfg, legacyReconfigure, err := loadConfig(flags.cfgPath, client, !upgradeApply)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			fmt.Printf("[upgrade/config] In order to upgrade, a ConfigMap called %q in the %s namespace must exist.\n", constants.KubeadmConfigConfigMap, metav1.NamespaceSystem)
@@ -90,6 +150,11 @@ func enforceRequirements(flags *applyPlanFlags, args []string, dryRun bool, upgr
 			err = errors.Errorf("the ConfigMap %q in the %s namespace used for getting configuration information was not found", constants.KubeadmConfigConfigMap, metav1.NamespaceSystem)
 		}
 		return nil, nil, nil, errors.Wrap(err, "[upgrade/config] FATAL")
+	} else if legacyReconfigure {
+		// Set the newK8sVersion to the value in the ClusterConfiguration. This is done, so that users who use the --config option
+		// to supply a new ClusterConfiguration don't have to specify the Kubernetes version twice,
+		// if they don't want to upgrade but just change a setting.
+		newK8sVersion = cfg.KubernetesVersion
 	}
 
 	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(flags.ignorePreflightErrors, cfg.NodeRegistration.IgnorePreflightErrors)

cmd/kubeadm/app/componentconfigs/configset.go

@@ -239,6 +239,49 @@ func FetchFromDocumentMap(clusterCfg *kubeadmapi.ClusterConfiguration, docmap ku
 	return nil
 }
 
+// FetchFromClusterWithLocalOverwrites fetches component configs from a cluster and overwrites them locally with
+// the ones present in the supplied document map. If any UnsupportedConfigVersionError are not handled by the configs
+// in the document map, the function returns them all as a single UnsupportedConfigVersionsErrorMap.
+// This function is normally called only in some specific cases during upgrade.
+func FetchFromClusterWithLocalOverwrites(clusterCfg *kubeadmapi.ClusterConfiguration, client clientset.Interface, docmap kubeadmapi.DocumentMap) error {
+	ensureInitializedComponentConfigs(clusterCfg)
+
+	oldVersionErrs := UnsupportedConfigVersionsErrorMap{}
+
+	for _, handler := range known {
+		componentCfg, err := handler.FromCluster(client, clusterCfg)
+		if err != nil {
+			if vererr, ok := err.(*UnsupportedConfigVersionError); ok {
+				oldVersionErrs[handler.GroupVersion.Group] = vererr
+			} else {
+				return err
+			}
+		} else if componentCfg != nil {
+			clusterCfg.ComponentConfigs[handler.GroupVersion.Group] = componentCfg
+		}
+	}
+
+	for _, handler := range known {
+		componentCfg, err := handler.FromDocumentMap(docmap)
+		if err != nil {
+			if vererr, ok := err.(*UnsupportedConfigVersionError); ok {
+				oldVersionErrs[handler.GroupVersion.Group] = vererr
+			} else {
+				return err
+			}
+		} else if componentCfg != nil {
+			clusterCfg.ComponentConfigs[handler.GroupVersion.Group] = componentCfg
+			delete(oldVersionErrs, handler.GroupVersion.Group)
+		}
+	}
+
+	if len(oldVersionErrs) != 0 {
+		return oldVersionErrs
+	}
+
+	return nil
+}
+
 // Validate is a placeholder for performing a validation on an already loaded component configs in a ClusterConfiguration
 // Currently it prints a warning that no validation was performed
 func Validate(clusterCfg *kubeadmapi.ClusterConfiguration) field.ErrorList {

cmd/kubeadm/app/componentconfigs/configset_test.go

@@ -110,3 +110,139 @@ func TestFetchFromDocumentMap(t *testing.T) {
 		t.Fatalf("missmatch between supplied and loaded type numbers:\n\tgot: %d\n\texpected: %d", len(clusterCfg.ComponentConfigs), len(gvkmap))
 	}
 }
+
+func kubeproxyConfigMap(contents string) *v1.ConfigMap {
+	return &v1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      constants.KubeProxyConfigMap,
+			Namespace: metav1.NamespaceSystem,
+		},
+		Data: map[string]string{
+			constants.KubeProxyConfigMapKey: dedent.Dedent(contents),
+		},
+	}
+}
+
+func TestFetchFromClusterWithLocalUpgrades(t *testing.T) {
+	cases := []struct {
+		desc          string
+		obj           runtime.Object
+		config        string
+		expectedValue string
+		expectedErr   bool
+	}{
+		{
+			desc: "reconginzed cluster object without overwrite is used",
+			obj: kubeproxyConfigMap(`
+				apiVersion: kubeproxy.config.k8s.io/v1alpha1
+				kind: KubeProxyConfiguration
+				hostnameOverride: foo
+			`),
+			expectedValue: "foo",
+		},
+		{
+			desc: "reconginzed cluster object with overwrite is not used",
+			obj: kubeproxyConfigMap(`
+				apiVersion: kubeproxy.config.k8s.io/v1alpha1
+				kind: KubeProxyConfiguration
+				hostnameOverride: foo
+			`),
+			config: dedent.Dedent(`
+				apiVersion: kubeproxy.config.k8s.io/v1alpha1
+				kind: KubeProxyConfiguration
+				hostnameOverride: bar
+			`),
+			expectedValue: "bar",
+		},
+		{
+			desc: "old config without overwrite returns an error",
+			obj: kubeproxyConfigMap(`
+				apiVersion: kubeproxy.config.k8s.io/v1alpha0
+				kind: KubeProxyConfiguration
+				hostnameOverride: foo
+			`),
+			expectedErr: true,
+		},
+		{
+			desc: "old config with recognized overwrite returns success",
+			obj: kubeproxyConfigMap(`
+				apiVersion: kubeproxy.config.k8s.io/v1alpha0
+				kind: KubeProxyConfiguration
+				hostnameOverride: foo
+			`),
+			config: dedent.Dedent(`
+				apiVersion: kubeproxy.config.k8s.io/v1alpha1
+				kind: KubeProxyConfiguration
+				hostnameOverride: bar
+			`),
+			expectedValue: "bar",
+		},
+		{
+			desc: "old config with old overwrite returns an error",
+			obj: kubeproxyConfigMap(`
+				apiVersion: kubeproxy.config.k8s.io/v1alpha0
+				kind: KubeProxyConfiguration
+				hostnameOverride: foo
+			`),
+			config: dedent.Dedent(`
+				apiVersion: kubeproxy.config.k8s.io/v1alpha0
+				kind: KubeProxyConfiguration
+				hostnameOverride: bar
+			`),
+			expectedErr: true,
+		},
+	}
+	for _, test := range cases {
+		t.Run(test.desc, func(t *testing.T) {
+			clusterCfg := &kubeadmapi.ClusterConfiguration{
+				KubernetesVersion: constants.CurrentKubernetesVersion.String(),
+			}
+
+			k8sVersion := version.MustParseGeneric(clusterCfg.KubernetesVersion)
+
+			client := clientsetfake.NewSimpleClientset(
+				test.obj,
+				&v1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      constants.GetKubeletConfigMapName(k8sVersion),
+						Namespace: metav1.NamespaceSystem,
+					},
+					Data: map[string]string{
+						constants.KubeletBaseConfigurationConfigMapKey: dedent.Dedent(`
+							apiVersion: kubelet.config.k8s.io/v1beta1
+							kind: KubeletConfiguration
+						`),
+					},
+				},
+			)
+
+			docmap, err := kubeadmutil.SplitYAMLDocuments([]byte(test.config))
+			if err != nil {
+				t.Fatalf("unexpected failure of SplitYAMLDocuments: %v", err)
+			}
+
+			err = FetchFromClusterWithLocalOverwrites(clusterCfg, client, docmap)
+			if err != nil {
+				if !test.expectedErr {
+					t.Errorf("unexpected failure: %v", err)
+				}
+			} else {
+				if test.expectedErr {
+					t.Error("unexpected success")
+				} else {
+					kubeproxyCfg, ok := clusterCfg.ComponentConfigs[KubeProxyGroup]
+					if !ok {
+						t.Error("the config was reported as loaded, but was not in reality")
+					} else {
+						actualConfig, ok := kubeproxyCfg.(*kubeProxyConfig)
+						if !ok {
+							t.Error("the config is not of the expected type")
+						} else if actualConfig.config.HostnameOverride != test.expectedValue {
+							t.Errorf("unexpected value:\n\tgot: %q\n\texpected: %q", actualConfig.config.HostnameOverride, test.expectedValue)
+						}
+					}
+				}
+			}
+		})
+	}
+}

cmd/kubeadm/app/componentconfigs/utils.go

@@ -18,6 +18,8 @@ package componentconfigs
 
 import (
 	"fmt"
+	"sort"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/klog/v2"
@@ -40,6 +42,27 @@ func (err *UnsupportedConfigVersionError) Error() string {
 	return fmt.Sprintf("unsupported apiVersion %q, you may have to do manual conversion to %q and run kubeadm again", err.OldVersion, err.CurrentVersion)
 }
 
+// UnsupportedConfigVersionsErrorMap is a cumulative version of the UnsupportedConfigVersionError type
+type UnsupportedConfigVersionsErrorMap map[string]*UnsupportedConfigVersionError
+
+// Error implements the standard Golang error interface for UnsupportedConfigVersionsErrorMap
+func (errs UnsupportedConfigVersionsErrorMap) Error() string {
+	// Make sure the error messages we print are predictable by sorting them by the group names involved
+	groups := make([]string, 0, len(errs))
+	for group := range errs {
+		groups = append(groups, group)
+	}
+	sort.Strings(groups)
+
+	msgs := make([]string, 1, 1+len(errs))
+	msgs[0] = "multiple unsupported config version errors encountered:"
+	for _, group := range groups {
+		msgs = append(msgs, errs[group].Error())
+	}
+
+	return strings.Join(msgs, "\n\t- ")
+}
+
 // warnDefaultComponentConfigValue prints a warning if the user modified a field in a certain
 // CompomentConfig from the default recommended value in kubeadm.
 func warnDefaultComponentConfigValue(componentConfigKind, paramName string, defaultValue, userValue interface{}) {