From 7d2f636d6ea2f05a785d035e410ba8bc45359cd7 Mon Sep 17 00:00:00 2001
From: Ankita Thomas <ankithom@redhat.com>
Date: Tue, 10 Sep 2024 12:37:45 -0400
Subject: [PATCH] remove serving condition check for unpacking, refactor
 updateStatusProgressing

Signed-off-by: Ankita Thomas <ankithom@redhat.com>
---
 .../core/clustercatalog_controller.go         | 58 ++++++++-----------
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/internal/controllers/core/clustercatalog_controller.go b/internal/controllers/core/clustercatalog_controller.go
index 7aafcefd..3da68503 100644
--- a/internal/controllers/core/clustercatalog_controller.go
+++ b/internal/controllers/core/clustercatalog_controller.go
@@ -124,11 +124,13 @@ 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{}, updateStatusProgressing(catalog, err)
+			updateStatusProgressing(catalog, err)
+			return ctrl.Result{}, err
 		}
 		updateStatusNotServing(&catalog.Status)
 		if err := r.Unpacker.Cleanup(ctx, catalog); err != nil {
-			return ctrl.Result{}, updateStatusProgressing(catalog, err)
+			updateStatusProgressing(catalog, err)
+			return ctrl.Result{}, err
 		}
 		controllerutil.RemoveFinalizer(catalog, fbcDeletionFinalizer)
 		return ctrl.Result{}, nil
@@ -140,7 +142,9 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp
 
 	unpackResult, err := r.Unpacker.Unpack(ctx, catalog)
 	if err != nil {
-		return ctrl.Result{}, updateStatusProgressing(catalog, err)
+		unpackErr := fmt.Errorf("source bundle content: %w", err)
+		updateStatusProgressing(catalog, unpackErr)
+		return ctrl.Result{}, unpackErr
 	}
 
 	switch unpackResult.State {
@@ -151,7 +155,9 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp
 		//   of the unpacking steps.
 		err := r.Storage.Store(ctx, catalog.Name, unpackResult.FS)
 		if err != nil {
-			return ctrl.Result{}, updateStatusProgressing(catalog, 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)
 
@@ -159,11 +165,9 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp
 
 		if unpackResult != nil && unpackResult.ResolvedSource != nil && unpackResult.ResolvedSource.Image != nil {
 			lastUnpacked = unpackResult.ResolvedSource.Image.LastUnpacked
-		} else {
-			lastUnpacked = metav1.Time{}
 		}
 
-		_ = updateStatusProgressing(catalog, nil)
+		updateStatusProgressing(catalog, nil)
 		updateStatusServing(&catalog.Status, unpackResult, contentURL, catalog.Generation, lastUnpacked)
 
 		var requeueAfter time.Duration
@@ -180,34 +184,34 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp
 	}
 }
 
-func updateStatusProgressing(catalog *v1alpha1.ClusterCatalog, err error) error {
-	var unrecov *catalogderrors.Unrecoverable
+func updateStatusProgressing(catalog *v1alpha1.ClusterCatalog, err error) {
 	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) {
+		return
+	}
+
+	var unrecov *catalogderrors.Unrecoverable
+	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
 	}
-	return nil
+
+	meta.SetStatusCondition(&catalog.Status.Conditions, metav1.Condition{
+		Type:    v1alpha1.TypeProgressing,
+		Status:  metav1.ConditionTrue,
+		Reason:  v1alpha1.ReasonRetrying,
+		Message: err.Error(),
+	})
 }
 
 func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string, generation int64, unpackedAt metav1.Time) {
@@ -241,9 +245,6 @@ 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
@@ -264,12 +265,3 @@ 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
-}