diff --git a/pkg/descheduler/strategies/topologyspreadconstraint.go b/pkg/descheduler/strategies/topologyspreadconstraint.go index d586daa0f..035896bbc 100644 --- a/pkg/descheduler/strategies/topologyspreadconstraint.go +++ b/pkg/descheduler/strategies/topologyspreadconstraint.go @@ -19,7 +19,6 @@ package strategies import ( "context" "math" - "reflect" "sort" v1 "k8s.io/api/core/v1" @@ -105,20 +104,14 @@ func RemovePodsViolatingTopologySpreadConstraint( } // ...where there is a topology constraint - namespaceTopologySpreadConstraints := []v1.TopologySpreadConstraint{} + namespaceTopologySpreadConstraints := make(map[v1.TopologySpreadConstraint]struct{}) for _, pod := range namespacePods.Items { for _, constraint := range pod.Spec.TopologySpreadConstraints { // Ignore soft topology constraints if they are not included if constraint.WhenUnsatisfiable == v1.ScheduleAnyway && (strategy.Params == nil || !strategy.Params.IncludeSoftConstraints) { continue } - // Need to check v1.TopologySpreadConstraint deepEquality because - // v1.TopologySpreadConstraint has pointer fields - // and we don't need to go over duplicated constraints later on - if hasIdenticalConstraints(constraint, namespaceTopologySpreadConstraints) { - continue - } - namespaceTopologySpreadConstraints = append(namespaceTopologySpreadConstraints, constraint) + namespaceTopologySpreadConstraints[constraint] = struct{}{} } } if len(namespaceTopologySpreadConstraints) == 0 { @@ -126,7 +119,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,16 +183,6 @@ func RemovePodsViolatingTopologySpreadConstraint( } } -// 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 - } - } - 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 { diff --git a/pkg/descheduler/strategies/topologyspreadconstraint_test.go b/pkg/descheduler/strategies/topologyspreadconstraint_test.go index 5a0b96e01..f2b8ac8fa 100644 --- a/pkg/descheduler/strategies/topologyspreadconstraint_test.go +++ b/pkg/descheduler/strategies/topologyspreadconstraint_test.go @@ -1041,54 +1041,3 @@ func getDefaultTopologyConstraints(maxSkew int32) []v1.TopologySpreadConstraint }, } } - -func TestCheckIdenticalConstraints(t *testing.T) { - newConstraintSame := v1.TopologySpreadConstraint{ - MaxSkew: 2, - TopologyKey: "zone", - WhenUnsatisfiable: v1.DoNotSchedule, - LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, - } - newConstraintDifferent := v1.TopologySpreadConstraint{ - MaxSkew: 3, - TopologyKey: "node", - WhenUnsatisfiable: v1.DoNotSchedule, - LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, - } - 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 []v1.TopologySpreadConstraint - newConstraint v1.TopologySpreadConstraint - expectedResult bool - }{ - { - name: "new constraint is identical", - namespaceTopologySpreadConstraints: namespaceTopologySpreadConstraint, - newConstraint: newConstraintSame, - expectedResult: true, - }, - { - name: "new constraint is different", - namespaceTopologySpreadConstraints: namespaceTopologySpreadConstraint, - newConstraint: newConstraintDifferent, - expectedResult: false, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - 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) - } - }) - } -}