From 948f1c877029385be770c69833b1947d72f7bce0 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 3 Oct 2024 14:16:19 +0200 Subject: [PATCH 1/3] vendor: update c/storage Signed-off-by: Giuseppe Scrivano --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 7738ddbfce..8a14d7f39b 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/BurntSushi/toml v1.4.0 github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 github.com/containers/ocicrypt v1.2.0 - github.com/containers/storage v1.55.1-0.20240930161746-4bf3f075cf3f + github.com/containers/storage v1.55.1-0.20241002203117-0eb3a0231575 github.com/cyberphone/json-canonicalization v0.0.0-20231217050601-ba74d44ecf5f github.com/distribution/reference v0.6.0 github.com/docker/cli v27.3.1+incompatible diff --git a/go.sum b/go.sum index a97a8e833e..6c0b705d99 100644 --- a/go.sum +++ b/go.sum @@ -52,8 +52,8 @@ github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 h1:Qzk5C6cYgle github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY= github.com/containers/ocicrypt v1.2.0 h1:X14EgRK3xNFvJEfI5O4Qn4T3E25ANudSOZz/sirVuPM= github.com/containers/ocicrypt v1.2.0/go.mod h1:ZNviigQajtdlxIZGibvblVuIFBKIuUI2M0QM12SD31U= -github.com/containers/storage v1.55.1-0.20240930161746-4bf3f075cf3f h1:Qc/dmzukhCunvppC9sfQ/gC7PBNW5tUhiRqS+ZFp7Ck= -github.com/containers/storage v1.55.1-0.20240930161746-4bf3f075cf3f/go.mod h1:Cu9jwKPeZzPCc1qE/3PmNbL1f3ymm8ltFQVwUxdz+lM= +github.com/containers/storage v1.55.1-0.20241002203117-0eb3a0231575 h1:kTjd9n1IV+6bo+4DN6HbAIEFHpX8U1aRcvjYF/b387Q= +github.com/containers/storage v1.55.1-0.20241002203117-0eb3a0231575/go.mod h1:Cu9jwKPeZzPCc1qE/3PmNbL1f3ymm8ltFQVwUxdz+lM= github.com/coreos/go-oidc/v3 v3.11.0 h1:Ia3MxdwpSw702YW0xgfmP1GVCMA9aEFWu12XUZ3/OtI= github.com/coreos/go-oidc/v3 v3.11.0/go.mod h1:gE3LgjOgFoHi9a4ce4/tJczr0Ai2/BoDhf0r5lltWI0= github.com/cyberphone/json-canonicalization v0.0.0-20231217050601-ba74d44ecf5f h1:eHnXnuK47UlSTOQexbzxAZfekVz6i+LKRdj1CU5DPaM= From 985156155293f566a342610710653c0a923d9a32 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 3 Oct 2024 14:18:55 +0200 Subject: [PATCH 2/3] image: define error for partial pulls not available define a new error type so that the caller can determine whether it is safe to ignore the error and retrieve the resource fully. Signed-off-by: Giuseppe Scrivano --- copy/single.go | 11 ++++++--- .../stubs/put_blob_partial.go | 5 ++-- internal/private/private.go | 24 +++++++++++++++++-- oci/archive/oci_dest.go | 5 ++-- openshift/openshift_dest.go | 5 ++-- pkg/blobcache/dest.go | 5 ++-- storage/storage_dest.go | 5 ++-- 7 files changed, 45 insertions(+), 15 deletions(-) diff --git a/copy/single.go b/copy/single.go index 324785a8bf..983235158a 100644 --- a/copy/single.go +++ b/copy/single.go @@ -819,11 +819,16 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to logrus.Debugf("Retrieved partial blob %v", srcInfo.Digest) return true, updatedBlobInfoFromUpload(srcInfo, uploadedBlob), nil } - logrus.Debugf("Failed to retrieve partial blob: %v", err) - return false, types.BlobInfo{}, nil + // On a "partial content not available" error, ignore it and retrieve the whole layer. + var perr private.ErrFallbackToOrdinaryLayerDownload + if errors.As(err, &perr) { + logrus.Debugf("Failed to retrieve partial blob: %v", err) + return false, types.BlobInfo{}, nil + } + return false, types.BlobInfo{}, err }() if err != nil { - return types.BlobInfo{}, "", err + return types.BlobInfo{}, "", fmt.Errorf("reading blob %s: %w", srcInfo.Digest, err) } if reused { return blobInfo, cachedDiffID, nil diff --git a/internal/imagedestination/stubs/put_blob_partial.go b/internal/imagedestination/stubs/put_blob_partial.go index bbb53c198f..22bed4b0fa 100644 --- a/internal/imagedestination/stubs/put_blob_partial.go +++ b/internal/imagedestination/stubs/put_blob_partial.go @@ -36,8 +36,9 @@ func (stub NoPutBlobPartialInitialize) SupportsPutBlobPartial() bool { // PutBlobPartial attempts to create a blob using the data that is already present // at the destination. chunkAccessor is accessed in a non-sequential way to retrieve the missing chunks. // It is available only if SupportsPutBlobPartial(). -// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller -// should fall back to PutBlobWithOptions. +// Even if SupportsPutBlobPartial() returns true, the call can fail. +// If the call fails with ErrFallbackToOrdinaryLayerDownload, the caller can fall back to PutBlobWithOptions. +// The fallback _must not_ be done otherwise. func (stub NoPutBlobPartialInitialize) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { return private.UploadedBlob{}, fmt.Errorf("internal error: PutBlobPartial is not supported by the %q transport", stub.transportName) } diff --git a/internal/private/private.go b/internal/private/private.go index d81ea6703e..7390d91efa 100644 --- a/internal/private/private.go +++ b/internal/private/private.go @@ -53,8 +53,9 @@ type ImageDestinationInternalOnly interface { // PutBlobPartial attempts to create a blob using the data that is already present // at the destination. chunkAccessor is accessed in a non-sequential way to retrieve the missing chunks. // It is available only if SupportsPutBlobPartial(). - // Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller - // should fall back to PutBlobWithOptions. + // Even if SupportsPutBlobPartial() returns true, the call can fail. + // If the call fails with ErrFallbackToOrdinaryLayerDownload, the caller can fall back to PutBlobWithOptions. + // The fallback _must not_ be done otherwise. PutBlobPartial(ctx context.Context, chunkAccessor BlobChunkAccessor, srcInfo types.BlobInfo, options PutBlobPartialOptions) (UploadedBlob, error) // TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination @@ -183,3 +184,22 @@ type UnparsedImage interface { // UntrustedSignatures is like ImageSource.GetSignaturesWithFormat, but the result is cached; it is OK to call this however often you need. UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) } + +// ErrFallbackToOrdinaryLayerDownload is a custom error type returned by PutBlobPartial. +// It suggests to the caller that a fallback mechanism can be used instead of a hard failure; +// otherwise the caller of PutBlobPartial _must not_ fall back to PutBlob. +type ErrFallbackToOrdinaryLayerDownload struct { + err error +} + +func (c ErrFallbackToOrdinaryLayerDownload) Error() string { + return c.err.Error() +} + +func (c ErrFallbackToOrdinaryLayerDownload) Unwrap() error { + return c.err +} + +func NewErrFallbackToOrdinaryLayerDownload(err error) error { + return ErrFallbackToOrdinaryLayerDownload{err: err} +} diff --git a/oci/archive/oci_dest.go b/oci/archive/oci_dest.go index a3eb5d7a1b..f9c05e39d7 100644 --- a/oci/archive/oci_dest.go +++ b/oci/archive/oci_dest.go @@ -117,8 +117,9 @@ func (d *ociArchiveImageDestination) PutBlobWithOptions(ctx context.Context, str // PutBlobPartial attempts to create a blob using the data that is already present // at the destination. chunkAccessor is accessed in a non-sequential way to retrieve the missing chunks. // It is available only if SupportsPutBlobPartial(). -// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller -// should fall back to PutBlobWithOptions. +// Even if SupportsPutBlobPartial() returns true, the call can fail. +// If the call fails with ErrFallbackToOrdinaryLayerDownload, the caller can fall back to PutBlobWithOptions. +// The fallback _must not_ be done otherwise. func (d *ociArchiveImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { return d.unpackedDest.PutBlobPartial(ctx, chunkAccessor, srcInfo, options) } diff --git a/openshift/openshift_dest.go b/openshift/openshift_dest.go index 4170d6e208..833e670941 100644 --- a/openshift/openshift_dest.go +++ b/openshift/openshift_dest.go @@ -125,8 +125,9 @@ func (d *openshiftImageDestination) PutBlobWithOptions(ctx context.Context, stre // PutBlobPartial attempts to create a blob using the data that is already present // at the destination. chunkAccessor is accessed in a non-sequential way to retrieve the missing chunks. // It is available only if SupportsPutBlobPartial(). -// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller -// should fall back to PutBlobWithOptions. +// Even if SupportsPutBlobPartial() returns true, the call can fail. +// If the call fails with ErrFallbackToOrdinaryLayerDownload, the caller can fall back to PutBlobWithOptions. +// The fallback _must not_ be done otherwise. func (d *openshiftImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { return d.docker.PutBlobPartial(ctx, chunkAccessor, srcInfo, options) } diff --git a/pkg/blobcache/dest.go b/pkg/blobcache/dest.go index d8899f2ed9..0b229d5a9a 100644 --- a/pkg/blobcache/dest.go +++ b/pkg/blobcache/dest.go @@ -238,8 +238,9 @@ func (d *blobCacheDestination) SupportsPutBlobPartial() bool { // PutBlobPartial attempts to create a blob using the data that is already present // at the destination. chunkAccessor is accessed in a non-sequential way to retrieve the missing chunks. // It is available only if SupportsPutBlobPartial(). -// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller -// should fall back to PutBlobWithOptions. +// Even if SupportsPutBlobPartial() returns true, the call can fail. +// If the call fails with ErrFallbackToOrdinaryLayerDownload, the caller can fall back to PutBlobWithOptions. +// The fallback _must not_ be done otherwise. func (d *blobCacheDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { return d.destination.PutBlobPartial(ctx, chunkAccessor, srcInfo, options) } diff --git a/storage/storage_dest.go b/storage/storage_dest.go index a7a2865fc9..8f9de1a5ee 100644 --- a/storage/storage_dest.go +++ b/storage/storage_dest.go @@ -311,8 +311,9 @@ func (f *zstdFetcher) GetBlobAt(chunks []chunked.ImageSourceChunk) (chan io.Read // PutBlobPartial attempts to create a blob using the data that is already present // at the destination. chunkAccessor is accessed in a non-sequential way to retrieve the missing chunks. // It is available only if SupportsPutBlobPartial(). -// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller -// should fall back to PutBlobWithOptions. +// Even if SupportsPutBlobPartial() returns true, the call can fail. +// If the call fails with ErrFallbackToOrdinaryLayerDownload, the caller can fall back to PutBlobWithOptions. +// The fallback _must not_ be done otherwise. func (s *storageImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { fetcher := zstdFetcher{ chunkAccessor: chunkAccessor, From 3dc265d332ec93f2460e1519ae2c779faac22f0b Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 3 Oct 2024 14:56:34 +0200 Subject: [PATCH 3/3] storage: convert from storage error type Signed-off-by: Giuseppe Scrivano --- storage/storage_dest.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/storage/storage_dest.go b/storage/storage_dest.go index 8f9de1a5ee..c71616caca 100644 --- a/storage/storage_dest.go +++ b/storage/storage_dest.go @@ -314,13 +314,20 @@ func (f *zstdFetcher) GetBlobAt(chunks []chunked.ImageSourceChunk) (chan io.Read // Even if SupportsPutBlobPartial() returns true, the call can fail. // If the call fails with ErrFallbackToOrdinaryLayerDownload, the caller can fall back to PutBlobWithOptions. // The fallback _must not_ be done otherwise. -func (s *storageImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { +func (s *storageImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (_ private.UploadedBlob, retErr error) { fetcher := zstdFetcher{ chunkAccessor: chunkAccessor, ctx: ctx, blobInfo: srcInfo, } + defer func() { + var perr chunked.ErrFallbackToOrdinaryLayerDownload + if errors.As(retErr, &perr) { + retErr = private.NewErrFallbackToOrdinaryLayerDownload(retErr) + } + }() + differ, err := chunked.GetDiffer(ctx, s.imageRef.transport.store, srcInfo.Digest, srcInfo.Size, srcInfo.Annotations, &fetcher) if err != nil { return private.UploadedBlob{}, err