From 95dd99905f1bff2c7929a6f2bde59547f496b4d9 Mon Sep 17 00:00:00 2001 From: Aldo Culquicondor Date: Tue, 6 Oct 2020 17:13:22 -0400 Subject: [PATCH] Map SelectorSpreadPriority to PodTopologySpread plugin when DefaultPodTopologySpread feature is enabled If SelectorSpreadPriority is in use, PodTopologySpread gets inevitably enabled. When only EvenPodsSpreadPriority is in use, PodTopologySpread is configured without system defaults. Change-Id: I2389a585cd8ad0bd35b0d2acae1665cd46908b3e --- pkg/scheduler/framework/plugins/BUILD | 13 ++ .../framework/plugins/legacy_registry.go | 55 +++++- .../framework/plugins/legacy_registry_test.go | 160 ++++++++++++++++++ 3 files changed, 224 insertions(+), 4 deletions(-) diff --git a/pkg/scheduler/framework/plugins/BUILD b/pkg/scheduler/framework/plugins/BUILD index 8327c03cfd6..f71375c36fc 100644 --- a/pkg/scheduler/framework/plugins/BUILD +++ b/pkg/scheduler/framework/plugins/BUILD @@ -9,6 +9,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/scheduler/framework/plugins", visibility = ["//visibility:public"], deps = [ + "//pkg/features:go_default_library", "//pkg/scheduler/apis/config:go_default_library", "//pkg/scheduler/framework/plugins/defaultbinder:go_default_library", "//pkg/scheduler/framework/plugins/defaultpreemption:go_default_library", @@ -32,6 +33,7 @@ go_library( "//pkg/scheduler/framework/plugins/volumezone:go_default_library", "//pkg/scheduler/framework/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//vendor/k8s.io/klog/v2:go_default_library", ], ) @@ -79,10 +81,21 @@ go_test( srcs = ["legacy_registry_test.go"], embed = [":go_default_library"], deps = [ + "//pkg/features:go_default_library", "//pkg/scheduler/apis/config:go_default_library", + "//pkg/scheduler/framework/plugins/imagelocality:go_default_library", + "//pkg/scheduler/framework/plugins/interpodaffinity:go_default_library", + "//pkg/scheduler/framework/plugins/nodeaffinity:go_default_library", + "//pkg/scheduler/framework/plugins/nodepreferavoidpods:go_default_library", + "//pkg/scheduler/framework/plugins/noderesources:go_default_library", "//pkg/scheduler/framework/plugins/nodeunschedulable:go_default_library", + "//pkg/scheduler/framework/plugins/podtopologyspread:go_default_library", + "//pkg/scheduler/framework/plugins/selectorspread:go_default_library", "//pkg/scheduler/framework/plugins/tainttoleration:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/component-base/featuregate:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/github.com/google/go-cmp/cmp:go_default_library", ], ) diff --git a/pkg/scheduler/framework/plugins/legacy_registry.go b/pkg/scheduler/framework/plugins/legacy_registry.go index ce753c2d6b3..8105b656989 100644 --- a/pkg/scheduler/framework/plugins/legacy_registry.go +++ b/pkg/scheduler/framework/plugins/legacy_registry.go @@ -21,7 +21,9 @@ import ( "sort" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/scheduler/apis/config" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/imagelocality" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity" @@ -327,9 +329,29 @@ func NewLegacyRegistry() *LegacyRegistry { // Register Priorities. registry.registerPriorityConfigProducer(SelectorSpreadPriority, - func(args ConfigProducerArgs, plugins *config.Plugins, _ *[]config.PluginConfig) { - plugins.Score = appendToPluginSet(plugins.Score, selectorspread.Name, &args.Weight) - plugins.PreScore = appendToPluginSet(plugins.PreScore, selectorspread.Name, nil) + func(args ConfigProducerArgs, plugins *config.Plugins, pluginConfig *[]config.PluginConfig) { + if !feature.DefaultFeatureGate.Enabled(features.DefaultPodTopologySpread) { + plugins.Score = appendToPluginSet(plugins.Score, selectorspread.Name, &args.Weight) + plugins.PreScore = appendToPluginSet(plugins.PreScore, selectorspread.Name, nil) + return + } + plugins.Score = appendToPluginSet(plugins.Score, podtopologyspread.Name, &args.Weight) + plugins.PreScore = appendToPluginSet(plugins.PreScore, podtopologyspread.Name, nil) + plArgs := config.PodTopologySpreadArgs{ + DefaultingType: config.SystemDefaulting, + } + // The order in which SelectorSpreadPriority or EvenPodsSpreadPriority producers + // are called is not guaranteed. Override or append configuration. + for i, e := range *pluginConfig { + if e.Name == podtopologyspread.Name { + (*pluginConfig)[i].Args = &plArgs + return + } + } + *pluginConfig = append(*pluginConfig, config.PluginConfig{ + Name: podtopologyspread.Name, + Args: &plArgs, + }) }) registry.registerPriorityConfigProducer(TaintTolerationPriority, func(args ConfigProducerArgs, plugins *config.Plugins, _ *[]config.PluginConfig) { @@ -402,9 +424,25 @@ func NewLegacyRegistry() *LegacyRegistry { } }) registry.registerPriorityConfigProducer(EvenPodsSpreadPriority, - func(args ConfigProducerArgs, plugins *config.Plugins, _ *[]config.PluginConfig) { + func(args ConfigProducerArgs, plugins *config.Plugins, pluginConfig *[]config.PluginConfig) { plugins.PreScore = appendToPluginSet(plugins.PreScore, podtopologyspread.Name, nil) plugins.Score = appendToPluginSet(plugins.Score, podtopologyspread.Name, &args.Weight) + if feature.DefaultFeatureGate.Enabled(features.DefaultPodTopologySpread) { + // The order in which SelectorSpreadPriority or EvenPodsSpreadPriority producers + // are called is not guaranteed. If plugin was not configured yet, append + // configuration where system default constraints are disabled. + for _, e := range *pluginConfig { + if e.Name == podtopologyspread.Name { + return + } + } + *pluginConfig = append(*pluginConfig, config.PluginConfig{ + Name: podtopologyspread.Name, + Args: &config.PodTopologySpreadArgs{ + DefaultingType: config.ListDefaulting, + }, + }) + } }) return registry @@ -489,6 +527,15 @@ func appendToPluginSet(set *config.PluginSet, name string, weight *int32) *confi if set == nil { set = &config.PluginSet{} } + for _, e := range set.Enabled { + if e.Name == name { + // Keep the max weight. + if weight != nil && *weight > e.Weight { + e.Weight = *weight + } + return set + } + } cfg := config.Plugin{Name: name} if weight != nil { cfg.Weight = *weight diff --git a/pkg/scheduler/framework/plugins/legacy_registry_test.go b/pkg/scheduler/framework/plugins/legacy_registry_test.go index 306d00210b3..77bf0391e33 100644 --- a/pkg/scheduler/framework/plugins/legacy_registry_test.go +++ b/pkg/scheduler/framework/plugins/legacy_registry_test.go @@ -21,8 +21,19 @@ import ( "github.com/google/go-cmp/cmp" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apiserver/pkg/util/feature" + "k8s.io/component-base/featuregate" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/scheduler/apis/config" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/imagelocality" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/nodeaffinity" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/nodepreferavoidpods" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/noderesources" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/nodeunschedulable" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/podtopologyspread" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/selectorspread" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/tainttoleration" ) @@ -93,3 +104,152 @@ func TestRegisterConfigProducers(t *testing.T) { t.Errorf("unexpected plugin configuration (-want, +got): %s", diff) } } + +func TestAppendPriorityConfigs(t *testing.T) { + cases := []struct { + name string + features map[featuregate.Feature]bool + keys map[string]int64 + args ConfigProducerArgs + wantPlugins config.Plugins + wantPluginConfig []config.PluginConfig + }{ + { + name: "default priorities", + wantPlugins: config.Plugins{ + PreScore: &config.PluginSet{ + Enabled: []config.Plugin{ + {Name: podtopologyspread.Name}, + {Name: interpodaffinity.Name}, + {Name: selectorspread.Name}, + {Name: tainttoleration.Name}, + }, + }, + Score: &config.PluginSet{ + Enabled: []config.Plugin{ + {Name: noderesources.BalancedAllocationName, Weight: 1}, + {Name: podtopologyspread.Name, Weight: 2}, + {Name: imagelocality.Name, Weight: 1}, + {Name: interpodaffinity.Name, Weight: 1}, + {Name: noderesources.LeastAllocatedName, Weight: 1}, + {Name: nodeaffinity.Name, Weight: 1}, + {Name: nodepreferavoidpods.Name, Weight: 10000}, + {Name: selectorspread.Name, Weight: 1}, + {Name: tainttoleration.Name, Weight: 1}, + }, + }, + }, + }, + { + name: "DefaultPodTopologySpread enabled, SelectorSpreadPriority only", + features: map[featuregate.Feature]bool{ + features.DefaultPodTopologySpread: true, + }, + keys: map[string]int64{ + SelectorSpreadPriority: 3, + }, + wantPlugins: config.Plugins{ + PreScore: &config.PluginSet{ + Enabled: []config.Plugin{ + {Name: podtopologyspread.Name}, + }, + }, + Score: &config.PluginSet{ + Enabled: []config.Plugin{ + {Name: podtopologyspread.Name, Weight: 3}, + }, + }, + }, + wantPluginConfig: []config.PluginConfig{ + { + Name: podtopologyspread.Name, + Args: &config.PodTopologySpreadArgs{ + DefaultingType: config.SystemDefaulting, + }, + }, + }, + }, + { + name: "DefaultPodTopologySpread enabled, EvenPodsSpreadPriority only", + features: map[featuregate.Feature]bool{ + features.DefaultPodTopologySpread: true, + }, + keys: map[string]int64{ + EvenPodsSpreadPriority: 4, + }, + wantPlugins: config.Plugins{ + PreScore: &config.PluginSet{ + Enabled: []config.Plugin{ + {Name: podtopologyspread.Name}, + }, + }, + Score: &config.PluginSet{ + Enabled: []config.Plugin{ + {Name: podtopologyspread.Name, Weight: 4}, + }, + }, + }, + wantPluginConfig: []config.PluginConfig{ + { + Name: podtopologyspread.Name, + Args: &config.PodTopologySpreadArgs{ + DefaultingType: config.ListDefaulting, + }, + }, + }, + }, + { + name: "DefaultPodTopologySpread enabled, SelectorSpreadPriority+EvenPodsSpreadPriority", + features: map[featuregate.Feature]bool{ + features.DefaultPodTopologySpread: true, + }, + keys: map[string]int64{ + SelectorSpreadPriority: 1, + EvenPodsSpreadPriority: 2, + }, + wantPlugins: config.Plugins{ + PreScore: &config.PluginSet{ + Enabled: []config.Plugin{ + {Name: podtopologyspread.Name}, + }, + }, + Score: &config.PluginSet{ + Enabled: []config.Plugin{ + {Name: podtopologyspread.Name, Weight: 2}, + }, + }, + }, + wantPluginConfig: []config.PluginConfig{ + { + Name: podtopologyspread.Name, + Args: &config.PodTopologySpreadArgs{ + DefaultingType: config.SystemDefaulting, + }, + }, + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + for k, v := range tc.features { + defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, k, v)() + } + + r := NewLegacyRegistry() + keys := tc.keys + if keys == nil { + keys = r.DefaultPriorities + } + plugins, pluginConfig, err := r.AppendPriorityConfigs(keys, &tc.args, config.Plugins{}, nil) + if err != nil { + t.Fatalf("Appending Priority Configs: %v", err) + } + if diff := cmp.Diff(tc.wantPlugins, plugins); diff != "" { + t.Errorf("Unexpected Plugin (-want,+got):\n%s", diff) + } + if diff := cmp.Diff(tc.wantPluginConfig, pluginConfig); diff != "" { + t.Errorf("Unexpected PluginConfig (-want,+got):\n%s", diff) + } + }) + } +}