Merge pull request #105924 from Huang-Wei/revert-105712

Revert "sched: ensure --leader-elect* CLI args are honored"

cmd/kube-scheduler/app/options/options.go

@@ -67,26 +67,17 @@ type Options struct {
 	WriteConfigTo string
 
 	Master string
-
-	// flags hold the parsed CLI flags.
-	flags *cliflag.NamedFlagSets
 }
 
 // NewOptions returns default scheduler app options.
 func NewOptions() (*Options, error) {
-	cfg, err := latest.Default()
-	if err != nil {
-		return nil, err
-	}
-
 	o := &Options{
-		ComponentConfig: *cfg,
-		SecureServing:   apiserveroptions.NewSecureServingOptions().WithLoopback(),
-		Authentication:  apiserveroptions.NewDelegatingAuthenticationOptions(),
-		Authorization:   apiserveroptions.NewDelegatingAuthorizationOptions(),
-		Deprecated:      &DeprecatedOptions{},
-		Metrics:         metrics.NewOptions(),
-		Logs:            logs.NewOptions(),
+		SecureServing:  apiserveroptions.NewSecureServingOptions().WithLoopback(),
+		Authentication: apiserveroptions.NewDelegatingAuthenticationOptions(),
+		Authorization:  apiserveroptions.NewDelegatingAuthorizationOptions(),
+		Deprecated:     &DeprecatedOptions{},
+		Metrics:        metrics.NewOptions(),
+		Logs:           logs.NewOptions(),
 	}
 
 	o.Authentication.TolerateInClusterLookupFailure = true
@@ -101,11 +92,42 @@ func NewOptions() (*Options, error) {
 	return o, nil
 }
 
-// Complete obtains the CLI args related with leaderelection, and override the values in `cfg`.
-// Then the `cfg` object is injected into the `options` object.
-func (o *Options) Complete(cfg *kubeschedulerconfig.KubeSchedulerConfiguration) {
-	// Obtain CLI args related with leaderelection. Set them to `cfg` if specified in command line.
-	leaderelection := o.Flags().FlagSet("leader election")
+// Complete completes the remaining instantiation of the options obj.
+// In particular, it injects the latest internal versioned ComponentConfig.
+func (o *Options) Complete(nfs *cliflag.NamedFlagSets) error {
+	cfg, err := latest.Default()
+	if err != nil {
+		return err
+	}
+
+	// Obtain deprecated CLI args. Set them to cfg if specified in command line.
+	deprecated := nfs.FlagSet("deprecated")
+	if deprecated.Changed("profiling") {
+		cfg.EnableProfiling = o.ComponentConfig.EnableProfiling
+	}
+	if deprecated.Changed("contention-profiling") {
+		cfg.EnableContentionProfiling = o.ComponentConfig.EnableContentionProfiling
+	}
+	if deprecated.Changed("kubeconfig") {
+		cfg.ClientConnection.Kubeconfig = o.ComponentConfig.ClientConnection.Kubeconfig
+	}
+	if deprecated.Changed("kube-api-content-type") {
+		cfg.ClientConnection.ContentType = o.ComponentConfig.ClientConnection.ContentType
+	}
+	if deprecated.Changed("kube-api-qps") {
+		cfg.ClientConnection.QPS = o.ComponentConfig.ClientConnection.QPS
+	}
+	if deprecated.Changed("kube-api-burst") {
+		cfg.ClientConnection.Burst = o.ComponentConfig.ClientConnection.Burst
+	}
+	if deprecated.Changed("lock-object-namespace") {
+		cfg.LeaderElection.ResourceNamespace = o.ComponentConfig.LeaderElection.ResourceNamespace
+	}
+	if deprecated.Changed("lock-object-name") {
+		cfg.LeaderElection.ResourceName = o.ComponentConfig.LeaderElection.ResourceName
+	}
+	// Obtain CLI args related with leaderelection. Set them to cfg if specified in command line.
+	leaderelection := nfs.FlagSet("leader election")
 	if leaderelection.Changed("leader-elect") {
 		cfg.LeaderElection.LeaderElect = o.ComponentConfig.LeaderElection.LeaderElect
 	}
@@ -129,24 +151,17 @@ func (o *Options) Complete(cfg *kubeschedulerconfig.KubeSchedulerConfiguration)
 	}
 
 	o.ComponentConfig = *cfg
+	return nil
 }
 
 // Flags returns flags for a specific scheduler by section name
-func (o *Options) Flags() *cliflag.NamedFlagSets {
-	if o.flags != nil {
-		return o.flags
-	}
-
-	nfs := cliflag.NamedFlagSets{}
+func (o *Options) Flags() (nfs cliflag.NamedFlagSets) {
 	fs := nfs.FlagSet("misc")
 	fs.StringVar(&o.ConfigFile, "config", o.ConfigFile, "The path to the configuration file.")
 	fs.StringVar(&o.WriteConfigTo, "write-config-to", o.WriteConfigTo, "If set, write the configuration values to this file and exit.")
 	fs.StringVar(&o.Master, "master", o.Master, "The address of the Kubernetes API server (overrides any value in kubeconfig)")
 
-	// In some tests, o.SecureServing can be nil.
-	if o.SecureServing != nil {
-		o.SecureServing.AddFlags(nfs.FlagSet("secure serving"))
-	}
+	o.SecureServing.AddFlags(nfs.FlagSet("secure serving"))
 	o.Authentication.AddFlags(nfs.FlagSet("authentication"))
 	o.Authorization.AddFlags(nfs.FlagSet("authorization"))
 	o.Deprecated.AddFlags(nfs.FlagSet("deprecated"), &o.ComponentConfig)
@@ -156,8 +171,7 @@ func (o *Options) Flags() *cliflag.NamedFlagSets {
 	o.Metrics.AddFlags(nfs.FlagSet("metrics"))
 	o.Logs.AddFlags(nfs.FlagSet("logs"))
 
-	o.flags = &nfs
-	return o.flags
+	return nfs
 }
 
 // ApplyTo applies the scheduler options to the given scheduler app configuration.
@@ -169,9 +183,6 @@ func (o *Options) ApplyTo(c *schedulerappconfig.Config) error {
 		if err != nil {
 			return err
 		}
-		// Honor the CLI args before assigning `cfg` to `c.ComponentConfig`.
-		o.Complete(cfg)
-
 		if err := validation.ValidateKubeSchedulerConfiguration(cfg); err != nil {
 			return err
 		}

cmd/kube-scheduler/app/server.go

@@ -69,6 +69,7 @@ func NewSchedulerCommand(registryOptions ...Option) *cobra.Command {
 		os.Exit(1)
 	}
 
+	namedFlagSets := opts.Flags()
 	cmd := &cobra.Command{
 		Use: "kube-scheduler",
 		Long: `The Kubernetes scheduler is a control plane process which assigns
@@ -80,6 +81,9 @@ kube-scheduler is the reference implementation.
 See [scheduling](https://kubernetes.io/docs/concepts/scheduling-eviction/)
 for more information about scheduling and the kube-scheduler component.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
+			if err := opts.Complete(&namedFlagSets); err != nil {
+				return err
+			}
 			return runCommand(cmd, opts, registryOptions...)
 		},
 		Args: func(cmd *cobra.Command, args []string) error {
@@ -93,7 +97,6 @@ for more information about scheduling and the kube-scheduler component.`,
 	}
 
 	fs := cmd.Flags()
-	namedFlagSets := opts.Flags()
 	verflag.AddFlags(namedFlagSets.FlagSet("global"))
 	globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name())
 	for _, f := range namedFlagSets.FlagSets {
@@ -101,7 +104,7 @@ for more information about scheduling and the kube-scheduler component.`,
 	}
 
 	cols, _, _ := term.TerminalSize(cmd.OutOrStdout())
-	cliflag.SetUsageAndHelpFunc(cmd, *namedFlagSets, cols)
+	cliflag.SetUsageAndHelpFunc(cmd, namedFlagSets, cols)
 
 	cmd.MarkFlagFilename("config", "yaml", "yml", "json")
 

cmd/kube-scheduler/app/server_test.go

@@ -26,13 +26,9 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
-	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/spf13/pflag"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	componentbaseconfig "k8s.io/component-base/config"
-	"k8s.io/kube-scheduler/config/v1beta3"
 	"k8s.io/kubernetes/cmd/kube-scheduler/app/options"
 	"k8s.io/kubernetes/pkg/scheduler/apis/config"
 	"k8s.io/kubernetes/pkg/scheduler/apis/config/testing/defaults"
@@ -143,35 +139,10 @@ profiles:
 		t.Fatal(err)
 	}
 
-	// empty leader-election config
-	emptyLeaderElectionConfig := filepath.Join(tmpDir, "empty-leader-election-config.yaml")
-	if err := ioutil.WriteFile(emptyLeaderElectionConfig, []byte(fmt.Sprintf(`
-apiVersion: kubescheduler.config.k8s.io/v1beta3
-kind: KubeSchedulerConfiguration
-clientConnection:
-  kubeconfig: "%s"
-`, configKubeconfig)), os.FileMode(0600)); err != nil {
-		t.Fatal(err)
-	}
-
-	// leader-election config
-	leaderElectionConfig := filepath.Join(tmpDir, "leader-election-config.yaml")
-	if err := ioutil.WriteFile(leaderElectionConfig, []byte(fmt.Sprintf(`
-apiVersion: kubescheduler.config.k8s.io/v1beta3
-kind: KubeSchedulerConfiguration
-clientConnection:
-  kubeconfig: "%s"
-leaderElection:
-  leaseDuration: 1h
-`, configKubeconfig)), os.FileMode(0600)); err != nil {
-		t.Fatal(err)
-	}
-
 	testcases := []struct {
-		name               string
-		flags              []string
-		wantPlugins        map[string]*config.Plugins
-		wantLeaderElection *componentbaseconfig.LeaderElectionConfiguration
+		name        string
+		flags       []string
+		wantPlugins map[string]*config.Plugins
 	}{
 		{
 			name: "default config",
@@ -222,75 +193,6 @@ leaderElection:
 				},
 			},
 		},
-		{
-			name: "leader election CLI args, along with --config arg",
-			flags: []string{
-				"--leader-elect=false",
-				"--leader-elect-lease-duration=2h", // CLI args are favored over the fields in ComponentConfig
-				"--lock-object-namespace=default",  // deprecated CLI arg will be ignored if --config is specified
-				"--config", emptyLeaderElectionConfig,
-			},
-			wantLeaderElection: &componentbaseconfig.LeaderElectionConfiguration{
-				LeaderElect:       false,                                    // from CLI args
-				LeaseDuration:     metav1.Duration{Duration: 2 * time.Hour}, // from CLI args
-				RenewDeadline:     metav1.Duration{Duration: 10 * time.Second},
-				RetryPeriod:       metav1.Duration{Duration: 2 * time.Second},
-				ResourceLock:      "leases",
-				ResourceName:      v1beta3.SchedulerDefaultLockObjectName,
-				ResourceNamespace: v1beta3.SchedulerDefaultLockObjectNamespace,
-			},
-		},
-		{
-			name: "leader election CLI args, without --config arg",
-			flags: []string{
-				"--leader-elect=false",
-				"--leader-elect-lease-duration=2h",
-				"--lock-object-namespace=default", // deprecated CLI arg is honored if --config is not specified
-				"--kubeconfig", configKubeconfig,
-			},
-			wantLeaderElection: &componentbaseconfig.LeaderElectionConfiguration{
-				LeaderElect:       false,                                    // from CLI args
-				LeaseDuration:     metav1.Duration{Duration: 2 * time.Hour}, // from CLI args
-				RenewDeadline:     metav1.Duration{Duration: 10 * time.Second},
-				RetryPeriod:       metav1.Duration{Duration: 2 * time.Second},
-				ResourceLock:      "leases",
-				ResourceName:      v1beta3.SchedulerDefaultLockObjectName,
-				ResourceNamespace: "default", // from deprecated CLI args
-			},
-		},
-		{
-			name: "leader election settings specified by ComponentConfig only",
-			flags: []string{
-				"--config", leaderElectionConfig,
-			},
-			wantLeaderElection: &componentbaseconfig.LeaderElectionConfiguration{
-				LeaderElect:       true,
-				LeaseDuration:     metav1.Duration{Duration: 1 * time.Hour}, // from CC
-				RenewDeadline:     metav1.Duration{Duration: 10 * time.Second},
-				RetryPeriod:       metav1.Duration{Duration: 2 * time.Second},
-				ResourceLock:      "leases",
-				ResourceName:      v1beta3.SchedulerDefaultLockObjectName,
-				ResourceNamespace: v1beta3.SchedulerDefaultLockObjectNamespace,
-			},
-		},
-		{
-			name: "leader election settings specified by CLI args and ComponentConfig",
-			flags: []string{
-				"--leader-elect=true",
-				"--leader-elect-renew-deadline=5s",
-				"--leader-elect-retry-period=1s",
-				"--config", leaderElectionConfig,
-			},
-			wantLeaderElection: &componentbaseconfig.LeaderElectionConfiguration{
-				LeaderElect:       true,
-				LeaseDuration:     metav1.Duration{Duration: 1 * time.Hour},   // from CC
-				RenewDeadline:     metav1.Duration{Duration: 5 * time.Second}, // from CLI args
-				RetryPeriod:       metav1.Duration{Duration: 1 * time.Second}, // from CLI args
-				ResourceLock:      "leases",
-				ResourceName:      v1beta3.SchedulerDefaultLockObjectName,
-				ResourceNamespace: v1beta3.SchedulerDefaultLockObjectNamespace,
-			},
-		},
 	}
 
 	makeListener := func(t *testing.T) net.Listener {
@@ -309,9 +211,6 @@ leaderElection:
 			if err != nil {
 				t.Fatal(err)
 			}
-			// use listeners instead of static ports so parallel test runs don't conflict
-			opts.SecureServing.Listener = makeListener(t)
-			defer opts.SecureServing.Listener.Close()
 
 			nfs := opts.Flags()
 			for _, f := range nfs.FlagSets {
@@ -321,6 +220,14 @@ leaderElection:
 				t.Fatal(err)
 			}
 
+			if err := opts.Complete(&nfs); err != nil {
+				t.Fatal(err)
+			}
+
+			// use listeners instead of static ports so parallel test runs don't conflict
+			opts.SecureServing.Listener = makeListener(t)
+			defer opts.SecureServing.Listener.Close()
+
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 			_, sched, err := Setup(ctx, opts)
@@ -328,22 +235,13 @@ leaderElection:
 				t.Fatal(err)
 			}
 
-			if tc.wantPlugins != nil {
-				gotPlugins := make(map[string]*config.Plugins)
-				for n, p := range sched.Profiles {
-					gotPlugins[n] = p.ListPlugins()
-				}
-
-				if diff := cmp.Diff(tc.wantPlugins, gotPlugins); diff != "" {
-					t.Errorf("Unexpected plugins diff (-want, +got): %s", diff)
-				}
+			gotPlugins := make(map[string]*config.Plugins)
+			for n, p := range sched.Profiles {
+				gotPlugins[n] = p.ListPlugins()
 			}
 
-			if tc.wantLeaderElection != nil {
-				gotLeaderElection := opts.ComponentConfig.LeaderElection
-				if diff := cmp.Diff(*tc.wantLeaderElection, gotLeaderElection); diff != "" {
-					t.Errorf("Unexpected leaderElection diff (-want, +got): %s", diff)
-				}
+			if diff := cmp.Diff(tc.wantPlugins, gotPlugins); diff != "" {
+				t.Errorf("unexpected plugins diff (-want, +got): %s", diff)
 			}
 		})
 	}

cmd/kube-scheduler/app/testing/testserver.go

@@ -93,6 +93,10 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err
 	}
 	fs.Parse(customFlags)
 
+	if err := opts.Complete(&namedFlagSets); err != nil {
+		return TestServer{}, err
+	}
+
 	if opts.SecureServing.BindPort != 0 {
 		opts.SecureServing.Listener, opts.SecureServing.BindPort, err = createListenerOnFreePort()
 		if err != nil {