diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 6de62b0e12..a10292a50e 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -470,6 +470,14 @@ type BundleMetadata struct { Version string `json:"version"` } +type RevisionStatus struct { + Name string `json:"name"` + // +listType=map + // +listMapKey=type + // +optional + Conditions []metav1.Condition `json:"conditions,omitempty"` +} + // ClusterExtensionStatus defines the observed state of a ClusterExtension. type ClusterExtensionStatus struct { // The set of condition types which apply to all spec.source variations are Installed and Progressing. @@ -499,6 +507,12 @@ type ClusterExtensionStatus struct { // // +optional Install *ClusterExtensionInstallStatus `json:"install,omitempty"` + + // +listType=map + // +listMapKey=name + // +optional + // + ActiveRevisions []RevisionStatus `json:"activeRevisions,omitempty"` } // ClusterExtensionInstallStatus is a representation of the status of the identified bundle. @@ -517,7 +531,7 @@ type ClusterExtensionInstallStatus struct { // +kubebuilder:subresource:status // +kubebuilder:printcolumn:name="Installed Bundle",type=string,JSONPath=`.status.install.bundle.name` // +kubebuilder:printcolumn:name=Version,type=string,JSONPath=`.status.install.bundle.version` -// +kubebuilder:printcolumn:name="Installed",type=string,JSONPath=`.status.conditions[?(@.type=='Installed')].status` +// +kubebuilder:printcolumn:name="Available",type=string,JSONPath=`.status.conditions[?(@.type=='Available')].status` // +kubebuilder:printcolumn:name="Progressing",type=string,JSONPath=`.status.conditions[?(@.type=='Progressing')].status` // +kubebuilder:printcolumn:name=Age,type=date,JSONPath=`.metadata.creationTimestamp` diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go index 13ac4ce2a7..0880303d07 100644 --- a/api/v1/clusterextensionrevision_types.go +++ b/api/v1/clusterextensionrevision_types.go @@ -26,8 +26,9 @@ const ( ClusterExtensionRevisionKind = "ClusterExtensionRevision" // Condition Types - ClusterExtensionRevisionTypeAvailable = "Available" - ClusterExtensionRevisionTypeSucceeded = "Succeeded" + ClusterExtensionRevisionTypeAvailable = "Available" + ClusterExtensionRevisionTypeSucceeded = "Succeeded" + ClusterExtensionRevisionTypeProgressing = "Progressing" // Condition Reasons ClusterExtensionRevisionReasonAvailable = "Available" @@ -37,9 +38,13 @@ const ( ClusterExtensionRevisionReasonObjectCollisions = "ObjectCollisions" ClusterExtensionRevisionReasonRolloutSuccess = "RolloutSuccess" ClusterExtensionRevisionReasonProbeFailure = "ProbeFailure" + ClusterExtensionRevisionReasonProbesSucceeded = "ProbesSucceeded" ClusterExtensionRevisionReasonIncomplete = "Incomplete" ClusterExtensionRevisionReasonProgressing = "Progressing" ClusterExtensionRevisionReasonArchived = "Archived" + ClusterExtensionRevisionReasonRolloutInProgress = "RollingOut" + ClusterExtensionRevisionReasonRolloutError = "RolloutError" + ClusterExtensionRevisionReasonRolledOut = "RolledOut" ) // ClusterExtensionRevisionSpec defines the desired state of ClusterExtensionRevision. @@ -150,6 +155,7 @@ type ClusterExtensionRevisionStatus struct { // ClusterExtensionRevision is the Schema for the clusterextensionrevisions API // +kubebuilder:printcolumn:name="Available",type=string,JSONPath=`.status.conditions[?(@.type=='Available')].status` +// +kubebuilder:printcolumn:name="Progressing",type=string,JSONPath=`.status.conditions[?(@.type=='Progressing')].status` // +kubebuilder:printcolumn:name=Age,type=date,JSONPath=`.metadata.creationTimestamp` type ClusterExtensionRevision struct { metav1.TypeMeta `json:",inline"` diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index e13f1532b0..b8069d7837 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -542,6 +542,13 @@ func (in *ClusterExtensionStatus) DeepCopyInto(out *ClusterExtensionStatus) { *out = new(ClusterExtensionInstallStatus) **out = **in } + if in.ActiveRevisions != nil { + in, out := &in.ActiveRevisions, &out.ActiveRevisions + *out = make([]RevisionStatus, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionStatus. @@ -629,6 +636,28 @@ func (in *ResolvedImageSource) DeepCopy() *ResolvedImageSource { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RevisionStatus) DeepCopyInto(out *RevisionStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RevisionStatus. +func (in *RevisionStatus) DeepCopy() *RevisionStatus { + if in == nil { + return nil + } + out := new(RevisionStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServiceAccountReference) DeepCopyInto(out *ServiceAccountReference) { *out = *in diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 20d55bc3cf..c7234a73ca 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -589,6 +589,14 @@ func setupBoxcutter( ActionClientGetter: acg, RevisionGenerator: rg, } + ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{ + controllers.HandleFinalizers(ceReconciler.Finalizers), + controllers.MigrateStorage(ceReconciler.StorageMigrator), + controllers.RetrieveRevisionStates(ceReconciler.RevisionStatesGetter), + controllers.RetrieveRevisionMetadata(ceReconciler.Resolver), + controllers.UnpackBundle(ceReconciler.ImagePuller, ceReconciler.ImageCache), + controllers.ApplyBundleWithBoxcutter(ceReconciler.Applier), + } baseDiscoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig()) if err != nil { @@ -700,6 +708,14 @@ func setupHelm( Manager: cm, } ceReconciler.RevisionStatesGetter = &controllers.HelmRevisionStatesGetter{ActionClientGetter: acg} + ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{ + controllers.HandleFinalizers(ceReconciler.Finalizers), + controllers.RetrieveRevisionStates(ceReconciler.RevisionStatesGetter), + controllers.RetrieveRevisionMetadata(ceReconciler.Resolver), + controllers.UnpackBundle(ceReconciler.ImagePuller, ceReconciler.ImageCache), + controllers.ApplyBundle(ceReconciler.Applier), + } + return nil } diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 317b46a00c..d0b5768149 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -361,6 +361,7 @@ _Appears in:_ | --- | --- | --- | --- | | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | The set of condition types which apply to all spec.source variations are Installed and Progressing.
The Installed condition represents whether or not the bundle has been installed for this ClusterExtension.
When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.
When Installed is False and the Reason is Failed, the bundle has failed to install.
The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.
When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.
These are indications from a package owner to guide users away from a particular package, channel, or bundle.
BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
PackageDeprecated is set if the requested package is marked deprecated in the catalog.
Deprecated is a rollup condition that is present when any of the deprecated conditions are present. | | | | `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ | install is a representation of the current installation status for this ClusterExtension. | | | +| `activeRevisions` _[RevisionStatus](#revisionstatus) array_ | | | | @@ -435,6 +436,23 @@ _Appears in:_ | `ref` _string_ | ref contains the resolved image digest-based reference.
The digest format is used so users can use other tooling to fetch the exact
OCI manifests that were used to extract the catalog contents. | | MaxLength: 1000
Required: \{\}
| +#### RevisionStatus + + + + + + + +_Appears in:_ +- [ClusterExtensionStatus](#clusterextensionstatus) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `name` _string_ | | | | +| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | | | | + + #### ServiceAccountReference diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml index 5004c8c6fd..d4cab64d14 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml @@ -19,6 +19,9 @@ spec: - jsonPath: .status.conditions[?(@.type=='Available')].status name: Available type: string + - jsonPath: .status.conditions[?(@.type=='Progressing')].status + name: Progressing + type: string - jsonPath: .metadata.creationTimestamp name: Age type: date diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index 1038b7fdf0..33f7b1a4b8 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -22,8 +22,8 @@ spec: - jsonPath: .status.install.bundle.version name: Version type: string - - jsonPath: .status.conditions[?(@.type=='Installed')].status - name: Installed + - jsonPath: .status.conditions[?(@.type=='Available')].status + name: Available type: string - jsonPath: .status.conditions[?(@.type=='Progressing')].status name: Progressing @@ -505,6 +505,78 @@ spec: description: status is an optional field that defines the observed state of the ClusterExtension. properties: + activeRevisions: + items: + properties: + conditions: + items: + description: Condition contains details for one aspect of + the current state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, + Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + name: + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map conditions: description: |- The set of condition types which apply to all spec.source variations are Installed and Progressing. diff --git a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml index a0983e41f9..bc0e51e099 100644 --- a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml @@ -22,8 +22,8 @@ spec: - jsonPath: .status.install.bundle.version name: Version type: string - - jsonPath: .status.conditions[?(@.type=='Installed')].status - name: Installed + - jsonPath: .status.conditions[?(@.type=='Available')].status + name: Available type: string - jsonPath: .status.conditions[?(@.type=='Progressing')].status name: Progressing diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 32279bdf69..6c21de043c 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -325,8 +325,6 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust return false, "New revision created", nil } else if progressingCondition != nil && progressingCondition.Status == metav1.ConditionTrue { return false, progressingCondition.Message, nil - } else if availableCondition != nil && availableCondition.Status != metav1.ConditionTrue { - return false, "", errors.New(availableCondition.Message) } else if succeededCondition != nil && succeededCondition.Status != metav1.ConditionTrue { return false, succeededCondition.Message, nil } 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 0000000000..c64f178913 --- /dev/null +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go @@ -0,0 +1,177 @@ +/* +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" + "io/fs" + "slices" + + 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/client" + "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/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{ + ClusterExtensionRevisionOwnerLabel: 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 "storageLabels") + // 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{ + RevName: rev.Name, + Package: rev.Labels[labels.PackageNameKey], + Image: rev.Annotations[labels.BundleReferenceKey], + Conditions: rev.Status.Conditions, + 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, ext *ocv1.ClusterExtension) (context.Context, *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 ctx, nil, fmt.Errorf("migrating storage: %w", err) + } + return ctx, nil, nil + } +} + +func ApplyBundleWithBoxcutter(a Applier) ReconcileStepFunc { + return func(ctx context.Context, ext *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error) { + l := log.FromContext(ctx) + resolvedRevisionMetadata := ctx.Value(resolvedRevisionMetadataKey{}).(*RevisionMetadata) + imageFS := ctx.Value(imageFSKey{}).(fs.FS) + revisionStates := ctx.Value(revisionStatesKey{}).(*RevisionStates) + storeLbls := map[string]string{ + labels.BundleNameKey: resolvedRevisionMetadata.Name, + labels.PackageNameKey: resolvedRevisionMetadata.Package, + labels.BundleVersionKey: resolvedRevisionMetadata.Version, + labels.BundleReferenceKey: resolvedRevisionMetadata.Image, + } + objLbls := map[string]string{ + labels.OwnerKindKey: ocv1.ClusterExtensionKind, + labels.OwnerNameKey: ext.GetName(), + } + + l.Info("applying bundle contents") + if _, _, err := a.Apply(ctx, imageFS, ext, objLbls, storeLbls); err != nil { + // If there was an error applying the resolved bundle, + // report the error via the Progressing condition. + setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedRevisionMetadata.BundleMetadata, err)) + return ctx, nil, err + } + + // Repopulate active revisions to avoid duplicates + ext.Status.ActiveRevisions = nil + + // Mirror Available/Progressing conditions from the installed revision + if i := revisionStates.Installed; i != nil { + for _, cndType := range []string{ocv1.ClusterExtensionRevisionTypeAvailable, ocv1.ClusterExtensionRevisionTypeProgressing} { + if cnd := apimeta.FindStatusCondition(i.Conditions, cndType); cnd != nil { + apimeta.SetStatusCondition(&ext.Status.Conditions, *cnd) + } + } + ext.Status.Install = &ocv1.ClusterExtensionInstallStatus{ + Bundle: i.BundleMetadata, + } + ext.Status.ActiveRevisions = []ocv1.RevisionStatus{{Name: i.RevName}} + } + for idx, r := range revisionStates.RollingOut { + rs := ocv1.RevisionStatus{Name: r.RevName} + // Mirror Progressing condition from the latest active revision + if idx == len(revisionStates.RollingOut)-1 { + pcnd := apimeta.FindStatusCondition(r.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + if pcnd != nil { + apimeta.SetStatusCondition(&ext.Status.Conditions, *pcnd) + } + acnd := apimeta.FindStatusCondition(r.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + if acnd != nil && pcnd != nil && pcnd.Status == metav1.ConditionFalse && acnd.Status != metav1.ConditionTrue { + apimeta.SetStatusCondition(&ext.Status.Conditions, *acnd) + } + } + ext.Status.ActiveRevisions = append(ext.Status.ActiveRevisions, rs) + } + + // Rewrite Progressing condition to respect the existing contract + // Currently ClusterExtension treats the Progressing condition in the same way as the Deployment resource + // It signals there's nothing in the way of the resource progressing rather than the resource is undergoing + // active reconciliation at the moment. In order to not break the original contract, we rewrite the condition. + // This should be temporary until we understand whether we can change how the Progressing condition operates. + if cnd := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing); cnd != nil { + switch cnd.Reason { + case ocv1.ClusterExtensionRevisionReasonRolledOut: + cnd.Status = metav1.ConditionTrue + cnd.Reason = ocv1.ReasonSucceeded + case ocv1.ClusterExtensionRevisionReasonRolloutError: + cnd.Status = metav1.ConditionTrue + cnd.Reason = ocv1.ReasonRetrying + } + apimeta.SetStatusCondition(&ext.Status.Conditions, *cnd) + } + setInstalledStatusFromRevisionStates(ext, revisionStates) + return ctx, nil, nil + } +} diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 7bcedde656..38978c8b96 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" @@ -49,8 +47,6 @@ 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" @@ -62,6 +58,39 @@ const ( ClusterExtensionCleanupContentManagerCacheFinalizer = "olm.operatorframework.io/cleanup-contentmanager-cache" ) +// ReconcileStepFunc represents a single step in the ClusterExtension reconciliation process. +// It takes a context and ClusterExtension object as input and returns: +// - A potentially modified context for the next step +// - An optional reconciliation result that if non-nil will stop reconciliation +// - Any error that occurred during reconciliation, which will be returned to the caller +type ReconcileStepFunc func(context.Context, *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error) +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 + stepCtx := ctx + for _, step := range *steps { + stepCtx, res, err = step(stepCtx, ext) + if err != nil { + return ctrl.Result{}, err + } + if res != nil { + return *res, nil + } + if stepCtx == nil { + return ctrl.Result{}, nil + } + } + return ctrl.Result{}, nil +} + // ClusterExtensionReconciler reconciles a ClusterExtension object type ClusterExtensionReconciler struct { client.Client @@ -74,6 +103,7 @@ type ClusterExtensionReconciler struct { Applier Applier RevisionStatesGetter RevisionStatesGetter Finalizers crfinalizer.Finalizers + ReconcileSteps ReconcileSteps } type StorageMigrator interface { @@ -106,7 +136,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,168 +194,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 - } - - storeLbls := 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, storeLbls) - - // 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) { @@ -441,7 +309,6 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager, opts ... for _, applyOpt := range opts { applyOpt(ctrlBuilder) } - return ctrlBuilder.Build(r) } @@ -474,9 +341,11 @@ func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger) crh } type RevisionMetadata struct { + RevName string Package string Image string ocv1.BundleMetadata + Conditions []metav1.Condition } type RevisionStates struct { @@ -520,53 +389,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{ - ClusterExtensionRevisionOwnerLabel: 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 "storageLabels") - // 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.Labels[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 437f62dcec..95920e5421 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -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(reconciler *controllers.ClusterExtensionReconciler) { + reconciler.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(reconciler *controllers.ClusterExtensionReconciler) { + 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) + }) }) + 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(reconciler *controllers.ClusterExtensionReconciler) { + reconciler.ImagePuller = &imageutil.MockPuller{ + Error: tc.pullErr, + } + }, + func(reconciler *controllers.ClusterExtensionReconciler) { + 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 + }) + }, + ) + 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(reconciler *controllers.ClusterExtensionReconciler) { + reconciler.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + 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"), + } + }) 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(reconciler *controllers.ClusterExtensionReconciler) { + reconciler.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))} @@ -401,10 +415,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(reconciler *controllers.ClusterExtensionReconciler) { + reconciler.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + 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 + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -439,34 +475,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) @@ -496,10 +511,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(reconciler *controllers.ClusterExtensionReconciler) { + reconciler.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + 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"), + } + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -534,18 +562,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) @@ -572,10 +588,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(reconciler *controllers.ClusterExtensionReconciler) { + reconciler.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + 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"), + } + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -611,18 +640,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) @@ -649,10 +667,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(reconciler *controllers.ClusterExtensionReconciler) { + reconciler.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + 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, + } + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -687,17 +717,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) @@ -724,10 +743,32 @@ 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" + cl, reconciler := newClientAndReconciler(t, func(reconciler *controllers.ClusterExtensionReconciler) { + reconciler.ImagePuller = &imageutil.MockPuller{ + ImageFS: fstest.MapFS{}, + } + 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, + } + 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", + }, + }, + } + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -761,27 +802,6 @@ 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) { 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 0000000000..b8cfd0c8a4 --- /dev/null +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -0,0 +1,211 @@ +/* +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" + "io/fs" + + 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" +) + +type revisionStatesKey struct{} +type resolvedRevisionMetadataKey struct{} +type imageFSKey struct{} + +func HandleFinalizers(f finalizer.Finalizer) ReconcileStepFunc { + return func(ctx context.Context, ext *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error) { + l := log.FromContext(ctx) + + l.Info("handling finalizers") + finalizeResult, err := f.Finalize(ctx, ext) + if err != nil { + setStatusProgressing(ext, err) + return ctx, 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 ctx, &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 nil, nil, nil + } + return ctx, nil, nil + } +} + +func RetrieveRevisionStates(r RevisionStatesGetter) ReconcileStepFunc { + return func(ctx context.Context, ext *ocv1.ClusterExtension) (context.Context, *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 ctx, nil, err + } + setInstalledStatusConditionUnknown(ext, err.Error()) + setStatusProgressing(ext, errors.New("retrying to get installed bundle")) + return ctx, nil, err + } + return context.WithValue(ctx, revisionStatesKey{}, revisionStates), nil, nil + } +} + +func RetrieveRevisionMetadata(r resolve.Resolver) ReconcileStepFunc { + return func(ctx context.Context, ext *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error) { + l := log.FromContext(ctx) + revisionStates := ctx.Value(revisionStatesKey{}).(*RevisionStates) + 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.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 ctx, 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 = revisionStates.RollingOut[0] + } + return context.WithValue(ctx, resolvedRevisionMetadataKey{}, resolvedRevisionMetadata), nil, nil + } +} + +func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc { + return func(ctx context.Context, ext *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error) { + l := log.FromContext(ctx) + l.Info("unpacking resolved bundle") + resolvedRevisionMetadata := ctx.Value(resolvedRevisionMetadataKey{}).(*RevisionMetadata) + imageFS, _, _, err := i.Pull(ctx, ext.GetName(), resolvedRevisionMetadata.Image, cache) + if err != nil { + revisionStates := ctx.Value(revisionStatesKey{}).(*RevisionStates) + // 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 ctx, nil, err + } + return context.WithValue(ctx, imageFSKey{}, imageFS), nil, nil + } +} + +func ApplyBundle(a Applier) ReconcileStepFunc { + return func(ctx context.Context, ext *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error) { + l := log.FromContext(ctx) + resolvedRevisionMetadata := ctx.Value(resolvedRevisionMetadataKey{}).(*RevisionMetadata) + imageFS := ctx.Value(imageFSKey{}).(fs.FS) + revisionStates := ctx.Value(revisionStatesKey{}).(*RevisionStates) + storeLbls := map[string]string{ + labels.BundleNameKey: resolvedRevisionMetadata.Name, + labels.PackageNameKey: resolvedRevisionMetadata.Package, + labels.BundleVersionKey: resolvedRevisionMetadata.Version, + labels.BundleReferenceKey: 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, imageFS, ext, objLbls, storeLbls) + + // 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 ctx, 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 ctx, nil, nil + } +} diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index 7e98faa654..f88d86c276 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/labels" ) const ( @@ -122,29 +123,27 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev return c.teardown(ctx, rev, revision) } + revVersion := rev.GetAnnotations()[labels.BundleVersionKey] // // Reconcile // if err := c.ensureFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer); err != nil { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, - Message: err.Error(), - ObservedGeneration: rev.Generation, - }) return ctrl.Result{}, fmt.Errorf("error ensuring teardown finalizer: %v", err) } - if err := c.establishWatch(ctx, rev, revision); err != nil { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, - Message: err.Error(), - ObservedGeneration: rev.Generation, - }) - return ctrl.Result{}, fmt.Errorf("establish watch: %v", err) + // If the Available condition is not present, we are still rolling out the objects. + // NOTE: This assumes that once the Available condition is set, it will always be present. + // If the Available condition is ever removed or reset during the lifecycle, + // this could lead to incorrect behavior. + inRollout := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) == nil + if inRollout { + if err := c.establishWatch(ctx, rev, revision); err != nil { + werr := fmt.Errorf("establish watch: %v", err) + // this error is very likely transient, so we should keep revision as progressing + markAsProgressing(rev, ocv1.ClusterExtensionRevisionReasonReconcileFailure, werr.Error()) + return ctrl.Result{}, werr + } + markAsProgressing(rev, ocv1.ClusterExtensionRevisionReasonRolloutInProgress, fmt.Sprintf("Revision %s is being rolled out.", revVersion)) } rres, err := c.RevisionEngine.Reconcile(ctx, *revision, opts...) @@ -154,13 +153,19 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev } else { l.Error(err, "revision reconcile failed") } - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, - Message: err.Error(), - ObservedGeneration: rev.Generation, - }) + if inRollout { + markAsProgressing(rev, ocv1.ClusterExtensionRevisionReasonRolloutError, err.Error()) + } else { + // it is a probably transient error, and we do not know if the revision is available or not + // perhaps we should not report it at all, hoping that it is going to be mitigated in the next reconcile? + meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Status: metav1.ConditionUnknown, + Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, + Message: err.Error(), + ObservedGeneration: rev.Generation, + }) + } return ctrl.Result{}, fmt.Errorf("revision reconcile: %v", err) } // Log detailed reconcile reports only in debug mode (V(1)) to reduce verbosity. @@ -171,13 +176,20 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev if verr := rres.GetValidationError(); verr != nil { l.Error(fmt.Errorf("%w", verr), "preflight validation failed, retrying after 10s") - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonRevisionValidationFailure, - Message: fmt.Sprintf("revision validation error: %s", verr), - ObservedGeneration: rev.Generation, - }) + if inRollout { + // given that we retry, we are going to keep Progressing condition True + markAsProgressing(rev, ocv1.ClusterExtensionRevisionReasonRevisionValidationFailure, fmt.Sprintf("revision validation error: %s", verr)) + } else { + // it is a probably transient error, and we do not know if the revision is available or not + // perhaps we should not report it at all, hoping that it is going to be mitigated in the next reconcile? + meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Status: metav1.ConditionUnknown, + Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, + Message: fmt.Sprintf("revision validation error: %s", verr), + ObservedGeneration: rev.Generation, + }) + } return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } @@ -185,13 +197,20 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev if verr := pres.GetValidationError(); verr != nil { l.Error(fmt.Errorf("%w", verr), "phase preflight validation failed, retrying after 10s", "phase", i) - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonPhaseValidationError, - Message: fmt.Sprintf("phase %d validation error: %s", i, verr), - ObservedGeneration: rev.Generation, - }) + if inRollout { + // given that we retry, we are going to keep Progressing condition True + markAsProgressing(rev, ocv1.ClusterExtensionRevisionReasonPhaseValidationError, fmt.Sprintf("phase %d validation error: %s", i, verr)) + } else { + // it is a probably transient error, and we do not know if the revision is available or not + // perhaps we should not report it at all, hoping that it is going to be mitigated in the next reconcile? + meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Status: metav1.ConditionUnknown, + Reason: ocv1.ClusterExtensionRevisionReasonPhaseValidationError, + Message: fmt.Sprintf("phase %d validation error: %s", i, verr), + ObservedGeneration: rev.Generation, + }) + } return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } @@ -204,18 +223,22 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev if len(collidingObjs) > 0 { l.Error(fmt.Errorf("object collision detected"), "object collision, retrying after 10s", "phase", i, "collisions", collidingObjs) + // collisions are probably stickier than phase roll out probe failures - so we'd probably want to set + // Progressing to false here due to the collision + if inRollout { + markAsNotProgressing(rev, ocv1.ClusterExtensionRevisionReasonObjectCollisions, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n"))) - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonObjectCollisions, - Message: fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")), - ObservedGeneration: rev.Generation, - }) - return ctrl.Result{RequeueAfter: 10 * time.Second}, nil + // NOTE(pedjak): not sure if we want to retry here - collisions are probably not transient? + return ctrl.Result{RequeueAfter: 10 * time.Second}, nil + } } } + if !rres.InTransistion() { + // we have rolled out all objects in all phases, not interested in probes here + markAsNotProgressing(rev, ocv1.ClusterExtensionRevisionReasonRolledOut, fmt.Sprintf("Revision %s is rolled out.", revVersion)) + } + //nolint:nestif if rres.IsComplete() { // Archive other revisions. @@ -229,23 +252,20 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev } } - // Report status. + // It would be good to understand from Nico how we can distinguish between progression and availability probes + // and how to best check that all Availability probes are passing + markAsAvailable(rev, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, "Objects are available and pass all probes.") + + // We'll probably only want to remove this once we are done updating the ClusterExtension conditions + // as its one of the interfaces between the revision and the extension. If we still have the Succeeded for now + // that's fine. meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Type: ocv1.ClusterExtensionRevisionTypeSucceeded, Status: metav1.ConditionTrue, - Reason: ocv1.ClusterExtensionRevisionReasonAvailable, - Message: "Object is available and passes all probes.", + Reason: ocv1.ClusterExtensionRevisionReasonRolloutSuccess, + Message: "Revision succeeded rolling out.", ObservedGeneration: rev.Generation, }) - if !meta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeSucceeded, - Status: metav1.ConditionTrue, - Reason: ocv1.ClusterExtensionRevisionReasonRolloutSuccess, - Message: "Revision succeeded rolling out.", - ObservedGeneration: rev.Generation, - }) - } } else { var probeFailureMsgs []string for _, pres := range rres.GetPhases() { @@ -253,6 +273,8 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev continue } for _, ores := range pres.GetObjects() { + // we probably want an AvailabilityProbeType and run through all of them independently of whether + // the revision is complete or not pr := ores.Probes()[boxcutter.ProgressProbeType] if pr.Success { continue @@ -260,6 +282,8 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev obj := ores.Object() gvk := obj.GetObjectKind().GroupVersionKind() + // I think these can be pretty large and verbose. We may want to + // work a little on the formatting...? probeFailureMsgs = append(probeFailureMsgs, fmt.Sprintf( "Object %s.%s %s/%s: %v", gvk.Kind, gvk.GroupVersion().String(), @@ -269,34 +293,11 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev } } if len(probeFailureMsgs) > 0 { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonProbeFailure, - Message: strings.Join(probeFailureMsgs, "\n"), - ObservedGeneration: rev.Generation, - }) + markAsUnavailable(rev, ocv1.ClusterExtensionRevisionReasonProbeFailure, strings.Join(probeFailureMsgs, "\n")) } else { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonIncomplete, - Message: "Revision has not been rolled out completely.", - ObservedGeneration: rev.Generation, - }) + markAsUnavailable(rev, ocv1.ClusterExtensionRevisionReasonIncomplete, fmt.Sprintf("Revision %s has not been rolled out completely.", revVersion)) } } - if rres.InTransistion() { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.TypeProgressing, - Status: metav1.ConditionTrue, - Reason: ocv1.ClusterExtensionRevisionReasonProgressing, - Message: "Rollout in progress.", - ObservedGeneration: rev.Generation, - }) - } else { - meta.RemoveStatusCondition(&rev.Status.Conditions, ocv1.TypeProgressing) - } return ctrl.Result{}, nil } @@ -341,17 +342,26 @@ func (c *ClusterExtensionRevisionReconciler) teardown(ctx context.Context, rev * return ctrl.Result{}, fmt.Errorf("error stopping informers: %v", err) } - // Ensure Available condition is set to Unknown before removing the finalizer when archiving - if rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived && - !meta.IsStatusConditionPresentAndEqual(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable, metav1.ConditionUnknown) { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ + // Ensure conditions are set before removing the finalizer when archiving + if rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived { + condUpdated := meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionUnknown, Reason: ocv1.ClusterExtensionRevisionReasonArchived, Message: "revision is archived", ObservedGeneration: rev.Generation, }) - return ctrl.Result{}, nil + condUpdated = meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeProgressing, + Status: metav1.ConditionFalse, + Reason: ocv1.ClusterExtensionRevisionReasonArchived, + Message: "revision is archived", + ObservedGeneration: rev.Generation, + }) || condUpdated + + if condUpdated { + return ctrl.Result{}, nil + } } if err := c.removeFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer); err != nil { @@ -551,3 +561,43 @@ var ( FieldB: ".status.replicas", } ) + +func markAsProgressing(cer *ocv1.ClusterExtensionRevision, reason, message string) { + meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeProgressing, + Status: metav1.ConditionTrue, + Reason: reason, + Message: message, + ObservedGeneration: cer.Generation, + }) +} + +func markAsNotProgressing(cer *ocv1.ClusterExtensionRevision, reason, message string) { + meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeProgressing, + Status: metav1.ConditionFalse, + Reason: reason, + Message: message, + ObservedGeneration: cer.Generation, + }) +} + +func markAsAvailable(cer *ocv1.ClusterExtensionRevision, reason, message string) { + meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: reason, + Message: message, + ObservedGeneration: cer.Generation, + }) +} + +func markAsUnavailable(cer *ocv1.ClusterExtensionRevision, reason, message string) { + meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Status: metav1.ConditionFalse, + Reason: reason, + Message: message, + ObservedGeneration: cer.Generation, + }) +} diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go index 873a6cc748..57cade047a 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go @@ -2,13 +2,14 @@ package controllers_test import ( "context" + "errors" "fmt" "testing" "time" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -29,9 +30,10 @@ 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/labels" ) -func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *testing.T) { +func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t *testing.T) { const ( clusterExtensionRevisionName = "test-ext-1" ) @@ -39,10 +41,11 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te testScheme := newScheme(t) for _, tc := range []struct { - name string - existingObjs func() []client.Object - revisionResult machinery.RevisionResult - validate func(*testing.T, client.Client) + name string + existingObjs func() []client.Object + revisionResult machinery.RevisionResult + revisionReconcileErr error + validate func(*testing.T, client.Client) }{ { name: "sets teardown finalizer", @@ -81,13 +84,106 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te require.NotNil(t, cond) require.Equal(t, metav1.ConditionFalse, cond.Status) require.Equal(t, ocv1.ClusterExtensionRevisionReasonIncomplete, cond.Reason) - require.Equal(t, "Revision has not been rolled out completely.", cond.Message) + require.Equal(t, "Revision 1.0.0 has not been rolled out completely.", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) + }, + }, + { + name: "set Available:False:ProbeFailure condition when probe failures are detected and revision is in transition ", + revisionResult: mockRevisionResult{ + inTransition: true, + isComplete: false, + phases: []machinery.PhaseResult{ + mockPhaseResult{ + name: "somephase", + isComplete: false, + objects: []machinery.ObjectResult{ + mockObjectResult{ + success: true, + probes: map[string]machinery.ObjectProbeResult{ + boxcutter.ProgressProbeType: { + Success: true, + }, + }, + }, + mockObjectResult{ + success: false, + object: func() client.Object { + obj := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + Namespace: "my-namespace", + }, + } + obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service")) + return obj + }(), + probes: map[string]machinery.ObjectProbeResult{ + boxcutter.ProgressProbeType: { + Success: false, + Messages: []string{ + "something bad happened", + "something worse happened", + }, + }, + }, + }, + }, + }, + mockPhaseResult{ + name: "someotherphase", + isComplete: false, + objects: []machinery.ObjectResult{ + mockObjectResult{ + success: false, + object: func() client.Object { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-configmap", + Namespace: "my-namespace", + }, + } + obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) + return obj + }(), + probes: map[string]machinery.ObjectProbeResult{ + boxcutter.ProgressProbeType: { + Success: false, + Messages: []string{ + "we have a problem", + }, + }, + }, + }, + }, + }, + }, + }, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonProbeFailure, cond.Reason) + require.Equal(t, "Object Service.v1 my-namespace/my-service: something bad happened and something worse happened\nObject ConfigMap.v1 my-namespace/my-configmap: we have a problem", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) }, }, { - name: "set Available:False:ProbeFailure condition when probe failures are detected", + name: "set Available:False:ProbeFailure condition when probe failures are detected and revision is not in transition", revisionResult: mockRevisionResult{ + inTransition: false, + isComplete: false, phases: []machinery.PhaseResult{ mockPhaseResult{ name: "somephase", @@ -175,7 +271,31 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te }, }, { - name: "set Progressing:True:Progressing condition while revision is transitioning", + name: "set Progressing:True:RolloutError when there's an error reconciling the revision", + revisionReconcileErr: errors.New("some error"), + + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionTrue, cond.Status) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonRolloutError, cond.Reason) + require.Equal(t, "some error", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) + }, + }, + { + name: "set Progressing:True:RollingOut condition while revision is transitioning", revisionResult: mockRevisionResult{ inTransition: true, }, @@ -194,13 +314,13 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing) require.NotNil(t, cond) require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ClusterExtensionRevisionReasonProgressing, cond.Reason) - require.Equal(t, "Rollout in progress.", cond.Message) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonRolloutInProgress, cond.Reason) + require.Equal(t, "Revision 1.0.0 is being rolled out.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) }, }, { - name: "remove Progressing condition once transition rollout is finished", + name: "set Progressing:False:RolledOut once transition rollout is finished", revisionResult: mockRevisionResult{ inTransition: false, }, @@ -211,8 +331,8 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, - Reason: ocv1.ClusterExtensionRevisionReasonProgressing, - Message: "some message", + Reason: ocv1.ClusterExtensionRevisionReasonRolledOut, + Message: "Revision 1.0.0 is rolled out.", ObservedGeneration: 1, }) return []client.Object{ext, rev1} @@ -224,11 +344,15 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te }, rev) require.NoError(t, err) cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing) - require.Nil(t, cond) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonRolledOut, cond.Reason) + require.Equal(t, "Revision 1.0.0 is rolled out.", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) }, }, { - name: "set Available:True:Available and Succeeded:True:RolloutSuccess conditions on successful revision rollout", + name: "set Available:True:ProbesSucceeded and Succeeded:True:RolloutSuccess conditions on successful revision rollout", revisionResult: mockRevisionResult{ isComplete: true, }, @@ -247,8 +371,8 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) require.NotNil(t, cond) require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ClusterExtensionRevisionReasonAvailable, cond.Reason) - require.Equal(t, "Object is available and passes all probes.", cond.Message) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + require.Equal(t, "Objects are available and pass all probes.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) cond = meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) @@ -312,7 +436,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te Client: testClient, RevisionEngine: &mockRevisionEngine{ reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { - return tc.revisionResult, nil + return tc.revisionResult, tc.revisionReconcileErr }, }, TrackingCache: &mockTrackingCache{}, @@ -322,9 +446,13 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te }, }) - // reconcile cluster extensionr evision + // reconcile cluster extension revision require.Equal(t, ctrl.Result{}, result) - require.NoError(t, err) + if tc.revisionReconcileErr == nil { + require.NoError(t, err) + } else { + require.Contains(t, err.Error(), tc.revisionReconcileErr.Error()) + } // validate test case tc.validate(t, testClient) @@ -438,7 +566,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_ValidationError_Retries(t }, }) - // reconcile cluster extensionr evision + // reconcile cluster extension revision require.Equal(t, ctrl.Result{ RequeueAfter: 10 * time.Second, }, result) @@ -512,7 +640,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { Name: clusterExtensionRevisionName, }, rev) require.Error(t, err) - require.True(t, errors.IsNotFound(err)) + require.True(t, apierrors.IsNotFound(err)) }, }, { @@ -545,7 +673,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { }, }, { - name: "set Available condition to Unknown with reason Archived when archiving revision", + name: "set Available:Archived:Unknown and Progressing:False:Unknown conditions when a revision is archived", revisionResult: mockRevisionResult{}, existingObjs: func() []client.Object { ext := newTestClusterExtension() @@ -576,6 +704,13 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { require.Equal(t, ocv1.ClusterExtensionRevisionReasonArchived, cond.Reason) require.Equal(t, "revision is archived", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) + + cond = meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonArchived, cond.Reason) + require.Equal(t, "revision is archived", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) }, }, { @@ -595,6 +730,13 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { Message: "revision is archived", ObservedGeneration: rev1.Generation, }) + meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeProgressing, + Status: metav1.ConditionFalse, + Reason: ocv1.ClusterExtensionRevisionReasonArchived, + Message: "revision is archived", + ObservedGeneration: rev1.Generation, + }) require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) return []client.Object{rev1, ext} }, @@ -709,6 +851,12 @@ func newTestClusterExtensionRevision(name string) *ocv1.ClusterExtensionRevision Name: name, UID: types.UID(name), Generation: int64(1), + Annotations: map[string]string{ + labels.PackageNameKey: "some-package", + labels.BundleNameKey: "some-package.v1.0.0", + labels.BundleReferenceKey: "registry.io/some-repo/some-package:v1.0.0", + labels.BundleVersionKey: "1.0.0", + }, }, Spec: ocv1.ClusterExtensionRevisionSpec{ Phases: []ocv1.ClusterExtensionRevisionPhase{ diff --git a/internal/operator-controller/controllers/suite_test.go b/internal/operator-controller/controllers/suite_test.go index 02d5382371..280002ef7e 100644 --- a/internal/operator-controller/controllers/suite_test.go +++ b/internal/operator-controller/controllers/suite_test.go @@ -76,7 +76,9 @@ 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(*controllers.ClusterExtensionReconciler) + +func newClientAndReconciler(t *testing.T, opts ...reconcilerOption) (client.Client, *controllers.ClusterExtensionReconciler) { cl := newClient(t) reconciler := &controllers.ClusterExtensionReconciler{ @@ -86,6 +88,20 @@ func newClientAndReconciler(t *testing.T) (client.Client, *controllers.ClusterEx }, Finalizers: crfinalizer.NewFinalizers(), } + for _, opt := range opts { + opt(reconciler) + } + reconciler.ReconcileSteps = []controllers.ReconcileStepFunc{controllers.HandleFinalizers(reconciler.Finalizers), controllers.RetrieveRevisionStates(reconciler.RevisionStatesGetter)} + if r := reconciler.Resolver; r != nil { + reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.RetrieveRevisionMetadata(r)) + } + if i := reconciler.ImagePuller; i != nil { + reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.UnpackBundle(i, reconciler.ImageCache)) + } + if a := reconciler.Applier; a != nil { + reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.ApplyBundle(a)) + } + return cl, reconciler } diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index db03c11a8d..fdf9b0e70d 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -610,6 +610,9 @@ spec: - jsonPath: .status.conditions[?(@.type=='Available')].status name: Available type: string + - jsonPath: .status.conditions[?(@.type=='Progressing')].status + name: Progressing + type: string - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -827,8 +830,8 @@ spec: - jsonPath: .status.install.bundle.version name: Version type: string - - jsonPath: .status.conditions[?(@.type=='Installed')].status - name: Installed + - jsonPath: .status.conditions[?(@.type=='Available')].status + name: Available type: string - jsonPath: .status.conditions[?(@.type=='Progressing')].status name: Progressing @@ -1310,6 +1313,78 @@ spec: description: status is an optional field that defines the observed state of the ClusterExtension. properties: + activeRevisions: + items: + properties: + conditions: + items: + description: Condition contains details for one aspect of + the current state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, + Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + name: + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map conditions: description: |- The set of condition types which apply to all spec.source variations are Installed and Progressing. diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 664f8599cc..42ec28ec2c 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -575,6 +575,9 @@ spec: - jsonPath: .status.conditions[?(@.type=='Available')].status name: Available type: string + - jsonPath: .status.conditions[?(@.type=='Progressing')].status + name: Progressing + type: string - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -792,8 +795,8 @@ spec: - jsonPath: .status.install.bundle.version name: Version type: string - - jsonPath: .status.conditions[?(@.type=='Installed')].status - name: Installed + - jsonPath: .status.conditions[?(@.type=='Available')].status + name: Available type: string - jsonPath: .status.conditions[?(@.type=='Progressing')].status name: Progressing @@ -1275,6 +1278,78 @@ spec: description: status is an optional field that defines the observed state of the ClusterExtension. properties: + activeRevisions: + items: + properties: + conditions: + items: + description: Condition contains details for one aspect of + the current state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, + Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + name: + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map conditions: description: |- The set of condition types which apply to all spec.source variations are Installed and Progressing. diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 5c95907841..24eb7a0c4a 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -613,8 +613,8 @@ spec: - jsonPath: .status.install.bundle.version name: Version type: string - - jsonPath: .status.conditions[?(@.type=='Installed')].status - name: Installed + - jsonPath: .status.conditions[?(@.type=='Available')].status + name: Available type: string - jsonPath: .status.conditions[?(@.type=='Progressing')].status name: Progressing diff --git a/manifests/standard.yaml b/manifests/standard.yaml index 95e400c264..bb9a991e2f 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -578,8 +578,8 @@ spec: - jsonPath: .status.install.bundle.version name: Version type: string - - jsonPath: .status.conditions[?(@.type=='Installed')].status - name: Installed + - jsonPath: .status.conditions[?(@.type=='Available')].status + name: Available type: string - jsonPath: .status.conditions[?(@.type=='Progressing')].status name: Progressing diff --git a/test/e2e/cluster_extension_revision_test.go b/test/e2e/cluster_extension_revision_test.go new file mode 100644 index 0000000000..6add08deab --- /dev/null +++ b/test/e2e/cluster_extension_revision_test.go @@ -0,0 +1,241 @@ +package e2e + +import ( + "context" + "fmt" + "os" + "slices" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/remotecommand" + 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/features" + . "github.com/operator-framework/operator-controller/internal/shared/util/testutils" + . "github.com/operator-framework/operator-controller/test/helpers" +) + +func TestClusterExtensionRevision(t *testing.T) { + SkipIfFeatureGateDisabled(t, string(features.BoxcutterRuntime)) + t.Log("When a cluster extension is installed from a catalog") + t.Log("When the extension bundle format is registry+v1") + + clusterExtension, extensionCatalog, sa, ns := TestInit(t) + defer TestCleanup(t, extensionCatalog, clusterExtension, sa, ns) + defer CollectTestArtifacts(t, artifactName, c, cfg) + + clusterExtension.Spec = ocv1.ClusterExtensionSpec{ + Source: ocv1.SourceConfig{ + SourceType: "Catalog", + Catalog: &ocv1.CatalogFilter{ + PackageName: "test", + Version: "1.0.1", + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name}, + }, + }, + }, + Namespace: ns.Name, + ServiceAccount: ocv1.ServiceAccountReference{ + Name: sa.Name, + }, + } + t.Log("It resolves the specified package with correct bundle path") + t.Log("By creating the ClusterExtension resource") + require.NoError(t, c.Create(context.Background(), clusterExtension)) + + t.Log("By eventually reporting a successful resolution and bundle path") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + }, pollDuration, pollInterval) + + t.Log("By revision-1 eventually reporting Progressing:False:RolledOut and Available:True:ProbesSucceeded conditions") + var clusterExtensionRevision ocv1.ClusterExtensionRevision + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("%s-1", clusterExtension.Name)}, &clusterExtensionRevision)) + cond := apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionFalse, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonRolledOut, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By eventually reporting progressing as True") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By eventually reporting available as True") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By eventually installing the package successfully") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + require.Contains(ct, cond.Message, "Installed bundle") + require.NotEmpty(ct, clusterExtension.Status.Install.Bundle) + }, pollDuration, pollInterval) + + t.Log("Check Deployment Availability Probe") + t.Log("By making the operator pod not ready") + podName := getPodName(t, clusterExtension.Spec.Namespace, client.MatchingLabels{"app": "olme2etest"}) + podExec(t, clusterExtension.Spec.Namespace, podName, []string{"rm", "/var/www/ready"}) + + t.Log("By revision-1 eventually reporting Progressing:False:RolledOut and Available:False:ProbeFailure conditions") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("%s-1", clusterExtension.Name)}, &clusterExtensionRevision)) + cond := apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionFalse, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonRolledOut, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionFalse, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbeFailure, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By extension eventually reporting available as False") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionFalse, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbeFailure, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By making the operator pod ready") + podName = getPodName(t, clusterExtension.Spec.Namespace, client.MatchingLabels{"app": "olme2etest"}) + podExec(t, clusterExtension.Spec.Namespace, podName, []string{"touch", "/var/www/ready"}) + + t.Log("By revision-1 eventually reporting Progressing:False:RolledOut and Available:True:ProbesSucceeded conditions") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("%s-1", clusterExtension.Name)}, &clusterExtensionRevision)) + cond := apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionFalse, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonRolledOut, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By extension eventually reporting available as True") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("Check archiving") + t.Log("By upgrading the cluster extension to v1.2.0") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + clusterExtension.Spec.Source.Catalog.Version = "1.2.0" + require.NoError(t, c.Update(context.Background(), clusterExtension)) + }, pollDuration, pollInterval) + + t.Log("By revision-2 eventually reporting Progressing:False:RolledOut and Available:True:ProbesSucceeded conditions") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("%s-2", clusterExtension.Name)}, &clusterExtensionRevision)) + cond := apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionFalse, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonRolledOut, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By eventually reporting progressing, available, and installed as True") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + require.Contains(ct, cond.Message, "Installed bundle") + require.NotEmpty(ct, clusterExtension.Status.Install.Bundle) + }, pollDuration, pollInterval) + + t.Log("By revision-1 eventually reporting Progressing:False:Archived and Available:Unknown:Archived conditions") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("%s-1", clusterExtension.Name)}, &clusterExtensionRevision)) + cond := apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionFalse, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonArchived, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionUnknown, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonArchived, cond.Reason) + }, pollDuration, pollInterval) +} + +func getPodName(t *testing.T, podNamespace string, matchingLabels client.MatchingLabels) string { + var podList corev1.PodList + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.List(context.Background(), &podList, client.InNamespace(podNamespace), matchingLabels)) + podList.Items = slices.DeleteFunc(podList.Items, func(pod corev1.Pod) bool { + // Ignore terminating pods + return pod.DeletionTimestamp != nil + }) + require.Len(ct, podList.Items, 1) + }, pollDuration, pollInterval) + return podList.Items[0].Name +} + +func podExec(t *testing.T, podNamespace string, podName string, cmd []string) { + req := cs.CoreV1().RESTClient().Post().Resource("pods").Name(podName).Namespace(podNamespace).SubResource("exec") + req.VersionedParams(&corev1.PodExecOptions{ + Command: cmd, + Stdout: true, + }, scheme.ParameterCodec) + exec, err := remotecommand.NewSPDYExecutor(ctrl.GetConfigOrDie(), "POST", req.URL()) + require.NoError(t, err) + err = exec.StreamWithContext(context.Background(), remotecommand.StreamOptions{Stdout: os.Stdout}) + require.NoError(t, err) +} diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index aa033a2f1e..bb28ccdf77 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -8,6 +8,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -20,6 +21,7 @@ import ( var ( cfg *rest.Config c client.Client + cs *kubernetes.Clientset ) const ( @@ -35,6 +37,9 @@ func TestMain(m *testing.M) { c, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) utilruntime.Must(err) + cs, err = kubernetes.NewForConfig(cfg) + utilruntime.Must(err) + res := m.Run() path := os.Getenv(testSummaryOutputEnvVar) if path == "" { diff --git a/testdata/images/bundles/test-operator/v1.0.2/manifests/bundle.configmap.yaml b/testdata/images/bundles/test-operator/v1.0.2/manifests/bundle.configmap.yaml new file mode 100644 index 0000000000..0279603bfc --- /dev/null +++ b/testdata/images/bundles/test-operator/v1.0.2/manifests/bundle.configmap.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-configmap + annotations: + shouldNotTemplate: > + The namespace is {{ $labels.namespace }}. The templated $labels.namespace is NOT expected to be processed by OLM's rendering engine for registry+v1 bundles. + +data: + version: "v1.0.2" + name: "test-configmap" diff --git a/testdata/images/bundles/test-operator/v1.0.2/manifests/olm.operatorframework.com_olme2etest.yaml b/testdata/images/bundles/test-operator/v1.0.2/manifests/olm.operatorframework.com_olme2etest.yaml new file mode 100644 index 0000000000..44e64cef79 --- /dev/null +++ b/testdata/images/bundles/test-operator/v1.0.2/manifests/olm.operatorframework.com_olme2etest.yaml @@ -0,0 +1,27 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.16.1 + name: olme2etests.olm.operatorframework.io +spec: + group: olm.operatorframework.io + names: + kind: OLME2ETest + listKind: OLME2ETestList + plural: olme2etests + singular: olme2etest + scope: Cluster + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + testField: + type: string diff --git a/testdata/images/bundles/test-operator/v1.0.2/manifests/testoperator.clusterserviceversion.yaml b/testdata/images/bundles/test-operator/v1.0.2/manifests/testoperator.clusterserviceversion.yaml new file mode 100644 index 0000000000..f39fd69f5f --- /dev/null +++ b/testdata/images/bundles/test-operator/v1.0.2/manifests/testoperator.clusterserviceversion.yaml @@ -0,0 +1,151 @@ +apiVersion: operators.coreos.com/v1alpha1 +kind: ClusterServiceVersion +metadata: + annotations: + alm-examples: |- + [ + { + "apiVersion": "olme2etests.olm.operatorframework.io/v1", + "kind": "OLME2ETests", + "metadata": { + "labels": { + "app.kubernetes.io/managed-by": "kustomize", + "app.kubernetes.io/name": "test" + }, + "name": "test-sample" + }, + "spec": null + } + ] + capabilities: Basic Install + createdAt: "2024-10-24T19:21:40Z" + operators.operatorframework.io/builder: operator-sdk-v1.34.1 + operators.operatorframework.io/project_layout: go.kubebuilder.io/v4 + name: testoperator.v1.0.2 + namespace: placeholder +spec: + apiservicedefinitions: {} + customresourcedefinitions: + owned: + - description: Configures subsections of Alertmanager configuration specific to each namespace + displayName: OLME2ETest + kind: OLME2ETest + name: olme2etests.olm.operatorframework.io + version: v1 + description: OLM E2E Testing Operator with a wrong image ref + displayName: test-operator + icon: + - base64data: "" + mediatype: "" + install: + spec: + deployments: + - label: + app.kubernetes.io/component: controller + app.kubernetes.io/name: test-operator + app.kubernetes.io/version: 1.0.2 + name: test-operator + spec: + replicas: 1 + selector: + matchLabels: + app: olme2etest + template: + metadata: + labels: + app: olme2etest + spec: + terminationGracePeriodSeconds: 0 + volumes: + - name: scripts + configMap: + name: httpd-script + defaultMode: 0755 + containers: + - name: busybox-httpd-container + # This image ref is wrong and should trigger ImagePullBackOff condition + image: wrong/image + serviceAccountName: simple-bundle-manager + clusterPermissions: + - rules: + - apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create + - apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create + serviceAccountName: simple-bundle-manager + permissions: + - rules: + - apiGroups: + - "" + resources: + - configmaps + - serviceaccounts + verbs: + - get + - list + - watch + - create + - update + - patch + - delete + - apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - get + - list + - create + - update + - delete + - apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - get + - list + - watch + - create + - update + - patch + - delete + - apiGroups: + - "" + resources: + - events + verbs: + - create + - patch + serviceAccountName: simple-bundle-manager + strategy: deployment + installModes: + - supported: false + type: OwnNamespace + - supported: true + type: SingleNamespace + - supported: false + type: MultiNamespace + - supported: true + type: AllNamespaces + keywords: + - registry + links: + - name: simple-bundle + url: https://simple-bundle.domain + maintainers: + - email: main#simple-bundle.domain + name: Simple Bundle + maturity: beta + provider: + name: Simple Bundle + url: https://simple-bundle.domain + version: 1.0.2 diff --git a/testdata/images/bundles/test-operator/v1.0.2/manifests/testoperator.networkpolicy.yaml b/testdata/images/bundles/test-operator/v1.0.2/manifests/testoperator.networkpolicy.yaml new file mode 100644 index 0000000000..20a5ea834f --- /dev/null +++ b/testdata/images/bundles/test-operator/v1.0.2/manifests/testoperator.networkpolicy.yaml @@ -0,0 +1,8 @@ +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: test-operator-network-policy +spec: + podSelector: {} + policyTypes: + - Ingress diff --git a/testdata/images/bundles/test-operator/v1.0.2/metadata/annotations.yaml b/testdata/images/bundles/test-operator/v1.0.2/metadata/annotations.yaml new file mode 100644 index 0000000000..404f0f4a34 --- /dev/null +++ b/testdata/images/bundles/test-operator/v1.0.2/metadata/annotations.yaml @@ -0,0 +1,10 @@ +annotations: + # Core bundle annotations. + operators.operatorframework.io.bundle.mediatype.v1: registry+v1 + operators.operatorframework.io.bundle.manifests.v1: manifests/ + operators.operatorframework.io.bundle.metadata.v1: metadata/ + operators.operatorframework.io.bundle.package.v1: test + operators.operatorframework.io.bundle.channels.v1: beta + operators.operatorframework.io.metrics.builder: operator-sdk-v1.28.0 + operators.operatorframework.io.metrics.mediatype.v1: metrics+v1 + operators.operatorframework.io.metrics.project_layout: unknown diff --git a/testdata/images/bundles/test-operator/v1.2.0/manifests/testoperator.clusterserviceversion.yaml b/testdata/images/bundles/test-operator/v1.2.0/manifests/testoperator.clusterserviceversion.yaml index 6fc91adf3c..7d9e560bad 100644 --- a/testdata/images/bundles/test-operator/v1.2.0/manifests/testoperator.clusterserviceversion.yaml +++ b/testdata/images/bundles/test-operator/v1.2.0/manifests/testoperator.clusterserviceversion.yaml @@ -89,7 +89,7 @@ spec: port: 80 initialDelaySeconds: 1 periodSeconds: 1 - serviceAccountName: simple-bundle-manager + serviceAccountName: simple-bundle-manager clusterPermissions: - rules: - apiGroups: diff --git a/testdata/images/catalogs/test-catalog/v1/configs/catalog.yaml b/testdata/images/catalogs/test-catalog/v1/configs/catalog.yaml index 437175a4e3..49340a2f96 100644 --- a/testdata/images/catalogs/test-catalog/v1/configs/catalog.yaml +++ b/testdata/images/catalogs/test-catalog/v1/configs/catalog.yaml @@ -7,6 +7,7 @@ name: alpha package: test entries: - name: test-operator.1.0.0 + - name: test-operator.1.0.2 --- schema: olm.channel name: beta @@ -39,6 +40,16 @@ properties: version: 1.0.1 --- schema: olm.bundle +name: test-operator.1.0.2 +package: test +image: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/test-operator:v1.0.2 +properties: + - type: olm.package + value: + packageName: test + version: 1.0.2 +--- +schema: olm.bundle name: test-operator.1.2.0 package: test image: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/test-operator:v1.2.0