Remove the no longer necessary BICReplacementCandidate2.CompressorNam…

…e field Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
containers · Mar 25, 2024 · 75a01e9 · 75a01e9
1 parent fbe8c96
commit 75a01e9
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 51 deletions.
diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go
@@ -360,8 +360,8 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context,
 			}
 		}
 		if !candidate.UnknownLocation {
-			if candidate.CompressorName != blobinfocache.Uncompressed {
-				logrus.Debugf("Trying to reuse blob with cached digest %s compressed with %s in destination repo %s", candidate.Digest.String(), candidate.CompressorName, candidateRepo.Name())
+			if candidate.CompressionAlgorithm != nil {
+				logrus.Debugf("Trying to reuse blob with cached digest %s compressed with %s in destination repo %s", candidate.Digest.String(), candidate.CompressionAlgorithm.Name(), candidateRepo.Name())
 			} else {
 				logrus.Debugf("Trying to reuse blob with cached digest %s in destination repo %s", candidate.Digest.String(), candidateRepo.Name())
 			}
@@ -376,8 +376,8 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context,
 				continue
 			}
 		} else {
-			if candidate.CompressorName != blobinfocache.Uncompressed {
-				logrus.Debugf("Trying to reuse blob with cached digest %s compressed with %s with no location match, checking current repo", candidate.Digest.String(), candidate.CompressorName)
+			if candidate.CompressionAlgorithm != nil {
+				logrus.Debugf("Trying to reuse blob with cached digest %s compressed with %s with no location match, checking current repo", candidate.Digest.String(), candidate.CompressionAlgorithm.Name())
 			} else {
 				logrus.Debugf("Trying to reuse blob with cached digest %s in destination repo with no location match, checking current repo", candidate.Digest.String())
 			}

diff --git a/internal/blobinfocache/types.go b/internal/blobinfocache/types.go
@@ -36,8 +36,6 @@ type BlobInfoCache2 interface {
 	// CandidateLocations2 returns a prioritized, limited, number of blobs and their locations (if known)
 	// that could possibly be reused within the specified (transport scope) (if they still
 	// exist, which is not guaranteed).
-	//
-	// The CompressorName fields in returned data must never be UnknownCompression.
 	CandidateLocations2(transport types.ImageTransport, scope types.BICTransportScope, digest digest.Digest, options CandidateLocations2Options) []BICReplacementCandidate2
 }
 
@@ -54,7 +52,6 @@ type CandidateLocations2Options struct {
 // 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
 	CompressionOperation types.LayerCompression      // Either types.Decompress for uncompressed, or types.Compress for compressed
 	CompressionAlgorithm *compressiontypes.Algorithm // An algorithm when the candidate is compressed, or nil when it is uncompressed
 	UnknownLocation      bool                        // is true when `Location` for this blob is not set

diff --git a/pkg/blobinfocache/boltdb/boltdb.go b/pkg/blobinfocache/boltdb/boltdb.go
@@ -331,7 +331,6 @@ func (bdc *cache) appendReplacementCandidates(candidates []prioritize.CandidateW
 			candidates = append(candidates, prioritize.CandidateWithTime{
 				Candidate: blobinfocache.BICReplacementCandidate2{
 					Digest:               digest,
-					CompressorName:       compressorName,
 					CompressionOperation: compressionOp,
 					CompressionAlgorithm: compressionAlgo,
 					Location:             types.BICLocationReference{Opaque: string(k)},
@@ -344,7 +343,6 @@ func (bdc *cache) appendReplacementCandidates(candidates []prioritize.CandidateW
 		candidates = append(candidates, prioritize.CandidateWithTime{
 			Candidate: blobinfocache.BICReplacementCandidate2{
 				Digest:               digest,
-				CompressorName:       compressorName,
 				CompressionOperation: compressionOp,
 				CompressionAlgorithm: compressionAlgo,
 				UnknownLocation:      true,
@@ -359,8 +357,6 @@ func (bdc *cache) appendReplacementCandidates(candidates []prioritize.CandidateW
 // CandidateLocations2 returns a prioritized, limited, number of blobs and their locations (if known)
 // that could possibly be reused within the specified (transport scope) (if they still
 // exist, which is not guaranteed).
-//
-// The CompressorName fields in returned data must never be UnknownCompression.
 func (bdc *cache) CandidateLocations2(transport types.ImageTransport, scope types.BICTransportScope, primaryDigest digest.Digest, options blobinfocache.CandidateLocations2Options) []blobinfocache.BICReplacementCandidate2 {
 	return bdc.candidateLocations(transport, scope, primaryDigest, options.CanSubstitute, &options)
 }

diff --git a/pkg/blobinfocache/internal/prioritize/prioritize_test.go b/pkg/blobinfocache/internal/prioritize/prioritize_test.go
@@ -6,7 +6,7 @@ import (
 	"time"
 
 	"github.com/containers/image/v5/internal/blobinfocache"
-	compressiontypes "github.com/containers/image/v5/pkg/compression/types"
+	"github.com/containers/image/v5/pkg/compression"
 	"github.com/containers/image/v5/types"
 	"github.com/opencontainers/go-digest"
 	"github.com/stretchr/testify/assert"
@@ -23,33 +23,33 @@ const (
 var (
 	// inputReplacementCandidates contains a non-trivial candidateSortState shared among several tests below.
 	inputReplacementCandidates = []CandidateWithTime{
-		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A1"}, CompressorName: compressiontypes.XzAlgorithmName}, time.Unix(1, 0)},
-		{blobinfocache.BICReplacementCandidate2{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U2"}, CompressorName: compressiontypes.GzipAlgorithmName}, 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: compressiontypes.Bzip2AlgorithmName}, time.Unix(1, 1)},
-		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P2"}, CompressorName: compressiontypes.GzipAlgorithmName}, 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)},
-		{blobinfocache.BICReplacementCandidate2{Digest: digestUncompressed, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}, CompressorName: blobinfocache.UnknownCompression}, time.Time{}},
-		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedA, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}, CompressorName: blobinfocache.UnknownCompression}, time.Time{}},
-		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedB, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}, CompressorName: blobinfocache.UnknownCompression}, time.Time{}},
-		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedPrimary, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}, CompressorName: blobinfocache.UnknownCompression}, time.Time{}},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A1"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Xz}, time.Unix(1, 0)},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U2"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Gzip}, time.Unix(1, 1)},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A2"}, CompressionOperation: types.Decompress, CompressionAlgorithm: nil}, time.Unix(1, 1)},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P1"}}, time.Unix(1, 0)},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B1"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Bzip2}, time.Unix(1, 1)},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P2"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Gzip}, time.Unix(1, 1)},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B2"}, CompressionOperation: types.Decompress, CompressionAlgorithm: nil}, time.Unix(2, 0)},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U1"}}, time.Unix(1, 0)},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestUncompressed, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}}, time.Time{}},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedA, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}}, time.Time{}},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedB, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}}, time.Time{}},
+		{blobinfocache.BICReplacementCandidate2{Digest: digestCompressedPrimary, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}}, time.Time{}},
 	}
 	// expectedReplacementCandidates is the fully-sorted, unlimited, result of prioritizing inputReplacementCandidates.
 	expectedReplacementCandidates = []blobinfocache.BICReplacementCandidate2{
-		{Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P2"}, CompressorName: compressiontypes.GzipAlgorithmName},
-		{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: compressiontypes.Bzip2AlgorithmName},
-		{Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A1"}, CompressorName: compressiontypes.XzAlgorithmName},
-		{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U2"}, CompressorName: compressiontypes.GzipAlgorithmName},
-		{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U1"}, CompressorName: blobinfocache.UnknownCompression},
-		{Digest: digestCompressedPrimary, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}, CompressorName: blobinfocache.UnknownCompression},
-		{Digest: digestCompressedA, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}, CompressorName: blobinfocache.UnknownCompression},
-		{Digest: digestCompressedB, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}, CompressorName: blobinfocache.UnknownCompression},
-		{Digest: digestUncompressed, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}, CompressorName: blobinfocache.UnknownCompression},
+		{Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P2"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Gzip},
+		{Digest: digestCompressedPrimary, Location: types.BICLocationReference{Opaque: "P1"}},
+		{Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B2"}, CompressionOperation: types.Decompress, CompressionAlgorithm: nil},
+		{Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A2"}, CompressionOperation: types.Decompress, CompressionAlgorithm: nil},
+		{Digest: digestCompressedB, Location: types.BICLocationReference{Opaque: "B1"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Bzip2},
+		{Digest: digestCompressedA, Location: types.BICLocationReference{Opaque: "A1"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Xz},
+		{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U2"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Gzip},
+		{Digest: digestUncompressed, Location: types.BICLocationReference{Opaque: "U1"}},
+		{Digest: digestCompressedPrimary, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}},
+		{Digest: digestCompressedA, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}},
+		{Digest: digestCompressedB, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}},
+		{Digest: digestUncompressed, UnknownLocation: true, Location: types.BICLocationReference{Opaque: ""}},
 	}
 )
 
@@ -71,8 +71,8 @@ func TestCandidateSortStateLess(t *testing.T) {
 	} {
 		for _, tms := range [][2]int64{{1, 2}, {2, 1}, {1, 1}} {
 			caseName := fmt.Sprintf("%s %v", c.name, tms)
-			c0 := CandidateWithTime{blobinfocache.BICReplacementCandidate2{Digest: c.d0, Location: types.BICLocationReference{Opaque: "L0"}, CompressorName: compressiontypes.GzipAlgorithmName}, time.Unix(tms[0], 0)}
-			c1 := CandidateWithTime{blobinfocache.BICReplacementCandidate2{Digest: c.d1, Location: types.BICLocationReference{Opaque: "L1"}, CompressorName: compressiontypes.ZstdAlgorithmName}, time.Unix(tms[1], 0)}
+			c0 := CandidateWithTime{blobinfocache.BICReplacementCandidate2{Digest: c.d0, Location: types.BICLocationReference{Opaque: "L0"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Gzip}, time.Unix(tms[0], 0)}
+			c1 := CandidateWithTime{blobinfocache.BICReplacementCandidate2{Digest: c.d1, Location: types.BICLocationReference{Opaque: "L1"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Zstd}, time.Unix(tms[1], 0)}
 			css := candidateSortState{
 				primaryDigest:      digestCompressedPrimary,
 				uncompressedDigest: digestUncompressed,
@@ -108,8 +108,8 @@ func TestCandidateSortStateLess(t *testing.T) {
 		{"any: t=1 == t=1, d=A < d=B", -1, p{digestCompressedA, 1}, p{digestCompressedB, 1}},
 		{"any: t=1 == t=1, d=A == d=A", 0, p{digestCompressedA, 1}, p{digestCompressedA, 1}},
 	} {
-		c0 := CandidateWithTime{blobinfocache.BICReplacementCandidate2{Digest: c.p0.d, Location: types.BICLocationReference{Opaque: "L0"}, CompressorName: compressiontypes.GzipAlgorithmName}, time.Unix(c.p0.t, 0)}
-		c1 := CandidateWithTime{blobinfocache.BICReplacementCandidate2{Digest: c.p1.d, Location: types.BICLocationReference{Opaque: "L1"}, CompressorName: compressiontypes.ZstdAlgorithmName}, time.Unix(c.p1.t, 0)}
+		c0 := CandidateWithTime{blobinfocache.BICReplacementCandidate2{Digest: c.p0.d, Location: types.BICLocationReference{Opaque: "L0"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Gzip}, time.Unix(c.p0.t, 0)}
+		c1 := CandidateWithTime{blobinfocache.BICReplacementCandidate2{Digest: c.p1.d, Location: types.BICLocationReference{Opaque: "L1"}, CompressionOperation: types.Compress, CompressionAlgorithm: &compression.Zstd}, time.Unix(c.p1.t, 0)}
 		css := candidateSortState{
 			primaryDigest:      digestCompressedPrimary,
 			uncompressedDigest: digestUncompressed,

diff --git a/pkg/blobinfocache/internal/test/test.go b/pkg/blobinfocache/internal/test/test.go
@@ -171,7 +171,6 @@ func assertCandidatesMatch2(t *testing.T, scopeName string, expected []candidate
 		}
 		e[i] = blobinfocache.BICReplacementCandidate2{
 			Digest:               ev.d,
-			CompressorName:       ev.cn,
 			CompressionOperation: op,
 			CompressionAlgorithm: algo,
 			UnknownLocation:      false,
@@ -298,7 +297,6 @@ func testGenericCandidateLocations2(t *testing.T, cache blobinfocache.BlobInfoCa
 		assertCandidatesMatch2Native(t, []blobinfocache.BICReplacementCandidate2{
 			{
 				Digest:               digestUnknownLocation,
-				CompressorName:       compressiontypes.Bzip2AlgorithmName,
 				CompressionOperation: types.Compress,
 				CompressionAlgorithm: &compression.Bzip2,
 				UnknownLocation:      true,
@@ -313,7 +311,6 @@ func testGenericCandidateLocations2(t *testing.T, cache blobinfocache.BlobInfoCa
 		assertCandidatesMatch2Native(t, []blobinfocache.BICReplacementCandidate2{
 			{
 				Digest:               digestUnknownLocation,
-				CompressorName:       compressiontypes.Bzip2AlgorithmName,
 				CompressionOperation: types.Compress,
 				CompressionAlgorithm: &compression.Bzip2,
 				UnknownLocation:      false,

diff --git a/pkg/blobinfocache/memory/memory.go b/pkg/blobinfocache/memory/memory.go
@@ -154,7 +154,6 @@ func (mem *cache) appendReplacementCandidates(candidates []prioritize.CandidateW
 			candidates = append(candidates, prioritize.CandidateWithTime{
 				Candidate: blobinfocache.BICReplacementCandidate2{
 					Digest:               digest,
-					CompressorName:       compressorName,
 					CompressionOperation: compressionOp,
 					CompressionAlgorithm: compressionAlgo,
 					Location:             l,
@@ -166,7 +165,6 @@ func (mem *cache) appendReplacementCandidates(candidates []prioritize.CandidateW
 		candidates = append(candidates, prioritize.CandidateWithTime{
 			Candidate: blobinfocache.BICReplacementCandidate2{
 				Digest:               digest,
-				CompressorName:       compressorName,
 				CompressionOperation: compressionOp,
 				CompressionAlgorithm: compressionAlgo,
 				UnknownLocation:      true,
@@ -191,8 +189,6 @@ func (mem *cache) CandidateLocations(transport types.ImageTransport, scope types
 // CandidateLocations2 returns a prioritized, limited, number of blobs and their locations (if known)
 // that could possibly be reused within the specified (transport scope) (if they still
 // exist, which is not guaranteed).
-//
-// The CompressorName fields in returned data must never be UnknownCompression.
 func (mem *cache) CandidateLocations2(transport types.ImageTransport, scope types.BICTransportScope, primaryDigest digest.Digest, options blobinfocache.CandidateLocations2Options) []blobinfocache.BICReplacementCandidate2 {
 	return mem.candidateLocations(transport, scope, primaryDigest, options.CanSubstitute, &options)
 }

diff --git a/pkg/blobinfocache/sqlite/sqlite.go b/pkg/blobinfocache/sqlite/sqlite.go
@@ -466,7 +466,6 @@ func (sqc *cache) appendReplacementCandidates(candidates []prioritize.CandidateW
 		candidates = append(candidates, prioritize.CandidateWithTime{
 			Candidate: blobinfocache.BICReplacementCandidate2{
 				Digest:               digest,
-				CompressorName:       compressorName,
 				CompressionOperation: compressionOp,
 				CompressionAlgorithm: compressionAlgo,
 				Location:             types.BICLocationReference{Opaque: location},
@@ -483,7 +482,6 @@ func (sqc *cache) appendReplacementCandidates(candidates []prioritize.CandidateW
 		candidates = append(candidates, prioritize.CandidateWithTime{
 			Candidate: blobinfocache.BICReplacementCandidate2{
 				Digest:               digest,
-				CompressorName:       compressorName,
 				CompressionOperation: compressionOp,
 				CompressionAlgorithm: compressionAlgo,
 				UnknownLocation:      true,
@@ -498,8 +496,6 @@ func (sqc *cache) appendReplacementCandidates(candidates []prioritize.CandidateW
 // CandidateLocations2 returns a prioritized, limited, number of blobs and their locations (if known)
 // that could possibly be reused within the specified (transport scope) (if they still
 // exist, which is not guaranteed).
-//
-// The CompressorName fields in returned data must never be UnknownCompression.
 func (sqc *cache) CandidateLocations2(transport types.ImageTransport, scope types.BICTransportScope, digest digest.Digest, options blobinfocache.CandidateLocations2Options) []blobinfocache.BICReplacementCandidate2 {
 	return sqc.candidateLocations(transport, scope, digest, options.CanSubstitute, &options)
 }