compression: add support for the zstd algorithm#41759
compression: add support for the zstd algorithm#41759
Conversation
391d21c to
357f3b1
Compare
|
@thaJeztah PTAL |
357f3b1 to
34dfd5d
Compare
|
Can we exec C implementation? I assume it is faster than Go. |
wouldn't that be a problem to require the zstd compression tool to be installed? Should the Go version be used as fallback? |
|
Thanks for opening this PR! So, I definitely would like to see support for zstd (it can be a huge space-saver!), I still have my concerns about the current approach in the image-spec. I realise this is not something "new", and not your fault, but something that should be looked at, to prevent being painted into a corner. Also see the more detailed discussion on opencontainers/image-spec#803. I did discuss this (and related) PRs internally (also with our Hub team), and I see that @justincormack has added this topic to Today's OCI call agenda. I may not be able to join that call myself, but looking forward to the outcome of that, hoping that we can move support for |
Same here, I may not be able to join the meeting. Is it possible to keep the discussion asynchronous first? |
|
I think we should move on with this. I (mostly) read through opencontainers/image-spec#803. It doesn't seem to be going anywhere and we shouldn't let it block us. We already support non-gzip blobs, so this is nothing new or breaking in that perspective. It follows the same rules used previously. The only real current issue in opencontainers/image-spec#803 is that mediatype is not in OCI release. But given that Moby implementation doesn't validate mediatypes at all and many other tools already use this, I don't see a chance for any breaking changes there. All the discussion about possible better ways to migrate between compression formats are separate from this and optional. If it involves a schema change, then everything will be broken anyway. If some kind of manifest list extension, then no client implement that currently, and that is a different component. One thing to note is that this same function is used in Dockerfile
I created issue for the same thing in buildkit moby/buildkit#2345 . It looks like there isn't a good package with cgo/no-cgo switch atm(or exec), and the existing cgo packages are not ideal either. I agree with providing the fastest implementation but it doesn't need to be in the first PR. LGTM from me after rebase |
34dfd5d to
ac3feda
Compare
thanks for your review. I've just rebased it and pushed a new version |
ac3feda to
993d8cc
Compare
|
Can we have an integration test to run a zstd image? Just “echo hello” would be fine. |
a249d37 to
71b28ef
Compare
zstd is a compression algorithm that has a very fast decoder, while providing also good compression ratios. The fast decoder makes it suitable for container images, as decompressing the tarballs is a very expensive operation. opencontainers/image-spec#788 added support for zstd to the OCI image specs. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
9606e2a to
e187eb2
Compare
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
discussing with @tonistiigi and @tianon and integration tests for (for example) xz is also missing, so we're ok with looking at integration tests for all of them (with some test images) in a follow up
|
For the follow-up test I pushed |
|
thanks everyone for the review. Is there anything more left before this can be merged? |
Allow to use the zstd compression for docker images now that Docker supports it: moby/moby#41759 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Allow to use the zstd compression for docker images now that Docker supports it: moby/moby#41759 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
zstd is a compression algorithm that has a very fast decoder, while
providing also good compression ratios. The fast decoder makes it
suitable for container images, as decompressing the tarballs is a very
expensive operation.
opencontainers/image-spec#788 added support
for zstd to the OCI image specs.
Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com
- What I did
added read support for layers compressed with zstd
- How I did it
added a new zstd compression format to pkg/archive
- How to verify it
try pulling an image compressed with zstd, e.g.
docker pull gscrivano/zstd-chunked:fedora33- Description for the changelog
support zstd compressed layers
- A picture of a cute animal (not mandatory but encouraged)