Merge pull request #764 from ingvagabund/taints-prefer-no-scheduler

RemovePodsViolatingNodeTaints: optionally include PreferNoSchedule taint

pkg/api/types.go

@@ -88,6 +88,7 @@ type StrategyParameters struct {
 	ThresholdPriorityClassName        string
 	LabelSelector                     *metav1.LabelSelector
 	NodeFit                           bool
+	IncludePreferNoSchedule           bool
 }
 
 type Percentage float64

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

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
 }
 

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 {

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,
 				},
 			}