Merge pull request #875 from kubernetes-sigs/revert-868-existing-contraints-backport-1.24

Revert "Existing contraints fix backport 1.24"

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 {

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