Split up library builds into individual builder stages to preserve layer cache

[lightswitch05]

Why

Hello! I've noticed the docker image is quite hefty, and I was curious if I could improve it a bit. After taking a look, I realized I couldn't help much with reducing the image size 😆 .... but I thought it might be possible to improve layer reuse between builds. In the end, this feature branch is only 7.98mb smaller then whats on master, but I believe layer reuse is now a possibility depending on how the builds and caching are set up.

If no one thinks this PR provides any value, that’s no problem! It does introduce a bit more complexity, so I totally understand. Anyway, on to the changes I made:

What

I've moved each major build phase into its own builder stage using multi-stage builds: ssocr, pip, libcec, PicoTTS, and Telldus. The results of those builder stages are then copied out into the 'main' stage. All temporary files were already being pruned nicely, so again, no real space savings. However, using the COPY --link command from the builder stages enables this cool docker feature:

Use --link to reuse already built layers in subsequent builds with --cache-from even if the previous layers have changed. This is especially important for multi-stage builds where a COPY --from statement would previously get invalidated if any previous commands in the same stage changed, causing the need to rebuild the intermediate stages again. With --link the layer the previous build generated is reused and merged on top of the new layers. This also means you can easily rebase your images when the base images receive updates, without having to execute the whole build again.

So, if you need to bump a version in requirements.txt, using COPY --link will allow those other layer - like ssocr - to remain unchanged. Pretty cool! If this PR works as expected, I hope that the next time I run docker compose pull, it will require fewer layers to be pulled.

Now... there is a bit of a gotcha with all this. This caching logic only works if the builds are correctly set up with caching. For example, docker-compose builds cannot create a multi-stage build cache. Looking around, I see buildx is being used over at home-assistant/builder/, but there was a lot of logic going on, and I couldn't quite follow it all.

So, there's a chance some follow-up changes might be needed before the benefits of this PR can be realized - for example, using cache-to and ensuring mode=max is set to enable the mutli-stage build cache. But one step at a time - if you all think this is an improvement worth making, we can iterate from here.

Testing

For testing, I ran the build and verified that it runs. However, that doesn’t fully confirm that the libraries I modified are still being installed correctly. Some follow-up work is definitely required to verify everything is functioning as expected.

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[lightswitch05]

Also, I think that for reducing the layer count even further, we could do something like this as the last step, instead of COPYing the individual parts [...]. Indeed, you won't leverage the --link feature then, but IMO, since it happens by the end of the build, there's no disadvantage of cache invalidation, and the copy commands are not that much time consuming. Or the builds can be combined in another FROM scratch as merged-libs builder and then copied to the final image with COPY --link --from=merged-libs / /. What do you think?

@sairon I think the pros/cons here come down to implementation details later on in the build pipeline. Its been a while since I looked at all this 😆... but I think I remember that the main build pipeline re-triggers this base image build on each release? If so, I think reducing layers and/or dropping the --link feature does not actually help much with the final result - as the entire base image still has to be pulled on each update - which also invalidates any layers that are build FROM this base image. I believe, using the --link feature will allow certain layers of this base image to survive the re-build. So, while you might get a brand new docker base image on each release, the libcec layer - which had no changes - could be reused, and would not need to be re-pulled on update.

Anyways, that is how I was thinking all this could work. Its hard to say for sure, there are a lot of moving parts downstream of this repo. If you still want to reduce the image layers and/or remove -link then I'm happy to make those changes too.

[lightswitch05]

Oh, and just another note, besides validating the build works, I have no idea how to actually test these changes. For example, how to validate picotts is still being installed correctly and is usable in the downstream builds.

[sairon]

So, while you might get a brand new docker base image on each release, the libcec layer - which had no changes - could be reused, and would not need to be re-pulled on update.

Being nice in theory, I don't think this would work given how the images are built now. The BuildKit cache is not preserved between builds by the builder currently, so the layer hashes will be different between each run of the base images' build. In such case it makes more sense to me to squash it to a single layer to reduce the number of layers the resulting HA image has.

But we don't necessarily need to do it here, for now the PR is good as is. I'll do a full local build of Core based on this image and then ask at least for another pair of eyes to have a look at this.

[lightswitch05]

Being nice in theory, I don't think this would work given how the images are built now.

Yeah, I totally agree, it will require followup work on the downstream repo. The way I've done this elsewhere with buildx is to use --cache-from type=registry,ref=xxx with a cache-specific tag like cache-master. On official releases you would want to include --cache-to type=registry,mode=max,ref=xxx to update the cache. Back when I did this work, I had a glace at how the other build was working to determine how much extra effort it would be to add that cache logic... it seemed non-trivial. The way all platform images are built individually and then grouped together into a single multi-platform manifest makes it a bit more complicated. To handle cache correctly, I imagine it will need a platform specific cache, like cache-master-aarch64, etc.

From a pure docker layer efficiency point-of-view, it might be prudent to ask: Does it make sense to continue supporting a 'base image'? If caching is optimized enough, would it be acceptable to have the full image requirements built out in a single Dockerfile? From a local-dev point-of-view, I can see how having a base image is useful, but if cache is working well, maybe it wouldn't make a big difference? It would also allow for more streamlined development if someone needs to make changes to these "base image" dependencies - all being in the same Dockerfile. From a release image point of view - since these base images are always re-building, I don't think there is any benefit keeping them separate. I would be happy to help with PRs if this sounds like a good option.

[agners]

If no one thinks this PR provides any value, that’s no problem! It does introduce a bit more complexity, so I totally understand. Anyway, on to the changes I made:

There hasn't been much maintenance on the base image, I am not even sure if these libraries are used. That said, I think your PR does things cleaner, and is definitely a step forward in case we want/need to keep some of these dependencies, so I am happy to merge this.

From a pure docker layer efficiency point-of-view, it might be prudent to ask: Does it make sense to continue supporting a 'base image'? If caching is optimized enough, would it be acceptable to have the full image requirements built out in a single Dockerfile?

It probably makes sense to fold this into the Dockerfile in the Core repository indeed. But I think it makes sense to first check what is necessary here, and maybe drop dependencies or replace them with existing Alpine packages.

[lightswitch05]

I can't believe this got merged in after so many months! 🥳

[joneshf]

👋 Hey folks! This change seems to have broken the Pico TTS integration. There's an issue in the core repository about it: https://github.com/home-assistant/core/issues/155148, but the long and short is that it seems pico2wave doesn't like to be built in one location and then moved. Possibly because of this line: https://github.com/ihuguet/picotts/blob/21089d223e177ba3cb7e385db8613a093dff74b5/pico/Makefile.am#L95

Seems like adding --prefix=/opt/picotts is causing pico2wave to look for stuff in /opt/picotts despite all of its data being in /usr/local in the final stage:

homeassistant:/config# strings $(which pico2wave) | grep picotts
/opt/picotts/lib
/opt/picotts/share/pico/lang/

Is the change to building in /opt/picotts material to this change? Or could the picotts stuff still be built without specifying a different prefix in the picotts-builder stage, and still copied over to the final stage? Maybe something like this would work?:

# Copy from picotts builder
COPY --link --from=picotts-builder --exclude=/usr/src/pico /usr/ /usr/

I dunno, I'm just guessing there.

[lightswitch05]

I'm going to try and put together a PR to fix this tonight.