From 285523f0d908ba1e9f101953bec0282677fa3def Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Mon, 14 Mar 2022 16:09:49 +0100 Subject: [PATCH] RemovePodsViolatingNodeTaints: optionally include PreferNoSchedule taint --- pkg/api/types.go | 1 + pkg/api/v1alpha1/types.go | 1 + pkg/api/v1alpha1/zz_generated.conversion.go | 2 + pkg/descheduler/strategies/node_taint.go | 9 +++- pkg/descheduler/strategies/node_taint_test.go | 50 ++++++++++++++++--- 5 files changed, 56 insertions(+), 7 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index f0626896c..f67840ca3 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -88,6 +88,7 @@ type StrategyParameters struct { ThresholdPriorityClassName string LabelSelector *metav1.LabelSelector NodeFit bool + IncludePreferNoSchedule bool } type Percentage float64 diff --git a/pkg/api/v1alpha1/types.go b/pkg/api/v1alpha1/types.go index 24c50e9f4..569014bad 100644 --- a/pkg/api/v1alpha1/types.go +++ b/pkg/api/v1alpha1/types.go @@ -86,6 +86,7 @@ type StrategyParameters struct { ThresholdPriorityClassName string `json:"thresholdPriorityClassName"` LabelSelector *metav1.LabelSelector `json:"labelSelector"` NodeFit bool `json:"nodeFit"` + IncludePreferNoSchedule bool `json:"includePreferNoSchedule"` } type Percentage float64 diff --git a/pkg/api/v1alpha1/zz_generated.conversion.go b/pkg/api/v1alpha1/zz_generated.conversion.go index 5457d9c34..8e965f5a2 100644 --- a/pkg/api/v1alpha1/zz_generated.conversion.go +++ b/pkg/api/v1alpha1/zz_generated.conversion.go @@ -361,6 +361,7 @@ func autoConvert_v1alpha1_StrategyParameters_To_api_StrategyParameters(in *Strat out.ThresholdPriorityClassName = in.ThresholdPriorityClassName out.LabelSelector = (*v1.LabelSelector)(unsafe.Pointer(in.LabelSelector)) out.NodeFit = in.NodeFit + out.IncludePreferNoSchedule = in.IncludePreferNoSchedule return nil } @@ -382,6 +383,7 @@ func autoConvert_api_StrategyParameters_To_v1alpha1_StrategyParameters(in *api.S out.ThresholdPriorityClassName = in.ThresholdPriorityClassName out.LabelSelector = (*v1.LabelSelector)(unsafe.Pointer(in.LabelSelector)) out.NodeFit = in.NodeFit + out.IncludePreferNoSchedule = in.IncludePreferNoSchedule return nil } diff --git a/pkg/descheduler/strategies/node_taint.go b/pkg/descheduler/strategies/node_taint.go index 3a7c205dc..53e23f52e 100644 --- a/pkg/descheduler/strategies/node_taint.go +++ b/pkg/descheduler/strategies/node_taint.go @@ -89,6 +89,13 @@ func RemovePodsViolatingNodeTaints(ctx context.Context, client clientset.Interfa return } + taintFilterFnc := func(taint *v1.Taint) bool { return taint.Effect == v1.TaintEffectNoSchedule } + if strategy.Params != nil && strategy.Params.IncludePreferNoSchedule { + taintFilterFnc = func(taint *v1.Taint) bool { + return taint.Effect == v1.TaintEffectNoSchedule || taint.Effect == v1.TaintEffectPreferNoSchedule + } + } + for _, node := range nodes { klog.V(1).InfoS("Processing node", "node", klog.KObj(node)) pods, err := podutil.ListAllPodsOnANode(node.Name, getPodsAssignedToNode, podFilter) @@ -101,7 +108,7 @@ func RemovePodsViolatingNodeTaints(ctx context.Context, client clientset.Interfa if !utils.TolerationsTolerateTaintsWithFilter( pods[i].Spec.Tolerations, node.Spec.Taints, - func(taint *v1.Taint) bool { return taint.Effect == v1.TaintEffectNoSchedule }, + taintFilterFnc, ) { klog.V(2).InfoS("Not all taints with NoSchedule effect are tolerated after update for pod on node", "pod", klog.KObj(pods[i]), "node", klog.KObj(node)) if _, err := podEvictor.EvictPod(ctx, pods[i], node, "NodeTaint"); err != nil { diff --git a/pkg/descheduler/strategies/node_taint_test.go b/pkg/descheduler/strategies/node_taint_test.go index 0dc722214..e21914f93 100644 --- a/pkg/descheduler/strategies/node_taint_test.go +++ b/pkg/descheduler/strategies/node_taint_test.go @@ -27,6 +27,14 @@ func createNoScheduleTaint(key, value string, index int) v1.Taint { } } +func createPreferNoScheduleTaint(key, value string, index int) v1.Taint { + return v1.Taint{ + Key: "testTaint" + fmt.Sprintf("%v", index), + Value: "test" + fmt.Sprintf("%v", index), + Effect: v1.TaintEffectPreferNoSchedule, + } +} + func addTaintsToNode(node *v1.Node, key, value string, indices []int) *v1.Node { taints := []v1.Taint{} for _, index := range indices { @@ -36,12 +44,12 @@ func addTaintsToNode(node *v1.Node, key, value string, indices []int) *v1.Node { return node } -func addTolerationToPod(pod *v1.Pod, key, value string, index int) *v1.Pod { +func addTolerationToPod(pod *v1.Pod, key, value string, index int, effect v1.TaintEffect) *v1.Pod { if pod.Annotations == nil { pod.Annotations = map[string]string{} } - pod.Spec.Tolerations = []v1.Toleration{{Key: key + fmt.Sprintf("%v", index), Value: value + fmt.Sprintf("%v", index), Effect: v1.TaintEffectNoSchedule}} + pod.Spec.Tolerations = []v1.Toleration{{Key: key + fmt.Sprintf("%v", index), Value: value + fmt.Sprintf("%v", index), Effect: effect}} return pod } @@ -63,6 +71,11 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) { } }) + node5 := test.BuildTestNode("n5", 2000, 3000, 10, nil) + node5.Spec.Taints = []v1.Taint{ + createPreferNoScheduleTaint("testTaint", "test", 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) @@ -109,14 +122,20 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) { // A Mirror Pod. p10.Annotations = test.GetMirrorPodAnnotation() - p1 = addTolerationToPod(p1, "testTaint", "test", 1) - p3 = addTolerationToPod(p3, "testTaint", "test", 1) - p4 = addTolerationToPod(p4, "testTaintX", "testX", 1) + p1 = addTolerationToPod(p1, "testTaint", "test", 1, v1.TaintEffectNoSchedule) + p3 = addTolerationToPod(p3, "testTaint", "test", 1, v1.TaintEffectNoSchedule) + p4 = addTolerationToPod(p4, "testTaintX", "testX", 1, v1.TaintEffectNoSchedule) p12.Spec.NodeSelector = map[string]string{ "datacenter": "west", } + p13 := test.BuildTestPod("p13", 100, 0, node5.Name, nil) + p13.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() + // node5 has PreferNoSchedule:testTaint1=test1, so the p13 has to have + // PreferNoSchedule:testTaint0=test0 so the pod is not tolarated + p13 = addTolerationToPod(p13, "testTaint", "test", 0, v1.TaintEffectPreferNoSchedule) + var uint1 uint = 1 tests := []struct { @@ -129,6 +148,7 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) { maxNoOfPodsToEvictPerNamespace *uint expectedEvictedPodCount uint nodeFit bool + includePreferNoSchedule bool }{ { @@ -224,6 +244,23 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) { expectedEvictedPodCount: 0, //p2 gets evicted nodeFit: true, }, + { + description: "Pods not tolerating PreferNoSchedule node taint should not be evicted when not enabled", + pods: []*v1.Pod{p13}, + nodes: []*v1.Node{node5}, + evictLocalStoragePods: false, + evictSystemCriticalPods: false, + expectedEvictedPodCount: 0, + }, + { + description: "Pods not tolerating PreferNoSchedule node taint should be evicted when enabled", + pods: []*v1.Pod{p13}, + nodes: []*v1.Node{node5}, + evictLocalStoragePods: false, + evictSystemCriticalPods: false, + includePreferNoSchedule: true, + expectedEvictedPodCount: 1, // p13 gets evicted + }, } for _, tc := range tests { @@ -268,7 +305,8 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) { strategy := api.DeschedulerStrategy{ Params: &api.StrategyParameters{ - NodeFit: tc.nodeFit, + NodeFit: tc.nodeFit, + IncludePreferNoSchedule: tc.includePreferNoSchedule, }, }