✨ 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)
operator-framework · Sep 6, 2024 · 0b0c5dd · 0b0c5dd
1 parent 6584e7e
commit 0b0c5dd
Show file tree

Hide file tree

Showing 7 changed files with 190 additions and 247 deletions.
diff --git a/api/core/v1alpha1/clustercatalog_types.go b/api/core/v1alpha1/clustercatalog_types.go
@@ -26,15 +26,17 @@ type SourceType string
 const (
 	SourceTypeImage SourceType = "image"
 
-	TypeUnpacked = "Unpacked"
-	TypeDelete   = "Delete"
-
-	ReasonUnpackPending       = "UnpackPending"
-	ReasonUnpacking           = "Unpacking"
-	ReasonUnpackSuccessful    = "UnpackSuccessful"
-	ReasonUnpackFailed        = "UnpackFailed"
-	ReasonStorageFailed       = "FailedToStore"
-	ReasonStorageDeleteFailed = "FailedToDelete"
+	TypeProgressing = "Progressing"
+	TypeServing     = "Serving"
+
+	// Serving reasons
+	ReasonAvailable   = "Available"
+	ReasonUnavailable = "Unavailable"
+
+	// Progressing reasons
+	ReasonSucceeded     = "Succeeded"
+	ReasonRetrying      = "Retrying"
+	ReasonUnrecoverable = "Unrecoverable"
 
 	MetadataNameLabel = "olm.operatorframework.io/metadata.name"
 )

diff --git a/internal/controllers/core/clustercatalog_controller.go b/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{}, updateStatusProgressError(&catalog.Status, err)
 		}
+		updateStatusNotServing(&catalog.Status)
 		if err := r.Unpacker.Cleanup(ctx, catalog); err != nil {
-			return ctrl.Result{}, updateStatusStorageDeleteError(&catalog.Status, err)
+			return ctrl.Result{}, updateStatusProgressError(&catalog.Status, 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{}, updateStatusProgressError(&catalog.Status, fmt.Errorf("source bundle content: %v", 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{}, updateStatusProgressError(&catalog.Status, 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.Status)
+		updateStatusServing(&catalog.Status, unpackResult, contentURL, catalog.Generation, lastUnpacked)
 
 		var requeueAfter time.Duration
 		switch catalog.Spec.Source.Type {
@@ -180,73 +176,57 @@ 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) {
+func updateStatusProgressing(status *v1alpha1.ClusterCatalogStatus) {
 	status.ResolvedSource = nil
 	meta.SetStatusCondition(&status.Conditions, metav1.Condition{
-		Type:    v1alpha1.TypeUnpacked,
-		Status:  metav1.ConditionFalse,
-		Reason:  v1alpha1.ReasonUnpackPending,
-		Message: result.Message,
+		Type:   v1alpha1.TypeProgressing,
+		Status: metav1.ConditionFalse,
+		Reason: v1alpha1.ReasonSucceeded,
 	})
 }
 
-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 updateStatusProgressError(status *v1alpha1.ClusterCatalogStatus, err error) error {
+	var unrecov *catalogderrors.Unrecoverable
+	if errors.As(err, &unrecov) {
+		meta.SetStatusCondition(&status.Conditions, metav1.Condition{
+			Type:    v1alpha1.TypeProgressing,
+			Status:  metav1.ConditionFalse,
+			Reason:  v1alpha1.ReasonUnrecoverable,
+			Message: err.Error(),
+		})
+	} else {
+		meta.SetStatusCondition(&status.Conditions, metav1.Condition{
+			Type:    v1alpha1.TypeProgressing,
+			Status:  metav1.ConditionTrue,
+			Reason:  v1alpha1.ReasonRetrying,
+			Message: err.Error(),
+		})
+	}
+	return err
 }
 
-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,
-		Status:  metav1.ConditionTrue,
-		Reason:  v1alpha1.ReasonUnpackSuccessful,
-		Message: result.Message,
+		Type:   v1alpha1.TypeServing,
+		Status: metav1.ConditionTrue,
+		Reason: v1alpha1.ReasonAvailable,
 	})
 }
 
-func updateStatusUnpackFailing(status *v1alpha1.ClusterCatalogStatus, err error) error {
-	status.ResolvedSource = nil
+func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus) {
 	meta.SetStatusCondition(&status.Conditions, metav1.Condition{
-		Type:    v1alpha1.TypeUnpacked,
-		Status:  metav1.ConditionFalse,
-		Reason:  v1alpha1.ReasonUnpackFailed,
-		Message: err.Error(),
+		Type:   v1alpha1.TypeServing,
+		Status: metav1.ConditionFalse,
+		Reason: v1alpha1.ReasonUnavailable,
 	})
-	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()),
-	})
-	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()),
-	})
-	return err
 }
 
 func (r *ClusterCatalogReconciler) needsUnpacking(catalog *v1alpha1.ClusterCatalog) bool {
@@ -258,6 +238,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 +261,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
+}