From b7697869f2ca1a895aef8240fbcf69f0ded870e5 Mon Sep 17 00:00:00 2001 From: etoster Date: Mon, 12 Feb 2024 15:01:53 +0100 Subject: [PATCH] add argument to remove only pods violating specific node taints --- README.md | 26 +++++++++- pkg/api/v1alpha1/types.go | 1 + pkg/api/v1alpha1/zz_generated.deepcopy.go | 5 ++ .../removepodsviolatingnodetaints/defaults.go | 3 ++ .../defaults_test.go | 3 ++ .../node_taint.go | 13 ++++- .../node_taint_test.go | 50 +++++++++++++++++++ .../removepodsviolatingnodetaints/types.go | 1 + .../validation.go | 4 ++ .../validation_test.go | 15 ++++++ .../zz_generated.deepcopy.go | 5 ++ 11 files changed, 122 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index f172236e0..39aabad63 100644 --- a/README.md +++ b/README.md @@ -500,18 +500,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: @@ -526,7 +530,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 diff --git a/pkg/api/v1alpha1/types.go b/pkg/api/v1alpha1/types.go index f70b90068..5d5b321c7 100644 --- a/pkg/api/v1alpha1/types.go +++ b/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 ( diff --git a/pkg/api/v1alpha1/zz_generated.deepcopy.go b/pkg/api/v1alpha1/zz_generated.deepcopy.go index 2440c2400..eff39bda9 100644 --- a/pkg/api/v1alpha1/zz_generated.deepcopy.go +++ b/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 } diff --git a/pkg/framework/plugins/removepodsviolatingnodetaints/defaults.go b/pkg/framework/plugins/removepodsviolatingnodetaints/defaults.go index ca026650b..1e0b679a7 100644 --- a/pkg/framework/plugins/removepodsviolatingnodetaints/defaults.go +++ b/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 + } } diff --git a/pkg/framework/plugins/removepodsviolatingnodetaints/defaults_test.go b/pkg/framework/plugins/removepodsviolatingnodetaints/defaults_test.go index 9b5bc4971..0fa7dfd25 100644 --- a/pkg/framework/plugins/removepodsviolatingnodetaints/defaults_test.go +++ b/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"}, }, }, } diff --git a/pkg/framework/plugins/removepodsviolatingnodetaints/node_taint.go b/pkg/framework/plugins/removepodsviolatingnodetaints/node_taint.go index 446efbeaf..ce8fabf07 100644 --- a/pkg/framework/plugins/removepodsviolatingnodetaints/node_taint.go +++ b/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) } } diff --git a/pkg/framework/plugins/removepodsviolatingnodetaints/node_taint_test.go b/pkg/framework/plugins/removepodsviolatingnodetaints/node_taint_test.go index 7eeabd4d2..1cbfc3e54 100644 --- a/pkg/framework/plugins/removepodsviolatingnodetaints/node_taint_test.go +++ b/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, ) diff --git a/pkg/framework/plugins/removepodsviolatingnodetaints/types.go b/pkg/framework/plugins/removepodsviolatingnodetaints/types.go index 06d146048..f7d8e1c53 100644 --- a/pkg/framework/plugins/removepodsviolatingnodetaints/types.go +++ b/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"` } diff --git a/pkg/framework/plugins/removepodsviolatingnodetaints/validation.go b/pkg/framework/plugins/removepodsviolatingnodetaints/validation.go index c7f6fc6b3..def7a06df 100644 --- a/pkg/framework/plugins/removepodsviolatingnodetaints/validation.go +++ b/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 } diff --git a/pkg/framework/plugins/removepodsviolatingnodetaints/validation_test.go b/pkg/framework/plugins/removepodsviolatingnodetaints/validation_test.go index abc9925f9..8b83ba0ec 100644 --- a/pkg/framework/plugins/removepodsviolatingnodetaints/validation_test.go +++ b/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 { diff --git a/pkg/framework/plugins/removepodsviolatingnodetaints/zz_generated.deepcopy.go b/pkg/framework/plugins/removepodsviolatingnodetaints/zz_generated.deepcopy.go index 2a41e552f..ba0ca72ff 100644 --- a/pkg/framework/plugins/removepodsviolatingnodetaints/zz_generated.deepcopy.go +++ b/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 }