kubeadm: Remove the support of configurable component configs

`kubeadm upgrade plan` uses to support the configure of component
configs(kubeproxy and kubelet) in a config file and then check if
the version is supported or not, if it's not supported it will be
marked as a unsupported version and require to manually upgrade
the component.

This feature will make the upgrade config API much harder as this
violates the no-mutation principle for upgrade, and we have seen it's
quite problematic to do like this.

This change removes the support of configurable component configs for
`kubeadm upgrade plan`, along with the removal, the logic to parse
the config file to decide whether a manual upgrade for the component
configs is needed is removed as well.

NOTE that API is not changed, i.e. `ManualUpgradeRequired` is not removed
from `ComponentConfigVersionState` but it's no-op now.

Signed-off-by: Dave Chen <dave.chen@arm.com>

cmd/kubeadm/app/cmd/upgrade/plan.go

@@ -32,16 +32,13 @@ import (
 	"k8s.io/apimachinery/pkg/util/version"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"k8s.io/cli-runtime/pkg/printers"
-	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
 
-	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	outputapischeme "k8s.io/kubernetes/cmd/kubeadm/app/apis/output/scheme"
 	outputapiv1alpha2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/output/v1alpha2"
 	"k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/upgrade"
-	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	"k8s.io/kubernetes/cmd/kubeadm/app/util/output"
 )
 
@@ -268,7 +265,7 @@ func runPlan(flags *planFlags, args []string, printer output.Printer) error {
 
 	// Fetch the current state of the component configs
 	klog.V(1).Infoln("[upgrade/plan] analysing component config version states")
-	configVersionStates, err := getComponentConfigVersionStates(&cfg.ClusterConfiguration, client, flags.cfgPath)
+	configVersionStates, err := componentconfigs.GetVersionStates(&cfg.ClusterConfiguration, client)
 	if err != nil {
 		return errors.WithMessage(err, "[upgrade/versions] FATAL")
 	}
@@ -352,24 +349,6 @@ func genUpgradePlan(up *upgrade.Upgrade, isExternalEtcd bool) (*outputapiv1alpha
 	return &outputapiv1alpha2.UpgradePlan{Components: components}, unstableVersionFlag, nil
 }
 
-func getComponentConfigVersionStates(cfg *kubeadmapi.ClusterConfiguration, client clientset.Interface, cfgPath string) ([]outputapiv1alpha2.ComponentConfigVersionState, error) {
-	docmap := kubeadmapi.DocumentMap{}
-
-	if cfgPath != "" {
-		bytes, err := os.ReadFile(cfgPath)
-		if err != nil {
-			return nil, errors.Wrapf(err, "unable to read config file %q", cfgPath)
-		}
-
-		docmap, err = kubeadmutil.SplitYAMLDocuments(bytes)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	return componentconfigs.GetVersionStates(cfg, client, docmap)
-}
-
 // printUpgradePlan prints a UX-friendly overview of what versions are available to upgrade to
 func printUpgradePlan(up *upgrade.Upgrade, plan *outputapiv1alpha2.UpgradePlan, unstableVersionFlag string, isExternalEtcd bool, writer io.Writer, printer output.Printer) {
 	printHeader := true

cmd/kubeadm/app/componentconfigs/configset.go

@@ -289,38 +289,22 @@ func FetchFromClusterWithLocalOverwrites(clusterCfg *kubeadmapi.ClusterConfigura
 
 // GetVersionStates returns a slice of ComponentConfigVersionState structs
 // describing all supported component config groups that were identified on the cluster
-func GetVersionStates(clusterCfg *kubeadmapi.ClusterConfiguration, client clientset.Interface, docmap kubeadmapi.DocumentMap) ([]outputapiv1alpha2.ComponentConfigVersionState, error) {
+func GetVersionStates(clusterCfg *kubeadmapi.ClusterConfiguration, client clientset.Interface) ([]outputapiv1alpha2.ComponentConfigVersionState, error) {
 	// We don't want to modify clusterCfg so we make a working deep copy of it.
 	// Also, we don't want the defaulted component configs so we get rid of them.
 	scratchClusterCfg := clusterCfg.DeepCopy()
 	scratchClusterCfg.ComponentConfigs = kubeadmapi.ComponentConfigMap{}
 
-	// Call FetchFromClusterWithLocalOverwrites. This will populate the configs it can load and will return all
+	err := FetchFromCluster(scratchClusterCfg, client)
-	// UnsupportedConfigVersionError(s) in a sinle instance of a MultipleUnsupportedConfigVersionsError.
-	var multipleVerErrs UnsupportedConfigVersionsErrorMap
-	err := FetchFromClusterWithLocalOverwrites(scratchClusterCfg, client, docmap)
 	if err != nil {
-		if vererrs, ok := err.(UnsupportedConfigVersionsErrorMap); ok {
-			multipleVerErrs = vererrs
-		} else {
 		// This seems to be a genuine error so we end here
 		return nil, err
 	}
-	}
 
 	results := []outputapiv1alpha2.ComponentConfigVersionState{}
 	for _, handler := range known {
 		group := handler.GroupVersion.Group
-		if vererr, ok := multipleVerErrs[group]; ok {
+		if _, ok := scratchClusterCfg.ComponentConfigs[group]; ok {
-			// If there is an UnsupportedConfigVersionError then we are dealing with a case where the config was user
-			// supplied and requires manual upgrade
-			results = append(results, outputapiv1alpha2.ComponentConfigVersionState{
-				Group:                 group,
-				CurrentVersion:        vererr.OldVersion.Version,
-				PreferredVersion:      vererr.CurrentVersion.Version,
-				ManualUpgradeRequired: true,
-			})
-		} else if _, ok := scratchClusterCfg.ComponentConfigs[group]; ok {
 			// Normally loaded component config. No manual upgrade required on behalf of users.
 			results = append(results, outputapiv1alpha2.ComponentConfigVersionState{
 				Group:            group,

cmd/kubeadm/app/componentconfigs/fakeconfig_test.go

@@ -617,72 +617,30 @@ func TestGetVersionStates(t *testing.T) {
 			CurrentVersion:   currentClusterConfigVersion,
 			PreferredVersion: currentClusterConfigVersion,
 		}
-		versionStateOld := outputapiv1alpha2.ComponentConfigVersionState{
-			Group:                 kubeadmapiv1.GroupName,
-			CurrentVersion:        oldClusterConfigVersion,
-			PreferredVersion:      currentClusterConfigVersion,
-			ManualUpgradeRequired: true,
-		}
 
 		cases := []struct {
 			desc        string
 			obj         runtime.Object
-			config   string
+			expectedErr bool
 			expected    outputapiv1alpha2.ComponentConfigVersionState
 		}{
 			{
-				desc:     "appropriate cluster object without overwrite",
+				desc:     "appropriate cluster object",
 				obj:      testClusterConfigMap(currentFooClusterConfig, false),
 				expected: versionStateCurrent,
 			},
 			{
-				desc:     "appropriate cluster object with appropriate overwrite",
+				desc:        "old config returns an error",
-				obj:      testClusterConfigMap(currentFooClusterConfig, false),
-				config:   dedent.Dedent(currentBarClusterConfig),
-				expected: versionStateCurrent,
-			},
-			{
-				desc:     "appropriate cluster object with old overwrite",
-				obj:      testClusterConfigMap(currentFooClusterConfig, false),
-				config:   dedent.Dedent(oldBarClusterConfig),
-				expected: versionStateOld,
-			},
-			{
-				desc:     "old config without overwrite returns an error",
 				obj:         testClusterConfigMap(oldFooClusterConfig, false),
-				expected: versionStateOld,
+				expectedErr: true,
 			},
 			{
-				desc:     "old config with appropriate overwrite",
+				desc:     "appropriate signed cluster object",
-				obj:      testClusterConfigMap(oldFooClusterConfig, false),
-				config:   dedent.Dedent(currentBarClusterConfig),
-				expected: versionStateCurrent,
-			},
-			{
-				desc:     "old config with old overwrite",
-				obj:      testClusterConfigMap(oldFooClusterConfig, false),
-				config:   dedent.Dedent(oldBarClusterConfig),
-				expected: versionStateOld,
-			},
-			{
-				desc:     "appropriate signed cluster object without overwrite",
 				obj:      testClusterConfigMap(currentFooClusterConfig, true),
 				expected: versionStateCurrent,
 			},
 			{
-				desc:     "appropriate signed cluster object with appropriate overwrite",
+				desc: "old signed config",
-				obj:      testClusterConfigMap(currentFooClusterConfig, true),
-				config:   dedent.Dedent(currentBarClusterConfig),
-				expected: versionStateCurrent,
-			},
-			{
-				desc:     "appropriate signed cluster object with old overwrit",
-				obj:      testClusterConfigMap(currentFooClusterConfig, true),
-				config:   dedent.Dedent(oldBarClusterConfig),
-				expected: versionStateOld,
-			},
-			{
-				desc: "old signed config without an overwrite",
 				obj:  testClusterConfigMap(oldFooClusterConfig, true),
 				expected: outputapiv1alpha2.ComponentConfigVersionState{
 					Group:            kubeadmapiv1.GroupName,
@@ -690,39 +648,28 @@ func TestGetVersionStates(t *testing.T) {
 					PreferredVersion: currentClusterConfigVersion,
 				},
 			},
-			{
-				desc:     "old signed config with appropriate overwrite",
-				obj:      testClusterConfigMap(oldFooClusterConfig, true),
-				config:   dedent.Dedent(currentBarClusterConfig),
-				expected: versionStateCurrent,
-			},
-			{
-				desc:     "old signed config with old overwrite",
-				obj:      testClusterConfigMap(oldFooClusterConfig, true),
-				config:   dedent.Dedent(oldBarClusterConfig),
-				expected: versionStateOld,
-			},
 		}
 
 		for _, test := range cases {
 			t.Run(test.desc, func(t *testing.T) {
 				client := clientsetfake.NewSimpleClientset(test.obj)
 
-				docmap, err := kubeadmutil.SplitYAMLDocuments([]byte(test.config))
-				if err != nil {
-					t.Fatalf("unexpected failure of SplitYAMLDocuments: %v", err)
-				}
-
 				clusterCfg := testClusterCfg()
 
-				got, err := GetVersionStates(clusterCfg, client, docmap)
+				got, err := GetVersionStates(clusterCfg, client)
-				if err != nil {
+				if err != nil && !test.expectedErr {
 					t.Errorf("unexpected error: %v", err)
-				} else if len(got) != 1 {
+				}
+				if err == nil {
+					if test.expectedErr {
+						t.Errorf("expected error not found: %v", test.expectedErr)
+					}
+					if len(got) != 1 {
 						t.Errorf("got %d, but expected only a single result: %v", len(got), got)
 					} else if got[0] != test.expected {
 						t.Errorf("unexpected result:\n\texpected: %v\n\tgot: %v", test.expected, got[0])
 					}
+				}
 			})
 		}
 	})