From d0d2af4eac4b8151d49a8545c0c1aec51e563bd6 Mon Sep 17 00:00:00 2001
From: Jan Chaloupka <jchaloup@redhat.com>
Date: Thu, 7 Jul 2022 16:30:27 +0200
Subject: [PATCH] Revert "Existing contraints fix backport 1.23"

---
 .../strategies/topologyspreadconstraint.go    | 23 ++-------
 .../topologyspreadconstraint_test.go          | 51 -------------------
 2 files changed, 3 insertions(+), 71 deletions(-)

diff --git a/pkg/descheduler/strategies/topologyspreadconstraint.go b/pkg/descheduler/strategies/topologyspreadconstraint.go
index 01c174797..2f0fda87a 100644
--- a/pkg/descheduler/strategies/topologyspreadconstraint.go
+++ b/pkg/descheduler/strategies/topologyspreadconstraint.go
@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 	"math"
-	"reflect"
 	"sort"
 
 	v1 "k8s.io/api/core/v1"
@@ -107,20 +106,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 {
@@ -128,7 +121,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)
@@ -192,16 +185,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 cd4fd4bac..22a863651 100644
--- a/pkg/descheduler/strategies/topologyspreadconstraint_test.go
+++ b/pkg/descheduler/strategies/topologyspreadconstraint_test.go
@@ -971,54 +971,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)
-			}
-		})
-	}
-}