Rename ShouldWarmup.

Author: godwinpang

pkg/config/controller.go

@@ -60,9 +60,9 @@ 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.
+// EnableWarmup indicates whether the controller needs to use warm up.
 // Defaults to false, which means the controller will not use warm up.
-NeedWarmUp *bool
+NeedWarmup *bool
 
 // UsePriorityQueue configures the controllers queue to use the controller-runtime provided
 // priority queue.

pkg/controller/controller.go

@@ -94,11 +94,11 @@ type TypedOptions[request comparable] struct {
 // Note: This flag is disabled by default until a future version. It's currently in beta.
 UsePriorityQueue *bool
 
-// ShouldWarmupWithoutLeadership specifies whether the controller should start its sources
+// NeedWarmup specifies whether the controller should start its sources
 // when the manager is not the leader.
 // Defaults to false, which means that the controller will wait for leader election to start
 // before starting sources.
-ShouldWarmupWithoutLeadership *bool
+NeedWarmup *bool
 }
 
 // DefaultFromConfig defaults the config from a config.Controller
@@ -130,6 +130,10 @@ func (options *TypedOptions[request]) DefaultFromConfig(config config.Controller
 if options.NeedLeaderElection == nil {
 options.NeedLeaderElection = config.NeedLeaderElection
 }
+
+ if options.NeedWarmup == nil {
+ options.NeedWarmup = config.NeedWarmup
+ }
 }
 
 // Controller implements an API. A Controller manages a work queue fed reconcile.Requests
@@ -259,7 +263,7 @@ func NewTypedUnmanaged[request comparable](name string, options TypedOptions[req
 LogConstructor: options.LogConstructor,
 RecoverPanic: options.RecoverPanic,
 LeaderElected: options.NeedLeaderElection,
-ShouldWarmupWithoutLeadership: options.ShouldWarmupWithoutLeadership,
+NeedWarmup: options.NeedWarmup,
 }, nil
 }
 

pkg/controller/controller_test.go

@@ -482,26 +482,26 @@ var _ = Describe("controller.Controller", func() {
 // Test with ShouldWarmupWithoutLeadership set to true
 ctrlWithWarmup, err := controller.New("warmup-enabled-ctrl", m, controller.Options{
 Reconciler: reconcile.Func(nil),
-ShouldWarmupWithoutLeadership: ptr.To(true),
+NeedWarmup: ptr.To(true),
 })
 Expect(err).NotTo(HaveOccurred())
 
 internalCtrlWithWarmup, ok := ctrlWithWarmup.(*internalcontroller.Controller[reconcile.Request])
 Expect(ok).To(BeTrue())
-Expect(internalCtrlWithWarmup.ShouldWarmupWithoutLeadership).NotTo(BeNil())
-Expect(*internalCtrlWithWarmup.ShouldWarmupWithoutLeadership).To(BeTrue())
+Expect(internalCtrlWithWarmup.NeedWarmup).NotTo(BeNil())
+Expect(*internalCtrlWithWarmup.NeedWarmup).To(BeTrue())
 
 // Test with ShouldWarmupWithoutLeadership set to false
 ctrlWithoutWarmup, err := controller.New("warmup-disabled-ctrl", m, controller.Options{
 Reconciler: reconcile.Func(nil),
-ShouldWarmupWithoutLeadership: ptr.To(false),
+NeedWarmup: ptr.To(false),
 })
 Expect(err).NotTo(HaveOccurred())
 
 internalCtrlWithoutWarmup, ok := ctrlWithoutWarmup.(*internalcontroller.Controller[reconcile.Request])
 Expect(ok).To(BeTrue())
-Expect(internalCtrlWithoutWarmup.ShouldWarmupWithoutLeadership).NotTo(BeNil())
-Expect(*internalCtrlWithoutWarmup.ShouldWarmupWithoutLeadership).To(BeFalse())
+Expect(internalCtrlWithoutWarmup.NeedWarmup).NotTo(BeNil())
+Expect(*internalCtrlWithoutWarmup.NeedWarmup).To(BeFalse())
 
 // Test with ShouldWarmupWithoutLeadership not set (should default to nil)
 ctrlWithDefaultWarmup, err := controller.New("warmup-default-ctrl", m, controller.Options{
@@ -511,14 +511,14 @@ var _ = Describe("controller.Controller", func() {
 
 internalCtrlWithDefaultWarmup, ok := ctrlWithDefaultWarmup.(*internalcontroller.Controller[reconcile.Request])
 Expect(ok).To(BeTrue())
-Expect(internalCtrlWithDefaultWarmup.ShouldWarmupWithoutLeadership).To(BeNil())
+Expect(internalCtrlWithDefaultWarmup.NeedWarmup).To(BeNil())
 })
 
 It("should inherit ShouldWarmupWithoutLeadership from manager config", func() {
 // Test with manager default setting ShouldWarmupWithoutLeadership to true
 managerWithWarmup, err := manager.New(cfg, manager.Options{
 Controller: config.Controller{
-NeedWarmUp: ptr.To(true),
+NeedWarmup: ptr.To(true),
 },
 })
 Expect(err).NotTo(HaveOccurred())
@@ -529,23 +529,20 @@ var _ = Describe("controller.Controller", func() {
 
 internalCtrlInheritingWarmup, ok := ctrlInheritingWarmup.(*internalcontroller.Controller[reconcile.Request])
 Expect(ok).To(BeTrue())
-// Note: This test will fail until the DefaultFromConfig method is updated to set
-// ShouldWarmupWithoutLeadership from config.Controller.NeedWarmUp
-// This test demonstrates that the feature needs to be completed
-Expect(internalCtrlInheritingWarmup.ShouldWarmupWithoutLeadership).NotTo(BeNil())
-Expect(*internalCtrlInheritingWarmup.ShouldWarmupWithoutLeadership).To(BeTrue())
+Expect(internalCtrlInheritingWarmup.NeedWarmup).NotTo(BeNil())
+Expect(*internalCtrlInheritingWarmup.NeedWarmup).To(BeTrue())
 
 // Test that explicit controller setting overrides manager setting
 ctrlOverridingWarmup, err := controller.New("override-warmup-disabled", managerWithWarmup, controller.Options{
 Reconciler: reconcile.Func(nil),
-ShouldWarmupWithoutLeadership: ptr.To(false),
+NeedWarmup: ptr.To(false),
 })
 Expect(err).NotTo(HaveOccurred())
 
 internalCtrlOverridingWarmup, ok := ctrlOverridingWarmup.(*internalcontroller.Controller[reconcile.Request])
 Expect(ok).To(BeTrue())
-Expect(internalCtrlOverridingWarmup.ShouldWarmupWithoutLeadership).NotTo(BeNil())
-Expect(*internalCtrlOverridingWarmup.ShouldWarmupWithoutLeadership).To(BeFalse())
+Expect(internalCtrlOverridingWarmup.NeedWarmup).NotTo(BeNil())
+Expect(*internalCtrlOverridingWarmup.NeedWarmup).To(BeFalse())
 })
 })
 })

pkg/internal/controller/controller.go

@@ -99,11 +99,11 @@ type Controller[request comparable] struct {
 // LeaderElected indicates whether the controller is leader elected or always running.
 LeaderElected *bool
 
-// ShouldWarmupWithoutLeadership specifies whether the controller should start its sources
+// NeedWarmup specifies whether the controller should start its sources
 // when the manager is not the leader.
 // Defaults to false, which means that the controller will wait for leader election to start
 // before starting sources.
-ShouldWarmupWithoutLeadership *bool
+NeedWarmup *bool
 }
 
 // Reconcile implements reconcile.Reconciler.
@@ -155,7 +155,7 @@ func (c *Controller[request]) NeedLeaderElection() bool {
 
 // Warmup implements the manager.WarmupRunnable interface.
 func (c *Controller[request]) Warmup(ctx context.Context) error {
-if c.ShouldWarmupWithoutLeadership == nil || !*c.ShouldWarmupWithoutLeadership {
+if c.NeedWarmup == nil || !*c.NeedWarmup {
 return nil
 }
 

pkg/internal/controller/controller_test.go

@@ -403,7 +403,7 @@ var _ = Describe("controller", func() {
 return expectedErr
 })
 
-// // Set a sufficiently long timeout to avoid timeouts interfering with the error being returned
+// Set a sufficiently long timeout to avoid timeouts interfering with the error being returned
 ctrl.CacheSyncTimeout = 5 * time.Second
 ctrl.startWatches = []source.TypedSource[reconcile.Request]{src}
 err := ctrl.startEventSources(ctx)
@@ -1016,7 +1016,7 @@ var _ = Describe("controller", func() {
 })
 
 Describe("Warmup", func() {
-It("should start event sources when ShouldWarmupWithoutLeadership is true", func() {
+It("should start event sources when NeedWarmup is true", func() {
 // Setup
 ctx, cancel := context.WithCancel(context.Background())
 defer cancel()
@@ -1028,8 +1028,9 @@ var _ = Describe("controller", func() {
 return nil
 })
 
+ctrl.CacheSyncTimeout = time.Second
 ctrl.startWatches = []source.TypedSource[reconcile.Request]{mockSource}
-ctrl.ShouldWarmupWithoutLeadership = ptr.To(true)
+ctrl.NeedWarmup = ptr.To(true)
 
 // Act
 err := ctrl.Warmup(ctx)
@@ -1040,7 +1041,7 @@ var _ = Describe("controller", func() {
 Expect(ctrl.didStartEventSources.Load()).To(BeTrue(), "didStartEventSources flag should be set")
 })
 
-It("should not start event sources when ShouldWarmupWithoutLeadership is false", func() {
+It("should not start event sources when NeedWarmup is false", func() {
 // Setup
 ctx, cancel := context.WithCancel(context.Background())
 defer cancel()
@@ -1053,7 +1054,7 @@ var _ = Describe("controller", func() {
 })
 
 ctrl.startWatches = []source.TypedSource[reconcile.Request]{mockSource}
-ctrl.ShouldWarmupWithoutLeadership = ptr.To(false)
+ctrl.NeedWarmup = ptr.To(false)
 
 // Act
 err := ctrl.Warmup(ctx)
@@ -1064,7 +1065,7 @@ var _ = Describe("controller", func() {
 Expect(ctrl.didStartEventSources.Load()).To(BeFalse(), "didStartEventSources flag should not be set")
 })
 
-It("should not start event sources when ShouldWarmupWithoutLeadership is nil", func() {
+It("should not start event sources when NeedWarmup is nil", func() {
 // Setup
 ctx, cancel := context.WithCancel(context.Background())
 defer cancel()
@@ -1077,7 +1078,7 @@ var _ = Describe("controller", func() {
 })
 
 ctrl.startWatches = []source.TypedSource[reconcile.Request]{mockSource}
-ctrl.ShouldWarmupWithoutLeadership = nil
+ctrl.NeedWarmup = nil
 
 // Act
 err := ctrl.Warmup(ctx)
@@ -1100,8 +1101,9 @@ var _ = Describe("controller", func() {
 return nil
 })
 
+ctrl.CacheSyncTimeout = time.Second
 ctrl.startWatches = []source.TypedSource[reconcile.Request]{mockSource}
-ctrl.ShouldWarmupWithoutLeadership = ptr.To(true)
+ctrl.NeedWarmup = ptr.To(true)
 
 // Act
 err1 := ctrl.Warmup(ctx)
@@ -1125,8 +1127,9 @@ var _ = Describe("controller", func() {
 return expectedErr
 })
 
+ctrl.CacheSyncTimeout = time.Second
 ctrl.startWatches = []source.TypedSource[reconcile.Request]{mockSource}
-ctrl.ShouldWarmupWithoutLeadership = ptr.To(true)
+ctrl.NeedWarmup = ptr.To(true)
 
 // Act
 err := ctrl.Warmup(ctx)