diff --git a/internal/catalogd/controllers/core/clustercatalog_controller.go b/internal/catalogd/controllers/core/clustercatalog_controller.go index fedfe500f0..a8ced78f22 100644 --- a/internal/catalogd/controllers/core/clustercatalog_controller.go +++ b/internal/catalogd/controllers/core/clustercatalog_controller.go @@ -270,6 +270,12 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *ocv1. } baseURL := r.Storage.BaseURL(catalog.Name) + if err := r.Storage.VerifyAndSync(catalog.Name); err != nil { + verifyErr := fmt.Errorf("error verifying catalog content before serving: %w", err) + updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), verifyErr) + return ctrl.Result{}, verifyErr + } + updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), nil) updateStatusServing(&catalog.Status, canonicalRef, unpackTime, baseURL, catalog.GetGeneration()) diff --git a/internal/catalogd/controllers/core/clustercatalog_controller_test.go b/internal/catalogd/controllers/core/clustercatalog_controller_test.go index f6cbe46dfb..786fa07696 100644 --- a/internal/catalogd/controllers/core/clustercatalog_controller_test.go +++ b/internal/catalogd/controllers/core/clustercatalog_controller_test.go @@ -31,9 +31,11 @@ func newMockStore(ctrl *gomock.Controller, shouldError bool) *mockstorage.MockIn if shouldError { m.EXPECT().Store(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("mockstore store error")).AnyTimes() m.EXPECT().Delete(gomock.Any()).Return(errors.New("mockstore delete error")).AnyTimes() + m.EXPECT().VerifyAndSync(gomock.Any()).Return(errors.New("mockstore verify error")).AnyTimes() } else { m.EXPECT().Store(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() m.EXPECT().Delete(gomock.Any()).Return(nil).AnyTimes() + m.EXPECT().VerifyAndSync(gomock.Any()).Return(nil).AnyTimes() } m.EXPECT().BaseURL(gomock.Any()).Return("URL").AnyTimes() m.EXPECT().ContentExists(gomock.Any()).Return(true).AnyTimes() diff --git a/internal/catalogd/storage/localdir.go b/internal/catalogd/storage/localdir.go index fa1319b665..5735beda50 100644 --- a/internal/catalogd/storage/localdir.go +++ b/internal/catalogd/storage/localdir.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "io/fs" "net/http" "net/url" @@ -230,6 +231,38 @@ func (s *LocalDirV1) ContentExists(catalog string) bool { return true } +// VerifyAndSync verifies that catalog.jsonl exists at RootDir//catalog.jsonl, +// calls fsync to ensure the file is durably written to disk, and confirms the file is +// readable by attempting a small read. It should be called after Store() succeeds and +// before marking the catalog as Serving. +func (s *LocalDirV1) VerifyAndSync(catalog string) error { + s.m.RLock() + defer s.m.RUnlock() + + path := catalogFilePath(s.catalogDir(catalog)) + + if _, err := os.Stat(path); err != nil { + return fmt.Errorf("catalog.jsonl not found at %q: %w", path, err) + } + + f, err := os.Open(path) + if err != nil { + return fmt.Errorf("catalog.jsonl not readable at %q: %w", path, err) + } + defer f.Close() + + if err := f.Sync(); err != nil { + return fmt.Errorf("fsync failed for catalog.jsonl at %q: %w", path, err) + } + + buf := make([]byte, 1) + if _, err := f.Read(buf); err != nil && !errors.Is(err, io.EOF) { + return fmt.Errorf("catalog.jsonl read failed at %q: %w", path, err) + } + + return nil +} + func (s *LocalDirV1) catalogDir(catalog string) string { return filepath.Join(s.RootDir, catalog) } diff --git a/internal/catalogd/storage/storage.go b/internal/catalogd/storage/storage.go index af78a669fc..a16a7f4cad 100644 --- a/internal/catalogd/storage/storage.go +++ b/internal/catalogd/storage/storage.go @@ -15,6 +15,12 @@ type Instance interface { Delete(catalog string) error ContentExists(catalog string) bool + // VerifyAndSync confirms that catalog.jsonl exists on disk for the given + // catalog, flushes it to stable storage via fsync, and verifies the file + // is readable. It must be called after Store and before marking the + // catalog as Serving. + VerifyAndSync(catalog string) error + BaseURL(catalog string) string StorageServerHandler() http.Handler } diff --git a/internal/testutil/mock/storage/mock_instance.go b/internal/testutil/mock/storage/mock_instance.go index 2aa9613e3b..245a803c9d 100644 --- a/internal/testutil/mock/storage/mock_instance.go +++ b/internal/testutil/mock/storage/mock_instance.go @@ -111,3 +111,17 @@ func (mr *MockInstanceMockRecorder) Store(ctx, catalog, fsys any) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Store", reflect.TypeOf((*MockInstance)(nil).Store), ctx, catalog, fsys) } + +// VerifyAndSync mocks base method. +func (m *MockInstance) VerifyAndSync(catalog string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "VerifyAndSync", catalog) + ret0, _ := ret[0].(error) + return ret0 +} + +// VerifyAndSync indicates an expected call of VerifyAndSync. +func (mr *MockInstanceMockRecorder) VerifyAndSync(catalog any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VerifyAndSync", reflect.TypeOf((*MockInstance)(nil).VerifyAndSync), catalog) +}