Check existing constraints before assigning

pkg/descheduler/strategies/topologyspreadconstraint.go

@@ -19,6 +19,7 @@ package strategies
 import (
 	"context"
 	"math"
+	"reflect"
 	"sort"
 
 	v1 "k8s.io/api/core/v1"
@@ -111,6 +112,12 @@ func RemovePodsViolatingTopologySpreadConstraint(
 				if constraint.WhenUnsatisfiable == v1.ScheduleAnyway && (strategy.Params == nil || !strategy.Params.IncludeSoftConstraints) {
 					continue
 				}
+				// Need to check v1.TopologySpreadConstraint deepEquality because
+				// v1.TopologySpreadConstraint.LabelSelector is a pointer
+				// and assigning new constrains would always add more keys
+				if hasIdenticalConstraints(constraint, namespaceTopologySpreadConstraints) {
+					continue
+				}
 				namespaceTopologySpreadConstraints[constraint] = struct{}{}
 			}
 		}
@@ -183,6 +190,16 @@ 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 {
+		if reflect.DeepEqual(newConstraint, constraint) {
+			return true
+		}
+	}
+	return false
+}
+
 // topologyIsBalanced checks if any domains in the topology differ by more than the MaxSkew
 // this is called before any sorting or other calculations and is used to skip topologies that don't need to be balanced
 func topologyIsBalanced(topology map[topologyPair][]*v1.Pod, constraint v1.TopologySpreadConstraint) bool {

pkg/descheduler/strategies/topologyspreadconstraint_test.go

@@ -1041,3 +1041,50 @@ func getDefaultTopologyConstraints(maxSkew int32) []v1.TopologySpreadConstraint
 		},
 	}
 }
+
+func TestCheckIdenticalConstraints(t *testing.T) {
+	newConstrainSame := v1.TopologySpreadConstraint{
+		MaxSkew:           2,
+		TopologyKey:       "zone",
+		WhenUnsatisfiable: v1.DoNotSchedule,
+		LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
+	}
+	newConstrainDifferent := 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{}{}
+	testCases := []struct {
+		name                               string
+		namespaceTopologySpreadConstraints map[v1.TopologySpreadConstraint]struct{}
+		newConstraint                      v1.TopologySpreadConstraint
+		expectedResult                     bool
+	}{
+		{
+			name:                               "new constraint is identical",
+			namespaceTopologySpreadConstraints: namespaceTopologySpreadConstraint,
+			newConstraint:                      newConstrainSame, expectedResult: true},
+		{
+			name:                               "new constraint is different",
+			namespaceTopologySpreadConstraints: namespaceTopologySpreadConstraint,
+			newConstraint:                      newConstrainDifferent, 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)
+			}
+		})
+	}
+}