⚠ Status condition clean up

[RFC: ClusterCatalog Status Conditions](https://docs.google.com/document/d/1Kg8ovX8d25OqGa5utwcKbX3nN3obBl2KIYJHYz6IlKU/edit) implementation.

Closes [363](#363)
Closes [364](#364)
Closes [365](#365)

README.md

@@ -87,9 +87,14 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A
       Conditions:
         Last Transition Time:  2023-06-23T18:35:13Z
         Message:               
-        Reason:                Unpacking
+        Reason:                Progressing
         Status:                False
-        Type:                  Unpacked
+        Type:                  Succeeded
+        Last Transition Time:  2023-06-23T18:35:13Z
+        Message:               
+        Reason:                Serving
+        Status:                True
+        Type:                  Available
     Events:                    <none>
     ```
 

api/core/v1alpha1/clustercatalog_types.go

@@ -26,15 +26,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"
+	ReasonUnrecoverable = "Unrecoverable"
 
 	MetadataNameLabel = "olm.operatorframework.io/metadata.name"
 )
@@ -43,6 +45,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 is the Schema for the ClusterCatalogs API

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

@@ -86,10 +86,15 @@ of a catalog can be read from:
   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
+      message: successfully unpacked and stored content from "quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76"
+      reason: Succeeded
+      status: "False"
+      type: Progressing
+    - lastTransitionTime: "2023-09-14T15:21:18Z"
+    message: Content from "quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76" is being served
+    reason: Available
+    status: "True"
+    type: Serving
     contentURL: https://catalogd-catalogserver.olmv1-system.svc/catalogs/operatorhubio/all.json
     resolvedSource:
       image:

hack/scripts/demo-script.sh

@@ -19,7 +19,7 @@ kubectl apply -f config/samples/core_v1alpha1_catalog.yaml
 # shows catalog-sample
 kubectl get catalog -A 
 # waiting for catalog to report ready status
-time kubectl wait --for=condition=Unpacked catalog/operatorhubio --timeout=1m
+time kubectl wait --for=condition=Serving catalog/operatorhubio --timeout=1m
 
 # port forward the catalogd-catalogserver service to interact with the HTTP server serving catalog contents
 (kubectl -n olmv1-system port-forward svc/catalogd-catalogserver 8080:80)&

hack/scripts/gzip-demo-script.sh

@@ -9,7 +9,7 @@ kubectl apply -f $HOME/devel/tmp/operatorhubio-clustercatalog.yaml
 # shows catalog
 kubectl get clustercatalog -A 
 # waiting for clustercatalog to report ready status
-time kubectl wait --for=condition=Unpacked clustercatalog/operatorhubio --timeout=1m
+time kubectl wait --for=condition=Serving clustercatalog/operatorhubio --timeout=1m
 
 # port forward the catalogd-catalogserver service to interact with the HTTP server serving catalog contents
 (kubectl -n olmv1-system port-forward svc/catalogd-catalogserver 8080:443)&

internal/controllers/core/clustercatalog_controller.go

@@ -124,10 +124,11 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp
 	}
 	if !catalog.DeletionTimestamp.IsZero() {
 		if err := r.Storage.Delete(catalog.Name); err != nil {
-			return ctrl.Result{}, updateStatusStorageDeleteError(&catalog.Status, err)
+			return ctrl.Result{}, updateStatusProgressing(catalog, err)
 		}
+		updateStatusNotServing(&catalog.Status)
 		if err := r.Unpacker.Cleanup(ctx, catalog); err != nil {
-			return ctrl.Result{}, updateStatusStorageDeleteError(&catalog.Status, err)
+			return ctrl.Result{}, updateStatusProgressing(catalog, err)
 		}
 		controllerutil.RemoveFinalizer(catalog, fbcDeletionFinalizer)
 		return ctrl.Result{}, nil
@@ -139,24 +140,18 @@ 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))
+		return ctrl.Result{}, updateStatusProgressing(catalog, err)
 	}
 
 	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))
+			return ctrl.Result{}, updateStatusProgressing(catalog, fmt.Errorf("error storing fbc: %v", err))
 		}
 		contentURL = r.Storage.ContentURL(catalog.Name)
 
@@ -168,7 +163,8 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp
 			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 {
@@ -180,73 +176,60 @@ 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,
-		Status:  metav1.ConditionFalse,
-		Reason:  v1alpha1.ReasonUnpackPending,
-		Message: result.Message,
-	})
-}
-
-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,
-	})
+func updateStatusProgressing(catalog *v1alpha1.ClusterCatalog, err error) error {
+	var unrecov *catalogderrors.Unrecoverable
+	if err == nil {
+		catalog.Status.ResolvedSource = nil
+		meta.SetStatusCondition(&catalog.Status.Conditions, metav1.Condition{
+			Type:    v1alpha1.TypeProgressing,
+			Status:  metav1.ConditionFalse,
+			Reason:  v1alpha1.ReasonSucceeded,
+			Message: fmt.Sprintf("Successfully unpacked and stored content from %s", catalog.Spec.Source.Image.Ref),
+		})
+	} else if errors.As(err, &unrecov) {
+		meta.SetStatusCondition(&catalog.Status.Conditions, metav1.Condition{
+			Type:    v1alpha1.TypeProgressing,
+			Status:  metav1.ConditionFalse,
+			Reason:  v1alpha1.ReasonUnrecoverable,
+			Message: err.Error(),
+		})
+		return err
+	} else {
+		meta.SetStatusCondition(&catalog.Status.Conditions, metav1.Condition{
+			Type:    v1alpha1.TypeProgressing,
+			Status:  metav1.ConditionTrue,
+			Reason:  v1alpha1.ReasonRetrying,
+			Message: err.Error(),
+		})
+		return err
+	}
+	return nil
 }
 
-func updateStatusUnpacked(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string, generation int64, lastUnpacked metav1.Time) {
+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(),
-	})
-	return err
-}
-
-func updateStatusStorageError(status *v1alpha1.ClusterCatalogStatus, err error) error {
-	status.ResolvedSource = nil
-	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()),
+		Reason:  v1alpha1.ReasonAvailable,
+		Message: fmt.Sprintf("Content from %s is being served", status.ResolvedSource.Image.Ref),
 	})
-	return err
 }
 
-func updateStatusStorageDeleteError(status *v1alpha1.ClusterCatalogStatus, err error) error {
+func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus) {
+	status.ContentURL = ""
 	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 {
@@ -258,6 +241,9 @@ func (r *ClusterCatalogReconciler) needsUnpacking(catalog *v1alpha1.ClusterCatal
 	if !r.Storage.ContentExists(catalog.Name) {
 		return true
 	}
+	if !servingStatusConditionExists(catalog) {
+		return true
+	}
 	// if there is no spec.Source.Image, don't unpack again
 	if catalog.Spec.Source.Image == nil {
 		return false
@@ -278,3 +264,12 @@ func (r *ClusterCatalogReconciler) needsUnpacking(catalog *v1alpha1.ClusterCatal
 	// time to unpack
 	return true
 }
+
+func servingStatusConditionExists(catalog *v1alpha1.ClusterCatalog) bool {
+	for _, cond := range catalog.Status.Conditions {
+		if cond.Type == v1alpha1.TypeServing && cond.Status == metav1.ConditionTrue {
+			return true
+		}
+	}
+	return false
+}