Skip to content

Commit 1a8646a

Browse files
committed
OCPBUGS-97746: verify and sync catalog.jsonl file is available before populating updateStatus
1 parent 7909550 commit 1a8646a

5 files changed

Lines changed: 61 additions & 0 deletions

File tree

internal/catalogd/controllers/core/clustercatalog_controller.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,12 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *ocv1.
270270
}
271271
baseURL := r.Storage.BaseURL(catalog.Name)
272272

273+
if err := r.Storage.VerifyAndSync(catalog.Name); err != nil {
274+
verifyErr := fmt.Errorf("error verifying catalog content before serving: %w", err)
275+
updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), verifyErr)
276+
return ctrl.Result{}, verifyErr
277+
}
278+
273279
updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), nil)
274280
updateStatusServing(&catalog.Status, canonicalRef, unpackTime, baseURL, catalog.GetGeneration())
275281

internal/catalogd/controllers/core/clustercatalog_controller_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@ func newMockStore(ctrl *gomock.Controller, shouldError bool) *mockstorage.MockIn
3131
if shouldError {
3232
m.EXPECT().Store(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("mockstore store error")).AnyTimes()
3333
m.EXPECT().Delete(gomock.Any()).Return(errors.New("mockstore delete error")).AnyTimes()
34+
m.EXPECT().VerifyAndSync(gomock.Any()).Return(errors.New("mockstore verify error")).AnyTimes()
3435
} else {
3536
m.EXPECT().Store(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
3637
m.EXPECT().Delete(gomock.Any()).Return(nil).AnyTimes()
38+
m.EXPECT().VerifyAndSync(gomock.Any()).Return(nil).AnyTimes()
3739
}
3840
m.EXPECT().BaseURL(gomock.Any()).Return("URL").AnyTimes()
3941
m.EXPECT().ContentExists(gomock.Any()).Return(true).AnyTimes()

internal/catalogd/storage/localdir.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"errors"
77
"fmt"
8+
"io"
89
"io/fs"
910
"net/http"
1011
"net/url"
@@ -230,6 +231,38 @@ func (s *LocalDirV1) ContentExists(catalog string) bool {
230231
return true
231232
}
232233

234+
// VerifyAndSync verifies that catalog.jsonl exists at RootDir/<catalog>/catalog.jsonl,
235+
// calls fsync to ensure the file is durably written to disk, and confirms the file is
236+
// readable by attempting a small read. It should be called after Store() succeeds and
237+
// before marking the catalog as Serving.
238+
func (s *LocalDirV1) VerifyAndSync(catalog string) error {
239+
s.m.RLock()
240+
defer s.m.RUnlock()
241+
242+
path := catalogFilePath(s.catalogDir(catalog))
243+
244+
if _, err := os.Stat(path); err != nil {
245+
return fmt.Errorf("catalog.jsonl not found at %q: %w", path, err)
246+
}
247+
248+
f, err := os.Open(path)
249+
if err != nil {
250+
return fmt.Errorf("catalog.jsonl not readable at %q: %w", path, err)
251+
}
252+
defer f.Close()
253+
254+
if err := f.Sync(); err != nil {
255+
return fmt.Errorf("fsync failed for catalog.jsonl at %q: %w", path, err)
256+
}
257+
258+
buf := make([]byte, 1)
259+
if _, err := f.Read(buf); err != nil && !errors.Is(err, io.EOF) {
260+
return fmt.Errorf("catalog.jsonl read failed at %q: %w", path, err)
261+
}
262+
263+
return nil
264+
}
265+
233266
func (s *LocalDirV1) catalogDir(catalog string) string {
234267
return filepath.Join(s.RootDir, catalog)
235268
}

internal/catalogd/storage/storage.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ type Instance interface {
1515
Delete(catalog string) error
1616
ContentExists(catalog string) bool
1717

18+
// VerifyAndSync confirms that catalog.jsonl exists on disk for the given
19+
// catalog, flushes it to stable storage via fsync, and verifies the file
20+
// is readable. It must be called after Store and before marking the
21+
// catalog as Serving.
22+
VerifyAndSync(catalog string) error
23+
1824
BaseURL(catalog string) string
1925
StorageServerHandler() http.Handler
2026
}

internal/testutil/mock/storage/mock_instance.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)