From 22bd26fefb9e5f66cfd39ac8feed87e2c542ada7 Mon Sep 17 00:00:00 2001 From: Shawn Rebello Date: Fri, 24 Jan 2020 02:50:55 +0530 Subject: [PATCH] Adding taint toleration error reasons --- pkg/apis/core/v1/helper/helpers.go | 37 +++++++++++++------ .../tainttoleration/taint_toleration.go | 11 ++++-- .../tainttoleration/taint_toleration_test.go | 30 ++++++++------- 3 files changed, 50 insertions(+), 28 deletions(-) diff --git a/pkg/apis/core/v1/helper/helpers.go b/pkg/apis/core/v1/helper/helpers.go index 7a573a5227c..21697bf5a0f 100644 --- a/pkg/apis/core/v1/helper/helpers.go +++ b/pkg/apis/core/v1/helper/helpers.go @@ -395,22 +395,37 @@ type taintsFilterFunc func(*v1.Taint) bool // TolerationsTolerateTaintsWithFilter checks if given tolerations tolerates // all the taints that apply to the filter in given taint list. +// DEPRECATED: Please use FindMatchingUntoleratedTaint instead. func TolerationsTolerateTaintsWithFilter(tolerations []v1.Toleration, taints []v1.Taint, applyFilter taintsFilterFunc) bool { - if len(taints) == 0 { - return true - } + _, isUntolerated := FindMatchingUntoleratedTaint(taints, tolerations, applyFilter) + return !isUntolerated +} - for i := range taints { - if applyFilter != nil && !applyFilter(&taints[i]) { +// FindMatchingUntoleratedTaint checks if the given tolerations tolerates +// all the filtered taints, and returns the first taint without a toleration +func FindMatchingUntoleratedTaint(taints []v1.Taint, tolerations []v1.Toleration, inclusionFilter taintsFilterFunc) (v1.Taint, bool) { + filteredTaints := getFilteredTaints(taints, inclusionFilter) + for _, taint := range filteredTaints { + if !TolerationsTolerateTaint(tolerations, &taint) { + return taint, true + } + } + return v1.Taint{}, false +} + +// getFilteredTaints returns a list of taints satisfying the filter predicate +func getFilteredTaints(taints []v1.Taint, inclusionFilter taintsFilterFunc) []v1.Taint { + if inclusionFilter == nil { + return taints + } + filteredTaints := []v1.Taint{} + for _, taint := range taints { + if !inclusionFilter(&taint) { continue } - - if !TolerationsTolerateTaint(tolerations, &taints[i]) { - return false - } + filteredTaints = append(filteredTaints, taint) } - - return true + return filteredTaints } // Returns true and list of Tolerations matching all Taints if all are tolerated, or false otherwise. diff --git a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go index 7f596ca101f..93164dd895d 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go +++ b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go @@ -62,14 +62,19 @@ func (pl *TaintToleration) Filter(ctx context.Context, state *framework.CycleSta return framework.NewStatus(framework.Error, err.Error()) } - if v1helper.TolerationsTolerateTaintsWithFilter(pod.Spec.Tolerations, taints, func(t *v1.Taint) bool { + filterPredicate := func(t *v1.Taint) bool { // PodToleratesNodeTaints is only interested in NoSchedule and NoExecute taints. return t.Effect == v1.TaintEffectNoSchedule || t.Effect == v1.TaintEffectNoExecute - }) { + } + + taint, isUntolerated := v1helper.FindMatchingUntoleratedTaint(taints, pod.Spec.Tolerations, filterPredicate) + if !isUntolerated { return nil } - return framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonNotMatch) + errReason := fmt.Sprintf("node(s) had taint {%s: %s}, that the pod didn't tolerate", + taint.Key, taint.Value) + return framework.NewStatus(framework.UnschedulableAndUnresolvable, errReason) } // postFilterState computed at PostFilter and used at Score. diff --git a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go index 48c6f937f29..eb1eb208f0f 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go +++ b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go @@ -260,7 +260,6 @@ func TestTaintTolerationScore(t *testing.T) { } func TestTaintTolerationFilter(t *testing.T) { - unschedulable := framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonNotMatch) tests := []struct { name string pod *v1.Pod @@ -268,10 +267,11 @@ func TestTaintTolerationFilter(t *testing.T) { wantStatus *framework.Status }{ { - name: "A pod having no tolerations can't be scheduled onto a node with nonempty taints", - pod: podWithTolerations("pod1", []v1.Toleration{}), - node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), - wantStatus: unschedulable, + name: "A pod having no tolerations can't be scheduled onto a node with nonempty taints", + pod: podWithTolerations("pod1", []v1.Toleration{}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, + "node(s) had taint {dedicated: user1}, that the pod didn't tolerate"), }, { name: "A pod which can be scheduled on a dedicated node assigned to user1 with effect NoSchedule", @@ -279,10 +279,11 @@ func TestTaintTolerationFilter(t *testing.T) { node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), }, { - name: "A pod which can't be scheduled on a dedicated node assigned to user2 with effect NoSchedule", - pod: podWithTolerations("pod1", []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}), - node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), - wantStatus: unschedulable, + name: "A pod which can't be scheduled on a dedicated node assigned to user2 with effect NoSchedule", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, + "node(s) had taint {dedicated: user1}, that the pod didn't tolerate"), }, { name: "A pod can be scheduled onto the node, with a toleration uses operator Exists that tolerates the taints on the node", @@ -303,9 +304,10 @@ func TestTaintTolerationFilter(t *testing.T) { { name: "A pod has a toleration that keys and values match the taint on the node, but (non-empty) effect doesn't match, " + "can't be scheduled onto the node", - pod: podWithTolerations("pod1", []v1.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "PreferNoSchedule"}}), - node: nodeWithTaints("nodeA", []v1.Taint{{Key: "foo", Value: "bar", Effect: "NoSchedule"}}), - wantStatus: unschedulable, + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "PreferNoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "foo", Value: "bar", Effect: "NoSchedule"}}), + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, + "node(s) had taint {foo: bar}, that the pod didn't tolerate"), }, { name: "The pod has a toleration that keys and values match the taint on the node, the effect of toleration is empty, " + @@ -315,13 +317,13 @@ func TestTaintTolerationFilter(t *testing.T) { }, { name: "The pod has a toleration that key and value don't match the taint on the node, " + - "but the effect of taint on node is PreferNochedule. Pod can be scheduled onto the node", + "but the effect of taint on node is PreferNoSchedule. Pod can be scheduled onto the node", pod: podWithTolerations("pod1", []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}), node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "PreferNoSchedule"}}), }, { name: "The pod has no toleration, " + - "but the effect of taint on node is PreferNochedule. Pod can be scheduled onto the node", + "but the effect of taint on node is PreferNoSchedule. Pod can be scheduled onto the node", pod: podWithTolerations("pod1", []v1.Toleration{}), node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "PreferNoSchedule"}}), },