Skip to content

Commit

Permalink
blob: TryReusingBlobWithOptions consider requiredCompression if set
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
flouthoc committed Jul 10, 2023
1 parent 44a5d07 commit 77b9752
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 17 deletions.
3 changes: 3 additions & 0 deletions directory/directory_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.BlobMatchesRequiredCompression(options, info.CompressionAlgorithm) {
return false, private.ReusedBlob{}, nil
}
if info.Digest == "" {
return false, private.ReusedBlob{}, fmt.Errorf("Can not check for a blob with unknown digest")
}
Expand Down
31 changes: 18 additions & 13 deletions docker/docker_image_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,13 +321,15 @@ 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.BlobMatchesRequiredCompression(options, info.CompressionAlgorithm) {
// 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
}
}

// Then try reusing blobs from other locations.
Expand Down Expand Up @@ -363,6 +365,15 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context,
remoteName: reference.Path(candidateRepo),
actions: "pull",
}
compressionOperation, compressionAlgorithm, err := blobinfocache.OperationAndAlgorithmForCompressor(candidate.CompressorName)
if err != nil {
logrus.Debugf("... Failed: %v", err)
continue
}
if !impl.BlobMatchesRequiredCompression(options, compressionAlgorithm) {
logrus.Debugf("Ignoring candidate blob %s as reuse candidate since compression %s was requested and blob has compression %s", candidate.Digest.String(), options.RequiredCompression.Name(), compressionAlgorithm.Name())
continue
}
// This existence check is not, strictly speaking, necessary: We only _really_ need it to get the blob size, and we could record that in the cache instead.
// But a "failed" d.mountBlob currently leaves around an unterminated server-side upload, which we would try to cancel.
// So, without this existence check, it would be 1 request on success, 2 requests on failure; with it, it is 2 requests on success, 1 request on failure.
Expand All @@ -388,12 +399,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,
Expand Down
3 changes: 3 additions & 0 deletions docker/internal/tarfile/dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.BlobMatchesRequiredCompression(options, info.CompressionAlgorithm) {
return false, private.ReusedBlob{}, nil
}
if err := d.archive.lock(); err != nil {
return false, private.ReusedBlob{}, err
}
Expand Down
12 changes: 12 additions & 0 deletions internal/imagedestination/impl/helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
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 exisiting blob.
func BlobMatchesRequiredCompression(options private.TryReusingBlobOptions, algo *compression.Algorithm) bool {
return options.RequiredCompression == nil || (options.RequiredCompression.Name() == algo.Name())
}
3 changes: 3 additions & 0 deletions internal/imagedestination/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions internal/private/private.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ 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, only reuse blobs with a matching algorithm .Name()
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.
Expand Down
3 changes: 3 additions & 0 deletions oci/layout/oci_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.BlobMatchesRequiredCompression(options, info.CompressionAlgorithm) {
return false, private.ReusedBlob{}, nil
}
if info.Digest == "" {
return false, private.ReusedBlob{}, errors.New("Can not check for a blob with unknown digest")
}
Expand Down
3 changes: 3 additions & 0 deletions ostree/ostree_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.BlobMatchesRequiredCompression(options, info.CompressionAlgorithm) {
return false, private.ReusedBlob{}, nil
}
if d.repo == nil {
repo, err := openRepo(d.ref.repo)
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions storage/storage_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.BlobMatchesRequiredCompression(options, blobinfo.CompressionAlgorithm) {
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
Expand Down

0 comments on commit 77b9752

Please sign in to comment.