Add unit tests for inter-pod anti-affinity check

* Test logic in GroupByNodeName
* Test NodeFit() case where pods matches inter-pod anti-affinity
* Test for inter-pod anti-affinity pods  match terms, have label selector

Author: nikimanoledaki

pkg/descheduler/node/node_test.go

@@ -754,15 +754,11 @@ func TestPodFitsAnyOtherNode(t *testing.T) {
 }
 
 func TestNodeFit(t *testing.T) {
-	node := test.BuildTestNode("node", 64000, 128*1000*1000*1000, 2, nil)
-
-	// pod anti-affinity pods
-	podAntiAffinityPod1 := test.BuildTestPod("p1", 1000, 1000, "node", nil)
-	test.SetPodAntiAffinity(podAntiAffinityPod1, "foo", "bar")
-
-	podAntiAffinityPod2 := test.BuildTestPod("p2", 1000, 1000, "node2", nil)
-	test.SetPodAntiAffinity(podAntiAffinityPod2, "foo", "bar")
-
+	node := test.BuildTestNode("node", 64000, 128*1000*1000*1000, 2, func(node *v1.Node) {
+		node.ObjectMeta.Labels = map[string]string{
+			"region": "main-region",
+		}
+	})
 	tests := []struct {
 		description string
 		pod         *v1.Pod
@@ -791,14 +787,12 @@ func TestNodeFit(t *testing.T) {
 		},
 		{
 			description: "matches inter-pod anti-affinity rule of pod on node",
-			pod:         podAntiAffinityPod1,
-			node: test.BuildTestNode("node2", 2000, 3000, 10, func(node *v1.Node) {
-				node.ObjectMeta.Labels = map[string]string{
-					"region": "main-region",
-				}
-			}),
-			podsOnNode: []*v1.Pod{podAntiAffinityPod2},
-			err:        errors.New("pod matches inter-pod anti-affinity rule of other pod on node"),
+			pod:         test.PodWithPodAntiAffinity(test.BuildTestPod("p1", 1000, 1000, node.Name, nil), "foo", "bar"),
+			node:        node,
+			podsOnNode: []*v1.Pod{
+				test.PodWithPodAntiAffinity(test.BuildTestPod("p2", 1000, 1000, node.Name, nil), "foo", "bar"),
+			},
+			err: errors.New("pod matches inter-pod anti-affinity rule of other pod on node"),
 		},
 		{
 			description: "pod fits on node",

pkg/descheduler/pod/pods_test.go

@@ -176,3 +176,67 @@ func TestSortPodsBasedOnAge(t *testing.T) {
 		}
 	}
 }
+
+func TestGroupByNodeName(t *testing.T) {
+	tests := []struct {
+		name   string
+		pods   []*v1.Pod
+		expMap map[string][]*v1.Pod
+	}{
+		{
+			name:   "list of pods is empty",
+			pods:   []*v1.Pod{},
+			expMap: map[string][]*v1.Pod{},
+		},
+		{
+			name: "pods are on same node",
+			pods: []*v1.Pod{
+				{Spec: v1.PodSpec{
+					NodeName: "node1",
+				}},
+				{Spec: v1.PodSpec{
+					NodeName: "node1",
+				}},
+			},
+			expMap: map[string][]*v1.Pod{"node1": {
+				{Spec: v1.PodSpec{
+					NodeName: "node1",
+				}},
+				{Spec: v1.PodSpec{
+					NodeName: "node1",
+				}},
+			}},
+		},
+		{
+			name: "pods are on different nodes",
+			pods: []*v1.Pod{
+				{Spec: v1.PodSpec{
+					NodeName: "node1",
+				}},
+				{Spec: v1.PodSpec{
+					NodeName: "node2",
+				}},
+			},
+			expMap: map[string][]*v1.Pod{
+				"node1": {
+					{Spec: v1.PodSpec{
+						NodeName: "node1",
+					}},
+				},
+				"node2": {
+					{Spec: v1.PodSpec{
+						NodeName: "node2",
+					}},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			resultMap := GroupByNodeName(test.pods)
+			if !reflect.DeepEqual(resultMap, test.expMap) {
+				t.Errorf("Expected %v node map, got %v", test.expMap, resultMap)
+			}
+		})
+	}
+}

pkg/framework/plugins/removepodsviolatinginterpodantiaffinity/pod_antiaffinity.go

@@ -24,9 +24,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/descheduler/pkg/descheduler/evictions"
 	podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod"
-	"sigs.k8s.io/descheduler/pkg/utils"
-
 	frameworktypes "sigs.k8s.io/descheduler/pkg/framework/types"
+	"sigs.k8s.io/descheduler/pkg/utils"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/klog/v2"

pkg/utils/predicates.go

@@ -332,7 +332,7 @@ func CheckPodsWithAntiAffinityExist(pod *v1.Pod, pods map[string][]*v1.Pod, node
 							continue
 						}
 						if hasSameLabelValue(node, nodeHavingExistingPod, term.TopologyKey) {
-							klog.V(1).InfoS("Found Pods violating PodAntiAffinity", "pod to evicted", klog.KObj(pod))
+							klog.V(1).InfoS("Found Pods matching PodAntiAffinity", "pod with anti-affinity", klog.KObj(pod))
 							return true
 						}
 					}

pkg/utils/predicates_test.go

@@ -6,6 +6,7 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/descheduler/test"
 )
 
 func TestUniqueSortTolerations(t *testing.T) {
@@ -1041,3 +1042,71 @@ func TestPodNodeAffinityWeight(t *testing.T) {
 		})
 	}
 }
+
+func TestCheckPodsWithAntiAffinityExist(t *testing.T) {
+	tests := []struct {
+		name            string
+		pod             *v1.Pod
+		podsInNamespace map[string][]*v1.Pod
+		nodeMap         map[string]*v1.Node
+		affinity        *v1.PodAntiAffinity
+		expMatch        bool
+	}{
+		{
+			name: "found pod matching pod anti-affinity",
+			pod:  test.PodWithPodAntiAffinity(test.BuildTestPod("p1", 1000, 1000, "node", nil), "foo", "bar"),
+			podsInNamespace: map[string][]*v1.Pod{
+				"default": {
+					test.PodWithPodAntiAffinity(test.BuildTestPod("p2", 1000, 1000, "node", nil), "foo", "bar"),
+				},
+			},
+			nodeMap: map[string]*v1.Node{
+				"node": test.BuildTestNode("node", 64000, 128*1000*1000*1000, 2, func(node *v1.Node) {
+					node.ObjectMeta.Labels = map[string]string{
+						"region": "main-region",
+					}
+				}),
+			},
+			expMatch: true,
+		},
+		{
+			name: "no match with invalid label selector",
+			pod: &v1.Pod{
+				Spec: v1.PodSpec{
+					Affinity: &v1.Affinity{
+						PodAntiAffinity: &v1.PodAntiAffinity{
+							RequiredDuringSchedulingIgnoredDuringExecution: []v1.PodAffinityTerm{
+								{
+									LabelSelector: &metav1.LabelSelector{
+										MatchLabels: map[string]string{
+											"wrong": "selector",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			podsInNamespace: map[string][]*v1.Pod{},
+			nodeMap:         map[string]*v1.Node{},
+			expMatch:        false,
+		},
+		{
+			name: "no match if pod does not match terms namespace",
+			pod: test.PodWithPodAntiAffinity(test.BuildTestPod("p1", 1000, 1000, "node", func(pod *v1.Pod) {
+				pod.Namespace = "other"
+			}), "foo", "bar"),
+			podsInNamespace: map[string][]*v1.Pod{},
+			nodeMap:         map[string]*v1.Node{},
+			expMatch:        false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if match := CheckPodsWithAntiAffinityExist(test.pod, test.podsInNamespace, test.nodeMap); match != test.expMatch {
+				t.Errorf("exp %v got %v", test.expMatch, match)
+			}
+		})
+	}
+}

test/test_utils.go

@@ -358,3 +358,9 @@ func SetPodAntiAffinity(inputPod *v1.Pod, labelKey, labelValue string) {
 		},
 	}
 }
+
+func PodWithPodAntiAffinity(inputPod *v1.Pod, labelKey, labelValue string) *v1.Pod {
+	SetPodAntiAffinity(inputPod, labelKey, labelValue)
+	inputPod.Labels = map[string]string{labelKey: labelValue}
+	return inputPod
+}