-
-
Notifications
You must be signed in to change notification settings - Fork 383
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
Add support to create Docker ARM64 images #1532
base: main
Are you sure you want to change the base?
Conversation
@edoren is attempting to deploy a commit to the rallly Team on Vercel. A member of the Team first needs to authorize it. |
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces significant enhancements to Docker image build workflows across two GitHub Actions configuration files. The primary improvements focus on multi-platform image building, supporting both Changes
Sequence DiagramsequenceDiagram
participant Build as Build Job
participant QEMU as QEMU Setup
participant Buildx as Docker Buildx
participant DockerHub as Docker Hub
participant Merge as Merge & Publish Job
Build->>QEMU: Setup multi-arch support
Build->>Buildx: Configure build environment
Build->>DockerHub: Build and push platform-specific images
Build-->>Merge: Export image digests
Merge->>DockerHub: Login
Merge->>Buildx: Reconfigure
Merge->>Merge: Download digests
Merge->>DockerHub: Create manifest list
Merge->>DockerHub: Push manifest
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (2)
.github/workflows/docker-image-manual.yml (2)
66-66
: Remove trailing whitespace.There's unnecessary trailing whitespace on this line.
- +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 66-66: trailing spaces
(trailing-spaces)
Line range hint
1-120
: Consider implementing workflow reuse.Both workflow files share significant common configuration. Consider:
- Creating a reusable workflow for the shared Docker build and publish logic
- Calling this workflow from both the manual and version release workflows
Would you like me to help create a reusable workflow that both files can reference?
🧰 Tools
🪛 actionlint (1.7.4)
109-109: property "extractgitbranch" is not defined in object type {}
(expression)
🪛 YAMLlint (1.35.1)
[error] 66-66: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/docker-image-manual.yml
(3 hunks).github/workflows/docker-image-version-release.yml
(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/docker-image-manual.yml
20-20: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
109-109: property "extractgitbranch" is not defined in object type {}
(expression)
.github/workflows/docker-image-version-release.yml
23-23: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 YAMLlint (1.35.1)
.github/workflows/docker-image-manual.yml
[error] 66-66: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.github/workflows/docker-image-version-release.yml (5)
7-9
: LGTM! Good practice centralizing the Docker repository path.The introduction of
DOCKERHUB_REPO
environment variable improves maintainability and follows security best practices by using secrets.
13-24
: Verify the availability of the ARM runner.The matrix strategy for multi-platform builds is well-structured, but there's a potential issue:
- The runner label
ubuntu-24.04-arm
is not a standard GitHub-hosted runner.Please confirm:
- Is this a self-hosted runner?
- If yes, ensure the runner is properly configured in your GitHub organization/repository settings.
- If no, consider using the standard GitHub-hosted ARM runners (e.g.,
ubuntu-latest
withruns-on: ['self-hosted', 'linux', 'arm64']
).🧰 Tools
🪛 actionlint (1.7.4)
23-23: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
47-64
: LGTM! Well-structured multi-platform build configuration.The setup properly implements:
- QEMU for cross-platform builds
- Docker Buildx for multi-architecture support
- Digest-based pushing for manifest lists
65-78
: LGTM! Robust digest handling implementation.The digest export and upload process is well-implemented with:
- Proper error handling
- Secure temporary directory usage
- Reasonable artifact retention period
79-120
: LGTM! Comprehensive merge and publish implementation.The job effectively:
- Merges digests from multiple platforms
- Creates and pushes the manifest list
- Includes image inspection for verification
- Handles semantic versioning tags appropriately
with: | ||
images: | | ||
${{ env.DOCKERHUB_REPO }} | ||
tags: ${{ steps.extractGitBranch.outputs.branchName }} |
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.
Fix undefined property reference.
The steps.extractGitBranch.outputs.branchName
reference might fail as the extractGitBranch
step is not defined in this context.
You need to add the Get branch name
step before the Docker meta
step in the merge-and-publish
job:
+ - name: Get branch name
+ id: extractGitBranch
+ run: echo "branchName=$(echo ${GITHUB_REF#refs/heads/})" >> $GITHUB_OUTPUT
+
- name: Docker meta
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.4)
109-109: property "extractgitbranch" is not defined in object type {}
(expression)
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.
@edoren I think this is valid. The extractGitBranch
is running in the build
job. I think we just need to move it to merge-and-publish
?
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
I'll need to take a closer look, but I think we can drop the qemu stuff entirely. The whole point running on native arm runners is that we don't have to run in emulation. |
Thanks for letting me know about the issue with QEMU, it was actually not needed as both runners we already using the appropriate architecture to build, I removed that job. Time related from what I've seen it was increased a bit but not something drastic, all the builds I executed took about 5:30 mins: |
Description
Hello! This adds support to use the new ARM runners to create Docker images
https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/
Also I added some changes to support creating a multiplatform image as explained here so it can share the same tag, it will look something like this in Dockerhub:
https://hub.docker.com/repository/docker/edoren/rallly/tags
Checklist
Please check off all the following items with an "x" in the boxes before requesting a review.