Skip to content

mediatypes: support zstd compression#9859

Merged
estesp merged 1 commit intocontainerd:mainfrom
DataDog:jb/zstd-support
Feb 27, 2024
Merged

mediatypes: support zstd compression#9859
estesp merged 1 commit intocontainerd:mainfrom
DataDog:jb/zstd-support

Conversation

@JulienBalestra
Copy link
Copy Markdown
Contributor

This PR enables to support zstd compression over docker media types.

Before this change, containerd is unable to pull zstd compressed images and is returning the following error log:

ATA[0005] pulling image: failed to pull and unpack image "registry.somewhere.io/image:jb-zstd": failed to extract layer sha256:2be6f9ac9ccbcab5c4ea65a6ff0df74002b910d8a52e9147cf1dbc0a38f01azz: failed to get stream processor for application/vnd.docker.image.rootfs.diff.tar.zstd: no processor for media-type: unknown 

@k8s-ci-robot
Copy link
Copy Markdown

Hi @JulienBalestra. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Signed-off-by: Julien Balestra <julien.balestra@gmail.com>
@estesp
Copy link
Copy Markdown
Member

estesp commented Feb 26, 2024

So, what tools are creating images with the docker zstd mediatype? I thought the goal would be to work towards an OCI media type for zstd although I know that discussion has somewhat stalled. It would be good to understand the intention going forward since we already support zstd compression, just not using a unique media type at this point.

@estesp estesp added status/needs-discussion Needs discussion and decision from maintainers and removed needs-ok-to-test labels Feb 26, 2024
@slonopotamus
Copy link
Copy Markdown
Contributor

slonopotamus commented Feb 26, 2024

According to moby/buildkit#3968 and comments in moby/buildkit#2344, buildkit is capable of producing application/vnd.docker.image.rootfs.diff.tar.zstd.

@dmcgowan
Copy link
Copy Markdown
Member

How impactful is this really? It seems years ago BuildKit had an experimental support for zstd with this non-standard media type and then it full added support with the standard OCI media type. Our recommendation here would normally be to re-build anything that was built with an experimental version. Its a small change but the impact still needs to be evaluated. We usually only accept these types of changes with a comment explaining why it is there and the range of impact (which version of the builder and when it was deprecated/removed).

@slonopotamus
Copy link
Copy Markdown
Contributor

then it full added support with the standard OCI media type

You're reading moby/buildkit#3968 the wrong way. It added an ability to unpack application/vnd.docker.image.rootfs.diff.tar.zstd. I believe that current buildkit code as of 0.13.x is still able to both produce and consume it.

@JulienBalestra
Copy link
Copy Markdown
Contributor Author

JulienBalestra commented Feb 27, 2024

Thank you all for your feedback !

So, what tools are creating images with the docker zstd mediatype?

As @slonopotamus mentioned, it's possible to use zstd compression with buildx like:

docker buildx build ... \
--output compression=zstd,force-compression=true,push=true,type=image,compression-level=22

Officially documented here.

@estesp & @dmcgowan you would be more inclined to migrate to OCI media for use cases that require a different compression than gzip ?

@tonistiigi
Copy link
Copy Markdown
Member

Latest versions of buildkit create OCI mediatypes by default but users can set oci-mediatypes=false as output option. If they request both zstd and --oci-mediatypes=false, then the mediatype is application/vnd.docker.image.rootfs.diff.tar.zstd.

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Feb 27, 2024

@JulienBalestra I would say it is our expectation that newer functionality is using the OCI media types, but ultimately the application/vnd.docker. namespace is determined by docker/moby tools. If these are still being produced and supported by the build tools, then we can add it. I didn't want to add support for something that was already deprecated. Thanks for the context @tonistiigi

@dmcgowan dmcgowan removed the status/needs-discussion Needs discussion and decision from maintainers label Feb 27, 2024
@estesp estesp added this pull request to the merge queue Feb 27, 2024
Merged via the queue into containerd:main with commit f0c64a9 Feb 27, 2024
@JulienBalestra JulienBalestra deleted the jb/zstd-support branch February 28, 2024 09:03
@jonjohnsonjr
Copy link
Copy Markdown

Shouldn't this be +zstd not .zstd?

@samuelkarp
Copy link
Copy Markdown
Member

@jonjohnsonjr For the OCI mediatypes, I would expect +zstd. It looks like Buildkit is producing a Docker mediatype with .zstd matching the existing .gzip.

@jonjohnsonjr
Copy link
Copy Markdown

That is really unfortunate.

EricMountain added a commit to DataDog/cilium that referenced this pull request May 27, 2025
buildx is producing `application/vnd.docker.image.rootfs.diff.tar.zstd`
layers which are incompatible with containerd 1.7 as it is lacking
containerd/containerd#9859.

Revert while we look for a solution.
EricMountain added a commit to DataDog/cilium that referenced this pull request May 27, 2025
buildx is producing `application/vnd.docker.image.rootfs.diff.tar.zstd`
layers which are incompatible with containerd 1.7 as it is lacking
containerd/containerd#9859.

Revert while we look for a solution.
@neersighted
Copy link
Copy Markdown
Contributor

This fixed #9263

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants