Skip to content

Commit 5d9e763

Browse files
nexus49nexus49
committed
refactor: simplify lifecycle manager configuration and condition management checks
1 parent fdb9daf commit 5d9e763

File tree

5 files changed

+52
-57
lines changed

5 files changed

+52
-57
lines changed

‎controller/lifecycle/controllerruntime/lifecycle.go‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,8 @@ func NewLifecycleManager(log *logger.Logger, operatorName string, controllerName
3737
client: client,
3838
subroutines: subroutines,
3939
config: lifecycle.Config{
40-
OperatorName: operatorName,
41-
ControllerName: controllerName,
42-
SpreadReconciles: false,
40+
OperatorName: operatorName,
41+
ControllerName: controllerName,
4342
},
4443
}
4544
}
@@ -57,14 +56,14 @@ func (l *LifecycleManager) PrepareContextFunc() lifecycle.PrepareContextFunc {
5756
return l.prepareContextFunc
5857
}
5958
func (l *LifecycleManager) ConditionsManager() *conditions.ConditionManager {
60-
if !l.config.ManageConditions {
59+
if l.conditionsManager == nil {
6160
l.log.Fatal().Msg("conditions management not enabled")
6261
}
6362
return l.conditionsManager
6463
}
6564

6665
func (l *LifecycleManager) Spreader() *spread.Spreader {
67-
if !l.config.SpreadReconciles {
66+
if l.spreader == nil {
6867
l.log.Fatal().Msg("spread reconciles not enabled")
6968
}
7069
return l.spreader
@@ -75,13 +74,13 @@ func (l *LifecycleManager) Reconcile(ctx context.Context, req ctrl.Request, inst
7574
}
7675

7776
func (l *LifecycleManager) validateInterfaces(instance runtimeobject.RuntimeObject, log *logger.Logger) error {
78-
if l.Config().SpreadReconciles {
77+
if l.Spreader() != nil {
7978
_, err := l.Spreader().ToRuntimeObjectSpreadReconcileStatusInterface(instance, log)
8079
if err != nil {
8180
return err
8281
}
8382
}
84-
if l.Config().ManageConditions {
83+
if l.ConditionsManager() != nil {
8584
_, err := l.ConditionsManager().ToRuntimeObjectConditionsInterface(instance, log)
8685
if err != nil {
8786
return err
@@ -95,7 +94,7 @@ func (l *LifecycleManager) SetupWithManagerBuilder(mgr ctrl.Manager, maxReconcil
9594
return nil, err
9695
}
9796

98-
if (l.Config().ManageConditions || l.Config().SpreadReconciles) && l.Config().ReadOnly {
97+
if (l.ConditionsManager() != nil || l.Spreader() != nil) && l.Config().ReadOnly {
9998
return nil, fmt.Errorf("cannot use conditions or spread reconciles in read-only mode")
10099
}
101100

@@ -133,13 +132,11 @@ func (l *LifecycleManager) WithReadOnly() *LifecycleManager {
133132

134133
// WithSpreadingReconciles sets the LifecycleManager to spread out the reconciles
135134
func (l *LifecycleManager) WithSpreadingReconciles() *LifecycleManager {
136-
l.config.SpreadReconciles = true
137135
l.spreader = spread.NewSpreader()
138136
return l
139137
}
140138

141139
func (l *LifecycleManager) WithConditionManagement() *LifecycleManager {
142-
l.config.ManageConditions = true
143140
l.conditionsManager = conditions.NewConditionManager()
144141
return l
145142
}

‎controller/lifecycle/controllerruntime/lifecycle_test.go‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ func TestLifecycle(t *testing.T) {
304304
t.Run("Lifecycle with spread reconciles", func(t *testing.T) {
305305
// Arrange
306306
instance := &pmtesting.ImplementingSpreadReconciles{
307-
testSupport.TestApiObject{
307+
TestApiObject: testSupport.TestApiObject{
308308
ObjectMeta: metav1.ObjectMeta{
309309
Name: name,
310310
Namespace: namespace,
@@ -336,7 +336,7 @@ func TestLifecycle(t *testing.T) {
336336
t.Run("Lifecycle with spread reconciles on deleted object", func(t *testing.T) {
337337
// Arrange
338338
instance := &pmtesting.ImplementingSpreadReconciles{
339-
testSupport.TestApiObject{
339+
TestApiObject: testSupport.TestApiObject{
340340
ObjectMeta: metav1.ObjectMeta{
341341
Name: name,
342342
Namespace: namespace,
@@ -372,7 +372,7 @@ func TestLifecycle(t *testing.T) {
372372
// Arrange
373373
nextReconcileTime := metav1.NewTime(time.Now().Add(1 * time.Hour))
374374
instance := &pmtesting.ImplementingSpreadReconciles{
375-
testSupport.TestApiObject{
375+
TestApiObject: testSupport.TestApiObject{
376376
ObjectMeta: metav1.ObjectMeta{
377377
Name: name,
378378
Namespace: namespace,
@@ -402,7 +402,7 @@ func TestLifecycle(t *testing.T) {
402402
t.Run("Lifecycle with spread reconciles and processing fails (no-retry)", func(t *testing.T) {
403403
// Arrange
404404
instance := &pmtesting.ImplementingSpreadReconciles{
405-
testSupport.TestApiObject{
405+
TestApiObject: testSupport.TestApiObject{
406406
ObjectMeta: metav1.ObjectMeta{
407407
Name: name,
408408
Namespace: namespace,
@@ -430,7 +430,7 @@ func TestLifecycle(t *testing.T) {
430430
t.Run("Lifecycle with spread reconciles and processing fails (retry)", func(t *testing.T) {
431431
// Arrange
432432
instance := &pmtesting.ImplementingSpreadReconciles{
433-
testSupport.TestApiObject{
433+
TestApiObject: testSupport.TestApiObject{
434434
ObjectMeta: metav1.ObjectMeta{
435435
Name: name,
436436
Namespace: namespace,
@@ -458,7 +458,7 @@ func TestLifecycle(t *testing.T) {
458458
t.Run("Lifecycle with spread reconciles and processing needs requeue", func(t *testing.T) {
459459
// Arrange
460460
instance := &pmtesting.ImplementingSpreadReconciles{
461-
testSupport.TestApiObject{
461+
TestApiObject: testSupport.TestApiObject{
462462
ObjectMeta: metav1.ObjectMeta{
463463
Name: name,
464464
Namespace: namespace,
@@ -486,7 +486,7 @@ func TestLifecycle(t *testing.T) {
486486
t.Run("Lifecycle with spread reconciles and processing needs requeueAfter", func(t *testing.T) {
487487
// Arrange
488488
instance := &pmtesting.ImplementingSpreadReconciles{
489-
testSupport.TestApiObject{
489+
TestApiObject: testSupport.TestApiObject{
490490
ObjectMeta: metav1.ObjectMeta{
491491
Name: name,
492492
Namespace: namespace,
@@ -589,7 +589,7 @@ func TestLifecycle(t *testing.T) {
589589
t.Run("Lifecycle with spread reconciles and refresh label", func(t *testing.T) {
590590
// Arrange
591591
instance := &pmtesting.ImplementingSpreadReconciles{
592-
testSupport.TestApiObject{
592+
TestApiObject: testSupport.TestApiObject{
593593
ObjectMeta: metav1.ObjectMeta{
594594
Name: name,
595595
Namespace: namespace,
@@ -643,7 +643,7 @@ func TestLifecycle(t *testing.T) {
643643
t.Run("Lifecycle with manage conditions reconciles w/o subroutines", func(t *testing.T) {
644644
// Arrange
645645
instance := &pmtesting.ImplementConditions{
646-
testSupport.TestApiObject{
646+
TestApiObject: testSupport.TestApiObject{
647647
ObjectMeta: metav1.ObjectMeta{
648648
Name: name,
649649
Namespace: namespace,
@@ -671,7 +671,7 @@ func TestLifecycle(t *testing.T) {
671671
t.Run("Lifecycle with manage conditions reconciles with subroutine", func(t *testing.T) {
672672
// Arrange
673673
instance := &pmtesting.ImplementConditions{
674-
testSupport.TestApiObject{
674+
TestApiObject: testSupport.TestApiObject{
675675
ObjectMeta: metav1.ObjectMeta{
676676
Name: name,
677677
Namespace: namespace,
@@ -704,7 +704,7 @@ func TestLifecycle(t *testing.T) {
704704
t.Run("Lifecycle with manage conditions reconciles with subroutine that adds a condition", func(t *testing.T) {
705705
// Arrange
706706
instance := &pmtesting.ImplementConditions{
707-
testSupport.TestApiObject{
707+
TestApiObject: testSupport.TestApiObject{
708708
ObjectMeta: metav1.ObjectMeta{
709709
Name: name,
710710
Namespace: namespace,
@@ -739,7 +739,7 @@ func TestLifecycle(t *testing.T) {
739739
t.Run("Lifecycle with manage conditions reconciles with subroutine that adds a condition", func(t *testing.T) {
740740
// Arrange
741741
instance := &pmtesting.ImplementConditions{
742-
testSupport.TestApiObject{
742+
TestApiObject: testSupport.TestApiObject{
743743
ObjectMeta: metav1.ObjectMeta{
744744
Name: name,
745745
Namespace: namespace,
@@ -774,7 +774,7 @@ func TestLifecycle(t *testing.T) {
774774
t.Run("Lifecycle with manage conditions reconciles with subroutine that adds a condition with preexisting conditions (update)", func(t *testing.T) {
775775
// Arrange
776776
instance := &pmtesting.ImplementConditions{
777-
testSupport.TestApiObject{
777+
TestApiObject: testSupport.TestApiObject{
778778
ObjectMeta: metav1.ObjectMeta{
779779
Name: name,
780780
Namespace: namespace,
@@ -818,7 +818,7 @@ func TestLifecycle(t *testing.T) {
818818
t.Run("Lifecycle with manage conditions reconciles with subroutine that adds a condition with preexisting conditions", func(t *testing.T) {
819819
// Arrange
820820
instance := &pmtesting.ImplementConditions{
821-
testSupport.TestApiObject{
821+
TestApiObject: testSupport.TestApiObject{
822822
ObjectMeta: metav1.ObjectMeta{
823823
Name: name,
824824
Namespace: namespace,
@@ -862,7 +862,7 @@ func TestLifecycle(t *testing.T) {
862862
t.Run("Lifecycle w/o manage conditions reconciles with subroutine that adds a condition", func(t *testing.T) {
863863
// Arrange
864864
instance := &pmtesting.ImplementConditions{
865-
testSupport.TestApiObject{
865+
TestApiObject: testSupport.TestApiObject{
866866
ObjectMeta: metav1.ObjectMeta{
867867
Name: name,
868868
Namespace: namespace,
@@ -890,7 +890,7 @@ func TestLifecycle(t *testing.T) {
890890
t.Run("Lifecycle with manage conditions reconciles with subroutine failing Status update", func(t *testing.T) {
891891
// Arrange
892892
instance := &pmtesting.ImplementConditions{
893-
testSupport.TestApiObject{
893+
TestApiObject: testSupport.TestApiObject{
894894
ObjectMeta: metav1.ObjectMeta{
895895
Name: name,
896896
Namespace: namespace,
@@ -924,7 +924,7 @@ func TestLifecycle(t *testing.T) {
924924
t.Run("Lifecycle with manage conditions finalizes with multiple subroutines partially succeeding", func(t *testing.T) {
925925
// Arrange
926926
instance := &pmtesting.ImplementConditions{
927-
testSupport.TestApiObject{
927+
TestApiObject: testSupport.TestApiObject{
928928
ObjectMeta: metav1.ObjectMeta{
929929
Name: name,
930930
Namespace: namespace,
@@ -962,7 +962,7 @@ func TestLifecycle(t *testing.T) {
962962
t.Run("Lifecycle with manage conditions reconciles with ReqeueAfter subroutine", func(t *testing.T) {
963963
// Arrange
964964
instance := &pmtesting.ImplementConditions{
965-
testSupport.TestApiObject{
965+
TestApiObject: testSupport.TestApiObject{
966966
ObjectMeta: metav1.ObjectMeta{
967967
Name: name,
968968
Namespace: namespace,
@@ -994,7 +994,7 @@ func TestLifecycle(t *testing.T) {
994994
t.Run("Lifecycle with manage conditions reconciles with Error subroutine (no-retry)", func(t *testing.T) {
995995
// Arrange
996996
instance := &pmtesting.ImplementConditions{
997-
testSupport.TestApiObject{
997+
TestApiObject: testSupport.TestApiObject{
998998
ObjectMeta: metav1.ObjectMeta{
999999
Name: name,
10001000
Namespace: namespace,
@@ -1026,7 +1026,7 @@ func TestLifecycle(t *testing.T) {
10261026
t.Run("Lifecycle with manage conditions reconciles with Error subroutine (retry)", func(t *testing.T) {
10271027
// Arrange
10281028
instance := &pmtesting.ImplementConditions{
1029-
testSupport.TestApiObject{
1029+
TestApiObject: testSupport.TestApiObject{
10301030
ObjectMeta: metav1.ObjectMeta{
10311031
Name: name,
10321032
Namespace: namespace,
@@ -1088,7 +1088,7 @@ func TestLifecycle(t *testing.T) {
10881088
t.Run("Lifecycle with manage conditions failing finalize", func(t *testing.T) {
10891089
// Arrange
10901090
instance := &pmtesting.ImplementConditions{
1091-
testSupport.TestApiObject{
1091+
TestApiObject: testSupport.TestApiObject{
10921092
ObjectMeta: metav1.ObjectMeta{
10931093
Name: name,
10941094
Namespace: namespace,
@@ -1118,7 +1118,7 @@ func TestLifecycle(t *testing.T) {
11181118
t.Run("Lifecycle with spread reconciles and manage conditions and processing fails (retry)", func(t *testing.T) {
11191119
// Arrange
11201120
instance := &pmtesting.ImplementConditionsAndSpreadReconciles{
1121-
testSupport.TestApiObject{
1121+
TestApiObject: testSupport.TestApiObject{
11221122
ObjectMeta: metav1.ObjectMeta{
11231123
Name: name,
11241124
Namespace: namespace,
@@ -1152,7 +1152,7 @@ func TestLifecycle(t *testing.T) {
11521152
t.Run("Lifecycle with spread reconciles and manage conditions and processing fails (no-retry)", func(t *testing.T) {
11531153
// Arrange
11541154
instance := &pmtesting.ImplementConditionsAndSpreadReconciles{
1155-
testSupport.TestApiObject{
1155+
TestApiObject: testSupport.TestApiObject{
11561156
ObjectMeta: metav1.ObjectMeta{
11571157
Name: name,
11581158
Namespace: namespace,
@@ -1357,7 +1357,7 @@ func TestLifecycleManager_WithConditionManagement(t *testing.T) {
13571357
l := NewLifecycleManager(log.Logger, "test-operator", "test-controller", fakeClient, []subroutine.Subroutine{}).WithConditionManagement()
13581358

13591359
// Then
1360-
assert.True(t, true, l.Config().ManageConditions)
1360+
assert.True(t, true, l.ConditionsManager() != nil)
13611361
}
13621362

13631363
type testReconciler struct {

‎controller/lifecycle/lifecycle.go‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,9 @@ type Lifecycle interface {
3636
}
3737

3838
type Config struct {
39-
OperatorName string
40-
ControllerName string
41-
SpreadReconciles bool
42-
ManageConditions bool
43-
ReadOnly bool
39+
OperatorName string
40+
ControllerName string
41+
ReadOnly bool
4442
}
4543

4644
type PrepareContextFunc func(ctx context.Context, instance runtimeobject.RuntimeObject) (context.Context, errors.OperatorError)
@@ -72,7 +70,7 @@ func Reconcile(ctx context.Context, req ctrl.Request, instance runtimeobject.Run
7270
originalCopy := instance.DeepCopyObject()
7371
inDeletion := instance.GetDeletionTimestamp() != nil
7472

75-
if l.Config().SpreadReconciles && instance.GetDeletionTimestamp().IsZero() {
73+
if l.Spreader() != nil && instance.GetDeletionTimestamp().IsZero() {
7674
instanceStatusObj := l.Spreader().MustToRuntimeObjectSpreadReconcileStatusInterface(instance, log)
7775
generationChanged = instance.GetGeneration() != instanceStatusObj.GetObservedGeneration()
7876
isAfterNextReconcileTime := v1.Now().UTC().After(instanceStatusObj.GetNextReconcileTime().UTC())
@@ -92,7 +90,7 @@ func Reconcile(ctx context.Context, req ctrl.Request, instance runtimeobject.Run
9290
}
9391

9492
var condArr []v1.Condition
95-
if l.Config().ManageConditions {
93+
if l.ConditionsManager() != nil {
9694
condArr = l.ConditionsManager().MustToRuntimeObjectConditionsInterface(instance, log).GetConditions()
9795
l.ConditionsManager().SetInstanceConditionUnknownIfNotSet(&condArr)
9896
}
@@ -114,21 +112,21 @@ func Reconcile(ctx context.Context, req ctrl.Request, instance runtimeobject.Run
114112

115113
// Continue with reconciliation
116114
for _, subroutine := range subroutines {
117-
if l.Config().ManageConditions {
115+
if l.ConditionsManager() != nil {
118116
l.ConditionsManager().SetSubroutineConditionToUnknownIfNotSet(&condArr, subroutine, inDeletion, log)
119117
}
120118

121119
// Set current condArr before reconciling the subroutine
122-
if l.Config().ManageConditions {
120+
if l.ConditionsManager() != nil {
123121
l.ConditionsManager().MustToRuntimeObjectConditionsInterface(instance, log).SetConditions(condArr)
124122
}
125123
subResult, retry, err := reconcileSubroutine(ctx, instance, subroutine, cl, l, log, generationChanged, sentryTags)
126124
// Update condArr with any changes the subroutine did
127-
if l.Config().ManageConditions {
125+
if l.ConditionsManager() != nil {
128126
condArr = l.ConditionsManager().MustToRuntimeObjectConditionsInterface(instance, log).GetConditions()
129127
}
130128
if err != nil {
131-
if l.Config().ManageConditions {
129+
if l.ConditionsManager() != nil {
132130
l.ConditionsManager().SetSubroutineCondition(&condArr, subroutine, result, err, inDeletion, log)
133131
l.ConditionsManager().SetInstanceConditionReady(&condArr, v1.ConditionFalse)
134132
l.ConditionsManager().MustToRuntimeObjectConditionsInterface(instance, log).SetConditions(condArr)
@@ -149,7 +147,7 @@ func Reconcile(ctx context.Context, req ctrl.Request, instance runtimeobject.Run
149147
result.RequeueAfter = subResult.RequeueAfter
150148
}
151149
}
152-
if l.Config().ManageConditions {
150+
if l.ConditionsManager() != nil {
153151
if subResult.RequeueAfter == 0 {
154152
l.ConditionsManager().SetSubroutineCondition(&condArr, subroutine, subResult, err, inDeletion, log)
155153
}
@@ -160,12 +158,12 @@ func Reconcile(ctx context.Context, req ctrl.Request, instance runtimeobject.Run
160158
// Reconciliation was successful
161159
MarkResourceAsFinal(instance, log, condArr, v1.ConditionTrue, l)
162160
} else {
163-
if l.Config().ManageConditions {
161+
if l.ConditionsManager() != nil {
164162
l.ConditionsManager().SetInstanceConditionReady(&condArr, v1.ConditionFalse)
165163
}
166164
}
167165

168-
if l.Config().ManageConditions {
166+
if l.ConditionsManager() != nil {
169167
l.ConditionsManager().MustToRuntimeObjectConditionsInterface(instance, log).SetConditions(condArr)
170168
}
171169

@@ -176,7 +174,7 @@ func Reconcile(ctx context.Context, req ctrl.Request, instance runtimeobject.Run
176174
}
177175
}
178176

179-
if l.Config().SpreadReconciles && instance.GetDeletionTimestamp().IsZero() {
177+
if l.Spreader() != nil && instance.GetDeletionTimestamp().IsZero() {
180178
original := instance.DeepCopyObject().(client.Object)
181179
removed := l.Spreader().RemoveRefreshLabelIfExists(instance)
182180
if removed {
@@ -320,13 +318,13 @@ func HandleClientError(msg string, log *logger.Logger, err error, generationChan
320318
}
321319

322320
func MarkResourceAsFinal(instance runtimeobject.RuntimeObject, log *logger.Logger, conditions []v1.Condition, status v1.ConditionStatus, l Lifecycle) {
323-
if l.Config().SpreadReconciles && instance.GetDeletionTimestamp().IsZero() {
321+
if l.Spreader() != nil && instance.GetDeletionTimestamp().IsZero() {
324322
instanceStatusObj := l.Spreader().MustToRuntimeObjectSpreadReconcileStatusInterface(instance, log)
325323
l.Spreader().SetNextReconcileTime(instanceStatusObj, log)
326324
l.Spreader().UpdateObservedGeneration(instanceStatusObj, log)
327325
}
328326

329-
if l.Config().ManageConditions {
327+
if l.ConditionsManager() != nil {
330328
l.ConditionsManager().SetInstanceConditionReady(&conditions, status)
331329
}
332330
}

0 commit comments

Comments
 (0)