Open
Conversation
Signed-off-by: Timo Reichl <thetredev@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I just finished watching your video about making your homelab open source and thought I'd look around to see if there's something that might need improvement. While searching I stumbled upon this repository and thought I'd help improve the
Dockerfilea bit.This PR improves the docker build process. This is accomplished with two steps:
temp-files, no temporary files will be present in the built imageCustomize Container Hereare drastically reduced to 1RUNlayerThe first point takes advantage of Docker BuildKit (https://docs.docker.com/build/buildkit/) and its ability for multi-stage builds. With the temporary stage, all config files are put in place in under a new directory
/copy. Normally we'd use ascratchimage for that (which contains no files), but since the DockerfileADDinstruction doesn't support extractingzipfiles (yet), I needed a small image which hasunzipinstalled, So I ended up withbusybox. Since the target stage fordocker build/docker buildxalways defaults to theFROMinstruction without anyASinstruction, thetemp-filesstage will be discarded after the build is finished, and is not part of the final image.For this change to work, you might need the env var
DOCKER_BUILDKIT=1for CI to work though. But that's generally a good idea anyway.While the second point isn't necessary in all use cases, it is best practice to reduce the amount of image layers when possible and when it makes sense. In this case, I think it makes sense to only have 1 layer overall. For any APT-based distribution, it is generally a good idea to combine the APT-specific processes
update,upgrade,installandcleaninto one layer, so that no temporary files (APT lists, archives downloaded via APT) are present in the final image. It's also best practice to useapt-getinstead ofaptuntil theaptCLI is stable. And for package installations, if applicable we should always use the APT parameter--no-install-recommends, otherwise APT might install packages we don't want/need. But this depends on the specific package(s) to be installed. I assumed you only want the 3 packages with its dependencies and nothing else, so I added the parameter as well.As for the local files from
src/assetsandsrc/config, it was just my personal preference to keep it in the same layer.For example, the implementation of this PR could be changed to this:
This would have the same effect in the end. But we wouldn't save much time by parallelizing the download process of the image layers at all (a few config files + Starship installation), plus the layer must be extracted on a
docker pullwhich might actually be slower in the end. But it doesn't really matter in this case. Just personal preference.For reference, here's a good read on best practices: https://docs.docker.com/develop/develop-images/guidelines/
With BuildKit, the
RUNinstruction supports--mountbesides other great things (see https://docs.docker.com/build/guide/mounts/ for details). This specific instruction (--mount=type=bind,from=temp-files,source=/copy,target=/tmp/copy) will mount/copyfrom thetemp-filesstage as/tmp/copyinto the layer theRUNinstruction creates. Because it's just a mount,/tmp/copywill not be part of the built image.For reference, here is a comparison of image size:
where
hackboxis built with the latest master (commit hash 4c11938) andhackbox-newis built with the commit from this PR (982998f).Oh, almost forgot to mention: I've also added some
ARGinstructions to configure the nerd fonts to be installed, thought it might be best to put it at the top so you can easily update them if needed.