chore: add caching of container images and related features#1796
Conversation
Reviewer's GuideIntroduces a new GitHub Actions workflow to build/test/push dev container images with hash-based freshness to GHCR, and updates the Maven CI workflow and DB init scripts to prefer pulling these images with robust fallbacks, caching, and more reliable database readiness checks. Sequence diagram for Maven CI dev container pull-first fallback strategysequenceDiagram
actor Dev as Developer
participant GitHub as GitHub
participant MavenWorkflow as maven-project.yml
participant Runner as GitHub_Actions_Runner
participant GHCR as GHCR
participant Docker as Docker_engine
Dev->>GitHub: Open PR or push
GitHub->>MavenWorkflow: Trigger workflow
MavenWorkflow->>Runner: Start build job
Runner->>Docker: docker login ghcr.io
Runner->>GHCR: docker pull ghcr.io/IMAGE_PREFIX/openo-tomcat-dev:latest
alt Image pull succeeds
GHCR-->>Runner: Image layers
Runner->>Docker: tag pulled image as openo-tomcat-dev
Runner->>Runner: Skip Docker layer cache and local build
else Image not found (manifest unknown)
GHCR-->>Runner: Not found error
Runner->>Runner: Enable Docker layer cache
Runner->>Docker: buildx build openo-tomcat-dev (fallback)
else Unexpected error
GHCR-->>Runner: Other pull error
Runner-->>MavenWorkflow: Set reason=error
MavenWorkflow->>Runner: Fail job with error message
end
opt Successful image available
Runner->>Docker: run dev container
Runner->>Runner: execute make install and tests inside container
end
Flow diagram for container-images workflow hash-based build and pushflowchart TD
Start([Start container-images workflow])
CheckInputs[Check force_rebuild input]
subgraph Job_check_containers[check-containers job]
A_checkout[Checkout code]
A_login[Login to GHCR]
A_hashes[Compute content hash per container]
A_pull_inspect[Pull existing image and inspect org.openosp.content-hash label]
A_compare[Compare hashes and set *_needs_build flags]
end
subgraph Job_build_tomcat[build-tomcat job]
T_cond{{tomcat_needs_build == true?}}
T_build[Build openo-tomcat-dev:test with labels]
T_test[Test container: Maven, Java, Tomcat, Node.js]
T_push{Event is push?}
T_push_action[Tag and push latest and sha tags]
end
subgraph Job_build_mariadb[build-mariadb job]
M_cond{{mariadb_needs_build == true?}}
M_build[Build openo-mariadb-dev:test with labels]
M_test_start[Run container with MYSQL_ROOT_PASSWORD]
M_wait_ready[Wait for MariaDB readiness]
M_wait_init[Wait for Database initialization complete! log]
M_verify_db[Verify oscar, oscar_test, drugref2 databases]
M_push{Event is push?}
M_push_action[Tag and push latest and sha tags]
end
subgraph Job_build_drugref[build-drugref job]
D_cond{{drugref_needs_build == true?}}
D_build[Build openo-drugref:test with labels]
D_test_start[Run container and wait for Tomcat HTTP response]
D_verify[Verify Java version and WAR deployment]
D_push{Event is push?}
D_push_action[Tag and push latest and sha tags]
end
subgraph Job_summary[summary job]
S_collect[Collect job results]
S_report[Print per-container build summary and fail on errors]
end
Start --> CheckInputs --> A_checkout --> A_login --> A_hashes --> A_pull_inspect --> A_compare
A_compare --> T_cond
A_compare --> M_cond
A_compare --> D_cond
T_cond -- "true" --> T_build --> T_test --> T_push
T_cond -- "false" --> M_cond
T_push -- "yes" --> T_push_action --> M_cond
T_push -- "no" --> M_cond
M_cond -- "true" --> M_build --> M_test_start --> M_wait_ready --> M_wait_init --> M_verify_db --> M_push
M_cond -- "false" --> D_cond
M_push -- "yes" --> M_push_action --> D_cond
M_push -- "no" --> D_cond
D_cond -- "true" --> D_build --> D_test_start --> D_verify --> D_push
D_cond -- "false" --> S_collect
D_push -- "yes" --> D_push_action --> S_collect
D_push -- "no" --> S_collect
S_collect --> S_report --> End([End workflow])
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. OpenSSF Scorecard
Scanned Files
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…tain permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds DB_PASSWORD (defaulting from MYSQL_ROOT_PASSWORD) and a completion echo to the DB init script; introduces a new GitHub Actions workflow to compute content hashes and conditionally build/test/push three dev images; and extends the Maven CI workflow to pull pre-built images with registry login, fallback local builds, and enhanced DB readiness/verification including Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Developer
participant GH as GitHub Actions
participant Reg as ghcr.io
participant Builder as Buildx
participant Tester as Test Container
participant Summary as Summary Job
Dev->>GH: Trigger container-images workflow (push/dispatch/PR)
GH->>GH: Compute content-hash from Dockerfile/context
GH->>Reg: Attempt to pull existing image and read content-hash label
alt Image up-to-date
Reg-->>GH: Image up-to-date
GH->>Summary: Mark skip for that image
else Needs rebuild / not found
Reg-->>GH: Not found or hash mismatch
GH->>Builder: Build image (label with content-hash)
Builder-->>Tester: Run containerized validation checks
Tester-->>GH: Validation result
alt Tests pass and event is push
GH->>Reg: Push tags (latest + sha)
Reg-->>GH: Confirm push
end
GH->>Summary: Record build outcome
end
sequenceDiagram
participant MavenJob as Maven Job
participant Reg as ghcr.io
participant LocalBuild as Local Docker Build
participant Network as Docker Network
participant DB as DB Container
participant Tests as Test Suite
MavenJob->>Reg: Try pull pre-built `openo-tomcat-dev`
alt Pull succeeded
Reg-->>MavenJob: Use pulled image
else Not found
Reg-->>MavenJob: 404 Not Found
MavenJob->>LocalBuild: Build dev container (fallback)
LocalBuild-->>MavenJob: Image built
else Pull error
Reg-->>MavenJob: Error -> conditional fail step
end
MavenJob->>Network: Create network (legacy path)
MavenJob->>DB: Start DB container(s) (uses `DB_PASSWORD`)
DB->>DB: Initialize databases and emit readiness logs
DB-->>MavenJob: Readiness detected via logs
MavenJob->>Tests: Run tests against running containers
Tests-->>MavenJob: Test results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces container image caching to improve CI/CD performance by pre-building and storing development container images in GitHub Container Registry (ghcr.io). The workflow attempts to pull pre-built images first, falling back to local builds when unavailable.
Changes:
- New
container-images.ymlworkflow that builds, tests, and pushes three container images (tomcat-dev, mariadb-dev, drugref) with content-hash based freshness checking - Modified
maven-project.ymlto pull pre-built images from ghcr.io before building locally, with conditional build steps - Updated database initialization script to emit a completion message for easier detection in CI workflows
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
.github/workflows/container-images.yml |
New workflow for building and pushing container images to ghcr.io with hash-based caching and comprehensive testing |
.github/workflows/maven-project.yml |
Added image pull logic, registry authentication, and conditional builds; updated database initialization check to use log messages |
.devcontainer/db/scripts/populate_db.sh |
Added completion message to enable log-based initialization detection |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 4/5
- Safe to merge overall with minimal risk; the issue is a medium-severity caching concern rather than a functional code bug.
.github/workflows/container-images.ymlomits.devcontainer/development/config/shared/my.cnffrom the mariadb cache hash even though the Dockerfile copies it, so image rebuilds may not pick up config changes.- Pay close attention to
.github/workflows/container-images.yml- ensure the cache hash includes.devcontainer/development/config/shared/my.cnfto avoid stale images.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".github/workflows/container-images.yml">
<violation number="1" location=".github/workflows/container-images.yml:144">
P2: The mariadb cache hash omits `.devcontainer/development/config/shared/my.cnf`, even though the Dockerfile copies it. Changes to that config won’t trigger a rebuild, so the cached image can drift from the Dockerfile inputs. Include this path in the hash inputs used for `check_container`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The GHCR login steps in
maven-project.ymlrun unconditionally; consider guarding them (e.g. only on non-fork events or withcontinue-on-error) so that PRs from forks don’t fail purely because they can’t authenticate to the registry while still allowing the local build fallback to run. - The repeated 'Try to pull pre-built image from ghcr.io' logic across the Maven workflow jobs could be extracted into a reusable composite action or shared step to reduce duplication and make future updates to the pull‑first/fallback behavior easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The GHCR login steps in `maven-project.yml` run unconditionally; consider guarding them (e.g. only on non-fork events or with `continue-on-error`) so that PRs from forks don’t fail purely because they can’t authenticate to the registry while still allowing the local build fallback to run.
- The repeated 'Try to pull pre-built image from ghcr.io' logic across the Maven workflow jobs could be extracted into a reusable composite action or shared step to reduce duplication and make future updates to the pull‑first/fallback behavior easier to maintain.
## Individual Comments
### Comment 1
<location> `.github/workflows/container-images.yml:269-270` </location>
<code_context>
+ id: test
+ run: |
+ echo "Testing container starts correctly..."
+ docker run -d --name test-mariadb \
+ -e MYSQL_ROOT_PASSWORD=testpassword \
+ openo-mariadb-dev:test
+
</code_context>
<issue_to_address>
**issue (bug_risk):** MariaDB test uses a different root password than the init scripts expect, which will cause initialization to fail.
`populate_db.sh` assumes `mysql -u root -ppassword`, but this test starts the container with `MYSQL_ROOT_PASSWORD=testpassword`. With that mismatch, all `mysql -ppassword` calls will fail, the init script won’t reach the "Database initialization complete!" log, and the `initialized` wait loop will time out. Please either set `MYSQL_ROOT_PASSWORD=password` here or change the init scripts to use `$MYSQL_ROOT_PASSWORD` instead of a hard-coded value.
</issue_to_address>
### Comment 2
<location> `.github/workflows/maven-project.yml:68-73` </location>
<code_context>
+ - name: Checkout code
+ uses: actions/checkout@v4
+
+ - name: Log in to Container Registry
+ uses: docker/login-action@v3
+ with:
+ registry: ${{ env.REGISTRY }}
+ username: ${{ github.actor }}
+ password: ${{ secrets.GITHUB_TOKEN }}
+
+ - name: Check container freshness
</code_context>
<issue_to_address>
**suggestion:** The registry login + pull logic is duplicated across jobs and could be refactored to reduce maintenance overhead.
Build, test-modern, and test-legacy now repeat the same ghcr.io login, `openo-tomcat-dev:latest` pull, and cache/build fallback steps. Any future change to registry settings, image name, or pull logic would need to be updated in three places. Please consider centralizing this (e.g., reusable workflow, shared action, or YAML anchors) so the login/pull/fallback behavior is defined once.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Enhanced all "Try to pull" steps to capture docker pull output - Added GitHub Actions warnings when pulls fail to help troubleshoot auth/permission issues - Logs now distinguish between missing images vs authentication/permission errors - Uses ::warning:: annotations to make failures visible in workflow logs - Applied to all 4 pull operations: build job, modern-tests job, and both tomcat/mariadb in legacy-tests job Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".github/workflows/maven-project.yml">
<violation number="1" location=".github/workflows/maven-project.yml:95">
P3: The docker pull error output is redirected to a file and then deleted without being printed, so the workflow logs won’t show the real failure reason. Emit the captured stderr before removing the file to aid troubleshooting.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>
|
Hey @yingbull, I've posted a new review for you! |
fix: use exact-match grep for database verification in maven-project.yml
…istry Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>
fix: enable image push on manual workflow_dispatch with force_rebuild
|
@claude review |
|
@copilot review |
|
@sourcery-ai review |
|
Claude finished @yingbull's task in 1m 55s —— View job Code Review Progress
Code Review SummaryI've completed a comprehensive review of this PR. Overall, the implementation is solid and introduces a valuable optimization to the CI/CD pipeline. The hash-based container caching strategy is well-designed and the pull-first, build-fallback pattern is correctly implemented. Multiple sub-PRs have been merged to address issues identified by other reviewers. Most critical bugs have been fixed. However, one caching issue remains unresolved. Issues Already Fixed ✅The following issues were identified by other reviewers and have been successfully merged:
Remaining Issue
|
1 similar comment
|
@copilot fix the remaining my.cnf related hash issue that claude highlighted |
Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
fix: include my.cnf in mariadb-dev container hash calculation
|
@claude please give a final review. |
|
Claude finished @yingbull's task in 1m 39s —— View job Final Review Progress
✅ Final Review: Ready to MergeI've completed a comprehensive final review of PR #1796. This PR introduces a well-architected container image caching system that will significantly improve CI/CD performance. All previously identified issues have been successfully resolved through merged sub-PRs. Summary of ChangesThree files modified:
Issues Resolved ✅All issues identified by previous reviewers (Sourcery, Cubic, CodeRabbit, Copilot) have been successfully addressed through the following merged sub-PRs: PR #1798 - Improved error logging
PR #1799 - Password consistency
PR #1800 - Enhanced DB verification
PR #1801 & #1802 - Exact-match grep
PR #1804 - workflow_dispatch push support
PR #1806 - MariaDB hash includes my.cnf
Code Quality AssessmentStrengths ✅
Design Decisions Worth Noting
Compliance with CLAUDE.md✅ Workflow security - Appropriate permissions set for each job Minor Observations (Non-blocking)These are architectural decisions made by the team, documented for awareness:
Performance ImpactThis PR will deliver significant performance improvements:
Estimated time savings: 5-10 minutes per CI run when images are cached Security Considerations✅ Minimal attack surface:
✅ Audit trail:
Recommendation✅ APPROVED - Ready to merge This PR is production-ready and represents a significant improvement to the CI/CD infrastructure. All identified issues have been resolved, the implementation is robust, and the code quality is excellent. No blocking issues remain. The duplicate code in pull logic is acknowledged as future refactoring work, which is an acceptable trade-off for the immediate value this PR provides. What This EnablesOnce merged, this infrastructure enables:
|
Summary by Sourcery
Introduce a dedicated workflow to build, test, and publish reusable dev container images and update the Maven CI workflow to consume these images with a robust fallback to local builds and improved DB initialization checks.
Enhancements:
CI:
Summary by cubic
Adds a GitHub Actions workflow to build and cache dev container images with hash-based freshness, and updates the Maven CI to pull pre-built images with a local build fallback. This speeds up CI runs and makes database initialization checks more reliable.
New Features
Refactors
Written for commit 17d4a65. Summary will update on new commits.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.