Skip to content

Commit 65a04d5

Browse files
committed
Fix UT to verify runnable ordering.
1 parent d9cc96b commit 65a04d5

File tree

2 files changed

+31
-18
lines changed

2 files changed

+31
-18
lines changed

pkg/manager/manager_test.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1931,8 +1931,10 @@ var _ = Describe("manger.Manager", func() {
19311931

19321932
It("should run warmup runnables before leader election is won", func() {
19331933
By("Creating channels to track execution order")
1934-
warmupCalled := make(chan struct{})
1935-
leaderElectionRunnableCalled := make(chan struct{})
1934+
warmupCalled := atomic.Bool{}
1935+
leaderElectionRunnableStarted := atomic.Bool{}
1936+
// warmupRunnable's WarmupFunc will block until this channel is closed
1937+
warmupRunnableWarmupBlockingChan := make(chan struct{})
19361938

19371939
By("Creating a manager with leader election enabled")
19381940
m, err := New(cfg, Options{
@@ -1946,31 +1948,29 @@ var _ = Describe("manger.Manager", func() {
19461948
})
19471949
Expect(err).NotTo(HaveOccurred())
19481950

1949-
// Override onStoppedLeading to prevent os.Exit
1950-
cm := m.(*controllerManager)
1951-
cm.onStoppedLeading = func() {}
1952-
19531951
By("Creating a runnable that implements WarmupRunnable interface")
19541952
// Create a warmup runnable
19551953
warmupRunnable := warmupRunnableFunc{
19561954
StartFunc: func(ctx context.Context) error {
19571955
// This is the main runnable that will be executed after leader election
1956+
// Block forever
19581957
<-ctx.Done()
19591958
return nil
19601959
},
19611960
WarmupFunc: func(ctx context.Context) error {
19621961
// This should be called during startup before leader election
1963-
close(warmupCalled)
1962+
warmupCalled.Store(true)
1963+
<-warmupRunnableWarmupBlockingChan
19641964
return nil
19651965
},
1966+
didWarmupFinish: make(chan bool),
19661967
}
19671968
Expect(m.Add(warmupRunnable)).To(Succeed())
19681969

19691970
By("Creating a runnable that requires leader election")
19701971
leaderElectionRunnable := leaderElectionRunnableFunc{
19711972
StartFunc: func(ctx context.Context) error {
1972-
// This will only be called after leader election is won
1973-
close(leaderElectionRunnableCalled)
1973+
leaderElectionRunnableStarted.Store(true)
19741974
<-ctx.Done()
19751975
return nil
19761976
},
@@ -1985,17 +1985,21 @@ var _ = Describe("manger.Manager", func() {
19851985

19861986
go func() {
19871987
defer GinkgoRecover()
1988-
Expect(m.Start(ctx)).NotTo(HaveOccurred())
1988+
Expect(m.Start(ctx)).To(Succeed())
19891989
}()
19901990

19911991
By("Waiting for the warmup runnable to be called")
1992-
<-warmupCalled
1992+
Eventually(warmupCalled.Load).Should(BeTrue())
1993+
Expect(leaderElectionRunnableStarted.Load()).To(BeFalse())
1994+
1995+
By("Closing the channel to unblock the warmup runnable")
1996+
close(warmupRunnableWarmupBlockingChan)
19931997

19941998
By("Waiting for leader election to be won")
19951999
<-m.Elected()
19962000

19972001
By("Verifying the leader election runnable is called after election")
1998-
<-leaderElectionRunnableCalled
2002+
Eventually(leaderElectionRunnableStarted.Load).Should(BeTrue())
19992003
})
20002004
})
20012005

pkg/manager/runnable_group_test.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -377,39 +377,48 @@ var _ warmupRunnable = &warmupRunnableFunc{}
377377
// warmupRunnableFunc is a helper struct that implements WarmupRunnable
378378
// for testing purposes.
379379
type warmupRunnableFunc struct {
380-
StartFunc func(context.Context) error
381-
WarmupFunc func(context.Context) error
380+
StartFunc func(context.Context) error
381+
WarmupFunc func(context.Context) error
382+
didWarmupFinish chan bool
382383
}
383384

384385
func (r warmupRunnableFunc) Start(ctx context.Context) error {
385386
return r.StartFunc(ctx)
386387
}
387388

388389
func (r warmupRunnableFunc) Warmup(ctx context.Context) error {
389-
return r.WarmupFunc(ctx)
390+
err := r.WarmupFunc(ctx)
391+
r.didWarmupFinish <- (err == nil)
392+
return err
390393
}
391394

392395
func (r warmupRunnableFunc) WaitForWarmupComplete(ctx context.Context) bool {
393-
return true
396+
return <-r.didWarmupFinish
394397
}
395398

399+
var _ LeaderElectionRunnable = &leaderElectionAndWarmupRunnable{}
400+
var _ warmupRunnable = &leaderElectionAndWarmupRunnable{}
401+
396402
// leaderElectionAndWarmupRunnable implements both WarmupRunnable and LeaderElectionRunnable
397403
type leaderElectionAndWarmupRunnable struct {
398404
StartFunc func(context.Context) error
399405
WarmupFunc func(context.Context) error
400406
NeedLeaderElectionFunc func() bool
407+
didWarmupFinish chan bool
401408
}
402409

403410
func (r leaderElectionAndWarmupRunnable) Start(ctx context.Context) error {
404411
return r.StartFunc(ctx)
405412
}
406413

407414
func (r leaderElectionAndWarmupRunnable) Warmup(ctx context.Context) error {
408-
return r.WarmupFunc(ctx)
415+
err := r.WarmupFunc(ctx)
416+
r.didWarmupFinish <- (err == nil)
417+
return err
409418
}
410419

411420
func (r leaderElectionAndWarmupRunnable) WaitForWarmupComplete(ctx context.Context) bool {
412-
return true
421+
return <-r.didWarmupFinish
413422
}
414423

415424
func (r leaderElectionAndWarmupRunnable) NeedLeaderElection() bool {

0 commit comments

Comments
 (0)