Skip to content

Commit be1b1c2

Browse files
committed
Rename ShouldWarmup.
1 parent 73fc8fa commit be1b1c2

File tree

5 files changed

+37
-33
lines changed

5 files changed

+37
-33
lines changed

Diff for: pkg/config/controller.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ type Controller struct {
6060
// Defaults to true, which means the controller will use leader election.
6161
NeedLeaderElection *bool
6262

63-
// NeedWarmUp indicates whether the controller needs to use warm up.
63+
// EnableWarmup indicates whether the controller needs to use warm up.
6464
// Defaults to false, which means the controller will not use warm up.
65-
NeedWarmUp *bool
65+
NeedWarmup *bool
6666

6767
// UsePriorityQueue configures the controllers queue to use the controller-runtime provided
6868
// priority queue.

Diff for: pkg/controller/controller.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,11 @@ type TypedOptions[request comparable] struct {
9494
// Note: This flag is disabled by default until a future version. It's currently in beta.
9595
UsePriorityQueue *bool
9696

97-
// ShouldWarmupWithoutLeadership specifies whether the controller should start its sources
97+
// NeedWarmup specifies whether the controller should start its sources
9898
// when the manager is not the leader.
9999
// Defaults to false, which means that the controller will wait for leader election to start
100100
// before starting sources.
101-
ShouldWarmupWithoutLeadership *bool
101+
NeedWarmup *bool
102102
}
103103

104104
// DefaultFromConfig defaults the config from a config.Controller
@@ -130,6 +130,10 @@ func (options *TypedOptions[request]) DefaultFromConfig(config config.Controller
130130
if options.NeedLeaderElection == nil {
131131
options.NeedLeaderElection = config.NeedLeaderElection
132132
}
133+
134+
if options.NeedWarmup == nil {
135+
options.NeedWarmup = config.NeedWarmup
136+
}
133137
}
134138

135139
// 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
259263
LogConstructor: options.LogConstructor,
260264
RecoverPanic: options.RecoverPanic,
261265
LeaderElected: options.NeedLeaderElection,
262-
ShouldWarmupWithoutLeadership: options.ShouldWarmupWithoutLeadership,
266+
NeedWarmup: options.NeedWarmup,
263267
}, nil
264268
}
265269

Diff for: pkg/controller/controller_test.go

+13-16
Original file line numberDiff line numberDiff line change
@@ -482,26 +482,26 @@ var _ = Describe("controller.Controller", func() {
482482
// Test with ShouldWarmupWithoutLeadership set to true
483483
ctrlWithWarmup, err := controller.New("warmup-enabled-ctrl", m, controller.Options{
484484
Reconciler: reconcile.Func(nil),
485-
ShouldWarmupWithoutLeadership: ptr.To(true),
485+
NeedWarmup: ptr.To(true),
486486
})
487487
Expect(err).NotTo(HaveOccurred())
488488

489489
internalCtrlWithWarmup, ok := ctrlWithWarmup.(*internalcontroller.Controller[reconcile.Request])
490490
Expect(ok).To(BeTrue())
491-
Expect(internalCtrlWithWarmup.ShouldWarmupWithoutLeadership).NotTo(BeNil())
492-
Expect(*internalCtrlWithWarmup.ShouldWarmupWithoutLeadership).To(BeTrue())
491+
Expect(internalCtrlWithWarmup.NeedWarmup).NotTo(BeNil())
492+
Expect(*internalCtrlWithWarmup.NeedWarmup).To(BeTrue())
493493

494494
// Test with ShouldWarmupWithoutLeadership set to false
495495
ctrlWithoutWarmup, err := controller.New("warmup-disabled-ctrl", m, controller.Options{
496496
Reconciler: reconcile.Func(nil),
497-
ShouldWarmupWithoutLeadership: ptr.To(false),
497+
NeedWarmup: ptr.To(false),
498498
})
499499
Expect(err).NotTo(HaveOccurred())
500500

501501
internalCtrlWithoutWarmup, ok := ctrlWithoutWarmup.(*internalcontroller.Controller[reconcile.Request])
502502
Expect(ok).To(BeTrue())
503-
Expect(internalCtrlWithoutWarmup.ShouldWarmupWithoutLeadership).NotTo(BeNil())
504-
Expect(*internalCtrlWithoutWarmup.ShouldWarmupWithoutLeadership).To(BeFalse())
503+
Expect(internalCtrlWithoutWarmup.NeedWarmup).NotTo(BeNil())
504+
Expect(*internalCtrlWithoutWarmup.NeedWarmup).To(BeFalse())
505505

506506
// Test with ShouldWarmupWithoutLeadership not set (should default to nil)
507507
ctrlWithDefaultWarmup, err := controller.New("warmup-default-ctrl", m, controller.Options{
@@ -511,14 +511,14 @@ var _ = Describe("controller.Controller", func() {
511511

512512
internalCtrlWithDefaultWarmup, ok := ctrlWithDefaultWarmup.(*internalcontroller.Controller[reconcile.Request])
513513
Expect(ok).To(BeTrue())
514-
Expect(internalCtrlWithDefaultWarmup.ShouldWarmupWithoutLeadership).To(BeNil())
514+
Expect(internalCtrlWithDefaultWarmup.NeedWarmup).To(BeNil())
515515
})
516516

517517
It("should inherit ShouldWarmupWithoutLeadership from manager config", func() {
518518
// Test with manager default setting ShouldWarmupWithoutLeadership to true
519519
managerWithWarmup, err := manager.New(cfg, manager.Options{
520520
Controller: config.Controller{
521-
NeedWarmUp: ptr.To(true),
521+
NeedWarmup: ptr.To(true),
522522
},
523523
})
524524
Expect(err).NotTo(HaveOccurred())
@@ -529,23 +529,20 @@ var _ = Describe("controller.Controller", func() {
529529

530530
internalCtrlInheritingWarmup, ok := ctrlInheritingWarmup.(*internalcontroller.Controller[reconcile.Request])
531531
Expect(ok).To(BeTrue())
532-
// Note: This test will fail until the DefaultFromConfig method is updated to set
533-
// ShouldWarmupWithoutLeadership from config.Controller.NeedWarmUp
534-
// This test demonstrates that the feature needs to be completed
535-
Expect(internalCtrlInheritingWarmup.ShouldWarmupWithoutLeadership).NotTo(BeNil())
536-
Expect(*internalCtrlInheritingWarmup.ShouldWarmupWithoutLeadership).To(BeTrue())
532+
Expect(internalCtrlInheritingWarmup.NeedWarmup).NotTo(BeNil())
533+
Expect(*internalCtrlInheritingWarmup.NeedWarmup).To(BeTrue())
537534

538535
// Test that explicit controller setting overrides manager setting
539536
ctrlOverridingWarmup, err := controller.New("override-warmup-disabled", managerWithWarmup, controller.Options{
540537
Reconciler: reconcile.Func(nil),
541-
ShouldWarmupWithoutLeadership: ptr.To(false),
538+
NeedWarmup: ptr.To(false),
542539
})
543540
Expect(err).NotTo(HaveOccurred())
544541

545542
internalCtrlOverridingWarmup, ok := ctrlOverridingWarmup.(*internalcontroller.Controller[reconcile.Request])
546543
Expect(ok).To(BeTrue())
547-
Expect(internalCtrlOverridingWarmup.ShouldWarmupWithoutLeadership).NotTo(BeNil())
548-
Expect(*internalCtrlOverridingWarmup.ShouldWarmupWithoutLeadership).To(BeFalse())
544+
Expect(internalCtrlOverridingWarmup.NeedWarmup).NotTo(BeNil())
545+
Expect(*internalCtrlOverridingWarmup.NeedWarmup).To(BeFalse())
549546
})
550547
})
551548
})

Diff for: pkg/internal/controller/controller.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,11 @@ type Controller[request comparable] struct {
9999
// LeaderElected indicates whether the controller is leader elected or always running.
100100
LeaderElected *bool
101101

102-
// ShouldWarmupWithoutLeadership specifies whether the controller should start its sources
102+
// NeedWarmup specifies whether the controller should start its sources
103103
// when the manager is not the leader.
104104
// Defaults to false, which means that the controller will wait for leader election to start
105105
// before starting sources.
106-
ShouldWarmupWithoutLeadership *bool
106+
NeedWarmup *bool
107107
}
108108

109109
// Reconcile implements reconcile.Reconciler.
@@ -155,7 +155,7 @@ func (c *Controller[request]) NeedLeaderElection() bool {
155155

156156
// Warmup implements the manager.WarmupRunnable interface.
157157
func (c *Controller[request]) Warmup(ctx context.Context) error {
158-
if c.ShouldWarmupWithoutLeadership == nil || !*c.ShouldWarmupWithoutLeadership {
158+
if c.NeedWarmup == nil || !*c.NeedWarmup {
159159
return nil
160160
}
161161

Diff for: pkg/internal/controller/controller_test.go

+12-9
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ var _ = Describe("controller", func() {
403403
return expectedErr
404404
})
405405

406-
// // Set a sufficiently long timeout to avoid timeouts interfering with the error being returned
406+
// Set a sufficiently long timeout to avoid timeouts interfering with the error being returned
407407
ctrl.CacheSyncTimeout = 5 * time.Second
408408
ctrl.startWatches = []source.TypedSource[reconcile.Request]{src}
409409
err := ctrl.startEventSources(ctx)
@@ -1016,7 +1016,7 @@ var _ = Describe("controller", func() {
10161016
})
10171017

10181018
Describe("Warmup", func() {
1019-
It("should start event sources when ShouldWarmupWithoutLeadership is true", func() {
1019+
It("should start event sources when NeedWarmup is true", func() {
10201020
// Setup
10211021
ctx, cancel := context.WithCancel(context.Background())
10221022
defer cancel()
@@ -1028,8 +1028,9 @@ var _ = Describe("controller", func() {
10281028
return nil
10291029
})
10301030

1031+
ctrl.CacheSyncTimeout = time.Second
10311032
ctrl.startWatches = []source.TypedSource[reconcile.Request]{mockSource}
1032-
ctrl.ShouldWarmupWithoutLeadership = ptr.To(true)
1033+
ctrl.NeedWarmup = ptr.To(true)
10331034

10341035
// Act
10351036
err := ctrl.Warmup(ctx)
@@ -1040,7 +1041,7 @@ var _ = Describe("controller", func() {
10401041
Expect(ctrl.didStartEventSources.Load()).To(BeTrue(), "didStartEventSources flag should be set")
10411042
})
10421043

1043-
It("should not start event sources when ShouldWarmupWithoutLeadership is false", func() {
1044+
It("should not start event sources when NeedWarmup is false", func() {
10441045
// Setup
10451046
ctx, cancel := context.WithCancel(context.Background())
10461047
defer cancel()
@@ -1053,7 +1054,7 @@ var _ = Describe("controller", func() {
10531054
})
10541055

10551056
ctrl.startWatches = []source.TypedSource[reconcile.Request]{mockSource}
1056-
ctrl.ShouldWarmupWithoutLeadership = ptr.To(false)
1057+
ctrl.NeedWarmup = ptr.To(false)
10571058

10581059
// Act
10591060
err := ctrl.Warmup(ctx)
@@ -1064,7 +1065,7 @@ var _ = Describe("controller", func() {
10641065
Expect(ctrl.didStartEventSources.Load()).To(BeFalse(), "didStartEventSources flag should not be set")
10651066
})
10661067

1067-
It("should not start event sources when ShouldWarmupWithoutLeadership is nil", func() {
1068+
It("should not start event sources when NeedWarmup is nil", func() {
10681069
// Setup
10691070
ctx, cancel := context.WithCancel(context.Background())
10701071
defer cancel()
@@ -1077,7 +1078,7 @@ var _ = Describe("controller", func() {
10771078
})
10781079

10791080
ctrl.startWatches = []source.TypedSource[reconcile.Request]{mockSource}
1080-
ctrl.ShouldWarmupWithoutLeadership = nil
1081+
ctrl.NeedWarmup = nil
10811082

10821083
// Act
10831084
err := ctrl.Warmup(ctx)
@@ -1100,8 +1101,9 @@ var _ = Describe("controller", func() {
11001101
return nil
11011102
})
11021103

1104+
ctrl.CacheSyncTimeout = time.Second
11031105
ctrl.startWatches = []source.TypedSource[reconcile.Request]{mockSource}
1104-
ctrl.ShouldWarmupWithoutLeadership = ptr.To(true)
1106+
ctrl.NeedWarmup = ptr.To(true)
11051107

11061108
// Act
11071109
err1 := ctrl.Warmup(ctx)
@@ -1125,8 +1127,9 @@ var _ = Describe("controller", func() {
11251127
return expectedErr
11261128
})
11271129

1130+
ctrl.CacheSyncTimeout = time.Second
11281131
ctrl.startWatches = []source.TypedSource[reconcile.Request]{mockSource}
1129-
ctrl.ShouldWarmupWithoutLeadership = ptr.To(true)
1132+
ctrl.NeedWarmup = ptr.To(true)
11301133

11311134
// Act
11321135
err := ctrl.Warmup(ctx)

0 commit comments

Comments
 (0)