Skip to content
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

improve docker build #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thetredev
Copy link

@thetredev thetredev commented Jan 3, 2024

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 Dockerfile a bit.


This PR improves the docker build process. This is accomplished with two steps:

  1. Using a temporary stage temp-files, no temporary files will be present in the built image
  2. Image layers in the Customize Container Here are drastically reduced to 1 RUN layer

The 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 a scratch image for that (which contains no files), but since the Dockerfile ADD instruction doesn't support extracting zip files (yet), I needed a small image which has unzip installed, So I ended up with busybox. Since the target stage for docker build / docker buildx always defaults to the FROM instruction without any AS instruction, the temp-files stage 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=1 for 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, install and clean into 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 use apt-get instead of apt until the apt CLI 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/assets and src/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:

# Install packages and configure the container
RUN export DEBIAN_FRONTEND=noninteractive && \
    # Fix locales when installing packages
    # (only for this specific shell)
    export LC_ALL=C \
    && \
        # Upgrade the system
        apt-get update && \
        apt-get -y full-upgrade \
    && \
        # Install packages
        apt-get -y install --no-install-recommends \
            numix-gtk-theme \
            openvpn \
            terminator \
    && \
        # Clear APT caches
        apt-get -y clean && \
        rm -rf /var/lib/apt/lists/*

# Copy config files from mounted /tmp/copy/configs directory
RUN --mount=type=bind,from=temp-files,source=/copy,target=/tmp/copy \
    mkdir -p root/.config/terminator && \
    cp -R /tmp/copy/configs/* / && \
    # Install Starship
    /tmp/copy/starship/install.sh -y && \
    # Configure Bash for Starship
    echo 'eval "$(starship init bash)"' >> .bashrc

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 pull which 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 RUN instruction supports --mount besides 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 /copy from the temp-files stage as /tmp/copy into the layer the RUN instruction creates. Because it's just a mount, /tmp/copy will not be part of the built image.

For reference, here is a comparison of image size:

$ docker images --format "{{.ID}}: {{.Repository}} / {{.Size}} " hackbox
4d638140c633: hackbox / 1.65GB

$ docker images --format "{{.ID}}: {{.Repository}} / {{.Size}} " hackbox-new
18b031f19d47: hackbox-new / 1.57GB

where hackbox is built with the latest master (commit hash 4c11938) and hackbox-new is built with the commit from this PR (982998f).

Oh, almost forgot to mention: I've also added some ARG instructions 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.

Signed-off-by: Timo Reichl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant