-
Notifications
You must be signed in to change notification settings - Fork 497
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
chore(wren-ai-service): multi arch build #1189
Conversation
WalkthroughThe pull request introduces comprehensive updates to three GitHub Actions workflow files for AI service image releases. The changes focus on enhancing multi-architecture build capabilities by implementing a matrix strategy that supports building Docker images for Changes
Possibly related PRs
Suggested reviewers
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: 0
🧹 Nitpick comments (5)
.github/workflows/ai-service-release-nightly-image.yaml (3)
20-27
: Add documentation about runner requirements.While the matrix strategy is well-implemented, it would be helpful to add a comment explaining that the
linux_arm64_runner
needs to be configured in the repository settings.strategy: fail-fast: false matrix: + # Note: linux_arm64_runner must be configured in repository settings + # as a self-hosted runner with ARM64 architecture arch: - runner: ubuntu-latest platform: linux/amd64 - runner: linux_arm64_runner platform: linux/arm64
44-47
: Fix potential word splitting issue in platform preparation.The platform preparation step should use quotes to prevent word splitting.
- name: Prepare platform run: | platform=${{ matrix.arch.platform }} - echo "PLATFORM_PAIR=${platform//\//-}" >> $GITHUB_ENV + echo "PLATFORM_PAIR=${platform//\//\-}" >> $GITHUB_ENV🧰 Tools
🪛 actionlint (1.7.4)
45-45: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
96-100
: Fix word splitting issues in manifest creation.The manifest creation step should properly quote variables and command substitutions.
- name: Create manifest list and push working-directory: /tmp/digests run: | TAGS=$(echo "${{ steps.meta.outputs.tags }}" | awk '{printf "--tag %s ", $0}') - docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.WREN_AI_SERVICE_IMAGE }}@sha256:%s ' *) \ - $TAGS + docker buildx imagetools create "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ + "$(printf '${{ env.WREN_AI_SERVICE_IMAGE }}@sha256:%s ' *)" \ + "$TAGS"🧰 Tools
🪛 actionlint (1.7.4)
96-96: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
96-96: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
96-96: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ai-service-release-image.yaml (1)
Line range hint
41-46
: Enhance tag name validation.Consider adding validation for the tag name input to ensure it follows semantic versioning or your preferred format.
if [ -n "${{ github.event.inputs.tag_name }}" ]; then + # Validate tag format (e.g., v1.2.3 or 1.2.3) + if ! echo "${{ github.event.inputs.tag_name }}" | grep -qE '^v?\d+\.\d+\.\d+'; then + echo "Error: Tag must follow semantic versioning format (e.g., v1.2.3 or 1.2.3)" + exit 1 + fi tag_name=${{ github.event.inputs.tag_name }} else tag_name=commit-$(git log -1 --pretty=%h) fi🧰 Tools
🪛 actionlint (1.7.4)
50-50: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ai-service-release-stable-image.yaml (1)
Line range hint
47-71
: Enhance changelog generation error handling.The changelog generation could be more resilient by handling empty commit logs and providing clearer feedback.
echo "Previous version: $PREVIOUS_VERSION" + +# Ensure the previous version tag exists +if ! git rev-parse "$PREVIOUS_VERSION" >/dev/null 2>&1; then + echo "Warning: Previous version tag not found, using first commit" + PREVIOUS_VERSION=$(git rev-list --max-parents=0 HEAD) +fi # Get the change log from the previous version to the current HEAD -# If there is no change log, the command will return an error, so we use || true to ignore the error -CHANGE_LOG=$(git log --pretty=format:"%s" $PREVIOUS_VERSION..HEAD | grep wren-ai-service || true) +CHANGE_LOG=$(git log --pretty=format:"%s" "$PREVIOUS_VERSION"..HEAD | grep -E "^(feat|fix|chore).*wren-ai-service" || true) +# Check if changelog is empty +if [ -z "$CHANGE_LOG" ]; then + echo "Warning: No conventional commits found for changelog" + CHANGE_LOG="- No changes documented" +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ai-service-release-image.yaml
(2 hunks).github/workflows/ai-service-release-nightly-image.yaml
(1 hunks).github/workflows/ai-service-release-stable-image.yaml
(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ai-service-release-stable-image.yaml
108-108: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
159-159: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
159-159: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
159-159: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ai-service-release-image.yaml
50-50: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ai-service-release-nightly-image.yaml
39-39: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
45-45: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
96-96: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
96-96: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
96-96: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: validate-pull-request-title
- GitHub Check: validate-pull-request-title
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
.github/workflows/ai-service-release-nightly-image.yaml (1)
9-10
: LGTM! Environment variable configuration is clean and consistent.The
WREN_AI_SERVICE_IMAGE
environment variable is well-defined and follows GitHub Actions best practices..github/workflows/ai-service-release-stable-image.yaml (1)
84-85
: Consider dynamic tag name for stable releases.The stable image workflow hardcodes the tag name as 'latest', which might not be ideal for stable releases.
Consider using the version input as the tag name:
outputs: - tag_name: latest + tag_name: ${{ github.event.inputs.version }}✅ Verification successful
The review comment is correct - stable releases should use version-specific tags.
Using a hardcoded 'latest' tag for stable releases makes it difficult to track which version is deployed and doesn't follow container image tagging best practices. The suggested change to use the version input is appropriate since this is a stable release workflow where the version is already required and used consistently throughout other parts of the workflow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # View the complete workflow file cat .github/workflows/ai-service-release-stable-image.yaml # Search for any references to 'latest' tag in workflow files rg "latest" .github/workflows/ -A 3 -B 3 # Check for README or documentation about releases fd "README" --type f --exec cat {}Length of output: 51728
a521dc6
to
f4d5b42
Compare
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: 2
♻️ Duplicate comments (2)
.github/workflows/ai-service-release-image.yaml (1)
101-105
:⚠️ Potential issueFix shell script quoting issues to prevent word splitting.
Similar to the nightly workflow, the script needs proper quoting.
🧰 Tools
🪛 actionlint (1.7.4)
101-101: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ai-service-release-stable-image.yaml (1)
159-163
:⚠️ Potential issueFix shell script quoting issues to prevent word splitting.
Similar to other workflows, the script needs proper quoting.
🧰 Tools
🪛 actionlint (1.7.4)
159-159: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
159-159: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
159-159: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ai-service-release-image.yaml
(2 hunks).github/workflows/ai-service-release-nightly-image.yaml
(1 hunks).github/workflows/ai-service-release-stable-image.yaml
(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ai-service-release-image.yaml
50-50: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ai-service-release-stable-image.yaml
108-108: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
159-159: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
159-159: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
159-159: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ai-service-release-nightly-image.yaml
39-39: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
45-45: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
96-96: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
96-96: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
96-96: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
.github/workflows/ai-service-release-nightly-image.yaml (2)
9-11
: LGTM! Good practice using environment variables.Using an environment variable for the image name improves maintainability and reduces the risk of inconsistencies.
20-28
: LGTM! Well-structured matrix strategy for multi-arch builds.The matrix strategy is well-designed:
- Supports both amd64 and arm64 architectures
- Uses appropriate runners for each architecture
- Includes fail-fast configuration for better error handling
.github/workflows/ai-service-release-image.yaml (1)
Line range hint
41-46
: LGTM! Good handling of optional tag input.The tag preparation logic correctly handles both user-provided and auto-generated tags.
🧰 Tools
🪛 actionlint (1.7.4)
50-50: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2046:warning:2:33: Quote this to prevent word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2046:warning:3:3: Quote this to prevent word splitting
(shellcheck)
101-101: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ai-service-release-stable-image.yaml (1)
Line range hint
41-77
: LGTM! Well-structured changelog generation.The changelog generation logic is well-implemented:
- Properly categorizes changes into features and fixes
- Handles cases where no changes are found
- Maintains good formatting for readability
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.
lgtm
tested using release build, build time is shortened from 8-9 minutes to 2-3 minutes
https://github.com/Canner/WrenAI/actions/runs/12838438473
Summary by CodeRabbit
WREN_AI_SERVICE_IMAGE
for flexible image naming.