-
-
Notifications
You must be signed in to change notification settings - Fork 74
Use Zstd compression for published images to reduce their size and speedup decompression #245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
…eedup decompression
Do you know the exact support timeline of this? The reason I ask is since we seem to have users with multiple years old OS installations according to our analytics. That said, we are working on mark those actively as unsupported and stop updating Supervisor etc. (see home-assistant/supervisor#6098). But I rather prefer to not break existing installations. What is the support status for zstd on server side (registries)? |
I believe it is supported since Moby v23.0.0: moby/moby@6014c1e
Registries do not need to have any special support (well, unless the registry explicitly denies uploading things it cannot recognize). From registry POV image layers are just opaque blobs. |
Ok, so any HAOS 10 or newer should be fine then. From opt-in analytics that is 0.9% of users, probably mostly inactive instances. Currently Supervisor advertises 20.10.1 as minimal supported Docker version (see https://github.com/home-assistant/supervisor/blob/2025.08.1/supervisor/docker/manager.py#L43). Now I am not really sure if such an old Docker is out there, and any OS is probably officially considered unsupported. But we should bump that minimal version before publishing our artifacts with zstd, just in the odd chance that there are active instances with such old Docker versions. But I rather prefer to have home-assistant/supervisor#6098 in first, which essentially freezes old systems. Once that is out, we can bump the minimal Docker version in Supervisor. |
Home Assistant OS 10.0 update to Docker 23.0.3, lets make this Docker version the minimum we support. This will allow us to use zstd compression for layers (see home-assistant/builder#245).
|
So with home-assistant/supervisor#6178 we declare Docker 23 to be the minimum. However, we currently don't stop auto-updating Supervisor if Docker is unsupported. This means that there could be instances out there running older than Docker 23 and trying to update to the latest Supervisor. If that fails gracefully I am not really concerned. However, if that leads to a repeated update attempt we are basically load the ghcr.io infrastructure unnecessarily. So I'd like to test how Supervisor behaves in such a case, and if necessary disable auto-updates similar to how we do it with outdated operating systems (see home-assistant/supervisor#6098). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this actually changes a couple of things:
- It builds using
docker-containerdriver instead ofdocker - This has the side effect that an attestation is created and actually preserved: "Provenance attestations are enabled by default, with the mode=min option." (see https://docs.docker.com/build/metadata/attestations/#creating-attestations)
- This causes a container image with an image index (
application/vnd.oci.image.index.v1+json, which comes with platform declarations) - We push images directly using docker buildx (which is also reason why the attestation is actually preserved)
So far the image type was a single image (without index) of application/vnd.docker.distribution.manifest.v2+json. So this essentially switches to OCI as well as a image index.
Now ultimately using OCI metadata definitely make sense, and from a quick test the image index seems to be correct also when cross-building (see details).
Details
{
"schemaVersion": 2,
"mediaType": "application/vnd.oci.image.index.v1+json",
"manifests": [
{
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"size": 3147,
"digest": "sha256:13363584ecf5b39a57f0042413994ccf6574641471877f3ed1c14b3ef45a4883",
"platform": {
"architecture": "arm64",
"os": "linux"
}
},
{
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"size": 566,
"digest": "sha256:28be7311bbb7c376ee732518216412024acdb5238b95b8ae5db7a12aa32ffc11",
"platform": {
"architecture": "unknown",
"os": "unknown"
}
}
]
}But I am a bit scared that this causes some weird side effects. We did had issues in the past with images suddenly declare themselfs of being of a different arch variant (specifically home-assistant/supervisor#5898).
We need to do this transition in smaller steps, otherwise if we hit issues it will be hard to attribute what change was causing it. I'd suggest to move to docker-container driver along with OCI metadata first (lets not use the image index just yet, pass --provenance=false to create an image without image index). Once we have that running we can evaluate switching to zstd.
| function create_buildx_builder() { | ||
| bashio::log.info "Creating buildx builder: ${DOCKER_BUILDX_BUILDER}..." | ||
| if ! docker inspect "${DOCKER_BUILDX_BUILDER}" >/dev/null 2>&1; then | ||
| if ! docker buildx create --name "${DOCKER_BUILDX_BUILDER}" >/dev/null; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I was wondering why this is necessary, and now realized this is required so that the docker-container BuildKit driver is used. Maybe for clarity lets specify this explicitly here (use --driver docker-container).
| fi | ||
| # Sign image (cosign) | ||
| if bashio::var.true "${DOCKER_PUSH}" && bashio::var.true "${COSIGN}"; then | ||
| image_digest=$(docker inspect --format='{{index .RepoDigests 0}}' "${repository}/${image}:${version}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This stops working with:
template parsing error: template: :1:2: executing "" at <index .RepoDigests 0>: error calling index: reflect: slice index out of range
It does make sense, since we load the image directly from the builder, with that Docker does not know the resulting digest in the repository...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this is really a limitation of the graph driver Docker container backend. Using the containerd snapshotter seems to retain the digest
By default the containerd snapshotter is not enabled, but this GitHub Action would allow to enable it:
https://github.com/depot/use-containerd-snapshotter-action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Researching a bit more into it, it seems that containerd snapshotter also supports storing the layers as zstd. From what I understand it will still required buildx to actually create the layer in zstd format, but the import into Docker storage will retain the compression. With that regular docker push can be used to upload zstd compressed layers. It would also make it viable for hassio-addons (refs hassio-addons/workflows#220 (comment)).
| --build-arg "BUILD_ARCH=${build_arch}" \ | ||
| --file "${dockerfile}" \ | ||
| --output "type=image,oci-mediatypes=true,compression=zstd,push=${DOCKER_PUSH}" \ | ||
| --load \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we push BuildKit, is that even necessary? Maybe it is because of cosign, but cosign has another problem, see below.
* Bump minimal Docker to 23.0.0 Home Assistant OS 10.0 update to Docker 23.0.3, lets make this Docker version the minimum we support. This will allow us to use zstd compression for layers (see home-assistant/builder#245). * Bump minimal Docker version to 24.0.0
* Bump minimal Docker to 23.0.0 Home Assistant OS 10.0 update to Docker 23.0.3, lets make this Docker version the minimum we support. This will allow us to use zstd compression for layers (see home-assistant/builder#245). * Bump minimal Docker version to 24.0.0
Zstd is a faster and more efficient compression algorithm than Gzip. This PR enables Zstd for all images created using
home-assistant/builder. Docker supports Zstd images for multiple years already.I am not sure how to test these changes easily because HA GitHub Actions script actively doesn't want to be run in forks.