Merge pull request #121290 from pacoxu/revert-119946-ignore_preflight_error
Revert "kubeadm: FeatureGate MergeCLIArgumentsWithConfig is added for ignorePreflightErrors"
This commit is contained in:
		@@ -650,21 +650,15 @@ func ValidateCertificateKey(certificateKey string, fldPath *field.Path) field.Er
 | 
			
		||||
// ValidateIgnorePreflightErrors validates duplicates in:
 | 
			
		||||
// - ignore-preflight-errors flag and
 | 
			
		||||
// - ignorePreflightErrors field in {Init,Join}Configuration files.
 | 
			
		||||
// and make sure ignore "all" cannot configured with individual checks.
 | 
			
		||||
func ValidateIgnorePreflightErrors(featureList map[string]bool, ignorePreflightErrorsFromCLI, ignorePreflightErrorsFromConfigFile []string) (sets.Set[string], error) {
 | 
			
		||||
func ValidateIgnorePreflightErrors(ignorePreflightErrorsFromCLI, ignorePreflightErrorsFromConfigFile []string) (sets.Set[string], error) {
 | 
			
		||||
	ignoreErrors := sets.New[string]()
 | 
			
		||||
	allErrs := field.ErrorList{}
 | 
			
		||||
	ignorePreflightErr := ignorePreflightErrorsFromConfigFile
 | 
			
		||||
 | 
			
		||||
	if ignorePreflightErrorsFromCLI != nil {
 | 
			
		||||
		if features.Enabled(featureList, features.MergeCLIArgumentsWithConfig) {
 | 
			
		||||
			ignorePreflightErr = append(ignorePreflightErr, ignorePreflightErrorsFromCLI...)
 | 
			
		||||
		} else {
 | 
			
		||||
			ignorePreflightErr = ignorePreflightErrorsFromCLI
 | 
			
		||||
		}
 | 
			
		||||
	for _, item := range ignorePreflightErrorsFromConfigFile {
 | 
			
		||||
		ignoreErrors.Insert(strings.ToLower(item)) // parameters are case insensitive
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	for _, item := range ignorePreflightErr {
 | 
			
		||||
	for _, item := range ignorePreflightErrorsFromCLI {
 | 
			
		||||
		ignoreErrors.Insert(strings.ToLower(item)) // parameters are case insensitive
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -29,7 +29,6 @@ import (
 | 
			
		||||
 | 
			
		||||
	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 | 
			
		||||
	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3"
 | 
			
		||||
	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
func TestValidateToken(t *testing.T) {
 | 
			
		||||
@@ -806,14 +805,17 @@ func TestValidateIgnorePreflightErrors(t *testing.T) {
 | 
			
		||||
		ignorePreflightErrorsFromConfigFile []string
 | 
			
		||||
		expectedSet                         sets.Set[string]
 | 
			
		||||
		expectedError                       bool
 | 
			
		||||
		mergeCLIArgumentsWithConfig         bool
 | 
			
		||||
	}{
 | 
			
		||||
		// FG is off
 | 
			
		||||
		{ // empty lists in CLI and config file
 | 
			
		||||
			[]string{},
 | 
			
		||||
			[]string{},
 | 
			
		||||
			sets.New[string](),
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // empty list in CLI only
 | 
			
		||||
			[]string{},
 | 
			
		||||
			[]string{"a"},
 | 
			
		||||
			sets.New("a"),
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // empty list in config file only
 | 
			
		||||
@@ -821,103 +823,64 @@ func TestValidateIgnorePreflightErrors(t *testing.T) {
 | 
			
		||||
			[]string{},
 | 
			
		||||
			sets.New("a"),
 | 
			
		||||
			false,
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // no duplicates, no overlap
 | 
			
		||||
			[]string{"a", "b"},
 | 
			
		||||
			[]string{"c", "d"},
 | 
			
		||||
			sets.New("a", "b"),
 | 
			
		||||
			false,
 | 
			
		||||
			sets.New("a", "b", "c", "d"),
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // some duplicates, with some overlapping duplicates
 | 
			
		||||
			[]string{"a", "b", "a"},
 | 
			
		||||
			[]string{"c", "b"},
 | 
			
		||||
			sets.New("a", "b"),
 | 
			
		||||
			false,
 | 
			
		||||
			sets.New("a", "b", "c"),
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // CLI is not set, but 'all' present in config file
 | 
			
		||||
			nil,
 | 
			
		||||
		{ // empty list in CLI, but 'all' present in config file
 | 
			
		||||
			[]string{},
 | 
			
		||||
			[]string{"all"},
 | 
			
		||||
			sets.New("all"),
 | 
			
		||||
			false,
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // empty list in config file, but 'all' present in CLI
 | 
			
		||||
			[]string{"all"},
 | 
			
		||||
			[]string{},
 | 
			
		||||
			sets.New("all"),
 | 
			
		||||
			false,
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // some duplicates, only 'all' present in CLI and config file
 | 
			
		||||
			[]string{"all"},
 | 
			
		||||
			[]string{"all"},
 | 
			
		||||
			sets.New("all"),
 | 
			
		||||
			false,
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // non-duplicate, but 'all' present together with individual checks in CLI
 | 
			
		||||
			[]string{"a", "b", "all"},
 | 
			
		||||
			[]string{},
 | 
			
		||||
			sets.New[string](),
 | 
			
		||||
			true,
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // non-duplicate, but 'all' present together with individual checks in config file
 | 
			
		||||
			nil,
 | 
			
		||||
			[]string{},
 | 
			
		||||
			[]string{"a", "b", "all"},
 | 
			
		||||
			sets.New[string](),
 | 
			
		||||
			true,
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // non-duplicate, but 'all' present in config file, while values are in CLI, "all" from config file will be ignored
 | 
			
		||||
		{ // non-duplicate, but 'all' present in config file, while values are in CLI, which is forbidden
 | 
			
		||||
			[]string{"a", "b"},
 | 
			
		||||
			[]string{"all"},
 | 
			
		||||
			sets.New[string]("a", "b"),
 | 
			
		||||
			false,
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // non-duplicate, but 'all' present in CLI, while values are in config file, values from config file will be ignored
 | 
			
		||||
			[]string{"all"},
 | 
			
		||||
			[]string{"a", "b"},
 | 
			
		||||
			sets.New[string]("all"),
 | 
			
		||||
			false,
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // set from CLI will take precedence of the config file
 | 
			
		||||
			[]string{"a", "b"},
 | 
			
		||||
			[]string{"c", "d"},
 | 
			
		||||
			sets.New[string]("a", "b"),
 | 
			
		||||
			false,
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		{ // empty list in CLI only
 | 
			
		||||
			[]string{},
 | 
			
		||||
			[]string{"a"},
 | 
			
		||||
			sets.New[string](),
 | 
			
		||||
			false,
 | 
			
		||||
			false,
 | 
			
		||||
			true,
 | 
			
		||||
		},
 | 
			
		||||
		{ // flag from CLI is not set
 | 
			
		||||
			nil,
 | 
			
		||||
			[]string{"c", "d"},
 | 
			
		||||
			sets.New[string]("c", "d"),
 | 
			
		||||
			false,
 | 
			
		||||
			false,
 | 
			
		||||
		},
 | 
			
		||||
		// FG is on
 | 
			
		||||
		{
 | 
			
		||||
		{ // non-duplicate, but 'all' present in CLI, while values are in config file, which is forbidden
 | 
			
		||||
			[]string{"all"},
 | 
			
		||||
			[]string{"a", "b"},
 | 
			
		||||
			[]string{"c", "d"},
 | 
			
		||||
			sets.New[string]("a", "b", "c", "d"),
 | 
			
		||||
			false,
 | 
			
		||||
			sets.New[string](),
 | 
			
		||||
			true,
 | 
			
		||||
		},
 | 
			
		||||
	}
 | 
			
		||||
	for _, rt := range tests {
 | 
			
		||||
		result, err := ValidateIgnorePreflightErrors(map[string]bool{features.MergeCLIArgumentsWithConfig: rt.mergeCLIArgumentsWithConfig}, rt.ignorePreflightErrorsFromCLI, rt.ignorePreflightErrorsFromConfigFile)
 | 
			
		||||
		result, err := ValidateIgnorePreflightErrors(rt.ignorePreflightErrorsFromCLI, rt.ignorePreflightErrorsFromConfigFile)
 | 
			
		||||
		switch {
 | 
			
		||||
		case err != nil && !rt.expectedError:
 | 
			
		||||
			t.Errorf("ValidateIgnorePreflightErrors: unexpected error for input (%s, %s), error: %v", rt.ignorePreflightErrorsFromCLI, rt.ignorePreflightErrorsFromConfigFile, err)
 | 
			
		||||
 
 | 
			
		||||
@@ -309,7 +309,7 @@ func newInitData(cmd *cobra.Command, args []string, initOptions *initOptions, ou
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(cfg.FeatureGates, initOptions.ignorePreflightErrors, cfg.NodeRegistration.IgnorePreflightErrors)
 | 
			
		||||
	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(initOptions.ignorePreflightErrors, cfg.NodeRegistration.IgnorePreflightErrors)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
 
 | 
			
		||||
@@ -181,7 +181,7 @@ func TestNewInitData(t *testing.T) {
 | 
			
		||||
				options.CfgPath:               configFilePath,
 | 
			
		||||
				options.IgnorePreflightErrors: "a,b",
 | 
			
		||||
			},
 | 
			
		||||
			validate: expectedInitIgnorePreflightErrors("a", "b"),
 | 
			
		||||
			validate: expectedInitIgnorePreflightErrors("a", "b", "c", "d"),
 | 
			
		||||
		},
 | 
			
		||||
	}
 | 
			
		||||
	for _, tc := range testCases {
 | 
			
		||||
 
 | 
			
		||||
@@ -226,7 +226,7 @@ func newCmdJoin(out io.Writer, joinOptions *joinOptions) *cobra.Command {
 | 
			
		||||
			// assume that the command execution does not depend on CRISocket when --cri-socket flag is not set
 | 
			
		||||
			joinOptions.skipCRIDetect = true
 | 
			
		||||
		}
 | 
			
		||||
		data, err := newJoinData(cmd, args, joinOptions, out, kubeadmconstants.GetAdminKubeConfigPath(), false)
 | 
			
		||||
		data, err := newJoinData(cmd, args, joinOptions, out, kubeadmconstants.GetAdminKubeConfigPath())
 | 
			
		||||
		if err != nil {
 | 
			
		||||
			return nil, err
 | 
			
		||||
		}
 | 
			
		||||
@@ -335,10 +335,8 @@ func newJoinOptions() *joinOptions {
 | 
			
		||||
 | 
			
		||||
// newJoinData returns a new joinData struct to be used for the execution of the kubeadm join workflow.
 | 
			
		||||
// This func takes care of validating joinOptions passed to the command, and then it converts
 | 
			
		||||
// options into the internal JoinConfiguration type that is used as input all the phases in the kubeadm join workflow.
 | 
			
		||||
// Set the lazilyFetchInitCfg to true to lazily fetch the InitConfiguration from cluster, e.g. unittest to test joinData
 | 
			
		||||
// without the interaction with cluster.
 | 
			
		||||
func newJoinData(cmd *cobra.Command, args []string, opt *joinOptions, out io.Writer, adminKubeConfigPath string, lazilyFetchInitCfg bool) (*joinData, error) {
 | 
			
		||||
// options into the internal JoinConfiguration type that is used as input all the phases in the kubeadm join workflow
 | 
			
		||||
func newJoinData(cmd *cobra.Command, args []string, opt *joinOptions, out io.Writer, adminKubeConfigPath string) (*joinData, error) {
 | 
			
		||||
 | 
			
		||||
	// Validate the mixed arguments with --config and return early on errors
 | 
			
		||||
	if err := validation.ValidateMixedArguments(cmd.Flags()); err != nil {
 | 
			
		||||
@@ -436,18 +434,7 @@ func newJoinData(cmd *cobra.Command, args []string, opt *joinOptions, out io.Wri
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	initCfg := &kubeadmapi.InitConfiguration{}
 | 
			
		||||
	if !lazilyFetchInitCfg {
 | 
			
		||||
		if tlsBootstrapCfg == nil {
 | 
			
		||||
			if tlsBootstrapCfg, err = discovery.For(cfg); err != nil {
 | 
			
		||||
				return nil, err
 | 
			
		||||
			}
 | 
			
		||||
		}
 | 
			
		||||
		if initCfg, err = fetchInitConfigurationFromJoinConfiguration(cfg, tlsBootstrapCfg); err != nil {
 | 
			
		||||
			return nil, err
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(initCfg.FeatureGates, opt.ignorePreflightErrors, cfg.NodeRegistration.IgnorePreflightErrors)
 | 
			
		||||
	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(opt.ignorePreflightErrors, cfg.NodeRegistration.IgnorePreflightErrors)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
@@ -479,7 +466,6 @@ func newJoinData(cmd *cobra.Command, args []string, opt *joinOptions, out io.Wri
 | 
			
		||||
	return &joinData{
 | 
			
		||||
		cfg:                   cfg,
 | 
			
		||||
		dryRun:                cmdutil.ValueFromFlagsOrConfig(cmd.Flags(), options.DryRun, cfg.DryRun, opt.dryRun).(bool),
 | 
			
		||||
		initCfg:               initCfg,
 | 
			
		||||
		tlsBootstrapCfg:       tlsBootstrapCfg,
 | 
			
		||||
		ignorePreflightErrors: ignorePreflightErrorsSet,
 | 
			
		||||
		outputWriter:          out,
 | 
			
		||||
 
 | 
			
		||||
@@ -289,7 +289,7 @@ func TestNewJoinData(t *testing.T) {
 | 
			
		||||
				options.CfgPath:               configFilePath,
 | 
			
		||||
				options.IgnorePreflightErrors: "a,b",
 | 
			
		||||
			},
 | 
			
		||||
			validate: expectedJoinIgnorePreflightErrors(sets.New("a", "b")),
 | 
			
		||||
			validate: expectedJoinIgnorePreflightErrors(sets.New("a", "b", "c", "d")),
 | 
			
		||||
		},
 | 
			
		||||
		{
 | 
			
		||||
			name: "warn if --control-plane flag is not set",
 | 
			
		||||
@@ -335,7 +335,7 @@ func TestNewJoinData(t *testing.T) {
 | 
			
		||||
			}
 | 
			
		||||
 | 
			
		||||
			// test newJoinData method
 | 
			
		||||
			data, err := newJoinData(cmd, tc.args, joinOptions, nil, kubeconfigFilePath, true)
 | 
			
		||||
			data, err := newJoinData(cmd, tc.args, joinOptions, nil, kubeconfigFilePath)
 | 
			
		||||
			klog.Flush()
 | 
			
		||||
			msg := "WARNING: --control-plane is also required when passing control-plane"
 | 
			
		||||
			if tc.expectWarn {
 | 
			
		||||
 
 | 
			
		||||
@@ -105,7 +105,7 @@ func newResetData(cmd *cobra.Command, opts *resetOptions, in io.Reader, out io.W
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	initCfg := &kubeadmapi.InitConfiguration{}
 | 
			
		||||
	var initCfg *kubeadmapi.InitConfiguration
 | 
			
		||||
 | 
			
		||||
	// Either use the config file if specified, or convert public kubeadm API to the internal ResetConfiguration and validates cfg.
 | 
			
		||||
	resetCfg, err := configutil.LoadOrDefaultResetConfiguration(opts.cfgPath, opts.externalcfg, configutil.LoadOrDefaultConfigurationOptions{
 | 
			
		||||
@@ -127,7 +127,7 @@ func newResetData(cmd *cobra.Command, opts *resetOptions, in io.Reader, out io.W
 | 
			
		||||
		klog.V(1).Infof("[reset] Could not obtain a client set from the kubeconfig file: %s", opts.kubeconfigPath)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(initCfg.FeatureGates, opts.ignorePreflightErrors, resetCfg.IgnorePreflightErrors)
 | 
			
		||||
	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(opts.ignorePreflightErrors, resetCfg.IgnorePreflightErrors)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
 
 | 
			
		||||
@@ -209,7 +209,7 @@ func TestNewResetData(t *testing.T) {
 | 
			
		||||
				options.CfgPath:               configFilePath,
 | 
			
		||||
				options.IgnorePreflightErrors: "c,d",
 | 
			
		||||
			},
 | 
			
		||||
			validate: expectedResetIgnorePreflightErrors(sets.New("c", "d")),
 | 
			
		||||
			validate: expectedResetIgnorePreflightErrors(sets.New("a", "b", "c", "d")),
 | 
			
		||||
		},
 | 
			
		||||
	}
 | 
			
		||||
	for _, tc := range testCases {
 | 
			
		||||
 
 | 
			
		||||
@@ -175,7 +175,7 @@ func enforceRequirements(flags *applyPlanFlags, args []string, dryRun bool, upgr
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(cfg.FeatureGates, flags.ignorePreflightErrors, cfg.NodeRegistration.IgnorePreflightErrors)
 | 
			
		||||
	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(flags.ignorePreflightErrors, cfg.NodeRegistration.IgnorePreflightErrors)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, nil, nil, err
 | 
			
		||||
	}
 | 
			
		||||
 
 | 
			
		||||
@@ -153,7 +153,7 @@ func newNodeData(cmd *cobra.Command, args []string, options *nodeOptions, out io
 | 
			
		||||
		return nil, errors.Wrap(err, "unable to fetch the kubeadm-config ConfigMap")
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(cfg.FeatureGates, options.ignorePreflightErrors, cfg.NodeRegistration.IgnorePreflightErrors)
 | 
			
		||||
	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(options.ignorePreflightErrors, cfg.NodeRegistration.IgnorePreflightErrors)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
 
 | 
			
		||||
@@ -38,8 +38,6 @@ const (
 | 
			
		||||
	EtcdLearnerMode = "EtcdLearnerMode"
 | 
			
		||||
	// UpgradeAddonsBeforeControlPlane is expected to be in deprecated in v1.28 and will be removed in future release
 | 
			
		||||
	UpgradeAddonsBeforeControlPlane = "UpgradeAddonsBeforeControlPlane"
 | 
			
		||||
	// MergeCLIArgumentsWithConfig is considered to be in deprecated in v1.29 and is expected to be removed in 1.30
 | 
			
		||||
	MergeCLIArgumentsWithConfig = "MergeCLIArgumentsWithConfig"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
// InitFeatureGates are the default feature gates for the init command
 | 
			
		||||
@@ -50,10 +48,6 @@ var InitFeatureGates = FeatureList{
 | 
			
		||||
	},
 | 
			
		||||
	RootlessControlPlane: {FeatureSpec: featuregate.FeatureSpec{Default: false, PreRelease: featuregate.Alpha}},
 | 
			
		||||
	EtcdLearnerMode:      {FeatureSpec: featuregate.FeatureSpec{Default: true, PreRelease: featuregate.Beta}},
 | 
			
		||||
	MergeCLIArgumentsWithConfig: {
 | 
			
		||||
		FeatureSpec:        featuregate.FeatureSpec{Default: false, PreRelease: featuregate.Deprecated},
 | 
			
		||||
		DeprecationMessage: "The MergeCLIArgumentsWithConfig feature gate is deprecated and will be removed in a future release.",
 | 
			
		||||
	},
 | 
			
		||||
	UpgradeAddonsBeforeControlPlane: {
 | 
			
		||||
		FeatureSpec:        featuregate.FeatureSpec{Default: false, PreRelease: featuregate.Deprecated},
 | 
			
		||||
		DeprecationMessage: "The UpgradeAddonsBeforeControlPlane feature gate is deprecated and will be removed in a future release.",
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user