Skip to content

Commit

Permalink
copy: implement instanceCopyClone for zstd compression
Browse files Browse the repository at this point in the history 
* copy.Options now contains a new field `EnsureCompressionVariantExists
  map[string]int` which allows users to specify if they want to clone
images with specified `compression`.

* copyMultipleImages now implements `instanceCopyClone` and extends
  function specifically for `zstd` compression.

Signed-off-by: Aditya R <[email protected]>
  • Loading branch information
@flouthoc
flouthoc committed Jul 13, 2023
1 parent eedacf8 commit e96cf0f
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 27 deletions.
14 changes: 12 additions & 2 deletions copy/copy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/manifest"
"github.com/containers/image/v5/pkg/blobinfocache"
compression "github.com/containers/image/v5/pkg/compression/types"
"github.com/containers/image/v5/signature"
"github.com/containers/image/v5/signature/signer"
"github.com/containers/image/v5/transports"
Expand All @@ -38,6 +39,11 @@ var (
maxParallelDownloads = uint(6)
)

type CopyOptionCompressionVariant struct {
Algorithm *compression.Algorithm
Level int
}

const (
// CopySystemImage is the default value which, when set in
// Options.ImageListSelection, indicates that the caller expects only one
Expand Down Expand Up @@ -126,6 +132,10 @@ type Options struct {
// Download layer contents with "nondistributable" media types ("foreign" layers) and translate the layer media type
// to not indicate "nondistributable".
DownloadForeignLayers bool

// Contains a map of compression algorithm and compression level, where c/image will ensure that selected
// images in the manifest list will be replicated with provided compression algorithms.
EnsureCompressionVariantExists []CopyOptionCompressionVariant
}

// copier allows us to keep track of diffID values for blobs, and other
Expand Down Expand Up @@ -257,7 +267,7 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,

if !multiImage {
// The simple case: just copy a single image.
if copiedManifest, _, _, err = c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedToplevel, nil); err != nil {
if copiedManifest, _, _, err = c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedToplevel, nil, nil); err != nil {
return nil, err
}
} else if options.ImageListSelection == CopySystemImage {
Expand All @@ -278,7 +288,7 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,
logrus.Debugf("Source is a manifest list; copying (only) instance %s for current system", instanceDigest)
unparsedInstance := image.UnparsedInstance(rawSource, &instanceDigest)

if copiedManifest, _, _, err = c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedInstance, nil); err != nil {
if copiedManifest, _, _, err = c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedInstance, nil, nil); err != nil {
return nil, fmt.Errorf("copying system image from manifest list: %w", err)
}
} else { /* options.ImageListSelection == CopyAllImages or options.ImageListSelection == CopySpecificImages, */
Expand Down
68 changes: 58 additions & 10 deletions copy/multiple.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"github.com/containers/image/v5/internal/image"
internalManifest "github.com/containers/image/v5/internal/manifest"
"github.com/containers/image/v5/manifest"
"github.com/containers/image/v5/pkg/compression"
compressionTypes "github.com/containers/image/v5/pkg/compression/types"
"github.com/containers/image/v5/signature"
digest "github.com/opencontainers/go-digest"
imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
Expand All @@ -26,25 +28,42 @@ const (
)

type instanceCopy struct {
op instanceCopyKind
sourceDigest digest.Digest
op instanceCopyKind
sourceDigest digest.Digest
compressionVariant CopyOptionCompressionVariant
}

// prepareInstanceCopies prepares a list of instances which needs to copied to the manifest list.
func prepareInstanceCopies(instanceDigests []digest.Digest, options *Options) []instanceCopy {
func prepareInstanceCopies(instanceDigests []digest.Digest, options *Options) ([]instanceCopy, error) {
res := []instanceCopy{}
for i, instanceDigest := range instanceDigests {
if options.ImageListSelection == CopySpecificImages &&
!slices.Contains(options.Instances, instanceDigest) {
logrus.Debugf("Skipping instance %s (%d/%d)", instanceDigest, i+1, len(instanceDigests))
continue
}
for _, compressionVariant := range options.EnsureCompressionVariantExists {
switch compressionVariant.Algorithm.Name() {
case compressionTypes.ZstdAlgorithmName:
res = append(res, instanceCopy{
op: instanceCopyClone,
sourceDigest: instanceDigest,
compressionVariant: compressionVariant,
})
case compressionTypes.GzipAlgorithmName:
continue
default:
return res, fmt.Errorf("unsupported compression algorithm for instance %s: %s", instanceDigest, compressionVariant.Algorithm.Name())
}
}
defaultCompressionVariant := CopyOptionCompressionVariant{Algorithm: &compression.Gzip, Level: -1}
res = append(res, instanceCopy{
op: instanceCopyCopy,
sourceDigest: instanceDigest,
op: instanceCopyCopy,
sourceDigest: instanceDigest,
compressionVariant: defaultCompressionVariant,
})
}
return res
return res, nil
}

// copyMultipleImages copies some or all of an image list's instances, using
Expand Down Expand Up @@ -119,7 +138,10 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
// Copy each image, or just the ones we want to copy, in turn.
instanceDigests := updatedList.Instances()
instanceEdits := []internalManifest.ListEdit{}
instanceCopyList := prepareInstanceCopies(instanceDigests, options)
instanceCopyList, err := prepareInstanceCopies(instanceDigests, options)
if err != nil {
return nil, fmt.Errorf("while preparing instances for copy: %w", err)
}
c.Printf("Copying %d of %d images in list\n", len(instanceCopyList), len(instanceDigests))
for i, instance := range instanceCopyList {
// Update instances to be edited by their `ListOperation` and
Expand All @@ -129,7 +151,7 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
logrus.Debugf("Copying instance %s (%d/%d)", instance.sourceDigest, i+1, len(instanceCopyList))
c.Printf("Copying image %s (%d/%d)\n", instance.sourceDigest, i+1, len(instanceCopyList))
unparsedInstance := image.UnparsedInstance(c.rawSource, &instanceCopyList[i].sourceDigest)
updatedManifest, updatedManifestType, updatedManifestDigest, err := c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedInstance, &instanceCopyList[i].sourceDigest)
updatedManifest, updatedManifestType, updatedManifestDigest, err := c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedInstance, &instanceCopyList[i].sourceDigest, instance.compressionVariant.Algorithm)
if err != nil {
return nil, fmt.Errorf("copying image %d/%d from manifest list: %w", i+1, len(instanceCopyList), err)
}
Expand All @@ -140,8 +162,34 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
UpdateDigest: updatedManifestDigest,
UpdateSize: int64(len(updatedManifest)),
UpdateMediaType: updatedManifestType})
default:
return nil, fmt.Errorf("copying image: invalid copy operation %d", instance.op)
case instanceCopyClone:
c.Printf("Replicating image %s (%d/%d)\n", instance.sourceDigest, i+1, len(instanceCopyList))
unparsedInstance := image.UnparsedInstance(c.rawSource, &instanceCopyList[i].sourceDigest)
instanceDetails, err := updatedList.Instance(instanceCopyList[i].sourceDigest)
if err != nil {
return nil, fmt.Errorf("Replicating image %d/%d from manifest list: %w", i+1, len(instanceCopyList), err)
}
addCompressionAlgorithms := []compressionTypes.Algorithm{}
optionsClone := options
if instance.compressionVariant.Algorithm != nil {
// copy options with new compression format
optionsClone.DestinationCtx.CompressionFormat = instance.compressionVariant.Algorithm
optionsClone.DestinationCtx.CompressionLevel = &instance.compressionVariant.Level
addCompressionAlgorithms = append(addCompressionAlgorithms, *instance.compressionVariant.Algorithm)
}
addManifest, addManifestType, addManifestDigest, err := c.copySingleImage(ctx, policyContext, optionsClone, unparsedToplevel, unparsedInstance, &instanceCopyList[i].sourceDigest, instance.compressionVariant.Algorithm)
if err != nil {
return nil, fmt.Errorf("Replicating image %d/%d from manifest list: %w", i+1, len(instanceCopyList), err)
}
// Record the result of a possible conversion here.
instanceEdits = append(instanceEdits, internalManifest.ListEdit{
ListOperation: internalManifest.ListOpAdd,
AddDigest: addManifestDigest,
AddSize: int64(len(addManifest)),
AddMediaType: addManifestType,
AddPlatform: instanceDetails.Platform,
AddCompressionAlgorithms: addCompressionAlgorithms,
})
}
}

Expand Down
13 changes: 8 additions & 5 deletions copy/multiple_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package copy
import (
"testing"

"github.com/containers/image/v5/pkg/compression"
digest "github.com/opencontainers/go-digest"
"github.com/stretchr/testify/assert"
)
Expand All @@ -15,22 +16,24 @@ func TestPrepareCopyInstances(t *testing.T) {
digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"),
}

instancesToCopy := prepareInstanceCopies(sourceInstances, &Options{})
instancesToCopy, _ := prepareInstanceCopies(sourceInstances, &Options{})
compare := []instanceCopy{}
defaultCompressionVariant := CopyOptionCompressionVariant{Algorithm: &compression.Gzip, Level: -1}
for _, instance := range sourceInstances {
compare = append(compare, instanceCopy{op: instanceCopyCopy,
sourceDigest: instance})
sourceDigest: instance, compressionVariant: defaultCompressionVariant})
}
assert.Equal(t, instancesToCopy, compare)

// Test with CopySpecific Images
copyOnly := []digest.Digest{
digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
}
instancesToCopy = prepareInstanceCopies(sourceInstances, &Options{
instancesToCopy, _ = prepareInstanceCopies(sourceInstances, &Options{
Instances: copyOnly,
ImageListSelection: CopySpecificImages})
assert.Equal(t, instancesToCopy, []instanceCopy{{
op: instanceCopyCopy,
sourceDigest: copyOnly[0]}})
op: instanceCopyCopy,
sourceDigest: copyOnly[0],
compressionVariant: defaultCompressionVariant}})
}
22 changes: 12 additions & 10 deletions copy/single.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type imageCopier struct {

// copySingleImage copies a single (non-manifest-list) image unparsedImage, using policyContext to validate
// source image admissibility.
func (c *copier) copySingleImage(ctx context.Context, policyContext *signature.PolicyContext, options *Options, unparsedToplevel, unparsedImage *image.UnparsedImage, targetInstance *digest.Digest) (retManifest []byte, retManifestType string, retManifestDigest digest.Digest, retErr error) {
func (c *copier) copySingleImage(ctx context.Context, policyContext *signature.PolicyContext, options *Options, unparsedToplevel, unparsedImage *image.UnparsedImage, targetInstance *digest.Digest, requiredCompression *compression.Algorithm) (retManifest []byte, retManifestType string, retManifestDigest digest.Digest, retErr error) {
// The caller is handling manifest lists; this could happen only if a manifest list contains a manifest list.
// Make sure we fail cleanly in such cases.
multiImage, err := isMultiImage(ctx, unparsedImage)
Expand Down Expand Up @@ -188,7 +188,7 @@ func (c *copier) copySingleImage(ctx context.Context, policyContext *signature.P
}
}

if err := ic.copyLayers(ctx); err != nil {
if err := ic.copyLayers(ctx, requiredCompression); err != nil {
return nil, "", "", err
}

Expand Down Expand Up @@ -359,7 +359,7 @@ func compareImageDestinationManifestEqual(ctx context.Context, options *Options,
}

// copyLayers copies layers from ic.src/ic.c.rawSource to dest, using and updating ic.manifestUpdates if necessary and ic.cannotModifyManifestReason == "".
func (ic *imageCopier) copyLayers(ctx context.Context) error {
func (ic *imageCopier) copyLayers(ctx context.Context, requiredCompression *compression.Algorithm) error {
srcInfos := ic.src.LayerInfos()
numLayers := len(srcInfos)
updatedSrcInfos, err := ic.src.LayerInfosForCopy(ctx)
Expand Down Expand Up @@ -408,7 +408,7 @@ func (ic *imageCopier) copyLayers(ctx context.Context) error {
logrus.Debugf("Skipping foreign layer %q copy to %s", cld.destInfo.Digest, ic.c.dest.Reference().Transport().Name())
}
} else {
cld.destInfo, cld.diffID, cld.err = ic.copyLayer(ctx, srcLayer, toEncrypt, pool, index, srcRef, manifestLayerInfos[index].EmptyLayer)
cld.destInfo, cld.diffID, cld.err = ic.copyLayer(ctx, srcLayer, toEncrypt, pool, index, srcRef, manifestLayerInfos[index].EmptyLayer, requiredCompression)
}
data[index] = cld
}
Expand Down Expand Up @@ -581,7 +581,7 @@ type diffIDResult struct {
// copyLayer copies a layer with srcInfo (with known Digest and Annotations and possibly known Size) in src to dest, perhaps (de/re/)compressing it,
// and returns a complete blobInfo of the copied layer, and a value for LayerDiffIDs if diffIDIsNeeded
// srcRef can be used as an additional hint to the destination during checking whether a layer can be reused but srcRef can be nil.
func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, toEncrypt bool, pool *mpb.Progress, layerIndex int, srcRef reference.Named, emptyLayer bool) (types.BlobInfo, digest.Digest, error) {
func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, toEncrypt bool, pool *mpb.Progress, layerIndex int, srcRef reference.Named, emptyLayer bool, requiredCompression *compression.Algorithm) (types.BlobInfo, digest.Digest, error) {
// If the srcInfo doesn't contain compression information, try to compute it from the
// MediaType, which was either read from a manifest by way of LayerInfos() or constructed
// by LayerInfosForCopy(), if it was supplied at all. If we succeed in copying the blob,
Expand Down Expand Up @@ -625,11 +625,13 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to
// Fixing that will probably require passing more information to TryReusingBlob() than the current version of
// the ImageDestination interface lets us pass in.
reused, reusedBlob, err := ic.c.dest.TryReusingBlobWithOptions(ctx, srcInfo, private.TryReusingBlobOptions{
Cache: ic.c.blobInfoCache,
CanSubstitute: canSubstitute,
EmptyLayer: emptyLayer,
LayerIndex: &layerIndex,
SrcRef: srcRef,
Cache: ic.c.blobInfoCache,
CanSubstitute: canSubstitute,
EmptyLayer: emptyLayer,
LayerIndex: &layerIndex,
SrcRef: srcRef,
RequiredCompression: requiredCompression,
OriginalCompression: srcInfo.CompressionAlgorithm,
})
if err != nil {
return types.BlobInfo{}, "", fmt.Errorf("trying to reuse blob %s at destination: %w", srcInfo.Digest, err)
Expand Down

0 comments on commit e96cf0f

Please sign in to comment.