diff --git a/pkg/scheduler/internal/queue/scheduling_queue.go b/pkg/scheduler/internal/queue/scheduling_queue.go index 725a05b27f5..393d11ff392 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue.go +++ b/pkg/scheduler/internal/queue/scheduling_queue.go @@ -452,31 +452,32 @@ func (p *PriorityQueue) Update(oldPod, newPod *v1.Pod) error { if oldPodInfo, exists, _ := p.podBackoffQ.Get(oldPodInfo); exists { pInfo := updatePod(oldPodInfo, newPod) p.PodNominator.UpdateNominatedPod(oldPod, pInfo.PodInfo) - p.podBackoffQ.Delete(oldPodInfo) - if err := p.activeQ.Add(pInfo); err != nil { - return err - } - p.cond.Broadcast() - return nil + return p.podBackoffQ.Update(pInfo) } } // If the pod is in the unschedulable queue, updating it may make it schedulable. if usPodInfo := p.unschedulableQ.get(newPod); usPodInfo != nil { - if isPodUpdated(oldPod, newPod) { - p.unschedulableQ.delete(usPodInfo.Pod) - pInfo := updatePod(usPodInfo, newPod) - p.PodNominator.UpdateNominatedPod(oldPod, pInfo.PodInfo) - if err := p.activeQ.Add(pInfo); err != nil { - return err - } - p.cond.Broadcast() - return nil - } pInfo := updatePod(usPodInfo, newPod) p.PodNominator.UpdateNominatedPod(oldPod, pInfo.PodInfo) - // Pod is already in unschedulable queue and hasn't updated, no need to backoff again - p.unschedulableQ.addOrUpdate(pInfo) + if isPodUpdated(oldPod, newPod) { + if p.isPodBackingoff(usPodInfo) { + if err := p.podBackoffQ.Add(pInfo); err != nil { + return err + } + p.unschedulableQ.delete(usPodInfo.Pod) + } else { + if err := p.activeQ.Add(pInfo); err != nil { + return err + } + p.unschedulableQ.delete(usPodInfo.Pod) + p.cond.Broadcast() + } + } else { + // Pod update didn't make it schedulable, keep it in the unschedulable queue. + p.unschedulableQ.addOrUpdate(pInfo) + } + return nil } // If pod is not in any of the queues, we put it in the active queue. diff --git a/pkg/scheduler/internal/queue/scheduling_queue_test.go b/pkg/scheduler/internal/queue/scheduling_queue_test.go index 078e72856d8..ab1adb80533 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue_test.go +++ b/pkg/scheduler/internal/queue/scheduling_queue_test.go @@ -290,7 +290,8 @@ func TestPriorityQueue_Pop(t *testing.T) { } func TestPriorityQueue_Update(t *testing.T) { - q := NewTestQueue(context.Background(), newDefaultQueueSort()) + c := clock.NewFakeClock(time.Now()) + q := NewTestQueue(context.Background(), newDefaultQueueSort(), WithClock(c)) q.Update(nil, highPriorityPodInfo.Pod) if _, exists, _ := q.activeQ.Get(newQueuedPodInfoForLookup(highPriorityPodInfo.Pod)); !exists { t.Errorf("Expected %v to be added to activeQ.", highPriorityPodInfo.Pod.Name) @@ -323,8 +324,22 @@ func TestPriorityQueue_Update(t *testing.T) { if p, err := q.Pop(); err != nil || p.Pod != highPriNominatedPodInfo.Pod { t.Errorf("Expected: %v after Pop, but got: %v", highPriorityPodInfo.Pod.Name, p.Pod.Name) } - // Updating a pod that is in unschedulableQ in a way that it may - // become schedulable should add the pod to the activeQ. + + // Updating a pod that is in backoff queue and it is still backing off + // pod will not be moved to active queue, and it will be updated in backoff queue + podInfo := q.newQueuedPodInfo(medPriorityPodInfo.Pod) + if err := q.podBackoffQ.Add(podInfo); err != nil { + t.Errorf("adding pod to backoff queue error: %v", err) + } + q.Update(podInfo.Pod, podInfo.Pod) + rawPodInfo, err := q.podBackoffQ.Pop() + podGotFromBackoffQ := rawPodInfo.(*framework.QueuedPodInfo).Pod + if err != nil || podGotFromBackoffQ != medPriorityPodInfo.Pod { + t.Errorf("Expected: %v after Pop, but got: %v", medPriorityPodInfo.Pod.Name, podGotFromBackoffQ.Name) + } + + // updating a pod which is in unschedulable queue, and it is still backing off, + // we will move it to backoff queue q.AddUnschedulableIfNotPresent(q.newQueuedPodInfo(medPriorityPodInfo.Pod), q.SchedulingCycle()) if len(q.unschedulableQ.podInfoMap) != 1 { t.Error("Expected unschedulableQ to be 1.") @@ -332,6 +347,24 @@ func TestPriorityQueue_Update(t *testing.T) { updatedPod := medPriorityPodInfo.Pod.DeepCopy() updatedPod.ClusterName = "test" q.Update(medPriorityPodInfo.Pod, updatedPod) + rawPodInfo, err = q.podBackoffQ.Pop() + podGotFromBackoffQ = rawPodInfo.(*framework.QueuedPodInfo).Pod + if err != nil || podGotFromBackoffQ != updatedPod { + t.Errorf("Expected: %v after Pop, but got: %v", updatedPod.Name, podGotFromBackoffQ.Name) + } + + // updating a pod which is in unschedulable queue, and it is not backing off, + // we will move it to active queue + q.AddUnschedulableIfNotPresent(q.newQueuedPodInfo(medPriorityPodInfo.Pod), q.SchedulingCycle()) + if len(q.unschedulableQ.podInfoMap) != 1 { + t.Error("Expected unschedulableQ to be 1.") + } + updatedPod = medPriorityPodInfo.Pod.DeepCopy() + updatedPod.ClusterName = "test1" + // Move clock by podInitialBackoffDuration, so that pods in the unschedulableQ would pass the backing off, + // and the pods will be moved into activeQ. + c.Step(q.podInitialBackoffDuration) + q.Update(medPriorityPodInfo.Pod, updatedPod) if p, err := q.Pop(); err != nil || p.Pod != updatedPod { t.Errorf("Expected: %v after Pop, but got: %v", updatedPod.Name, p.Pod.Name) }