Merge pull request #66291 from resouer/fix-extender

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Extender preemption should respect IsInterested() **What this PR does / why we need it**: Extender preemption should respect IsInterested() **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes #66289 **Special notes for your reviewer**: The bug is reported and the first commit is co-authored by: @chenchun **Release note**: ```release-note Extender preemption should respect IsInterested() ```

Merge pull request #66291 from resouer/fix-extender
2119d349 · Kubernetes Submit Queue · GitHub · 05cc15f3 · d644162a · 2119d349
Unverified Commit 2119d349 authored Jul 24, 2018 by Kubernetes Submit Queue Committed by GitHub Jul 24, 2018
Hide whitespace changes
Inline Side-by-side

Showing with 26 additions and 2 deletions

generic_scheduler.go pkg/scheduler/core/generic_scheduler.go +1 -1

generic_scheduler_test.go pkg/scheduler/core/generic_scheduler_test.go +25 -1

No files found.
--- a/pkg/scheduler/core/generic_scheduler.go
+++ b/pkg/scheduler/core/generic_scheduler.go
@@ -282,7 +282,7 @@ func (g *genericScheduler) processPreemptionWithExtenders(
 ) (map[*v1.Node]*schedulerapi.Victims, error) {
 	if len(nodeToVictims) > 0 {
 		for _, extender := range g.extenders {
-			if extender.SupportsPreemption() {
+			if extender.SupportsPreemption() && extender.IsInterested(pod) {
 				newNodeToVictims, err := extender.ProcessPreemption(
 					pod,
 					nodeToVictims,

--- a/pkg/scheduler/core/generic_scheduler_test.go
+++ b/pkg/scheduler/core/generic_scheduler_test.go
@@ -1305,6 +1305,30 @@ func TestPreempt(t *testing.T) {
 			expectedNode: "machine1",
 			expectedPods: []string{"m1.1", "m1.2"},
 		},
+		{
+			name: "One scheduler extender allows only machine1, but it is not interested in given pod, otherwise machine1 would have been chosen",
+			pod: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "pod1", UID: types.UID("pod1")}, Spec: v1.PodSpec{
+				Containers: veryLargeContainers,
+				Priority:   &highPriority},
+			},
+			pods: []*v1.Pod{
+				{ObjectMeta: metav1.ObjectMeta{Name: "m1.1", UID: types.UID("m1.1")}, Spec: v1.PodSpec{Containers: smallContainers, Priority: &midPriority, NodeName: "machine1"}, Status: v1.PodStatus{Phase: v1.PodRunning}},
+				{ObjectMeta: metav1.ObjectMeta{Name: "m1.2", UID: types.UID("m1.2")}, Spec: v1.PodSpec{Containers: smallContainers, Priority: &lowPriority, NodeName: "machine1"}, Status: v1.PodStatus{Phase: v1.PodRunning}},
+
+				{ObjectMeta: metav1.ObjectMeta{Name: "m2.1", UID: types.UID("m2.1")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &midPriority, NodeName: "machine2"}, Status: v1.PodStatus{Phase: v1.PodRunning}},
+			},
+			extenders: []*FakeExtender{
+				{
+					predicates:   []fitPredicate{machine1PredicateExtender},
+					unInterested: true,
+				},
+				{
+					predicates: []fitPredicate{truePredicateExtender},
+				},
+			},
+			expectedNode: "machine3",
+			expectedPods: []string{},
+		},
 	}

 	for _, test := range tests {
@@ -1349,7 +1373,7 @@ func TestPreempt(t *testing.T) {
 				t.Errorf("unexpected error in preemption: %v", err)
 			}
 			if (node != nil && node.Name != test.expectedNode) || (node == nil && len(test.expectedNode) != 0) {
-				t.Errorf("expected node: %v, got: %v", test.expectedNode, node)
+				t.Errorf("expected node: %v, got: %v", test.expectedNode, node.GetName())
 			}
 			if len(victims) != len(test.expectedPods) {
 				t.Errorf("expected %v pods, got %v.", len(test.expectedPods), len(victims))