Skip to content

Commit

Permalink
fix: remove critical cache failure in oras GetBlobContent (#1740)
Browse files Browse the repository at this point in the history
  • Loading branch information
binbin-li authored Aug 28, 2024
1 parent 8632ea5 commit 269d176
Show file tree
Hide file tree
Showing 3 changed files with 560 additions and 339 deletions.
4 changes: 4 additions & 0 deletions pkg/referrerstore/oras/mocks/oras_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type TestStorage struct {
ExistsMap map[digest.Digest]io.Reader
ExistsErr error
FetchErr error
PushErr error
}

func (s TestStorage) Exists(_ context.Context, target oci.Descriptor) (bool, error) {
Expand All @@ -43,6 +44,9 @@ func (s TestStorage) Exists(_ context.Context, target oci.Descriptor) (bool, err
}

func (s TestStorage) Push(_ context.Context, expected oci.Descriptor, content io.Reader) error {
if s.PushErr != nil {
return s.PushErr
}
s.ExistsMap[expected.Digest] = content
return nil
}
Expand Down
26 changes: 21 additions & 5 deletions pkg/referrerstore/oras/oras.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ func (store *orasStore) ListReferrers(ctx context.Context, subjectReference comm

func (store *orasStore) GetBlobContent(ctx context.Context, subjectReference common.Reference, digest digest.Digest) ([]byte, error) {
var err error
var blobContent []byte

repository, err := store.createRepository(ctx, store, subjectReference)
if err != nil {
return nil, err
Expand All @@ -270,10 +272,18 @@ func (store *orasStore) GetBlobContent(ctx context.Context, subjectReference com
// check if blob exists in local ORAS cache
isCached, err := store.localCache.Exists(ctx, blobDescriptor)
if err != nil {
return nil, err
logger.GetLogger(ctx, logOpt).Warnf("failed to check if blob [%s] exists in cache: %v", blobDescriptor.Digest.String(), err)
}
metrics.ReportBlobCacheCount(ctx, isCached)

if isCached {
blobContent, err = store.getRawContentFromCache(ctx, blobDescriptor)
if err != nil {
isCached = false
logger.GetLogger(ctx, logOpt).Warnf("failed to get blob [%s] from cache: %v", blobDescriptor.Digest.String(), err)
}
}

if !isCached {
// generate the reference path with digest
ref := fmt.Sprintf("%s@%s", subjectReference.Path, digest)
Expand All @@ -284,16 +294,20 @@ func (store *orasStore) GetBlobContent(ctx context.Context, subjectReference com
evictOnError(ctx, err, subjectReference.Original)
return nil, err
}
if blobContent, err = io.ReadAll(rc); err != nil {
return nil, re.ErrorCodeGetBlobContentFailure.WithError(err)
}

// push fetched content to local ORAS cache
// If multiple goroutines try to push the same blob to the cache, oras-go
// may return `ErrAlreadyExists` error. This is expected and can be ignored.
orasExistsExpectedError := fmt.Errorf("%s: %s: %w", blobDesc.Digest, blobDesc.MediaType, errdef.ErrAlreadyExists)
err = store.localCache.Push(ctx, blobDesc, rc)
if err != nil && err.Error() != orasExistsExpectedError.Error() {
return nil, err
if err = store.localCache.Push(ctx, blobDesc, bytes.NewReader(blobContent)); err != nil && err.Error() != orasExistsExpectedError.Error() {
logger.GetLogger(ctx, logOpt).Warnf("failed to save blob [%s] in cache: %v", blobDesc.Digest, err)
}
}

return store.getRawContentFromCache(ctx, blobDescriptor)
return blobContent, nil
}

func (store *orasStore) GetReferenceManifest(ctx context.Context, subjectReference common.Reference, referenceDesc ocispecs.ReferenceDescriptor) (ocispecs.ReferenceManifest, error) {
Expand Down Expand Up @@ -331,6 +345,8 @@ func (store *orasStore) GetReferenceManifest(ctx context.Context, subjectReferen
}

// push fetched manifest to local ORAS cache
// If multiple goroutines try to push the same manifest to the cache, oras-go
// may return `ErrAlreadyExists` error. This is expected and can be ignored.
orasExistsExpectedError := fmt.Errorf("%s: %s: %w", referenceDesc.Descriptor.Digest, referenceDesc.Descriptor.MediaType, errdef.ErrAlreadyExists)
err = store.localCache.Push(ctx, referenceDesc.Descriptor, bytes.NewReader(manifestBytes))
if err != nil && err.Error() != orasExistsExpectedError.Error() {
Expand Down
Loading

0 comments on commit 269d176

Please sign in to comment.