diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 7425c7b66..81acf1e73 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -50,7 +50,6 @@ import ( crcache "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/certwatcher" "sigs.k8s.io/controller-runtime/pkg/client" - crcontroller "sigs.k8s.io/controller-runtime/pkg/controller" crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log" @@ -68,6 +67,7 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/cache" catalogclient "github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager" + cmcache "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager/cache" "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" "github.com/operator-framework/operator-controller/internal/operator-controller/features" "github.com/operator-framework/operator-controller/internal/operator-controller/finalizers" @@ -108,6 +108,31 @@ type config struct { globalPullSecret string } +type clusterExtensionReconcilerConfigurator interface { + Configure(cer *controllers.ClusterExtensionReconciler) error +} + +type boxcutterCERConfigurator struct { + mgr manager.Manager + preflights []applier.Preflight + regv1ManifestProvider applier.ManifestProvider + resolver resolve.Resolver + imageCache imageutil.Cache + imagePuller imageutil.Puller + finalizers crfinalizer.Finalizers +} + +type helmCERConfigurator struct { + mgr manager.Manager + preflights []applier.Preflight + regv1ManifestProvider applier.ManifestProvider + resolver resolve.Resolver + imageCache imageutil.Cache + imagePuller imageutil.Puller + finalizers crfinalizer.Finalizers + watcher cmcache.Watcher +} + const ( authFilePrefix = "operator-controller-global-pull-secrets" fieldOwnerPrefix = "olm.operatorframework.io" @@ -440,11 +465,7 @@ func run() error { } ceReconciler := &controllers.ClusterExtensionReconciler{ - Client: cl, - Resolver: resolver, - ImageCache: imageCache, - ImagePuller: imagePuller, - Finalizers: clusterExtensionFinalizers, + Client: cl, } ceController, err := ceReconciler.SetupWithManager(mgr, ctrlBuilderOpts...) if err != nil { @@ -459,13 +480,30 @@ func run() error { IsWebhookSupportEnabled: certProvider != nil, IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport), } - + var cerCfg clusterExtensionReconcilerConfigurator if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { - err = setupBoxcutter(mgr, ceReconciler, preflights, clusterExtensionFinalizers, regv1ManifestProvider) + cerCfg = &boxcutterCERConfigurator{ + mgr: mgr, + preflights: preflights, + regv1ManifestProvider: regv1ManifestProvider, + resolver: resolver, + imageCache: imageCache, + imagePuller: imagePuller, + finalizers: clusterExtensionFinalizers, + } } else { - err = setupHelm(mgr, ceReconciler, preflights, ceController, clusterExtensionFinalizers, regv1ManifestProvider) + cerCfg = &helmCERConfigurator{ + mgr: mgr, + preflights: preflights, + regv1ManifestProvider: regv1ManifestProvider, + resolver: resolver, + imageCache: imageCache, + imagePuller: imagePuller, + finalizers: clusterExtensionFinalizers, + watcher: ceController, + } } - if err != nil { + if err := cerCfg.Configure(ceReconciler); err != nil { setupLog.Error(err, "unable to setup lifecycler") return err } @@ -524,19 +562,13 @@ func getCertificateProvider() render.CertificateProvider { return nil } -func setupBoxcutter( - mgr manager.Manager, - ceReconciler *controllers.ClusterExtensionReconciler, - preflights []applier.Preflight, - clusterExtensionFinalizers crfinalizer.Registerer, - regv1ManifestProvider applier.ManifestProvider, -) error { - coreClient, err := corev1client.NewForConfig(mgr.GetConfig()) +func (c *boxcutterCERConfigurator) Configure(ceReconciler *controllers.ClusterExtensionReconciler) error { + coreClient, err := corev1client.NewForConfig(c.mgr.GetConfig()) if err != nil { return fmt.Errorf("unable to create core client: %w", err) } - cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(), - helmclient.StorageDriverMapper(action.ChunkedStorageDriverMapper(coreClient, mgr.GetAPIReader(), cfg.systemNamespace)), + cfgGetter, err := helmclient.NewActionConfigGetter(c.mgr.GetConfig(), c.mgr.GetRESTMapper(), + helmclient.StorageDriverMapper(action.ChunkedStorageDriverMapper(coreClient, c.mgr.GetAPIReader(), cfg.systemNamespace)), helmclient.ClientNamespaceMapper(func(obj client.Object) (string, error) { ext := obj.(*ocv1.ClusterExtension) return ext.Spec.Namespace, nil @@ -557,7 +589,7 @@ func setupBoxcutter( // This finalizer was added by the Helm applier for ClusterExtensions created // before BoxcutterRuntime was enabled. Boxcutter doesn't use contentmanager, // so we just need to acknowledge the finalizer to allow deletion to proceed. - err = clusterExtensionFinalizers.Register(controllers.ClusterExtensionCleanupContentManagerCacheFinalizer, finalizers.FinalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { + err = c.finalizers.Register(controllers.ClusterExtensionCleanupContentManagerCacheFinalizer, finalizers.FinalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { // No-op: Boxcutter doesn't use contentmanager, so no cleanup is needed return crfinalizer.Result{}, nil })) @@ -568,27 +600,35 @@ func setupBoxcutter( // TODO: add support for preflight checks // TODO: better scheme handling - which types do we want to support? - _ = apiextensionsv1.AddToScheme(mgr.GetScheme()) + _ = apiextensionsv1.AddToScheme(c.mgr.GetScheme()) rg := &applier.SimpleRevisionGenerator{ - Scheme: mgr.GetScheme(), - ManifestProvider: regv1ManifestProvider, + Scheme: c.mgr.GetScheme(), + ManifestProvider: c.regv1ManifestProvider, } - ceReconciler.Applier = &applier.Boxcutter{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + appl := &applier.Boxcutter{ + Client: c.mgr.GetClient(), + Scheme: c.mgr.GetScheme(), RevisionGenerator: rg, - Preflights: preflights, + Preflights: c.preflights, FieldOwner: fmt.Sprintf("%s/clusterextension-controller", fieldOwnerPrefix), } - ceReconciler.RevisionStatesGetter = &controllers.BoxcutterRevisionStatesGetter{Reader: mgr.GetClient()} - ceReconciler.StorageMigrator = &applier.BoxcutterStorageMigrator{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + revisionStatesGetter := &controllers.BoxcutterRevisionStatesGetter{Reader: c.mgr.GetClient()} + storageMigrator := &applier.BoxcutterStorageMigrator{ + Client: c.mgr.GetClient(), + Scheme: c.mgr.GetScheme(), ActionClientGetter: acg, RevisionGenerator: rg, } + ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{ + controllers.HandleFinalizers(c.finalizers), + controllers.MigrateStorage(storageMigrator), + controllers.RetrieveRevisionStates(revisionStatesGetter), + controllers.RetrieveRevisionMetadata(c.resolver), + controllers.UnpackBundle(c.imagePuller, c.imageCache), + controllers.ApplyBundle(appl), + } - baseDiscoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig()) + baseDiscoveryClient, err := discovery.NewDiscoveryClientForConfig(c.mgr.GetConfig()) if err != nil { return fmt.Errorf("unable to create discovery client: %w", err) } @@ -598,48 +638,41 @@ func setupBoxcutter( trackingCache, err := managedcache.NewTrackingCache( ctrl.Log.WithName("trackingCache"), - mgr.GetConfig(), + c.mgr.GetConfig(), crcache.Options{ - Scheme: mgr.GetScheme(), Mapper: mgr.GetRESTMapper(), + Scheme: c.mgr.GetScheme(), Mapper: c.mgr.GetRESTMapper(), }, ) if err != nil { return fmt.Errorf("unable to create boxcutter tracking cache: %v", err) } - if err := mgr.Add(trackingCache); err != nil { + if err := c.mgr.Add(trackingCache); err != nil { return fmt.Errorf("unable to add tracking cache to manager: %v", err) } if err = (&controllers.ClusterExtensionRevisionReconciler{ - Client: mgr.GetClient(), + Client: c.mgr.GetClient(), RevisionEngine: machinery.NewRevisionEngine( machinery.NewPhaseEngine( machinery.NewObjectEngine( - mgr.GetScheme(), trackingCache, mgr.GetClient(), - ownerhandling.NewNative(mgr.GetScheme()), - machinery.NewComparator(ownerhandling.NewNative(mgr.GetScheme()), discoveryClient, mgr.GetScheme(), fieldOwnerPrefix), + c.mgr.GetScheme(), trackingCache, c.mgr.GetClient(), + ownerhandling.NewNative(c.mgr.GetScheme()), + machinery.NewComparator(ownerhandling.NewNative(c.mgr.GetScheme()), discoveryClient, c.mgr.GetScheme(), fieldOwnerPrefix), fieldOwnerPrefix, fieldOwnerPrefix, ), - validation.NewClusterPhaseValidator(mgr.GetRESTMapper(), mgr.GetClient()), + validation.NewClusterPhaseValidator(c.mgr.GetRESTMapper(), c.mgr.GetClient()), ), - validation.NewRevisionValidator(), mgr.GetClient(), + validation.NewRevisionValidator(), c.mgr.GetClient(), ), TrackingCache: trackingCache, - }).SetupWithManager(mgr); err != nil { + }).SetupWithManager(c.mgr); err != nil { return fmt.Errorf("unable to setup ClusterExtensionRevision controller: %w", err) } return nil } -func setupHelm( - mgr manager.Manager, - ceReconciler *controllers.ClusterExtensionReconciler, - preflights []applier.Preflight, - ceController crcontroller.Controller, - clusterExtensionFinalizers crfinalizer.Registerer, - regv1ManifestProvider applier.ManifestProvider, -) error { - coreClient, err := corev1client.NewForConfig(mgr.GetConfig()) +func (c *helmCERConfigurator) Configure(ceReconciler *controllers.ClusterExtensionReconciler) error { + coreClient, err := corev1client.NewForConfig(c.mgr.GetConfig()) if err != nil { return fmt.Errorf("unable to create core client: %w", err) } @@ -649,8 +682,8 @@ func setupHelm( clientRestConfigMapper = action.SyntheticUserRestConfigMapper(clientRestConfigMapper) } - cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(), - helmclient.StorageDriverMapper(action.ChunkedStorageDriverMapper(coreClient, mgr.GetAPIReader(), cfg.systemNamespace)), + cfgGetter, err := helmclient.NewActionConfigGetter(c.mgr.GetConfig(), c.mgr.GetRESTMapper(), + helmclient.StorageDriverMapper(action.ChunkedStorageDriverMapper(coreClient, c.mgr.GetAPIReader(), cfg.systemNamespace)), helmclient.ClientNamespaceMapper(func(obj client.Object) (string, error) { ext := obj.(*ocv1.ClusterExtension) return ext.Spec.Namespace, nil @@ -671,11 +704,11 @@ func setupHelm( // determine if PreAuthorizer should be enabled based on feature gate var preAuth authorization.PreAuthorizer if features.OperatorControllerFeatureGate.Enabled(features.PreflightPermissions) { - preAuth = authorization.NewRBACPreAuthorizer(mgr.GetClient()) + preAuth = authorization.NewRBACPreAuthorizer(c.mgr.GetClient()) } - cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()) - err = clusterExtensionFinalizers.Register(controllers.ClusterExtensionCleanupContentManagerCacheFinalizer, finalizers.FinalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { + cm := contentmanager.NewManager(clientRestConfigMapper, c.mgr.GetConfig(), c.mgr.GetRESTMapper()) + err = c.finalizers.Register(controllers.ClusterExtensionCleanupContentManagerCacheFinalizer, finalizers.FinalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { ext := obj.(*ocv1.ClusterExtension) err := cm.Delete(ext) return crfinalizer.Result{}, err @@ -686,18 +719,26 @@ func setupHelm( } // now initialize the helmApplier, assigning the potentially nil preAuth - ceReconciler.Applier = &applier.Helm{ + appl := &applier.Helm{ ActionClientGetter: acg, - Preflights: preflights, + Preflights: c.preflights, HelmChartProvider: &applier.RegistryV1HelmChartProvider{ - ManifestProvider: regv1ManifestProvider, + ManifestProvider: c.regv1ManifestProvider, }, HelmReleaseToObjectsConverter: &applier.HelmReleaseToObjectsConverter{}, PreAuthorizer: preAuth, - Watcher: ceController, + Watcher: c.watcher, Manager: cm, } - ceReconciler.RevisionStatesGetter = &controllers.HelmRevisionStatesGetter{ActionClientGetter: acg} + revisionStatesGetter := &controllers.HelmRevisionStatesGetter{ActionClientGetter: acg} + ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{ + controllers.HandleFinalizers(c.finalizers), + controllers.RetrieveRevisionStates(revisionStatesGetter), + controllers.RetrieveRevisionMetadata(c.resolver), + controllers.UnpackBundle(c.imagePuller, c.imageCache), + controllers.ApplyBundle(appl), + } + return nil } diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 4e7026894..22ed096fe 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -21,13 +21,13 @@ import ( apimachyaml "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" - crcontroller "sigs.k8s.io/controller-runtime/pkg/controller" helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/authorization" "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager" + "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager/cache" "github.com/operator-framework/operator-controller/internal/operator-controller/features" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" @@ -65,7 +65,7 @@ type Helm struct { HelmReleaseToObjectsConverter HelmReleaseToObjectsConverterInterface Manager contentmanager.Manager - Watcher crcontroller.Controller + Watcher cache.Watcher } // runPreAuthorizationChecks performs pre-authorization checks for a Helm release diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go new file mode 100644 index 000000000..78e3ef132 --- /dev/null +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go @@ -0,0 +1,95 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "cmp" + "context" + "fmt" + "slices" + + apimeta "k8s.io/apimachinery/pkg/api/meta" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/labels" +) + +type BoxcutterRevisionStatesGetter struct { + Reader client.Reader +} + +func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, ext *ocv1.ClusterExtension) (*RevisionStates, error) { + // TODO: boxcutter applier has a nearly identical bit of code for listing and sorting revisions + // only difference here is that it sorts in reverse order to start iterating with the most + // recent revisions. We should consolidate to avoid code duplication. + existingRevisionList := &ocv1.ClusterExtensionRevisionList{} + if err := d.Reader.List(ctx, existingRevisionList, client.MatchingLabels{ + labels.OwnerNameKey: ext.Name, + }); err != nil { + return nil, fmt.Errorf("listing revisions: %w", err) + } + slices.SortFunc(existingRevisionList.Items, func(a, b ocv1.ClusterExtensionRevision) int { + return cmp.Compare(a.Spec.Revision, b.Spec.Revision) + }) + + rs := &RevisionStates{} + for _, rev := range existingRevisionList.Items { + switch rev.Spec.LifecycleState { + case ocv1.ClusterExtensionRevisionLifecycleStateActive, + ocv1.ClusterExtensionRevisionLifecycleStatePaused: + default: + // Skip anything not active or paused, which should only be "Archived". + continue + } + + // TODO: the setting of these annotations (happens in boxcutter applier when we pass in "revisionAnnotations") + // is fairly decoupled from this code where we get the annotations back out. We may want to co-locate + // the set/get logic a bit better to make it more maintainable and less likely to get out of sync. + rm := &RevisionMetadata{ + Package: rev.Annotations[labels.PackageNameKey], + Image: rev.Annotations[labels.BundleReferenceKey], + BundleMetadata: ocv1.BundleMetadata{ + Name: rev.Annotations[labels.BundleNameKey], + Version: rev.Annotations[labels.BundleVersionKey], + }, + } + + if apimeta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) { + rs.Installed = rm + } else { + rs.RollingOut = append(rs.RollingOut, rm) + } + } + + return rs, nil +} + +func MigrateStorage(m StorageMigrator) ReconcileStepFunc { + return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { + objLbls := map[string]string{ + labels.OwnerKindKey: ocv1.ClusterExtensionKind, + labels.OwnerNameKey: ext.GetName(), + } + + if err := m.Migrate(ctx, ext, objLbls); err != nil { + return nil, fmt.Errorf("migrating storage: %w", err) + } + return nil, nil + } +} diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index d8d9d8de0..396755b8f 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -17,12 +17,10 @@ limitations under the License. package controllers import ( - "cmp" "context" "errors" "fmt" "io/fs" - "slices" "strings" "github.com/go-logr/logr" @@ -38,7 +36,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" crcontroller "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" - crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer" crhandler "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -49,12 +46,8 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/operator-controller/authentication" - "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" "github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets" "github.com/operator-framework/operator-controller/internal/operator-controller/labels" - "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" - imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" ) const ( @@ -62,18 +55,48 @@ const ( ClusterExtensionCleanupContentManagerCacheFinalizer = "olm.operatorframework.io/cleanup-contentmanager-cache" ) +type reconcileState struct { + revisionStates *RevisionStates + resolvedRevisionMetadata *RevisionMetadata + imageFS fs.FS +} + +// ReconcileStepFunc represents a single step in the ClusterExtension reconciliation process. +// It takes a context, state and ClusterExtension object as input and returns: +// - Any error that occurred during reconciliation, which will be returned to the caller +// - A ctrl.Result that indicates whether reconciliation should complete immediately or be retried later +type ReconcileStepFunc func(context.Context, *reconcileState, *ocv1.ClusterExtension) (*ctrl.Result, error) + +// ReconcileSteps is an ordered collection of reconciliation steps that are executed sequentially. +// Each step receives the output context from the previous step, allowing data to flow through the pipeline. +type ReconcileSteps []ReconcileStepFunc + +// Reconcile executes a series of reconciliation steps in sequence for a ClusterExtension. +// It takes a context and ClusterExtension object as input and executes each step in the ReconcileSteps slice. +// If any step returns an error, reconciliation stops and the error is returned. +// If any step returns a non-nil ctrl.Result, reconciliation stops and that result is returned. +// If any step returns a nil context, reconciliation stops successfully. +// If all steps complete successfully, returns an empty ctrl.Result and nil error. +func (steps *ReconcileSteps) Reconcile(ctx context.Context, ext *ocv1.ClusterExtension) (ctrl.Result, error) { + var res *ctrl.Result + var err error + s := &reconcileState{} + for _, step := range *steps { + res, err = step(ctx, s, ext) + if err != nil { + return ctrl.Result{}, err + } + if res != nil { + return *res, nil + } + } + return ctrl.Result{}, nil +} + // ClusterExtensionReconciler reconciles a ClusterExtension object type ClusterExtensionReconciler struct { client.Client - Resolver resolve.Resolver - - ImageCache imageutil.Cache - ImagePuller imageutil.Puller - - StorageMigrator StorageMigrator - Applier Applier - RevisionStatesGetter RevisionStatesGetter - Finalizers crfinalizer.Finalizers + ReconcileSteps ReconcileSteps } type StorageMigrator interface { @@ -106,7 +129,7 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req defer l.Info("reconcile ending") reconciledExt := existingExt.DeepCopy() - res, reconcileErr := r.reconcile(ctx, reconciledExt) + res, reconcileErr := r.ReconcileSteps.Reconcile(ctx, reconciledExt) // Do checks before any Update()s, as Update() may modify the resource structure! updateStatus := !equality.Semantic.DeepEqual(existingExt.Status, reconciledExt.Status) @@ -164,169 +187,6 @@ func checkForUnexpectedClusterExtensionFieldChange(a, b ocv1.ClusterExtension) b return !equality.Semantic.DeepEqual(a, b) } -// Helper function to do the actual reconcile -// -// Today we always return ctrl.Result{} and an error. -// But in the future we might update this function -// to return different results (e.g. requeue). -// -/* The reconcile functions performs the following major tasks: -1. Resolution: Run the resolution to find the bundle from the catalog which needs to be installed. -2. Validate: Ensure that the bundle returned from the resolution for install meets our requirements. -3. Unpack: Unpack the contents from the bundle and store in a localdir in the pod. -4. Install: The process of installing involves: -4.1 Converting the CSV in the bundle into a set of plain k8s objects. -4.2 Generating a chart from k8s objects. -4.3 Store the release on cluster. -*/ -//nolint:unparam -func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.ClusterExtension) (ctrl.Result, error) { - l := log.FromContext(ctx) - - l.Info("handling finalizers") - finalizeResult, err := r.Finalizers.Finalize(ctx, ext) - if err != nil { - setStatusProgressing(ext, err) - return ctrl.Result{}, err - } - if finalizeResult.Updated || finalizeResult.StatusUpdated { - // On create: make sure the finalizer is applied before we do anything - // On delete: make sure we do nothing after the finalizer is removed - return ctrl.Result{}, nil - } - - if ext.GetDeletionTimestamp() != nil { - // If we've gotten here, that means the cluster extension is being deleted, we've handled all of - // _our_ finalizers (above), but the cluster extension is still present in the cluster, likely - // because there are _other_ finalizers that other controllers need to handle, (e.g. the orphan - // deletion finalizer). - return ctrl.Result{}, nil - } - - objLbls := map[string]string{ - labels.OwnerKindKey: ocv1.ClusterExtensionKind, - labels.OwnerNameKey: ext.GetName(), - } - - if r.StorageMigrator != nil { - if err := r.StorageMigrator.Migrate(ctx, ext, objLbls); err != nil { - return ctrl.Result{}, fmt.Errorf("migrating storage: %w", err) - } - } - - l.Info("getting installed bundle") - revisionStates, err := r.RevisionStatesGetter.GetRevisionStates(ctx, ext) - if err != nil { - setInstallStatus(ext, nil) - var saerr *authentication.ServiceAccountNotFoundError - if errors.As(err, &saerr) { - setInstalledStatusConditionUnknown(ext, saerr.Error()) - setStatusProgressing(ext, errors.New("installation cannot proceed due to missing ServiceAccount")) - return ctrl.Result{}, err - } - setInstalledStatusConditionUnknown(ext, err.Error()) - setStatusProgressing(ext, errors.New("retrying to get installed bundle")) - return ctrl.Result{}, err - } - - var resolvedRevisionMetadata *RevisionMetadata - if len(revisionStates.RollingOut) == 0 { - l.Info("resolving bundle") - var bm *ocv1.BundleMetadata - if revisionStates.Installed != nil { - bm = &revisionStates.Installed.BundleMetadata - } - resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, ext, bm) - if err != nil { - // Note: We don't distinguish between resolution-specific errors and generic errors - setStatusProgressing(ext, err) - setInstalledStatusFromRevisionStates(ext, revisionStates) - ensureAllConditionsWithReason(ext, ocv1.ReasonFailed, err.Error()) - return ctrl.Result{}, err - } - - // set deprecation status after _successful_ resolution - // TODO: - // 1. It seems like deprecation status should reflect the currently installed bundle, not the resolved - // bundle. So perhaps we should set package and channel deprecations directly after resolution, but - // defer setting the bundle deprecation until we successfully install the bundle. - // 2. If resolution fails because it can't find a bundle, that doesn't mean we wouldn't be able to find - // a deprecation for the ClusterExtension's spec.packageName. Perhaps we should check for a non-nil - // resolvedDeprecation even if resolution returns an error. If present, we can still update some of - // our deprecation status. - // - Open question though: what if different catalogs have different opinions of what's deprecated. - // If we can't resolve a bundle, how do we know which catalog to trust for deprecation information? - // Perhaps if the package shows up in multiple catalogs and deprecations don't match, we can set - // the deprecation status to unknown? Or perhaps we somehow combine the deprecation information from - // all catalogs? - SetDeprecationStatus(ext, resolvedBundle.Name, resolvedDeprecation) - resolvedRevisionMetadata = &RevisionMetadata{ - Package: resolvedBundle.Package, - Image: resolvedBundle.Image, - BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion), - } - } else { - resolvedRevisionMetadata = revisionStates.RollingOut[0] - } - - l.Info("unpacking resolved bundle") - imageFS, _, _, err := r.ImagePuller.Pull(ctx, ext.GetName(), resolvedRevisionMetadata.Image, r.ImageCache) - if err != nil { - // Wrap the error passed to this with the resolution information until we have successfully - // installed since we intend for the progressing condition to replace the resolved condition - // and will be removing the .status.resolution field from the ClusterExtension status API - setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedRevisionMetadata.BundleMetadata, err)) - setInstalledStatusFromRevisionStates(ext, revisionStates) - return ctrl.Result{}, err - } - - // The following values will be stored as annotations and not labels - revisionAnnotations := map[string]string{ - labels.BundleNameKey: resolvedRevisionMetadata.Name, - labels.PackageNameKey: resolvedRevisionMetadata.Package, - labels.BundleVersionKey: resolvedRevisionMetadata.Version, - labels.BundleReferenceKey: resolvedRevisionMetadata.Image, - } - - l.Info("applying bundle contents") - // NOTE: We need to be cautious of eating errors here. - // We should always return any error that occurs during an - // attempt to apply content to the cluster. Only when there is - // a verifiable reason to eat the error (i.e it is recoverable) - // should an exception be made. - // The following kinds of errors should be returned up the stack - // to ensure exponential backoff can occur: - // - Permission errors (it is not possible to watch changes to permissions. - // The only way to eventually recover from permission errors is to keep retrying). - rolloutSucceeded, rolloutStatus, err := r.Applier.Apply(ctx, imageFS, ext, objLbls, revisionAnnotations) - - // Set installed status - if rolloutSucceeded { - revisionStates = &RevisionStates{Installed: resolvedRevisionMetadata} - } else if err == nil && revisionStates.Installed == nil && len(revisionStates.RollingOut) == 0 { - revisionStates = &RevisionStates{RollingOut: []*RevisionMetadata{resolvedRevisionMetadata}} - } - setInstalledStatusFromRevisionStates(ext, revisionStates) - - // If there was an error applying the resolved bundle, - // report the error via the Progressing condition. - if err != nil { - setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedRevisionMetadata.BundleMetadata, err)) - return ctrl.Result{}, err - } else if !rolloutSucceeded { - apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ - Type: ocv1.TypeProgressing, - Status: metav1.ConditionTrue, - Reason: ocv1.ReasonRolloutInProgress, - Message: rolloutStatus, - ObservedGeneration: ext.GetGeneration(), - }) - } else { - setStatusProgressing(ext, nil) - } - return ctrl.Result{}, nil -} - // SetDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension // based on the provided bundle func SetDeprecationStatus(ext *ocv1.ClusterExtension, bundleName string, deprecation *declcfg.Deprecation) { @@ -521,53 +381,3 @@ func (d *HelmRevisionStatesGetter) GetRevisionStates(ctx context.Context, ext *o } return rs, nil } - -type BoxcutterRevisionStatesGetter struct { - Reader client.Reader -} - -func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, ext *ocv1.ClusterExtension) (*RevisionStates, error) { - // TODO: boxcutter applier has a nearly identical bit of code for listing and sorting revisions - // only difference here is that it sorts in reverse order to start iterating with the most - // recent revisions. We should consolidate to avoid code duplication. - existingRevisionList := &ocv1.ClusterExtensionRevisionList{} - if err := d.Reader.List(ctx, existingRevisionList, client.MatchingLabels{ - labels.OwnerNameKey: ext.Name, - }); err != nil { - return nil, fmt.Errorf("listing revisions: %w", err) - } - slices.SortFunc(existingRevisionList.Items, func(a, b ocv1.ClusterExtensionRevision) int { - return cmp.Compare(a.Spec.Revision, b.Spec.Revision) - }) - - rs := &RevisionStates{} - for _, rev := range existingRevisionList.Items { - switch rev.Spec.LifecycleState { - case ocv1.ClusterExtensionRevisionLifecycleStateActive, - ocv1.ClusterExtensionRevisionLifecycleStatePaused: - default: - // Skip anything not active or paused, which should only be "Archived". - continue - } - - // TODO: the setting of these annotations (happens in boxcutter applier when we pass in "revisionAnnotations") - // is fairly decoupled from this code where we get the annotations back out. We may want to co-locate - // the set/get logic a bit better to make it more maintainable and less likely to get out of sync. - rm := &RevisionMetadata{ - Package: rev.Annotations[labels.PackageNameKey], - Image: rev.Annotations[labels.BundleReferenceKey], - BundleMetadata: ocv1.BundleMetadata{ - Name: rev.Annotations[labels.BundleNameKey], - Version: rev.Annotations[labels.BundleVersionKey], - }, - } - - if apimeta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) { - rs.Installed = rm - } else { - rs.RollingOut = append(rs.RollingOut, rm) - } - } - - return rs, nil -} diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index 20761aec9..9146a1557 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -31,7 +31,7 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/authentication" "github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets" "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" - "github.com/operator-framework/operator-controller/internal/operator-controller/finalizers" + finalizers "github.com/operator-framework/operator-controller/internal/operator-controller/finalizers" "github.com/operator-framework/operator-controller/internal/operator-controller/labels" "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" @@ -49,15 +49,17 @@ func TestClusterExtensionDoesNotExist(t *testing.T) { } func TestClusterExtensionShortCircuitsReconcileDuringDeletion(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) - installedBundleGetterCalledErr := errors.New("revision states getter called") + + cl, reconciler := newClientAndReconciler(t, func(d *deps) { + d.RevisionStatesGetter = &MockRevisionStatesGetter{ + Err: installedBundleGetterCalledErr, + } + }) + checkInstalledBundleGetterCalled := func(t require.TestingT, err error, args ...interface{}) { require.Equal(t, installedBundleGetterCalledErr, err) } - reconciler.RevisionStatesGetter = &MockRevisionStatesGetter{ - Err: installedBundleGetterCalledErr, - } type testCase struct { name string @@ -123,10 +125,12 @@ func TestClusterExtensionShortCircuitsReconcileDuringDeletion(t *testing.T) { func TestClusterExtensionResolutionFails(t *testing.T) { pkgName := fmt.Sprintf("non-existent-%s", rand.String(6)) - cl, reconciler := newClientAndReconciler(t) - reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { - return nil, nil, nil, fmt.Errorf("no package %q found", pkgName) + cl, reconciler := newClientAndReconciler(t, func(d *deps) { + d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + return nil, nil, nil, fmt.Errorf("no package %q found", pkgName) + }) }) + ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -190,11 +194,6 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) - reconciler.ImagePuller = &imageutil.MockPuller{ - Error: tc.pullErr, - } - ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -223,19 +222,30 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) { }, }, } + cl, reconciler := newClientAndReconciler(t, + func(d *deps) { + d.ImagePuller = &imageutil.MockPuller{ + Error: tc.pullErr, + } + }, + func(d *deps) { + d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + v := bsemver.MustParse("1.0.0") + return &declcfg.Bundle{ + Name: "prometheus.v1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + }, &v, nil, nil + }) + }, + ) + err := cl.Create(ctx, clusterExtension) require.NoError(t, err) t.Log("It sets resolution success status") t.Log("By running reconcile") - reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { - v := bsemver.MustParse("1.0.0") - return &declcfg.Bundle{ - Name: "prometheus.v1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - }, &v, nil, nil - }) + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Equal(t, ctrl.Result{}, res) require.Error(t, err) @@ -270,10 +280,23 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) { } func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) - reconciler.ImagePuller = &imageutil.MockPuller{ - ImageFS: fstest.MapFS{}, - } + cl, reconciler := newClientAndReconciler(t, + func(d *deps) { + d.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + v := bsemver.MustParse("1.0.0") + return &declcfg.Bundle{ + Name: "prometheus.v1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + }, &v, nil, nil + }) + d.Applier = &MockApplier{ + err: errors.New("apply failure"), + } + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -308,17 +331,7 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T) t.Log("It sets resolution success status") t.Log("By running reconcile") - reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { - v := bsemver.MustParse("1.0.0") - return &declcfg.Bundle{ - Name: "prometheus.v1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - }, &v, nil, nil - }) - reconciler.Applier = &MockApplier{ - err: errors.New("apply failure"), - } + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Equal(t, ctrl.Result{}, res) require.Error(t, err) @@ -347,12 +360,13 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T) } func TestClusterExtensionServiceAccountNotFound(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) - reconciler.RevisionStatesGetter = &MockRevisionStatesGetter{ - Err: &authentication.ServiceAccountNotFoundError{ - ServiceAccountName: "missing-sa", - ServiceAccountNamespace: "default", - }} + cl, reconciler := newClientAndReconciler(t, func(d *deps) { + d.RevisionStatesGetter = &MockRevisionStatesGetter{ + Err: &authentication.ServiceAccountNotFoundError{ + ServiceAccountName: "missing-sa", + ServiceAccountNamespace: "default", + }} + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -402,10 +416,32 @@ func TestClusterExtensionServiceAccountNotFound(t *testing.T) { } func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) - reconciler.ImagePuller = &imageutil.MockPuller{ - ImageFS: fstest.MapFS{}, + mockApplier := &MockApplier{ + installCompleted: true, } + cl, reconciler := newClientAndReconciler(t, func(d *deps) { + d.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + v := bsemver.MustParse("1.0.0") + return &declcfg.Bundle{ + Name: "prometheus.v1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + }, &v, nil, nil + }) + + d.RevisionStatesGetter = &MockRevisionStatesGetter{ + RevisionStates: &controllers.RevisionStates{ + Installed: &controllers.RevisionMetadata{ + BundleMetadata: ocv1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + }, + }, + } + d.Applier = mockApplier + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -440,34 +476,13 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) { t.Log("It sets resolution success status") t.Log("By running reconcile") - reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { - v := bsemver.MustParse("1.0.0") - return &declcfg.Bundle{ - Name: "prometheus.v1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - }, &v, nil, nil - }) - - reconciler.RevisionStatesGetter = &MockRevisionStatesGetter{ - RevisionStates: &controllers.RevisionStates{ - Installed: &controllers.RevisionMetadata{ - BundleMetadata: ocv1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - }, - }, - } - reconciler.Applier = &MockApplier{ - installCompleted: true, - } res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Equal(t, ctrl.Result{}, res) require.NoError(t, err) - reconciler.Applier = &MockApplier{ - err: errors.New("apply failure"), - } + mockApplier.installCompleted = false + mockApplier.err = errors.New("apply failure") res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Equal(t, ctrl.Result{}, res) @@ -497,10 +512,23 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) { } func TestClusterExtensionManagerFailed(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) - reconciler.ImagePuller = &imageutil.MockPuller{ - ImageFS: fstest.MapFS{}, - } + cl, reconciler := newClientAndReconciler(t, func(d *deps) { + d.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + v := bsemver.MustParse("1.0.0") + return &declcfg.Bundle{ + Name: "prometheus.v1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + }, &v, nil, nil + }) + d.Applier = &MockApplier{ + installCompleted: true, + err: errors.New("manager fail"), + } + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -535,18 +563,6 @@ func TestClusterExtensionManagerFailed(t *testing.T) { t.Log("It sets resolution success status") t.Log("By running reconcile") - reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { - v := bsemver.MustParse("1.0.0") - return &declcfg.Bundle{ - Name: "prometheus.v1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - }, &v, nil, nil - }) - reconciler.Applier = &MockApplier{ - installCompleted: true, - err: errors.New("manager fail"), - } res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Equal(t, ctrl.Result{}, res) require.Error(t, err) @@ -573,10 +589,23 @@ func TestClusterExtensionManagerFailed(t *testing.T) { } func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) - reconciler.ImagePuller = &imageutil.MockPuller{ - ImageFS: fstest.MapFS{}, - } + cl, reconciler := newClientAndReconciler(t, func(d *deps) { + d.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + v := bsemver.MustParse("1.0.0") + return &declcfg.Bundle{ + Name: "prometheus.v1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + }, &v, nil, nil + }) + d.Applier = &MockApplier{ + installCompleted: true, + err: errors.New("watch error"), + } + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -612,18 +641,7 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) { t.Log("It sets resolution success status") t.Log("By running reconcile") - reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { - v := bsemver.MustParse("1.0.0") - return &declcfg.Bundle{ - Name: "prometheus.v1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - }, &v, nil, nil - }) - reconciler.Applier = &MockApplier{ - installCompleted: true, - err: errors.New("watch error"), - } + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Equal(t, ctrl.Result{}, res) require.Error(t, err) @@ -650,10 +668,22 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) { } func TestClusterExtensionInstallationSucceeds(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) - reconciler.ImagePuller = &imageutil.MockPuller{ - ImageFS: fstest.MapFS{}, - } + cl, reconciler := newClientAndReconciler(t, func(d *deps) { + d.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + v := bsemver.MustParse("1.0.0") + return &declcfg.Bundle{ + Name: "prometheus.v1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + }, &v, nil, nil + }) + d.Applier = &MockApplier{ + installCompleted: true, + } + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -688,17 +718,6 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) { t.Log("It sets resolution success status") t.Log("By running reconcile") - reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { - v := bsemver.MustParse("1.0.0") - return &declcfg.Bundle{ - Name: "prometheus.v1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - }, &v, nil, nil - }) - reconciler.Applier = &MockApplier{ - installCompleted: true, - } res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Equal(t, ctrl.Result{}, res) require.NoError(t, err) @@ -725,10 +744,34 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) { } func TestClusterExtensionDeleteFinalizerFails(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) - reconciler.ImagePuller = &imageutil.MockPuller{ - ImageFS: fstest.MapFS{}, - } + fakeFinalizer := "fake.testfinalizer.io" + finalizersMessage := "still have finalizers" + var rfinalizers crfinalizer.Finalizers + cl, reconciler := newClientAndReconciler(t, func(d *deps) { + d.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + v := bsemver.MustParse("1.0.0") + return &declcfg.Bundle{ + Name: "prometheus.v1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + }, &v, nil, nil + }) + d.Applier = &MockApplier{ + installCompleted: true, + } + d.RevisionStatesGetter = &MockRevisionStatesGetter{ + RevisionStates: &controllers.RevisionStates{ + Installed: &controllers.RevisionMetadata{ + BundleMetadata: ocv1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + }, + }, + } + rfinalizers = d.Finalizers + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -762,28 +805,7 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) { require.NoError(t, err) t.Log("It sets resolution success status") t.Log("By running reconcile") - reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { - v := bsemver.MustParse("1.0.0") - return &declcfg.Bundle{ - Name: "prometheus.v1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - }, &v, nil, nil - }) - fakeFinalizer := "fake.testfinalizer.io" - finalizersMessage := "still have finalizers" - reconciler.Applier = &MockApplier{ - installCompleted: true, - } - reconciler.RevisionStatesGetter = &MockRevisionStatesGetter{ - RevisionStates: &controllers.RevisionStates{ - Installed: &controllers.RevisionMetadata{ - BundleMetadata: ocv1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - }, - }, - } - err = reconciler.Finalizers.Register(fakeFinalizer, finalizers.FinalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { + err = rfinalizers.Register(fakeFinalizer, finalizers.FinalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { return crfinalizer.Result{}, errors.New(finalizersMessage) })) diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go new file mode 100644 index 000000000..954454e66 --- /dev/null +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -0,0 +1,203 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "errors" + + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/finalizer" + "sigs.k8s.io/controller-runtime/pkg/log" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/authentication" + "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" + "github.com/operator-framework/operator-controller/internal/operator-controller/labels" + "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" + imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" +) + +func HandleFinalizers(f finalizer.Finalizer) ReconcileStepFunc { + return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { + l := log.FromContext(ctx) + + l.Info("handling finalizers") + finalizeResult, err := f.Finalize(ctx, ext) + if err != nil { + setStatusProgressing(ext, err) + return nil, err + } + if finalizeResult.Updated || finalizeResult.StatusUpdated { + // On create: make sure the finalizer is applied before we do anything + // On delete: make sure we do nothing after the finalizer is removed + return &ctrl.Result{}, nil + } + + if ext.GetDeletionTimestamp() != nil { + // If we've gotten here, that means the cluster extension is being deleted, we've handled all of + // _our_ finalizers (above), but the cluster extension is still present in the cluster, likely + // because there are _other_ finalizers that other controllers need to handle, (e.g. the orphan + // deletion finalizer). + return &ctrl.Result{}, nil + } + return nil, nil + } +} + +func RetrieveRevisionStates(r RevisionStatesGetter) ReconcileStepFunc { + return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { + l := log.FromContext(ctx) + l.Info("getting installed bundle") + revisionStates, err := r.GetRevisionStates(ctx, ext) + if err != nil { + setInstallStatus(ext, nil) + var saerr *authentication.ServiceAccountNotFoundError + if errors.As(err, &saerr) { + setInstalledStatusConditionUnknown(ext, saerr.Error()) + setStatusProgressing(ext, errors.New("installation cannot proceed due to missing ServiceAccount")) + return nil, err + } + setInstalledStatusConditionUnknown(ext, err.Error()) + setStatusProgressing(ext, errors.New("retrying to get installed bundle")) + return nil, err + } + state.revisionStates = revisionStates + return nil, nil + } +} + +func RetrieveRevisionMetadata(r resolve.Resolver) ReconcileStepFunc { + return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { + l := log.FromContext(ctx) + var resolvedRevisionMetadata *RevisionMetadata + if len(state.revisionStates.RollingOut) == 0 { + l.Info("resolving bundle") + var bm *ocv1.BundleMetadata + if state.revisionStates.Installed != nil { + bm = &state.revisionStates.Installed.BundleMetadata + } + resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolve(ctx, ext, bm) + if err != nil { + // Note: We don't distinguish between resolution-specific errors and generic errors + setStatusProgressing(ext, err) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + ensureAllConditionsWithReason(ext, ocv1.ReasonFailed, err.Error()) + return nil, err + } + + // set deprecation status after _successful_ resolution + // TODO: + // 1. It seems like deprecation status should reflect the currently installed bundle, not the resolved + // bundle. So perhaps we should set package and channel deprecations directly after resolution, but + // defer setting the bundle deprecation until we successfully install the bundle. + // 2. If resolution fails because it can't find a bundle, that doesn't mean we wouldn't be able to find + // a deprecation for the ClusterExtension's spec.packageName. Perhaps we should check for a non-nil + // resolvedDeprecation even if resolution returns an error. If present, we can still update some of + // our deprecation status. + // - Open question though: what if different catalogs have different opinions of what's deprecated. + // If we can't resolve a bundle, how do we know which catalog to trust for deprecation information? + // Perhaps if the package shows up in multiple catalogs and deprecations don't match, we can set + // the deprecation status to unknown? Or perhaps we somehow combine the deprecation information from + // all catalogs? + SetDeprecationStatus(ext, resolvedBundle.Name, resolvedDeprecation) + resolvedRevisionMetadata = &RevisionMetadata{ + Package: resolvedBundle.Package, + Image: resolvedBundle.Image, + BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion), + } + } else { + resolvedRevisionMetadata = state.revisionStates.RollingOut[0] + } + state.resolvedRevisionMetadata = resolvedRevisionMetadata + return nil, nil + } +} + +func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc { + return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { + l := log.FromContext(ctx) + l.Info("unpacking resolved bundle") + imageFS, _, _, err := i.Pull(ctx, ext.GetName(), state.resolvedRevisionMetadata.Image, cache) + if err != nil { + // Wrap the error passed to this with the resolution information until we have successfully + // installed since we intend for the progressing condition to replace the resolved condition + // and will be removing the .status.resolution field from the ClusterExtension status API + setStatusProgressing(ext, wrapErrorWithResolutionInfo(state.resolvedRevisionMetadata.BundleMetadata, err)) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + return nil, err + } + state.imageFS = imageFS + return nil, nil + } +} + +func ApplyBundle(a Applier) ReconcileStepFunc { + return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { + l := log.FromContext(ctx) + revisionAnnotations := map[string]string{ + labels.BundleNameKey: state.resolvedRevisionMetadata.Name, + labels.PackageNameKey: state.resolvedRevisionMetadata.Package, + labels.BundleVersionKey: state.resolvedRevisionMetadata.Version, + labels.BundleReferenceKey: state.resolvedRevisionMetadata.Image, + } + objLbls := map[string]string{ + labels.OwnerKindKey: ocv1.ClusterExtensionKind, + labels.OwnerNameKey: ext.GetName(), + } + + l.Info("applying bundle contents") + // NOTE: We need to be cautious of eating errors here. + // We should always return any error that occurs during an + // attempt to apply content to the cluster. Only when there is + // a verifiable reason to eat the error (i.e it is recoverable) + // should an exception be made. + // The following kinds of errors should be returned up the stack + // to ensure exponential backoff can occur: + // - Permission errors (it is not possible to watch changes to permissions. + // The only way to eventually recover from permission errors is to keep retrying). + rolloutSucceeded, rolloutStatus, err := a.Apply(ctx, state.imageFS, ext, objLbls, revisionAnnotations) + + // Set installed status + if rolloutSucceeded { + state.revisionStates = &RevisionStates{Installed: state.resolvedRevisionMetadata} + } else if err == nil && state.revisionStates.Installed == nil && len(state.revisionStates.RollingOut) == 0 { + state.revisionStates = &RevisionStates{RollingOut: []*RevisionMetadata{state.resolvedRevisionMetadata}} + } + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + + // If there was an error applying the resolved bundle, + // report the error via the Progressing condition. + if err != nil { + setStatusProgressing(ext, wrapErrorWithResolutionInfo(state.resolvedRevisionMetadata.BundleMetadata, err)) + return nil, err + } else if !rolloutSucceeded { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1.TypeProgressing, + Status: metav1.ConditionTrue, + Reason: ocv1.ReasonRolloutInProgress, + Message: rolloutStatus, + ObservedGeneration: ext.GetGeneration(), + }) + } else { + setStatusProgressing(ext, nil) + } + return nil, nil + } +} diff --git a/internal/operator-controller/controllers/suite_test.go b/internal/operator-controller/controllers/suite_test.go index 02d538237..529067a81 100644 --- a/internal/operator-controller/controllers/suite_test.go +++ b/internal/operator-controller/controllers/suite_test.go @@ -32,6 +32,8 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" + "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" + "github.com/operator-framework/operator-controller/internal/shared/util/image" "github.com/operator-framework/operator-controller/test" ) @@ -76,16 +78,43 @@ func (m *MockApplier) Apply(_ context.Context, _ fs.FS, _ *ocv1.ClusterExtension return m.installCompleted, m.installStatus, m.err } -func newClientAndReconciler(t *testing.T) (client.Client, *controllers.ClusterExtensionReconciler) { +type reconcilerOption func(*deps) + +type deps struct { + RevisionStatesGetter controllers.RevisionStatesGetter + Finalizers crfinalizer.Finalizers + Resolver resolve.Resolver + ImagePuller image.Puller + ImageCache image.Cache + Applier controllers.Applier +} + +func newClientAndReconciler(t *testing.T, opts ...reconcilerOption) (client.Client, *controllers.ClusterExtensionReconciler) { cl := newClient(t) - reconciler := &controllers.ClusterExtensionReconciler{ - Client: cl, + d := &deps{ RevisionStatesGetter: &MockRevisionStatesGetter{ RevisionStates: &controllers.RevisionStates{}, }, Finalizers: crfinalizer.NewFinalizers(), } + reconciler := &controllers.ClusterExtensionReconciler{ + Client: cl, + } + for _, opt := range opts { + opt(d) + } + reconciler.ReconcileSteps = []controllers.ReconcileStepFunc{controllers.HandleFinalizers(d.Finalizers), controllers.RetrieveRevisionStates(d.RevisionStatesGetter)} + if r := d.Resolver; r != nil { + reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.RetrieveRevisionMetadata(r)) + } + if i := d.ImagePuller; i != nil { + reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.UnpackBundle(i, d.ImageCache)) + } + if a := d.Applier; a != nil { + reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.ApplyBundle(a)) + } + return cl, reconciler }