Support for Docker multistage build cache #337
Conversation
lib/mrsk/commands/builder/native.rb
Outdated
| docker(:build, *build_options, build_context), | ||
| docker(:push, config.absolute_image), | ||
| docker(:push, config.latest_image) | ||
| end | ||
| end |
There was a problem hiding this comment.
Instead of doing conditionals on all of these, I think maybe we should have Mrsk::Commands::Builder::Native::Cached?
There was a problem hiding this comment.
Good point!
There was a problem hiding this comment.
Done, added Mrsk::Commands::Builder::Native::Cached builder.
lib/mrsk/configuration/builder.rb
Outdated
| [ | ||
| "type=gha", | ||
| @options["cache"]&.fetch("options", nil), | ||
| ].compact.join(",") |
There was a problem hiding this comment.
Extract this as cache_config_for_gha or similar.
lib/mrsk/configuration/builder.rb
Outdated
| "type=registry", | ||
| @options["cache"]&.fetch("options", nil), | ||
| "ref=#{@server}/#{cache_image}" | ||
| ].compact.join(",") |
There was a problem hiding this comment.
Extract this as cache_config_for_registry or similar.
| remove_context(local["arch"]), | ||
| remove_context(remote["arch"]) | ||
| remove_context(config.builder.local_arch), | ||
| remove_context(config.builder.remote_arch) |
There was a problem hiding this comment.
Maybe setup a delegate_to for config.builder for remote/local_arch|host.
There was a problem hiding this comment.
Added delegation of all used public methods from Mrsk::Configuration::Builder to builder_config. Lines became shorter, readability improved.
|
Nice use of the extracted builder config object 👍 |
|
David, thanks for your comments. Will take a look later today. |
lib/mrsk/configuration/builder.rb
Outdated
| case @options["cache"]["type"] | ||
| when 'gha' | ||
| "type=gha" | ||
| when 'registry' |
lib/mrsk/configuration/builder.rb
Outdated
| if cached? | ||
| case @options["cache"]["type"] | ||
| when 'gha' | ||
| "type=gha" |
There was a problem hiding this comment.
Use cache_from_config_for_gha to remain at the same level of abstraction.
lib/mrsk/configuration/builder.rb
Outdated
| [ | ||
| "type=registry", | ||
| "ref=#{@server}/#{cache_image}" | ||
| ].compact.join(",") |
lib/mrsk/configuration/builder.rb
Outdated
| [ | ||
| "type=gha", | ||
| @options["cache"]&.fetch("options", nil), | ||
| ].compact.join(",") |
lib/mrsk/configuration/builder.rb
Outdated
| "type=registry", | ||
| @options["cache"]&.fetch("options", nil), | ||
| "ref=#{@server}/#{cache_image}" | ||
| ].compact.join(",") |
| *build_options, | ||
| build_context | ||
| end | ||
| end |
|
Just a few more minor points. Looking good! |
|
Fixed all code style comments. This is why I proposed Rubocop :) |
This PR implements support for Docker build cache and closes #329.
Currently I added support for two cache types: gha and registry. Both cache types allow to specified additional options that will be used in
cache-toparam. Registry cache also allows to specify image to be used for cache, by default application image is used with-build-cachesuffix.I've tested this on Rails 7 application. Both gha and registry cache works and saves about 40% of build time.