⚠ Status condition clean up

README.md

@@ -40,58 +40,48 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A
     ```sh
     Name:         operatorhubio
     Namespace:    
-    Labels:       <none>
+    Labels:       olm.operatorframework.io/metadata.name=operatorhubio
     Annotations:  <none>
     API Version:  olm.operatorframework.io/v1alpha1
     Kind:         ClusterCatalog
     Metadata:
-      Creation Timestamp:  2023-06-23T18:35:13Z
-      Generation:          1
-      Managed Fields:
-        API Version:  olm.operatorframework.io/v1alpha1
-        Fields Type:  FieldsV1
-        fieldsV1:
-          f:metadata:
-            f:annotations:
-              .:
-              f:kubectl.kubernetes.io/last-applied-configuration:
-          f:spec:
-            .:
-            f:source:
-              .:
-              f:image:
-                .:
-                f:ref:
-              f:type:
-        Manager:      kubectl-client-side-apply
-        Operation:    Update
-        Time:         2023-06-23T18:35:13Z
-        API Version:  olm.operatorframework.io/v1alpha1
-        Fields Type:  FieldsV1
-        fieldsV1:
-          f:status:
-            .:
-            f:conditions:
-        Manager:         manager
-        Operation:       Update
-        Subresource:     status
-        Time:            2023-06-23T18:35:43Z
-      Resource Version:  1397
-      UID:               709cee9d-c669-46e1-97d0-e97dcce8f388
+      Creation Timestamp:  2024-09-12T13:37:04Z
+      Finalizers:
+        olm.operatorframework.io/delete-server-cache
+      Generation:        1
+      Resource Version:  961
+      UID:               fa6bb9cf-1a36-4189-a7a0-83284c3f6f55
     Spec:
+      Priority:  0
       Source:
         Image:
-          Ref:  quay.io/operatorhubio/catalog:latest
-        Type:   image
+          Poll Interval:  10m0s
+          Ref:            quay.io/operatorhubio/catalog:latest
+        Type:             image
     Status:
       Conditions:
-        Last Transition Time:  2023-06-23T18:35:13Z
-        Message:               
-        Reason:                Unpacking
+        Last Transition Time:  2024-09-12T13:37:53Z
+        Message:               Successfully unpacked and stored content from quay.io/operatorhubio/catalog:latest
+        Reason:                Succeeded
         Status:                False
-        Type:                  Unpacked
-    Events:                    <none>
-    ```
+        Type:                  Progressing
+        Last Transition Time:  2024-09-12T13:37:53Z
+        Message:               Content from quay.io/operatorhubio/catalog:latest is being served
+        Reason:                Available
+        Status:                True
+        Type:                  Serving
+      Content URL:             https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio/all.json
+      Last Unpacked:           2024-09-12T13:37:52Z
+      Observed Generation:     1
+      Resolved Source:
+        Image:
+          Last Poll Attempt:  2024-09-12T13:37:52Z
+          Last Unpacked:      2024-09-12T13:37:52Z
+          Ref:                quay.io/operatorhubio/catalog:latest
+          Resolved Ref:       quay.io/operatorhubio/catalog@sha256:4453a361198d39d0390fd8c1a7f07b5a5a3ae1e8dac9979ef0c4eba46299df16
+        Type:                 image
+   Events:                   <none>
+   ```
 
 1. Port forward the `catalogd-service` service in the `olmv1-system` namespace:
     ```sh

api/core/v1alpha1/clustercatalog_types.go

@@ -27,15 +27,17 @@ type SourceType string
 const (
 	SourceTypeImage SourceType = "image"
 
-	TypeUnpacked = "Unpacked"
-	TypeDelete   = "Delete"
+	TypeProgressing = "Progressing"
+	TypeServing     = "Serving"
 
-	ReasonUnpackPending       = "UnpackPending"
-	ReasonUnpacking           = "Unpacking"
-	ReasonUnpackSuccessful    = "UnpackSuccessful"
-	ReasonUnpackFailed        = "UnpackFailed"
-	ReasonStorageFailed       = "FailedToStore"
-	ReasonStorageDeleteFailed = "FailedToDelete"
+	// Serving reasons
+	ReasonAvailable   = "Available"
+	ReasonUnavailable = "Unavailable"
+
+	// Progressing reasons
+	ReasonSucceeded = "Succeeded"
+	ReasonRetrying  = "Retrying"
+	ReasonTerminal  = "Terminal"
 
 	MetadataNameLabel = "olm.operatorframework.io/metadata.name"
 )
@@ -44,6 +46,7 @@ const (
 //+kubebuilder:resource:scope=Cluster
 //+kubebuilder:subresource:status
 //+kubebuilder:printcolumn:name=LastUnpacked,type=date,JSONPath=`.status.lastUnpacked`
+//+kubebuilder:printcolumn:name="Serving",type=string,JSONPath=`.status.conditions[?(@.type=="Serving")].status`
 //+kubebuilder:printcolumn:name=Age,type=date,JSONPath=`.metadata.creationTimestamp`
 
 // ClusterCatalog enables users to make File-Based Catalog (FBC) catalog data available to the cluster.

config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml

@@ -18,6 +18,9 @@ spec:
     - jsonPath: .status.lastUnpacked
       name: LastUnpacked
       type: date
+    - jsonPath: .status.conditions[?(@.type=="Serving")].status
+      name: Serving
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date

docs/fetching-catalog-contents.md

@@ -84,13 +84,9 @@ of a catalog can be read from:
 
 ```yaml
   status:
-    conditions:
-    - lastTransitionTime: "2023-09-14T15:21:18Z"
-      message: successfully unpacked the catalog image "quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76"
-      reason: UnpackSuccessful
-      status: "True"
-      type: Unpacked
-    contentURL: https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio/all.json
+    .
+    .
+    contentURL: https://catalogd-catalogserver.olmv1-system.svc/catalogs/operatorhubio/all.json
     resolvedSource:
       image:
         ref: quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76

internal/controllers/core/clustercatalog_controller.go

@@ -30,6 +30,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/operator-framework/catalogd/api/core/v1alpha1"
 	"github.com/operator-framework/catalogd/internal/source"
@@ -124,13 +125,6 @@ func (r *ClusterCatalogReconciler) SetupWithManager(mgr ctrl.Manager) error {
 func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.ClusterCatalog) (ctrl.Result, error) {
 	finalizeResult, err := r.Finalizers.Finalize(ctx, catalog)
 	if err != nil {
-		// TODO: For now, this error handling follows the pattern of other error handling.
-		//  Namely: zero just about everything out, throw our hands up, and return an error.
-		//  This is not ideal, and we should consider a more nuanced approach that resolves
-		//  as much status as possible before returning, or at least keeps previous state if
-		//  it is properly labeled with its observed generation.
-		_ = updateStatusStorageError(&catalog.Status, err)
-		_ = updateStatusUnpackFailing(&catalog.Status, err)
 		return ctrl.Result{}, err
 	}
 	if finalizeResult.Updated || finalizeResult.StatusUpdated {
@@ -145,36 +139,33 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp
 
 	unpackResult, err := r.Unpacker.Unpack(ctx, catalog)
 	if err != nil {
-		return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("source bundle content: %v", err))
+		unpackErr := fmt.Errorf("source bundle content: %w", err)
+		updateStatusProgressing(catalog, unpackErr)
+		return ctrl.Result{}, unpackErr
 	}
 
 	switch unpackResult.State {
-	case source.StatePending:
-		updateStatusUnpackPending(&catalog.Status, unpackResult)
-		return ctrl.Result{}, nil
-	case source.StateUnpacking:
-		updateStatusUnpacking(&catalog.Status, unpackResult)
-		return ctrl.Result{}, nil
 	case source.StateUnpacked:
 		contentURL := ""
 		// TODO: We should check to see if the unpacked result has the same content
 		//   as the already unpacked content. If it does, we should skip this rest
 		//   of the unpacking steps.
 		err := r.Storage.Store(ctx, catalog.Name, unpackResult.FS)
 		if err != nil {
-			return ctrl.Result{}, updateStatusStorageError(&catalog.Status, fmt.Errorf("error storing fbc: %v", err))
+			storageErr := fmt.Errorf("error storing fbc: %v", err)
+			updateStatusProgressing(catalog, storageErr)
+			return ctrl.Result{}, storageErr
 		}
 		contentURL = r.Storage.ContentURL(catalog.Name)
 
 		var lastUnpacked metav1.Time
 
 		if unpackResult != nil && unpackResult.ResolvedSource != nil && unpackResult.ResolvedSource.Image != nil {
 			lastUnpacked = unpackResult.ResolvedSource.Image.LastUnpacked
-		} else {
-			lastUnpacked = metav1.Time{}
 		}
 
-		updateStatusUnpacked(&catalog.Status, unpackResult, contentURL, catalog.Generation, lastUnpacked)
+		updateStatusProgressing(catalog, nil)
+		updateStatusServing(&catalog.Status, unpackResult, contentURL, catalog.Generation, lastUnpacked)
 
 		var requeueAfter time.Duration
 		switch catalog.Spec.Source.Type {
@@ -186,73 +177,54 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp
 
 		return ctrl.Result{RequeueAfter: requeueAfter}, nil
 	default:
-		return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("unknown unpack state %q: %v", unpackResult.State, err))
+		panic(fmt.Sprintf("unknown unpack state %q", unpackResult.State))
 	}
 }
 
-func updateStatusUnpackPending(status *v1alpha1.ClusterCatalogStatus, result *source.Result) {
-	status.ResolvedSource = nil
-	meta.SetStatusCondition(&status.Conditions, metav1.Condition{
-		Type:    v1alpha1.TypeUnpacked,
+func updateStatusProgressing(catalog *v1alpha1.ClusterCatalog, err error) {
+	progressingCond := metav1.Condition{
+		Type:    v1alpha1.TypeProgressing,
 		Status:  metav1.ConditionFalse,
-		Reason:  v1alpha1.ReasonUnpackPending,
-		Message: result.Message,
-	})
-}
+		Reason:  v1alpha1.ReasonSucceeded,
+		Message: "Successfully unpacked and stored content from resolved source",
+	}
 
-func updateStatusUnpacking(status *v1alpha1.ClusterCatalogStatus, result *source.Result) {
-	status.ResolvedSource = nil
-	meta.SetStatusCondition(&status.Conditions, metav1.Condition{
-		Type:    v1alpha1.TypeUnpacked,
-		Status:  metav1.ConditionFalse,
-		Reason:  v1alpha1.ReasonUnpacking,
-		Message: result.Message,
-	})
-}
+	if err != nil {
+		progressingCond.Status = metav1.ConditionTrue
+		progressingCond.Reason = v1alpha1.ReasonRetrying
+		progressingCond.Message = err.Error()
+	}
 
-func updateStatusUnpacked(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string, generation int64, lastUnpacked metav1.Time) {
+	if errors.Is(err, reconcile.TerminalError(nil)) {
+		progressingCond.Status = metav1.ConditionFalse
+		progressingCond.Reason = v1alpha1.ReasonTerminal
+	}
+
+	meta.SetStatusCondition(&catalog.Status.Conditions, progressingCond)
+}
+func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string, generation int64, unpackedAt metav1.Time) {
 	status.ResolvedSource = result.ResolvedSource
 	status.ContentURL = contentURL
 	status.ObservedGeneration = generation
-	status.LastUnpacked = lastUnpacked
+	status.LastUnpacked = unpackedAt
 	meta.SetStatusCondition(&status.Conditions, metav1.Condition{
-		Type:    v1alpha1.TypeUnpacked,
+		Type:    v1alpha1.TypeServing,
 		Status:  metav1.ConditionTrue,
-		Reason:  v1alpha1.ReasonUnpackSuccessful,
-		Message: result.Message,
-	})
-}
-
-func updateStatusUnpackFailing(status *v1alpha1.ClusterCatalogStatus, err error) error {
-	status.ResolvedSource = nil
-	meta.SetStatusCondition(&status.Conditions, metav1.Condition{
-		Type:    v1alpha1.TypeUnpacked,
-		Status:  metav1.ConditionFalse,
-		Reason:  v1alpha1.ReasonUnpackFailed,
-		Message: err.Error(),
+		Reason:  v1alpha1.ReasonAvailable,
+		Message: "Serving desired content from resolved source",
 	})
-	return err
 }
 
-func updateStatusStorageError(status *v1alpha1.ClusterCatalogStatus, err error) error {
+func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus) {
 	status.ResolvedSource = nil
+	status.ContentURL = ""
+	status.ObservedGeneration = 0
+	status.LastUnpacked = metav1.Time{}
 	meta.SetStatusCondition(&status.Conditions, metav1.Condition{
-		Type:    v1alpha1.TypeUnpacked,
-		Status:  metav1.ConditionFalse,
-		Reason:  v1alpha1.ReasonStorageFailed,
-		Message: fmt.Sprintf("failed to store bundle: %s", err.Error()),
-	})
-	return err
-}
-
-func updateStatusStorageDeleteError(status *v1alpha1.ClusterCatalogStatus, err error) error {
-	meta.SetStatusCondition(&status.Conditions, metav1.Condition{
-		Type:    v1alpha1.TypeDelete,
-		Status:  metav1.ConditionFalse,
-		Reason:  v1alpha1.ReasonStorageDeleteFailed,
-		Message: fmt.Sprintf("failed to delete storage: %s", err.Error()),
+		Type:   v1alpha1.TypeServing,
+		Status: metav1.ConditionFalse,
+		Reason: v1alpha1.ReasonUnavailable,
 	})
-	return err
 }
 
 func (r *ClusterCatalogReconciler) needsUnpacking(catalog *v1alpha1.ClusterCatalog) bool {
@@ -308,12 +280,15 @@ func NewFinalizers(localStorage storage.Instance, unpacker source.Unpacker) (crf
 			panic("could not convert object to clusterCatalog")
 		}
 		if err := localStorage.Delete(catalog.Name); err != nil {
-			return crfinalizer.Result{}, updateStatusStorageDeleteError(&catalog.Status, err)
+			updateStatusProgressing(catalog, err)
+			return crfinalizer.Result{StatusUpdated: true}, err
 		}
+		updateStatusNotServing(&catalog.Status)
 		if err := unpacker.Cleanup(ctx, catalog); err != nil {
-			return crfinalizer.Result{}, updateStatusStorageDeleteError(&catalog.Status, err)
+			updateStatusProgressing(catalog, err)
+			return crfinalizer.Result{StatusUpdated: true}, err
 		}
-		return crfinalizer.Result{}, nil
+		return crfinalizer.Result{StatusUpdated: true}, nil
 	}))
 	if err != nil {
 		return f, err