Merge pull request #1360 from etoster/remove-only-pods-violating-included-node-taints

Add argument to remove only pods violating allowlisted taints

README.md

@@ -502,18 +502,22 @@ key=value matches an excludedTaints entry, the taint will be ignored.
 For example, excludedTaints entry "dedicated" would match all taints with key "dedicated", regardless of value.
 excludedTaints entry "dedicated=special-user" would match taints with key "dedicated" and value "special-user".
 
+If a list of includedTaints is provided, a taint will be considered if and only if it matches an included key **or** key=value from the list. Otherwise it will be ignored. Leaving includedTaints unset will include any taint by default. 
+
 **Parameters:**
 
 |Name|Type|
 |---|---|
 |`excludedTaints`|list(string)|
+|`includedTaints`|list(string)|
 |`includePreferNoSchedule`|bool|
 |`namespaces`|(see [namespace filtering](#namespace-filtering))|
 |`labelSelector`|(see [label filtering](#label-filtering))|
 
 **Example:**
 
-````yaml
+Setting `excludedTaints`
+```yaml
 apiVersion: "descheduler/v1alpha2"
 kind: "DeschedulerPolicy"
 profiles:
@@ -528,7 +532,25 @@ profiles:
       deschedule:
         enabled:
           - "RemovePodsViolatingNodeTaints"
-````
+```
+
+Setting `includedTaints`
+```yaml
+apiVersion: "descheduler/v1alpha2"
+kind: "DeschedulerPolicy"
+profiles:
+  - name: ProfileName
+    pluginConfig:
+    - name: "RemovePodsViolatingNodeTaints"
+      args:
+        includedTaints:
+        - decommissioned=end-of-life # include only taints with key "decommissioned" and value "end-of-life"
+        - reserved # include all taints with key "reserved"
+    plugins:
+      deschedule:
+        enabled:
+          - "RemovePodsViolatingNodeTaints"
+```
 
 ### RemovePodsViolatingTopologySpreadConstraint
 

pkg/api/v1alpha1/types.go

@@ -90,6 +90,7 @@ type StrategyParameters struct {
 	NodeFit                           bool                               `json:"nodeFit"`
 	IncludePreferNoSchedule           bool                               `json:"includePreferNoSchedule"`
 	ExcludedTaints                    []string                           `json:"excludedTaints,omitempty"`
+	IncludedTaints                    []string                           `json:"includedTaints,omitempty"`
 }
 
 type (

pkg/api/v1alpha1/zz_generated.deepcopy.go

@@ -366,6 +366,11 @@ func (in *StrategyParameters) DeepCopyInto(out *StrategyParameters) {
 		*out = make([]string, len(*in))
 		copy(*out, *in)
 	}
+	if in.IncludedTaints != nil {
+		in, out := &in.IncludedTaints, &out.IncludedTaints
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	return
 }
 

pkg/framework/plugins/removepodsviolatingnodetaints/defaults.go

@@ -37,4 +37,7 @@ func SetDefaults_RemovePodsViolatingNodeTaintsArgs(obj runtime.Object) {
 	if args.ExcludedTaints == nil {
 		args.ExcludedTaints = nil
 	}
+	if args.IncludedTaints == nil {
+		args.IncludedTaints = nil
+	}
 }

pkg/framework/plugins/removepodsviolatingnodetaints/defaults_test.go

@@ -37,6 +37,7 @@ func TestSetDefaults_RemovePodsViolatingNodeTaintsArgs(t *testing.T) {
 				LabelSelector:           nil,
 				IncludePreferNoSchedule: false,
 				ExcludedTaints:          nil,
+				IncludedTaints:          nil,
 			},
 		},
 		{
@@ -46,12 +47,14 @@ func TestSetDefaults_RemovePodsViolatingNodeTaintsArgs(t *testing.T) {
 				LabelSelector:           &metav1.LabelSelector{},
 				IncludePreferNoSchedule: false,
 				ExcludedTaints:          []string{"ExcludedTaints"},
+				IncludedTaints:          []string{"IncludedTaints"},
 			},
 			want: &RemovePodsViolatingNodeTaintsArgs{
 				Namespaces:              &api.Namespaces{},
 				LabelSelector:           &metav1.LabelSelector{},
 				IncludePreferNoSchedule: false,
 				ExcludedTaints:          []string{"ExcludedTaints"},
+				IncludedTaints:          []string{"IncludedTaints"},
 			},
 		},
 	}

pkg/framework/plugins/removepodsviolatingnodetaints/node_taint.go

@@ -67,16 +67,25 @@ func New(args runtime.Object, handle frameworktypes.Handle) (frameworktypes.Plug
 		return nil, fmt.Errorf("error initializing pod filter function: %v", err)
 	}
 
+	includedTaints := sets.New(nodeTaintsArgs.IncludedTaints...)
+	includeTaint := func(taint *v1.Taint) bool {
+		// Include only taints by key *or* key=value
+		// Always returns true if no includedTaints argument is provided
+		return (nodeTaintsArgs.IncludedTaints == nil) || includedTaints.Has(taint.Key) || (taint.Value != "" && includedTaints.Has(fmt.Sprintf("%s=%s", taint.Key, taint.Value)))
+	}
+
 	excludedTaints := sets.New(nodeTaintsArgs.ExcludedTaints...)
 	excludeTaint := func(taint *v1.Taint) bool {
 		// Exclude taints by key *or* key=value
 		return excludedTaints.Has(taint.Key) || (taint.Value != "" && excludedTaints.Has(fmt.Sprintf("%s=%s", taint.Key, taint.Value)))
 	}
 
-	taintFilterFnc := func(taint *v1.Taint) bool { return (taint.Effect == v1.TaintEffectNoSchedule) && !excludeTaint(taint) }
+	taintFilterFnc := func(taint *v1.Taint) bool {
+		return (taint.Effect == v1.TaintEffectNoSchedule) && !excludeTaint(taint) && includeTaint(taint)
+	}
 	if nodeTaintsArgs.IncludePreferNoSchedule {
 		taintFilterFnc = func(taint *v1.Taint) bool {
-			return (taint.Effect == v1.TaintEffectNoSchedule || taint.Effect == v1.TaintEffectPreferNoSchedule) && !excludeTaint(taint)
+			return (taint.Effect == v1.TaintEffectNoSchedule || taint.Effect == v1.TaintEffectPreferNoSchedule) && !excludeTaint(taint) && includeTaint(taint)
 		}
 	}
 

pkg/framework/plugins/removepodsviolatingnodetaints/node_taint_test.go

@@ -100,6 +100,10 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) {
 		createPreferNoScheduleTaint("testTaint", "test", 1),
 	}
 
+	node7 := test.BuildTestNode("n7", 2000, 3000, 10, nil)
+	node7 = addTaintsToNode(node7, "testTaint", "test", []int{1})
+	node7 = addTaintsToNode(node7, "testingTaint", "testing", []int{1})
+
 	p1 := test.BuildTestPod("p1", 100, 0, node1.Name, nil)
 	p2 := test.BuildTestPod("p2", 100, 0, node1.Name, nil)
 	p3 := test.BuildTestPod("p3", 100, 0, node1.Name, nil)
@@ -161,6 +165,15 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) {
 	// PreferNoSchedule:testTaint0=test0 so the pod is not tolarated
 	p13 = addTolerationToPod(p13, "testTaint", "test", 0, v1.TaintEffectPreferNoSchedule)
 
+	p14 := test.BuildTestPod("p14", 100, 0, node7.Name, nil)
+	p14.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
+	p14 = addTolerationToPod(p14, "testTaint", "test", 1, v1.TaintEffectNoSchedule)
+
+	p15 := test.BuildTestPod("p15", 100, 0, node7.Name, nil)
+	p15.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
+	p15 = addTolerationToPod(p15, "testTaint", "test", 1, v1.TaintEffectNoSchedule)
+	p15 = addTolerationToPod(p15, "testingTaint", "testing", 1, v1.TaintEffectNoSchedule)
+
 	var uint1 uint = 1
 
 	tests := []struct {
@@ -175,6 +188,7 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) {
 		nodeFit                        bool
 		includePreferNoSchedule        bool
 		excludedTaints                 []string
+		includedTaints                 []string
 	}{
 		{
 			description:             "Pods not tolerating node taint should be evicted",
@@ -322,6 +336,41 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) {
 			expectedEvictedPodCount: 0, // p2 and p7 can't be evicted
 			nodeFit:                 true,
 		},
+		{
+			description:             "Pods tolerating included taints should not get evicted even with other taints present",
+			pods:                    []*v1.Pod{p1},
+			nodes:                   []*v1.Node{node7},
+			evictLocalStoragePods:   false,
+			evictSystemCriticalPods: false,
+			includedTaints:          []string{"testTaint1=test1"},
+			expectedEvictedPodCount: 0, // nothing gets evicted, as p1 tolerates the included taint, and taint "testingTaint1=testing1" is not included
+		},
+		{
+			description:             "Pods not tolerating not included taints should not get evicted",
+			pods:                    []*v1.Pod{p1, p2, p4},
+			nodes:                   []*v1.Node{node1},
+			evictLocalStoragePods:   false,
+			evictSystemCriticalPods: false,
+			includedTaints:          []string{"testTaint2=test2"},
+			expectedEvictedPodCount: 0, // nothing gets evicted, as taint is not included, even though the pods' p2 and p4 tolerations do not match node1's taint
+		},
+		{
+			description:             "Pods tolerating includedTaint should not get evicted. Pods not tolerating includedTaints should get evicted",
+			pods:                    []*v1.Pod{p1, p2, p3},
+			nodes:                   []*v1.Node{node1},
+			evictLocalStoragePods:   false,
+			evictSystemCriticalPods: false,
+			includedTaints:          []string{"testTaint1=test1"},
+			expectedEvictedPodCount: 1, // node1 taint is included. p1 and p3 tolerate the included taint, p2 gets evicted
+		},
+		{
+			description:             "Pods not tolerating all taints are evicted when includedTaints is empty",
+			pods:                    []*v1.Pod{p14, p15},
+			nodes:                   []*v1.Node{node7},
+			evictLocalStoragePods:   false,
+			evictSystemCriticalPods: false,
+			expectedEvictedPodCount: 1, // includedTaints is empty so all taints are included. p15 tolerates both node taints and does not get evicted. p14 tolerate only one and gets evicted
+		},
 	}
 
 	for _, tc := range tests {
@@ -393,6 +442,7 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) {
 			plugin, err := New(&RemovePodsViolatingNodeTaintsArgs{
 				IncludePreferNoSchedule: tc.includePreferNoSchedule,
 				ExcludedTaints:          tc.excludedTaints,
+				IncludedTaints:          tc.includedTaints,
 			},
 				handle,
 			)

pkg/framework/plugins/removepodsviolatingnodetaints/types.go

@@ -32,4 +32,5 @@ type RemovePodsViolatingNodeTaintsArgs struct {
 	LabelSelector           *metav1.LabelSelector `json:"labelSelector"`
 	IncludePreferNoSchedule bool                  `json:"includePreferNoSchedule"`
 	ExcludedTaints          []string              `json:"excludedTaints"`
+	IncludedTaints          []string              `json:"includedTaints"`
 }

pkg/framework/plugins/removepodsviolatingnodetaints/validation.go

@@ -37,5 +37,9 @@ func ValidateRemovePodsViolatingNodeTaintsArgs(obj runtime.Object) error {
 		}
 	}
 
+	if len(args.ExcludedTaints) > 0 && len(args.IncludedTaints) > 0 {
+		return fmt.Errorf("either includedTaints or excludedTaints can be set, but not both")
+	}
+
 	return nil
 }

pkg/framework/plugins/removepodsviolatingnodetaints/validation_test.go

@@ -54,6 +54,21 @@ func TestValidateRemovePodsViolatingNodeTaintsArgs(t *testing.T) {
 			},
 			expectError: true,
 		},
+		{
+			description: "valid taint filters, no errors",
+			args: &RemovePodsViolatingNodeTaintsArgs{
+				ExcludedTaints: []string{"testTaint1=test1"},
+			},
+			expectError: false,
+		},
+		{
+			description: "invalid taint filters args, expects errors",
+			args: &RemovePodsViolatingNodeTaintsArgs{
+				ExcludedTaints: []string{"do-not-evict"},
+				IncludedTaints: []string{"testTaint1=test1"},
+			},
+			expectError: true,
+		},
 	}
 
 	for _, tc := range testCases {

pkg/framework/plugins/removepodsviolatingnodetaints/zz_generated.deepcopy.go

@@ -46,6 +46,11 @@ func (in *RemovePodsViolatingNodeTaintsArgs) DeepCopyInto(out *RemovePodsViolati
 		*out = make([]string, len(*in))
 		copy(*out, *in)
 	}
+	if in.IncludedTaints != nil {
+		in, out := &in.IncludedTaints, &out.IncludedTaints
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	return
 }