From ef1fab76421a1c3b832ef728a66c331a81d1ec18 Mon Sep 17 00:00:00 2001 From: Shingo Omura Date: Wed, 29 Jul 2020 23:02:53 +0900 Subject: [PATCH 1/2] expose Run[Pre]ScorePlugins functions in PluginRunner interface --- pkg/scheduler/framework/v1alpha1/interface.go | 4 ++ test/integration/scheduler/framework_test.go | 55 ++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/pkg/scheduler/framework/v1alpha1/interface.go b/pkg/scheduler/framework/v1alpha1/interface.go index 8427f9b7f7f..612707dae8b 100644 --- a/pkg/scheduler/framework/v1alpha1/interface.go +++ b/pkg/scheduler/framework/v1alpha1/interface.go @@ -555,6 +555,10 @@ type PodNominator interface { // This is used by preemption PostFilter plugins when evaluating the feasibility of // scheduling the pod on nodes when certain running pods get evicted. type PluginsRunner interface { + // RunPreScorePlugins runs the set of configured PreScore plugins for pod on the given nodes + RunPreScorePlugins(context.Context, *CycleState, *v1.Pod, []*v1.Node) *Status + // RunScorePlugins runs the set of configured Score plugins for pod on the given nodes + RunScorePlugins(context.Context, *CycleState, *v1.Pod, []*v1.Node) (PluginToNodeScores, *Status) // RunFilterPlugins runs the set of configured filter plugins for pod on the given node. RunFilterPlugins(context.Context, *CycleState, *v1.Pod, *NodeInfo) PluginToStatus // RunPreFilterExtensionAddPod calls the AddPod interface for the set of configured PreFilter plugins. diff --git a/test/integration/scheduler/framework_test.go b/test/integration/scheduler/framework_test.go index 9657f035fd8..c3613f2581a 100644 --- a/test/integration/scheduler/framework_test.go +++ b/test/integration/scheduler/framework_test.go @@ -407,10 +407,17 @@ func (pp *PostFilterPlugin) PostFilter(ctx context.Context, state *framework.Cyc if err != nil { return nil, framework.NewStatus(framework.Error, err.Error()) } + ph := pp.fh.PreemptHandle() for _, nodeInfo := range nodeInfos { ph.RunFilterPlugins(ctx, state, pod, nodeInfo) } + var nodes []*v1.Node + for _, nodeInfo := range nodeInfos { + nodes = append(nodes, nodeInfo.Node()) + } + ph.RunScorePlugins(ctx, state, pod, nodes) + if pp.failPostFilter { return nil, framework.NewStatus(framework.Error, fmt.Sprintf("injecting failure for pod %v", pod.Name)) } @@ -578,30 +585,52 @@ func TestPostFilterPlugin(t *testing.T) { numNodes := 1 tests := []struct { name string + numNodes int rejectFilter bool + failScore bool rejectPostFilter bool expectFilterNumCalled int + expectScoreNumCalled int32 expectPostFilterNumCalled int }{ { - name: "Filter passed", + name: "Filter passed and Score success", + numNodes: 3, rejectFilter: false, + failScore: false, rejectPostFilter: false, - expectFilterNumCalled: numNodes, + expectFilterNumCalled: 3, + expectScoreNumCalled: 3, expectPostFilterNumCalled: 0, }, { name: "Filter failed and PostFilter passed", + numNodes: numNodes, rejectFilter: true, + failScore: false, rejectPostFilter: false, expectFilterNumCalled: numNodes * 2, + expectScoreNumCalled: 1, expectPostFilterNumCalled: 1, }, { name: "Filter failed and PostFilter failed", + numNodes: numNodes, rejectFilter: true, + failScore: false, rejectPostFilter: true, expectFilterNumCalled: numNodes * 2, + expectScoreNumCalled: 1, + expectPostFilterNumCalled: 1, + }, + { + name: "Score failed and PostFilter failed", + numNodes: numNodes, + rejectFilter: true, + failScore: true, + rejectPostFilter: true, + expectFilterNumCalled: numNodes * 2, + expectScoreNumCalled: 1, expectPostFilterNumCalled: 1, }, } @@ -611,12 +640,15 @@ func TestPostFilterPlugin(t *testing.T) { // Create a plugin registry for testing. Register a combination of filter and postFilter plugin. var ( filterPlugin = &FilterPlugin{} + scorePlugin = &ScorePlugin{} postFilterPlugin = &PostFilterPlugin{} ) filterPlugin.rejectFilter = tt.rejectFilter + scorePlugin.failScore = tt.failScore postFilterPlugin.rejectPostFilter = tt.rejectPostFilter registry := frameworkruntime.Registry{ filterPluginName: newPlugin(filterPlugin), + scorePluginName: newPlugin(scorePlugin), postfilterPluginName: newPostFilterPlugin(postFilterPlugin), } @@ -629,6 +661,16 @@ func TestPostFilterPlugin(t *testing.T) { {Name: filterPluginName}, }, }, + Score: &schedulerconfig.PluginSet{ + Enabled: []schedulerconfig.Plugin{ + {Name: scorePluginName}, + }, + // disable default in-tree Score plugins + // to make it easy to control configured ScorePlugins failure + Disabled: []schedulerconfig.Plugin{ + {Name: "*"}, + }, + }, PostFilter: &schedulerconfig.PluginSet{ Enabled: []schedulerconfig.Plugin{ {Name: postfilterPluginName}, @@ -646,7 +688,7 @@ func TestPostFilterPlugin(t *testing.T) { testCtx := initTestSchedulerForFrameworkTest( t, testutils.InitTestMaster(t, fmt.Sprintf("postfilter%v-", i), nil), - numNodes, + tt.numNodes, scheduler.WithProfiles(prof), scheduler.WithFrameworkOutOfTreeRegistry(registry), ) @@ -662,9 +704,13 @@ func TestPostFilterPlugin(t *testing.T) { if err = wait.Poll(10*time.Millisecond, 10*time.Second, podUnschedulable(testCtx.ClientSet, pod.Namespace, pod.Name)); err != nil { t.Errorf("Didn't expect the pod to be scheduled.") } + if filterPlugin.numFilterCalled < tt.expectFilterNumCalled { t.Errorf("Expected the filter plugin to be called at least %v times, but got %v.", tt.expectFilterNumCalled, filterPlugin.numFilterCalled) } + if numScoreCalled := atomic.LoadInt32(&scorePlugin.numScoreCalled); numScoreCalled < tt.expectScoreNumCalled { + t.Errorf("Expected the score plugin to be called at least %v times, but got %v.", tt.expectScoreNumCalled, numScoreCalled) + } if postFilterPlugin.numPostFilterCalled < tt.expectPostFilterNumCalled { t.Errorf("Expected the postfilter plugin to be called at least %v times, but got %v.", tt.expectPostFilterNumCalled, postFilterPlugin.numPostFilterCalled) } @@ -675,6 +721,9 @@ func TestPostFilterPlugin(t *testing.T) { if filterPlugin.numFilterCalled != tt.expectFilterNumCalled { t.Errorf("Expected the filter plugin to be called %v times, but got %v.", tt.expectFilterNumCalled, filterPlugin.numFilterCalled) } + if numScoreCalled := atomic.LoadInt32(&scorePlugin.numScoreCalled); numScoreCalled != tt.expectScoreNumCalled { + t.Errorf("Expected the score plugin to be called %v times, but got %v.", tt.expectScoreNumCalled, numScoreCalled) + } if postFilterPlugin.numPostFilterCalled != tt.expectPostFilterNumCalled { t.Errorf("Expected the postfilter plugin to be called %v times, but got %v.", tt.expectPostFilterNumCalled, postFilterPlugin.numPostFilterCalled) } From d0421fa9c3eb64b12db1cd6ae8cd353e08a09ed4 Mon Sep 17 00:00:00 2001 From: Shingo Omura Date: Tue, 4 Aug 2020 22:46:22 +0900 Subject: [PATCH 2/2] Fix TestScorePlugin: numScore should be accessed with atomic.LoadInt32 --- test/integration/scheduler/framework_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/scheduler/framework_test.go b/test/integration/scheduler/framework_test.go index c3613f2581a..fa9da2c7e4a 100644 --- a/test/integration/scheduler/framework_test.go +++ b/test/integration/scheduler/framework_test.go @@ -797,7 +797,7 @@ func TestScorePlugin(t *testing.T) { } } - if scorePlugin.numScoreCalled == 0 { + if numScoreCalled := atomic.LoadInt32(&scorePlugin.numScoreCalled); numScoreCalled == 0 { t.Errorf("Expected the score plugin to be called.") }