Address comments.

Author: godwinpang

pkg/config/controller.go

@@ -60,8 +60,14 @@ type Controller struct {
 	// Defaults to true, which means the controller will use leader election.
 	NeedLeaderElection *bool
 
-	// NeedWarmup indicates whether the controller needs to use warm up.
-	// Defaults to false, which means the controller will not use warm up.
+	// NeedWarmup specifies whether the controller should start its sources when the manager is not
+	// the leader. This is useful for cases where sources take a long time to start, as it allows
+	// for the controller to warm up its caches even before it is elected as the leader. This
+	// improves leadership failover time, as the caches will be prepopulated before the controller
+	// transitions to be leader.
+	//
+	// When set to true, the controller will start its sources without transitioning to be leader.
+	// Defaults to false.
 	NeedWarmup *bool
 
 	// UsePriorityQueue configures the controllers queue to use the controller-runtime provided

pkg/controller/controller.go

@@ -1,5 +1,4 @@
-/*
-Copyright 2018 The Kubernetes Authors.
+/* Copyright 2018 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -95,9 +94,12 @@ type TypedOptions[request comparable] struct {
 	UsePriorityQueue *bool
 
 	// NeedWarmup specifies whether the controller should start its sources when the manager is not
-	// the leader. When set to true, the controller will not restart its sources when/if it
+	// the leader. This is useful for cases where sources take a long time to start, as it allows
+	// for the controller to warm up its caches even before it is elected as the leader. This
+	// improves leadership failover time, as the caches will be prepopulated before the controller
 	// transitions to be leader.
 	//
+	// When set to true, the controller will start its sources without transitioning to be leader.
 	// Defaults to false.
 	NeedWarmup *bool
 }

pkg/internal/controller/controller.go

@@ -170,10 +170,8 @@ func (c *Controller[request]) NeedLeaderElection() bool {
 // Warmup implements the manager.WarmupRunnable interface.
 func (c *Controller[request]) Warmup(ctx context.Context) error {
 	if c.NeedWarmup == nil || !*c.NeedWarmup {
-		c.didEventSourcesFinishSync.Store(ptr.To(true))
 		return nil
 	}
-
 	return c.startEventSources(ctx)
 }
 
@@ -291,7 +289,6 @@ func (c *Controller[request]) startEventSources(ctx context.Context) error {
 		_, ok := watch.(interface {
 			String() string
 		})
-
 		if !ok {
 			log = log.WithValues("source", fmt.Sprintf("%T", watch))
 		} else {

pkg/internal/controller/controller_test.go

@@ -21,6 +21,7 @@ import (
 	"errors"
 	"fmt"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -1062,32 +1063,38 @@ var _ = Describe("controller", func() {
 		})
 	})
 
-	Describe("Warmup without warmup enabled", func() {
+	Describe("Warmup with warmup disabled", func() {
 		JustBeforeEach(func() {
 			ctrl.NeedWarmup = ptr.To(false)
 		})
 
-		It("should not start sources if warmup is disabled.", func() {
+		It("should not start sources when Warmup is called if warmup is disabled but start it when Start is called.", func() {
 			// Setup controller with sources that complete successfully
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
 			ctrl.CacheSyncTimeout = time.Second
-			isSourceStarted := false
+			var isSourceStarted atomic.Bool
+			isSourceStarted.Store(false)
 			ctrl.startWatches = []source.TypedSource[reconcile.Request]{
 				source.Func(func(ctx context.Context, _ workqueue.TypedRateLimitingInterface[reconcile.Request]) error {
-					isSourceStarted = true
+					isSourceStarted.Store(true)
 					return nil
 				}),
 			}
 
+			By("Calling Warmup when NeedWarmup is false")
 			err := ctrl.Warmup(ctx)
 			Expect(err).NotTo(HaveOccurred())
+			Expect(isSourceStarted.Load()).To(BeFalse())
 
-			result := ctrl.DidFinishWarmup(ctx)
-			Expect(result).To(BeTrue())
-
-			Expect(isSourceStarted).To(BeFalse())
+			By("Calling Start when NeedWarmup is false")
+			// Now call Start
+			go func() {
+				defer GinkgoRecover()
+				Expect(ctrl.Start(ctx)).To(Succeed())
+			}()
+			Eventually(func() bool { return isSourceStarted.Load() }).Should(BeTrue())
 		})
 	})
 })

pkg/manager/manager.go

@@ -316,6 +316,8 @@ type LeaderElectionRunnable interface {
 
 // WarmupRunnable knows if a Runnable should be a warmup runnable. A warmup runnable is a runnable
 // that should be run when the manager is started, but before the leader election is acquired.
+// Note: Implementing this interface is only useful when LeaderElection is enabled, as the behavior
+// without leaderelection is to run LeaderElectionRunnables immediately.
 type WarmupRunnable interface {
 	// Warmup is the implementation of the warmup runnable.
 	Warmup(context.Context) error