Merge pull request #2023 from flouthoc/tryreusingblob-compression

blob: `TryReusingBlobWithOptions` consider `RequiredCompression` if set

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")
 	}

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,

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
 	}

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)
+}

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)
+	}
+}

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

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.

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")
 	}

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 {

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

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