Fix max pods to evict per node.

aveshagarwal · Apr 26, 2018 · 7e2c551 · 7e2c551
1 parent e42e372
commit 7e2c551
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 46 deletions.
diff --git a/pkg/descheduler/strategies/duplicates.go b/pkg/descheduler/strategies/duplicates.go
@@ -54,7 +54,7 @@ func deleteDuplicatePods(client clientset.Interface, policyGroupVersion string,
 				glog.V(1).Infof("%#v", creator)
 				// i = 0 does not evict the first pod
 				for i := 1; i < len(pods); i++ {
-					if nodepodCount[node]+1 > maxPodsToEvict {
+					if maxPodsToEvict > 0 && nodepodCount[node]+1 > maxPodsToEvict {
 						break
 					}
 					success, err := evictions.EvictPod(client, pods[i], policyGroupVersion, dryRun)

diff --git a/pkg/descheduler/strategies/lownodeutilization.go b/pkg/descheduler/strategies/lownodeutilization.go
@@ -218,7 +218,7 @@ func evictPods(inputPods []*v1.Pod,
 	if IsNodeAboveTargetUtilization(nodeUsage, targetThresholds) && (*totalPods > 0 || *totalCpu > 0 || *totalMem > 0) {
 		onePodPercentage := api.Percentage((float64(1) * 100) / float64(nodeCapacity.Pods().Value()))
 		for _, pod := range inputPods {
-			if *podsEvicted+1 > maxPodsToEvict {
+			if maxPodsToEvict > 0 && *podsEvicted+1 > maxPodsToEvict {
 				break
 			}
 			cUsage := helper.GetResourceRequest(pod, v1.ResourceCPU)

diff --git a/pkg/descheduler/strategies/node_affinity.go b/pkg/descheduler/strategies/node_affinity.go
@@ -51,7 +51,7 @@ func removePodsViolatingNodeAffinityCount(ds *options.DeschedulerServer, strateg
 				}
 
 				for _, pod := range pods {
-					if nodepodCount[node]+1 > maxPodsToEvict {
+					if maxPodsToEvict > 0 && nodepodCount[node]+1 > maxPodsToEvict {
 						break
 					}
 					if pod.Spec.Affinity != nil && pod.Spec.Affinity.NodeAffinity != nil && pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil {

diff --git a/pkg/descheduler/strategies/node_affinity_test.go b/pkg/descheduler/strategies/node_affinity_test.go
@@ -143,16 +143,16 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 			pods:                    addPodsToNode(nodeWithoutLabels),
 			nodes:                   []*v1.Node{nodeWithoutLabels, nodeWithLabels},
 			npe:                     nodePodEvictedCount{nodeWithoutLabels: 0, nodeWithLabels: 0},
-			maxPodsToEvict:          1,
+			maxPodsToEvict:          0,
 		},
 		{
-			description:             "Pod is scheduled on node without matching labels, another schedulable node available, maxPodsToEvict set to 0, should not be evicted",
-			expectedEvictedPodCount: 0,
+			description:             "Pod is scheduled on node without matching labels, another schedulable node available, maxPodsToEvict set to 1, should not be evicted",
+			expectedEvictedPodCount: 1,
 			strategy:                requiredDuringSchedulingIgnoredDuringExecutionStrategy,
 			pods:                    addPodsToNode(nodeWithoutLabels),
 			nodes:                   []*v1.Node{nodeWithoutLabels, nodeWithLabels},
 			npe:                     nodePodEvictedCount{nodeWithoutLabels: 0, nodeWithLabels: 0},
-			maxPodsToEvict:          0,
+			maxPodsToEvict:          1,
 		},
 		{
 			description:             "Pod is scheduled on node without matching labels, but no node where pod fits is available, should not evict",

diff --git a/pkg/descheduler/strategies/pod_antiaffinity.go b/pkg/descheduler/strategies/pod_antiaffinity.go
@@ -49,7 +49,7 @@ func removePodsWithAffinityRules(client clientset.Interface, policyGroupVersion
 		}
 		totalPods := len(pods)
 		for i := 0; i < totalPods; i++ {
-			if nodePodCount[node]+1 > maxPodsToEvict {
+			if maxPodsToEvict > 0 && nodePodCount[node]+1 > maxPodsToEvict {
 				break
 			}
 			if checkPodsWithAntiAffinityExist(pods[i], pods) {

diff --git a/pkg/descheduler/strategies/pod_antiaffinity_test.go b/pkg/descheduler/strategies/pod_antiaffinity_test.go
@@ -32,29 +32,43 @@ func TestPodAntiAffinity(t *testing.T) {
 	p1 := test.BuildTestPod("p1", 100, 0, node.Name)
 	p2 := test.BuildTestPod("p2", 100, 0, node.Name)
 	p3 := test.BuildTestPod("p3", 100, 0, node.Name)
-	p3.Labels = map[string]string{"foo": "bar"}
+	p4 := test.BuildTestPod("p4", 100, 0, node.Name)
+	p2.Labels = map[string]string{"foo": "bar"}
 	p1.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
 	p2.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
 	p3.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
-	p1.Spec.Affinity = &v1.Affinity{
-		PodAntiAffinity: &v1.PodAntiAffinity{
-			RequiredDuringSchedulingIgnoredDuringExecution: []v1.PodAffinityTerm{
-				{
-					LabelSelector: &metav1.LabelSelector{
-						MatchExpressions: []metav1.LabelSelectorRequirement{
-							{
-								Key:      "foo",
-								Operator: metav1.LabelSelectorOpIn,
-								Values:   []string{"bar"},
-							},
-						},
-					},
-					TopologyKey: "region",
-				},
-			},
-		},
+	p4.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
+
+	// set pod anti affinity
+	setPodAntiAffinity(p1)
+	setPodAntiAffinity(p3)
+	setPodAntiAffinity(p4)
+
+	// create fake client
+	fakeClient := &fake.Clientset{}
+	fakeClient.Fake.AddReactor("list", "pods", func(action core.Action) (bool, runtime.Object, error) {
+		return true, &v1.PodList{Items: []v1.Pod{*p1, *p2, *p3, *p4}}, nil
+	})
+	fakeClient.Fake.AddReactor("get", "nodes", func(action core.Action) (bool, runtime.Object, error) {
+		return true, node, nil
+	})
+	npe := nodePodEvictedCount{}
+	npe[node] = 0
+	expectedEvictedPodCount := 3
+	podsEvicted := removePodsWithAffinityRules(fakeClient, "v1", []*v1.Node{node}, false, npe, 0)
+	if podsEvicted != expectedEvictedPodCount {
+		t.Errorf("Unexpected no of pods evicted: pods evicted: %d, expected: %d", podsEvicted, expectedEvictedPodCount)
 	}
-	p3.Spec.Affinity = &v1.Affinity{
+	npe[node] = 0
+	expectedEvictedPodCount = 1
+	podsEvicted = removePodsWithAffinityRules(fakeClient, "v1", []*v1.Node{node}, false, npe, 1)
+	if podsEvicted != expectedEvictedPodCount {
+		t.Errorf("Unexpected no of pods evicted: pods evicted: %d, expected: %d", podsEvicted, expectedEvictedPodCount)
+	}
+}
+
+func setPodAntiAffinity(inputPod *v1.Pod) {
+	inputPod.Spec.Affinity = &v1.Affinity{
 		PodAntiAffinity: &v1.PodAntiAffinity{
 			RequiredDuringSchedulingIgnoredDuringExecution: []v1.PodAffinityTerm{
 				{
@@ -72,23 +86,4 @@ func TestPodAntiAffinity(t *testing.T) {
 			},
 		},
 	}
-	fakeClient := &fake.Clientset{}
-	fakeClient.Fake.AddReactor("list", "pods", func(action core.Action) (bool, runtime.Object, error) {
-		return true, &v1.PodList{Items: []v1.Pod{*p1, *p2, *p3}}, nil
-	})
-	fakeClient.Fake.AddReactor("get", "nodes", func(action core.Action) (bool, runtime.Object, error) {
-		return true, node, nil
-	})
-	npe := nodePodEvictedCount{}
-	npe[node] = 0
-	expectedEvictedPodCount := 0
-	podsEvicted := removePodsWithAffinityRules(fakeClient, "v1", []*v1.Node{node}, false, npe, 0)
-	if podsEvicted != expectedEvictedPodCount {
-		t.Errorf("Unexpected no of pods evicted")
-	}
-	expectedEvictedPodCount = 1
-	podsEvicted = removePodsWithAffinityRules(fakeClient, "v1", []*v1.Node{node}, false, npe, 1)
-	if podsEvicted != expectedEvictedPodCount {
-		t.Errorf("Unexpected no of pods evicted")
-	}
 }