diff --git a/pkg/descheduler/strategies/node_taint.go b/pkg/descheduler/strategies/node_taint.go index f85a8403d..3fa63518b 100644 --- a/pkg/descheduler/strategies/node_taint.go +++ b/pkg/descheduler/strategies/node_taint.go @@ -28,11 +28,6 @@ import ( "k8s.io/klog" ) -const ( - TolerationOpExists v1.TolerationOperator = "Exists" - TolerationOpEqual v1.TolerationOperator = "Equal" -) - // RemovePodsViolatingNodeTaints with elimination strategy func RemovePodsViolatingNodeTaints(ds *options.DeschedulerServer, strategy api.DeschedulerStrategy, policyGroupVersion string, nodes []*v1.Node, nodePodCount utils.NodePodEvictedCount) { if !strategy.Enabled { @@ -56,7 +51,12 @@ func deletePodsViolatingNodeTaints(client clientset.Interface, policyGroupVersio if maxPodsToEvict > 0 && nodePodCount[node]+1 > maxPodsToEvict { break } - if !checkPodsSatisfyTolerations(pods[i], node) { + if !utils.TolerationsTolerateTaintsWithFilter( + pods[i].Spec.Tolerations, + node.Spec.Taints, + func(taint *v1.Taint) bool { return taint.Effect == v1.TaintEffectNoSchedule }, + ) { + klog.V(2).Infof("Not all taints with NoSchedule effect are tolerated after update for pod %v on node %v", pods[i].Name, node.Name) success, err := evictions.EvictPod(client, pods[i], policyGroupVersion, dryRun) if !success { klog.Errorf("Error when evicting pod: %#v (%#v)\n", pods[i].Name, err) @@ -70,70 +70,3 @@ func deletePodsViolatingNodeTaints(client clientset.Interface, policyGroupVersio } return podsEvicted } - -// checkPodsSatisfyTolerations checks if the node's taints (NoSchedule) are still satisfied by pods' tolerations. -func checkPodsSatisfyTolerations(pod *v1.Pod, node *v1.Node) bool { - tolerations := pod.Spec.Tolerations - taints := node.Spec.Taints - if len(taints) == 0 { - return true - } - noScheduleTaints := getNoScheduleTaints(taints) - if !allTaintsTolerated(noScheduleTaints, tolerations) { - klog.V(2).Infof("Not all taints are tolerated after update for Pod %v on node %v", pod.Name, node.Name) - return false - } - return true -} - -// getNoScheduleTaints return a slice of NoSchedule taints from the a slice of taints that it receives. -func getNoScheduleTaints(taints []v1.Taint) []v1.Taint { - result := []v1.Taint{} - for i := range taints { - if taints[i].Effect == v1.TaintEffectNoSchedule { - result = append(result, taints[i]) - } - } - return result -} - -//toleratesTaint returns true if a toleration tolerates a taint, or false otherwise -func toleratesTaint(toleration *v1.Toleration, taint *v1.Taint) bool { - - if (len(toleration.Key) > 0 && toleration.Key != taint.Key) || - (len(toleration.Effect) > 0 && toleration.Effect != taint.Effect) { - return false - } - switch toleration.Operator { - // empty operator means Equal - case "", TolerationOpEqual: - return toleration.Value == taint.Value - case TolerationOpExists: - return true - default: - return false - } -} - -// allTaintsTolerated returns true if all are tolerated, or false otherwise. -func allTaintsTolerated(taints []v1.Taint, tolerations []v1.Toleration) bool { - if len(taints) == 0 { - return true - } - if len(tolerations) == 0 && len(taints) > 0 { - return false - } - for i := range taints { - tolerated := false - for j := range tolerations { - if toleratesTaint(&tolerations[j], &taints[i]) { - tolerated = true - break - } - } - if !tolerated { - return false - } - } - return true -} diff --git a/pkg/descheduler/strategies/node_taint_test.go b/pkg/descheduler/strategies/node_taint_test.go index 726f59ddf..b76313ed2 100644 --- a/pkg/descheduler/strategies/node_taint_test.go +++ b/pkg/descheduler/strategies/node_taint_test.go @@ -188,7 +188,7 @@ func TestToleratesTaint(t *testing.T) { description: "toleration and taint have the same key and effect, and operator is Exists, and taint has no value, expect tolerated", toleration: v1.Toleration{ Key: "foo", - Operator: TolerationOpExists, + Operator: v1.TolerationOpExists, Effect: v1.TaintEffectNoSchedule, }, taint: v1.Taint{ @@ -201,7 +201,7 @@ func TestToleratesTaint(t *testing.T) { description: "toleration and taint have the same key and effect, and operator is Exists, and taint has some value, expect tolerated", toleration: v1.Toleration{ Key: "foo", - Operator: TolerationOpExists, + Operator: v1.TolerationOpExists, Effect: v1.TaintEffectNoSchedule, }, taint: v1.Taint{ @@ -215,7 +215,7 @@ func TestToleratesTaint(t *testing.T) { description: "toleration and taint have the same effect, toleration has empty key and operator is Exists, means match all taints, expect tolerated", toleration: v1.Toleration{ Key: "", - Operator: TolerationOpExists, + Operator: v1.TolerationOpExists, Effect: v1.TaintEffectNoSchedule, }, taint: v1.Taint{ @@ -229,7 +229,7 @@ func TestToleratesTaint(t *testing.T) { description: "toleration and taint have the same key, effect and value, and operator is Equal, expect tolerated", toleration: v1.Toleration{ Key: "foo", - Operator: TolerationOpEqual, + Operator: v1.TolerationOpEqual, Value: "bar", Effect: v1.TaintEffectNoSchedule, }, @@ -244,7 +244,7 @@ func TestToleratesTaint(t *testing.T) { description: "toleration and taint have the same key and effect, but different values, and operator is Equal, expect not tolerated", toleration: v1.Toleration{ Key: "foo", - Operator: TolerationOpEqual, + Operator: v1.TolerationOpEqual, Value: "value1", Effect: v1.TaintEffectNoSchedule, }, @@ -259,7 +259,7 @@ func TestToleratesTaint(t *testing.T) { description: "toleration and taint have the same key and value, but different effects, and operator is Equal, expect not tolerated", toleration: v1.Toleration{ Key: "foo", - Operator: TolerationOpEqual, + Operator: v1.TolerationOpEqual, Value: "bar", Effect: v1.TaintEffectNoSchedule, }, @@ -272,27 +272,8 @@ func TestToleratesTaint(t *testing.T) { }, } for _, tc := range testCases { - if tolerated := toleratesTaint(&tc.toleration, &tc.taint); tc.expectTolerated != tolerated { + if tolerated := tc.toleration.ToleratesTaint(&tc.taint); tc.expectTolerated != tolerated { t.Errorf("[%s] expect %v, got %v: toleration %+v, taint %s", tc.description, tc.expectTolerated, tolerated, tc.toleration, tc.taint.ToString()) } } } - -func TestFilterNoExecuteTaints(t *testing.T) { - taints := []v1.Taint{ - { - Key: "one", - Value: "one", - Effect: v1.TaintEffectNoExecute, - }, - { - Key: "two", - Value: "two", - Effect: v1.TaintEffectNoSchedule, - }, - } - taints = getNoScheduleTaints(taints) - if len(taints) != 1 || taints[0].Key != "two" { - t.Errorf("Filtering doesn't work. Got %v", taints) - } -}