From 50b697bacb5c9a2780123a8022250756e8d8df33 Mon Sep 17 00:00:00 2001 From: Abdullah Gharaibeh Date: Wed, 8 Jan 2020 16:43:26 -0500 Subject: [PATCH] move TaintToleration predicate to its plugin --- pkg/scheduler/algorithm/predicates/error.go | 7 +- .../algorithm/predicates/predicates.go | 35 +-- .../algorithm/predicates/predicates_test.go | 203 ------------------ pkg/scheduler/core/BUILD | 1 + pkg/scheduler/core/generic_scheduler_test.go | 5 +- .../framework/plugins/tainttoleration/BUILD | 5 +- .../tainttoleration/taint_toleration.go | 31 ++- .../tainttoleration/taint_toleration_test.go | 3 +- 8 files changed, 30 insertions(+), 260 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/error.go b/pkg/scheduler/algorithm/predicates/error.go index cb78437ff67..16bcd58468d 100644 --- a/pkg/scheduler/algorithm/predicates/error.go +++ b/pkg/scheduler/algorithm/predicates/error.go @@ -33,8 +33,6 @@ var ( // ErrNodeSelectorNotMatch is used for MatchNodeSelector predicate error. ErrNodeSelectorNotMatch = NewPredicateFailureError("MatchNodeSelector", "node(s) didn't match node selector") - // ErrTaintsTolerationsNotMatch is used for PodToleratesNodeTaints predicate error. - ErrTaintsTolerationsNotMatch = NewPredicateFailureError("PodToleratesNodeTaints", "node(s) had taints that the pod didn't tolerate") // ErrPodNotMatchHostName is used for HostName predicate error. ErrPodNotMatchHostName = NewPredicateFailureError("HostName", "node(s) didn't match the requested hostname") // ErrPodNotFitsHostPorts is used for PodFitsHostPorts predicate error. @@ -63,9 +61,8 @@ var ( ) var unresolvablePredicateFailureErrors = map[PredicateFailureReason]struct{}{ - ErrNodeSelectorNotMatch: {}, - ErrPodNotMatchHostName: {}, - ErrTaintsTolerationsNotMatch: {}, + ErrNodeSelectorNotMatch: {}, + ErrPodNotMatchHostName: {}, // Node conditions won't change when scheduler simulates removal of preemption victims. // So, it is pointless to try nodes that have not been able to host the pod due to node // conditions. These include ErrNodeNotReady, ErrNodeUnderPIDPressure, ErrNodeUnderMemoryPressure, .... diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index dd1dfd54961..ee7a277ecc7 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -52,8 +52,6 @@ const ( PodToleratesNodeTaintsPred = "PodToleratesNodeTaints" // CheckNodeUnschedulablePred defines the name of predicate CheckNodeUnschedulablePredicate. CheckNodeUnschedulablePred = "CheckNodeUnschedulable" - // PodToleratesNodeNoExecuteTaintsPred defines the name of predicate PodToleratesNodeNoExecuteTaints. - PodToleratesNodeNoExecuteTaintsPred = "PodToleratesNodeNoExecuteTaints" // CheckNodeLabelPresencePred defines the name of predicate CheckNodeLabelPresence. CheckNodeLabelPresencePred = "CheckNodeLabelPresence" // CheckServiceAffinityPred defines the name of predicate checkServiceAffinity. @@ -102,7 +100,7 @@ var ( predicatesOrdering = []string{CheckNodeUnschedulablePred, GeneralPred, HostNamePred, PodFitsHostPortsPred, MatchNodeSelectorPred, PodFitsResourcesPred, NoDiskConflictPred, - PodToleratesNodeTaintsPred, PodToleratesNodeNoExecuteTaintsPred, CheckNodeLabelPresencePred, + PodToleratesNodeTaintsPred, CheckNodeLabelPresencePred, CheckServiceAffinityPred, MaxEBSVolumeCountPred, MaxGCEPDVolumeCountPred, MaxCSIVolumeCountPred, MaxAzureDiskVolumeCountPred, MaxCinderVolumeCountPred, CheckVolumeBindingPred, NoVolumeZoneConflictPred, EvenPodsSpreadPred, MatchInterPodAffinityPred} @@ -306,34 +304,3 @@ func GeneralPredicates(pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.N return len(predicateFails) == 0, predicateFails, nil } - -// PodToleratesNodeTaints checks if a pod tolerations can tolerate the node taints. -func PodToleratesNodeTaints(pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) { - if nodeInfo == nil || nodeInfo.Node() == nil { - return false, []PredicateFailureReason{ErrNodeUnknownCondition}, nil - } - - return podToleratesNodeTaints(pod, nodeInfo, func(t *v1.Taint) bool { - // PodToleratesNodeTaints is only interested in NoSchedule and NoExecute taints. - return t.Effect == v1.TaintEffectNoSchedule || t.Effect == v1.TaintEffectNoExecute - }) -} - -// PodToleratesNodeNoExecuteTaints checks if a pod tolerations can tolerate the node's NoExecute taints. -func PodToleratesNodeNoExecuteTaints(pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) { - return podToleratesNodeTaints(pod, nodeInfo, func(t *v1.Taint) bool { - return t.Effect == v1.TaintEffectNoExecute - }) -} - -func podToleratesNodeTaints(pod *v1.Pod, nodeInfo *schedulernodeinfo.NodeInfo, filter func(t *v1.Taint) bool) (bool, []PredicateFailureReason, error) { - taints, err := nodeInfo.Taints() - if err != nil { - return false, nil, err - } - - if v1helper.TolerationsTolerateTaintsWithFilter(pod.Spec.Tolerations, taints, filter) { - return true, nil, nil - } - return false, []PredicateFailureReason{ErrTaintsTolerationsNotMatch}, nil -} diff --git a/pkg/scheduler/algorithm/predicates/predicates_test.go b/pkg/scheduler/algorithm/predicates/predicates_test.go index 1b4185d6326..56d1522a1f3 100644 --- a/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -1515,206 +1515,3 @@ func TestRunGeneralPredicates(t *testing.T) { }) } } - -func TestPodToleratesTaints(t *testing.T) { - podTolerateTaintsTests := []struct { - pod *v1.Pod - node v1.Node - fits bool - name string - }{ - { - pod: &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod0", - }, - }, - node: v1.Node{ - Spec: v1.NodeSpec{ - Taints: []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}, - }, - }, - fits: false, - name: "A pod having no tolerations can't be scheduled onto a node with nonempty taints", - }, - { - pod: &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod1", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{Image: "pod1:V1"}}, - Tolerations: []v1.Toleration{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}, - }, - }, - node: v1.Node{ - Spec: v1.NodeSpec{ - Taints: []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}, - }, - }, - fits: true, - name: "A pod which can be scheduled on a dedicated node assigned to user1 with effect NoSchedule", - }, - { - pod: &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod2", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{Image: "pod2:V1"}}, - Tolerations: []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}, - }, - }, - node: v1.Node{ - Spec: v1.NodeSpec{ - Taints: []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}, - }, - }, - fits: false, - name: "A pod which can't be scheduled on a dedicated node assigned to user2 with effect NoSchedule", - }, - { - pod: &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod2", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{Image: "pod2:V1"}}, - Tolerations: []v1.Toleration{{Key: "foo", Operator: "Exists", Effect: "NoSchedule"}}, - }, - }, - node: v1.Node{ - Spec: v1.NodeSpec{ - Taints: []v1.Taint{{Key: "foo", Value: "bar", Effect: "NoSchedule"}}, - }, - }, - fits: true, - name: "A pod can be scheduled onto the node, with a toleration uses operator Exists that tolerates the taints on the node", - }, - { - pod: &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod2", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{Image: "pod2:V1"}}, - Tolerations: []v1.Toleration{ - {Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}, - {Key: "foo", Operator: "Exists", Effect: "NoSchedule"}, - }, - }, - }, - node: v1.Node{ - Spec: v1.NodeSpec{ - Taints: []v1.Taint{ - {Key: "dedicated", Value: "user2", Effect: "NoSchedule"}, - {Key: "foo", Value: "bar", Effect: "NoSchedule"}, - }, - }, - }, - fits: true, - name: "A pod has multiple tolerations, node has multiple taints, all the taints are tolerated, pod can be scheduled onto the node", - }, - { - pod: &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod2", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{Image: "pod2:V1"}}, - Tolerations: []v1.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "PreferNoSchedule"}}, - }, - }, - node: v1.Node{ - Spec: v1.NodeSpec{ - Taints: []v1.Taint{ - {Key: "foo", Value: "bar", Effect: "NoSchedule"}, - }, - }, - }, - fits: false, - 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: &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod2", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{Image: "pod2:V1"}}, - Tolerations: []v1.Toleration{{Key: "foo", Operator: "Equal", Value: "bar"}}, - }, - }, - node: v1.Node{ - Spec: v1.NodeSpec{ - Taints: []v1.Taint{ - {Key: "foo", Value: "bar", Effect: "NoSchedule"}, - }, - }, - }, - fits: true, - name: "The pod has a toleration that keys and values match the taint on the node, the effect of toleration is empty, " + - "and the effect of taint is NoSchedule. Pod can be scheduled onto the node", - }, - { - pod: &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod2", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{Image: "pod2:V1"}}, - Tolerations: []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}, - }, - }, - node: v1.Node{ - Spec: v1.NodeSpec{ - Taints: []v1.Taint{ - {Key: "dedicated", Value: "user1", Effect: "PreferNoSchedule"}, - }, - }, - }, - fits: true, - 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", - }, - { - pod: &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod2", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{Image: "pod2:V1"}}, - }, - }, - node: v1.Node{ - Spec: v1.NodeSpec{ - Taints: []v1.Taint{ - {Key: "dedicated", Value: "user1", Effect: "PreferNoSchedule"}, - }, - }, - }, - fits: true, - name: "The pod has no toleration, " + - "but the effect of taint on node is PreferNochedule. Pod can be scheduled onto the node", - }, - } - expectedFailureReasons := []PredicateFailureReason{ErrTaintsTolerationsNotMatch} - - for _, test := range podTolerateTaintsTests { - t.Run(test.name, func(t *testing.T) { - nodeInfo := schedulernodeinfo.NewNodeInfo() - nodeInfo.SetNode(&test.node) - fits, reasons, err := PodToleratesNodeTaints(test.pod, nil, nodeInfo) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if !fits && !reflect.DeepEqual(reasons, expectedFailureReasons) { - t.Errorf("unexpected failure reason: %v, want: %v", reasons, expectedFailureReasons) - } - if fits != test.fits { - t.Errorf("expected: %v got %v", test.fits, fits) - } - }) - } -} diff --git a/pkg/scheduler/core/BUILD b/pkg/scheduler/core/BUILD index 48f8eb4fd95..651d078c910 100644 --- a/pkg/scheduler/core/BUILD +++ b/pkg/scheduler/core/BUILD @@ -58,6 +58,7 @@ go_test( "//pkg/scheduler/framework/plugins/nodelabel:go_default_library", "//pkg/scheduler/framework/plugins/noderesources:go_default_library", "//pkg/scheduler/framework/plugins/podtopologyspread:go_default_library", + "//pkg/scheduler/framework/plugins/tainttoleration:go_default_library", "//pkg/scheduler/framework/plugins/volumebinding:go_default_library", "//pkg/scheduler/framework/plugins/volumerestrictions:go_default_library", "//pkg/scheduler/framework/plugins/volumezone:go_default_library", diff --git a/pkg/scheduler/core/generic_scheduler_test.go b/pkg/scheduler/core/generic_scheduler_test.go index f53e00d1c39..d878fdeadf9 100644 --- a/pkg/scheduler/core/generic_scheduler_test.go +++ b/pkg/scheduler/core/generic_scheduler_test.go @@ -27,7 +27,7 @@ import ( "testing" "time" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -47,6 +47,7 @@ import ( "k8s.io/kubernetes/pkg/scheduler/framework/plugins/nodelabel" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/noderesources" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/podtopologyspread" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/tainttoleration" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumebinding" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumerestrictions" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumezone" @@ -1869,7 +1870,7 @@ func TestNodesWherePreemptionMightHelp(t *testing.T) { nodesStatuses: framework.NodeToStatusMap{ "machine1": framework.NewStatus(framework.UnschedulableAndUnresolvable, algorithmpredicates.ErrNodeSelectorNotMatch.GetReason()), "machine2": framework.NewStatus(framework.UnschedulableAndUnresolvable, algorithmpredicates.ErrPodNotMatchHostName.GetReason()), - "machine3": framework.NewStatus(framework.UnschedulableAndUnresolvable, algorithmpredicates.ErrTaintsTolerationsNotMatch.GetReason()), + "machine3": framework.NewStatus(framework.UnschedulableAndUnresolvable, tainttoleration.ErrReasonNotMatch), "machine4": framework.NewStatus(framework.UnschedulableAndUnresolvable, nodelabel.ErrReasonPresenceViolated), }, expected: map[string]bool{}, diff --git a/pkg/scheduler/framework/plugins/tainttoleration/BUILD b/pkg/scheduler/framework/plugins/tainttoleration/BUILD index ce4bee5d5c7..ed399b6d345 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/BUILD +++ b/pkg/scheduler/framework/plugins/tainttoleration/BUILD @@ -7,9 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/apis/core/v1/helper:go_default_library", - "//pkg/scheduler/algorithm/predicates:go_default_library", - "//pkg/scheduler/algorithm/priorities:go_default_library", - "//pkg/scheduler/framework/plugins/migration:go_default_library", + "//pkg/scheduler/framework/plugins/helper:go_default_library", "//pkg/scheduler/framework/v1alpha1:go_default_library", "//pkg/scheduler/nodeinfo:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", @@ -36,7 +34,6 @@ go_test( srcs = ["taint_toleration_test.go"], embed = [":go_default_library"], deps = [ - "//pkg/scheduler/algorithm/predicates:go_default_library", "//pkg/scheduler/framework/v1alpha1:go_default_library", "//pkg/scheduler/nodeinfo:go_default_library", "//pkg/scheduler/nodeinfo/snapshot:go_default_library", diff --git a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go index bca276d0c18..7184263f7c5 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go +++ b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go @@ -23,9 +23,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" - "k8s.io/kubernetes/pkg/scheduler/algorithm/predicates" - "k8s.io/kubernetes/pkg/scheduler/algorithm/priorities" - "k8s.io/kubernetes/pkg/scheduler/framework/plugins/migration" + pluginhelper "k8s.io/kubernetes/pkg/scheduler/framework/plugins/helper" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" "k8s.io/kubernetes/pkg/scheduler/nodeinfo" ) @@ -44,6 +42,8 @@ const ( Name = "TaintToleration" // postFilterStateKey is the key in CycleState to InterPodAffinity pre-computed data for Scoring. postFilterStateKey = "PostFilter" + Name + // ErrReasonNotMatch is the Filter reason status when not matching. + ErrReasonNotMatch = "node(s) had taints that the pod didn't tolerate" ) // Name returns name of the plugin. It is used in logs, etc. @@ -53,9 +53,23 @@ func (pl *TaintToleration) Name() string { // Filter invoked at the filter extension point. func (pl *TaintToleration) Filter(ctx context.Context, state *framework.CycleState, pod *v1.Pod, nodeInfo *nodeinfo.NodeInfo) *framework.Status { - // Note that PodToleratesNodeTaints doesn't use predicate metadata, hence passing nil here. - _, reasons, err := predicates.PodToleratesNodeTaints(pod, nil, nodeInfo) - return migration.PredicateResultToFrameworkStatus(reasons, err) + if nodeInfo == nil || nodeInfo.Node() == nil { + return framework.NewStatus(framework.Error, "invalid nodeInfo") + } + + taints, err := nodeInfo.Taints() + if err != nil { + return framework.NewStatus(framework.Error, err.Error()) + } + + if v1helper.TolerationsTolerateTaintsWithFilter(pod.Spec.Tolerations, taints, func(t *v1.Taint) bool { + // PodToleratesNodeTaints is only interested in NoSchedule and NoExecute taints. + return t.Effect == v1.TaintEffectNoSchedule || t.Effect == v1.TaintEffectNoExecute + }) { + return nil + } + + return framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonNotMatch) } // postFilterState computed at PostFilter and used at Score. @@ -140,10 +154,7 @@ func (pl *TaintToleration) Score(ctx context.Context, state *framework.CycleStat // NormalizeScore invoked after scoring all nodes. func (pl *TaintToleration) NormalizeScore(ctx context.Context, _ *framework.CycleState, pod *v1.Pod, scores framework.NodeScoreList) *framework.Status { - // Note that ComputeTaintTolerationPriorityReduce doesn't use priority metadata, hence passing nil here. - normalizeFun := priorities.NormalizeReduce(framework.MaxNodeScore, true) - err := normalizeFun(pod, nil, pl.handle.SnapshotSharedLister(), scores) - return migration.ErrorToFrameworkStatus(err) + return pluginhelper.DefaultNormalizeScore(framework.MaxNodeScore, true, scores) } // ScoreExtensions of the Score plugin. diff --git a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go index ebbb53b8b72..48c6f937f29 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go +++ b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go @@ -23,7 +23,6 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/kubernetes/pkg/scheduler/algorithm/predicates" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" nodeinfosnapshot "k8s.io/kubernetes/pkg/scheduler/nodeinfo/snapshot" @@ -261,7 +260,7 @@ func TestTaintTolerationScore(t *testing.T) { } func TestTaintTolerationFilter(t *testing.T) { - unschedulable := framework.NewStatus(framework.UnschedulableAndUnresolvable, predicates.ErrTaintsTolerationsNotMatch.GetReason()) + unschedulable := framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonNotMatch) tests := []struct { name string pod *v1.Pod