fix(setup): retry vault status and unseal during first-run init#3680
Open
shayan1995 wants to merge 1 commit into
Open
fix(setup): retry vault status and unseal during first-run init#3680shayan1995 wants to merge 1 commit into
shayan1995 wants to merge 1 commit into
Conversation
Contributor
|
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)
Summary by CodeRabbit
WalkthroughThe Vault setup script polls validated status JSON before proceeding and uses configurable multi-round unsealing. Pods already unsealed are skipped, while sealed pods receive all keys repeatedly until unsealed or the configured limit is exhausted. ChangesVault initialization and unsealing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SetupScript
participant Kubectl
participant VaultPod
SetupScript->>Kubectl: Poll vault status
Kubectl->>VaultPod: Execute status query
VaultPod-->>Kubectl: Return validated status JSON
Kubectl-->>SetupScript: Report initialized and sealed state
SetupScript->>Kubectl: Submit unseal keys
Kubectl->>VaultPod: Apply keys 1 through 3
VaultPod-->>SetupScript: Return updated sealed state
SetupScript->>Kubectl: Retry key sequence if still sealed
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/unseal_vault.sh`:
- Around line 34-36: Validate VAULT_STATUS_RETRIES and VAULT_UNSEAL_ROUNDS
immediately after their defaults are assigned and before either value is passed
to seq; require positive integers, including rejecting zero, negatives, and
malformed strings. On invalid input, exit with a clear message identifying the
offending variable and expected format, while leaving VAULT_STATUS_SLEEP_SECONDS
behavior unchanged.
- Line 94: Update the unseal retry loop surrounding _vault_status_json and the
unseal submission commands so a failed kubectl exec vault operator unseal is
caught instead of terminating the script under set -e. Break the current
share-processing round on failure, then recheck Vault status before starting the
next retry round.
🪄 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: 26bf7fc1-deaa-4d9d-a743-f83e42d056b8
📒 Files selected for processing (1)
helm-prereqs/unseal_vault.sh
On a fresh cluster, vault pods reach the Initialized condition before the Vault listener accepts connections, so the single-shot 'vault status' in unseal_vault.sh (with stderr suppressed and its exit code discarded) could return empty output and abort setup.sh with "Unable to retrieve Vault status from vault-0" even though the pods were healthy. Re-running setup.sh on the same cluster succeeded, as reported in NVIDIA#3129. Changes: - Add _vault_status_json: retries 'vault status' (tunable via VAULT_STATUS_RETRIES / VAULT_STATUS_SLEEP_SECONDS, default 36x5s) and validates the JSON payload before use, tolerating vault's nonzero exit codes for sealed/uninitialized states. On exhaustion it prints the last stdout/stderr instead of a generic message. - Fix a silent failure in unseal_pod: a failed status probe previously produced empty JSON and the pod was skipped as "already unsealed"; it now fails loudly. - Retry the full unseal key sequence (VAULT_UNSEAL_ROUNDS, default 3) and verify the pod actually reports unsealed, since the unseal nonce can reset mid-sequence and discard submitted key shares (visible in the issue's logs). A failed unseal exec within a round logs and falls through to the status check instead of aborting under set -e. - Validate the retry tunables as positive integers up front and document them in the script header. Verified on a fresh 3-node cluster (dev6): first-run init + unseal completed and setup.sh proceeded past phase [4/6]. Fixes NVIDIA#3129 Signed-off-by: Shayan Namaghi <snamaghi@nvidia.com>
shayan1995
force-pushed
the
snamaghi/issue-3129-vault-status-retry
branch
from
July 17, 2026 20:51
ac25d0c to
0b5b200
Compare
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On a fresh cluster, Vault pods reach the
Initializedcondition before Vault's listener accepts connections, so the single-shotvault statusinhelm-prereqs/unseal_vault.sh(stderr suppressed, exit code discarded) could return empty output and abortsetup.shwith "Unable to retrieve Vault status from vault-0" even though the pods were healthy. Re-runningsetup.shon the same cluster succeeded, as reported in #3129.Changes:
_vault_status_jsonhelper retriesvault status(tunable viaVAULT_STATUS_RETRIES/VAULT_STATUS_SLEEP_SECONDS, default 36×5s) and validates the JSON payload (.initialized/.sealedbooleans) before use, tolerating Vault's nonzero exit codes for sealed/uninitialized states. On exhaustion it prints the last stdout/stderr instead of a generic message.unseal_pod: a failed status probe previously produced empty JSON and the pod was skipped as "already unsealed"; it now fails loudly.VAULT_UNSEAL_ROUNDS, default 3) and the pod is verified to actually report unsealed, since the unseal nonce can reset mid-sequence and discard submitted key shares (visible in the issue's logs).Related issues
Fixes #3129
Type of Change
Breaking Changes
Testing
bash -nclean. (TODO before submitting: re-run setup.sh on a fresh 3-node cluster matching the issue's repro to confirm first-run init now succeeds; the script paths were reviewed but not executed against a live vault.)