-
-
Notifications
You must be signed in to change notification settings - Fork 208
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
Dockerfile improvements for final image size #92
base: main
Are you sure you want to change the base?
Conversation
Hi, Thanks for the suggestions. Here's a bit of feedback: build-essentialTo be honest I'm not sure how this image built without I have a larger project that uses this example app and I removed 11.88 Gem::Ext::BuildError: ERROR: Failed to build gem native extension.
11.88
11.88 current directory: /usr/local/bundle/gems/bigdecimal-3.1.8/ext/bigdecimal
11.88 /usr/local/bin/ruby extconf.rb
11.88 checking for __builtin_clz()... *** extconf.rb failed ***
11.88 Could not create Makefile due to some reason, probably lack of necessary
11.88 libraries and/or headers. Check the mkmf.log file for more details. You may
11.88 need configuration options. I included it out of convenience because tracking down a bunch of isolated C dependencies for gems that need to be compiled can be tricky for folks not experienced with Linux and there's quite a few of them in larger projects. libpq-devThat is another C dependency to compile the I don't see an arm64 binary which means the gem will need to be compiled on arm64 devices and that will fail without bundler changes--deployment flagI haven't tested this but will That change which also mean binary paths will fail to resolve since that's not on the system path. I think that's why your CI build is currently failing because puma isn't found anymore. BUNDLE_PATHOn the main branch of this project, that environment variable is unset. Does setting any of those flags in this PR somehow define this env var? --without development testWithout installing development and test gems, how will we run this locally and tests in CI? Ideally we would want builds to work with all environments. Have you compared the before and after in image size with only this change? Assorted paths being removedThe Ruby cache and .git paths you provided don't exist so they will always be empty but For example --no-cacheIf the cache directory were deleted then there would be nothing to cache. Based on the help menu for bundler, it sounds like this only comes into play if existing cache files exist? --no-cleanI couldn't find a reference to this flag in the help menu and bundler also mentions the --retryThis is a good idea for resiliency, lucky for us bundler defaults to 3. Here's the reference in the docs:
-jYou set Here's the reference in the docs:
Do you have benchmark results where setting 4 is faster than a higher number on a high CPU core machine? My workstation only has 4 cores so I can't test that. RUN mkdir .yarn public log tmpThese directories will always exist. |
I've tried to port over stuff from my current Dockerfile (which is a bit more bespoke with other entrypoints and built to run on various Azure platforms. For CI, I usually allow RAILS_ENV and BUNDLE_WITHOUT as ARGs (amongst a bunch of other ones I need for my projects) so I can override during build phases in CI or locally. The Trying not to over-impose my own cases! I'll clean up the bundle commands and update if you'd like. |
Oh I didn't catch that it still existed in the asset stage. I removed it from both stages in my other project. Now it makes sense.
The default set up uses
Can you do a before and after in image size on this project? The concern I have with this is if you build separate images for different environments you run the risk of an untested image getting shipped to production. That's mostly why I've avoided this pattern. For example, with the way things are now you can build your image + run your tests + push that image to your registry and run it in production. IMO that convenience is worth paying a 50MB image size tax or whatever it ends up being, of course it will vary with how many dev and test dependencies you have. Also unless you're running on something like AWS Fargate where images aren't cached, you can leverage Docker's layer caching to avoid pulling the whole image on every deploy. You would only pull your dependency layer if your Gemfile changes, otherwise Docker will happily only transfer the diff on your app's code layer. With these proposed changes you need your test dependencies to run your tests and then you would need to re-build a separate image with only production gems. Separate to that, you can run parallel Docker builds to at least avoid having to wait twice as long on every deploy, but most hosted CI servers have pretty weak CPUs (2 cores, ~2ghz, etc.), so if you're running parallel bundles for 2 Docker images in parallel you may end up still getting dinged pretty hard for build times. |
Personally I'd rather keep my production image as clean as I can, reduce the attack risks with less gems available. Similar for Since this is all CI builds, I'm personally not too bothered by run times, or duplicate runs. My CI agent will have layers cached and can reuse them during builds to speed it up if they're run in the same job (not sure if GH runs the same as ADO). Sounds like you're happy with just the |
For now, I think so. That seems like low hanging fruit to decrease the production image size without changing any behavior. Did you compare the size before and after just for that change? If it's substantial do you want to open a smaller PR just for that? I think Keep in mind there's the I've never been crazy about that pattern but it does work which is essentially running |
Changes to reduce production image size
build-essential
from final imageThese reduce the final output image from around 591MB to 226MB