Remove buildx cache. Do not delete builder instances after goal. Us… #1579

chonton · 2022-07-02T18:08:52Z

…e builder instance to cache artifacts

codecov · 2022-07-02T18:12:05Z

Codecov Report

Merging #1579 (46eb0a4) into master (db7a69b) will decrease coverage by 0.02%.
The diff coverage is 86.66%.

@@             Coverage Diff              @@
##             master    #1579      +/-   ##
============================================
- Coverage     63.54%   63.51%   -0.03%     
+ Complexity     2165     2163       -2     
============================================
  Files           170      170              
  Lines          9915     9905      -10     
  Branches       1359     1359              
============================================
- Hits           6300     6291       -9     
+ Misses         3086     3085       -1     
  Partials        529      529

Impacted Files	Coverage Δ
...bric8/maven/docker/config/BuildXConfiguration.java	`88.46% <80.00%> (ø)`
...io/fabric8/maven/docker/service/BuildXService.java	`71.53% <86.95%> (-1.26%)`	⬇️
...aven/docker/config/handler/property/ConfigKey.java	`100.00% <100.00%> (ø)`
...config/handler/property/PropertyConfigHandler.java	`85.90% <100.00%> (ø)`

…builder instance to cache artifacts closes #1576

chonton · 2022-07-03T17:17:34Z

@rohanKanojia, please review
@marcelstoer, this is proposal 1 mentioned in issue #1576. Does this branch solve the caching problem you experienced?

marcelstoer · 2022-07-06T14:59:04Z

Thanks, the proposal is sane. I checked out your branch and mvn installed it.

The first mvn deploy in my multi-arch test project was fine apart from the "insufficient_scope: authorization failed" failure at the end which was my bad. However

Based on the changes in this PR I was expecting to see a maven buildx builder to survive the Maven build. docker buildx ls says this be not the case.
Subsequent mvn deploy (re-)create the maven buildx builder but fail shortly after with [INFO] DOCKER> error: failed to solve: failed to read dockerfile: open /tmp/buildkit-mount1275820182/Dockerfile: no such file or directory. I don't know how to fix it as I don't see where this path is coming from.

chonton · 2022-07-06T16:05:54Z

A builder is nothing more than a set of configurations stored in config.json in the config directory. In order to list the builder using the cli, you must set --config to the same value that dmp uses (the dockerStateDir). By default, this directory is under the project target directory and will be deleted with mvn clean. Make sure the target directory is not deleted or set dockerStateDir to another location.

derkaiserreich · 2022-07-07T14:03:00Z

I also ran this version to build our platform and can confirm, that the cached versions from the build goal are now used in the push goal.

However wouldn't it in general make more sense to push the images directly, instead of trying to build them again in the push goal, even if the cached images are now used?

marcelstoer · 2022-07-07T14:07:22Z

wouldn't it in general make more sense to push the images directly

You get this behavior if you only have a single <execution> that uses the push goal. Building twice happens if you use the build-test-push approach (three executions).

derkaiserreich · 2022-07-07T14:17:30Z

I have only one <execution> with the goals build and push. Just using the push push goal leads to an error from the buildx system. The documentation of the push goal isn't really clear on how it can be used as the only one either, so I just assumed, that you have to use both goals in order to build and then push the images.

chonton · 2022-07-07T17:44:39Z

Currently, you will need both goals. This is consistent with the behavior of non-buildx builds: build, test, push. We will consider a single build/push goal if there is high demand. How much time do you find the separate goals add to your build? Is this a significant hit?

derkaiserreich · 2022-07-07T21:31:45Z

Now with the caching being enabled, the impact of the separate goals isn't that significant for our CI pipeline. For local development we can work around this and build the images without buildx, so for our usecase I don't see the immediate demand for a single goal.

However I can see other projects having their build time growing significantly when building multiple images for multiple platforms, so I can see the benefits of just one single, combined goal.

I really appreciate the work you're doing here, so thank you for that!