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

Docker: use multi-stage build to reduce size #6938

Merged
merged 2 commits into from
Aug 25, 2021

Conversation

soulteary
Copy link
Contributor

Hi, the current container size is too large, the local image size is close to 1GB.

docker images | grep shields                                                                                                                                                   
shieldsio/shields           server-2021-07-01              c567959e0651   7 weeks ago     848MB

And I've test the local build, the size is almost close to 1GB.

shields                                       latest                              83a611869ad9   7 minutes ago   919MB

For images used in the production environment, it is recommended to use docker multi-stage build, which can significantly reduce the size of the image in actual local use and reduce the size when pushed to dockerhub.

shields                                       latest                              c925e98df538   4 seconds ago   255MB

@shields-ci
Copy link

shields-ci commented Aug 22, 2021

Messages
📖 ✨ Thanks for your contribution to Shields, @soulteary!

Generated by 🚫 dangerJS against 556d471

@soulteary
Copy link
Contributor Author

Using the above code, I made an image submission, and the image size on DockerHub is 70 MB, :D

https://hub.docker.com/r/soulteary/shields/tags?page=1&ordering=last_updated

@chris48s chris48s added the self-hosting Discussion, problems, features, and documentation related to self-hosting Shields label Aug 22, 2021
@calebcartwright
Copy link
Member

Thank you for this! Admittedly having a compact image size isn't a real goal of the project, but I am generally in favor of leveraging builder images. However, I've not had any luck getting the image to build locally, neither from the PR ref nor from our default branch.

The changes themselves look fine, but I'm mildly apprehensive about moving forward unless one of the other maintainers is able to get it building given that we just recently lost our docker auto builds and have #6849 pending

@chris48s
Copy link
Member

I tried checking this PR out and running a docker build. For me this throws

mv: can't rename 'public': Directory not empty
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! shields.io@0.0.0 build: `run-s defs features && cd frontend && gatsby build && mv public ..`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the shields.io@0.0.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

..but then I checkout out master, ran a build and it throws the same error so that might be a red herring. I'm not sure I've ever built this image locally.
Is that the same error you got @calebcartwright ?

Shall we see if we can get #6849 done first and then come back to this?

@calebcartwright
Copy link
Member

Yup, that's the same thing I was getting. My weekly jobs that build from source are working fine though, including from master. I wonder if there's interference from the output of other scripts/general inner dev loop that don't pop up in the clone-and-build only scenarios

@chris48s
Copy link
Member

Right yeah I just tried it with a clean checkout and I think it is because of a dirty local working tree, not changes in this PR.
This branch does build and boots a container ok for me 👍
While I was doing this I did spot another completely unrelated issue with the docker build though so I'll submit another PR with that fix in a moment..

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

Approving since changes LGTM and we've identified the (high level) cause build issues we were experiencing locally. Going to hold off merging in case you'd still prefer we tackle #6849 first @chris48s

@repo-ranger repo-ranger bot merged commit 22995e4 into badges:master Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
self-hosting Discussion, problems, features, and documentation related to self-hosting Shields
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants