From e50a24d64cfa31eeb26549c2b06df9b60d722d5b Mon Sep 17 00:00:00 2001 From: Cong Liu Date: Mon, 19 Aug 2019 16:08:14 -0400 Subject: [PATCH] Move RunNormalizeScorePlugins and ApplyScoreWeights into RunScorePlugins; Also add unit tests for RunScorePlugins. --- pkg/scheduler/core/generic_scheduler.go | 12 - pkg/scheduler/framework/v1alpha1/BUILD | 1 + pkg/scheduler/framework/v1alpha1/framework.go | 50 +- .../framework/v1alpha1/framework_test.go | 703 +++++------------- pkg/scheduler/framework/v1alpha1/interface.go | 10 - .../internal/queue/scheduling_queue_test.go | 8 - 6 files changed, 192 insertions(+), 592 deletions(-) diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go index 42a8b9c0ef08..99c984ff555c 100644 --- a/pkg/scheduler/core/generic_scheduler.go +++ b/pkg/scheduler/core/generic_scheduler.go @@ -784,18 +784,6 @@ func PrioritizeNodes( return schedulerapi.HostPriorityList{}, scoreStatus.AsError() } - // Run the Normalize Score plugins. - status := framework.RunNormalizeScorePlugins(pluginContext, pod, scoresMap) - if !status.IsSuccess() { - return schedulerapi.HostPriorityList{}, status.AsError() - } - - // Apply weights for scores. - status = framework.ApplyScoreWeights(pluginContext, pod, scoresMap) - if !status.IsSuccess() { - return schedulerapi.HostPriorityList{}, status.AsError() - } - // Summarize all scores. result := make(schedulerapi.HostPriorityList, 0, len(nodes)) diff --git a/pkg/scheduler/framework/v1alpha1/BUILD b/pkg/scheduler/framework/v1alpha1/BUILD index 481eb106fdcd..02b272c235d3 100644 --- a/pkg/scheduler/framework/v1alpha1/BUILD +++ b/pkg/scheduler/framework/v1alpha1/BUILD @@ -48,6 +48,7 @@ go_test( deps = [ "//pkg/scheduler/apis/config:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", ], ) diff --git a/pkg/scheduler/framework/v1alpha1/framework.go b/pkg/scheduler/framework/v1alpha1/framework.go index 521e67b37c74..252d02e0cdc6 100644 --- a/pkg/scheduler/framework/v1alpha1/framework.go +++ b/pkg/scheduler/framework/v1alpha1/framework.go @@ -360,6 +360,8 @@ func (f *framework) RunScorePlugins(pc *PluginContext, pod *v1.Pod, nodes []*v1. } ctx, cancel := context.WithCancel(context.Background()) errCh := schedutil.NewErrorChannel() + + // Run Score method for each node in parallel. workqueue.ParallelizeUntil(ctx, 16, len(nodes), func(index int) { for _, pl := range f.scorePlugins { nodeName := nodes[index].Name @@ -374,31 +376,16 @@ func (f *framework) RunScorePlugins(pc *PluginContext, pod *v1.Pod, nodes []*v1. } } }) - if err := errCh.ReceiveError(); err != nil { msg := fmt.Sprintf("error while running score plugin for pod %q: %v", pod.Name, err) klog.Error(msg) return nil, NewStatus(Error, msg) } - return pluginToNodeScores, nil -} - -// RunNormalizeScorePlugins runs the NormalizeScore function of Score plugins. -// It should be called after RunScorePlugins with the PluginToNodeScores result. -// It then modifies the list with normalized scores. It returns a non-success Status -// if any of the NormalizeScore functions returns a non-success status. -func (f *framework) RunNormalizeScorePlugins(pc *PluginContext, pod *v1.Pod, scores PluginToNodeScores) *Status { - ctx, cancel := context.WithCancel(context.Background()) - errCh := schedutil.NewErrorChannel() + // Run NormalizeScore method for each ScoreWithNormalizePlugin in parallel. workqueue.ParallelizeUntil(ctx, 16, len(f.scoreWithNormalizePlugins), func(index int) { pl := f.scoreWithNormalizePlugins[index] - nodeScoreList, ok := scores[pl.Name()] - if !ok { - err := fmt.Errorf("normalize score plugin %q has no corresponding scores in the PluginToNodeScores", pl.Name()) - errCh.SendErrorWithCancel(err, cancel) - return - } + nodeScoreList := pluginToNodeScores[pl.Name()] status := pl.NormalizeScore(pc, pod, nodeScoreList) if !status.IsSuccess() { err := fmt.Errorf("normalize score plugin %q failed with error %v", pl.Name(), status.Message()) @@ -406,51 +393,36 @@ func (f *framework) RunNormalizeScorePlugins(pc *PluginContext, pod *v1.Pod, sco return } }) - if err := errCh.ReceiveError(); err != nil { msg := fmt.Sprintf("error while running normalize score plugin for pod %q: %v", pod.Name, err) klog.Error(msg) - return NewStatus(Error, msg) + return nil, NewStatus(Error, msg) } - return nil -} - -// ApplyScoreWeights applies weights to the score results. It should be called after -// RunNormalizeScorePlugins. -func (f *framework) ApplyScoreWeights(pc *PluginContext, pod *v1.Pod, scores PluginToNodeScores) *Status { - ctx, cancel := context.WithCancel(context.Background()) - errCh := schedutil.NewErrorChannel() + // Apply score defaultWeights for each ScorePlugin in parallel. workqueue.ParallelizeUntil(ctx, 16, len(f.scorePlugins), func(index int) { pl := f.scorePlugins[index] // Score plugins' weight has been checked when they are initialized. weight := f.pluginNameToWeightMap[pl.Name()] - nodeScoreList, ok := scores[pl.Name()] - if !ok { - err := fmt.Errorf("score plugin %q has no corresponding scores in the PluginToNodeScores", pl.Name()) - errCh.SendErrorWithCancel(err, cancel) - return - } + nodeScoreList := pluginToNodeScores[pl.Name()] for i, nodeScore := range nodeScoreList { // return error if score plugin returns invalid score. if nodeScore.Score > MaxNodeScore || nodeScore.Score < MinNodeScore { - err := fmt.Errorf("score plugin %q returns an invalid score %q, it should in the range of [MinNodeScore, MaxNodeScore] after normalizing", pl.Name(), nodeScore.Score) + err := fmt.Errorf("score plugin %q returns an invalid score %v, it should in the range of [%v, %v] after normalizing", pl.Name(), nodeScore.Score, MinNodeScore, MaxNodeScore) errCh.SendErrorWithCancel(err, cancel) return } - nodeScoreList[i].Score = nodeScore.Score * weight } }) - if err := errCh.ReceiveError(); err != nil { - msg := fmt.Sprintf("error while applying score weights for pod %q: %v", pod.Name, err) + msg := fmt.Sprintf("error while applying score defaultWeights for pod %q: %v", pod.Name, err) klog.Error(msg) - return NewStatus(Error, msg) + return nil, NewStatus(Error, msg) } - return nil + return pluginToNodeScores, nil } // RunPrebindPlugins runs the set of configured prebind plugins. It returns a diff --git a/pkg/scheduler/framework/v1alpha1/framework_test.go b/pkg/scheduler/framework/v1alpha1/framework_test.go index e6478ae21ebf..690b0986b0d5 100644 --- a/pkg/scheduler/framework/v1alpha1/framework_test.go +++ b/pkg/scheduler/framework/v1alpha1/framework_test.go @@ -20,160 +20,98 @@ import ( "reflect" "testing" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/kubernetes/pkg/scheduler/apis/config" ) const ( + scoreWithNormalizePlugin1 = "score-with-normalize-plugin-1" + scoreWithNormalizePlugin2 = "score-with-normalize-plugin-2" scorePlugin1 = "score-plugin-1" - scorePlugin2 = "score-plugin-2" - scorePlugin3 = "score-plugin-3" pluginNotImplementingScore = "plugin-not-implementing-score" - weight1 = 2 - weight2 = 3 - weight3 = 4 ) -// TestScorePlugin1 and 2 implements ScoreWithNormalizePlugin interface, -// TestScorePlugin3 only implements ScorePlugin interface. -var _ = ScoreWithNormalizePlugin(&TestScorePlugin1{}) -var _ = ScoreWithNormalizePlugin(&TestScorePlugin2{}) -var _ = ScorePlugin(&TestScorePlugin3{}) +// TestScoreWithNormalizePlugin implements ScoreWithNormalizePlugin interface. +// TestScorePlugin only implements ScorePlugin interface. +var _ = ScoreWithNormalizePlugin(&TestScoreWithNormalizePlugin{}) +var _ = ScorePlugin(&TestScorePlugin{}) -type TestScorePlugin1 struct { - // If fail is true, NormalizeScore will return error status. - fail bool +func newScoreWithNormalizePlugin1(inj injectedResult) Plugin { + return &TestScoreWithNormalizePlugin{scoreWithNormalizePlugin1, inj} } - -// NewScorePlugin1 is the factory for NormalizeScore plugin 1. -func NewScorePlugin1(_ *runtime.Unknown, _ FrameworkHandle) (Plugin, error) { - return &TestScorePlugin1{}, nil +func newScoreWithNormalizePlugin2(inj injectedResult) Plugin { + return &TestScoreWithNormalizePlugin{scoreWithNormalizePlugin2, inj} } - -// NewScorePlugin1InjectFailure creates a new TestScorePlugin1 which will -// return an error status for NormalizeScore. -func NewScorePlugin1InjectFailure(_ *runtime.Unknown, _ FrameworkHandle) (Plugin, error) { - return &TestScorePlugin1{fail: true}, nil +func newScorePlugin1(inj injectedResult) Plugin { + return &TestScorePlugin{scorePlugin1, inj} } - -func (pl *TestScorePlugin1) Name() string { - return scorePlugin1 +func newPluginNotImplementingScore(injectedResult) Plugin { + return &PluginNotImplementingScore{} } -func (pl *TestScorePlugin1) NormalizeScore(pc *PluginContext, pod *v1.Pod, scores NodeScoreList) *Status { - if pl.fail { - return NewStatus(Error, "injecting failure.") - } - // Simply decrease each node score by 1. - for i := range scores { - scores[i].Score = scores[i].Score - 1 - } - return nil +type TestScoreWithNormalizePlugin struct { + name string + inj injectedResult } -func (pl *TestScorePlugin1) Score(pc *PluginContext, p *v1.Pod, nodeName string) (int, *Status) { - // Score is currently not used in the tests so just return some dummy value. - return 0, nil +func (pl *TestScoreWithNormalizePlugin) Name() string { + return pl.name } -type TestScorePlugin2 struct{} - -// NewScorePlugin2 is the factory for NormalizeScore plugin 2. -func NewScorePlugin2(_ *runtime.Unknown, _ FrameworkHandle) (Plugin, error) { - return &TestScorePlugin2{}, nil +func (pl *TestScoreWithNormalizePlugin) NormalizeScore(pc *PluginContext, pod *v1.Pod, scores NodeScoreList) *Status { + return injectNormalizeRes(pl.inj, scores) } -func (pl *TestScorePlugin2) Name() string { - return scorePlugin2 +func (pl *TestScoreWithNormalizePlugin) Score(pc *PluginContext, p *v1.Pod, nodeName string) (int, *Status) { + return setScoreRes(pl.inj) } -func (pl *TestScorePlugin2) NormalizeScore(pc *PluginContext, pod *v1.Pod, scores NodeScoreList) *Status { - // Simply force each node score to 5. - for i := range scores { - scores[i].Score = 5 - } - return nil +// TestScorePlugin only implements ScorePlugin interface. +type TestScorePlugin struct { + name string + inj injectedResult } -func (pl *TestScorePlugin2) Score(pc *PluginContext, p *v1.Pod, nodeName string) (int, *Status) { - // Score is currently not used in the tests so just return some dummy value. - return 0, nil +func (pl *TestScorePlugin) Name() string { + return pl.name } -// TestScorePlugin3 only implements ScorePlugin interface. -type TestScorePlugin3 struct{} - -// NewScorePlugin3 is the factory for Score plugin 3. -func NewScorePlugin3(_ *runtime.Unknown, _ FrameworkHandle) (Plugin, error) { - return &TestScorePlugin3{}, nil -} - -func (pl *TestScorePlugin3) Name() string { - return scorePlugin3 -} - -func (pl *TestScorePlugin3) Score(pc *PluginContext, p *v1.Pod, nodeName string) (int, *Status) { - // Score is currently not used in the tests so just return some dummy value. - return 0, nil +func (pl *TestScorePlugin) Score(pc *PluginContext, p *v1.Pod, nodeName string) (int, *Status) { + return setScoreRes(pl.inj) } // PluginNotImplementingScore doesn't implement the ScorePlugin interface. type PluginNotImplementingScore struct{} -// NewPluginNotImplementingScore is the factory for PluginNotImplementingScore. -func NewPluginNotImplementingScore(_ *runtime.Unknown, _ FrameworkHandle) (Plugin, error) { - return &PluginNotImplementingScore{}, nil -} - func (pl *PluginNotImplementingScore) Name() string { return pluginNotImplementingScore } -var registry = Registry{ - scorePlugin1: NewScorePlugin1, - scorePlugin2: NewScorePlugin2, - scorePlugin3: NewScorePlugin3, - pluginNotImplementingScore: NewPluginNotImplementingScore, +var defaultConstructors = map[string]func(injectedResult) Plugin{ + scoreWithNormalizePlugin1: newScoreWithNormalizePlugin1, + scoreWithNormalizePlugin2: newScoreWithNormalizePlugin2, + scorePlugin1: newScorePlugin1, + pluginNotImplementingScore: newPluginNotImplementingScore, } -var plugin1 = &config.Plugins{ - Score: &config.PluginSet{ - Enabled: []config.Plugin{ - {Name: scorePlugin1, Weight: weight1}, - }, - }, -} -var plugin3 = &config.Plugins{ - Score: &config.PluginSet{ - Enabled: []config.Plugin{ - {Name: scorePlugin3, Weight: weight3}, - }, - }, -} -var plugin1And2 = &config.Plugins{ - Score: &config.PluginSet{ - Enabled: []config.Plugin{ - {Name: scorePlugin1, Weight: weight1}, - {Name: scorePlugin2, Weight: weight2}, - }, - }, -} -var plugin1And3 = &config.Plugins{ - Score: &config.PluginSet{ - Enabled: []config.Plugin{ - {Name: scorePlugin1, Weight: weight1}, - {Name: scorePlugin3, Weight: weight3}, - }, - }, +var defaultWeights = map[string]int32{ + scoreWithNormalizePlugin1: 1, + scoreWithNormalizePlugin2: 2, + scorePlugin1: 1, } // No specific config required. -var args = []config.PluginConfig{} +var args []config.PluginConfig var pc = &PluginContext{} // Pod is only used for logging errors. var pod = &v1.Pod{} +var nodes = []*v1.Node{ + {ObjectMeta: metav1.ObjectMeta{Name: "node1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "node2"}}, +} func TestInitFrameworkWithScorePlugins(t *testing.T) { tests := []struct { @@ -183,25 +121,13 @@ func TestInitFrameworkWithScorePlugins(t *testing.T) { initErr bool }{ { - name: "enabled Score plugin doesn't exist in registry", - plugins: &config.Plugins{ - Score: &config.PluginSet{ - Enabled: []config.Plugin{ - {Name: "notExist"}, - }, - }, - }, + name: "enabled Score plugin doesn't exist in registry", + plugins: buildConfigDefaultWeights("notExist"), initErr: true, }, { - name: "enabled Score plugin doesn't extend the ScorePlugin interface", - plugins: &config.Plugins{ - Score: &config.PluginSet{ - Enabled: []config.Plugin{ - {Name: pluginNotImplementingScore}, - }, - }, - }, + name: "enabled Score plugin doesn't extend the ScorePlugin interface", + plugins: buildConfigDefaultWeights(pluginNotImplementingScore), initErr: true, }, { @@ -209,38 +135,22 @@ func TestInitFrameworkWithScorePlugins(t *testing.T) { plugins: &config.Plugins{Score: nil}, }, { - name: "enabled Score plugin list is empty", - plugins: &config.Plugins{ - Score: &config.PluginSet{ - Enabled: []config.Plugin{}, - }, - }, + name: "enabled Score plugin list is empty", + plugins: buildConfigDefaultWeights(), }, { - name: "enabled plugin only implements ScorePlugin interface", - plugins: &config.Plugins{ - Score: &config.PluginSet{ - Enabled: []config.Plugin{ - {Name: scorePlugin3}, - }, - }, - }, + name: "enabled plugin only implements ScorePlugin interface", + plugins: buildConfigDefaultWeights(scorePlugin1), }, { - name: "enabled plugin implements ScoreWithNormalizePlugin interface", - plugins: &config.Plugins{ - Score: &config.PluginSet{ - Enabled: []config.Plugin{ - {Name: scorePlugin1}, - }, - }, - }, + name: "enabled plugin implements ScoreWithNormalizePlugin interface", + plugins: buildConfigDefaultWeights(scoreWithNormalizePlugin1), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := NewFramework(registry, tt.plugins, args) + _, err := NewFramework(toRegistry(defaultConstructors, make(map[string]injectedResult)), tt.plugins, args) if tt.initErr && err == nil { t.Fatal("Framework initialization should fail") } @@ -251,410 +161,106 @@ func TestInitFrameworkWithScorePlugins(t *testing.T) { } } -func TestRunNormalizeScorePlugins(t *testing.T) { +func TestRunScorePlugins(t *testing.T) { tests := []struct { - name string - registry Registry - plugins *config.Plugins - input PluginToNodeScores - want PluginToNodeScores - // If err is true, we expect RunNormalizeScorePlugin to fail. + name string + registry Registry + plugins *config.Plugins + injectedRes map[string]injectedResult + want PluginToNodeScores + // If err is true, we expect RunScorePlugin to fail. err bool }{ { - name: "no NormalizeScore plugins", - plugins: plugin3, - registry: registry, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 3, - }, - }, - }, - // No NormalizeScore plugin, map should be untouched. - want: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 3, - }, - }, - }, + name: "no Score plugins", + plugins: buildConfigDefaultWeights(), + want: PluginToNodeScores{}, }, { - name: "single Score plugin, single NormalizeScore plugin", - registry: registry, - plugins: plugin1, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 3, - }, - }, + name: "single Score plugin", + plugins: buildConfigDefaultWeights(scorePlugin1), + injectedRes: map[string]injectedResult{ + scorePlugin1: {scoreRes: 1}, }, + // scorePlugin1 Score returns 1, weight=1, so want=1. want: PluginToNodeScores{ - // For plugin1, want=input-1. - scorePlugin1: { - { - Name: "node1", - Score: 1, - }, - { - Name: "node2", - Score: 2, - }, - }, + scorePlugin1: {{Name: "node1", Score: 1}, {Name: "node2", Score: 1}}, }, }, { - name: "2 Score plugins, 2 NormalizeScore plugins", - registry: registry, - plugins: plugin1And2, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 3, - }, - }, - scorePlugin2: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 4, - }, - }, + name: "single ScoreWithNormalize plugin", + //registry: registry, + plugins: buildConfigDefaultWeights(scoreWithNormalizePlugin1), + injectedRes: map[string]injectedResult{ + scoreWithNormalizePlugin1: {scoreRes: 10, normalizeRes: 5}, }, + // scoreWithNormalizePlugin1 Score returns 10, but NormalizeScore overrides to 5, weight=1, so want=5 want: PluginToNodeScores{ - // For plugin1, want=input-1. - scorePlugin1: { - { - Name: "node1", - Score: 1, - }, - { - Name: "node2", - Score: 2, - }, - }, - // For plugin2, want=5. - scorePlugin2: { - { - Name: "node1", - Score: 5, - }, - { - Name: "node2", - Score: 5, - }, - }, + scoreWithNormalizePlugin1: {{Name: "node1", Score: 5}, {Name: "node2", Score: 5}}, }, }, { - name: "2 Score plugins, 1 NormalizeScore plugin", - registry: registry, - plugins: plugin1And3, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 3, - }, - }, - scorePlugin2: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 4, - }, - }, + name: "2 Score plugins, 2 NormalizeScore plugins", + plugins: buildConfigDefaultWeights(scorePlugin1, scoreWithNormalizePlugin1, scoreWithNormalizePlugin2), + injectedRes: map[string]injectedResult{ + scorePlugin1: {scoreRes: 1}, + scoreWithNormalizePlugin1: {scoreRes: 3, normalizeRes: 4}, + scoreWithNormalizePlugin2: {scoreRes: 4, normalizeRes: 5}, }, + // scorePlugin1 Score returns 1, weight =1, so want=1. + // scoreWithNormalizePlugin1 Score returns 3, but NormalizeScore overrides to 4, weight=1, so want=4. + // scoreWithNormalizePlugin2 Score returns 4, but NormalizeScore overrides to 5, weight=2, so want=10. want: PluginToNodeScores{ - // For plugin1, want=input-1. - scorePlugin1: { - { - Name: "node1", - Score: 1, - }, - { - Name: "node2", - Score: 2, - }, - }, - // No NormalizeScore for plugin 3. The node scores are untouched. - scorePlugin2: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 4, - }, - }, + scorePlugin1: {{Name: "node1", Score: 1}, {Name: "node2", Score: 1}}, + scoreWithNormalizePlugin1: {{Name: "node1", Score: 4}, {Name: "node2", Score: 4}}, + scoreWithNormalizePlugin2: {{Name: "node1", Score: 10}, {Name: "node2", Score: 10}}, }, }, { - name: "score map contains both test plugin 1 and 2 but plugin 1 fails", - registry: Registry{ - scorePlugin1: NewScorePlugin1InjectFailure, - scorePlugin2: NewScorePlugin2, + name: "score fails", + injectedRes: map[string]injectedResult{ + scoreWithNormalizePlugin1: {scoreErr: true}, }, - plugins: plugin1And2, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 3, - }, - }, - scorePlugin2: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 4, - }, - }, - }, - err: true, + plugins: buildConfigDefaultWeights(scorePlugin1, scoreWithNormalizePlugin1), + err: true, }, { - name: "2 plugins but score map only contains plugin1", - registry: registry, - plugins: plugin1And2, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 3, - }, - }, + name: "normalize fails", + injectedRes: map[string]injectedResult{ + scoreWithNormalizePlugin1: {normalizeErr: true}, }, - err: true, + plugins: buildConfigDefaultWeights(scorePlugin1, scoreWithNormalizePlugin1), + err: true, }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - f, err := NewFramework(tt.registry, tt.plugins, args) - if err != nil { - t.Fatalf("Failed to create framework for testing: %v", err) - } - - status := f.RunNormalizeScorePlugins(pc, pod, tt.input) - - if tt.err { - if status.IsSuccess() { - t.Errorf("Expected status to be non-success.") - } - } else { - if !status.IsSuccess() { - t.Errorf("Expected status to be success.") - } - if !reflect.DeepEqual(tt.input, tt.want) { - t.Errorf("Score map after RunNormalizeScorePlugin: %+v, want: %+v.", tt.input, tt.want) - } - } - }) - } -} - -func TestApplyScoreWeights(t *testing.T) { - tests := []struct { - name string - plugins *config.Plugins - input PluginToNodeScores - want PluginToNodeScores - // If err is true, we expect ApplyScoreWeights to fail. - err bool - }{ { - name: "single Score plugin, single nodeScoreList", - plugins: plugin1, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 3, - }, - }, - }, - want: PluginToNodeScores{ - // For plugin1, want=input*weight1. - scorePlugin1: { - { - Name: "node1", - Score: 4, - }, - { - Name: "node2", - Score: 6, - }, - }, - }, - }, - { - name: "2 Score plugins, 2 nodeScoreLists in scoreMap", - plugins: plugin1And2, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 3, - }, - }, - scorePlugin2: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 4, - }, - }, - }, - want: PluginToNodeScores{ - // For plugin1, want=input*weight1. - scorePlugin1: { - { - Name: "node1", - Score: 4, - }, - { - Name: "node2", - Score: 6, - }, - }, - // For plugin2, want=input*weight2. - scorePlugin2: { - { - Name: "node1", - Score: 6, - }, - { - Name: "node2", - Score: 12, - }, - }, + name: "Score plugin return score greater than MaxNodeScore", + plugins: buildConfigDefaultWeights(scorePlugin1), + injectedRes: map[string]injectedResult{ + scorePlugin1: {scoreRes: MaxNodeScore + 1}, }, + err: true, }, { - name: "2 Score plugins, 1 without corresponding nodeScoreList in the score map", - plugins: plugin1And2, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: 2, - }, - { - Name: "node2", - Score: 3, - }, - }, + name: "Score plugin return score less than MinNodeScore", + plugins: buildConfigDefaultWeights(scorePlugin1), + injectedRes: map[string]injectedResult{ + scorePlugin1: {scoreRes: MinNodeScore - 1}, }, err: true, }, { - name: "Score plugin return score greater than MaxNodeScore", - plugins: plugin1And2, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: MaxNodeScore + 1, - }, - { - Name: "node2", - Score: 3, - }, - }, - scorePlugin2: { - { - Name: "node1", - Score: MinNodeScore, - }, - { - Name: "node2", - Score: 5, - }, - }, + name: "ScoreWithNormalize plugin return score greater than MaxNodeScore", + plugins: buildConfigDefaultWeights(scoreWithNormalizePlugin1), + injectedRes: map[string]injectedResult{ + scoreWithNormalizePlugin1: {normalizeRes: MaxNodeScore + 1}, }, err: true, }, { - name: "Score plugin return score less than MinNodeScore", - plugins: plugin1And2, - input: PluginToNodeScores{ - scorePlugin1: { - { - Name: "node1", - Score: MaxNodeScore, - }, - { - Name: "node2", - Score: 3, - }, - }, - scorePlugin2: { - { - Name: "node1", - Score: MinNodeScore - 1, - }, - { - Name: "node2", - Score: 5, - }, - }, + name: "ScoreWithNormalize plugin return score less than MinNodeScore", + plugins: buildConfigDefaultWeights(scoreWithNormalizePlugin1), + injectedRes: map[string]injectedResult{ + scoreWithNormalizePlugin1: {normalizeRes: MinNodeScore - 1}, }, err: true, }, @@ -662,25 +268,76 @@ func TestApplyScoreWeights(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Inject the results for each plugin. + registry := toRegistry(defaultConstructors, tt.injectedRes) f, err := NewFramework(registry, tt.plugins, args) if err != nil { t.Fatalf("Failed to create framework for testing: %v", err) } - status := f.ApplyScoreWeights(pc, pod, tt.input) + res, status := f.RunScorePlugins(pc, pod, nodes) if tt.err { if status.IsSuccess() { - t.Errorf("Expected status to be non-success.") - } - } else { - if !status.IsSuccess() { - t.Errorf("Expected status to be success.") - } - if !reflect.DeepEqual(tt.input, tt.want) { - t.Errorf("Score map after RunNormalizeScorePlugin: %+v, want: %+v.", tt.input, tt.want) + t.Error("Expected status to be non-success.") } + return + } + + if !status.IsSuccess() { + t.Errorf("Expected status to be success.") + } + if !reflect.DeepEqual(res, tt.want) { + t.Errorf("Score map after RunScorePlugin: %+v, want: %+v.", res, tt.want) } }) } } + +func toRegistry(constructors map[string]func(injectedResult) Plugin, injectedRes map[string]injectedResult) Registry { + registry := make(Registry) + for pl, f := range constructors { + npl := pl + nf := f + registry[pl] = func(_ *runtime.Unknown, _ FrameworkHandle) (Plugin, error) { + return nf(injectedRes[npl]), nil + } + } + return registry +} + +func buildConfigDefaultWeights(ps ...string) *config.Plugins { + return buildConfigWithWeights(defaultWeights, ps...) +} + +func buildConfigWithWeights(weights map[string]int32, ps ...string) *config.Plugins { + var plugins []config.Plugin + for _, p := range ps { + plugins = append(plugins, config.Plugin{Name: p, Weight: weights[p]}) + } + return &config.Plugins{Score: &config.PluginSet{Enabled: plugins}} +} + +type injectedResult struct { + scoreRes int + normalizeRes int + scoreErr bool + normalizeErr bool +} + +func setScoreRes(inj injectedResult) (int, *Status) { + if inj.scoreErr { + return 0, NewStatus(Error, "injecting failure.") + } + return inj.scoreRes, nil +} + +func injectNormalizeRes(inj injectedResult, scores NodeScoreList) *Status { + if inj.normalizeErr { + return NewStatus(Error, "injecting failure.") + } + for i := range scores { + scores[i].Score = inj.normalizeRes + } + return nil +} diff --git a/pkg/scheduler/framework/v1alpha1/interface.go b/pkg/scheduler/framework/v1alpha1/interface.go index cde7300889e5..70c59490c36b 100644 --- a/pkg/scheduler/framework/v1alpha1/interface.go +++ b/pkg/scheduler/framework/v1alpha1/interface.go @@ -311,16 +311,6 @@ type Framework interface { // a non-success status. RunScorePlugins(pc *PluginContext, pod *v1.Pod, nodes []*v1.Node) (PluginToNodeScores, *Status) - // RunNormalizeScorePlugins runs the normalize score plugins. It should be called after - // RunScorePlugins with the PluginToNodeScores result. It then modifies the map with - // normalized scores. It returns a non-success Status if any of the normalize score plugins - // returns a non-success status. - RunNormalizeScorePlugins(pc *PluginContext, pod *v1.Pod, scores PluginToNodeScores) *Status - - // ApplyScoreWeights applies weights to the score results. It should be called after - // RunNormalizeScorePlugins. - ApplyScoreWeights(pc *PluginContext, pod *v1.Pod, scores PluginToNodeScores) *Status - // RunPrebindPlugins runs the set of configured prebind plugins. It returns // *Status and its code is set to non-success if any of the plugins returns // anything but Success. If the Status code is "Unschedulable", it is diff --git a/pkg/scheduler/internal/queue/scheduling_queue_test.go b/pkg/scheduler/internal/queue/scheduling_queue_test.go index 402b3c4575bc..1bdce2c517c5 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue_test.go +++ b/pkg/scheduler/internal/queue/scheduling_queue_test.go @@ -179,14 +179,6 @@ func (*fakeFramework) RunScorePlugins(pc *framework.PluginContext, pod *v1.Pod, return nil, nil } -func (*fakeFramework) RunNormalizeScorePlugins(pc *framework.PluginContext, pod *v1.Pod, scores framework.PluginToNodeScores) *framework.Status { - return nil -} - -func (*fakeFramework) ApplyScoreWeights(pc *framework.PluginContext, pod *v1.Pod, scores framework.PluginToNodeScores) *framework.Status { - return nil -} - func (*fakeFramework) RunPrebindPlugins(pc *framework.PluginContext, pod *v1.Pod, nodeName string) *framework.Status { return nil }