docker: allow uncompressed layers

[tych0]

We're interested in sending uncompressed layers to a docker registry, so
let's add a flag to support this.

Signed-off-by: Tycho Andersen [email protected]

[mtrmac]

Thanks! Just a few stylistic nits of the code.

I’m curious, though: Why send uncompressed layers to the registry? Is it something fairly specific to docker://? We would now be at 4 / 7 non-obsolete writable transports with an option for DesiredLayerCompression, so I’m a bit wondering whether this should be added to copy.Options instead.

[tych0]

Why send uncompressed layers to the registry?

It's not really about uncompressed, they're just not gzip compressed.

Many registries besides the actual docker registry are mostly supported by docker://, for example zot. Since #819 seems stalled, we're migrating towards using docker:// to copy to zot, so that we can move back to upstream skopeo and containers/image. We need this to put non-gzipped layers into the repo, though.

[mtrmac]

Why send uncompressed layers to the registry?

It's not really about uncompressed, they're just not gzip compressed.

We need this to put non-gzipped layers into the repo, though.

Hum, that’s a fairly indirect way to go about it. Why not add support for that format to pkg/compression instead? Even if it required adding a concept of a “recognized but unimplemented” algorithm, which ends up setting canModifyBlob in copyBlobFromStream to false.

And then the caller would not have to set any options.

[tych0]

Hum, that’s a fairly indirect way to go about it. Why not add support for that format to pkg/compression instead?

Because it's not really a compression format. It's a squashfs blob, which has metadata and stuff compressed, but is its own file format that is not a general purpose compression algorithm.

[mtrmac]

That feels like it should lead to a conversation around OCI non-image artifacts, or maybe about the copy code not just ignoring layer MIME types in manifests.

[tych0]

That feels like it should lead to a conversation around OCI non-image artifacts

Yes, I've been involved in several of those recently :)

maybe about the copy code not just ignoring layer MIME types in manifests.

Yes, that would also be good. I've long thought it was impolite that this ostensibly transport code decides to compress stuff sometimes.

[tych0]

Ping, any word on this?

[mtrmac]

I’m not much a fan of adding an option you don’t actually need for a side effect that should be achieved directly.

E.g. we already have the infrastructure for knowing what MIME types can/can’t be compressed in compressionVariantMIMEType.

[tych0]

I’m not much a fan of adding an option you don’t actually need

We do need to accomplish this somehow.

E.g. we already have the infrastructure for knowing what MIME types can/can’t be compressed in compressionVariantMIMEType.

I can send a patch to add our custom squashfs mime type to that list if you want.

But really, I don't think this should be that uncontroversial. It is also reasonable to want to send uncompressed tar blobs to a repository, in case you don't want to spend the time decompressing them at the other end. Maybe hoisting all these --dont-default-compress options to copy.CopyOptions() instead?

[mtrmac]

I’m not much a fan of adding an option you don’t actually need

We do need to accomplish this somehow.

There are some limits to what c/image cares about. I think we will be figuring out what those are during the coming year, as OCI artifacts take shape, or not.

Hypothetically, a compliant OCI artifact would be seriously considered; a Docker schema1 (obsolete format) image that contained SquashFS blobs instead of layers usable by any standard schema1 consumer would not be something I’m personally terribly interested in — if it can be supported by making the code for the formats as defined better, sure, but it would still not be something our test suites would ever cover (i.e. it could break any time), and adding very non-trivial special cases to support such non-images is not very attractive.

E.g. we already have the infrastructure for knowing what MIME types can/can’t be compressed in compressionVariantMIMEType.

I can send a patch to add our custom squashfs mime type to that list if you want.

Right now, that table does not actually drive the compress/decompress decision. But if it did (via an addition to types.Image), the fact that the SquashFS MIME type is not recognized could automatically prevent copyBlobFromStream from modifying it.

That would require “extending” types.Image without actually adding to the interface (and breaking possible external callers), but that’s something that several proposed PRs need — so we need to build that mechanism/design anyway.

But really, I don't think this should be that uncontroversial. It is also reasonable to want to send uncompressed tar blobs to a repository, in case you don't want to spend the time decompressing them at the other end.

(Anecdotally, the bottleneck tends to be the network bandwidth, the CPU cost of decompression is usually trivial compared to that.)

Maybe hoisting all these --dont-default-compress options to copy.CopyOptions() instead?

Maybe. The copy pipeline is complex enough that I’d rather not add options that are not necessary, especially if they don’t make it all that much more likely that the specialized format you are using will continue to work; the project will pay the price of continuously maintaining a feature, but not the feature you care about.

[tych0]

On Mon, Jun 08, 2020 at 07:53:14AM -0700, Miloslav Trmač wrote: > > I’m not much a fan of adding an option you don’t actually need > > We do need to accomplish this somehow. There are _some_ limits to what c/image cares about. I think we will be figuring out what those are during the coming year, as OCI artifacts take shape, or not. Hypothetically, a compliant OCI artifact would be seriously considered

Arbitrary mime types in OCI are supported; see cd2fbca for example.

; a Docker schema1 (obsolete format) image that contained SquashFS blobs instead of layers usable by any standard schema1 consumer would not be something I’m personally terribly interested in — if it can be supported by making the code for the formats as defined _better_, sure, but it would still not be something our test suites would ever cover (i.e. it could break any time), and adding very non-trivial special cases to support such non-images is not very attractive.

I don't think this is a "very non-trivial" patch. Indeed, it looks quite small. Plus, there are analogues as you mention in several other formats.

> > E.g. we already have the infrastructure for knowing what MIME types can/can’t be compressed in compressionVariantMIMEType. > > I can send a patch to add our custom squashfs mime type to that list if you want. Right now, that table does not actually drive the compress/decompress decision. But if it did (via an addition to `types.Image`), the fact that the SquashFS MIME type is not recognized could automatically prevent `copyBlobFromStream` from modifying it. That would require “extending” `types.Image` without actually adding to the interface (and breaking possible external callers), but that’s something that several proposed PRs need — so we need to build that mechanism/design anyway. > But really, I don't think this should be that uncontroversial. It is also reasonable to want to send uncompressed tar blobs to a repository, in case you don't want to spend the time decompressing them at the other end. (Anecdotally, the bottleneck tends to be the network bandwidth, the CPU cost of decompression is usually trivial compared to that.)

Our experience on a corporate intranet with a local repo is the inverse. There are many configurations in the world :)

> Maybe hoisting all these --dont-default-compress options to copy.CopyOptions() instead? Maybe. The copy pipeline is complex enough that I’d rather not add options that are not necessary, especially if they don’t make it all that much more likely that the specialized format you are using will _continue to_ work; the project will pay the price of continuously maintaining a feature, but not the feature you care about.

Ok, so are you saying "WONTFIX"?

[mtrmac]

I do think it would be useful to automatically not compress/decompress blobs with unknown MIME types; forcing the user to explicitly set a “don’t break my image” option is not as good an API.

[tych0]

Yes, agreed :). I'll see if I can find a nice way to do that.