fix(setup): increase nico-core helm install timeout to 600s#2995
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughThe ChangesNICo Core Helm Timeout Adjustment
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@helm-prereqs/setup.sh`:
- Line 438: The Helm install note is stale: setup.sh now uses a 600s timeout,
but NOTES.txt still tells operators to run the same helm upgrade --install nico
./helm command with --timeout 300s. Update the referenced note in
helm-prereqs/templates/NOTES.txt so it matches the new timeout used by setup.sh,
keeping the manual install instructions in sync with the behavior described by
helm install/upgrade.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e2aae19f-1965-4615-9db1-094112862333
📒 Files selected for processing (1)
helm-prereqs/setup.sh
| --set-string "global.image.repository=${NICO_IMAGE_REGISTRY}/nvmetal-carbide" | ||
| --set-string "global.image.tag=${NICO_CORE_IMAGE_TAG}" | ||
| --timeout 300s --wait | ||
| --timeout 600s --wait |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the matching Helm install note as well.
Line 438 now waits 600s, but helm-prereqs/templates/NOTES.txt:58-67 still tells operators to run the same helm upgrade --install nico ./helm command with --timeout 300s. That leaves the manual follow-up path stale and can reintroduce the original timeout failure outside setup.sh.
Suggested follow-up
- --timeout 300s --wait
+ --timeout 600s --waitAs per coding guidelines, "Keep OpenAPI specs, protobufs, database migrations, Helm manifests, generated code, and documentation in sync with the behavior they describe."
🤖 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 `@helm-prereqs/setup.sh` at line 438, The Helm install note is stale: setup.sh
now uses a 600s timeout, but NOTES.txt still tells operators to run the same
helm upgrade --install nico ./helm command with --timeout 300s. Update the
referenced note in helm-prereqs/templates/NOTES.txt so it matches the new
timeout used by setup.sh, keeping the manual install instructions in sync with
the behavior described by helm install/upgrade.
Source: Coding guidelines
nico-pxe boot-artifact init containers can take ~6 minutes to pull, exceeding the previous 300s timeout and causing the install to fail. Fixes: nvbug 6391901
7b3a00f to
2e5d458
Compare
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
nico-pxe boot-artifact init containers can take ~6 minutes to pull, exceeding the previous 300s timeout and causing the install to fail. Increased timeout to 600s.
Fixes: nvbug 6391901
Related issues
Type of Change
Breaking Changes
Testing
Additional Notes
nico-pxe observed taking ~5m50s to complete init containers in production. The previous 300s (5 min) hard timeout caused
helm upgrade --install nicoto fail before nico-pxe finished pulling boot-artifact images. 600s gives ~4 minutes of headroom.