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

Optimize Dockerfiles #4532

Merged
merged 1 commit into from May 19, 2024
Merged

Conversation

dfunkt
Copy link
Contributor

@dfunkt dfunkt commented May 1, 2024

Move some ARGs closer to the build stage (potentially improving caching):
Example: I usually set VW_VERSION to be the latest commit hash, without this change I'd have to also rebuild the dependencies even if they didn't change since the previous layers would get invalidated.

Remove redundant COPY commands:
They can be combined, no point in using extra layers if it can be avoided.

Remove redundant RUN command:
apt-get and xx-apt-get commands can be moved into the same RUN command, thus saving another layer.

Move CARGO_HOME's "&&" operator to the first line (improves consistency)

@dfunkt dfunkt force-pushed the docker-optimizations branch 2 times, most recently from e7da948 to ace607b Compare May 2, 2024 19:42
@BlackDex
Copy link
Collaborator

BlackDex commented May 2, 2024

Saving a lot of layers at the top is not that efficiënt i think.
Ill have to check, there were some reasons i didn't combined the apt's, but not sure.

@dfunkt
Copy link
Contributor Author

dfunkt commented May 2, 2024

Fair enough, if there's stuff here that you don't like just tell me and I'll remove it.

I've been running my builds for a while with these changes and I figured other people might benefit from them as well.

@BlackDex
Copy link
Collaborator

BlackDex commented May 2, 2024

Fair enough, if there's stuff here that you don't like just tell me and I'll remove it.

I've been running my builds for a while with these changes and I figured other people might benefit from them as well.

Do you build all arch's?

@dfunkt
Copy link
Contributor Author

dfunkt commented May 2, 2024

amd64 and arm64, both for personal use

@williamdes
Copy link
Contributor

A good tool to check this PR is https://github.com/wagoodman/dive

@BlackDex
Copy link
Collaborator

BlackDex commented May 2, 2024

A good tool to check this PR is https://github.com/wagoodman/dive

I use that too indeed. But that will only work for the final image, not for the build layer.

@dfunkt
Copy link
Contributor Author

dfunkt commented May 2, 2024

What do you guys think about including this as well for apt?
https://docs.docker.com/build/cache/#use-the-dedicated-run-cache

@BlackDex
Copy link
Collaborator

BlackDex commented May 2, 2024

It doesn't really add anything useful i think.
The main reason is that we went fresh images which have the latest packages available to keep up-to-date and all CVE's are gone as good as possible.

Also, docker buildx does some nice caching by it self. And we already use build caches during our release pipeline.

We used that before btw, which was useful back then, and not building in parallel. When building in parallel it gave some issues, which can be solved, but then it loses its benefits.

@BlackDex
Copy link
Collaborator

BlackDex commented May 3, 2024

While optimizing for layers does not have any benefit for the build part, it also doesn't hurt in this case.
The only time it will hurt is when you are testing/debugging the last apt commands in the same RUN. But that will not be often i hope.

Copy link
Collaborator

@BlackDex BlackDex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine by me.

docker/Dockerfile.j2 Outdated Show resolved Hide resolved
Move some ARGs closer to the build stage (potentially improving caching)
Remove redundant COPY commands
Remove redundant RUN command
Move CARGO_HOME's "&&" operator to the first line (improves consistency)
@dani-garcia dani-garcia merged commit 3261534 into dani-garcia:main May 19, 2024
5 checks passed
@dfunkt dfunkt deleted the docker-optimizations branch May 20, 2024 05:18
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.

None yet

5 participants