From 32d05320472623ed5b78c427098a6c2e820e5755 Mon Sep 17 00:00:00 2001
From: Nalin Dahyabhai <nalin@redhat.com>
Date: Wed, 18 Nov 2020 14:01:32 -0500
Subject: [PATCH] blobinfocache: track compression types for locations
Extend the blob info cache to also cache the name of the type of
compression used on a blob that we've seen, or specific values that
indicate that we know the blob was not compressed, or that we don't
know whether or not it was compressed.
New methods for adding known blob-compression pairs and reading
candidate locations including compression information are part of a new
internal BlobInfoCache2 interface which the library's BlobInfoCache
implementors also implement.
When we copy a blob, try to record the state of compression for the
source blob, and if we applied any changes, the blob we produced.
Make sure that when TryReusingBlob successfully uses a blob from the
blob info cache, that it provides compression information in the
BlobInfo that it returns, so that manifests can be updated to describe
layers using the correct MIME types.
When attempting to write a manifest, if a manifest can't be written
because layers were compressed using an algorithm which can't be
expressed using that manifest type, continue on to trying other manifest
formats.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
---
copy/copy.go | 36 +++++-
directory/directory_dest.go | 5 +-
docker/docker_image_dest.go | 25 +++-
docker/tarfile/dest.go | 4 +-
internal/blobinfocache/blobinfocache.go | 72 +++++++++++
internal/blobinfocache/types.go | 46 +++++++
manifest/common.go | 21 +++-
oci/archive/oci_dest.go | 4 +-
oci/layout/oci_dest.go | 5 +-
openshift/openshift.go | 4 +-
ostree/ostree_dest.go | 4 +-
pkg/blobinfocache/boltdb/boltdb.go | 73 +++++++++--
pkg/blobinfocache/boltdb/boltdb_test.go | 8 +-
.../internal/prioritize/prioritize.go | 12 +-
.../internal/prioritize/prioritize_test.go | 44 +++----
pkg/blobinfocache/internal/test/test.go | 117 +++++++++++++++++-
pkg/blobinfocache/memory/memory.go | 45 ++++++-
pkg/blobinfocache/memory/memory_test.go | 8 +-
pkg/blobinfocache/none/none.go | 3 +-
pkg/blobinfocache/none/none_test.go | 7 ++
types/types.go | 3 +
21 files changed, 469 insertions(+), 77 deletions(-)
create mode 100644 internal/blobinfocache/blobinfocache.go
create mode 100644 internal/blobinfocache/types.go
create mode 100644 pkg/blobinfocache/none/none_test.go
diff --git a/copy/copy.go b/copy/copy.go
index 485db4d30a..a0b49c5a00 100644
--- a/copy/copy.go
+++ b/copy/copy.go
@@ -14,6 +14,7 @@ import (
"github.com/containers/image/v5/docker/reference"
"github.com/containers/image/v5/image"
+ internalblobinfocache "github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/internal/pkg/platform"
"github.com/containers/image/v5/manifest"
"github.com/containers/image/v5/pkg/blobinfocache"
@@ -654,7 +655,9 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli
if err != nil {
logrus.Debugf("Writing manifest using preferred type %s failed: %v", preferredManifestMIMEType, err)
// … if it fails, _and_ the failure is because the manifest is rejected, we may have other options.
- if _, isManifestRejected := errors.Cause(err).(types.ManifestTypeRejectedError); !isManifestRejected || len(otherManifestMIMETypeCandidates) == 0 {
+ _, isManifestRejected := errors.Cause(err).(types.ManifestTypeRejectedError)
+ _, isCompressionIncompatible := errors.Cause(err).(manifest.ManifestLayerCompressionIncompatibilityError)
+ if (!isManifestRejected && !isCompressionIncompatible) || len(otherManifestMIMETypeCandidates) == 0 {
// We don’t have other options.
// In principle the code below would handle this as well, but the resulting error message is fairly ugly.
// Don’t bother the user with MIME types if we have no choice.
@@ -896,7 +899,7 @@ func (ic *imageCopier) copyLayers(ctx context.Context) error {
return nil
}
-// layerDigestsDiffer return true iff the digests in a and b differ (ignoring sizes and possible other fields)
+// layerDigestsDiffer returns true iff the digests in a and b differ (ignoring sizes and possible other fields)
func layerDigestsDiffer(a, b []types.BlobInfo) bool {
if len(a) != len(b) {
return true
@@ -951,7 +954,7 @@ func (ic *imageCopier) copyUpdatedConfigAndManifest(ctx context.Context, instanc
instanceDigest = &manifestDigest
}
if err := ic.c.dest.PutManifest(ctx, man, instanceDigest); err != nil {
- return nil, "", errors.Wrap(err, "Error writing manifest")
+ return nil, "", errors.Wrapf(err, "Error writing manifest %q", string(man))
}
return man, manifestDigest, nil
}
@@ -1390,7 +1393,7 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcStream io.Reader, sr
}
}
- // This is fairly horrible: the writer from getOriginalLayerCopyWriter wants to consumer
+ // This is fairly horrible: the writer from getOriginalLayerCopyWriter wants to consume
// all of the input (to compute DiffIDs), even if dest.PutBlob does not need it.
// So, read everything from originalLayerReader, which will cause the rest to be
// sent there if we are not already at EOF.
@@ -1408,18 +1411,41 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcStream io.Reader, sr
if inputInfo.Digest != "" && uploadedInfo.Digest != inputInfo.Digest {
return types.BlobInfo{}, errors.Errorf("Internal error writing blob %s, blob with digest %s saved with digest %s", srcInfo.Digest, inputInfo.Digest, uploadedInfo.Digest)
}
+
if digestingReader.validationSucceeded {
+ bic := internalblobinfocache.FromBlobInfoCache(c.blobInfoCache)
// If compressionOperation != types.PreserveOriginal, we now have two reliable digest values:
// srcinfo.Digest describes the pre-compressionOperation input, verified by digestingReader
// uploadedInfo.Digest describes the post-compressionOperation output, computed by PutBlob
// (because inputInfo.Digest == "", this must have been computed afresh).
switch compressionOperation {
case types.PreserveOriginal:
- break // Do nothing, we have only one digest and we might not have even verified it.
+ if desiredCompressionFormat.Name() != "" && isCompressed {
+ // we recompressed it
+ bic.RecordDigestCompressorName(uploadedInfo.Digest, desiredCompressionFormat.Name())
+ } else {
+ // it wasn't, and still isn't, compressed
+ bic.RecordDigestCompressorName(uploadedInfo.Digest, internalblobinfocache.Uncompressed)
+ }
+ // Do nothing else, we have only one digest and we might not have even verified it.
case types.Compress:
c.blobInfoCache.RecordDigestUncompressedPair(uploadedInfo.Digest, srcInfo.Digest)
+ if srcInfo.Digest != "" && !isCompressed {
+ // the original blob wasn't compressed
+ bic.RecordDigestCompressorName(srcInfo.Digest, internalblobinfocache.Uncompressed)
+ }
+ if desiredCompressionFormat.Name() != "" {
+ // we compressed the original blob
+ bic.RecordDigestCompressorName(uploadedInfo.Digest, desiredCompressionFormat.Name())
+ }
case types.Decompress:
c.blobInfoCache.RecordDigestUncompressedPair(srcInfo.Digest, uploadedInfo.Digest)
+ if srcInfo.Digest != "" && isCompressed {
+ // the original blob was compressed
+ bic.RecordDigestCompressorName(srcInfo.Digest, compressionFormat.Name())
+ }
+ // we decompressed the original blob
+ bic.RecordDigestCompressorName(uploadedInfo.Digest, internalblobinfocache.Uncompressed)
default:
return types.BlobInfo{}, errors.Errorf("Internal error: Unexpected compressionOperation value %#v", compressionOperation)
}
diff --git a/directory/directory_dest.go b/directory/directory_dest.go
index 2b81c83600..f9cbc718ba 100644
--- a/directory/directory_dest.go
+++ b/directory/directory_dest.go
@@ -194,7 +194,9 @@ func (d *dirImageDestination) PutBlob(ctx context.Context, stream io.Reader, inp
// (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree).
// info.Digest must not be empty.
// If canSubstitute, TryReusingBlob can use an equivalent equivalent of the desired blob; in that case the returned info may not match the input.
-// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size.
+// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size, and may
+// include CompressionOperation and CompressionAlgorithm fields to indicate that a change to the compression type should be
+// 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.
// May use and/or update cache.
func (d *dirImageDestination) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache, canSubstitute bool) (bool, types.BlobInfo, error) {
@@ -210,7 +212,6 @@ func (d *dirImageDestination) TryReusingBlob(ctx context.Context, info types.Blo
return false, types.BlobInfo{}, err
}
return true, types.BlobInfo{Digest: info.Digest, Size: finfo.Size()}, nil
-
}
// PutManifest writes manifest to the destination.
diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go
index ac63ac121f..83c5ea2587 100644
--- a/docker/docker_image_dest.go
+++ b/docker/docker_image_dest.go
@@ -15,6 +15,7 @@ import (
"strings"
"github.com/containers/image/v5/docker/reference"
+ "github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/internal/iolimits"
"github.com/containers/image/v5/internal/uploadreader"
"github.com/containers/image/v5/manifest"
@@ -162,6 +163,7 @@ func (d *dockerImageDestination) PutBlob(ctx context.Context, stream io.Reader,
digester := digest.Canonical.Digester()
sizeCounter := &sizeCounter{}
+
uploadLocation, err = func() (*url.URL, error) { // A scope for defer
uploadReader := uploadreader.NewUploadReader(io.TeeReader(stream, io.MultiWriter(digester.Hash(), sizeCounter)))
// This error text should never be user-visible, we terminate only after makeRequestToResolvedURL
@@ -299,17 +301,23 @@ func (d *dockerImageDestination) TryReusingBlob(ctx context.Context, info types.
}
if exists {
cache.RecordKnownLocation(d.ref.Transport(), bicTransportScope(d.ref), info.Digest, newBICLocationReference(d.ref))
- return true, types.BlobInfo{Digest: info.Digest, Size: size}, nil
+ return true, types.BlobInfo{Digest: info.Digest, MediaType: info.MediaType, Size: size}, nil
}
// Then try reusing blobs from other locations.
- for _, candidate := range cache.CandidateLocations(d.ref.Transport(), bicTransportScope(d.ref), info.Digest, canSubstitute) {
+ bic := blobinfocache.FromBlobInfoCache(cache)
+ candidates := bic.CandidateLocations2(d.ref.Transport(), bicTransportScope(d.ref), info.Digest, canSubstitute)
+ for _, candidate := range candidates {
candidateRepo, err := parseBICLocationReference(candidate.Location)
if err != nil {
logrus.Debugf("Error parsing BlobInfoCache location reference: %s", err)
continue
}
- logrus.Debugf("Trying to reuse cached location %s in %s", candidate.Digest.String(), candidateRepo.Name())
+ 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 {
+ logrus.Debugf("Trying to reuse cached location %s with no compression in %s", candidate.Digest.String(), candidateRepo.Name())
+ }
// Sanity checks:
if reference.Domain(candidateRepo) != reference.Domain(d.ref.ref) {
@@ -351,8 +359,15 @@ func (d *dockerImageDestination) TryReusingBlob(ctx context.Context, info types.
continue
}
}
- cache.RecordKnownLocation(d.ref.Transport(), bicTransportScope(d.ref), candidate.Digest, newBICLocationReference(d.ref))
- return true, types.BlobInfo{Digest: candidate.Digest, Size: size}, nil
+
+ bic.RecordKnownLocation(d.ref.Transport(), bicTransportScope(d.ref), candidate.Digest, newBICLocationReference(d.ref))
+
+ compressionOperation, compressionAlgorithm, err := blobinfocache.OperationAndAlgorithmForCompressor(candidate.CompressorName)
+ if err != nil {
+ return false, types.BlobInfo{}, err
+ }
+
+ return true, types.BlobInfo{Digest: candidate.Digest, MediaType: info.MediaType, Size: size, CompressionOperation: compressionOperation, CompressionAlgorithm: compressionAlgorithm}, nil
}
return false, types.BlobInfo{}, nil
diff --git a/docker/tarfile/dest.go b/docker/tarfile/dest.go
index e16829d965..4f2465cac4 100644
--- a/docker/tarfile/dest.go
+++ b/docker/tarfile/dest.go
@@ -86,7 +86,9 @@ func (d *Destination) PutBlob(ctx context.Context, stream io.Reader, inputInfo t
// (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree).
// info.Digest must not be empty.
// If canSubstitute, TryReusingBlob can use an equivalent equivalent of the desired blob; in that case the returned info may not match the input.
-// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size.
+// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size, and may
+// include CompressionOperation and CompressionAlgorithm fields to indicate that a change to the compression type should be
+// 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.
// May use and/or update cache.
func (d *Destination) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache, canSubstitute bool) (bool, types.BlobInfo, error) {
diff --git a/internal/blobinfocache/blobinfocache.go b/internal/blobinfocache/blobinfocache.go
new file mode 100644
index 0000000000..b77c194d41
--- /dev/null
+++ b/internal/blobinfocache/blobinfocache.go
@@ -0,0 +1,72 @@
+package blobinfocache
+
+import (
+ "github.com/containers/image/v5/pkg/compression"
+ "github.com/containers/image/v5/types"
+ digest "github.com/opencontainers/go-digest"
+)
+
+// FromBlobInfoCache returns a BlobInfoCache2 based on a BlobInfoCache, returning the original
+// object if it implements BlobInfoCache2, or a wrapper which discards compression information
+// if it only implements BlobInfoCache.
+func FromBlobInfoCache(bic types.BlobInfoCache) BlobInfoCache2 {
+ if bic2, ok := bic.(BlobInfoCache2); ok {
+ return bic2
+ }
+ return &wrappedBlobInfoCache{
+ BlobInfoCache: bic,
+ }
+}
+
+type wrappedBlobInfoCache struct {
+ types.BlobInfoCache
+}
+
+func (bic *wrappedBlobInfoCache) RecordDigestCompressorName(anyDigest digest.Digest, compressorName string) {
+}
+
+func (bic *wrappedBlobInfoCache) CandidateLocations2(transport types.ImageTransport, scope types.BICTransportScope, digest digest.Digest, canSubstitute bool) []BICReplacementCandidate2 {
+ oldCandidates := bic.CandidateLocations(transport, scope, digest, canSubstitute)
+ results := make([]BICReplacementCandidate2, 0, len(oldCandidates))
+ for _, c := range oldCandidates {
+ results = append(results, BICReplacementCandidate2{
+ Digest: c.Digest,
+ Location: c.Location,
+ CompressorName: UnknownCompression,
+ })
+ }
+ return results
+}
+
+// CandidateLocationsFromV2 converts a slice of BICReplacementCandidate2 to a slice of
+// types.BICReplacementCandidate, dropping compression information.
+func CandidateLocationsFromV2(v2candidates []BICReplacementCandidate2) []types.BICReplacementCandidate {
+ candidates := make([]types.BICReplacementCandidate, 0, len(v2candidates))
+ for _, c := range v2candidates {
+ candidates = append(candidates, types.BICReplacementCandidate{
+ Digest: c.Digest,
+ Location: c.Location,
+ })
+ }
+ return candidates
+}
+
+// OperationAndAlgorithmForCompressor returns CompressionOperation and CompressionAlgorithm
+// values suitable for inclusion in a types.BlobInfo structure, based on the name of the
+// compression algorithm, or Uncompressed, or UnknownCompression. This is typically used by
+// TryReusingBlob() implementations to set values in the BlobInfo structure that they return
+// upon success.
+func OperationAndAlgorithmForCompressor(compressorName string) (types.LayerCompression, *compression.Algorithm, error) {
+ switch compressorName {
+ case Uncompressed:
+ return types.Decompress, nil, nil
+ case UnknownCompression:
+ return types.PreserveOriginal, nil, nil
+ default:
+ algo, err := compression.AlgorithmByName(compressorName)
+ if err == nil {
+ return types.Compress, &algo, nil
+ }
+ return types.PreserveOriginal, nil, err
+ }
+}
diff --git a/internal/blobinfocache/types.go b/internal/blobinfocache/types.go
new file mode 100644
index 0000000000..7116b38d9c
--- /dev/null
+++ b/internal/blobinfocache/types.go
@@ -0,0 +1,46 @@
+package blobinfocache
+
+import (
+ "github.com/containers/image/v5/types"
+ digest "github.com/opencontainers/go-digest"
+)
+
+const (
+ // Uncompressed is the value we store in a blob info cache to indicate that we know that the blob in
+ // the corresponding location is not compressed.
+ Uncompressed = "uncompressed"
+ // UnknownCompression s the value we store in a blob info cache to indicate that we don't know if the
+ // blob in the corresponding location is compressed or not.
+ UnknownCompression = "unknown"
+)
+
+// BlobInfoCache2 extends BlobInfoCache by adding the ability to track information about what kind
+// of compression was applied to the blobs it keeps information about.
+type BlobInfoCache2 interface {
+ types.BlobInfoCache
+ blobInfoCache2WithoutV1
+}
+
+// BICReplacementCandidate2 is an item returned by BlobInfoCache2.CandidateLocations2.
+type BICReplacementCandidate2 struct {
+ Digest digest.Digest
+ CompressorName string // either the Name() of a known pkg/compression.Algorithm, or Uncompressed or UnknownCompression
+ Location types.BICLocationReference
+}
+
+// BlobInfoCache2WithoutV1 contains extended versions of the BlobInfoCache methods which add the
+// ability to track information about what kind of compression was applied to the blobs it keeps
+// information about.
+type blobInfoCache2WithoutV1 interface {
+ // RecordDigestCompressorName records a compressor for the blob with the specified digest,
+ // or Uncompressed or UnknownCompression.
+ RecordDigestCompressorName(anyDigest digest.Digest, compressorName string)
+ // CandidateLocations2 returns a prioritized, limited, number of blobs and their locations
+ // that could possibly be reused within the specified (transport scope) (if they still
+ // exist, which is not guaranteed).
+ //
+ // If !canSubstitute, the returned cadidates will match the submitted digest exactly; if
+ // canSubstitute, data from previous RecordDigestUncompressedPair calls is used to also look
+ // up variants of the blob which have the same uncompressed digest.
+ CandidateLocations2(transport types.ImageTransport, scope types.BICTransportScope, digest digest.Digest, canSubstitute bool) []BICReplacementCandidate2
+}
diff --git a/manifest/common.go b/manifest/common.go
index fa2b39e0ea..e0c7b1f9bd 100644
--- a/manifest/common.go
+++ b/manifest/common.go
@@ -5,7 +5,6 @@ import (
"github.com/containers/image/v5/pkg/compression"
"github.com/containers/image/v5/types"
- "github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
@@ -70,15 +69,15 @@ func compressionVariantMIMEType(variantTable []compressionMIMETypeSet, mimeType
return res, nil
}
if name != mtsUncompressed {
- return "", fmt.Errorf("%s compression is not supported", name)
+ return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("%s compression is not supported for type %q", name, mt)}
}
- return "", errors.New("uncompressed variant is not supported")
+ return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mt)}
}
if name != mtsUncompressed {
return "", fmt.Errorf("unknown compression algorithm %s", name)
}
// We can't very well say “the idea of no compression is unknown”
- return "", errors.New("uncompressed variant is not supported")
+ return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mt)}
}
}
}
@@ -99,6 +98,12 @@ func updatedMIMEType(variantTable []compressionMIMETypeSet, mimeType string, upd
// {de}compressed.
switch updated.CompressionOperation {
case types.PreserveOriginal:
+ // Force a change to the media type if we're being told to use a particular compressor,
+ // since it might be different from the one associated with the media type. Otherwise,
+ // try to keep the original media type.
+ if updated.CompressionAlgorithm != nil && updated.CompressionAlgorithm.Name() != mtsUncompressed {
+ return compressionVariantMIMEType(variantTable, mimeType, updated.CompressionAlgorithm)
+ }
// Keep the original media type.
return mimeType, nil
@@ -116,3 +121,11 @@ func updatedMIMEType(variantTable []compressionMIMETypeSet, mimeType string, upd
return "", fmt.Errorf("unknown compression operation (%d)", updated.CompressionOperation)
}
}
+
+type ManifestLayerCompressionIncompatibilityError struct {
+ text string
+}
+
+func (m ManifestLayerCompressionIncompatibilityError) Error() string {
+ return m.text
+}
diff --git a/oci/archive/oci_dest.go b/oci/archive/oci_dest.go
index 23d4713252..c874eb775c 100644
--- a/oci/archive/oci_dest.go
+++ b/oci/archive/oci_dest.go
@@ -103,7 +103,9 @@ func (d *ociArchiveImageDestination) PutBlob(ctx context.Context, stream io.Read
// (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree).
// info.Digest must not be empty.
// If canSubstitute, TryReusingBlob can use an equivalent equivalent of the desired blob; in that case the returned info may not match the input.
-// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size.
+// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size, and may
+// include CompressionOperation and CompressionAlgorithm fields to indicate that a change to the compression type should be
+// 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.
// May use and/or update cache.
func (d *ociArchiveImageDestination) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache, canSubstitute bool) (bool, types.BlobInfo, error) {
diff --git a/oci/layout/oci_dest.go b/oci/layout/oci_dest.go
index 0c88e1ef0c..1230e8ca37 100644
--- a/oci/layout/oci_dest.go
+++ b/oci/layout/oci_dest.go
@@ -186,7 +186,9 @@ func (d *ociImageDestination) PutBlob(ctx context.Context, stream io.Reader, inp
// (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree).
// info.Digest must not be empty.
// If canSubstitute, TryReusingBlob can use an equivalent equivalent of the desired blob; in that case the returned info may not match the input.
-// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size.
+// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size, and may
+// include CompressionOperation and CompressionAlgorithm fields to indicate that a change to the compression type should be
+// 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.
// May use and/or update cache.
func (d *ociImageDestination) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache, canSubstitute bool) (bool, types.BlobInfo, error) {
@@ -204,6 +206,7 @@ func (d *ociImageDestination) TryReusingBlob(ctx context.Context, info types.Blo
if err != nil {
return false, types.BlobInfo{}, err
}
+
return true, types.BlobInfo{Digest: info.Digest, Size: finfo.Size()}, nil
}
diff --git a/openshift/openshift.go b/openshift/openshift.go
index c4c84dd545..426046e662 100644
--- a/openshift/openshift.go
+++ b/openshift/openshift.go
@@ -410,7 +410,9 @@ func (d *openshiftImageDestination) PutBlob(ctx context.Context, stream io.Reade
// (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree).
// info.Digest must not be empty.
// If canSubstitute, TryReusingBlob can use an equivalent equivalent of the desired blob; in that case the returned info may not match the input.
-// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size.
+// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size, and may
+// include CompressionOperation and CompressionAlgorithm fields to indicate that a change to the compression type should be
+// 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.
// May use and/or update cache.
func (d *openshiftImageDestination) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache, canSubstitute bool) (bool, types.BlobInfo, error) {
diff --git a/ostree/ostree_dest.go b/ostree/ostree_dest.go
index b518122e29..c91a49c57a 100644
--- a/ostree/ostree_dest.go
+++ b/ostree/ostree_dest.go
@@ -339,7 +339,9 @@ func (d *ostreeImageDestination) importConfig(repo *otbuiltin.Repo, blob *blobTo
// (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree).
// info.Digest must not be empty.
// If canSubstitute, TryReusingBlob can use an equivalent equivalent of the desired blob; in that case the returned info may not match the input.
-// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size.
+// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size, and may
+// include CompressionOperation and CompressionAlgorithm fields to indicate that a change to the compression type should be
+// 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.
// May use and/or update cache.
func (d *ostreeImageDestination) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache, canSubstitute bool) (bool, types.BlobInfo, error) {
diff --git a/pkg/blobinfocache/boltdb/boltdb.go b/pkg/blobinfocache/boltdb/boltdb.go
index 200dab5934..535a74b8c9 100644
--- a/pkg/blobinfocache/boltdb/boltdb.go
+++ b/pkg/blobinfocache/boltdb/boltdb.go
@@ -7,6 +7,7 @@ import (
"sync"
"time"
+ "github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/pkg/blobinfocache/internal/prioritize"
"github.com/containers/image/v5/types"
"github.com/opencontainers/go-digest"
@@ -22,6 +23,8 @@ var (
// uncompressedDigestBucket stores a mapping from any digest to an uncompressed digest.
uncompressedDigestBucket = []byte("uncompressedDigest")
+ // digestCompressorBucket stores a mapping from any digest to a compressor, or blobinfocache.Uncompressed
+ digestCompressorBucket = []byte("digestCompressor")
// digestByUncompressedBucket stores a bucket per uncompressed digest, with the bucket containing a set of digests for that uncompressed digest
// (as a set of key=digest, value="" pairs)
digestByUncompressedBucket = []byte("digestByUncompressed")
@@ -95,6 +98,9 @@ type cache struct {
//
// Most users should call blobinfocache.DefaultCache instead.
func New(path string) types.BlobInfoCache {
+ return new2(path)
+}
+func new2(path string) *cache {
return &cache{path: path}
}
@@ -220,6 +226,30 @@ func (bdc *cache) RecordDigestUncompressedPair(anyDigest digest.Digest, uncompre
}) // FIXME? Log error (but throttle the log volume on repeated accesses)?
}
+// RecordDigestCompressorName records that the blob with digest anyDigest was compressed with the specified
+// compressor, or is blobinfocache.Uncompressed.
+// WARNING: Only call this for LOCALLY VERIFIED data; don’t record a digest pair just because some remote author claims so (e.g.
+// because a manifest/config pair exists); otherwise the cache could be poisoned and allow substituting unexpected blobs.
+// (Eventually, the DiffIDs in image config could detect the substitution, but that may be too late, and not all image formats contain that data.)
+func (bdc *cache) RecordDigestCompressorName(anyDigest digest.Digest, compressorName string) {
+ _ = bdc.update(func(tx *bolt.Tx) error {
+ b, err := tx.CreateBucketIfNotExists(digestCompressorBucket)
+ if err != nil {
+ return err
+ }
+ key := []byte(anyDigest.String())
+ if previousBytes := b.Get(key); previousBytes != nil {
+ if string(previousBytes) != compressorName {
+ logrus.Warnf("Compressor for blob with digest %s previously recorded as %s, now %s", anyDigest, string(previousBytes), compressorName)
+ }
+ }
+ if compressorName == blobinfocache.UnknownCompression {
+ return b.Delete([]byte(anyDigest.String()))
+ }
+ return b.Put([]byte(anyDigest.String()), []byte(compressorName))
+ }) // FIXME? Log error (but throttle the log volume on repeated accesses)?
+}
+
// RecordKnownLocation records that a blob with the specified digest exists within the specified (transport, scope) scope,
// and can be reused given the opaque location data.
func (bdc *cache) RecordKnownLocation(transport types.ImageTransport, scope types.BICTransportScope, blobDigest digest.Digest, location types.BICLocationReference) {
@@ -252,20 +282,28 @@ func (bdc *cache) RecordKnownLocation(transport types.ImageTransport, scope type
}
// appendReplacementCandiates creates prioritize.CandidateWithTime values for digest in scopeBucket, and returns the result of appending them to candidates.
-func (bdc *cache) appendReplacementCandidates(candidates []prioritize.CandidateWithTime, scopeBucket *bolt.Bucket, digest digest.Digest) []prioritize.CandidateWithTime {
- b := scopeBucket.Bucket([]byte(digest.String()))
+func (bdc *cache) appendReplacementCandidates(candidates []prioritize.CandidateWithTime, scopeBucket, compressionBucket *bolt.Bucket, digest digest.Digest) []prioritize.CandidateWithTime {
+ digestKey := []byte(digest.String())
+ b := scopeBucket.Bucket(digestKey)
if b == nil {
return candidates
}
+ compressorName := blobinfocache.UnknownCompression
+ if compressionBucket != nil {
+ if compressorNameValue := compressionBucket.Get(digestKey); len(compressorNameValue) > 0 {
+ compressorName = string(compressorNameValue)
+ }
+ }
_ = b.ForEach(func(k, v []byte) error {
t := time.Time{}
if err := t.UnmarshalBinary(v); err != nil {
return err
}
candidates = append(candidates, prioritize.CandidateWithTime{
- Candidate: types.BICReplacementCandidate{
- Digest: digest,
- Location: types.BICLocationReference{Opaque: string(k)},
+ Candidate: blobinfocache.BICReplacementCandidate2{
+ Digest: digest,
+ CompressorName: compressorName,
+ Location: types.BICLocationReference{Opaque: string(k)},
},
LastSeen: t,
})
@@ -274,13 +312,13 @@ func (bdc *cache) appendReplacementCandidates(candidates []prioritize.CandidateW
return candidates
}
-// CandidateLocations returns a prioritized, limited, number of blobs and their locations that could possibly be reused
+// CandidateLocations2 returns a prioritized, limited, number of blobs and their locations that could possibly be reused
// within the specified (transport scope) (if they still exist, which is not guaranteed).
//
// If !canSubstitute, the returned cadidates will match the submitted digest exactly; if canSubstitute,
// data from previous RecordDigestUncompressedPair calls is used to also look up variants of the blob which have the same
// uncompressed digest.
-func (bdc *cache) CandidateLocations(transport types.ImageTransport, scope types.BICTransportScope, primaryDigest digest.Digest, canSubstitute bool) []types.BICReplacementCandidate {
+func (bdc *cache) CandidateLocations2(transport types.ImageTransport, scope types.BICTransportScope, primaryDigest digest.Digest, canSubstitute bool) []blobinfocache.BICReplacementCandidate2 {
res := []prioritize.CandidateWithTime{}
var uncompressedDigestValue digest.Digest // = ""
if err := bdc.view(func(tx *bolt.Tx) error {
@@ -296,8 +334,11 @@ func (bdc *cache) CandidateLocations(transport types.ImageTransport, scope types
if scopeBucket == nil {
return nil
}
+ // compressionBucket won't have been created if previous writers never recorded info about compression,
+ // and we don't want to fail just because of that
+ compressionBucket := tx.Bucket(digestCompressorBucket)
- res = bdc.appendReplacementCandidates(res, scopeBucket, primaryDigest)
+ res = bdc.appendReplacementCandidates(res, scopeBucket, compressionBucket, primaryDigest)
if canSubstitute {
if uncompressedDigestValue = bdc.uncompressedDigest(tx, primaryDigest); uncompressedDigestValue != "" {
b := tx.Bucket(digestByUncompressedBucket)
@@ -310,7 +351,7 @@ func (bdc *cache) CandidateLocations(transport types.ImageTransport, scope types
return err
}
if d != primaryDigest && d != uncompressedDigestValue {
- res = bdc.appendReplacementCandidates(res, scopeBucket, d)
+ res = bdc.appendReplacementCandidates(res, scopeBucket, compressionBucket, d)
}
return nil
}); err != nil {
@@ -319,14 +360,24 @@ func (bdc *cache) CandidateLocations(transport types.ImageTransport, scope types
}
}
if uncompressedDigestValue != primaryDigest {
- res = bdc.appendReplacementCandidates(res, scopeBucket, uncompressedDigestValue)
+ res = bdc.appendReplacementCandidates(res, scopeBucket, compressionBucket, uncompressedDigestValue)
}
}
}
return nil
}); err != nil { // Including os.IsNotExist(err)
- return []types.BICReplacementCandidate{} // FIXME? Log err (but throttle the log volume on repeated accesses)?
+ return []blobinfocache.BICReplacementCandidate2{} // FIXME? Log err (but throttle the log volume on repeated accesses)?
}
return prioritize.DestructivelyPrioritizeReplacementCandidates(res, primaryDigest, uncompressedDigestValue)
}
+
+// CandidateLocations returns a prioritized, limited, number of blobs and their locations that could possibly be reused
+// within the specified (transport scope) (if they still exist, which is not guaranteed).
+//
+// If !canSubstitute, the returned cadidates will match the submitted digest exactly; if canSubstitute,
+// data from previous RecordDigestUncompressedPair calls is used to also look up variants of the blob which have the same
+// uncompressed digest.
+func (bdc *cache) CandidateLocations(transport types.ImageTransport, scope types.BICTransportScope, primaryDigest digest.Digest, canSubstitute bool) []types.BICReplacementCandidate {
+ return blobinfocache.CandidateLocationsFromV2(bdc.CandidateLocations2(transport, scope, primaryDigest, canSubstitute))
+}
diff --git a/pkg/blobinfocache/boltdb/boltdb_test.go b/pkg/blobinfocache/boltdb/boltdb_test.go
index 079ea280f4..a91ab6d7dd 100644
--- a/pkg/blobinfocache/boltdb/boltdb_test.go
+++ b/pkg/blobinfocache/boltdb/boltdb_test.go
@@ -6,19 +6,21 @@ import (
"path/filepath"
"testing"
+ "github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/pkg/blobinfocache/internal/test"
- "github.com/containers/image/v5/types"
"github.com/stretchr/testify/require"
)
-func newTestCache(t *testing.T) (types.BlobInfoCache, func(t *testing.T)) {
+var _ blobinfocache.BlobInfoCache2 = &cache{}
+
+func newTestCache(t *testing.T) (blobinfocache.BlobInfoCache2, func(t *testing.T)) {
// We need a separate temporary directory here, because bolt.Open(…, &bolt.Options{Readonly:true}) can't deal with
// an existing but empty file, and incorrectly fails without releasing the lock - which in turn causes
// any future writes to hang. Creating a temporary directory allows us to use a path to a
// non-existent file, thus replicating the expected conditions for creating a new DB.
dir, err := ioutil.TempDir("", "boltdb")
require.NoError(t, err)
- return New(filepath.Join(dir, "db")), func(t *testing.T) {
+ return new2(filepath.Join(dir, "db")), func(t *testing.T) {
err = os.RemoveAll(dir)
require.NoError(t, err)
}
diff --git a/pkg/blobinfocache/internal/prioritize/prioritize.go b/pkg/blobinfocache/internal/prioritize/prioritize.go
index 5deca4a82d..6f5506d94c 100644
--- a/pkg/blobinfocache/internal/prioritize/prioritize.go
+++ b/pkg/blobinfocache/internal/prioritize/prioritize.go
@@ -6,7 +6,7 @@ import (
"sort"
"time"
- "github.com/containers/image/v5/types"
+ "github.com/containers/image/v5/internal/blobinfocache"
"github.com/opencontainers/go-digest"
)
@@ -17,8 +17,8 @@ const replacementAttempts = 5
// CandidateWithTime is the input to types.BICReplacementCandidate prioritization.
type CandidateWithTime struct {
- Candidate types.BICReplacementCandidate // The replacement candidate
- LastSeen time.Time // Time the candidate was last known to exist (either read or written)
+ Candidate blobinfocache.BICReplacementCandidate2 // The replacement candidate
+ LastSeen time.Time // Time the candidate was last known to exist (either read or written)
}
// candidateSortState is a local state implementing sort.Interface on candidates to prioritize,
@@ -79,7 +79,7 @@ func (css *candidateSortState) Swap(i, j int) {
// destructivelyPrioritizeReplacementCandidatesWithMax is destructivelyPrioritizeReplacementCandidates with a parameter for the
// number of entries to limit, only to make testing simpler.
-func destructivelyPrioritizeReplacementCandidatesWithMax(cs []CandidateWithTime, primaryDigest, uncompressedDigest digest.Digest, maxCandidates int) []types.BICReplacementCandidate {
+func destructivelyPrioritizeReplacementCandidatesWithMax(cs []CandidateWithTime, primaryDigest, uncompressedDigest digest.Digest, maxCandidates int) []blobinfocache.BICReplacementCandidate2 {
// We don't need to use sort.Stable() because nanosecond timestamps are (presumably?) unique, so no two elements should
// compare equal.
sort.Sort(&candidateSortState{
@@ -92,7 +92,7 @@ func destructivelyPrioritizeReplacementCandidatesWithMax(cs []CandidateWithTime,
if resLength > maxCandidates {
resLength = maxCandidates
}
- res := make([]types.BICReplacementCandidate, resLength)
+ res := make([]blobinfocache.BICReplacementCandidate2, resLength)
for i := range res {
res[i] = cs[i].Candidate
}
@@ -105,6 +105,6 @@ func destructivelyPrioritizeReplacementCandidatesWithMax(cs []CandidateWithTime,
//
// WARNING: The array of candidates is destructively modified. (The implementation of this function could of course
// make a copy, but all CandidateLocations implementations build the slice of candidates only for the single purpose of calling this function anyway.)
-func DestructivelyPrioritizeReplacementCandidates(cs []CandidateWithTime, primaryDigest, uncompressedDigest digest.Digest) []types.BICReplacementCandidate {
+func DestructivelyPrioritizeReplacementCandidates(cs []CandidateWithTime, primaryDigest, uncompressedDigest digest.Digest) []blobinfocache.BICReplacementCandidate2 {
return destructivelyPrioritizeReplacementCandidatesWithMax(cs, primaryDigest, uncompressedDigest, replacementAttempts)
}
diff --git a/pkg/blobinfocache/internal/prioritize/prioritize_test.go b/pkg/blobinfocache/internal/prioritize/prioritize_test.go
index 7e4257d60d..2231af16ed 100644
--- a/pkg/blobinfocache/internal/prioritize/prioritize_test.go
+++ b/pkg/blobinfocache/internal/prioritize/prioritize_test.go
@@ -5,6 +5,8 @@ import (
"testing"
"time"
+ "github.com/containers/image/v5/internal/blobinfocache"
+ "github.com/containers/image/v5/pkg/compression"
"github.com/containers/image/v5/types"
"github.com/opencontainers/go-digest"
"github.com/stretchr/testify/assert"
@@ -21,28 +23,28 @@ var (
// cssLiteral contains a non-trivial candidateSortState shared among several tests below.
cssLiteral = candidateSortState{
cs: []CandidateWithTime{
- {types.BICReplacementCandidate{Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A1"}}, time.Unix(1, 0)},
- {types.BICReplacementCandidate{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U2"}}, time.Unix(1, 1)},
- {types.BICReplacementCandidate{Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A2"}}, time.Unix(1, 1)},
- {types.BICReplacementCandidate{Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P1"}}, time.Unix(1, 0)},
- {types.BICReplacementCandidate{Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B1"}}, time.Unix(1, 1)},
- {types.BICReplacementCandidate{Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P2"}}, time.Unix(1, 1)},
- {types.BICReplacementCandidate{Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B2"}}, time.Unix(2, 0)},
- {types.BICReplacementCandidate{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U1"}}, time.Unix(1, 0)},
+ {blobinfocache.BICReplacementCandidate2{Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A1"}, CompressorName: compression.Xz.Name()}, time.Unix(1, 0)},
+ {blobinfocache.BICReplacementCandidate2{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U2"}, CompressorName: compression.Gzip.Name()}, time.Unix(1, 1)},
+ {blobinfocache.BICReplacementCandidate2{Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A2"}, CompressorName: blobinfocache.Uncompressed}, time.Unix(1, 1)},
+ {blobinfocache.BICReplacementCandidate2{Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P1"}, CompressorName: blobinfocache.UnknownCompression}, time.Unix(1, 0)},
+ {blobinfocache.BICReplacementCandidate2{Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B1"}, CompressorName: compression.Bzip2.Name()}, time.Unix(1, 1)},
+ {blobinfocache.BICReplacementCandidate2{Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P2"}, CompressorName: compression.Gzip.Name()}, time.Unix(1, 1)},
+ {blobinfocache.BICReplacementCandidate2{Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B2"}, CompressorName: blobinfocache.Uncompressed}, time.Unix(2, 0)},
+ {blobinfocache.BICReplacementCandidate2{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U1"}, CompressorName: blobinfocache.UnknownCompression}, time.Unix(1, 0)},
},
primaryDigest: digestCompressedPrimary,
uncompressedDigest: digestUncompressed,
}
// cssExpectedReplacementCandidates is the fully-sorted, unlimited, result of prioritizing cssLiteral.
- cssExpectedReplacementCandidates = []types.BICReplacementCandidate{
- {Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P2"}},
- {Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P1"}},
- {Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B2"}},
- {Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A2"}},
- {Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B1"}},
- {Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A1"}},
- {Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U2"}},
- {Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U1"}},
+ cssExpectedReplacementCandidates = []blobinfocache.BICReplacementCandidate2{
+ {Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P2"}, CompressorName: compression.Gzip.Name()},
+ {Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P1"}, CompressorName: blobinfocache.UnknownCompression},
+ {Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B2"}, CompressorName: blobinfocache.Uncompressed},
+ {Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A2"}, CompressorName: blobinfocache.Uncompressed},
+ {Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B1"}, CompressorName: compression.Bzip2.Name()},
+ {Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A1"}, CompressorName: compression.Xz.Name()},
+ {Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U2"}, CompressorName: compression.Gzip.Name()},
+ {Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U1"}, CompressorName: blobinfocache.UnknownCompression},
}
)
@@ -74,8 +76,8 @@ func TestCandidateSortStateLess(t *testing.T) {
caseName := fmt.Sprintf("%s %v", c.name, tms)
css := candidateSortState{
cs: []CandidateWithTime{
- {types.BICReplacementCandidate{Digest: c.d0, Location: types.BICLocationReference{Opaque: "L0"}}, time.Unix(tms[0], 0)},
- {types.BICReplacementCandidate{Digest: c.d1, Location: types.BICLocationReference{Opaque: "L1"}}, time.Unix(tms[1], 0)},
+ {blobinfocache.BICReplacementCandidate2{Digest: c.d0, Location: types.BICLocationReference{Opaque: "L0"}, CompressorName: compression.Gzip.Name()}, time.Unix(tms[0], 0)},
+ {blobinfocache.BICReplacementCandidate2{Digest: c.d1, Location: types.BICLocationReference{Opaque: "L1"}, CompressorName: compression.Zstd.Name()}, time.Unix(tms[1], 0)},
},
primaryDigest: digestCompressedPrimary,
uncompressedDigest: digestUncompressed,
@@ -113,8 +115,8 @@ func TestCandidateSortStateLess(t *testing.T) {
} {
css := candidateSortState{
cs: []CandidateWithTime{
- {types.BICReplacementCandidate{Digest: c.p0.d, Location: types.BICLocationReference{Opaque: "L0"}}, time.Unix(c.p0.t, 0)},
- {types.BICReplacementCandidate{Digest: c.p1.d, Location: types.BICLocationReference{Opaque: "L1"}}, time.Unix(c.p1.t, 0)},
+ {blobinfocache.BICReplacementCandidate2{Digest: c.p0.d, Location: types.BICLocationReference{Opaque: "L0"}, CompressorName: compression.Gzip.Name()}, time.Unix(c.p0.t, 0)},
+ {blobinfocache.BICReplacementCandidate2{Digest: c.p1.d, Location: types.BICLocationReference{Opaque: "L1"}, CompressorName: compression.Zstd.Name()}, time.Unix(c.p1.t, 0)},
},
primaryDigest: digestCompressedPrimary,
uncompressedDigest: digestUncompressed,
diff --git a/pkg/blobinfocache/internal/test/test.go b/pkg/blobinfocache/internal/test/test.go
index b415e9c40e..c3d7562ddb 100644
--- a/pkg/blobinfocache/internal/test/test.go
+++ b/pkg/blobinfocache/internal/test/test.go
@@ -4,6 +4,7 @@ package test
import (
"testing"
+ "github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/internal/testing/mocks"
"github.com/containers/image/v5/types"
digest "github.com/opencontainers/go-digest"
@@ -16,19 +17,24 @@ const (
digestCompressedA = digest.Digest("sha256:3333333333333333333333333333333333333333333333333333333333333333")
digestCompressedB = digest.Digest("sha256:4444444444444444444444444444444444444444444444444444444444444444")
digestCompressedUnrelated = digest.Digest("sha256:5555555555555555555555555555555555555555555555555555555555555555")
+ compressorNameU = "compressorName/U"
+ compressorNameA = "compressorName/A"
+ compressorNameB = "compressorName/B"
+ compressorNameCU = "compressorName/CU"
)
// GenericCache runs an implementation-independent set of tests, given a
// newTestCache, which can be called repeatedly and always returns a (cache, cleanup callback) pair
-func GenericCache(t *testing.T, newTestCache func(t *testing.T) (types.BlobInfoCache, func(t *testing.T))) {
+func GenericCache(t *testing.T, newTestCache func(t *testing.T) (blobinfocache.BlobInfoCache2, func(t *testing.T))) {
for _, s := range []struct {
name string
- fn func(t *testing.T, cache types.BlobInfoCache)
+ fn func(t *testing.T, cache blobinfocache.BlobInfoCache2)
}{
{"UncompressedDigest", testGenericUncompressedDigest},
{"RecordDigestUncompressedPair", testGenericRecordDigestUncompressedPair},
{"RecordKnownLocations", testGenericRecordKnownLocations},
{"CandidateLocations", testGenericCandidateLocations},
+ {"CandidateLocations2", testGenericCandidateLocations2},
} {
t.Run(s.name, func(t *testing.T) {
cache, cleanup := newTestCache(t)
@@ -38,7 +44,7 @@ func GenericCache(t *testing.T, newTestCache func(t *testing.T) (types.BlobInfoC
}
}
-func testGenericUncompressedDigest(t *testing.T, cache types.BlobInfoCache) {
+func testGenericUncompressedDigest(t *testing.T, cache blobinfocache.BlobInfoCache2) {
// Nothing is known.
assert.Equal(t, digest.Digest(""), cache.UncompressedDigest(digestUnknown))
@@ -55,7 +61,7 @@ func testGenericUncompressedDigest(t *testing.T, cache types.BlobInfoCache) {
assert.Equal(t, digestCompressedUnrelated, cache.UncompressedDigest(digestCompressedUnrelated))
}
-func testGenericRecordDigestUncompressedPair(t *testing.T, cache types.BlobInfoCache) {
+func testGenericRecordDigestUncompressedPair(t *testing.T, cache blobinfocache.BlobInfoCache2) {
for i := 0; i < 2; i++ { // Record the same data twice to ensure redundant writes don’t break things.
// Known compressed→uncompressed mapping
cache.RecordDigestUncompressedPair(digestCompressedA, digestUncompressed)
@@ -70,7 +76,7 @@ func testGenericRecordDigestUncompressedPair(t *testing.T, cache types.BlobInfoC
}
}
-func testGenericRecordKnownLocations(t *testing.T, cache types.BlobInfoCache) {
+func testGenericRecordKnownLocations(t *testing.T, cache blobinfocache.BlobInfoCache2) {
transport := mocks.NameImageTransport("==BlobInfocache transport mock")
for i := 0; i < 2; i++ { // Record the same data twice to ensure redundant writes don’t break things.
for _, scopeName := range []string{"A", "B"} { // Run the test in two different scopes to verify they don't affect each other.
@@ -84,6 +90,10 @@ func testGenericRecordKnownLocations(t *testing.T, cache types.BlobInfoCache) {
{Digest: digest, Location: lr1},
{Digest: digest, Location: lr2},
}, cache.CandidateLocations(transport, scope, digest, false))
+ assert.Equal(t, []blobinfocache.BICReplacementCandidate2{
+ {Digest: digest, CompressorName: blobinfocache.UnknownCompression, Location: lr1},
+ {Digest: digest, CompressorName: blobinfocache.UnknownCompression, Location: lr2},
+ }, cache.CandidateLocations2(transport, scope, digest, false))
}
}
}
@@ -92,6 +102,7 @@ func testGenericRecordKnownLocations(t *testing.T, cache types.BlobInfoCache) {
// candidate is a shorthand for types.BICReplacementCandiddate
type candidate struct {
d digest.Digest
+ cn string
lr string
}
@@ -103,7 +114,15 @@ func assertCandidatesMatch(t *testing.T, scopeName string, expected []candidate,
assert.Equal(t, e, actual)
}
-func testGenericCandidateLocations(t *testing.T, cache types.BlobInfoCache) {
+func assertCandidatesMatch2(t *testing.T, scopeName string, expected []candidate, actual []blobinfocache.BICReplacementCandidate2) {
+ e := make([]blobinfocache.BICReplacementCandidate2, len(expected))
+ for i, ev := range expected {
+ e[i] = blobinfocache.BICReplacementCandidate2{Digest: ev.d, CompressorName: ev.cn, Location: types.BICLocationReference{Opaque: scopeName + ev.lr}}
+ }
+ assert.Equal(t, e, actual)
+}
+
+func testGenericCandidateLocations(t *testing.T, cache blobinfocache.BlobInfoCache2) {
transport := mocks.NameImageTransport("==BlobInfocache transport mock")
cache.RecordDigestUncompressedPair(digestCompressedA, digestUncompressed)
cache.RecordDigestUncompressedPair(digestCompressedB, digestUncompressed)
@@ -164,6 +183,92 @@ func testGenericCandidateLocations(t *testing.T, cache types.BlobInfoCache) {
assertCandidatesMatch(t, scopeName, []candidate{
{d: digestCompressedUnrelated, lr: "CU1"}, {d: digestCompressedUnrelated, lr: "CU2"},
}, cache.CandidateLocations(transport, scope, digestCompressedUnrelated, true))
+ }
+}
+
+func testGenericCandidateLocations2(t *testing.T, cache blobinfocache.BlobInfoCache2) {
+ transport := mocks.NameImageTransport("==BlobInfocache transport mock")
+ cache.RecordDigestUncompressedPair(digestCompressedA, digestUncompressed)
+ cache.RecordDigestUncompressedPair(digestCompressedB, digestUncompressed)
+ cache.RecordDigestUncompressedPair(digestUncompressed, digestUncompressed)
+ digestNameSet := []struct {
+ n string
+ d digest.Digest
+ m string
+ }{
+ {"U", digestUncompressed, compressorNameU},
+ {"A", digestCompressedA, compressorNameA},
+ {"B", digestCompressedB, compressorNameB},
+ {"CU", digestCompressedUnrelated, compressorNameCU},
+ }
+
+ for _, scopeName := range []string{"A", "B"} { // Run the test in two different scopes to verify they don't affect each other.
+ scope := types.BICTransportScope{Opaque: scopeName}
+ // Nothing is known.
+ assert.Equal(t, []blobinfocache.BICReplacementCandidate2{}, cache.CandidateLocations2(transport, scope, digestUnknown, false))
+ assert.Equal(t, []blobinfocache.BICReplacementCandidate2{}, cache.CandidateLocations2(transport, scope, digestUnknown, true))
+
+ // Record "2" entries before "1" entries; then results should sort "1" (more recent) before "2" (older)
+ for _, suffix := range []string{"2", "1"} {
+ for _, e := range digestNameSet {
+ cache.RecordKnownLocation(transport, scope, e.d, types.BICLocationReference{Opaque: scopeName + e.n + suffix})
+ }
+ }
+
+ for _, compressorKnown := range []bool{false, true} {
+ // Either set or clear the "known" compression
+ expectedCompressor := make(map[digest.Digest]string)
+ for _, e := range digestNameSet {
+ compressorName := e.m
+ if !compressorKnown {
+ compressorName = blobinfocache.UnknownCompression
+ }
+ cache.RecordDigestCompressorName(e.d, compressorName)
+ expectedCompressor[e.d] = compressorName
+ }
+
+ // No substitutions allowed:
+ for _, e := range digestNameSet {
+ assertCandidatesMatch2(t, scopeName, []candidate{
+ {d: e.d, cn: expectedCompressor[e.d], lr: e.n + "1"},
+ {d: e.d, cn: expectedCompressor[e.d], lr: e.n + "2"},
+ }, cache.CandidateLocations2(transport, scope, e.d, false))
+ }
+
+ // With substitutions: The original digest is always preferred, then other compressed, then the uncompressed one.
+ assertCandidatesMatch2(t, scopeName, []candidate{
+ {d: digestCompressedA, cn: expectedCompressor[digestCompressedA], lr: "A1"},
+ {d: digestCompressedA, cn: expectedCompressor[digestCompressedA], lr: "A2"},
+ {d: digestCompressedB, cn: expectedCompressor[digestCompressedB], lr: "B1"},
+ {d: digestCompressedB, cn: expectedCompressor[digestCompressedB], lr: "B2"},
+ {d: digestUncompressed, cn: expectedCompressor[digestUncompressed], lr: "U1"},
+ // Beyond the replacementAttempts limit: {d: digestUncompressed, cn: compressorNameCU, lr: "U2"},
+ }, cache.CandidateLocations2(transport, scope, digestCompressedA, true))
+
+ assertCandidatesMatch2(t, scopeName, []candidate{
+ {d: digestCompressedB, cn: expectedCompressor[digestCompressedB], lr: "B1"},
+ {d: digestCompressedB, cn: expectedCompressor[digestCompressedB], lr: "B2"},
+ {d: digestCompressedA, cn: expectedCompressor[digestCompressedA], lr: "A1"},
+ {d: digestCompressedA, cn: expectedCompressor[digestCompressedA], lr: "A2"},
+ {d: digestUncompressed, cn: expectedCompressor[digestUncompressed], lr: "U1"}, // Beyond the replacementAttempts limit: {d: digestUncompressed, lr: "U2"},
+ }, cache.CandidateLocations2(transport, scope, digestCompressedB, true))
+
+ assertCandidatesMatch2(t, scopeName, []candidate{
+ {d: digestUncompressed, cn: expectedCompressor[digestUncompressed], lr: "U1"},
+ {d: digestUncompressed, cn: expectedCompressor[digestUncompressed], lr: "U2"},
+ // "1" entries were added after "2", and A/Bs are sorted in the reverse of digestNameSet order
+ {d: digestCompressedB, cn: expectedCompressor[digestCompressedB], lr: "B1"},
+ {d: digestCompressedA, cn: expectedCompressor[digestCompressedA], lr: "A1"},
+ {d: digestCompressedB, cn: expectedCompressor[digestCompressedB], lr: "B2"},
+ // Beyond the replacementAttempts limit: {d: digestCompressedA, lr: "A2"},
+ }, cache.CandidateLocations2(transport, scope, digestUncompressed, true))
+
+ // Locations are known, but no relationships
+ assertCandidatesMatch2(t, scopeName, []candidate{
+ {d: digestCompressedUnrelated, cn: expectedCompressor[digestCompressedUnrelated], lr: "CU1"},
+ {d: digestCompressedUnrelated, cn: expectedCompressor[digestCompressedUnrelated], lr: "CU2"},
+ }, cache.CandidateLocations2(transport, scope, digestCompressedUnrelated, true))
+ }
}
}
diff --git a/pkg/blobinfocache/memory/memory.go b/pkg/blobinfocache/memory/memory.go
index 8f28c66238..ce0d1d8f95 100644
--- a/pkg/blobinfocache/memory/memory.go
+++ b/pkg/blobinfocache/memory/memory.go
@@ -5,6 +5,7 @@ import (
"sync"
"time"
+ "github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/pkg/blobinfocache/internal/prioritize"
"github.com/containers/image/v5/types"
digest "github.com/opencontainers/go-digest"
@@ -25,6 +26,7 @@ type cache struct {
uncompressedDigests map[digest.Digest]digest.Digest
digestsByUncompressed map[digest.Digest]map[digest.Digest]struct{} // stores a set of digests for each uncompressed digest
knownLocations map[locationKey]map[types.BICLocationReference]time.Time // stores last known existence time for each location reference
+ compressors map[digest.Digest]string // stores a compressor name, or blobinfocache.Unknown, for each digest
}
// New returns a BlobInfoCache implementation which is in-memory only.
@@ -36,10 +38,15 @@ type cache struct {
// Manual users of types.{ImageSource,ImageDestination} might also use
// this instead of a persistent cache.
func New() types.BlobInfoCache {
+ return new2()
+}
+
+func new2() *cache {
return &cache{
uncompressedDigests: map[digest.Digest]digest.Digest{},
digestsByUncompressed: map[digest.Digest]map[digest.Digest]struct{}{},
knownLocations: map[locationKey]map[types.BICLocationReference]time.Time{},
+ compressors: map[digest.Digest]string{},
}
}
@@ -87,8 +94,9 @@ func (mem *cache) RecordDigestUncompressedPair(anyDigest digest.Digest, uncompre
anyDigestSet[anyDigest] = struct{}{} // Possibly writing the same struct{}{} presence marker again.
}
-// RecordKnownLocation records that a blob with the specified digest exists within the specified (transport, scope) scope,
-// and can be reused given the opaque location data.
+// RecordKnownLocation records that a blob with the specified digest, whether compressed with the specified algorithm or
+// uncompressed if compressorName is "", exists within the specified (transport, scope) scope, and can be reused given
+// the opaque location data.
func (mem *cache) RecordKnownLocation(transport types.ImageTransport, scope types.BICTransportScope, blobDigest digest.Digest, location types.BICLocationReference) {
mem.mutex.Lock()
defer mem.mutex.Unlock()
@@ -101,14 +109,31 @@ func (mem *cache) RecordKnownLocation(transport types.ImageTransport, scope type
locationScope[location] = time.Now() // Possibly overwriting an older entry.
}
+// RecordDigestCompressorName records that the blob with the specified digest is either compressed with the specified
+// algorithm, or uncompressed, or that we no longer know.
+func (mem *cache) RecordDigestCompressorName(blobDigest digest.Digest, compressorName string) {
+ mem.mutex.Lock()
+ defer mem.mutex.Unlock()
+ if compressorName == blobinfocache.UnknownCompression {
+ delete(mem.compressors, blobDigest)
+ return
+ }
+ mem.compressors[blobDigest] = compressorName
+}
+
// appendReplacementCandiates creates prioritize.CandidateWithTime values for (transport, scope, digest), and returns the result of appending them to candidates.
func (mem *cache) appendReplacementCandidates(candidates []prioritize.CandidateWithTime, transport types.ImageTransport, scope types.BICTransportScope, digest digest.Digest) []prioritize.CandidateWithTime {
locations := mem.knownLocations[locationKey{transport: transport.Name(), scope: scope, blobDigest: digest}] // nil if not present
for l, t := range locations {
+ compressorName, compressorKnown := mem.compressors[digest]
+ if !compressorKnown {
+ compressorName = blobinfocache.UnknownCompression
+ }
candidates = append(candidates, prioritize.CandidateWithTime{
- Candidate: types.BICReplacementCandidate{
- Digest: digest,
- Location: l,
+ Candidate: blobinfocache.BICReplacementCandidate2{
+ Digest: digest,
+ CompressorName: compressorName,
+ Location: l,
},
LastSeen: t,
})
@@ -123,6 +148,16 @@ func (mem *cache) appendReplacementCandidates(candidates []prioritize.CandidateW
// data from previous RecordDigestUncompressedPair calls is used to also look up variants of the blob which have the same
// uncompressed digest.
func (mem *cache) CandidateLocations(transport types.ImageTransport, scope types.BICTransportScope, primaryDigest digest.Digest, canSubstitute bool) []types.BICReplacementCandidate {
+ return blobinfocache.CandidateLocationsFromV2(mem.CandidateLocations2(transport, scope, primaryDigest, canSubstitute))
+}
+
+// CandidateLocations2 returns a prioritized, limited, number of blobs and their locations that could possibly be reused
+// within the specified (transport scope) (if they still exist, which is not guaranteed).
+//
+// If !canSubstitute, the returned cadidates will match the submitted digest exactly; if canSubstitute,
+// data from previous RecordDigestUncompressedPair calls is used to also look up variants of the blob which have the same
+// uncompressed digest.
+func (mem *cache) CandidateLocations2(transport types.ImageTransport, scope types.BICTransportScope, primaryDigest digest.Digest, canSubstitute bool) []blobinfocache.BICReplacementCandidate2 {
mem.mutex.Lock()
defer mem.mutex.Unlock()
res := []prioritize.CandidateWithTime{}
diff --git a/pkg/blobinfocache/memory/memory_test.go b/pkg/blobinfocache/memory/memory_test.go
index 63040f8908..bc8d5035fb 100644
--- a/pkg/blobinfocache/memory/memory_test.go
+++ b/pkg/blobinfocache/memory/memory_test.go
@@ -3,12 +3,14 @@ package memory
import (
"testing"
+ "github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/pkg/blobinfocache/internal/test"
- "github.com/containers/image/v5/types"
)
-func newTestCache(t *testing.T) (types.BlobInfoCache, func(t *testing.T)) {
- return New(), func(t *testing.T) {}
+var _ blobinfocache.BlobInfoCache2 = &cache{}
+
+func newTestCache(t *testing.T) (blobinfocache.BlobInfoCache2, func(t *testing.T)) {
+ return new2(), func(t *testing.T) {}
}
func TestNew(t *testing.T) {
diff --git a/pkg/blobinfocache/none/none.go b/pkg/blobinfocache/none/none.go
index fa1879afdb..2a54ff312e 100644
--- a/pkg/blobinfocache/none/none.go
+++ b/pkg/blobinfocache/none/none.go
@@ -2,6 +2,7 @@
package none
import (
+ "github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/types"
"github.com/opencontainers/go-digest"
)
@@ -16,7 +17,7 @@ type noCache struct {
// Manifest.Inspect, because configs only have one representation.
// Any use of BlobInfoCache with blobs should usually use at least a
// short-lived cache, ideally blobinfocache.DefaultCache.
-var NoCache types.BlobInfoCache = noCache{}
+var NoCache blobinfocache.BlobInfoCache2 = blobinfocache.FromBlobInfoCache(&noCache{})
// UncompressedDigest returns an uncompressed digest corresponding to anyDigest.
// May return anyDigest if it is known to be uncompressed.
diff --git a/pkg/blobinfocache/none/none_test.go b/pkg/blobinfocache/none/none_test.go
new file mode 100644
index 0000000000..57d20109ee
--- /dev/null
+++ b/pkg/blobinfocache/none/none_test.go
@@ -0,0 +1,7 @@
+package none
+
+import (
+ "github.com/containers/image/v5/types"
+)
+
+var _ types.BlobInfoCache = &noCache{}
diff --git a/types/types.go b/types/types.go
index 3c5126b4e5..a93f2e296c 100644
--- a/types/types.go
+++ b/types/types.go
@@ -194,6 +194,9 @@ type BICReplacementCandidate struct {
//
// None of the methods return an error indication: errors when neither reading from, nor writing to, the cache, should be fatal;
// users of the cache should just fall back to copying the blobs the usual way.
+//
+// The BlobInfoCache interface is deprecated. Consumers of this library should use one of the implementations provided by
+// subpackages of the library's "pkg/blobinfocache" package in preference to implementing the interface on their own.
type BlobInfoCache interface {
// UncompressedDigest returns an uncompressed digest corresponding to anyDigest.
// May return anyDigest if it is known to be uncompressed.