Skip to content

Commit 10c9239

Browse files
Per G. da Silvaclaude
andcommitted
Rename rev to cer in ClusterExtensionRevision reconciler for consistency
Standardize the parameter name from `rev` to `cer` across reconcile, establishWatch, listPreviousRevisions, and toBoxcutterRevision methods to match the naming convention used elsewhere in the controller. Also simplify the establishWatch signature to a single line. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ddf921f commit 10c9239

1 file changed

Lines changed: 42 additions & 45 deletions

File tree

internal/operator-controller/controllers/clusterextensionrevision_controller.go

Lines changed: 42 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -131,40 +131,40 @@ func checkForUnexpectedClusterExtensionRevisionFieldChange(a, b ocv1.ClusterExte
131131
return !equality.Semantic.DeepEqual(a.Spec, b.Spec)
132132
}
133133

134-
func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev *ocv1.ClusterExtensionRevision) (ctrl.Result, error) {
134+
func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, cer *ocv1.ClusterExtensionRevision) (ctrl.Result, error) {
135135
l := log.FromContext(ctx)
136136

137-
if !rev.DeletionTimestamp.IsZero() {
138-
return c.delete(ctx, rev)
137+
if !cer.DeletionTimestamp.IsZero() {
138+
return c.delete(ctx, cer)
139139
}
140140

141-
revision, opts, err := c.toBoxcutterRevision(ctx, rev)
141+
revision, opts, err := c.toBoxcutterRevision(ctx, cer)
142142
if err != nil {
143-
setRetryingConditions(rev, err.Error())
143+
setRetryingConditions(cer, err.Error())
144144
return ctrl.Result{}, fmt.Errorf("converting to boxcutter revision: %v", err)
145145
}
146146

147-
revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, rev)
147+
revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, cer)
148148
if err != nil {
149-
setRetryingConditions(rev, err.Error())
149+
setRetryingConditions(cer, err.Error())
150150
return ctrl.Result{}, fmt.Errorf("failed to create revision engine: %v", err)
151151
}
152152

153-
if rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived {
154-
if err := c.TrackingCache.Free(ctx, rev); err != nil {
155-
markAsAvailableUnknown(rev, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error())
153+
if cer.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived {
154+
if err := c.TrackingCache.Free(ctx, cer); err != nil {
155+
markAsAvailableUnknown(cer, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error())
156156
return ctrl.Result{}, fmt.Errorf("error stopping informers: %v", err)
157157
}
158-
return c.archive(ctx, revisionEngine, rev, revision)
158+
return c.archive(ctx, revisionEngine, cer, revision)
159159
}
160160

161-
if err := c.ensureFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer); err != nil {
161+
if err := c.ensureFinalizer(ctx, cer, clusterExtensionRevisionTeardownFinalizer); err != nil {
162162
return ctrl.Result{}, fmt.Errorf("error ensuring teardown finalizer: %v", err)
163163
}
164164

165-
if err := c.establishWatch(ctx, rev, revision); err != nil {
165+
if err := c.establishWatch(ctx, cer, revision); err != nil {
166166
werr := fmt.Errorf("establish watch: %v", err)
167-
setRetryingConditions(rev, werr.Error())
167+
setRetryingConditions(cer, werr.Error())
168168
return ctrl.Result{}, werr
169169
}
170170

@@ -174,22 +174,22 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
174174
// Log detailed reconcile reports only in debug mode (V(1)) to reduce verbosity.
175175
l.V(1).Info("reconcile report", "report", rres.String())
176176
}
177-
setRetryingConditions(rev, err.Error())
177+
setRetryingConditions(cer, err.Error())
178178
return ctrl.Result{}, fmt.Errorf("revision reconcile: %v", err)
179179
}
180180

181181
// Retry failing preflight checks with a flat 10s retry.
182182
// TODO: report status, backoff?
183183
if verr := rres.GetValidationError(); verr != nil {
184184
l.Error(fmt.Errorf("%w", verr), "preflight validation failed, retrying after 10s")
185-
setRetryingConditions(rev, fmt.Sprintf("revision validation error: %s", verr))
185+
setRetryingConditions(cer, fmt.Sprintf("revision validation error: %s", verr))
186186
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
187187
}
188188

189189
for i, pres := range rres.GetPhases() {
190190
if verr := pres.GetValidationError(); verr != nil {
191191
l.Error(fmt.Errorf("%w", verr), "phase preflight validation failed, retrying after 10s", "phase", i)
192-
setRetryingConditions(rev, fmt.Sprintf("phase %d validation error: %s", i, verr))
192+
setRetryingConditions(cer, fmt.Sprintf("phase %d validation error: %s", i, verr))
193193
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
194194
}
195195

@@ -202,22 +202,22 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
202202

203203
if len(collidingObjs) > 0 {
204204
l.Error(fmt.Errorf("object collision detected"), "object collision, retrying after 10s", "phase", i, "collisions", collidingObjs)
205-
setRetryingConditions(rev, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")))
205+
setRetryingConditions(cer, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")))
206206
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
207207
}
208208
}
209209

210-
revVersion := rev.GetAnnotations()[labels.BundleVersionKey]
210+
revVersion := cer.GetAnnotations()[labels.BundleVersionKey]
211211
if !rres.InTransition() {
212-
markAsProgressing(rev, ocv1.ReasonSucceeded, fmt.Sprintf("Revision %s has rolled out.", revVersion))
212+
markAsProgressing(cer, ocv1.ReasonSucceeded, fmt.Sprintf("Revision %s has rolled out.", revVersion))
213213
} else {
214-
markAsProgressing(rev, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion))
214+
markAsProgressing(cer, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion))
215215
}
216216

217217
//nolint:nestif
218218
if rres.IsComplete() {
219219
// Archive previous revisions
220-
previous, err := c.listPreviousRevisions(ctx, rev)
220+
previous, err := c.listPreviousRevisions(ctx, cer)
221221
if err != nil {
222222
return ctrl.Result{}, fmt.Errorf("listing previous revisions: %v", err)
223223
}
@@ -231,17 +231,17 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
231231
}
232232
}
233233

234-
markAsAvailable(rev, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, "Objects are available and pass all probes.")
234+
markAsAvailable(cer, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, "Objects are available and pass all probes.")
235235

236236
// We'll probably only want to remove this once we are done updating the ClusterExtension conditions
237237
// as its one of the interfaces between the revision and the extension. If we still have the Succeeded for now
238238
// that's fine.
239-
meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
239+
meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{
240240
Type: ocv1.ClusterExtensionRevisionTypeSucceeded,
241241
Status: metav1.ConditionTrue,
242242
Reason: ocv1.ReasonSucceeded,
243243
Message: "Revision succeeded rolling out.",
244-
ObservedGeneration: rev.Generation,
244+
ObservedGeneration: cer.Generation,
245245
})
246246
} else {
247247
var probeFailureMsgs []string
@@ -271,9 +271,9 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
271271
}
272272

273273
if len(probeFailureMsgs) > 0 {
274-
markAsUnavailable(rev, ocv1.ClusterExtensionRevisionReasonProbeFailure, strings.Join(probeFailureMsgs, "\n"))
274+
markAsUnavailable(cer, ocv1.ClusterExtensionRevisionReasonProbeFailure, strings.Join(probeFailureMsgs, "\n"))
275275
} else {
276-
markAsUnavailable(rev, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion))
276+
markAsUnavailable(cer, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion))
277277
}
278278
}
279279

@@ -350,18 +350,15 @@ func (c *ClusterExtensionRevisionReconciler) SetupWithManager(mgr ctrl.Manager)
350350
Complete(c)
351351
}
352352

353-
func (c *ClusterExtensionRevisionReconciler) establishWatch(
354-
ctx context.Context, rev *ocv1.ClusterExtensionRevision,
355-
boxcutterRev *boxcutter.Revision,
356-
) error {
353+
func (c *ClusterExtensionRevisionReconciler) establishWatch(ctx context.Context, cer *ocv1.ClusterExtensionRevision, revision *boxcutter.Revision) error {
357354
gvks := sets.New[schema.GroupVersionKind]()
358-
for _, phase := range boxcutterRev.Phases {
355+
for _, phase := range revision.Phases {
359356
for _, obj := range phase.Objects {
360357
gvks.Insert(obj.GroupVersionKind())
361358
}
362359
}
363360

364-
return c.TrackingCache.Watch(ctx, rev, gvks)
361+
return c.TrackingCache.Watch(ctx, cer, gvks)
365362
}
366363

367364
func (c *ClusterExtensionRevisionReconciler) ensureFinalizer(
@@ -413,8 +410,8 @@ func (c *ClusterExtensionRevisionReconciler) removeFinalizer(ctx context.Context
413410

414411
// listPreviousRevisions returns active revisions belonging to the same ClusterExtension with lower revision numbers.
415412
// Filters out the current revision, archived revisions, deleting revisions, and revisions with equal or higher numbers.
416-
func (c *ClusterExtensionRevisionReconciler) listPreviousRevisions(ctx context.Context, rev *ocv1.ClusterExtensionRevision) ([]*ocv1.ClusterExtensionRevision, error) {
417-
ownerLabel, ok := rev.Labels[labels.OwnerNameKey]
413+
func (c *ClusterExtensionRevisionReconciler) listPreviousRevisions(ctx context.Context, cer *ocv1.ClusterExtensionRevision) ([]*ocv1.ClusterExtensionRevision, error) {
414+
ownerLabel, ok := cer.Labels[labels.OwnerNameKey]
418415
if !ok {
419416
// No owner label means this revision isn't properly labeled - return empty list
420417
return nil, nil
@@ -430,7 +427,7 @@ func (c *ClusterExtensionRevisionReconciler) listPreviousRevisions(ctx context.C
430427
previous := make([]*ocv1.ClusterExtensionRevision, 0, len(revList.Items))
431428
for i := range revList.Items {
432429
r := &revList.Items[i]
433-
if r.Name == rev.Name {
430+
if r.Name == cer.Name {
434431
continue
435432
}
436433
// Skip archived or deleting revisions
@@ -439,7 +436,7 @@ func (c *ClusterExtensionRevisionReconciler) listPreviousRevisions(ctx context.C
439436
continue
440437
}
441438
// Only include revisions with lower revision numbers (actual previous revisions)
442-
if r.Spec.Revision >= rev.Spec.Revision {
439+
if r.Spec.Revision >= cer.Spec.Revision {
443440
continue
444441
}
445442
previous = append(previous, r)
@@ -448,8 +445,8 @@ func (c *ClusterExtensionRevisionReconciler) listPreviousRevisions(ctx context.C
448445
return previous, nil
449446
}
450447

451-
func (c *ClusterExtensionRevisionReconciler) toBoxcutterRevision(ctx context.Context, rev *ocv1.ClusterExtensionRevision) (*boxcutter.Revision, []boxcutter.RevisionReconcileOption, error) {
452-
previous, err := c.listPreviousRevisions(ctx, rev)
448+
func (c *ClusterExtensionRevisionReconciler) toBoxcutterRevision(ctx context.Context, cer *ocv1.ClusterExtensionRevision) (*boxcutter.Revision, []boxcutter.RevisionReconcileOption, error) {
449+
previous, err := c.listPreviousRevisions(ctx, cer)
453450
if err != nil {
454451
return nil, nil, fmt.Errorf("listing previous revisions: %w", err)
455452
}
@@ -468,11 +465,11 @@ func (c *ClusterExtensionRevisionReconciler) toBoxcutterRevision(ctx context.Con
468465
}
469466

470467
r := &boxcutter.Revision{
471-
Name: rev.Name,
472-
Owner: rev,
473-
Revision: rev.Spec.Revision,
468+
Name: cer.Name,
469+
Owner: cer,
470+
Revision: cer.Spec.Revision,
474471
}
475-
for _, specPhase := range rev.Spec.Phases {
472+
for _, specPhase := range cer.Spec.Phases {
476473
phase := boxcutter.Phase{Name: specPhase.Name}
477474
for _, specObj := range specPhase.Objects {
478475
obj := specObj.Object.DeepCopy()
@@ -481,10 +478,10 @@ func (c *ClusterExtensionRevisionReconciler) toBoxcutterRevision(ctx context.Con
481478
if objLabels == nil {
482479
objLabels = map[string]string{}
483480
}
484-
objLabels[labels.OwnerNameKey] = rev.Labels[labels.OwnerNameKey]
481+
objLabels[labels.OwnerNameKey] = cer.Labels[labels.OwnerNameKey]
485482
obj.SetLabels(objLabels)
486483

487-
switch cp := EffectiveCollisionProtection(rev.Spec.CollisionProtection, specPhase.CollisionProtection, specObj.CollisionProtection); cp {
484+
switch cp := EffectiveCollisionProtection(cer.Spec.CollisionProtection, specPhase.CollisionProtection, specObj.CollisionProtection); cp {
488485
case ocv1.CollisionProtectionIfNoController, ocv1.CollisionProtectionNone:
489486
opts = append(opts, boxcutter.WithObjectReconcileOptions(
490487
obj, boxcutter.WithCollisionProtection(cp)))

0 commit comments

Comments
 (0)