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

Scheduler simplified MultiPoint plugin config #105611

Merged
merged 1 commit into from Nov 17, 2021
Merged

Scheduler simplified MultiPoint plugin config #105611

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
40 changes: 17 additions & 23 deletions cmd/kube-scheduler/app/options/options_test.go
View file
Open in desktop
Expand Up @@ -690,24 +690,24 @@ profiles:
{
SchedulerName: "default-scheduler",
Plugins: &kubeschedulerconfig.Plugins{
QueueSort: defaults.PluginsV1beta3.QueueSort,
PreFilter: defaults.PluginsV1beta3.PreFilter,
Filter: defaults.PluginsV1beta3.Filter,
PostFilter: defaults.PluginsV1beta3.PostFilter,
PreScore: defaults.PluginsV1beta3.PreScore,
Score: defaults.PluginsV1beta3.Score,
Reserve: kubeschedulerconfig.PluginSet{
Enabled: []kubeschedulerconfig.Plugin{
{Name: "foo"},
{Name: "bar"},
},
Disabled: []kubeschedulerconfig.Plugin{
{Name: names.VolumeBinding},
},
},
PreBind: kubeschedulerconfig.PluginSet{
Enabled: []kubeschedulerconfig.Plugin{
{Name: "foo"},
},
Disabled: []kubeschedulerconfig.Plugin{
{Name: names.VolumeBinding},
},
},
Bind: defaults.PluginsV1beta3.Bind,
MultiPoint: defaults.PluginsV1beta3.MultiPoint,
},
PluginConfig: []kubeschedulerconfig.PluginConfig{
{
Expand Down Expand Up @@ -918,34 +918,28 @@ profiles:
{
SchedulerName: "foo-profile",
Plugins: &kubeschedulerconfig.Plugins{
QueueSort: defaults.PluginsV1beta3.QueueSort,
PreFilter: defaults.PluginsV1beta3.PreFilter,
Filter: defaults.PluginsV1beta3.Filter,
PostFilter: defaults.PluginsV1beta3.PostFilter,
PreScore: defaults.PluginsV1beta3.PreScore,
Score: defaults.PluginsV1beta3.Score,
Bind: defaults.PluginsV1beta3.Bind,
PreBind: defaults.PluginsV1beta3.PreBind,
MultiPoint: defaults.PluginsV1beta3.MultiPoint,
Reserve: kubeschedulerconfig.PluginSet{
Enabled: []kubeschedulerconfig.Plugin{
{Name: "foo"},
{Name: names.VolumeBinding},
},
Disabled: []kubeschedulerconfig.Plugin{
{Name: names.VolumeBinding},
},
},
},
PluginConfig: defaults.PluginConfigsV1beta3,
},
{
SchedulerName: "bar-profile",
Plugins: &kubeschedulerconfig.Plugins{
QueueSort: defaults.PluginsV1beta3.QueueSort,
PreFilter: defaults.PluginsV1beta3.PreFilter,
Filter: defaults.PluginsV1beta3.Filter,
PostFilter: defaults.PluginsV1beta3.PostFilter,
PreScore: defaults.PluginsV1beta3.PreScore,
Score: defaults.PluginsV1beta3.Score,
Bind: defaults.PluginsV1beta3.Bind,
Reserve: defaults.PluginsV1beta3.Reserve,
MultiPoint: defaults.PluginsV1beta3.MultiPoint,
PreBind: kubeschedulerconfig.PluginSet{
Disabled: []kubeschedulerconfig.Plugin{
{Name: names.VolumeBinding},
},
},
},
PluginConfig: []kubeschedulerconfig.PluginConfig{
{
Expand Down
107 changes: 92 additions & 15 deletions cmd/kube-scheduler/app/server_test.go
View file
Open in desktop
Expand Up @@ -82,8 +82,46 @@ users:
}

// plugin config
pluginConfigFile := filepath.Join(tmpDir, "plugin.yaml")
if err := ioutil.WriteFile(pluginConfigFile, []byte(fmt.Sprintf(`
pluginConfigFilev1beta3 := filepath.Join(tmpDir, "pluginv1beta3.yaml")
if err := ioutil.WriteFile(pluginConfigFilev1beta3, []byte(fmt.Sprintf(`
apiVersion: kubescheduler.config.k8s.io/v1beta3
Copy link
Member

Choose a reason for hiding this comment

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

can we avoid changing the tested version and add a v1beta3-specific test? This makes it hard to tell if we’ve broken behavior of v1beta2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

v1beta2 is also getting a multiPoint field to handle round-tripping through the internal version, which is what all the logic operates on. So, this shouldn't make a difference right?

Copy link
Member

Choose a reason for hiding this comment

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

It matters if it changes behavior of existing v1beta2 config files. Changing this here makes it hard to tell if that happened

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't, but I split this out into a v1beta3 test and kept the old v1beta2 test.

The v1beta3 test looks different now because I specifically added MultiPoint plugins to achieve the same output as it would have been without multipoint (ie, v1beta2' s output). But the semantics for Disabled/Enabled in individual extension points is the same, and individual points take precedence, so even though default plugins are now "MultiPoint" the v1beta2 test doesn't break

kind: KubeSchedulerConfiguration
clientConnection:
kubeconfig: "%s"
profiles:
- plugins:
multiPoint:
enabled:
- name: DefaultBinder
- name: PrioritySort
- name: DefaultPreemption
- name: VolumeBinding
- name: NodeResourcesFit
- name: NodePorts
- name: InterPodAffinity
- name: TaintToleration
disabled:
- name: "*"
preFilter:
disabled:
- name: VolumeBinding
- name: InterPodAffinity
filter:
disabled:
- name: VolumeBinding
- name: InterPodAffinity
- name: TaintToleration
score:
disabled:
- name: VolumeBinding
- name: NodeResourcesFit
`, configKubeconfig)), os.FileMode(0600)); err != nil {
t.Fatal(err)
}

// plugin config
pluginConfigFilev1beta2 := filepath.Join(tmpDir, "pluginv1beta2.yaml")
if err := ioutil.WriteFile(pluginConfigFilev1beta2, []byte(fmt.Sprintf(`
apiVersion: kubescheduler.config.k8s.io/v1beta2
kind: KubeSchedulerConfiguration
clientConnection:
Expand Down Expand Up @@ -188,20 +226,19 @@ leaderElection:
wantPlugins: map[string]*config.Plugins{
"default-scheduler": func() *config.Plugins {
plugins := &config.Plugins{
QueueSort: defaults.PluginsV1beta3.QueueSort,
PreFilter: defaults.PluginsV1beta3.PreFilter,
Filter: defaults.PluginsV1beta3.Filter,
PostFilter: defaults.PluginsV1beta3.PostFilter,
PreScore: defaults.PluginsV1beta3.PreScore,
Score: defaults.PluginsV1beta3.Score,
Bind: defaults.PluginsV1beta3.Bind,
PreBind: defaults.PluginsV1beta3.PreBind,
Reserve: defaults.PluginsV1beta3.Reserve,
QueueSort: defaults.ExpandedPluginsV1beta3.QueueSort,
PreFilter: defaults.ExpandedPluginsV1beta3.PreFilter,
Filter: defaults.ExpandedPluginsV1beta3.Filter,
PostFilter: defaults.ExpandedPluginsV1beta3.PostFilter,
PreScore: defaults.ExpandedPluginsV1beta3.PreScore,
Score: defaults.ExpandedPluginsV1beta3.Score,
Bind: defaults.ExpandedPluginsV1beta3.Bind,
PreBind: defaults.ExpandedPluginsV1beta3.PreBind,
Reserve: defaults.ExpandedPluginsV1beta3.Reserve,
}
plugins.PreScore.Enabled = append(plugins.PreScore.Enabled, config.Plugin{Name: names.SelectorSpread, Weight: 0})
plugins.Score.Enabled = append(
plugins.Score.Enabled,
config.Plugin{Name: names.VolumeBinding, Weight: 1},
config.Plugin{Name: names.SelectorSpread, Weight: 1},
)
return plugins
Expand All @@ -218,13 +255,53 @@ leaderElection:
"--kubeconfig", configKubeconfig,
},
wantPlugins: map[string]*config.Plugins{
"default-scheduler": defaults.PluginsV1beta3,
"default-scheduler": defaults.ExpandedPluginsV1beta3,
},
},
{
name: "component configuration v1beta2",
flags: []string{
"--config", pluginConfigFilev1beta2,
"--kubeconfig", configKubeconfig,
},
wantPlugins: map[string]*config.Plugins{
"default-scheduler": {
Bind: config.PluginSet{Enabled: []config.Plugin{{Name: "DefaultBinder"}}},
Filter: config.PluginSet{
Enabled: []config.Plugin{
{Name: "NodeResourcesFit"},
{Name: "NodePorts"},
},
},
PreFilter: config.PluginSet{
Enabled: []config.Plugin{
{Name: "NodeResourcesFit"},
{Name: "NodePorts"},
},
},
PostFilter: config.PluginSet{Enabled: []config.Plugin{{Name: "DefaultPreemption"}}},
PreScore: config.PluginSet{
Enabled: []config.Plugin{
{Name: "InterPodAffinity"},
{Name: "TaintToleration"},
},
},
QueueSort: config.PluginSet{Enabled: []config.Plugin{{Name: "PrioritySort"}}},
Score: config.PluginSet{
Enabled: []config.Plugin{
{Name: "InterPodAffinity", Weight: 1},
{Name: "TaintToleration", Weight: 1},
},
},
Reserve: config.PluginSet{Enabled: []config.Plugin{{Name: "VolumeBinding"}}},
PreBind: config.PluginSet{Enabled: []config.Plugin{{Name: "VolumeBinding"}}},
},
},
},
{
name: "component configuration",
name: "component configuration v1beta3",
flags: []string{
"--config", pluginConfigFile,
"--config", pluginConfigFilev1beta3,
"--kubeconfig", configKubeconfig,
},
wantPlugins: map[string]*config.Plugins{
Expand Down
63 changes: 48 additions & 15 deletions pkg/scheduler/apis/config/testing/defaults/defaults.go
View file
Open in desktop
Expand Up @@ -151,22 +151,50 @@ var PluginConfigsV1beta2 = []config.PluginConfig{
},
}

// PluginsV1beta3 default set of v1beta3 plugins.
// PluginsV1beta3 is the set of default v1beta3 plugins (before MultiPoint expansion)
liggitt marked this conversation as resolved.
Show resolved Hide resolved
var PluginsV1beta3 = &config.Plugins{
MultiPoint: config.PluginSet{
Enabled: []config.Plugin{
{Name: names.PrioritySort},
{Name: names.NodeUnschedulable},
{Name: names.NodeName},
{Name: names.TaintToleration, Weight: 3},
{Name: names.NodeAffinity, Weight: 2},
{Name: names.NodePorts},
{Name: names.NodeResourcesFit, Weight: 1},
{Name: names.VolumeRestrictions},
{Name: names.EBSLimits},
{Name: names.GCEPDLimits},
{Name: names.NodeVolumeLimits},
{Name: names.AzureDiskLimits},
{Name: names.VolumeBinding},
{Name: names.VolumeZone},
{Name: names.PodTopologySpread, Weight: 2},
{Name: names.InterPodAffinity, Weight: 2},
{Name: names.DefaultPreemption},
{Name: names.NodeResourcesBalancedAllocation, Weight: 1},
{Name: names.ImageLocality, Weight: 1},
{Name: names.DefaultBinder},
},
},
}

// ExpandedPluginsV1beta3 default set of v1beta3 plugins after MultiPoint expansion
var ExpandedPluginsV1beta3 = &config.Plugins{
QueueSort: config.PluginSet{
Enabled: []config.Plugin{
{Name: names.PrioritySort},
},
},
PreFilter: config.PluginSet{
Enabled: []config.Plugin{
{Name: names.NodeResourcesFit},
{Name: names.NodeAffinity},
{Name: names.NodePorts},
{Name: names.NodeResourcesFit},
{Name: names.VolumeRestrictions},
{Name: names.VolumeBinding},
damemi marked this conversation as resolved.
Show resolved Hide resolved
{Name: names.PodTopologySpread},
{Name: names.InterPodAffinity},
{Name: names.VolumeBinding},
{Name: names.NodeAffinity},
},
},
Filter: config.PluginSet{
Expand Down Expand Up @@ -195,32 +223,37 @@ var PluginsV1beta3 = &config.Plugins{
},
PreScore: config.PluginSet{
Enabled: []config.Plugin{
{Name: names.InterPodAffinity},
{Name: names.PodTopologySpread},
{Name: names.TaintToleration},
{Name: names.NodeAffinity},
{Name: names.PodTopologySpread},
{Name: names.InterPodAffinity},
},
},
Score: config.PluginSet{
Enabled: []config.Plugin{
{Name: names.NodeResourcesBalancedAllocation, Weight: 1},
{Name: names.ImageLocality, Weight: 1},
{Name: names.NodeResourcesFit, Weight: 1},
// Weight is doubled because:
// Weight is tripled because:
// - This is a score coming from user preference.
{Name: names.InterPodAffinity, Weight: 2},
// - Usage of node tainting to group nodes in the cluster is increasing becoming a use-case
// for many user workloads
{Name: names.TaintToleration, Weight: 3},
damemi marked this conversation as resolved.
Show resolved Hide resolved
// Weight is doubled because:
// - This is a score coming from user preference.
{Name: names.NodeAffinity, Weight: 2},
{Name: names.NodeResourcesFit, Weight: 1},
// Weight is tripled because:
// - This is a score coming from user preference.
// - Usage of node tainting to group nodes in the cluster is increasing becoming a use-case
// for many user workloads
{Name: names.VolumeBinding, Weight: 1},
// Weight is doubled because:
// - This is a score coming from user preference.
// - It makes its signal comparable to NodeResourcesLeastAllocated.
{Name: names.PodTopologySpread, Weight: 2},
// Weight is tripled because:
damemi marked this conversation as resolved.
Show resolved Hide resolved
// Weight is doubled because:
// - This is a score coming from user preference.
// - Usage of node tainting to group nodes in the cluster is increasing becoming a use-case
// for many user workloads
{Name: names.TaintToleration, Weight: 3},
{Name: names.InterPodAffinity, Weight: 2},
{Name: names.NodeResourcesBalancedAllocation, Weight: 1},
{Name: names.ImageLocality, Weight: 1},
},
},
Reserve: config.PluginSet{
Expand Down
3 changes: 3 additions & 0 deletions pkg/scheduler/apis/config/types.go
View file
Open in desktop
Expand Up @@ -161,6 +161,9 @@ type Plugins struct {

// PostBind is a list of plugins that should be invoked after a pod is successfully bound.
PostBind PluginSet

// MultiPoint is a simplified config field for enabling plugins for all valid extension points
MultiPoint PluginSet
}

// PluginSet specifies enabled and disabled plugins for an extension point.
Expand Down
6 changes: 6 additions & 0 deletions pkg/scheduler/apis/config/v1beta2/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.