Propagate ClusterExtensionRevision conditions to ClusterExtension status (#2281)

* `ClusterExtension`'s `Available` and `Progressing` conditions are equal to the counterparts on the latest installed active revision
* When a new revision is rolling out, mirror its `Progressing` condition to its counterpart on `ClusterExtension` to improve visibility
  and to signal that a transition is in progress
* Adds `.status.activeRevisions` field to the `ClusterExtension` to track the active revisions and mirrors `Available` and `Progressing` conditions for those
  not marked yet as installed

This changes opens up a possibility for an observer to get insights on `ClusterExtension` update process:
* When Reason on `Progressing` condition flips to `RollingOut`, an update is in progress
* More then one revision could be listed under `.status.activeRevisions` during upgrade (installed + those in progress)
* `Available` and `Progressing` conditions of an active, but not yet available revision, are listed under `.status.activeRevisions.conditions`
  so that can be easier to understand update issues

The new behavior is marked as experimental behind Boxcutter feature gate.

Co-authored-by: Predrag Knezevic <pknezevi@redhat.com>

Author: perdasilva

api/v1/clusterextension_types.go

@@ -472,6 +472,21 @@ type BundleMetadata struct {
 	Version string `json:"version"`
 }
 
+// RevisionStatus defines the observed state of a ClusterExtensionRevision.
+type RevisionStatus struct {
+	// name of the ClusterExtensionRevision resource
+	Name string `json:"name"`
+	// conditions optionally expose Progressing and Available condition of the revision,
+	// in case when it is not yet marked as successfully installed (condition Succeeded is not set to True).
+	// Given that a ClusterExtension should remain available during upgrades, an observer may use these conditions
+	// to get more insights about reasons for its current state.
+	//
+	// +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.
@@ -504,6 +519,14 @@ type ClusterExtensionStatus struct {
 	//
 	// +optional
 	Install *ClusterExtensionInstallStatus `json:"install,omitempty"`
+
+	// activeRevisions holds a list of currently active (non-archived) ClusterExtensionRevisions,
+	// including both installed and rolling out revisions.
+	// +listType=map
+	// +listMapKey=name
+	// +optional
+	// <opcon:experimental>
+	ActiveRevisions []RevisionStatus `json:"activeRevisions,omitempty"`
 }
 
 // ClusterExtensionInstallStatus is a representation of the status of the identified bundle.

api/v1/zz_generated.deepcopy.go

@@ -628,7 +628,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl
 		controllers.RetrieveRevisionStates(revisionStatesGetter),
 		controllers.ResolveBundle(c.resolver),
 		controllers.UnpackBundle(c.imagePuller, c.imageCache),
-		controllers.ApplyBundle(appl),
+		controllers.ApplyBundleWithBoxcutter(appl),
 	}
 
 	baseDiscoveryClient, err := discovery.NewDiscoveryClientForConfig(c.mgr.GetConfig())

cmd/operator-controller/main.go

@@ -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.<br />The Installed condition represents whether or not the bundle has been installed for this ClusterExtension.<br />When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.<br />When Installed is False and the Reason is Failed, the bundle has failed to install.<br />The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.<br />When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.<br />When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.<br />When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.<br /><opcon:experimental:description><br />When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.<br /></opcon:experimental:description><br />When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.<br />These are indications from a package owner to guide users away from a particular package, channel, or bundle.<br />BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.<br />ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.<br />PackageDeprecated is set if the requested package is marked deprecated in the catalog.<br />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_ | activeRevisions holds a list of currently active (non-archived) ClusterExtensionRevisions,<br />including both installed and rolling out revisions.<br /><opcon:experimental> |  |  |
 
 
 
@@ -435,6 +436,23 @@ _Appears in:_
 | `ref` _string_ | ref contains the resolved image digest-based reference.<br />The digest format is used so users can use other tooling to fetch the exact<br />OCI manifests that were used to extract the catalog contents. |  | MaxLength: 1000 <br />Required: \{\} <br /> |
 
 
+#### RevisionStatus
+
+
+
+RevisionStatus defines the observed state of a ClusterExtensionRevision.
+
+
+
+_Appears in:_
+- [ClusterExtensionStatus](#clusterextensionstatus)
+
+| Field | Description | Default | Validation |
+| --- | --- | --- | --- |
+| `name` _string_ | name of the ClusterExtensionRevision resource |  |  |
+| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions optionally expose Progressing and Available condition of the revision,<br />in case when it is not yet marked as successfully installed (condition Succeeded is not set to True).<br />Given that a ClusterExtension should remain available during upgrades, an observer may use these conditions<br />to get more insights about reasons for its current state. |  |  |
+
+
 #### ServiceAccountReference
 
 

docs/api-reference/olmv1-api-reference.md

@@ -506,6 +506,88 @@ spec:
             description: status is an optional field that defines the observed state
               of the ClusterExtension.
             properties:
+              activeRevisions:
+                description: |-
+                  activeRevisions holds a list of currently active (non-archived) ClusterExtensionRevisions,
+                  including both installed and rolling out revisions.
+                items:
+                  description: RevisionStatus defines the observed state of a ClusterExtensionRevision.
+                  properties:
+                    conditions:
+                      description: |-
+                        conditions optionally expose Progressing and Available condition of the revision,
+                        in case when it is not yet marked as successfully installed (condition Succeeded is not set to True).
+                        Given that a ClusterExtension should remain available during upgrades, an observer may use these conditions
+                        to get more insights about reasons for its current state.
+                      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:
+                      description: name of the ClusterExtensionRevision resource
+                      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.

helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml

@@ -399,8 +399,6 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 		default:
 			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
 	}

internal/operator-controller/applier/boxcutter.go

@@ -25,6 +25,7 @@ import (
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	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"
@@ -58,8 +59,10 @@ func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, e
 		//   is fairly decoupled from this code where we get the annotations back out. We may want to co-locate
 		//   the set/get logic a bit better to make it more maintainable and less likely to get out of sync.
 		rm := &RevisionMetadata{
-			Package: rev.Annotations[labels.PackageNameKey],
-			Image:   rev.Annotations[labels.BundleReferenceKey],
+			RevisionName: rev.Name,
+			Package:      rev.Annotations[labels.PackageNameKey],
+			Image:        rev.Annotations[labels.BundleReferenceKey],
+			Conditions:   rev.Status.Conditions,
 			BundleMetadata: ocv1.BundleMetadata{
 				Name:    rev.Annotations[labels.BundleNameKey],
 				Version: rev.Annotations[labels.BundleVersionKey],
@@ -89,3 +92,61 @@ func MigrateStorage(m StorageMigrator) ReconcileStepFunc {
 		return nil, nil
 	}
 }
+
+func ApplyBundleWithBoxcutter(a Applier) ReconcileStepFunc {
+	return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) {
+		l := log.FromContext(ctx)
+		revisionAnnotations := map[string]string{
+			labels.BundleNameKey:      state.resolvedRevisionMetadata.Name,
+			labels.PackageNameKey:     state.resolvedRevisionMetadata.Package,
+			labels.BundleVersionKey:   state.resolvedRevisionMetadata.Version,
+			labels.BundleReferenceKey: state.resolvedRevisionMetadata.Image,
+		}
+		objLbls := map[string]string{
+			labels.OwnerKindKey: ocv1.ClusterExtensionKind,
+			labels.OwnerNameKey: ext.GetName(),
+		}
+
+		l.Info("applying bundle contents")
+		if _, _, err := a.Apply(ctx, state.imageFS, ext, objLbls, revisionAnnotations); err != nil {
+			// If there was an error applying the resolved bundle,
+			// report the error via the Progressing condition.
+			setStatusProgressing(ext, wrapErrorWithResolutionInfo(state.resolvedRevisionMetadata.BundleMetadata, err))
+			return nil, err
+		}
+
+		// Mirror Available/Progressing conditions from the installed revision
+		if i := state.revisionStates.Installed; i != nil {
+			for _, cndType := range []string{ocv1.ClusterExtensionRevisionTypeAvailable, ocv1.ClusterExtensionRevisionTypeProgressing} {
+				if cnd := apimeta.FindStatusCondition(i.Conditions, cndType); cnd != nil {
+					cnd.ObservedGeneration = ext.GetGeneration()
+					apimeta.SetStatusCondition(&ext.Status.Conditions, *cnd)
+				}
+			}
+			ext.Status.Install = &ocv1.ClusterExtensionInstallStatus{
+				Bundle: i.BundleMetadata,
+			}
+			ext.Status.ActiveRevisions = []ocv1.RevisionStatus{{Name: i.RevisionName}}
+		}
+		for idx, r := range state.revisionStates.RollingOut {
+			rs := ocv1.RevisionStatus{Name: r.RevisionName}
+			for _, cndType := range []string{ocv1.ClusterExtensionRevisionTypeAvailable, ocv1.ClusterExtensionRevisionTypeProgressing} {
+				if cnd := apimeta.FindStatusCondition(r.Conditions, cndType); cnd != nil {
+					cnd.ObservedGeneration = ext.GetGeneration()
+					apimeta.SetStatusCondition(&rs.Conditions, *cnd)
+				}
+			}
+			// Mirror Progressing condition from the latest active revision
+			if idx == len(state.revisionStates.RollingOut)-1 {
+				if pcnd := apimeta.FindStatusCondition(r.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing); pcnd != nil {
+					pcnd.ObservedGeneration = ext.GetGeneration()
+					apimeta.SetStatusCondition(&ext.Status.Conditions, *pcnd)
+				}
+			}
+			ext.Status.ActiveRevisions = append(ext.Status.ActiveRevisions, rs)
+		}
+
+		setInstalledStatusFromRevisionStates(ext, state.revisionStates)
+		return nil, nil
+	}
+}

internal/operator-controller/controllers/boxcutter_reconcile_steps.go

@@ -332,9 +332,11 @@ func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger) crh
 }
 
 type RevisionMetadata struct {
-	Package string
-	Image   string
+	RevisionName string
+	Package      string
+	Image        string
 	ocv1.BundleMetadata
+	Conditions []metav1.Condition
 }
 
 type RevisionStates struct {