Skip to content

Commit 4f5f616

Browse files
Expose GenericProvider reconciliation with phases
Implement a phase-based framework for reconciling generic providers. This refactors the main reconciliation loop into a sequence of distinct, ordered phases (e.g., download, install, delete). Introduce `PhaseFn` and `phaseReconciler` types to manage phase execution. Move existing reconciliation logic into dedicated phase functions. Update the external `controller` package to epose aliases pointing to the refactored internal types for external import. Signed-off-by: Danil-Grigorev <[email protected]>
1 parent 63f669e commit 4f5f616

File tree

5 files changed

+59
-80
lines changed

5 files changed

+59
-80
lines changed

controller/alias.go

Lines changed: 11 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,45 +21,21 @@ to allow external users to interact with the core controller logic.
2121
package controller
2222

2323
import (
24-
"context"
25-
26-
"k8s.io/client-go/rest"
27-
internalcontroller "sigs.k8s.io/cluster-api-operator/internal/controller"
28-
"sigs.k8s.io/cluster-api-operator/internal/controller/genericprovider"
24+
providercontroller "sigs.k8s.io/cluster-api-operator/internal/controller"
2925
internalhealthcheck "sigs.k8s.io/cluster-api-operator/internal/controller/healthcheck"
30-
ctrl "sigs.k8s.io/controller-runtime"
31-
"sigs.k8s.io/controller-runtime/pkg/client"
32-
"sigs.k8s.io/controller-runtime/pkg/controller"
3326
)
3427

3528
// GenericProviderReconciler wraps the internal GenericProviderReconciler.
36-
type GenericProviderReconciler struct {
37-
Provider genericprovider.GenericProvider
38-
ProviderList genericprovider.GenericProviderList
39-
Client client.Client
40-
Config *rest.Config
41-
WatchConfigSecretChanges bool
42-
}
29+
type GenericProviderReconciler = providercontroller.GenericProviderReconciler
30+
31+
// GenericProviderHealthCheckReconciler wraps the internal GenericProviderHealthCheckReconciler.
32+
type GenericProviderHealthCheckReconciler = internalhealthcheck.GenericProviderHealthCheckReconciler
4333

44-
// SetupWithManager sets up the GenericProviderReconciler with the Manager.
45-
func (r *GenericProviderReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
46-
return (&internalcontroller.GenericProviderReconciler{
47-
Provider: r.Provider,
48-
ProviderList: r.ProviderList,
49-
Client: r.Client,
50-
Config: r.Config,
51-
WatchConfigSecretChanges: r.WatchConfigSecretChanges,
52-
}).SetupWithManager(ctx, mgr, options)
53-
}
34+
// PhaseFn is an alias for the internal PhaseFn type.
35+
type PhaseFn = providercontroller.PhaseFn
5436

55-
// ProviderHealthCheckReconciler wraps the internal ProviderHealthCheckReconciler.
56-
type ProviderHealthCheckReconciler struct {
57-
Client client.Client
58-
}
37+
// Result is an alias for the internal Result type.
38+
type Result = providercontroller.Result
5939

60-
// SetupWithManager sets up the health check controllers with the Manager.
61-
func (r *ProviderHealthCheckReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
62-
return (&internalhealthcheck.ProviderHealthCheckReconciler{
63-
Client: r.Client,
64-
}).SetupWithManager(mgr, options)
65-
}
40+
// NewPhaseReconciler is an alias for the internal NewPhaseReconciler function.
41+
var NewPhaseReconciler = providercontroller.NewPhaseReconciler

internal/controller/genericprovider_controller.go

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ type GenericProviderReconciler struct {
4949
Client client.Client
5050
Config *rest.Config
5151
WatchConfigSecretChanges bool
52+
53+
DeletePhases []PhaseFn
54+
ReconcilePhases []PhaseFn
5255
}
5356

5457
const (
@@ -82,6 +85,23 @@ func (r *GenericProviderReconciler) SetupWithManager(ctx context.Context, mgr ct
8285
)
8386
}
8487

88+
reconciler := NewPhaseReconciler(*r, r.Provider, r.ProviderList)
89+
90+
r.ReconcilePhases = []PhaseFn{
91+
reconciler.PreflightChecks,
92+
reconciler.InitializePhaseReconciler,
93+
reconciler.DownloadManifests,
94+
reconciler.Load,
95+
reconciler.Fetch,
96+
reconciler.Upgrade,
97+
reconciler.Install,
98+
reconciler.ReportStatus,
99+
}
100+
101+
r.DeletePhases = []PhaseFn{
102+
reconciler.Delete,
103+
}
104+
85105
return builder.WithOptions(options).
86106
Complete(r)
87107
}
@@ -150,7 +170,7 @@ func (r *GenericProviderReconciler) Reconcile(ctx context.Context, req reconcile
150170
return ctrl.Result{}, nil
151171
}
152172

153-
res, err := r.reconcile(ctx, r.Provider, r.ProviderList)
173+
res, err := r.reconcile(ctx)
154174

155175
annotations := r.Provider.GetAnnotations()
156176
if annotations == nil {
@@ -189,27 +209,15 @@ func patchProvider(ctx context.Context, provider operatorv1.GenericProvider, pat
189209
return patchHelper.Patch(ctx, provider, options...)
190210
}
191211

192-
func (r *GenericProviderReconciler) reconcile(ctx context.Context, provider genericprovider.GenericProvider, genericProviderList genericprovider.GenericProviderList) (*Result, error) {
193-
reconciler := NewPhaseReconciler(*r, provider, genericProviderList)
194-
phases := []PhaseFn{
195-
reconciler.preflightChecks,
196-
reconciler.initializePhaseReconciler,
197-
reconciler.downloadManifests,
198-
reconciler.load,
199-
reconciler.fetch,
200-
reconciler.upgrade,
201-
reconciler.install,
202-
reconciler.reportStatus,
203-
}
204-
212+
func (r *GenericProviderReconciler) reconcile(ctx context.Context) (*Result, error) {
205213
var res Result
206214

207-
for _, phase := range phases {
215+
for _, phase := range r.ReconcilePhases {
208216
res, err := phase(ctx)
209217
if err != nil {
210218
var pe *PhaseError
211219
if errors.As(err, &pe) {
212-
conditions.Set(provider, conditions.FalseCondition(pe.Type, pe.Reason, pe.Severity, "%s", err.Error()))
220+
conditions.Set(r.Provider, conditions.FalseCondition(pe.Type, pe.Reason, pe.Severity, "%s", err.Error()))
213221
}
214222
}
215223

@@ -232,14 +240,9 @@ func (r *GenericProviderReconciler) reconcileDelete(ctx context.Context, provide
232240

233241
log.Info("Deleting provider resources")
234242

235-
reconciler := NewPhaseReconciler(*r, provider, nil)
236-
phases := []PhaseFn{
237-
reconciler.delete,
238-
}
239-
240243
var res Result
241244

242-
for _, phase := range phases {
245+
for _, phase := range r.DeletePhases {
243246
res, err := phase(ctx)
244247
if err != nil {
245248
var pe *PhaseError

internal/controller/manifests_downloader.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ const (
4545
ociSource = "oci"
4646
)
4747

48-
// downloadManifests downloads CAPI manifests from a url.
49-
func (p *PhaseReconciler) downloadManifests(ctx context.Context) (*Result, error) {
48+
// DownloadManifests downloads CAPI manifests from a url.
49+
func (p *PhaseReconciler) DownloadManifests(ctx context.Context) (*Result, error) {
5050
log := ctrl.LoggerFrom(ctx)
5151

5252
// Return immediately if a custom config map is used instead of a url.

internal/controller/manifests_downloader_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ func TestManifestsDownloader(t *testing.T) {
5151
},
5252
}
5353

54-
_, err := p.initializePhaseReconciler(ctx)
54+
_, err := p.InitializePhaseReconciler(ctx)
5555
g.Expect(err).ToNot(HaveOccurred())
5656

57-
_, err = p.downloadManifests(ctx)
57+
_, err = p.DownloadManifests(ctx)
5858
g.Expect(err).ToNot(HaveOccurred())
5959

6060
// Ensure that config map was created
@@ -94,16 +94,16 @@ func TestProviderDownloadWithOverrides(t *testing.T) {
9494
overridesClient: overridesClient,
9595
}
9696

97-
_, err = p.initializePhaseReconciler(ctx)
97+
_, err = p.InitializePhaseReconciler(ctx)
9898
g.Expect(err).ToNot(HaveOccurred())
9999

100-
_, err = p.downloadManifests(ctx)
100+
_, err = p.DownloadManifests(ctx)
101101
g.Expect(err).ToNot(HaveOccurred())
102102

103-
_, err = p.load(ctx)
103+
_, err = p.Load(ctx)
104104
g.Expect(err).ToNot(HaveOccurred())
105105

106-
_, err = p.fetch(ctx)
106+
_, err = p.Fetch(ctx)
107107
g.Expect(err).ToNot(HaveOccurred())
108108

109109
g.Expect(p.components.Images()).To(HaveExactElements([]string{"registry.k8s.io/cluster-api/cluster-api-controller:v1.4.3"}))

internal/controller/phases.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -160,13 +160,13 @@ func (i InNamespace) ApplyToConfigMapRepository(settings *ConfigMapRepositorySet
160160
settings.namespace = string(i)
161161
}
162162

163-
// preflightChecks a wrapper around the preflight checks.
164-
func (p *PhaseReconciler) preflightChecks(ctx context.Context) (*Result, error) {
163+
// PreflightChecks a wrapper around the preflight checks.
164+
func (p *PhaseReconciler) PreflightChecks(ctx context.Context) (*Result, error) {
165165
return &Result{}, preflightChecks(ctx, p.ctrlClient, p.provider, p.providerList)
166166
}
167167

168-
// initializePhaseReconciler initializes phase reconciler.
169-
func (p *PhaseReconciler) initializePhaseReconciler(ctx context.Context) (*Result, error) {
168+
// InitializePhaseReconciler initializes phase reconciler.
169+
func (p *PhaseReconciler) InitializePhaseReconciler(ctx context.Context) (*Result, error) {
170170
path := configPath
171171
if _, err := os.Stat(configPath); os.IsNotExist(err) {
172172
path = ""
@@ -227,8 +227,8 @@ func (p *PhaseReconciler) initializePhaseReconciler(ctx context.Context) (*Resul
227227
return &Result{}, nil
228228
}
229229

230-
// load provider specific configuration into phaseReconciler object.
231-
func (p *PhaseReconciler) load(ctx context.Context) (*Result, error) {
230+
// Load provider specific configuration into phaseReconciler object.
231+
func (p *PhaseReconciler) Load(ctx context.Context) (*Result, error) {
232232
log := ctrl.LoggerFrom(ctx)
233233

234234
log.Info("Loading provider")
@@ -511,8 +511,8 @@ func (p *PhaseReconciler) validateRepoCAPIVersion(ctx context.Context) error {
511511
return nil
512512
}
513513

514-
// fetch fetches the provider components from the repository and processes all yaml manifests.
515-
func (p *PhaseReconciler) fetch(ctx context.Context) (*Result, error) {
514+
// Fetch fetches the provider components from the repository and processes all yaml manifests.
515+
func (p *PhaseReconciler) Fetch(ctx context.Context) (*Result, error) {
516516
log := ctrl.LoggerFrom(ctx)
517517
log.Info("Fetching provider")
518518

@@ -559,9 +559,9 @@ func (p *PhaseReconciler) fetch(ctx context.Context) (*Result, error) {
559559
return &Result{}, nil
560560
}
561561

562-
// upgrade ensure all the clusterctl CRDs are available before installing the provider,
562+
// Upgrade ensure all the clusterctl CRDs are available before installing the provider,
563563
// and update existing components if required.
564-
func (p *PhaseReconciler) upgrade(ctx context.Context) (*Result, error) {
564+
func (p *PhaseReconciler) Upgrade(ctx context.Context) (*Result, error) {
565565
log := ctrl.LoggerFrom(ctx)
566566

567567
// Nothing to do if it's a fresh installation.
@@ -589,8 +589,8 @@ func (p *PhaseReconciler) upgrade(ctx context.Context) (*Result, error) {
589589
return &Result{}, nil
590590
}
591591

592-
// install installs the provider components using clusterctl library.
593-
func (p *PhaseReconciler) install(ctx context.Context) (*Result, error) {
592+
// Install installs the provider components using clusterctl library.
593+
func (p *PhaseReconciler) Install(ctx context.Context) (*Result, error) {
594594
log := ctrl.LoggerFrom(ctx)
595595

596596
// Provider was upgraded, nothing to do
@@ -617,7 +617,7 @@ func (p *PhaseReconciler) install(ctx context.Context) (*Result, error) {
617617
return &Result{}, nil
618618
}
619619

620-
func (p *PhaseReconciler) reportStatus(_ context.Context) (*Result, error) {
620+
func (p *PhaseReconciler) ReportStatus(_ context.Context) (*Result, error) {
621621
status := p.provider.GetStatus()
622622
status.Contract = &p.contract
623623
installedVersion := p.components.Version()
@@ -665,8 +665,8 @@ func loadCustomProviders(providers []operatorv1.GenericProvider, reader configcl
665665
return mr, nil
666666
}
667667

668-
// delete deletes the provider components using clusterctl library.
669-
func (p *PhaseReconciler) delete(ctx context.Context) (*Result, error) {
668+
// Delete deletes the provider components using clusterctl library.
669+
func (p *PhaseReconciler) Delete(ctx context.Context) (*Result, error) {
670670
log := ctrl.LoggerFrom(ctx)
671671
log.Info("Deleting provider")
672672

0 commit comments

Comments
 (0)