From 91e899f5f00cc8b61e926e7fb9aa53ddceb9bf55 Mon Sep 17 00:00:00 2001
From: Igor Troyanovsky <itroyano@redhat.com>
Date: Mon, 1 Jul 2024 16:56:37 +0300
Subject: [PATCH] check the underlying storage for existing cluster catalog
 content (#290)

Signed-off-by: Igor Troyanovsky <itroyano@redhat.com>
---
 .../core/clustercatalog_controller.go         | 20 ++++++++++++-------
 .../core/clustercatalog_controller_test.go    |  4 ++++
 pkg/storage/localdir.go                       | 12 +++++++++++
 pkg/storage/localdir_test.go                  |  6 ++++++
 pkg/storage/storage.go                        |  1 +
 5 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/pkg/controllers/core/clustercatalog_controller.go b/pkg/controllers/core/clustercatalog_controller.go
index a803730..6c116b0 100644
--- a/pkg/controllers/core/clustercatalog_controller.go
+++ b/pkg/controllers/core/clustercatalog_controller.go
@@ -132,13 +132,11 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp
 		controllerutil.RemoveFinalizer(catalog, fbcDeletionFinalizer)
 		return ctrl.Result{}, nil
 	}
-	// if ResolvedSource is not nil, it indicates that this is not the first time we're
-	// unpacking this catalog.
-	if catalog.Status.ResolvedSource != nil {
-		if !unpackAgain(catalog) {
-			return ctrl.Result{}, nil
-		}
+
+	if !r.needsUnpacking(catalog) {
+		return ctrl.Result{}, nil
 	}
+
 	unpackResult, err := r.Unpacker.Unpack(ctx, catalog)
 	if err != nil {
 		return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("source bundle content: %v", err))
@@ -247,7 +245,15 @@ func updateStatusStorageDeleteError(status *v1alpha1.ClusterCatalogStatus, err e
 	return err
 }
 
-func unpackAgain(catalog *v1alpha1.ClusterCatalog) bool {
+func (r *ClusterCatalogReconciler) needsUnpacking(catalog *v1alpha1.ClusterCatalog) bool {
+	// if ResolvedSource is nil, it indicates that this is the first time we're
+	// unpacking this catalog.
+	if catalog.Status.ResolvedSource == nil {
+		return true
+	}
+	if !r.Storage.ContentExists(catalog.Name) {
+		return true
+	}
 	// if the spec.Source.Image.Ref was changed, unpack the new ref
 	if catalog.Spec.Source.Image.Ref != catalog.Status.ResolvedSource.Image.Ref {
 		return true
diff --git a/pkg/controllers/core/clustercatalog_controller_test.go b/pkg/controllers/core/clustercatalog_controller_test.go
index e3d8ecc..a48e17c 100644
--- a/pkg/controllers/core/clustercatalog_controller_test.go
+++ b/pkg/controllers/core/clustercatalog_controller_test.go
@@ -75,6 +75,10 @@ func (m MockStore) StorageServerHandler() http.Handler {
 	panic("not needed")
 }
 
+func (m MockStore) ContentExists(_ string) bool {
+	return true
+}
+
 func TestCatalogdControllerReconcile(t *testing.T) {
 	for _, tt := range []struct {
 		name            string
diff --git a/pkg/storage/localdir.go b/pkg/storage/localdir.go
index 002655b..5c30943 100644
--- a/pkg/storage/localdir.go
+++ b/pkg/storage/localdir.go
@@ -62,6 +62,18 @@ func (s LocalDir) StorageServerHandler() http.Handler {
 	return mux
 }
 
+func (s LocalDir) ContentExists(catalog string) bool {
+	file, err := os.Stat(filepath.Join(s.RootDir, catalog, "all.json"))
+	if err != nil {
+		return false
+	}
+	if !file.Mode().IsRegular() {
+		// path is not valid content
+		return false
+	}
+	return true
+}
+
 // filesOnlyFilesystem is a file system that can open only regular
 // files from the underlying filesystem. All other file types result
 // in os.ErrNotExists
diff --git a/pkg/storage/localdir_test.go b/pkg/storage/localdir_test.go
index 0dfb4bc..c3f82cd 100644
--- a/pkg/storage/localdir_test.go
+++ b/pkg/storage/localdir_test.go
@@ -78,6 +78,9 @@ var _ = Describe("LocalDir Storage Test", func() {
 		It("should form the content URL correctly", func() {
 			Expect(store.ContentURL(catalog)).To(Equal(fmt.Sprintf("%s%s/all.json", baseURL, catalog)))
 		})
+		It("should report content exists", func() {
+			Expect(store.ContentExists(catalog)).To(BeTrue())
+		})
 		When("The stored content is deleted", func() {
 			BeforeEach(func() {
 				err := store.Delete(catalog)
@@ -89,6 +92,9 @@ var _ = Describe("LocalDir Storage Test", func() {
 				Expect(err).To(HaveOccurred())
 				Expect(os.IsNotExist(err)).To(BeTrue())
 			})
+			It("should report content does not exist", func() {
+				Expect(store.ContentExists(catalog)).To(BeFalse())
+			})
 		})
 	})
 })
diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
index 44f37f5..09fbded 100644
--- a/pkg/storage/storage.go
+++ b/pkg/storage/storage.go
@@ -15,4 +15,5 @@ type Instance interface {
 	Delete(catalog string) error
 	ContentURL(catalog string) string
 	StorageServerHandler() http.Handler
+	ContentExists(catalog string) bool
 }