From 4f8c03922a75f2444d131b0e643dc777850e8500 Mon Sep 17 00:00:00 2001 From: Aditya R Date: Sun, 2 Jul 2023 12:53:12 +0530 Subject: [PATCH] blob: TryReusingBlobWithOptions consider requiredCompression if set TryReusingBlob now contains a new option `RequiredCompression` which filters the blob by checking against a compression of the blob which is considerd to be resued, in case `RequiredCompression` is set and `info` of the blob being reused does not matches, no blob is returned. Signed-off-by: Aditya R --- directory/directory_dest.go | 3 ++ docker/docker_image_dest.go | 41 +++++++++++++------ docker/internal/tarfile/dest.go | 3 ++ internal/imagedestination/impl/helpers.go | 20 +++++++++ .../imagedestination/impl/helpers_test.go | 29 +++++++++++++ internal/imagedestination/wrapper.go | 3 ++ internal/private/private.go | 9 ++-- oci/layout/oci_dest.go | 3 ++ ostree/ostree_dest.go | 3 ++ pkg/blobcache/dest.go | 3 ++ storage/storage_dest.go | 3 ++ 11 files changed, 103 insertions(+), 17 deletions(-) create mode 100644 internal/imagedestination/impl/helpers.go create mode 100644 internal/imagedestination/impl/helpers_test.go diff --git a/directory/directory_dest.go b/directory/directory_dest.go index 974d23d5fa..222723a8f5 100644 --- a/directory/directory_dest.go +++ b/directory/directory_dest.go @@ -190,6 +190,9 @@ func (d *dirImageDestination) PutBlobWithOptions(ctx context.Context, stream io. // If the blob has been successfully reused, returns (true, info, nil). // If the transport can not reuse the requested blob, TryReusingBlob returns (false, {}, nil); it returns a non-nil error only on an unexpected failure. func (d *dirImageDestination) TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options private.TryReusingBlobOptions) (bool, private.ReusedBlob, error) { + if !impl.OriginalBlobMatchesRequiredCompression(options) { + return false, private.ReusedBlob{}, nil + } if info.Digest == "" { return false, private.ReusedBlob{}, fmt.Errorf("Can not check for a blob with unknown digest") } diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index 44e2aea23d..63e372d677 100644 --- a/docker/docker_image_dest.go +++ b/docker/docker_image_dest.go @@ -321,13 +321,21 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context, return false, private.ReusedBlob{}, errors.New("Can not check for a blob with unknown digest") } - // First, check whether the blob happens to already exist at the destination. - haveBlob, reusedInfo, err := d.tryReusingExactBlob(ctx, info, options.Cache) - if err != nil { - return false, private.ReusedBlob{}, err - } - if haveBlob { - return true, reusedInfo, nil + if impl.OriginalBlobMatchesRequiredCompression(options) { + // First, check whether the blob happens to already exist at the destination. + haveBlob, reusedInfo, err := d.tryReusingExactBlob(ctx, info, options.Cache) + if err != nil { + return false, private.ReusedBlob{}, err + } + if haveBlob { + return true, reusedInfo, nil + } + } else { + requiredCompression := "nil" + if options.OriginalCompression != nil { + requiredCompression = options.OriginalCompression.Name() + } + logrus.Debugf("Ignoring exact blob match case due to compression mismatch ( %s vs %s )", options.RequiredCompression.Name(), requiredCompression) } // Then try reusing blobs from other locations. @@ -338,6 +346,19 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context, logrus.Debugf("Error parsing BlobInfoCache location reference: %s", err) continue } + compressionOperation, compressionAlgorithm, err := blobinfocache.OperationAndAlgorithmForCompressor(candidate.CompressorName) + if err != nil { + logrus.Debugf("OperationAndAlgorithmForCompressor Failed: %v", err) + continue + } + if !impl.BlobMatchesRequiredCompression(options, compressionAlgorithm) { + requiredCompression := "nil" + if compressionAlgorithm != nil { + requiredCompression = compressionAlgorithm.Name() + } + logrus.Debugf("Ignoring candidate blob %s as reuse candidate due to compression mismatch ( %s vs %s ) in %s", candidate.Digest.String(), options.RequiredCompression.Name(), requiredCompression, candidateRepo.Name()) + continue + } if candidate.CompressorName != blobinfocache.Uncompressed { logrus.Debugf("Trying to reuse cached location %s compressed with %s in %s", candidate.Digest.String(), candidate.CompressorName, candidateRepo.Name()) } else { @@ -388,12 +409,6 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context, options.Cache.RecordKnownLocation(d.ref.Transport(), bicTransportScope(d.ref), candidate.Digest, newBICLocationReference(d.ref)) - compressionOperation, compressionAlgorithm, err := blobinfocache.OperationAndAlgorithmForCompressor(candidate.CompressorName) - if err != nil { - logrus.Debugf("... Failed: %v", err) - continue - } - return true, private.ReusedBlob{ Digest: candidate.Digest, Size: size, diff --git a/docker/internal/tarfile/dest.go b/docker/internal/tarfile/dest.go index 00e25748bd..7507d85595 100644 --- a/docker/internal/tarfile/dest.go +++ b/docker/internal/tarfile/dest.go @@ -129,6 +129,9 @@ func (d *Destination) PutBlobWithOptions(ctx context.Context, stream io.Reader, // If the blob has been successfully reused, returns (true, info, nil). // If the transport can not reuse the requested blob, TryReusingBlob returns (false, {}, nil); it returns a non-nil error only on an unexpected failure. func (d *Destination) TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options private.TryReusingBlobOptions) (bool, private.ReusedBlob, error) { + if !impl.OriginalBlobMatchesRequiredCompression(options) { + return false, private.ReusedBlob{}, nil + } if err := d.archive.lock(); err != nil { return false, private.ReusedBlob{}, err } diff --git a/internal/imagedestination/impl/helpers.go b/internal/imagedestination/impl/helpers.go new file mode 100644 index 0000000000..d5de81a613 --- /dev/null +++ b/internal/imagedestination/impl/helpers.go @@ -0,0 +1,20 @@ +package impl + +import ( + "github.com/containers/image/v5/internal/private" + compression "github.com/containers/image/v5/pkg/compression/types" +) + +// BlobMatchesRequiredCompression validates if compression is required by the caller while selecting a blob, if it is required +// then function performs a match against the compression requested by the caller and compression of existing blob +// (which can be nil to represent uncompressed or unknown) +func BlobMatchesRequiredCompression(options private.TryReusingBlobOptions, candidateCompression *compression.Algorithm) bool { + if options.RequiredCompression == nil { + return true // no requirement imposed + } + return candidateCompression != nil && (options.RequiredCompression.Name() == candidateCompression.Name()) +} + +func OriginalBlobMatchesRequiredCompression(opts private.TryReusingBlobOptions) bool { + return BlobMatchesRequiredCompression(opts, opts.OriginalCompression) +} diff --git a/internal/imagedestination/impl/helpers_test.go b/internal/imagedestination/impl/helpers_test.go new file mode 100644 index 0000000000..26af382d31 --- /dev/null +++ b/internal/imagedestination/impl/helpers_test.go @@ -0,0 +1,29 @@ +package impl + +import ( + "testing" + + "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/pkg/compression" + compressionTypes "github.com/containers/image/v5/pkg/compression/types" + "github.com/stretchr/testify/assert" +) + +func TestBlobMatchesRequiredCompression(t *testing.T) { + var opts private.TryReusingBlobOptions + cases := []struct { + requiredCompression *compressionTypes.Algorithm + candidateCompression *compressionTypes.Algorithm + result bool + }{ + {&compression.Zstd, &compression.Zstd, true}, + {&compression.Gzip, &compression.Zstd, false}, + {&compression.Zstd, nil, false}, + {nil, &compression.Zstd, true}, + } + + for _, c := range cases { + opts = private.TryReusingBlobOptions{RequiredCompression: c.requiredCompression} + assert.Equal(t, BlobMatchesRequiredCompression(opts, c.candidateCompression), c.result) + } +} diff --git a/internal/imagedestination/wrapper.go b/internal/imagedestination/wrapper.go index 41a81628bd..17e1870c19 100644 --- a/internal/imagedestination/wrapper.go +++ b/internal/imagedestination/wrapper.go @@ -64,6 +64,9 @@ func (w *wrapped) PutBlobWithOptions(ctx context.Context, stream io.Reader, inpu // If the blob has been successfully reused, returns (true, info, nil). // If the transport can not reuse the requested blob, TryReusingBlob returns (false, {}, nil); it returns a non-nil error only on an unexpected failure. func (w *wrapped) TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options private.TryReusingBlobOptions) (bool, private.ReusedBlob, error) { + if options.RequiredCompression != nil { + return false, private.ReusedBlob{}, nil + } reused, blob, err := w.TryReusingBlob(ctx, info, options.Cache, options.CanSubstitute) if !reused || err != nil { return reused, private.ReusedBlob{}, err diff --git a/internal/private/private.go b/internal/private/private.go index b1dd4ceb0d..95d561fcdd 100644 --- a/internal/private/private.go +++ b/internal/private/private.go @@ -112,10 +112,11 @@ type TryReusingBlobOptions struct { // Transports, OTOH, MUST support these fields being zero-valued for types.ImageDestination callers // if they use internal/imagedestination/impl.Compat; // in that case, they will all be consistently zero-valued. - - EmptyLayer bool // True if the blob is an "empty"/"throwaway" layer, and may not necessarily be physically represented. - LayerIndex *int // If the blob is a layer, a zero-based index of the layer within the image; nil otherwise. - SrcRef reference.Named // A reference to the source image that contains the input blob. + RequiredCompression *compression.Algorithm // If set, reuse blobs with a matching algorithm as per implementations in internal/imagedestination/impl.helpers.go + OriginalCompression *compression.Algorithm // Must be set if RequiredCompression is set; can be set to nil to indicate “uncompressed” or “unknown”. + EmptyLayer bool // True if the blob is an "empty"/"throwaway" layer, and may not necessarily be physically represented. + LayerIndex *int // If the blob is a layer, a zero-based index of the layer within the image; nil otherwise. + SrcRef reference.Named // A reference to the source image that contains the input blob. } // ReusedBlob is information about a blob reused in a destination. diff --git a/oci/layout/oci_dest.go b/oci/layout/oci_dest.go index 0a9e4eab91..8ff43d4480 100644 --- a/oci/layout/oci_dest.go +++ b/oci/layout/oci_dest.go @@ -172,6 +172,9 @@ func (d *ociImageDestination) PutBlobWithOptions(ctx context.Context, stream io. // If the blob has been successfully reused, returns (true, info, nil). // If the transport can not reuse the requested blob, TryReusingBlob returns (false, {}, nil); it returns a non-nil error only on an unexpected failure. func (d *ociImageDestination) TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options private.TryReusingBlobOptions) (bool, private.ReusedBlob, error) { + if !impl.OriginalBlobMatchesRequiredCompression(options) { + return false, private.ReusedBlob{}, nil + } if info.Digest == "" { return false, private.ReusedBlob{}, errors.New("Can not check for a blob with unknown digest") } diff --git a/ostree/ostree_dest.go b/ostree/ostree_dest.go index 48f3ee5a72..d00a0cdf86 100644 --- a/ostree/ostree_dest.go +++ b/ostree/ostree_dest.go @@ -335,6 +335,9 @@ func (d *ostreeImageDestination) importConfig(repo *otbuiltin.Repo, blob *blobTo // reflected in the manifest that will be written. // If the transport can not reuse the requested blob, TryReusingBlob returns (false, {}, nil); it returns a non-nil error only on an unexpected failure. func (d *ostreeImageDestination) TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options private.TryReusingBlobOptions) (bool, private.ReusedBlob, error) { + if !impl.OriginalBlobMatchesRequiredCompression(options) { + return false, private.ReusedBlob{}, nil + } if d.repo == nil { repo, err := openRepo(d.ref.repo) if err != nil { diff --git a/pkg/blobcache/dest.go b/pkg/blobcache/dest.go index a0e353d46f..9bda085158 100644 --- a/pkg/blobcache/dest.go +++ b/pkg/blobcache/dest.go @@ -237,6 +237,9 @@ func (d *blobCacheDestination) PutBlobPartial(ctx context.Context, chunkAccessor // If the blob has been successfully reused, returns (true, info, nil). // If the transport can not reuse the requested blob, TryReusingBlob returns (false, {}, nil); it returns a non-nil error only on an unexpected failure. func (d *blobCacheDestination) TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options private.TryReusingBlobOptions) (bool, private.ReusedBlob, error) { + if !impl.OriginalBlobMatchesRequiredCompression(options) { + return false, private.ReusedBlob{}, nil + } present, reusedInfo, err := d.destination.TryReusingBlobWithOptions(ctx, info, options) if err != nil || present { return present, reusedInfo, err diff --git a/storage/storage_dest.go b/storage/storage_dest.go index 576d510cc5..7bbbf1752c 100644 --- a/storage/storage_dest.go +++ b/storage/storage_dest.go @@ -307,6 +307,9 @@ func (s *storageImageDestination) PutBlobPartial(ctx context.Context, chunkAcces // If the blob has been successfully reused, returns (true, info, nil). // If the transport can not reuse the requested blob, TryReusingBlob returns (false, {}, nil); it returns a non-nil error only on an unexpected failure. func (s *storageImageDestination) TryReusingBlobWithOptions(ctx context.Context, blobinfo types.BlobInfo, options private.TryReusingBlobOptions) (bool, private.ReusedBlob, error) { + if !impl.OriginalBlobMatchesRequiredCompression(options) { + return false, private.ReusedBlob{}, nil + } reused, info, err := s.tryReusingBlobAsPending(blobinfo.Digest, blobinfo.Size, &options) if err != nil || !reused || options.LayerIndex == nil { return reused, info, err