docker: bake Maven dependencies into an image layer instead of a cache mount#3428
docker: bake Maven dependencies into an image layer instead of a cache mount#3428guusdk wants to merge 5 commits into
Conversation
…e mount The dependency-resolution steps wrote the local Maven repository to a BuildKit cache mount (`--mount=type=cache,target=/tmp/m2_repo`). A cache mount is builder-local scratch space that is deliberately excluded from image layers, so it was never captured by layer caching. On a fresh builder the mount was always empty and every build re-downloaded the full dependency tree (~3,600 artifacts, ~7 minutes), while the actual offline compile took only ~24 seconds. Drop the cache mount from the three dependency steps and the offline install so the populated `/tmp/m2_repo` lands in the build stage's image layer. That layer depends only on the pom.xml files (via the poms stage), so it stays cached until a POM changes: source-only changes now reuse the downloaded dependencies instead of fetching them again. Effect on produced images: none. The Maven repository lives only in the intermediate build stage; the runtime stage still copies out just distribution-base and entrypoint.sh, so the final image is byte-for-byte equivalent. The build stage's cached layer grows by the size of the Maven repository, which is the intended trade for skipping the downloads.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesThe Dockerfile removes BuildKit cache mounts from Maven plugin resolution, dependency prefetching, and the final offline install step while continuing to use Estimated code review effort: Medium Related issues: None specified Related PRs: None specified Suggested labels: build, docker Suggested reviewers: None specified 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the Docker build to make Maven dependency downloads persist in a cached image layer (instead of a BuildKit cache mount), improving rebuild performance when only source files change.
Changes:
- Removes
--mount=type=cache,target=/tmp/m2_repofrom Maven dependency resolution steps so/tmp/m2_repois written into an image layer. - Keeps the offline Maven build (
mvn -o install) but relies on the previously-populated/tmp/m2_repoin the build stage. - Updates comments to describe layer-based caching behavior for dependencies.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The poms stage now drops `.mvn/wrapper/maven-wrapper.jar` (the build stage copies `.mvn/wrapper` in separately), so a wrapper-jar change no longer invalidates the cached dependency layer. Other checked-in jars are still retained, as some are referenced by POMs and needed for the offline build.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
35-37: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSync the fetched BOM versions with xmppserver/pom.xml
Dockerfile:35-37
xmppserver/pom.xmlimportsorg.junit:junit-bom:6.1.0andorg.mockito:mockito-bom:5.23.0, but this layer prefetches5.13.4and5.22.0. Update thedependency:getversions so the offline build can resolve the imported BOMs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` around lines 35 - 37, The offline dependency prefetch in the Dockerfile is out of sync with the BOM versions imported by xmppserver/pom.xml, so the build can’t resolve them from the local Maven repo. Update the two dependency:get invocations for org.junit:junit-bom and org.mockito:mockito-bom to match the exact versions declared in the pom, keeping the existing plexus-utils fetch unchanged. Use the dependency:get commands in the Dockerfile layer as the source of truth and ensure the fetched versions align with the BOM imports.
🧹 Nitpick comments (1)
Dockerfile (1)
25-44: 🚀 Performance & Scalability | 🔵 TrivialLayer-based caching now depends on the CI builder preserving/importing Docker layer cache.
Switching from a
--mount=type=cachemount to baking/tmp/m2_repointo an image layer trades a mechanism that can persist independently of layer identity for one that relies on BuildKit's ordinary layer cache. In ephemeral CI runners, your laptop has a persistent builder cache. CI runners usually don't. Export/import BuildKit cache, or provide persistent storage for the builder. Worth confirming the CI pipeline building this image uses--cache-from/--cache-to(or an equivalent registry/GHA cache) so the new dependency layer is actually reused across builds/runners rather than only within a single long-lived builder instance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` around lines 25 - 44, The Maven dependency layer in the Dockerfile now relies on ordinary BuildKit layer caching for /tmp/m2_repo, so it may not be reused on ephemeral CI runners. Update the build/pipeline that builds this image to export and import the cache (for example via --cache-from/--cache-to or an equivalent registry/GHA cache) so the dependency-resolving steps stay reusable across runners. Reference the Dockerfile’s dependency resolution block with COPY --from=poms, the dependency:get calls, and the final offline mvn install when validating the cache strategy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Around line 9-12: The dependency cache is still being invalidated by
`.mvn/wrapper/maven-wrapper.jar` because the build stage copies `.mvn/wrapper`
before the dependency-resolution steps. Update the Dockerfile so the `build`
stage’s early `.mvn/wrapper` copy or the Maven prefetch flow in the
`poms`/dependency layers no longer includes the wrapper jar, keeping the cache
stable when the jar changes. Use the existing `poms` stage and dependency `RUN`
steps as the location to adjust this split.
---
Outside diff comments:
In `@Dockerfile`:
- Around line 35-37: The offline dependency prefetch in the Dockerfile is out of
sync with the BOM versions imported by xmppserver/pom.xml, so the build can’t
resolve them from the local Maven repo. Update the two dependency:get
invocations for org.junit:junit-bom and org.mockito:mockito-bom to match the
exact versions declared in the pom, keeping the existing plexus-utils fetch
unchanged. Use the dependency:get commands in the Dockerfile layer as the source
of truth and ensure the fetched versions align with the BOM imports.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 25-44: The Maven dependency layer in the Dockerfile now relies on
ordinary BuildKit layer caching for /tmp/m2_repo, so it may not be reused on
ephemeral CI runners. Update the build/pipeline that builds this image to export
and import the cache (for example via --cache-from/--cache-to or an equivalent
registry/GHA cache) so the dependency-resolving steps stay reusable across
runners. Reference the Dockerfile’s dependency resolution block with COPY
--from=poms, the dependency:get calls, and the final offline mvn install when
validating the cache strategy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
The poms stage removed `.mvn/wrapper/maven-wrapper.jar` to try to keep the dependency-download layer stable across wrapper-jar changes. It didn't work: the build stage copies `.mvn/wrapper` (jar included) before the `COPY --from=poms` and the dependency `RUN` steps, so a wrapper-jar change still invalidates every later layer regardless of what poms emits. Making the dependency layers independent of the wrapper jar would mean giving up the offline prefetch or restructuring how Maven is invoked, and the wrapper jar changes rarely enough that it isn't worth that complexity. Revert the exclusion and correct the comments to describe the layer's actual cache inputs.
32b52ca to
18ed22e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Dockerfile (1)
26-33: 🧹 Nitpick | 🔵 TrivialTrade-off note: full
/tmp/m2_reponow baked into build-stage layers.Removing the cache mounts means the populated Maven repo is captured across three intermediate layers (lines 26, 27, 31-33) instead of being excluded from the image. This is the intended trade-off per the PR description — layer caching is reused via
cache-to: type=ghain CI (as confirmed by the workflow snippet), whereastype=cachemounts aren't preserved by that caching mechanism. Worth keeping in mind that these build-stage layers will now be larger and contribute to push/pull time when pushing/pulling the GHA cache, even though they never reach the final runtime image.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` around lines 26 - 33, The Dockerfile change now bakes the populated /tmp/m2_repo into intermediate build layers, increasing build-stage layer size and cache push/pull cost. Keep the current explicit dependency:get approach in the Dockerfile, but verify the CI workflow still uses cache-to: type=gha and update the nearby Dockerfile comments or build docs to call out that cache mounts are intentionally avoided because they are not preserved by that caching path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Dockerfile`:
- Around line 26-33: The Dockerfile change now bakes the populated /tmp/m2_repo
into intermediate build layers, increasing build-stage layer size and cache
push/pull cost. Keep the current explicit dependency:get approach in the
Dockerfile, but verify the CI workflow still uses cache-to: type=gha and update
the nearby Dockerfile comments or build docs to call out that cache mounts are
intentionally avoided because they are not preserved by that caching path.
Note: this is fully AI-driven.
The dependency-resolution steps wrote the local Maven repository to a BuildKit cache mount (
--mount=type=cache,target=/tmp/m2_repo). A cache mount is builder-local scratch space that is deliberately excluded from image layers, so it was never captured by layer caching. On a fresh builder the mount was always empty and every build re-downloaded the full dependency tree (~3,600 artifacts, ~7 minutes), while the actual offline compile took only ~24 seconds.Drop the cache mount from the three dependency steps and the offline install so the populated
/tmp/m2_repolands in the build stage's image layer. That layer depends only on the pom.xml files (via the poms stage), so it stays cached until a POM changes: source-only changes now reuse the downloaded dependencies instead of fetching them again.Effect on produced images: none. The Maven repository lives only in the intermediate build stage; the runtime stage still copies out just distribution-base and entrypoint.sh, so the final image is byte-for-byte equivalent. The build stage's cached layer grows by the size of the Maven repository, which is the intended trade for skipping the downloads.