ci: Create per-platform Dockerfile for proper Docker image layer caching #3747
Conversation
📝 WalkthroughWalkthroughSwaps Docker Hub targets and credentials to ALCO-specific repos/secrets and adds a CI step that creates a per-platform temporary Dockerfile (path exported via Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Prep as Dockerfile Prep Step
participant Build as Docker Buildx (per-platform)
participant DH as Docker Hub
GH->>Prep: run per-platform step\ncreate temporary Dockerfile\nexport SYNC_SERVICE_DOCKERFILE
GH->>Build: invoke buildx with\nfile=SYNC_SERVICE_DOCKERFILE\nbuild per-platform images
Build->>DH: login using ALCO secrets\npush images (per-platform / multi-arch)
DH-->>Build: acknowledge push
Build-->>GH: report status
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3747 +/- ##
===========================================
+ Coverage 75.75% 87.41% +11.66%
===========================================
Files 11 23 +12
Lines 693 2011 +1318
Branches 171 528 +357
===========================================
+ Hits 525 1758 +1233
- Misses 167 251 +84
- Partials 1 2 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Note to self: need to test it before merging by temporarily substituting my own Docker Hub repo and confirmed the caching behaviour by creating a few runs of the workflow. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/sync_service_dockerhub_image.yml:
- Around line 168-169: Replace the temporary Docker Hub credentials used in the
workflow (the username and password fields referencing
secrets.DOCKERHUB_USERNAME_ALCO and DOCKERHUB_TOKEN_ALCO) with the production
secrets; update the username and password entries to point to the canonical
production secret names (the same ones used in the build job) so the workflow
uses the proper production DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets before
merging.
- Around line 114-115: Replace the temporary personal Docker Hub secrets used in
the workflow: change the username and password values that reference
DOCKERHUB_USERNAME_ALCO and DOCKERHUB_TOKEN_ALCO back to the production secrets
(restore the original secret names used for CI/CD). Edit the username/password
entries in the sync_service_dockerhub_image.yml workflow (the lines providing
username: and password:) to reference the correct production secret identifiers
instead of DOCKERHUB_USERNAME_ALCO and DOCKERHUB_TOKEN_ALCO.
- Around line 26-27: The DOCKERHUB_REPO and DOCKERHUB_CANARY_REPO environment
variables currently point to a personal account; change them back to the
official production repositories by replacing DOCKERHUB_REPO: alco/electric with
DOCKERHUB_REPO: electricsql/electric and DOCKERHUB_CANARY_REPO:
alco/electric-canary with DOCKERHUB_CANARY_REPO: electricsql/electric-canary
(update the values wherever DOCKERHUB_REPO or DOCKERHUB_CANARY_REPO appear in
the workflow file).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/sync_service_dockerhub_image.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build_and_push_image (linux/arm64/v8, arm64, blacksmith-4vcpu-ubuntu-2404-arm)
- GitHub Check: Test packages/typescript-client w/ sync-service
- GitHub Check: Test packages/experimental w/ sync-service
- GitHub Check: Test packages/start w/ sync-service
- GitHub Check: Test packages/react-hooks w/ sync-service
- GitHub Check: Test packages/y-electric w/ sync-service
- GitHub Check: Run Lux integration tests
🔇 Additional comments (2)
.github/workflows/sync_service_dockerhub_image.yml (2)
117-130: Core caching solution looks good.The approach of prepending a platform-specific
# blacksmith-cache-scopecomment creates unique Dockerfile paths per platform, which should give Blacksmith separate sticky disks for layer caching. The use ofset -euo pipefailensures failures are properly caught.
137-137: Correctly references the per-platform Dockerfile.The build step properly consumes the
SYNC_SERVICE_DOCKERFILEenvironment variable set in the previous step, ensuring each platform build uses its unique Dockerfile path.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| DOCKERHUB_REPO: alco/electric | ||
| DOCKERHUB_CANARY_REPO: alco/electric-canary |
There was a problem hiding this comment.
Revert temporary Docker Hub repositories before merging.
These point to a personal Docker Hub account (alco/*) instead of the official electricsql/* repositories. Per the commit message, this is intentionally temporary for testing. Ensure these are reverted to the production values before merge.
- DOCKERHUB_REPO: alco/electric
- DOCKERHUB_CANARY_REPO: alco/electric-canary
+ DOCKERHUB_REPO: electricsql/electric
+ DOCKERHUB_CANARY_REPO: electricsql/electric-canary📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DOCKERHUB_REPO: alco/electric | |
| DOCKERHUB_CANARY_REPO: alco/electric-canary | |
| DOCKERHUB_REPO: electricsql/electric | |
| DOCKERHUB_CANARY_REPO: electricsql/electric-canary |
🤖 Prompt for AI Agents
In @.github/workflows/sync_service_dockerhub_image.yml around lines 26 - 27, The
DOCKERHUB_REPO and DOCKERHUB_CANARY_REPO environment variables currently point
to a personal account; change them back to the official production repositories
by replacing DOCKERHUB_REPO: alco/electric with DOCKERHUB_REPO:
electricsql/electric and DOCKERHUB_CANARY_REPO: alco/electric-canary with
DOCKERHUB_CANARY_REPO: electricsql/electric-canary (update the values wherever
DOCKERHUB_REPO or DOCKERHUB_CANARY_REPO appear in the workflow file).
| username: ${{ secrets.DOCKERHUB_USERNAME_ALCO }} | ||
| password: ${{ secrets.DOCKERHUB_TOKEN_ALCO }} |
There was a problem hiding this comment.
Revert temporary credentials before merging.
These use personal secrets (DOCKERHUB_USERNAME_ALCO, DOCKERHUB_TOKEN_ALCO) instead of the production credentials. Revert to the original secrets.
- username: ${{ secrets.DOCKERHUB_USERNAME_ALCO }}
- password: ${{ secrets.DOCKERHUB_TOKEN_ALCO }}
+ username: ${{ secrets.DOCKERHUB_USERNAME }}
+ password: ${{ secrets.DOCKERHUB_TOKEN }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| username: ${{ secrets.DOCKERHUB_USERNAME_ALCO }} | |
| password: ${{ secrets.DOCKERHUB_TOKEN_ALCO }} | |
| username: ${{ secrets.DOCKERHUB_USERNAME }} | |
| password: ${{ secrets.DOCKERHUB_TOKEN }} |
🤖 Prompt for AI Agents
In @.github/workflows/sync_service_dockerhub_image.yml around lines 114 - 115,
Replace the temporary personal Docker Hub secrets used in the workflow: change
the username and password values that reference DOCKERHUB_USERNAME_ALCO and
DOCKERHUB_TOKEN_ALCO back to the production secrets (restore the original secret
names used for CI/CD). Edit the username/password entries in the
sync_service_dockerhub_image.yml workflow (the lines providing username: and
password:) to reference the correct production secret identifiers instead of
DOCKERHUB_USERNAME_ALCO and DOCKERHUB_TOKEN_ALCO.
| username: ${{ secrets.DOCKERHUB_USERNAME_ALCO }} | ||
| password: ${{ secrets.DOCKERHUB_TOKEN_ALCO }} |
There was a problem hiding this comment.
Revert temporary credentials before merging.
Same as the build job—revert to production secrets.
- username: ${{ secrets.DOCKERHUB_USERNAME_ALCO }}
- password: ${{ secrets.DOCKERHUB_TOKEN_ALCO }}
+ username: ${{ secrets.DOCKERHUB_USERNAME }}
+ password: ${{ secrets.DOCKERHUB_TOKEN }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| username: ${{ secrets.DOCKERHUB_USERNAME_ALCO }} | |
| password: ${{ secrets.DOCKERHUB_TOKEN_ALCO }} | |
| username: ${{ secrets.DOCKERHUB_USERNAME }} | |
| password: ${{ secrets.DOCKERHUB_TOKEN }} |
🤖 Prompt for AI Agents
In @.github/workflows/sync_service_dockerhub_image.yml around lines 168 - 169,
Replace the temporary Docker Hub credentials used in the workflow (the username
and password fields referencing secrets.DOCKERHUB_USERNAME_ALCO and
DOCKERHUB_TOKEN_ALCO) with the production secrets; update the username and
password entries to point to the canonical production secret names (the same
ones used in the build job) so the workflow uses the proper production
DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets before merging.
|
The first manual test has shown that this "fix" doesn't solve the problem. |
Summary
Problem
When building multi-platform Docker images (amd64 and arm64), both platform builds use the same Dockerfile path. Blacksmith's sticky disk caching keys on the Dockerfile path, causing the caches to overwrite each other. Only one platform benefits from caching in subsequent builds.
Evidence from run #21174363588
This run happened immediately after run #21173980401 where both platforms had successfully built. Despite both caches being freshly populated, only one platform could use its cache:
amd64 build (cache HIT) — most build steps are CACHED:
arm64 build (cache MISS) — expensive steps like
mix deps.compilerun from scratch:The arm64 build had to run
mix deps.compile(~60s) while amd64 got it from cache.Solution
Create a unique Dockerfile path per platform by prepending a cache scope comment:
This ensures Blacksmith treats each platform's build cache independently.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.