Filter pods by labelSelector during eviction for TopologySpreadConstraint strategy

README.md

@@ -346,6 +346,8 @@ This strategy requires k8s version 1.18 at a minimum.
 By default, this strategy only deals with hard constraints, setting parameter `includeSoftConstraints` to `true` will
 include soft constraints.
 
+Strategy parameter `labelSelector` is not utilized when balancing topology domains and is only applied during eviction to determine if the pod can be evicted.
+
 **Parameters:**
 
 |Name|Type|
@@ -354,6 +356,7 @@ include soft constraints.
 |`thresholdPriority`|int (see [priority filtering](#priority-filtering))|
 |`thresholdPriorityClassName`|string (see [priority filtering](#priority-filtering))|
 |`namespaces`|(see [namespace filtering](#namespace-filtering))|
+|`labelSelector`|(see [label filtering](#label-filtering))|
 |`nodeFit`|bool (see [node fit filtering](#node-fit-filtering))|
 
 **Example:**
@@ -537,6 +540,7 @@ to filter pods by their labels:
 * `RemovePodsViolatingNodeTaints`
 * `RemovePodsViolatingNodeAffinity`
 * `RemovePodsViolatingInterPodAntiAffinity`
+* `RemovePodsViolatingTopologySpreadConstraint`
 
 This allows running strategies among pods the descheduler is interested in.
 

pkg/descheduler/evictions/evictions.go

@@ -25,6 +25,7 @@ import (
 	policy "k8s.io/api/policy/v1beta1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/errors"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -166,8 +167,9 @@ func evictPod(ctx context.Context, client clientset.Interface, pod *v1.Pod, poli
 }
 
 type Options struct {
-	priority *int32
-	nodeFit  bool
+	priority      *int32
+	nodeFit       bool
+	labelSelector labels.Selector
 }
 
 // WithPriorityThreshold sets a threshold for pod's priority class.
@@ -180,7 +182,7 @@ func WithPriorityThreshold(priority int32) func(opts *Options) {
 }
 
 // WithNodeFit sets whether or not to consider taints, node selectors,
-// and pod affinity when evicting. A pod who's tolerations, node selectors,
+// and pod affinity when evicting. A pod whose tolerations, node selectors,
 // and affinity match a node other than the one it is currently running on
 // is evictable.
 func WithNodeFit(nodeFit bool) func(opts *Options) {
@@ -189,6 +191,14 @@ func WithNodeFit(nodeFit bool) func(opts *Options) {
 	}
 }
 
+// WithLabelSelector sets whether or not to apply label filtering when evicting.
+// Any pod matching the label selector is considered evictable.
+func WithLabelSelector(labelSelector labels.Selector) func(opts *Options) {
+	return func(opts *Options) {
+		opts.labelSelector = labelSelector
+	}
+}
+
 type constraint func(pod *v1.Pod) error
 
 type evictable struct {
@@ -247,6 +257,14 @@ func (pe *PodEvictor) Evictable(opts ...func(opts *Options)) *evictable {
 			return nil
 		})
 	}
+	if options.labelSelector != nil && !options.labelSelector.Empty() {
+		ev.constraints = append(ev.constraints, func(pod *v1.Pod) error {
+			if !options.labelSelector.Matches(labels.Set(pod.Labels)) {
+				return fmt.Errorf("pod labels do not match the labelSelector filter in the policy parameter")
+			}
+			return nil
+		})
+	}
 
 	return ev
 }

pkg/descheduler/strategies/topologyspreadconstraint.go

@@ -47,28 +47,51 @@ type topology struct {
 	pods []*v1.Pod
 }
 
-func validateAndParseTopologySpreadParams(ctx context.Context, client clientset.Interface, params *api.StrategyParameters) (int32, sets.String, sets.String, error) {
+// topologySpreadStrategyParams contains validated strategy parameters
+type topologySpreadStrategyParams struct {
+	thresholdPriority  int32
+	includedNamespaces sets.String
+	excludedNamespaces sets.String
+	labelSelector      labels.Selector
+	nodeFit            bool
+}
+
+// validateAndParseTopologySpreadParams will validate parameters to ensure that they do not contain invalid values.
+func validateAndParseTopologySpreadParams(ctx context.Context, client clientset.Interface, params *api.StrategyParameters) (*topologySpreadStrategyParams, error) {
 	var includedNamespaces, excludedNamespaces sets.String
 	if params == nil {
-		return 0, includedNamespaces, excludedNamespaces, nil
+		return &topologySpreadStrategyParams{includedNamespaces: includedNamespaces, excludedNamespaces: excludedNamespaces}, nil
 	}
 	// At most one of include/exclude can be set
 	if params.Namespaces != nil && len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
-		return 0, includedNamespaces, excludedNamespaces, fmt.Errorf("only one of Include/Exclude namespaces can be set")
+		return nil, fmt.Errorf("only one of Include/Exclude namespaces can be set")
 	}
 	if params.ThresholdPriority != nil && params.ThresholdPriorityClassName != "" {
-		return 0, includedNamespaces, excludedNamespaces, fmt.Errorf("only one of thresholdPriority and thresholdPriorityClassName can be set")
+		return nil, fmt.Errorf("only one of thresholdPriority and thresholdPriorityClassName can be set")
 	}
 	thresholdPriority, err := utils.GetPriorityFromStrategyParams(ctx, client, params)
 	if err != nil {
-		return 0, includedNamespaces, excludedNamespaces, fmt.Errorf("failed to get threshold priority from strategy's params: %+v", err)
+		return nil, fmt.Errorf("failed to get threshold priority from strategy's params: %+v", err)
 	}
 	if params.Namespaces != nil {
 		includedNamespaces = sets.NewString(params.Namespaces.Include...)
 		excludedNamespaces = sets.NewString(params.Namespaces.Exclude...)
 	}
+	var selector labels.Selector
+	if params.LabelSelector != nil {
+		selector, err = metav1.LabelSelectorAsSelector(params.LabelSelector)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get label selectors from strategy's params: %+v", err)
+		}
+	}
 
-	return thresholdPriority, includedNamespaces, excludedNamespaces, nil
+	return &topologySpreadStrategyParams{
+		thresholdPriority:  thresholdPriority,
+		includedNamespaces: includedNamespaces,
+		excludedNamespaces: excludedNamespaces,
+		labelSelector:      selector,
+		nodeFit:            params.NodeFit,
+	}, nil
 }
 
 func RemovePodsViolatingTopologySpreadConstraint(
@@ -78,18 +101,17 @@ func RemovePodsViolatingTopologySpreadConstraint(
 	nodes []*v1.Node,
 	podEvictor *evictions.PodEvictor,
 ) {
-	thresholdPriority, includedNamespaces, excludedNamespaces, err := validateAndParseTopologySpreadParams(ctx, client, strategy.Params)
+	strategyParams, err := validateAndParseTopologySpreadParams(ctx, client, strategy.Params)
 	if err != nil {
 		klog.ErrorS(err, "Invalid RemovePodsViolatingTopologySpreadConstraint parameters")
 		return
 	}
 
-	nodeFit := false
-	if strategy.Params != nil {
-		nodeFit = strategy.Params.NodeFit
-	}
-
-	evictable := podEvictor.Evictable(evictions.WithPriorityThreshold(thresholdPriority), evictions.WithNodeFit(nodeFit))
+	evictable := podEvictor.Evictable(
+		evictions.WithPriorityThreshold(strategyParams.thresholdPriority),
+		evictions.WithNodeFit(strategyParams.nodeFit),
+		evictions.WithLabelSelector(strategyParams.labelSelector),
+	)
 
 	nodeMap := make(map[string]*v1.Node, len(nodes))
 	for _, node := range nodes {
@@ -118,8 +140,8 @@ func RemovePodsViolatingTopologySpreadConstraint(
 	podsForEviction := make(map[*v1.Pod]struct{})
 	// 1. for each namespace...
 	for _, namespace := range namespaces.Items {
-		if (len(includedNamespaces) > 0 && !includedNamespaces.Has(namespace.Name)) ||
-			(len(excludedNamespaces) > 0 && excludedNamespaces.Has(namespace.Name)) {
+		if (len(strategyParams.includedNamespaces) > 0 && !strategyParams.includedNamespaces.Has(namespace.Name)) ||
+			(len(strategyParams.excludedNamespaces) > 0 && strategyParams.excludedNamespaces.Has(namespace.Name)) {
 			continue
 		}
 		namespacePods, err := client.CoreV1().Pods(namespace.Name).List(ctx, metav1.ListOptions{})

pkg/descheduler/strategies/topologyspreadconstraint_test.go

@@ -73,17 +73,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
 				},
 				{
 					count:  2,
@@ -147,18 +140,11 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
-					noOwners: true,
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
+					noOwners:    true,
 				},
 				{
 					count:    2,
@@ -189,17 +175,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
 				},
 				{
 					count:  2,
@@ -224,17 +203,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
 				},
 				{
 					count:  4,
@@ -263,17 +235,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
 				},
 				{
 					count:  3,
@@ -297,17 +262,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:        1,
+					node:         "n1",
+					labels:       map[string]string{"foo": "bar"},
+					constraints:  getDefaultTopologyConstraints(1),
 					nodeSelector: map[string]string{"zone": "zoneA"},
 				},
 				{
@@ -354,17 +312,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:        1,
+					node:         "n1",
+					labels:       map[string]string{"foo": "bar"},
+					constraints:  getDefaultTopologyConstraints(1),
 					nodeSelector: map[string]string{"region": "boston"},
 				},
 				{
@@ -403,17 +354,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n2",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:       1,
+					node:        "n2",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
 				},
 				{
 					count:  100,
@@ -435,17 +379,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n2",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:       1,
+					node:        "n2",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
 				},
 				{
 					count:  3,
@@ -474,17 +411,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           2,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(2),
 				},
 				{
 					count:  1,
@@ -675,17 +605,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
 					tolerations: []v1.Toleration{
 						{
 							Key:      "taint-test",
@@ -723,17 +646,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
 				},
 				{
 					count:        1,
@@ -763,17 +679,10 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			},
 			pods: createTestPods([]testPodList{
 				{
-					count:  1,
-					node:   "n1",
-					labels: map[string]string{"foo": "bar"},
-					constraints: []v1.TopologySpreadConstraint{
-						{
-							MaxSkew:           1,
-							TopologyKey:       "zone",
-							WhenUnsatisfiable: v1.DoNotSchedule,
-							LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-						},
-					},
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
 				},
 				{
 					count:        1,
@@ -786,6 +695,87 @@ func TestTopologySpreadConstraint(t *testing.T) {
 			strategy:             api.DeschedulerStrategy{},
 			namespaces:           []string{"ns1"},
 		},
+		{
+			name: "2 domains, sizes [2,0], maxSkew=1, move 0 pod for node with unmatched label filtering",
+			nodes: []*v1.Node{
+				test.BuildTestNode("n1", 2000, 3000, 10, func(n *v1.Node) { n.Labels["zone"] = "zoneA" }),
+				test.BuildTestNode("n2", 2000, 3000, 10, func(n *v1.Node) { n.Labels["zone"] = "zoneB" }),
+			},
+			pods: createTestPods([]testPodList{
+				{
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
+				},
+				{
+					count:  1,
+					node:   "n1",
+					labels: map[string]string{"foo": "bar"},
+				},
+			}),
+			expectedEvictedCount: 0,
+			strategy: api.DeschedulerStrategy{
+				Params: &api.StrategyParameters{
+					LabelSelector: getLabelSelector("foo", []string{"baz"}, metav1.LabelSelectorOpIn),
+				},
+			},
+			namespaces: []string{"ns1"},
+		},
+		{
+			name: "2 domains, sizes [2,0], maxSkew=1, move 1 pod for node with matched label filtering",
+			nodes: []*v1.Node{
+				test.BuildTestNode("n1", 2000, 3000, 10, func(n *v1.Node) { n.Labels["zone"] = "zoneA" }),
+				test.BuildTestNode("n2", 2000, 3000, 10, func(n *v1.Node) { n.Labels["zone"] = "zoneB" }),
+			},
+			pods: createTestPods([]testPodList{
+				{
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
+				},
+				{
+					count:  1,
+					node:   "n1",
+					labels: map[string]string{"foo": "bar"},
+				},
+			}),
+			expectedEvictedCount: 1,
+			strategy: api.DeschedulerStrategy{
+				Params: &api.StrategyParameters{
+					LabelSelector: getLabelSelector("foo", []string{"bar"}, metav1.LabelSelectorOpIn),
+				},
+			},
+			namespaces: []string{"ns1"},
+		},
+		{
+			name: "2 domains, sizes [2,0], maxSkew=1, move 1 pod for node with matched label filtering (NotIn op)",
+			nodes: []*v1.Node{
+				test.BuildTestNode("n1", 2000, 3000, 10, func(n *v1.Node) { n.Labels["zone"] = "zoneA" }),
+				test.BuildTestNode("n2", 2000, 3000, 10, func(n *v1.Node) { n.Labels["zone"] = "zoneB" }),
+			},
+			pods: createTestPods([]testPodList{
+				{
+					count:       1,
+					node:        "n1",
+					labels:      map[string]string{"foo": "bar"},
+					constraints: getDefaultTopologyConstraints(1),
+				},
+				{
+					count:  1,
+					node:   "n1",
+					labels: map[string]string{"foo": "bar"},
+				},
+			}),
+			expectedEvictedCount: 1,
+			strategy: api.DeschedulerStrategy{
+				Params: &api.StrategyParameters{
+					LabelSelector: getLabelSelector("foo", []string{"baz"}, metav1.LabelSelectorOpNotIn),
+				},
+			},
+			namespaces: []string{"ns1"},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -856,3 +846,20 @@ func createTestPods(testPods []testPodList) []*v1.Pod {
 	}
 	return pods
 }
+
+func getLabelSelector(key string, values []string, operator metav1.LabelSelectorOperator) *metav1.LabelSelector {
+	return &metav1.LabelSelector{
+		MatchExpressions: []metav1.LabelSelectorRequirement{{Key: key, Operator: operator, Values: values}},
+	}
+}
+
+func getDefaultTopologyConstraints(maxSkew int32) []v1.TopologySpreadConstraint {
+	return []v1.TopologySpreadConstraint{
+		{
+			MaxSkew:           maxSkew,
+			TopologyKey:       "zone",
+			WhenUnsatisfiable: v1.DoNotSchedule,
+			LabelSelector:     &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
+		},
+	}
+}