Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kube-scheduler: allow deprecated options to be set with configfile #92531

Merged
merged 1 commit into from
Jun 30, 2020
Merged

kube-scheduler: allow deprecated options to be set with configfile #92531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 22 additions & 14 deletions cmd/kube-scheduler/app/options/deprecated.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ func (o *DeprecatedOptions) AddFlags(fs *pflag.FlagSet, cfg *kubeschedulerconfig
return
}

fs.StringVar(&o.AlgorithmProvider, "algorithm-provider", o.AlgorithmProvider, "DEPRECATED: the scheduling algorithm provider to use, one of: "+algorithmprovider.ListAlgorithmProviders())
fs.StringVar(&o.PolicyConfigFile, "policy-config-file", o.PolicyConfigFile, "DEPRECATED: file with scheduler policy configuration. This file is used if policy ConfigMap is not provided or --use-legacy-policy-config=true")
usage := fmt.Sprintf("DEPRECATED: name of the ConfigMap object that contains scheduler's policy configuration. It must exist in the system namespace before scheduler initialization if --use-legacy-policy-config=false. The config must be provided as the value of an element in 'Data' map with the key='%v'", kubeschedulerconfig.SchedulerPolicyConfigMapKey)
fs.StringVar(&o.AlgorithmProvider, "algorithm-provider", o.AlgorithmProvider, "DEPRECATED: the scheduling algorithm provider to use, this sets the default plugins for component config profiles. Choose one of: "+algorithmprovider.ListAlgorithmProviders())
fs.StringVar(&o.PolicyConfigFile, "policy-config-file", o.PolicyConfigFile, "DEPRECATED: file with scheduler policy configuration. This file is used if policy ConfigMap is not provided or --use-legacy-policy-config=true. Note: The scheduler will fail if this is combined with Plugin configs")
usage := fmt.Sprintf("DEPRECATED: name of the ConfigMap object that contains scheduler's policy configuration. It must exist in the system namespace before scheduler initialization if --use-legacy-policy-config=false. The config must be provided as the value of an element in 'Data' map with the key='%v'. Note: The scheduler will fail if this is combined with Plugin configs", kubeschedulerconfig.SchedulerPolicyConfigMapKey)
fs.StringVar(&o.PolicyConfigMapName, "policy-configmap", o.PolicyConfigMapName, usage)
fs.StringVar(&o.PolicyConfigMapNamespace, "policy-configmap-namespace", o.PolicyConfigMapNamespace, "DEPRECATED: the namespace where policy ConfigMap is located. The kube-system namespace will be used if this is not provided or is empty.")
fs.BoolVar(&o.UseLegacyPolicyConfig, "use-legacy-policy-config", o.UseLegacyPolicyConfig, "DEPRECATED: when set to true, scheduler will ignore policy ConfigMap and uses policy config file")
fs.StringVar(&o.PolicyConfigMapNamespace, "policy-configmap-namespace", o.PolicyConfigMapNamespace, "DEPRECATED: the namespace where policy ConfigMap is located. The kube-system namespace will be used if this is not provided or is empty. Note: The scheduler will fail if this is combined with Plugin configs")
fs.BoolVar(&o.UseLegacyPolicyConfig, "use-legacy-policy-config", o.UseLegacyPolicyConfig, "DEPRECATED: when set to true, scheduler will ignore policy ConfigMap and uses policy config file. Note: The scheduler will fail if this is combined with Plugin configs")

fs.BoolVar(&cfg.EnableProfiling, "profiling", cfg.EnableProfiling, "DEPRECATED: enable profiling via web interface host:port/debug/pprof/")
fs.BoolVar(&cfg.EnableContentionProfiling, "contention-profiling", cfg.EnableContentionProfiling, "DEPRECATED: enable lock contention profiling, if profiling is enabled")
Expand Down Expand Up @@ -87,16 +87,14 @@ func (o *DeprecatedOptions) Validate() []error {
return errs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document in the applicable help strings above that scheduler would fail if the flags are used in combination with scheduling plugins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

// ApplyTo sets cfg.AlgorithmSource from flags passed on the command line in the following precedence order:
// ApplyAlgorithmSourceTo sets cfg.AlgorithmSource from flags passed on the command line in the following precedence order:
//
// 1. --use-legacy-policy-config to use a policy file.
// 2. --policy-configmap to use a policy config map value.
// 3. --algorithm-provider to use a named algorithm provider.
//
// This function is only called when no config file is provided.
func (o *DeprecatedOptions) ApplyTo(cfg *kubeschedulerconfig.KubeSchedulerConfiguration) error {
func (o *DeprecatedOptions) ApplyAlgorithmSourceTo(cfg *kubeschedulerconfig.KubeSchedulerConfiguration) {
damemi marked this conversation as resolved.
Show resolved Hide resolved
if o == nil {
return nil
return
}

switch {
Expand All @@ -122,10 +120,19 @@ func (o *DeprecatedOptions) ApplyTo(cfg *kubeschedulerconfig.KubeSchedulerConfig
Provider: &o.AlgorithmProvider,
}
}
}

// Deprecated flags have an effect iff no config file was provided, in which
// case this function expects a default KubeSchedulerConfiguration instance,
// which has a single profile.
// ApplyTo sets a default profile plugin config if no config file is specified
// It also calls ApplyAlgorithmSourceTo to set Policy settings in AlgorithmSource, if applicable.
// Deprecated flags have an effect iff no config file was provided, in which
// case this function expects a default KubeSchedulerConfiguration instance,
// which has a single profile.
func (o *DeprecatedOptions) ApplyTo(cfg *kubeschedulerconfig.KubeSchedulerConfiguration) {
if o == nil {
return
}
// The following deprecated options affect the only existing profile that is
// added by default.
profile := &cfg.Profiles[0]
if len(o.SchedulerName) > 0 {
profile.SchedulerName = o.SchedulerName
Expand All @@ -136,6 +143,7 @@ func (o *DeprecatedOptions) ApplyTo(cfg *kubeschedulerconfig.KubeSchedulerConfig
HardPodAffinityWeight: o.HardPodAffinitySymmetricWeight,
},
}

profile.PluginConfig = append(profile.PluginConfig, plCfg)
return nil
o.ApplyAlgorithmSourceTo(cfg)
}
36 changes: 28 additions & 8 deletions cmd/kube-scheduler/app/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,13 @@ func newDefaultComponentConfig() (*kubeschedulerconfig.KubeSchedulerConfiguratio
// Flags returns flags for a specific scheduler by section name
func (o *Options) Flags() (nfs cliflag.NamedFlagSets) {
fs := nfs.FlagSet("misc")
fs.StringVar(&o.ConfigFile, "config", o.ConfigFile, "The path to the configuration file. Flags override values in this file.")
fs.StringVar(&o.ConfigFile, "config", o.ConfigFile, `The path to the configuration file. The following flags can overwrite fields in this file:
--address
--port
--use-legacy-policy-config
--policy-configmap
--policy-config-file
--algorithm-provider`)
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)")

Expand All @@ -170,10 +176,8 @@ func (o *Options) ApplyTo(c *schedulerappconfig.Config) error {
if len(o.ConfigFile) == 0 {
c.ComponentConfig = o.ComponentConfig

// only apply deprecated flags if no config file is loaded (this is the old behaviour).
if err := o.Deprecated.ApplyTo(&c.ComponentConfig); err != nil {
return err
}
// apply deprecated flags if no config file is loaded (this is the old behaviour).
o.Deprecated.ApplyTo(&c.ComponentConfig)
if err := o.CombinedInsecureServing.ApplyTo(c, &c.ComponentConfig); err != nil {
return err
}
Expand All @@ -186,11 +190,18 @@ func (o *Options) ApplyTo(c *schedulerappconfig.Config) error {
return err
}

// use the loaded config file only, with the exception of --address and --port. This means that
// none of the deprecated flags in o.Deprecated are taken into consideration. This is the old
// behaviour of the flags we have to keep.
c.ComponentConfig = *cfg

// apply any deprecated Policy flags, if applicable
o.Deprecated.ApplyAlgorithmSourceTo(&c.ComponentConfig)

// if the user has set CC profiles and is trying to use a Policy config, error out
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could call ApplyAlgorithmSourceTo before this and check that .AlgorithmSource.Policy is empty

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

also note, that we can't check if algorithmSource.Provider is empty because when any config is provided the default algorithmProvider value is set by conversion to the internal type. Not sure if this matters, but I was checking against that before too

// these configs are no longer merged and they should not be used simultaneously
if !emptySchedulerProfileConfig(c.ComponentConfig.Profiles) && c.ComponentConfig.AlgorithmSource.Policy != nil {
return fmt.Errorf("cannot set a Plugin config and Policy config")
}

// use the loaded config file only, with the exception of --address and --port.
if err := o.CombinedInsecureServing.ApplyToFromLoadedConfig(c, &c.ComponentConfig); err != nil {
return err
}
Expand All @@ -211,6 +222,15 @@ func (o *Options) ApplyTo(c *schedulerappconfig.Config) error {
return nil
}

// emptySchedulerProfileConfig returns true if the list of profiles passed to it contains only
// the "default-scheduler" profile with no plugins or pluginconfigs registered
// (this is the default empty profile initialized by defaults.go)
func emptySchedulerProfileConfig(profiles []kubeschedulerconfig.KubeSchedulerProfile) bool {
return len(profiles) == 1 &&
len(profiles[0].PluginConfig) == 0 &&
profiles[0].Plugins == nil
}

// Validate validates all the required options.
func (o *Options) Validate() []error {
var errs []error
Expand Down
10 changes: 10 additions & 0 deletions cmd/kube-scheduler/app/options/options_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,16 @@ profiles:
},
},
},
{
name: "Attempting to set Component Config Profiles and Policy config",
options: &Options{
ConfigFile: pluginConfigFile,
Deprecated: &DeprecatedOptions{
PolicyConfigMapName: "bar",
},
},
expectedError: "cannot set a Plugin config and Policy config",
},
{
name: "unknown field",
options: &Options{
Expand Down