ci: switch release workflow to github runners #2346
Conversation
WalkthroughThe pull request updates the GitHub Actions workflow for building and publishing Docker images. It modifies the runner configurations to use specific operating systems and versions, replacing previous self-hosted definitions. Additionally, a new step is introduced in the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/release-docker-image.yml (1)
Line range hint
137-144: Enhance manifest creation step robustness and securityWhile the implementation is functional, consider these improvements:
- Add error handling for manifest creation
- Verify digest signatures
- Clean up temporary files
Suggested enhancement:
- name: Create manifest list and push working-directory: ${{ env.DIGEST_DIR_PATH }} run: | + set -euo pipefail ls -lah + # Verify all digests are present + expected_count=2 # amd64 and arm64 + actual_count=$(ls | wc -l) + if [ "$actual_count" -ne "$expected_count" ]; then + echo "Error: Expected $expected_count digests, found $actual_count" + exit 1 + fi docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ $(printf '${{ inputs.image_org }}/${{ inputs.image_name }}@sha256:%s ' *) + # Verify manifest creation + if ! docker buildx imagetools inspect ${{ inputs.image_org }}/${{ inputs.image_name }}:${{ steps.docker_meta.outputs.version }} > /dev/null; then + echo "Error: Failed to verify manifest" + exit 1 + fi + # Cleanup + rm -rf ${{ env.DIGEST_DIR_PATH }}/*🧰 Tools
🪛 actionlint (1.7.3)
140-140: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
140-140: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/release-docker-image.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/release-docker-image.yml (1)
Line range hint 1-144: Verify secrets access for GitHub-hosted runners
Since we're switching from self-hosted to GitHub-hosted runners, please verify that all required secrets are properly configured with appropriate permissions.
✅ Verification successful
Required secrets are properly referenced for GitHub-hosted runners
The workflow correctly references all necessary secrets for both AWS and Docker Hub authentication:
- AWS credentials:
AWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY,AWS_REGION - Docker Hub credentials:
DOCKERHUB_USERNAME,DOCKERHUB_TOKEN
These secrets are used with appropriate actions:
- AWS credentials with
aws-actions/configure-aws-credentials@v4 - Docker Hub credentials with
docker/login-action@v3
The transition to GitHub-hosted runners doesn't affect the secrets usage as they are properly scoped and referenced.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all required secrets are referenced in the workflow
echo "Required secrets:"
rg -o '\$\{\{\s*secrets\.[A-Za-z_]+\s*\}\}' | sort -u | sed 's/.*secrets\.\([A-Za-z_]*\).*/\1/'
echo -e "\nVerify these secrets are configured in repository settings:"
gh api repos/{owner}/{repo}/actions/secrets --jq '.secrets[].name'
Length of output: 560
Script:
#!/bin/bash
# Let's extract the required secrets directly from the workflow file
echo "Required secrets in the workflow:"
rg -o '\$\{\{\s*secrets\.[A-Za-z_]+\s*\}\}' .github/workflows/release-docker-image.yml | sort -u | sed 's/.*secrets\.\([A-Za-z_]*\).*/\1/'
Length of output: 303
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
Dockerfile (1)
Line range hint
1-700: Consider optimizing the build process furtherThe Dockerfile implementation is solid, but consider these optimizations:
- Use BuildKit's
--mount=type=cachefor npm/yarn cache in JavaScript builds- Implement multi-platform builds using
buildxfor better ARM64 support- Consider using
cache-fromandcache-tofor distributed cachingThese changes would improve build performance and cross-platform support.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Dockerfile(3 hunks)
🧰 Additional context used
🪛 Hadolint (2.12.0)
Dockerfile
[error] 427-427: COPY with more than 2 arguments requires the last argument to end with /
(DL3021)
🔇 Additional comments (1)
Dockerfile (1)
Line range hint 335-426: LGTM! Well-structured build stage with proper caching
The build stage implementation is solid with:
- Proper cargo registry and git caching
- Correct environment variable handling
- Appropriate build profile and feature flag management
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
Dockerfile (1)
Line range hint
1-700: Consider architectural improvements for better build performanceThe Dockerfile follows good practices but could benefit from the following improvements:
- Consider using BuildKit's
--mount=type=cachefor npm/yarn cache in the JavaScript build stages- The TODO comment about removing unnecessary workspace packages should be addressed to reduce build time and image size
- Consider implementing health checks for the services
Example implementation for health checks:
# Add to drive-abci stage HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ CMD nc -z localhost 26658 || exit 1 # Add to dapi stage HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ CMD nc -z localhost 2510 || exit 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Dockerfile(2 hunks)
🔇 Additional comments (1)
Dockerfile (1)
321-321:
Fix undefined $RELEASE variable in cargo chef prepare command
The $RELEASE variable is used but not defined, which could lead to build failures.
Apply this diff to fix the issue:
-RUN source $HOME/.cargo/env && \
- source /root/env && \
- cargo chef prepare $RELEASE --recipe-path recipe.json
+RUN source $HOME/.cargo/env && \
+ source /root/env && \
+ if [[ "${CARGO_BUILD_PROFILE}" == "release" ]] ; then \
+ cargo chef prepare --release --recipe-path recipe.json; \
+ else \
+ cargo chef prepare --recipe-path recipe.json; \
+ fiLikely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.github/workflows/release-docker-image.yml (2)
Line range hint
141-147: Add error handling for manifest creationWhile the manifest list creation is well-implemented, consider adding error handling to ensure the process fails gracefully if any issues occur.
- name: Create manifest list and push working-directory: ${{ env.DIGEST_DIR_PATH }} run: | ls -lah + if [ -z "$(ls -A)" ]; then + echo "Error: No digests found in directory" + exit 1 + fi docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ $(printf '${{ inputs.image_org }}/${{ inputs.image_name }}@sha256:%s ' *) || { + echo "Error: Failed to create manifest list" + exit 1 + }🧰 Tools
🪛 actionlint (1.7.3)
144-144: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
144-144: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
Line range hint
141-147: Consider adding manifest cleanupTo prevent accumulation of unused manifests over time, consider adding a cleanup step for old manifests after successful creation of new ones.
Example cleanup approach:
- List existing manifests older than X days
- Remove unused manifests
- Document the cleanup policy
Would you like me to provide a detailed implementation for this cleanup process?
🧰 Tools
🪛 actionlint (1.7.3)
144-144: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
144-144: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/release-docker-image.yml(3 hunks)
🔇 Additional comments (2)
.github/workflows/release-docker-image.yml (2)
40-43: LGTM! Good choice of runners
Using the same Ubuntu runner for both architectures ensures build environment consistency. The setup correctly defines the matrix for multi-architecture builds.
36-36: Consider investigating the need for such a long timeout
A 120-minute timeout is quite long for a build job. While cross-platform builds can be time-consuming, it would be worth investigating if there are any optimization opportunities in the build process.
Consider:
- Implementing build caching
- Optimizing the Dockerfile layers
- Running builds in parallel where possible
✅ Verification successful
120-minute timeout is justified for multi-architecture builds
The timeout value is appropriate given the workflow configuration:
- Builds for multiple architectures (linux/amd64 and linux/arm64)
- Uses QEMU for ARM64 emulation
- Includes artifact upload/download and manifest creation steps
- Performs complex Docker operations including cross-platform builds
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check recent workflow runs to analyze build times
gh run list --workflow=release-docker-image.yml --limit 5 --json durationInMinutes,conclusion \
| jq '.[] | {duration: .durationInMinutes, status: .conclusion}'
Length of output: 403
Script:
#!/bin/bash
# Check recent workflow runs with correct fields
gh run list --workflow=release-docker-image.yml --limit 5 --json startedAt,conclusion,updatedAt \
| jq '.[] | {
duration: (
if (.startedAt != null and .updatedAt != null) then
((sub("(?<date>[0-9]{4}-[0-9]{2}-[0-9]{2})T(?<time>[0-9]{2}:[0-9]{2}:[0-9]{2}).*"; "\(.date) \(.time)") | fromdate) -
(sub("(?<date>[0-9]{4}-[0-9]{2}-[0-9]{2})T(?<time>[0-9]{2}:[0-9]{2}:[0-9]{2}).*"; "\(.date) \(.time)") | fromdate)) / 60
else
null
end
),
status: .conclusion
}'
# Also check if there are any multi-platform build configurations
rg -l 'platform.*linux/(amd64|arm64)' --type yaml
Length of output: 584
Script:
#!/bin/bash
# Let's check the workflow file directly for platform configurations and build steps
cat .github/workflows/release-docker-image.yml
# Also check for any Docker build commands or platform specifications
rg "docker buildx" -A 5 -B 2 --type yaml
Length of output: 5354
Issue being fixed or feature implemented
Release workflow is still using self-hosted runners.
What was done?
How Has This Been Tested?
Releasing from this branch
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Summary by CodeRabbit
linux/arm64.cargo chef.