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

Cut down Docker image sizes #3916

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

ddundo
Copy link
Contributor

@ddundo ddundo commented Dec 11, 2024

Description

Currently unnecessary files are removed in a separate layer in Dockerfile.firedrake-env, which doesn't reduce the final image size. In this PR I removed them inside the build layer, so the image size is reduced by 3GB (from 6.58GB to 3.57GB).

Copy link
Contributor

@connorjward connorjward left a comment

Choose a reason for hiding this comment

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

This is a great spot. Thanks.

docker/Dockerfile.env Show resolved Hide resolved
@ddundo ddundo requested a review from connorjward December 11, 2024 12:54
@connorjward
Copy link
Contributor

Looks like this is running into repo permissions issues. I will copy your branch into firedrakeproject and open my own PR.

@ddundo
Copy link
Contributor Author

ddundo commented Dec 11, 2024

Looks like this is running into repo permissions issues. I will copy your branch into firedrakeproject and open my own PR.

Thanks @connorjward! And yeah, I tried to copy some of the CI changes I saw in your yesterday's PR.

@connorjward
Copy link
Contributor

#3917

@connorjward
Copy link
Contributor

I am happy with this. If you remove the CI-specific changes I will approve and merge this. Thanks again!

@ddundo ddundo force-pushed the docker_squash_layer branch from 6da57f5 to bbb6079 Compare December 11, 2024 14:34
@ddundo ddundo marked this pull request as ready for review December 11, 2024 14:34
@ddundo
Copy link
Contributor Author

ddundo commented Dec 11, 2024

I am happy with this. If you remove the CI-specific changes I will approve and merge this. Thanks again!

Done! Thanks for looking into this so fast

@connorjward connorjward merged commit 7b2b81a into firedrakeproject:master Dec 11, 2024
@ddundo
Copy link
Contributor Author

ddundo commented Dec 11, 2024

Btw @connorjward I just noticed that all subsequent images (firedrake-vanilla etc.) were built with firedrake-env:latest as base in the CI, and not firedrake-env:testing as I would have expected. Ignore me if that's all good

@connorjward
Copy link
Contributor

Btw @connorjward I just noticed that all subsequent images (firedrake-vanilla etc.) were built with firedrake-env:latest as base in the CI, and not firedrake-env:testing as I would have expected. Ignore me if that's all good

Yeah I wasn't sure why that happened but the changes you introduced were so minor I wasn't concerned. The CI demonstrated that Dockerfile.env ran fine which was what mattered.

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.

2 participants