Skip to content

Commit

Permalink
check the underlying storage for existing cluster catalog content (#290)
Browse files Browse the repository at this point in the history
Signed-off-by: Igor Troyanovsky <itroyano@redhat.com>
  • Loading branch information
itroyano committed Jul 1, 2024
1 parent 14768d6 commit 91e899f
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 7 deletions.
20 changes: 13 additions & 7 deletions pkg/controllers/core/clustercatalog_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/controllers/core/clustercatalog_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions pkg/storage/localdir.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions pkg/storage/localdir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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())
})
})
})
})
Expand Down
1 change: 1 addition & 0 deletions pkg/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@ type Instance interface {
Delete(catalog string) error
ContentURL(catalog string) string
StorageServerHandler() http.Handler
ContentExists(catalog string) bool
}

0 comments on commit 91e899f

Please sign in to comment.