Merge pull request #133518 from macsko/dont_run_preenqueue_when_pod_is_activated_from_backoffq

Don't run PreEnqueue when pod is activated from backoffQ
This commit is contained in:
Kubernetes Prow Robot
2025-08-27 18:30:38 -07:00
committed by GitHub
2 changed files with 45 additions and 13 deletions

View File

@@ -625,13 +625,15 @@ func (p *PriorityQueue) runPreEnqueuePlugin(ctx context.Context, logger klog.Log
// moveToActiveQ tries to add the pod to the active queue.
// If the pod doesn't pass PreEnqueue plugins, it gets added to unschedulablePods instead.
// movesFromBackoffQ should be set to true, if the pod directly moves from the backoffQ, so the PreEnqueue call can be skipped.
// It returns a boolean flag to indicate whether the pod is added successfully.
func (p *PriorityQueue) moveToActiveQ(logger klog.Logger, pInfo *framework.QueuedPodInfo, event string) bool {
func (p *PriorityQueue) moveToActiveQ(logger klog.Logger, pInfo *framework.QueuedPodInfo, event string, movesFromBackoffQ bool) bool {
gatedBefore := pInfo.Gated()
// If SchedulerPopFromBackoffQ feature gate is enabled,
// PreEnqueue plugins were called when the pod was added to the backoffQ.
// Don't need to repeat it here when the pod is directly moved from the backoffQ.
if !p.isPopFromBackoffQEnabled || event != framework.BackoffComplete {
skipPreEnqueue := p.isPopFromBackoffQEnabled && movesFromBackoffQ
if !skipPreEnqueue {
p.runPreEnqueuePlugins(context.Background(), pInfo)
}
@@ -699,7 +701,7 @@ func (p *PriorityQueue) Add(logger klog.Logger, pod *v1.Pod) {
defer p.lock.Unlock()
pInfo := p.newQueuedPodInfo(pod)
if added := p.moveToActiveQ(logger, pInfo, framework.EventUnscheduledPodAdd.Label()); added {
if added := p.moveToActiveQ(logger, pInfo, framework.EventUnscheduledPodAdd.Label(), false); added {
p.activeQ.broadcast()
}
}
@@ -735,6 +737,7 @@ func (p *PriorityQueue) Activate(logger klog.Logger, pods map[string]*v1.Pod) {
func (p *PriorityQueue) activate(logger klog.Logger, pod *v1.Pod) bool {
var pInfo *framework.QueuedPodInfo
var movesFromBackoffQ bool
// Verify if the pod is present in unschedulablePods or backoffQ.
if pInfo = p.unschedulablePods.get(pod); pInfo == nil {
// If the pod doesn't belong to unschedulablePods or backoffQ, don't activate it.
@@ -750,6 +753,7 @@ func (p *PriorityQueue) activate(logger klog.Logger, pod *v1.Pod) bool {
// Pod was popped from the backoffQ in the meantime. Don't activate it.
return false
}
movesFromBackoffQ = true
}
if pInfo == nil {
@@ -758,7 +762,7 @@ func (p *PriorityQueue) activate(logger klog.Logger, pod *v1.Pod) bool {
return false
}
return p.moveToActiveQ(logger, pInfo, framework.ForceActivate)
return p.moveToActiveQ(logger, pInfo, framework.ForceActivate, movesFromBackoffQ)
}
// SchedulingCycle returns current scheduling cycle.
@@ -916,7 +920,7 @@ func (p *PriorityQueue) flushBackoffQCompleted(logger klog.Logger) {
activated := false
podsCompletedBackoff := p.backoffQ.popAllBackoffCompleted(logger)
for _, pInfo := range podsCompletedBackoff {
if added := p.moveToActiveQ(logger, pInfo, framework.BackoffComplete); added {
if added := p.moveToActiveQ(logger, pInfo, framework.BackoffComplete, true); added {
activated = true
}
}
@@ -1069,7 +1073,7 @@ func (p *PriorityQueue) Update(logger klog.Logger, oldPod, newPod *v1.Pod) {
return
}
if added := p.moveToActiveQ(logger, pInfo, framework.EventUnscheduledPodUpdate.Label()); added {
if added := p.moveToActiveQ(logger, pInfo, framework.EventUnscheduledPodUpdate.Label(), false); added {
p.activeQ.broadcast()
}
return
@@ -1081,7 +1085,7 @@ func (p *PriorityQueue) Update(logger klog.Logger, oldPod, newPod *v1.Pod) {
}
// If pod is not in any of the queues, we put it in the active queue.
pInfo := p.newQueuedPodInfo(newPod)
if added := p.moveToActiveQ(logger, pInfo, framework.EventUnscheduledPodUpdate.Label()); added {
if added := p.moveToActiveQ(logger, pInfo, framework.EventUnscheduledPodUpdate.Label(), false); added {
p.activeQ.broadcast()
}
}
@@ -1182,7 +1186,7 @@ func (p *PriorityQueue) requeuePodWithQueueingStrategy(logger klog.Logger, pInfo
}
// Reach here if schedulingHint is QueueImmediately, or schedulingHint is Queue but the pod is not backing off.
if added := p.moveToActiveQ(logger, pInfo, event); added {
if added := p.moveToActiveQ(logger, pInfo, event, false); added {
return activeQ
}
// Pod is gated. We don't have to push it back to unschedulable queue, because moveToActiveQ should already have done that.

View File

@@ -1550,6 +1550,7 @@ func TestPriorityQueue_moveToActiveQ(t *testing.T) {
plugins []framework.PreEnqueuePlugin
pod *v1.Pod
event string
movesFromBackoffQ bool
popFromBackoffQEnabled []bool
wantUnschedulablePods int
wantSuccess bool
@@ -1581,9 +1582,7 @@ func TestPriorityQueue_moveToActiveQ(t *testing.T) {
wantSuccess: false,
},
{
// With SchedulerPopFromBackoffQ enabled, the queue assumes the pod has already passed PreEnqueue,
// and it doesn't run PreEnqueue again, always puts the pod to activeQ.
name: "preEnqueue plugin registered, preEnqueue plugin would reject the pod, but isn't run",
name: "preEnqueue plugin registered, preEnqueue rejects the pod, even if it is after backoff",
plugins: []framework.PreEnqueuePlugin{
&preEnqueuePlugin{allowlists: []string{"foo", "bar"}},
&preEnqueuePlugin{allowlists: []string{"foo"}},
@@ -1595,13 +1594,42 @@ func TestPriorityQueue_moveToActiveQ(t *testing.T) {
wantSuccess: false,
},
{
name: "preEnqueue plugin registered, pod would fail one preEnqueue plugin, but is after backoff",
// With SchedulerPopFromBackoffQ enabled, the queue assumes the pod has already passed PreEnqueue,
// and it doesn't run PreEnqueue again, always puts the pod to activeQ.
name: "preEnqueue plugin registered, pod would fail one preEnqueue plugin, but it is moved from backoffQ after completing backoff, so preEnqueue is not executed",
plugins: []framework.PreEnqueuePlugin{
&preEnqueuePlugin{allowlists: []string{"foo", "bar"}},
&preEnqueuePlugin{allowlists: []string{"foo"}},
},
pod: st.MakePod().Name("bar").Label("bar", "").Obj(),
event: framework.BackoffComplete,
movesFromBackoffQ: true,
popFromBackoffQEnabled: []bool{true},
wantUnschedulablePods: 0,
wantSuccess: true,
},
{
name: "preEnqueue plugin registered, pod failed one preEnqueue plugin when activated from unschedulablePods",
plugins: []framework.PreEnqueuePlugin{
&preEnqueuePlugin{allowlists: []string{"foo", "bar"}},
&preEnqueuePlugin{allowlists: []string{"foo"}},
},
pod: st.MakePod().Name("bar").Label("bar", "").Obj(),
event: framework.ForceActivate,
movesFromBackoffQ: false,
popFromBackoffQEnabled: []bool{true},
wantUnschedulablePods: 1,
wantSuccess: false,
},
{
name: "preEnqueue plugin registered, pod would fail one preEnqueue plugin, but was activated from backoffQ, so preEnqueue is not executed",
plugins: []framework.PreEnqueuePlugin{
&preEnqueuePlugin{allowlists: []string{"foo", "bar"}},
&preEnqueuePlugin{allowlists: []string{"foo"}},
},
pod: st.MakePod().Name("bar").Label("bar", "").Obj(),
event: framework.ForceActivate,
movesFromBackoffQ: true,
popFromBackoffQEnabled: []bool{true},
wantUnschedulablePods: 0,
wantSuccess: true,
@@ -1636,7 +1664,7 @@ func TestPriorityQueue_moveToActiveQ(t *testing.T) {
}
q := NewTestQueueWithObjects(ctx, newDefaultQueueSort(), []runtime.Object{tt.pod}, WithPreEnqueuePluginMap(m),
WithPodInitialBackoffDuration(time.Second*30), WithPodMaxBackoffDuration(time.Second*60))
got := q.moveToActiveQ(logger, q.newQueuedPodInfo(tt.pod), tt.event)
got := q.moveToActiveQ(logger, q.newQueuedPodInfo(tt.pod), tt.event, tt.movesFromBackoffQ)
if got != tt.wantSuccess {
t.Errorf("Unexpected result: want %v, but got %v", tt.wantSuccess, got)
}