Merge pull request #87 from aveshagarwal/master-pod-max-issue

Fix max pods to evict per node.

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)

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)

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 {

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",
+			description:             "Pod is scheduled on node without matching labels, another schedulable node available, maxPodsToEvict set to 1, should not be evicted",
-			expectedEvictedPodCount: 0,
+			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",

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) {

pkg/descheduler/strategies/pod_antiaffinity_test.go

@@ -32,63 +32,58 @@ 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{
+	p4.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
-		PodAntiAffinity: &v1.PodAntiAffinity{
+
-			RequiredDuringSchedulingIgnoredDuringExecution: []v1.PodAffinityTerm{
+	// set pod anti affinity
-				{
+	setPodAntiAffinity(p1)
-					LabelSelector: &metav1.LabelSelector{
+	setPodAntiAffinity(p3)
-						MatchExpressions: []metav1.LabelSelectorRequirement{
+	setPodAntiAffinity(p4)
-							{
+
-								Key:      "foo",
+	// create fake client
-								Operator: metav1.LabelSelectorOpIn,
-								Values:   []string{"bar"},
-							},
-						},
-					},
-					TopologyKey: "region",
-				},
-			},
-		},
-	}
-	p3.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",
-				},
-			},
-		},
-	}
 	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
+		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 := 0
+	expectedEvictedPodCount := 3
 	podsEvicted := removePodsWithAffinityRules(fakeClient, "v1", []*v1.Node{node}, false, npe, 0)
 	if podsEvicted != expectedEvictedPodCount {
-		t.Errorf("Unexpected no of pods evicted")
+		t.Errorf("Unexpected no of pods evicted: pods evicted: %d, expected: %d", podsEvicted, expectedEvictedPodCount)
 	}
+	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")
+		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{
+				{
+					LabelSelector: &metav1.LabelSelector{
+						MatchExpressions: []metav1.LabelSelectorRequirement{
+							{
+								Key:      "foo",
+								Operator: metav1.LabelSelectorOpIn,
+								Values:   []string{"bar"},
+							},
+						},
+					},
+					TopologyKey: "region",
+				},
+			},
+		},
 	}
 }