Skip to content

Commit 018c7f7

Browse files
committed
Add lock to TestAsyncPreemption to prevent races
1 parent b6fee16 commit 018c7f7

File tree

2 files changed

+12
-3
lines changed

2 files changed

+12
-3
lines changed

pkg/scheduler/backend/queue/scheduling_queue.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,6 @@ func (p *PriorityQueue) Update(logger klog.Logger, oldPod, newPod *v1.Pod) {
10081008
if pInfo := p.unschedulablePods.get(newPod); pInfo != nil {
10091009
_ = pInfo.Update(newPod)
10101010
p.UpdateNominatedPod(logger, oldPod, pInfo.PodInfo)
1011-
gated := pInfo.Gated()
10121011
if p.isSchedulingQueueHintEnabled {
10131012
// When unscheduled Pods are updated, we check with QueueingHint
10141013
// whether the update may make the pods schedulable.
@@ -1032,7 +1031,6 @@ func (p *PriorityQueue) Update(logger klog.Logger, oldPod, newPod *v1.Pod) {
10321031
// so we should check isPodBackingoff before moving the pod to backoffQ.
10331032
if p.backoffQ.isPodBackingoff(pInfo) {
10341033
if added := p.moveToBackoffQ(logger, pInfo, framework.EventUnscheduledPodUpdate.Label()); added {
1035-
p.unschedulablePods.delete(pInfo.Pod, gated)
10361034
if p.isPopFromBackoffQEnabled {
10371035
p.activeQ.broadcast()
10381036
}

test/integration/scheduler/preemption/preemption_test.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"context"
2323
"fmt"
2424
"strings"
25+
"sync"
2526
"testing"
2627
"time"
2728

@@ -813,9 +814,12 @@ func TestAsyncPreemption(t *testing.T) {
813814
t.Run(test.name, func(t *testing.T) {
814815
// We need to use a custom preemption plugin to test async preemption behavior
815816
delayedPreemptionPluginName := "delay-preemption"
817+
var lock sync.Mutex
816818
// keyed by the pod name
817819
preemptionDoneChannels := make(map[string]chan struct{})
818820
defer func() {
821+
lock.Lock()
822+
defer lock.Unlock()
819823
for _, ch := range preemptionDoneChannels {
820824
close(ch)
821825
}
@@ -841,7 +845,10 @@ func TestAsyncPreemption(t *testing.T) {
841845
preemptPodFn := preemptionPlugin.Evaluator.PreemptPod
842846
preemptionPlugin.Evaluator.PreemptPod = func(ctx context.Context, c preemption.Candidate, preemptor, victim *v1.Pod, pluginName string) error {
843847
// block the preemption goroutine to complete until the test case allows it to proceed.
844-
if ch, ok := preemptionDoneChannels[preemptor.Name]; ok {
848+
lock.Lock()
849+
ch, ok := preemptionDoneChannels[preemptor.Name]
850+
lock.Unlock()
851+
if ok {
845852
<-ch
846853
}
847854
return preemptPodFn(ctx, c, preemptor, victim, pluginName)
@@ -941,7 +948,9 @@ func TestAsyncPreemption(t *testing.T) {
941948
t.Fatal(lastFailure)
942949
}
943950

951+
lock.Lock()
944952
preemptionDoneChannels[scenario.schedulePod.podName] = make(chan struct{})
953+
lock.Unlock()
945954
testCtx.Scheduler.ScheduleOne(testCtx.Ctx)
946955
if scenario.schedulePod.expectSuccess {
947956
if err := wait.PollUntilContextTimeout(testCtx.Ctx, 200*time.Millisecond, wait.ForeverTestTimeout, false, testutils.PodScheduled(cs, testCtx.NS.Name, scenario.schedulePod.podName)); err != nil {
@@ -953,12 +962,14 @@ func TestAsyncPreemption(t *testing.T) {
953962
}
954963
}
955964
case scenario.completePreemption != "":
965+
lock.Lock()
956966
if _, ok := preemptionDoneChannels[scenario.completePreemption]; !ok {
957967
t.Fatalf("The preemptor Pod %q is not running preemption", scenario.completePreemption)
958968
}
959969

960970
close(preemptionDoneChannels[scenario.completePreemption])
961971
delete(preemptionDoneChannels, scenario.completePreemption)
972+
lock.Unlock()
962973
case scenario.podGatedInQueue != "":
963974
// make sure the Pod is in the queue in the first place.
964975
if !podInUnschedulablePodPool(t, testCtx.Scheduler.SchedulingQueue, scenario.podGatedInQueue) {

0 commit comments

Comments
 (0)