-
-
Notifications
You must be signed in to change notification settings - Fork 84
feat: optimize CI/CD with parallel builds, BuildKit cache mounts, and registry caching #583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis PR modernizes the container build infrastructure by splitting the API and View build workflows, transitioning from local layer caching to registry-based caching, introducing cross-compilation support and multi-stage builds to the API Dockerfile, optimizing BuildKit cache mounts, and adding a .dockerignore file for the API. Changes
Sequence DiagramsequenceDiagram
participant GH as GitHub Actions
participant BX as Buildx
participant REG as Container Registry
Note over GH: Old Flow
GH->>BX: Build API/View
BX->>BX: Use local /tmp/.buildx-cache
BX->>REG: Push image
GH->>GH: Move cache (manual)
Note over GH: New Flow
GH->>GH: Split into 2 jobs
rect rgb(200, 220, 255)
Note over GH: build-and-push-api
GH->>BX: Setup with driver-opts
BX->>REG: cache-from registry:buildcache-api
BX->>BX: Multi-stage build + cross-compile
BX->>BX: Cache mounts for /go/pkg/mod
BX->>REG: Push image + cache-to registry:buildcache-api
end
rect rgb(200, 220, 255)
Note over GH: build-and-push-view
GH->>REG: Authenticate
BX->>REG: cache-from registry:buildcache-view
BX->>BX: Cache mounts for yarn + .next
BX->>REG: Push image + cache-to registry:buildcache-view
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
@coderabbitai: Review the changes, update the title and description |
🧩 Analysis chain🏁 Script executed: Length of output: 10595 @zhravan: I'll review the changes and update the PR title and description. Review SummaryThis PR implements excellent build optimizations focused on parallelization and caching strategies. The changes are well-structured and follow Docker/BuildKit best practices. Key Improvements:1. Workflow Parallelization ✅
2. Advanced Caching Strategy ✅
3. BuildKit Optimization ✅
4. Build Context Optimization ✅
5. Cross-Compilation Support ✅
Minor Observations:
Suggested Title:Suggested Description:#### Issue
Optimizes Docker build performance in CI/CD pipeline
#### Description
This PR significantly improves build performance through parallelization and advanced caching strategies:
- **Parallel Builds**: Split API and View builds into separate jobs that run concurrently
- **BuildKit Cache Mounts**: Leverage BuildKit's cache mount feature for Go modules, build cache, and Yarn cache
- **Registry Caching**: Migrate from local filesystem cache to registry-based caching for better persistence
- **Concurrency Control**: Prevent duplicate workflow runs with automatic cancellation
- **Build Context Optimization**: Add .dockerignore to reduce API build context size
- **Cross-Compilation**: Add proper TARGETOS/TARGETARCH support for multi-platform builds
#### Scope of Change
- [x] Infra / Deployment
#### Additional Notes for Reviewers
- BuildKit cache layers are stored in the container registry with `:buildcache` tag
- First run after this merge will populate the cache; subsequent runs will see performance improvements
- Both jobs require `packages: write` permission to push cache layersI'll now update the PR and trigger a review of the changes. 🏁 Script executed: Length of output: 298 I'm unable to update the PR title and description directly due to workflow restrictions. However, I've provided the suggested title and description above. You can update them manually: Suggested Title: To update manually:
I'll proceed with the review of the changes now. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build_container.yml (1)
27-135: Consider refactoring to reduce job duplication.The build-and-push-api (lines 27–81) and build-and-push-view (lines 82–135) jobs are nearly identical. To improve maintainability and reduce drift, consider:
- Reusable workflow: Move shared steps into
.github/workflows/build-and-push.ymland call it from a matrix in the main workflow.- Matrix strategy: Use a single job with a matrix over
image_type: [api, view]and parameterize image name, context, build-args, etc.Example (matrix):
jobs: build-and-push: strategy: matrix: image: - { name: api, context: ./api, args: API_PORT=${{ env.API_PORT }} } - { name: view, context: ./view, args: NEXT_PUBLIC_PORT=${{ env.NEXT_PUBLIC_PORT }} } runs-on: ubuntu-latest steps: # ... shared steps using ${{ matrix.image.* }} ...This reduces duplication, makes future changes easier, and is easier to test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build_container.yml(5 hunks)api/.dockerignore(1 hunks)api/Dockerfile(1 hunks)view/Dockerfile(2 hunks)
🔇 Additional comments (11)
api/.dockerignore (1)
1-20: Solid .dockerignore configuration.Comprehensive exclusion of build artifacts, version control, IDE configuration, test files, and logs. This will effectively reduce build context and improve build speed. No concerns.
view/Dockerfile (1)
5-6: BuildKit cache mounts correctly applied.Cache mounts on yarn install and build are properly configured for standard cache paths (/usr/local/share/.cache/yarn, /app/.next/cache). This aligns with the registry-based caching strategy introduced in the workflow and will improve build performance on repeated builds.
Also applies to: 18-19
api/Dockerfile (4)
3-4: Multi-stage build with solid caching strategy.Cross-compilation args (TARGETOS, TARGETARCH) are properly declared and passed to make. Separating go mod download with a dedicated cache mount and dual cache mounts during build (go module cache + go build cache) follow BuildKit best practices and should significantly improve build times on cache hits.
Also applies to: 10-12, 18-20
28-30: Runtime directory setup is well-configured.Copying migrations and templates from builder and establishing restricted-permission directories (/etc/nixopus/{ssh,docker-certs,configs,db} all 700) follows security best practices for storing sensitive config and credentials.
Also applies to: 32-39
24-24: docker-cli installation is justified; add explanatory documentation.The API service is a deployment orchestration tool that manages Docker containers (pull, run, stop operations) and builds images. The docker-cli installation is necessary and intentional. Docker socket is mounted across all environments (docker-compose.yml, docker-compose-staging.yml, docker-compose-dev.yml) and DOCKER_HOST is configured accordingly.
However, add a comment in the Dockerfile documenting why docker-cli is required and acknowledging the security implications of mounting the Docker socket, as this is an important architectural decision:
# docker-cli and bash are required for deployment/orchestration functionality. # Docker socket must be mounted at runtime; ensure proper access controls and review security posture. RUN apk add --no-cache ca-certificates bash docker-cli
18-20: No action needed — the cross-compilation approach is correct.The Makefile's
buildtarget properly leverages cross-compilation. When environment variables likeGOOSandGOARCHare passed on the make command line (e.g.,GOOS=… GOARCH=… make build), they become available to the shell commands executed within the Makefile target. Thego buildcommand natively respects these environment variables without requiring explicit exports in the Makefile. The current setup will work correctly for cross-compilation as intended..github/workflows/build_container.yml (5)
15-17: Concurrency configuration prevents build queue and orphaned runs.The concurrency group uses workflow + ref to deduplicate builds on the same branch/tag, and cancel-in-progress ensures re-pushes don't queue. Appropriate for a release-triggered workflow.
27-27: Job split enables true parallelization of API and View builds.Separating build-and-push-api and build-and-push-view into independent jobs allows them to run concurrently without blocking. Both jobs have identical structure (checkout, buildx setup, registry login, metadata extraction, build/push), which is acceptable for maintainability and independence.
Also applies to: 82-96
47-49: BuildKit driver options properly configured.Setting
image=moby/buildkit:latestensures the latest BuildKit version with security patches and new features.network=hostallows the builder to access the host network for efficient registry pulls and multi-platform builds.Also applies to: 102-104
79-80: Registry-based caching replaces local cache dependency.Transitioning from local /tmp/.buildx-cache to registry-based caching (cache-from/cache-to with buildcache tag and mode=max) provides:
- Persistence across runs: Cache survives CI environment resets.
- Cross-machine efficiency: Distributed agents share the same cache.
- Cleaner CI state: No local filesystem state to manage.
This aligns well with the cache mount optimizations in both Dockerfiles.
Also applies to: 134-135
40-41: No issues found—.env.sample is properly committed and available for both workflow jobs.Verification confirms:
.env.sampleis tracked and committed in git- File exists at repo root with appropriate defaults
- Both workflow jobs (lines 40-41 and 95-96) correctly reference it for copying to
api/.envandview/.envThe workflow will not fail silently due to missing
.env.sample.
|
@coderabbitai: review the change and find the bugs |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 12072 @zhravan: I've completed a thorough bug analysis. I found 2 critical bugs and 2 moderate issues that will cause runtime problems: 🐛 Critical Bugs1. Invalid Go Version in api/Dockerfile (Line 1)Issue: FROM golang:1.25-alpine AS builderImpact: Docker build will fail with "manifest unknown" error. Fix: Use a valid Go version: FROM golang:1.23-alpine AS builder2. Missing ARG in view/Dockerfile Runner Stage (Lines 23-25)Issue: The FROM node:18-alpine AS runner
WORKDIR /app
ENV NODE_ENV=production
ENV NEXT_TELEMETRY_DISABLED=1
ENV PORT=$NEXT_PUBLIC_PORT # ❌ Empty - ARG not defined in this stage
ENV NEXT_PUBLIC_PORT=$NEXT_PUBLIC_PORT # ❌ Empty - ARG not defined in this stageImpact: The application will start with empty Fix: Redeclare the ARG in the runner stage: FROM node:18-alpine AS runner
ARG NEXT_PUBLIC_PORT=7443
WORKDIR /app
ENV NODE_ENV=production
ENV NEXT_TELEMETRY_DISABLED=1
ENV PORT=$NEXT_PUBLIC_PORT
ENV NEXT_PUBLIC_PORT=$NEXT_PUBLIC_PORT
|
Issue
Link to related issue(s):
Description
Short summary of what this PR changes or introduces.
Scope of Change
Select all applicable areas impacted by this PR:
Screenshot / Video / GIF (if applicable)
Attach or embed screenshots, screen recordings, or GIFs demonstrating the feature or fix.
Related PRs (if any)
Link any related or dependent PRs across repos.
Additional Notes for Reviewers (optional)
Anything reviewers should know before testing or merging (e.g., environment variables, setup steps).
Developer Checklist
To be completed by the developer who raised the PR.
Reviewer Checklist
To be completed by the reviewer before merge.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.