change namespaceTopologySpreadConstraints from map to slice

pkg/descheduler/strategies/topologyspreadconstraint.go

@@ -105,7 +105,7 @@ func RemovePodsViolatingTopologySpreadConstraint(
 		}
 
 		// ...where there is a topology constraint
-		namespaceTopologySpreadConstraints := make(map[v1.TopologySpreadConstraint]struct{})
+		namespaceTopologySpreadConstraints := []v1.TopologySpreadConstraint{}
 		for _, pod := range namespacePods.Items {
 			for _, constraint := range pod.Spec.TopologySpreadConstraints {
 				// Ignore soft topology constraints if they are not included
@@ -113,12 +113,12 @@ func RemovePodsViolatingTopologySpreadConstraint(
 					continue
 				}
 				// Need to check v1.TopologySpreadConstraint deepEquality because
-				// v1.TopologySpreadConstraint.LabelSelector is a pointer
-				// and assigning new constrains would always add more keys
+				// v1.TopologySpreadConstraint has pointer fields
+				// and we don't need to go over duplicated constraints later on
 				if hasIdenticalConstraints(constraint, namespaceTopologySpreadConstraints) {
 					continue
 				}
-				namespaceTopologySpreadConstraints[constraint] = struct{}{}
+				namespaceTopologySpreadConstraints = append(namespaceTopologySpreadConstraints, constraint)
 			}
 		}
 		if len(namespaceTopologySpreadConstraints) == 0 {
@@ -126,7 +126,7 @@ func RemovePodsViolatingTopologySpreadConstraint(
 		}
 
 		// 2. for each topologySpreadConstraint in that namespace
-		for constraint := range namespaceTopologySpreadConstraints {
+		for _, constraint := range namespaceTopologySpreadConstraints {
 			constraintTopologies := make(map[topologyPair][]*v1.Pod)
 			// pre-populate the topologyPair map with all the topologies available from the nodeMap
 			// (we can't just build it from existing pods' nodes because a topology may have 0 pods)
@@ -190,9 +190,9 @@ func RemovePodsViolatingTopologySpreadConstraint(
 	}
 }
 
-// hasIdenticalConstraints checks if already had an identical TopologySpreadConstraint in namespaceTopologySpreadConstraints map
-func hasIdenticalConstraints(newConstraint v1.TopologySpreadConstraint, namespaceTopologySpreadConstraints map[v1.TopologySpreadConstraint]struct{}) bool {
-	for constraint := range namespaceTopologySpreadConstraints {
+// hasIdenticalConstraints checks if we already had an identical TopologySpreadConstraint in namespaceTopologySpreadConstraints slice
+func hasIdenticalConstraints(newConstraint v1.TopologySpreadConstraint, namespaceTopologySpreadConstraints []v1.TopologySpreadConstraint) bool {
+	for _, constraint := range namespaceTopologySpreadConstraints {
 		if reflect.DeepEqual(newConstraint, constraint) {
 			return true
 		}

pkg/descheduler/strategies/topologyspreadconstraint_test.go

@@ -1043,47 +1043,51 @@ func getDefaultTopologyConstraints(maxSkew int32) []v1.TopologySpreadConstraint
 }
 
 func TestCheckIdenticalConstraints(t *testing.T) {
-	newConstrainSame := v1.TopologySpreadConstraint{
+	newConstraintSame := v1.TopologySpreadConstraint{
 		MaxSkew:           2,
 		TopologyKey:       "zone",
 		WhenUnsatisfiable: v1.DoNotSchedule,
 		LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
 	}
-	newConstrainDifferent := v1.TopologySpreadConstraint{
+	newConstraintDifferent := v1.TopologySpreadConstraint{
 		MaxSkew:           3,
 		TopologyKey:       "node",
 		WhenUnsatisfiable: v1.DoNotSchedule,
 		LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
 	}
-	namespaceTopologySpreadConstraint := make(map[v1.TopologySpreadConstraint]struct{})
-	namespaceTopologySpreadConstraint[v1.TopologySpreadConstraint{
-		MaxSkew:           2,
-		TopologyKey:       "zone",
-		WhenUnsatisfiable: v1.DoNotSchedule,
-		LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-	}] = struct{}{}
+	namespaceTopologySpreadConstraint := []v1.TopologySpreadConstraint{}
+	namespaceTopologySpreadConstraint = []v1.TopologySpreadConstraint{
+		{
+			MaxSkew:           2,
+			TopologyKey:       "zone",
+			WhenUnsatisfiable: v1.DoNotSchedule,
+			LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
+		},
+	}
 	testCases := []struct {
 		name                               string
-		namespaceTopologySpreadConstraints map[v1.TopologySpreadConstraint]struct{}
+		namespaceTopologySpreadConstraints []v1.TopologySpreadConstraint
 		newConstraint                      v1.TopologySpreadConstraint
 		expectedResult                     bool
 	}{
 		{
 			name:                               "new constraint is identical",
 			namespaceTopologySpreadConstraints: namespaceTopologySpreadConstraint,
-			newConstraint:                      newConstrainSame, expectedResult: true},
+			newConstraint:                      newConstraintSame,
+			expectedResult:                     true,
+		},
 		{
 			name:                               "new constraint is different",
 			namespaceTopologySpreadConstraints: namespaceTopologySpreadConstraint,
-			newConstraint:                      newConstrainDifferent, expectedResult: false,
+			newConstraint:                      newConstraintDifferent,
+			expectedResult:                     false,
 		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-			isIdentitcal := hasIdenticalConstraints(tc.newConstraint, tc.namespaceTopologySpreadConstraints)
-			if isIdentitcal != tc.expectedResult {
-				t.Errorf("Test error for description: %s. Expected result %v, got %v", tc.name, tc.expectedResult, isIdentitcal)
+			isIdentical := hasIdenticalConstraints(tc.newConstraint, tc.namespaceTopologySpreadConstraints)
+			if isIdentical != tc.expectedResult {
+				t.Errorf("Test error for description: %s. Expected result %v, got %v", tc.name, tc.expectedResult, isIdentical)
 			}
 		})
 	}