Skip to content

docker: bake Maven dependencies into an image layer instead of a cache mount#3428

Open
guusdk wants to merge 5 commits into
igniterealtime:mainfrom
guusdk:ci_docker-layer-instead-of-cache
Open

docker: bake Maven dependencies into an image layer instead of a cache mount#3428
guusdk wants to merge 5 commits into
igniterealtime:mainfrom
guusdk:ci_docker-layer-instead-of-cache

Conversation

@guusdk

@guusdk guusdk commented Jul 7, 2026

Copy link
Copy Markdown
Member

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_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.

…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.
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ba350c6a-15f9-45ec-ba2c-a87430422aa8

📥 Commits

Reviewing files that changed from the base of the PR and between 18ed22e and 18e6e3a.

📒 Files selected for processing (2)
  • Dockerfile
  • xmppserver/pom.xml
✅ Files skipped from review due to trivial changes (1)
  • xmppserver/pom.xml
🚧 Files skipped from review as they are similar to previous changes (1)
  • Dockerfile

📝 Walkthrough

Walkthrough

Changes

The Dockerfile removes BuildKit cache mounts from Maven plugin resolution, dependency prefetching, and the final offline install step while continuing to use /tmp/m2_repo. The xmppserver/pom.xml BOM version declarations now include inline comments that point to the Dockerfile pre-fetch settings.

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)
Check name Status Explanation
Description check ✅ Passed The description clearly matches the Docker and Maven dependency caching changes in the pull request.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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_repo from Maven dependency resolution steps so /tmp/m2_repo is written into an image layer.
  • Keeps the offline Maven build (mvn -o install) but relies on the previously-populated /tmp/m2_repo in 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.

Comment thread Dockerfile Outdated
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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread Dockerfile Outdated
Comment thread Dockerfile Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 win

Sync the fetched BOM versions with xmppserver/pom.xml Dockerfile:35-37

xmppserver/pom.xml imports org.junit:junit-bom:6.1.0 and org.mockito:mockito-bom:5.23.0, but this layer prefetches 5.13.4 and 5.22.0. Update the dependency:get versions 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 | 🔵 Trivial

Layer-based caching now depends on the CI builder preserving/importing Docker layer cache.

Switching from a --mount=type=cache mount to baking /tmp/m2_repo into 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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5fc922c2-15da-4e4a-8b99-c62a0ad3e99f

📥 Commits

Reviewing files that changed from the base of the PR and between 07c1ef9 and f892c67.

📒 Files selected for processing (1)
  • Dockerfile

Comment thread Dockerfile Outdated
guusdk added 2 commits July 7, 2026 15:50
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.
@guusdk guusdk force-pushed the ci_docker-layer-instead-of-cache branch from 32b52ca to 18ed22e Compare July 7, 2026 13:54

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
Dockerfile (1)

26-33: 🧹 Nitpick | 🔵 Trivial

Trade-off note: full /tmp/m2_repo now 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=gha in CI (as confirmed by the workflow snippet), whereas type=cache mounts 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.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 54b789f5-d1e4-4e8a-b7c8-e0d3b6409c8a

📥 Commits

Reviewing files that changed from the base of the PR and between 32b52ca and 18ed22e.

📒 Files selected for processing (1)
  • Dockerfile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants