Skip to content

Commit 570aaca

Browse files
(fix) catalog deletion resilience support
Enables installed extensions to continue working when their source catalog becomes unavailable or is deleted. When resolution fails due to catalog unavailability, the operator now continues reconciling with the currently installed bundle instead of failing. Changes: - Resolution falls back to installed bundle when catalog unavailable - Unpacking skipped when maintaining current installed state - Helm and Boxcutter appliers handle nil contentFS gracefully - Version upgrades properly blocked without catalog access This ensures workloads remain stable and operational even when the catalog they were installed from is temporarily unavailable or deleted, while appropriately preventing version changes that require catalog access. Assisted-by: Cursor
1 parent 69f5e82 commit 570aaca

12 files changed

Lines changed: 701 additions & 30 deletions

cmd/operator-controller/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl
623623
controllers.HandleFinalizers(c.finalizers),
624624
controllers.MigrateStorage(storageMigrator),
625625
controllers.RetrieveRevisionStates(revisionStatesGetter),
626-
controllers.ResolveBundle(c.resolver),
626+
controllers.ResolveBundle(c.resolver, c.mgr.GetClient()),
627627
controllers.UnpackBundle(c.imagePuller, c.imageCache),
628628
controllers.ApplyBundleWithBoxcutter(appl.Apply),
629629
}
@@ -742,7 +742,7 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster
742742
ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{
743743
controllers.HandleFinalizers(c.finalizers),
744744
controllers.RetrieveRevisionStates(revisionStatesGetter),
745-
controllers.ResolveBundle(c.resolver),
745+
controllers.ResolveBundle(c.resolver, c.mgr.GetClient()),
746746
controllers.UnpackBundle(c.imagePuller, c.imageCache),
747747
controllers.ApplyBundle(appl),
748748
}

internal/operator-controller/applier/boxcutter.go

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -303,21 +303,37 @@ func (bc *Boxcutter) createOrUpdate(ctx context.Context, obj client.Object) erro
303303
return bc.Client.Patch(ctx, obj, client.Apply, client.FieldOwner(bc.FieldOwner), client.ForceOwnership)
304304
}
305305

306-
func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) error {
307-
// Generate desired revision
308-
desiredRevision, err := bc.RevisionGenerator.GenerateRevision(ctx, contentFS, ext, objectLabels, revisionAnnotations)
306+
func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (bool, string, error) {
307+
// Note: We list revisions first (before checking contentFS) because we need the revision list
308+
// to determine if we can fall back when contentFS is nil. If the API call fails here,
309+
// it indicates a serious cluster connectivity issue, and we should not proceed even in fallback mode
310+
// since the ClusterExtensionRevision controller also requires API access to maintain resources.
311+
existingRevisions, err := bc.getExistingRevisions(ctx, ext.GetName())
309312
if err != nil {
310-
return err
313+
return false, "", err
311314
}
312315

313-
if err := controllerutil.SetControllerReference(ext, desiredRevision, bc.Scheme); err != nil {
314-
return fmt.Errorf("set ownerref: %w", err)
316+
// If contentFS is nil, we're maintaining the current state without catalog access.
317+
// In this case, we should use the existing installed revision without generating a new one.
318+
if contentFS == nil {
319+
if len(existingRevisions) == 0 {
320+
return false, "", fmt.Errorf("cannot maintain workload: no catalog content available and no previously installed revision found")
321+
}
322+
// Returning true here signals that the rollout has succeeded using the current revision. The
323+
// ClusterExtensionRevision controller will continue to reconcile, apply, and maintain the
324+
// resources defined in that revision via Server-Side Apply, ensuring the workload keeps running
325+
// even when catalog access (and thus new revision content) is unavailable.
326+
return true, "", nil
315327
}
316328

317-
// List all existing revisions
318-
existingRevisions, err := bc.getExistingRevisions(ctx, ext.GetName())
329+
// Generate desired revision
330+
desiredRevision, err := bc.RevisionGenerator.GenerateRevision(ctx, contentFS, ext, objectLabels, revisionAnnotations)
319331
if err != nil {
320-
return err
332+
return false, "", err
333+
}
334+
335+
if err := controllerutil.SetControllerReference(ext, desiredRevision, bc.Scheme); err != nil {
336+
return false, "", fmt.Errorf("set ownerref: %w", err)
321337
}
322338

323339
currentRevision := &ocv1.ClusterExtensionRevision{}
@@ -339,7 +355,7 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
339355
// inplace patch was successful, no changes in phases
340356
state = StateUnchanged
341357
default:
342-
return fmt.Errorf("patching %s Revision: %w", desiredRevision.Name, err)
358+
return false, "", fmt.Errorf("patching %s Revision: %w", desiredRevision.Name, err)
343359
}
344360
}
345361

@@ -353,7 +369,7 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
353369
case StateNeedsInstall:
354370
err := preflight.Install(ctx, plainObjs)
355371
if err != nil {
356-
return err
372+
return false, "", err
357373
}
358374
// TODO: jlanford's IDE says that "StateNeedsUpgrade" condition is always true, but
359375
// it isn't immediately obvious why that is. Perhaps len(existingRevisions) is
@@ -362,7 +378,7 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
362378
case StateNeedsUpgrade:
363379
err := preflight.Upgrade(ctx, plainObjs)
364380
if err != nil {
365-
return err
381+
return false, "", err
366382
}
367383
}
368384
}
@@ -376,15 +392,15 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
376392
desiredRevision.Spec.Revision = revisionNumber
377393

378394
if err = bc.garbageCollectOldRevisions(ctx, prevRevisions); err != nil {
379-
return fmt.Errorf("garbage collecting old revisions: %w", err)
395+
return false, "", fmt.Errorf("garbage collecting old revisions: %w", err)
380396
}
381397

382398
if err := bc.createOrUpdate(ctx, desiredRevision); err != nil {
383-
return fmt.Errorf("creating new Revision: %w", err)
399+
return false, "", fmt.Errorf("creating new Revision: %w", err)
384400
}
385401
}
386402

387-
return nil
403+
return true, "", nil
388404
}
389405

390406
// garbageCollectOldRevisions deletes archived revisions beyond ClusterExtensionRevisionRetentionLimit.

internal/operator-controller/applier/boxcutter_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -926,14 +926,18 @@ func TestBoxcutter_Apply(t *testing.T) {
926926
labels.PackageNameKey: "test-package",
927927
}
928928
}
929-
err := boxcutter.Apply(t.Context(), testFS, ext, nil, revisionAnnotations)
929+
completed, status, err := boxcutter.Apply(t.Context(), testFS, ext, nil, revisionAnnotations)
930930

931931
// Assert
932932
if tc.expectedErr != "" {
933933
require.Error(t, err)
934934
assert.Contains(t, err.Error(), tc.expectedErr)
935+
assert.False(t, completed)
936+
assert.Empty(t, status)
935937
} else {
936938
require.NoError(t, err)
939+
assert.True(t, completed)
940+
assert.Empty(t, status)
937941
}
938942

939943
if tc.validate != nil {

internal/operator-controller/applier/helm.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,16 @@ func (h *Helm) runPreAuthorizationChecks(ctx context.Context, ext *ocv1.ClusterE
103103
}
104104

105105
func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) (bool, string, error) {
106+
// If contentFS is nil, we're maintaining the current state without catalog access.
107+
// In this case, reconcile the existing Helm release if it exists.
108+
if contentFS == nil {
109+
ac, err := h.ActionClientGetter.ActionClientFor(ctx, ext)
110+
if err != nil {
111+
return false, "", err
112+
}
113+
return h.reconcileExistingRelease(ctx, ac, ext)
114+
}
115+
106116
chrt, err := h.buildHelmChart(contentFS, ext)
107117
if err != nil {
108118
return false, "", err
@@ -197,6 +207,62 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
197207
return true, "", nil
198208
}
199209

210+
// reconcileExistingRelease reconciles an existing Helm release without catalog access.
211+
// This is used when the catalog is unavailable but we need to maintain the current installation.
212+
// It reconciles the release to actively maintain resources, and sets up watchers for monitoring/observability.
213+
func (h *Helm) reconcileExistingRelease(ctx context.Context, ac helmclient.ActionInterface, ext *ocv1.ClusterExtension) (bool, string, error) {
214+
rel, err := ac.Get(ext.GetName())
215+
if errors.Is(err, driver.ErrReleaseNotFound) {
216+
return false, "", fmt.Errorf("cannot maintain workload: no catalog content available and no previously installed Helm release found")
217+
}
218+
if err != nil {
219+
return false, "", fmt.Errorf("getting current release: %w", err)
220+
}
221+
222+
// Reconcile the existing release to ensure resources are maintained
223+
if err := ac.Reconcile(rel); err != nil {
224+
// Reconcile failed - resources NOT maintained
225+
// Return false (rollout failed) with error
226+
return false, "", err
227+
}
228+
229+
// At this point: Reconcile succeeded - resources ARE maintained
230+
// The operations below are for setting up monitoring (watches).
231+
// If they fail, the resources are still successfully reconciled and maintained,
232+
// so we return true (rollout succeeded) and log the watch error instead of returning it.
233+
logger := klog.FromContext(ctx)
234+
235+
relObjects, err := util.ManifestObjects(strings.NewReader(rel.Manifest), fmt.Sprintf("%s-release-manifest", rel.Name))
236+
if err != nil {
237+
logger.Error(err, "failed to parse manifest objects for watching, resources are maintained but not being watched")
238+
return true, "", nil
239+
}
240+
241+
logger.Info("watching managed objects")
242+
243+
// Defensive nil checks to prevent panics if Manager or Watcher not properly initialized
244+
if h.Manager == nil {
245+
logger.Error(nil, "manager is nil, cannot set up watches (resources are maintained but not being watched)")
246+
return true, "", nil
247+
}
248+
cache, err := h.Manager.Get(ctx, ext)
249+
if err != nil {
250+
logger.Error(err, "failed to get managed content cache, resources are maintained but not being watched")
251+
return true, "", nil
252+
}
253+
254+
if h.Watcher == nil {
255+
logger.Error(nil, "watcher is nil, cannot set up watches (resources are maintained but not being watched)")
256+
return true, "", nil
257+
}
258+
if err := cache.Watch(ctx, h.Watcher, relObjects...); err != nil {
259+
logger.Error(err, "failed to set up watches on managed objects, resources are maintained but not being watched")
260+
return true, "", nil
261+
}
262+
263+
return true, "", nil
264+
}
265+
200266
func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) {
201267
if h.HelmChartProvider == nil {
202268
return nil, errors.New("HelmChartProvider is nil")

internal/operator-controller/controllers/boxcutter_reconcile_steps.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func MigrateStorage(m StorageMigrator) ReconcileStepFunc {
9494
}
9595
}
9696

97-
func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) error) ReconcileStepFunc {
97+
func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (bool, string, error)) ReconcileStepFunc {
9898
return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) {
9999
l := log.FromContext(ctx)
100100
revisionAnnotations := map[string]string{
@@ -109,7 +109,8 @@ func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, e
109109
}
110110

111111
l.Info("applying bundle contents")
112-
if err := apply(ctx, state.imageFS, ext, objLbls, revisionAnnotations); err != nil {
112+
_, _, err := apply(ctx, state.imageFS, ext, objLbls, revisionAnnotations)
113+
if err != nil {
113114
// If there was an error applying the resolved bundle,
114115
// report the error via the Progressing condition.
115116
setStatusProgressing(ext, wrapErrorWithResolutionInfo(state.resolvedRevisionMetadata.BundleMetadata, err))

internal/operator-controller/controllers/boxcutter_reconcile_steps_apply_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ func TestApplyBundleWithBoxcutter(t *testing.T) {
133133
imageFS: fstest.MapFS{},
134134
}
135135

136-
stepFunc := ApplyBundleWithBoxcutter(func(_ context.Context, _ fs.FS, _ *ocv1.ClusterExtension, _, _ map[string]string) error {
137-
return nil
136+
stepFunc := ApplyBundleWithBoxcutter(func(_ context.Context, _ fs.FS, _ *ocv1.ClusterExtension, _, _ map[string]string) (bool, string, error) {
137+
return true, "", nil
138138
})
139139
result, err := stepFunc(ctx, state, ext)
140140
require.NoError(t, err)

internal/operator-controller/controllers/clusterextension_admission_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ import (
1313
)
1414

1515
func TestClusterExtensionSourceConfig(t *testing.T) {
16-
sourceTypeEmptyError := "Invalid value: null"
16+
// NOTE: Kubernetes validation error format for JSON null values varies across K8s versions.
17+
// We check for the common part "Invalid value:" which appears in all versions.
18+
sourceTypeEmptyError := "Invalid value:"
1719
sourceTypeMismatchError := "spec.source.sourceType: Unsupported value"
1820
sourceConfigInvalidError := "spec.source: Invalid value"
1921
// unionField represents the required Catalog or (future) Bundle field required by SourceConfig

internal/operator-controller/controllers/clusterextension_controller.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req
168168

169169
// ensureAllConditionsWithReason checks that all defined condition types exist in the given ClusterExtension,
170170
// and assigns a specified reason and custom message to any missing condition.
171+
//
172+
//nolint:unparam // reason parameter is designed to be flexible, even if current callers use the same value
171173
func ensureAllConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.ConditionReason, message string) {
172174
for _, condType := range conditionsets.ConditionTypes {
173175
cond := apimeta.FindStatusCondition(ext.Status.Conditions, condType)

0 commit comments

Comments
 (0)