Skip to content

Commit 54f4fe3

Browse files
committed
Document + add UT for WaitForWarmupComplete behavior on ctx cancellation.
1 parent ccc7485 commit 54f4fe3

File tree

2 files changed

+47
-20
lines changed

2 files changed

+47
-20
lines changed

pkg/internal/controller/controller.go

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"k8s.io/apimachinery/pkg/types"
3030
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
3131
"k8s.io/apimachinery/pkg/util/uuid"
32-
"k8s.io/apimachinery/pkg/util/wait"
3332
"k8s.io/client-go/util/workqueue"
3433
"k8s.io/utils/ptr"
3534

@@ -176,28 +175,28 @@ func (c *Controller[request]) Warmup(ctx context.Context) error {
176175
}
177176

178177
// WaitForWarmupComplete returns true if warmup has completed without error, and false if there was
179-
// an error during warmup.
178+
// an error during warmup. If context is cancelled, it returns true.
180179
func (c *Controller[request]) WaitForWarmupComplete(ctx context.Context) bool {
181-
err := wait.PollUntilContextCancel(ctx, syncedPollPeriod, true, func(ctx context.Context) (bool, error) {
182-
didFinishSync, ok := c.didEventSourcesFinishSyncSuccessfully.Load().(*bool)
183-
if !ok {
184-
return false, errors.New("unexpected error: didEventSourcesFinishSync is not a bool pointer")
185-
}
186-
187-
if didFinishSync == nil {
188-
// event sources not finished syncing
189-
return false, nil
190-
}
180+
ticker := time.NewTicker(syncedPollPeriod)
181+
defer ticker.Stop()
182+
183+
for {
184+
select {
185+
case <-ctx.Done():
186+
return true
187+
case <-ticker.C:
188+
didFinishSync, ok := c.didEventSourcesFinishSyncSuccessfully.Load().(*bool)
189+
if !ok {
190+
return false
191+
}
191192

192-
if !*didFinishSync {
193-
// event sources finished syncing with an error
194-
return true, errors.New("event sources did not finish syncing successfully")
193+
if didFinishSync != nil && *didFinishSync {
194+
// event sources finished syncing successfully
195+
return true
196+
}
197+
return false
195198
}
196-
197-
return true, nil
198-
})
199-
200-
return err == nil
199+
}
201200
}
202201

203202
// Start implements controller.Controller.

pkg/internal/controller/controller_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,6 +1061,34 @@ var _ = Describe("controller", func() {
10611061
result := ctrl.WaitForWarmupComplete(ctx)
10621062
Expect(result).To(BeFalse())
10631063
})
1064+
1065+
It("should return true if context is cancelled", func() {
1066+
// Setup controller with sources that complete with error
1067+
ctx, cancel := context.WithCancel(context.Background())
1068+
defer cancel()
1069+
1070+
ctrl.CacheSyncTimeout = time.Second
1071+
ctrl.startWatches = []source.TypedSource[reconcile.Request]{
1072+
source.Func(func(ctx context.Context, _ workqueue.TypedRateLimitingInterface[reconcile.Request]) error {
1073+
<-ctx.Done()
1074+
return nil
1075+
}),
1076+
}
1077+
1078+
resultChan := make(chan bool)
1079+
1080+
// Invoked in goroutines because the Warmup / WaitForWarmupComplete will block forever.
1081+
go func() {
1082+
err := ctrl.Warmup(ctx)
1083+
Expect(err).NotTo(HaveOccurred())
1084+
}()
1085+
go func() {
1086+
resultChan <- ctrl.WaitForWarmupComplete(ctx)
1087+
}()
1088+
1089+
cancel()
1090+
Expect(<-resultChan).To(BeTrue())
1091+
})
10641092
})
10651093

10661094
Describe("Warmup with warmup disabled", func() {

0 commit comments

Comments
 (0)