-
Notifications
You must be signed in to change notification settings - Fork 77
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
use ci-container for update deps cron job #2744
Conversation
Signed-off-by: vankichi <[email protected]>
📝 WalkthroughWalkthroughThe pull request introduces modifications to the GitHub Actions workflow file Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
🧹 Outside diff range and nitpick comments (2)
.github/workflows/update-deps.yaml (2)
Line range hint
78-85
: Fix the version comparison syntaxThere's a syntax error in the version comparison that could cause the label selection to fail.
Apply this fix:
- if ${LATEST} eq "1.7"; then + if [ "${LATEST}" = "1.7" ]; thenAdditionally, consider using semantic version comparison for more robust version handling:
- if [ "${LATEST}" = "1.7" ]; then + if printf '%s\n' "1.7" "${LATEST}" | sort -V | head -n1 | grep -q "^1.7$"; then
Line range hint
46-61
: Add error handling for make commandsThe sequence of make commands could benefit from better error handling to ensure each step completes successfully.
Consider wrapping the commands in a shell function with error handling:
run: | + set -eo pipefail + + run_make() { + echo "Running: make $1" + if ! make "$1"; then + echo "::error::Failed to run make $1" + return 1 + fi + } + - make files - make update - make helm/schema/all + run_make files + run_make update + run_make helm/schema/all # ... apply to other make commands
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/update-deps.yaml
(2 hunks)
🔇 Additional comments (2)
.github/workflows/update-deps.yaml (2)
57-57
: Clarify the necessity of the license update step
The comment suggests uncertainty about whether make license
is needed. This should be definitively decided to maintain consistency across dependency updates.
#!/bin/bash
# Check if license files are being modified by the make target
git log --oneline -p | grep -B 5 "make license" | grep -B 5 "diff --git.*LICENSE"
# Check if the license make target is used in other workflows
fd -t f . .github/workflows -x grep -l "make license" {}
27-33
: Verify the CI container workflow and image availability
The changes introduce a dependency on a reusable workflow and a dynamic container image. We should ensure:
- The reusable workflow
_detect-ci-container.yaml
exists and is properly configured - The container image tag format is correct and the images are available
✅ Verification successful
CI container workflow setup is properly configured
The reusable workflow _detect-ci-container.yaml
exists and is well-configured with:
- A proper workflow_call interface that outputs the container TAG
- A robust tag detection mechanism using custom actions
- Integration with multiple workflows across the repository
The container image reference in update-deps.yaml
correctly uses the output from the reusable workflow:
image: ghcr.io/vdaas/vald/vald-ci-container:${{ needs.detect-ci-container.outputs.TAG }}
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the reusable workflow exists
if [ -f ".github/workflows/_detect-ci-container.yaml" ]; then
echo "Reusable workflow exists"
cat ".github/workflows/_detect-ci-container.yaml"
else
echo "Warning: Reusable workflow not found"
fi
# Check recent container images
gh api \
-H "Accept: application/vnd.github+json" \
"/orgs/vdaas/packages/container/vald-ci-container/versions" \
--jq '.[0:5] | .[] | {created_at: .created_at, tags: .metadata.container.tags}'
Length of output: 672
Script:
#!/bin/bash
# Let's check if the reusable workflow exists and its content
if [ -f ".github/workflows/_detect-ci-container.yaml" ]; then
echo "=== Reusable workflow content ==="
cat ".github/workflows/_detect-ci-container.yaml"
else
echo "Reusable workflow not found"
fi
# Let's also check the update-deps.yaml for any container image references
echo -e "\n=== Container image references in update-deps.yaml ==="
grep -A 2 -B 2 "container:" .github/workflows/update-deps.yaml
# Check for any other workflow files that might reference the container
echo -e "\n=== Other workflow files with container references ==="
fd -t f . .github/workflows -x grep -l "vald-ci-container" {}
Length of output: 3337
[CHATOPS:HELP] ChatOps commands.
|
Signed-off-by: vankichi <[email protected]>
Signed-off-by: vankichi <[email protected]> Co-authored-by: Kiichiro YUKAWA <[email protected]> Co-authored-by: Yusuke Kato <[email protected]>
Signed-off-by: vankichi <[email protected]>
Signed-off-by: vankichi <[email protected]>
Description
SSIA
Related Issue
Versions
Checklist
Special notes for your reviewer
Summary by CodeRabbit
New Features
Improvements
These changes streamline the update process and improve the overall efficiency of the CI workflow.