From a4bfaae8a467d3cee7a43709a7045e2860d7a514 Mon Sep 17 00:00:00 2001 From: wackxu Date: Tue, 18 Jul 2023 20:27:44 +0800 Subject: [PATCH] implement QueueingHint in TaintToleration --- .../tainttoleration/taint_toleration.go | 29 ++++++++- .../tainttoleration/taint_toleration_test.go | 65 +++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go index e6bb7bbe07c..3e95a06d156 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go +++ b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go @@ -23,9 +23,11 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" v1helper "k8s.io/component-helpers/scheduling/corev1" + "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/helper" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/names" + "k8s.io/kubernetes/pkg/scheduler/util" ) // TaintToleration is a plugin that checks if a pod tolerates a node's taints. @@ -56,10 +58,35 @@ func (pl *TaintToleration) Name() string { // failed by this plugin schedulable. func (pl *TaintToleration) EventsToRegister() []framework.ClusterEventWithHint { return []framework.ClusterEventWithHint{ - {Event: framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add | framework.Update}}, + {Event: framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterNodeChange}, } } +// isSchedulableAfterNodeChange is invoked for all node events reported by +// an informer. It checks whether that change made a previously unschedulable +// pod schedulable. +func (pl *TaintToleration) isSchedulableAfterNodeChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) { + originalNode, modifiedNode, err := util.As[*v1.Node](oldObj, newObj) + if err != nil { + return framework.Queue, err + } + + wasUntolerated := true + if originalNode != nil { + _, wasUntolerated = v1helper.FindMatchingUntoleratedTaint(originalNode.Spec.Taints, pod.Spec.Tolerations, helper.DoNotScheduleTaintsFilterFunc()) + } + + _, isUntolerated := v1helper.FindMatchingUntoleratedTaint(modifiedNode.Spec.Taints, pod.Spec.Tolerations, helper.DoNotScheduleTaintsFilterFunc()) + + if wasUntolerated && !isUntolerated { + logger.V(5).Info("node was created or updated, and this may make the Pod rejected by TaintToleration plugin in the previous scheduling cycle schedulable", "pod", klog.KObj(pod), "node", klog.KObj(modifiedNode)) + return framework.Queue, nil + } + + logger.V(5).Info("node was created or updated, but it doesn't change the TaintToleration plugin's decision", "pod", klog.KObj(pod), "node", klog.KObj(modifiedNode)) + return framework.QueueSkip, nil +} + // Filter invoked at the filter extension point. func (pl *TaintToleration) Filter(ctx context.Context, state *framework.CycleState, pod *v1.Pod, nodeInfo *framework.NodeInfo) *framework.Status { node := nodeInfo.Node() diff --git a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go index 1c9694c9306..1f31f675277 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go +++ b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go @@ -353,3 +353,68 @@ func TestTaintTolerationFilter(t *testing.T) { }) } } + +func TestIsSchedulableAfterNodeChange(t *testing.T) { + tests := []struct { + name string + pod *v1.Pod + oldObj interface{} + newObj interface{} + expectedHint framework.QueueingHint + wantErr bool + }{ + { + name: "backoff-wrong-new-object", + newObj: "not-a-node", + expectedHint: framework.Queue, + wantErr: true, + }, + { + name: "backoff-wrong-old-object", + newObj: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), + oldObj: "not-a-node", + expectedHint: framework.Queue, + wantErr: true, + }, + { + name: "skip-queue-on-untoleratedtaint-node-added", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}), + newObj: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), + expectedHint: framework.QueueSkip, + }, + { + name: "queue-on-toleratedtaint-node-added", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}), + newObj: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user2", Effect: "NoSchedule"}}), + expectedHint: framework.Queue, + }, + { + name: "skip-unrelated-change", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}), + newObj: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}, {Key: "dedicated", Value: "user3", Effect: "NoSchedule"}}), + oldObj: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), + expectedHint: framework.QueueSkip, + }, + { + name: "queue-on-taint-change", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}), + newObj: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user2", Effect: "NoSchedule"}}), + oldObj: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), + expectedHint: framework.Queue, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + logger, _ := ktesting.NewTestContext(t) + pl := &TaintToleration{} + got, err := pl.isSchedulableAfterNodeChange(logger, test.pod, test.oldObj, test.newObj) + if (err != nil) != test.wantErr { + t.Errorf("isSchedulableAfterNodeChange() error = %v, wantErr %v", err, test.wantErr) + } + if got != test.expectedHint { + t.Errorf("isSchedulableAfterNodeChange() = %v, want %v", got, test.expectedHint) + } + }) + } +}