From 4857fc05f8d9f9e0309198090a8228ad225d3fcf Mon Sep 17 00:00:00 2001
From: Shintaro Murakami <mrkm4ntr@gmail.com>
Date: Fri, 8 Nov 2019 13:08:50 +0900
Subject: [PATCH] Add unit test to catch scheduler's node order evaluation
 regressions

---
 pkg/scheduler/core/generic_scheduler.go      |  6 ++--
 pkg/scheduler/core/generic_scheduler_test.go | 29 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go
index 032493d4c1f..dd48972dd26 100644
--- a/pkg/scheduler/core/generic_scheduler.go
+++ b/pkg/scheduler/core/generic_scheduler.go
@@ -165,7 +165,7 @@ type genericScheduler struct {
 	disablePreemption        bool
 	percentageOfNodesToScore int32
 	enableNonPreempting      bool
-	lastProcessedNodeIndex   int
+	nextStartNodeIndex       int
 }
 
 // snapshot snapshots scheduler cache and node infos for all fit and priority
@@ -499,7 +499,7 @@ func (g *genericScheduler) findNodesThatFit(ctx context.Context, state *framewor
 		checkNode := func(i int) {
 			// We check the nodes starting from where we left off in the previous scheduling cycle,
 			// this is to make sure all nodes have the same chance of being examined across pods.
-			nodeInfo := g.nodeInfoSnapshot.NodeInfoList[(g.lastProcessedNodeIndex+i)%allNodes]
+			nodeInfo := g.nodeInfoSnapshot.NodeInfoList[(g.nextStartNodeIndex+i)%allNodes]
 			fits, failedPredicates, status, err := g.podFitsOnNode(
 				ctx,
 				state,
@@ -536,7 +536,7 @@ func (g *genericScheduler) findNodesThatFit(ctx context.Context, state *framewor
 		// are found.
 		workqueue.ParallelizeUntil(ctx, 16, allNodes, checkNode)
 		processedNodes := int(filteredLen) + len(filteredNodesStatuses) + len(failedPredicateMap)
-		g.lastProcessedNodeIndex = (g.lastProcessedNodeIndex + processedNodes) % allNodes
+		g.nextStartNodeIndex = (g.nextStartNodeIndex + processedNodes) % allNodes
 
 		filtered = filtered[:filteredLen]
 		if err := errCh.ReceiveError(); err != nil {
diff --git a/pkg/scheduler/core/generic_scheduler_test.go b/pkg/scheduler/core/generic_scheduler_test.go
index 013e45e0148..aabe92e6328 100644
--- a/pkg/scheduler/core/generic_scheduler_test.go
+++ b/pkg/scheduler/core/generic_scheduler_test.go
@@ -2282,3 +2282,32 @@ func assignDefaultStartTime(pods []*v1.Pod) {
 		}
 	}
 }
+
+func TestFairEvaluationForNodes(t *testing.T) {
+	defer algorithmpredicates.SetPredicatesOrderingDuringTest(order)()
+	predicates := map[string]algorithmpredicates.FitPredicate{"true": truePredicate}
+	numAllNodes := 500
+	nodeNames := make([]string, 0, numAllNodes)
+	for i := 0; i < numAllNodes; i++ {
+		nodeNames = append(nodeNames, strconv.Itoa(i))
+	}
+	nodes := makeNodeList(nodeNames)
+	g := makeScheduler(predicates, nodes)
+	// To make numAllNodes % nodesToFind != 0
+	g.percentageOfNodesToScore = 30
+	nodesToFind := int(g.numFeasibleNodesToFind(int32(numAllNodes)))
+
+	// Iterating over all nodes more than twice
+	for i := 0; i < 2*(numAllNodes/nodesToFind+1); i++ {
+		nodesThatFit, _, _, err := g.findNodesThatFit(context.Background(), framework.NewCycleState(), &v1.Pod{})
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+		if len(nodesThatFit) != nodesToFind {
+			t.Errorf("got %d nodes filtered, want %d", len(nodesThatFit), nodesToFind)
+		}
+		if g.nextStartNodeIndex != (i+1)*nodesToFind%numAllNodes {
+			t.Errorf("got %d lastProcessedNodeIndex, want %d", g.nextStartNodeIndex, (i+1)*nodesToFind%numAllNodes)
+		}
+	}
+}