Skip to content

Commit fa112fe

Browse files
committed
Refactor ClusterExtension reconciler to use composable step-based pipeline
Replaces the monolithic 170-line reconcile() method with a flexible step-based architecture that executes discrete reconciliation phases in sequence. Each phase (`HandleFinalizers`, `RetrieveRevisionStates`, `RetrieveRevisionMetadata`, `UnpackBundle`, `ApplyBundle`) is now a standalone function that can be composed differently for Helm vs Boxcutter workflows. Changes: - Introduce `ReconcileStepFunc` type and `ReconcileSteps` executor - Extract reconcile logic into individual step functions in new file `clusterextension_reconcile_steps.go` - Move `BoxcutterRevisionStatesGetter` to `boxcutter_reconcile_steps.go` alongside `MigrateStorage` step - Configure step pipelines in `main.go` for each applier type - Refactor tests to use functional options pattern for reconciler setup
1 parent 1355ff7 commit fa112fe

File tree

6 files changed

+531
-359
lines changed

6 files changed

+531
-359
lines changed

cmd/operator-controller/main.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,14 @@ func setupBoxcutter(
587587
ActionClientGetter: acg,
588588
RevisionGenerator: rg,
589589
}
590+
ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{
591+
controllers.HandleFinalizers(ceReconciler.Finalizers),
592+
controllers.MigrateStorage(ceReconciler.StorageMigrator),
593+
controllers.RetrieveRevisionStates(ceReconciler.RevisionStatesGetter),
594+
controllers.RetrieveRevisionMetadata(ceReconciler.Resolver),
595+
controllers.UnpackBundle(ceReconciler.ImagePuller, ceReconciler.ImageCache),
596+
controllers.ApplyBundle(ceReconciler.Applier),
597+
}
590598

591599
baseDiscoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig())
592600
if err != nil {
@@ -698,6 +706,14 @@ func setupHelm(
698706
Manager: cm,
699707
}
700708
ceReconciler.RevisionStatesGetter = &controllers.HelmRevisionStatesGetter{ActionClientGetter: acg}
709+
ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{
710+
controllers.HandleFinalizers(ceReconciler.Finalizers),
711+
controllers.RetrieveRevisionStates(ceReconciler.RevisionStatesGetter),
712+
controllers.RetrieveRevisionMetadata(ceReconciler.Resolver),
713+
controllers.UnpackBundle(ceReconciler.ImagePuller, ceReconciler.ImageCache),
714+
controllers.ApplyBundle(ceReconciler.Applier),
715+
}
716+
701717
return nil
702718
}
703719

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
Copyright 2025.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controllers
18+
19+
import (
20+
"cmp"
21+
"context"
22+
"fmt"
23+
"slices"
24+
25+
apimeta "k8s.io/apimachinery/pkg/api/meta"
26+
ctrl "sigs.k8s.io/controller-runtime"
27+
"sigs.k8s.io/controller-runtime/pkg/client"
28+
29+
ocv1 "github.com/operator-framework/operator-controller/api/v1"
30+
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
31+
)
32+
33+
type BoxcutterRevisionStatesGetter struct {
34+
Reader client.Reader
35+
}
36+
37+
func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, ext *ocv1.ClusterExtension) (*RevisionStates, error) {
38+
// TODO: boxcutter applier has a nearly identical bit of code for listing and sorting revisions
39+
// only difference here is that it sorts in reverse order to start iterating with the most
40+
// recent revisions. We should consolidate to avoid code duplication.
41+
existingRevisionList := &ocv1.ClusterExtensionRevisionList{}
42+
if err := d.Reader.List(ctx, existingRevisionList, client.MatchingLabels{
43+
labels.OwnerNameKey: ext.Name,
44+
}); err != nil {
45+
return nil, fmt.Errorf("listing revisions: %w", err)
46+
}
47+
slices.SortFunc(existingRevisionList.Items, func(a, b ocv1.ClusterExtensionRevision) int {
48+
return cmp.Compare(a.Spec.Revision, b.Spec.Revision)
49+
})
50+
51+
rs := &RevisionStates{}
52+
for _, rev := range existingRevisionList.Items {
53+
switch rev.Spec.LifecycleState {
54+
case ocv1.ClusterExtensionRevisionLifecycleStateActive,
55+
ocv1.ClusterExtensionRevisionLifecycleStatePaused:
56+
default:
57+
// Skip anything not active or paused, which should only be "Archived".
58+
continue
59+
}
60+
61+
// TODO: the setting of these annotations (happens in boxcutter applier when we pass in "revisionAnnotations")
62+
// is fairly decoupled from this code where we get the annotations back out. We may want to co-locate
63+
// the set/get logic a bit better to make it more maintainable and less likely to get out of sync.
64+
rm := &RevisionMetadata{
65+
Package: rev.Annotations[labels.PackageNameKey],
66+
Image: rev.Annotations[labels.BundleReferenceKey],
67+
BundleMetadata: ocv1.BundleMetadata{
68+
Name: rev.Annotations[labels.BundleNameKey],
69+
Version: rev.Annotations[labels.BundleVersionKey],
70+
},
71+
}
72+
73+
if apimeta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) {
74+
rs.Installed = rm
75+
} else {
76+
rs.RollingOut = append(rs.RollingOut, rm)
77+
}
78+
}
79+
80+
return rs, nil
81+
}
82+
83+
func MigrateStorage(m StorageMigrator) ReconcileStepFunc {
84+
return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) {
85+
objLbls := map[string]string{
86+
labels.OwnerKindKey: ocv1.ClusterExtensionKind,
87+
labels.OwnerNameKey: ext.GetName(),
88+
}
89+
90+
if err := m.Migrate(ctx, ext, objLbls); err != nil {
91+
return nil, fmt.Errorf("migrating storage: %w", err)
92+
}
93+
return nil, nil
94+
}
95+
}

0 commit comments

Comments
 (0)