From dd3af9a85b887633af4d3bb3acedc09e21808a1d Mon Sep 17 00:00:00 2001 From: Kensei Nakada Date: Sat, 15 Jun 2024 22:36:42 +0000 Subject: [PATCH] fix: skip isPodWorthRequeuing only when SchedulingGates gates the pod --- pkg/scheduler/internal/queue/scheduling_queue.go | 4 +++- .../internal/queue/scheduling_queue_test.go | 16 ++++++++++++---- .../scheduler/plugins/plugins_test.go | 9 +++++---- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/pkg/scheduler/internal/queue/scheduling_queue.go b/pkg/scheduler/internal/queue/scheduling_queue.go index 292e72916f6..6788d04d282 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue.go +++ b/pkg/scheduler/internal/queue/scheduling_queue.go @@ -47,6 +47,7 @@ import ( "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/names" "k8s.io/kubernetes/pkg/scheduler/internal/heap" "k8s.io/kubernetes/pkg/scheduler/metrics" "k8s.io/kubernetes/pkg/scheduler/util" @@ -1195,9 +1196,10 @@ func (p *PriorityQueue) movePodsToActiveOrBackoffQueue(logger klog.Logger, podIn for _, pInfo := range podInfoList { // Since there may be many gated pods and they will not move from the // unschedulable pool, we skip calling the expensive isPodWorthRequeueing. - if pInfo.Gated { + if pInfo.Gated && pInfo.UnschedulablePlugins.Has(names.SchedulingGates) { continue } + schedulingHint := p.isPodWorthRequeuing(logger, pInfo, event, oldObj, newObj) if schedulingHint == queueSkip { // QueueingHintFn determined that this Pod isn't worth putting to activeQ or backoffQ by this event. diff --git a/pkg/scheduler/internal/queue/scheduling_queue_test.go b/pkg/scheduler/internal/queue/scheduling_queue_test.go index c2ad3d8ce67..c12fd1f6726 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue_test.go +++ b/pkg/scheduler/internal/queue/scheduling_queue_test.go @@ -44,6 +44,7 @@ import ( "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/scheduler/framework" plfeature "k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/names" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/queuesort" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/schedulinggates" "k8s.io/kubernetes/pkg/scheduler/metrics" @@ -1499,13 +1500,21 @@ func TestPriorityQueue_MoveAllToActiveOrBackoffQueueWithQueueingHint(t *testing. expectedQ: unschedulablePods, }, { - name: "QueueHintFunction is not called when Pod is gated", - podInfo: setQueuedPodInfoGated(&framework.QueuedPodInfo{PodInfo: mustNewPodInfo(p), UnschedulablePlugins: sets.New("foo")}), + name: "QueueHintFunction is not called when Pod is gated by SchedulingGates plugin", + podInfo: setQueuedPodInfoGated(&framework.QueuedPodInfo{PodInfo: mustNewPodInfo(p), UnschedulablePlugins: sets.New(names.SchedulingGates, "foo")}), hint: func(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) { return framework.Queue, fmt.Errorf("QueueingHintFn should not be called as pod is gated") }, expectedQ: unschedulablePods, }, + { + name: "QueueHintFunction is called when Pod is gated by a plugin other than SchedulingGates", + podInfo: setQueuedPodInfoGated(&framework.QueuedPodInfo{PodInfo: mustNewPodInfo(p), UnschedulablePlugins: sets.New("foo")}), + hint: queueHintReturnQueue, + // FIXME: This should be backoffQ. + // https://github.com/kubernetes/kubernetes/issues/125538 + expectedQ: activeQ, + }, } for _, test := range tests { @@ -1518,14 +1527,13 @@ func TestPriorityQueue_MoveAllToActiveOrBackoffQueueWithQueueingHint(t *testing. QueueingHintFn: test.hint, }, } - test.podInfo.UnschedulablePlugins = sets.New("foo") cl := testingclock.NewFakeClock(now) q := NewTestQueue(ctx, newDefaultQueueSort(), WithQueueingHintMapPerProfile(m), WithClock(cl)) - // add to unsched pod pool q.activeQ.Add(q.newQueuedPodInfo(test.podInfo.Pod)) if p, err := q.Pop(logger); err != nil || p.Pod != test.podInfo.Pod { t.Errorf("Expected: %v after Pop, but got: %v", test.podInfo.Pod.Name, p.Pod.Name) } + // add to unsched pod pool err := q.AddUnschedulableIfNotPresent(logger, test.podInfo, q.SchedulingCycle()) if err != nil { t.Fatalf("unexpected error from AddUnschedulableIfNotPresent: %v", err) diff --git a/test/integration/scheduler/plugins/plugins_test.go b/test/integration/scheduler/plugins/plugins_test.go index 7f2c3d53fc0..306a8d398fc 100644 --- a/test/integration/scheduler/plugins/plugins_test.go +++ b/test/integration/scheduler/plugins/plugins_test.go @@ -2668,8 +2668,9 @@ func TestSchedulingGatesPluginEventsToRegister(t *testing.T) { } tests := []struct { - name string - withEvents bool + name string + withEvents bool + // count is the expected number of calls to PreEnqueue(). count int queueHintEnabled []bool expectedScheduled []bool @@ -2686,7 +2687,7 @@ func TestSchedulingGatesPluginEventsToRegister(t *testing.T) { { name: "preEnqueue plugin with event registered", withEvents: true, - count: 2, + count: 3, queueHintEnabled: []bool{false, true}, expectedScheduled: []bool{true, true}, }, @@ -2700,7 +2701,7 @@ func TestSchedulingGatesPluginEventsToRegister(t *testing.T) { t.Run(tt.name+fmt.Sprintf(" queueHint(%v)", queueHintEnabled), func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SchedulerQueueingHints, queueHintEnabled) - // new plugin every time to clear counts + // use new plugin every time to clear counts var plugin framework.PreEnqueuePlugin if tt.withEvents { plugin = &SchedulingGatesPluginWithEvents{SchedulingGates: schedulinggates.SchedulingGates{}}