test(helm): Add single-node kind cluster set-up script for chart validation. - #419
test(helm): Add single-node kind cluster set-up script for chart validation.#41920001020ycx wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughChangesHelm test cluster setup
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SetUpTest
participant Kind
participant Helm
participant Kubectl
User->>SetUpTest: provide image arguments
SetUpTest->>Kind: create named cluster
SetUpTest->>Helm: install chart with image values
SetUpTest->>Kubectl: wait for pods to become Ready
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
5f09a5c to
73a0750
Compare
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/deployment/spider-helm/set-up-test.sh`:
- Around line 28-31: The image loading currently occurs inside
get_image_helm_args command substitutions, allowing failures to be ignored
before helm install. In tools/deployment/spider-helm/.set-up-common.sh lines
36-37, keep get_image_helm_args limited to emitting Helm --set flags; in
tools/deployment/spider-helm/set-up-test.sh lines 28-31, add a checked
image-loading preflight before helm install and abort if any kind load
docker-image operation fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 391b122d-78b8-4e89-a506-fbdecb4ed286
📒 Files selected for processing (4)
tools/deployment/spider-helm/.helmignoretools/deployment/spider-helm/.set-up-common.shtools/deployment/spider-helm/Chart.yamltools/deployment/spider-helm/set-up-test.sh
| helm install "${RELEASE_NAME}" "${script_dir}" \ | ||
| $(get_image_helm_args "${CLUSTER_NAME}" "storage" "${STORAGE_IMAGE}") \ | ||
| $(get_image_helm_args "${CLUSTER_NAME}" "scheduler" "${SCHEDULER_IMAGE}") \ | ||
| $(get_image_helm_args "${CLUSTER_NAME}" "worker" "${WORKER_IMAGE}") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
fd -a '(\.set-up-common\.sh|set-up-test\.sh)$' . | sed 's#^\./##'
echo "== target files line counts =="
wc -l tools/deployment/spider-helm/.set-up-common.sh tools/deployment/spider-helm/set-up-test.sh
echo "== relevant target files =="
cat -n tools/deployment/spider-helm/.set-up-common.sh
printf '\n--- set-up-test.sh ---\n'
cat -n tools/deployment/spider-helm/set-up-test.sh
echo "== helpers references =="
rg -n "get_image_helm_args|load docker-image|kind load|helm install|set -|set -e|get_image" tools/deployment/spider-helm/.set-up-common.sh tools/deployment/spider-helm/set-up-test.shRepository: y-scope/spider
Length of output: 7540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== bash command-substitution exit-status probe =="
bash -c 'set -o errexit; set -o pipefail; f() { echo x >/dev/null; false >&2; echo y; }; echo "start"; x=$(f); echo "x=$x"; echo "outdone" || echo "cmd exited non-zero; final status=$?"'
echo "== check bash availability and version =="
command -v bash
bash --version | head -n1Repository: y-scope/spider
Length of output: 338
Separate checked image loading from Helm argument generation. A failed kind load docker-image inside command substitution is not a script error and will not stop the surrounding helm install from running.
- Move the image-load step into a checked preflight before
helm install. - Keep
get_image_helm_args()limited to emitting the Helm--setflags.
📍 Affects 2 files
tools/deployment/spider-helm/set-up-test.sh#L28-L31(this comment)tools/deployment/spider-helm/.set-up-common.sh#L36-L37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/deployment/spider-helm/set-up-test.sh` around lines 28 - 31, The image
loading currently occurs inside get_image_helm_args command substitutions,
allowing failures to be ignored before helm install. In
tools/deployment/spider-helm/.set-up-common.sh lines 36-37, keep
get_image_helm_args limited to emitting Helm --set flags; in
tools/deployment/spider-helm/set-up-test.sh lines 28-31, add a checked
image-loading preflight before helm install and abort if any kind load
docker-image operation fails.
697b4a2 to
d4c99f0
Compare
sitaowang1998
left a comment
sitaowang1998
left a comment
There was a problem hiding this comment.
We also need a explicit cleanup script/task.
| # @param {string} component Image component name (e.g., "storage", "scheduler", "worker") | ||
| # @param {string} [image] Docker image (e.g., "spider-worker:dev") | ||
| # @return Prints helm --set flags to stdout | ||
| get_image_helm_args() { |
There was a problem hiding this comment.
The function name should describe that it loads the images into kind cluster.
| #!/usr/bin/env bash | ||
|
|
||
| # Single-node kind cluster set-up for testing the Helm chart | ||
| # TODO: Submit a job through the deployed stack once an end-to-end test scenario is available. |
There was a problem hiding this comment.
Should we start the client submitting the job in the bash script? More generally, how much of work should we put in bash script, and how much should we put in taskfiles?
2 participants
Description
This PR adds a single-node kind cluster set-up flow for validating the Helm chart, mirroring the structure of CLP's
tools/deployment/package-helm/set-up scripts.Note for reviewer:
CrashLoopBackOffrestarts before converging. We have thoroughly discuss this behaviour offline and concludes this is a better behaviour rather thewaitforsemantic implemented in CLP.Checklist
breaking change.
Validation performed
./tools/deployment/spider-helm/set-up-test.shpassedSummary by CodeRabbit
New Features
Chores