Skip to content

Commit 66f64f0

Browse files
committed
Move reset watches critical section inside of startEventSources.
1 parent 9d5ddfb commit 66f64f0

File tree

2 files changed

+24
-7
lines changed

2 files changed

+24
-7
lines changed

pkg/internal/controller/controller.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -233,12 +233,6 @@ func (c *Controller[request]) Start(ctx context.Context) error {
233233

234234
c.LogConstructor(nil).Info("Starting Controller")
235235

236-
// All the watches have been started, we can reset the local slice.
237-
//
238-
// We should never hold watches more than necessary, each watch source can hold a backing cache,
239-
// which won't be garbage collected if we hold a reference to it.
240-
c.startWatches = nil
241-
242236
// Launch workers to process resources
243237
c.LogConstructor(nil).Info("Starting workers", "worker count", c.MaxConcurrentReconciles)
244238
wg.Add(c.MaxConcurrentReconciles)
@@ -325,6 +319,12 @@ func (c *Controller[request]) startEventSources(ctx context.Context) error {
325319
})
326320
}
327321
retErr = errGroup.Wait()
322+
323+
// All the watches have been started, we can reset the local slice.
324+
//
325+
// We should never hold watches more than necessary, each watch source can hold a backing cache,
326+
// which won't be garbage collected if we hold a reference to it.
327+
c.startWatches = nil
328328
})
329329

330330
return retErr

pkg/internal/controller/controller_test.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ var _ = Describe("controller", func() {
503503
Expect(err.Error()).To(ContainSubstring("timed out waiting for source"))
504504
})
505505

506-
It("should only start sources once when called multiple times", func() {
506+
It("should only start sources once when called multiple times concurrently", func() {
507507
ctx, cancel := context.WithCancel(context.Background())
508508
defer cancel()
509509

@@ -532,6 +532,23 @@ var _ = Describe("controller", func() {
532532
wg.Wait()
533533
Expect(startCount.Load()).To(Equal(int32(1)), "Source should only be started once even when called multiple times")
534534
})
535+
536+
It("should reset c.startWatches to nil after returning", func() {
537+
ctx, cancel := context.WithCancel(context.Background())
538+
defer cancel()
539+
540+
ctrl.CacheSyncTimeout = 1 * time.Millisecond
541+
542+
src := source.Func(func(ctx context.Context, _ workqueue.TypedRateLimitingInterface[reconcile.Request]) error {
543+
return nil
544+
})
545+
546+
ctrl.startWatches = []source.TypedSource[reconcile.Request]{src}
547+
548+
err := ctrl.startEventSources(ctx)
549+
Expect(err).NotTo(HaveOccurred())
550+
Expect(ctrl.startWatches).To(BeNil(), "startWatches should be reset to nil after returning")
551+
})
535552
})
536553

537554
Describe("Processing queue items from a Controller", func() {

0 commit comments

Comments
 (0)