Skip to content

fix(openbao-migrations): sleep between health check retries#294

Open
mesutoezdil wants to merge 2 commits into
NVIDIA:mainfrom
mesutoezdil:mesutoezdil/fix/openbao-healthcheck-retry-sleep
Open

fix(openbao-migrations): sleep between health check retries#294
mesutoezdil wants to merge 2 commits into
NVIDIA:mainfrom
mesutoezdil:mesutoezdil/fix/openbao-healthcheck-retry-sleep

Conversation

@mesutoezdil

@mesutoezdil mesutoezdil commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

TL;DR

The OpenBao health check loop logged "Retrying in 5 seconds" but never slept, so all 5 attempts ran within milliseconds and slow startups failed the migration Job. Adds the missing sleep. bash -n passes; entrypoint script with no test harness.

Issues

Closes #293

Summary by CodeRabbit

  • Bug Fixes
    • Added an explicit wait between OpenBao /v1/sys/health retry attempts.
    • Updated migration warning text formatting when failures are permitted, while keeping the same migration outcomes and exit behavior.

@mesutoezdil
mesutoezdil requested a review from a team as a code owner July 21, 2026 09:12
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9581821f-2a55-4876-8789-fa685bb72995

📥 Commits

Reviewing files that changed from the base of the PR and between 1bab501 and 96d97f4.

📒 Files selected for processing (1)
  • migrations/openbao/entrypoint.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • migrations/openbao/entrypoint.sh

📝 Walkthrough

Walkthrough

The OpenBao migration entrypoint now pauses between failed health-check attempts. It also clarifies migration failure comments and updates the warning emitted when failures are allowed.

Changes

OpenBao migration handling

Layer / File(s) Summary
Retry and failure handling
migrations/openbao/entrypoint.sh
Adds the configured retry delay to the health-check loop, clarifies downstream failure comments, and updates the allowed-failure warning wording.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: max-nv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits and accurately describes the main fix in the diff.
Linked Issues check ✅ Passed The added sleep directly addresses issue #293 by spacing out OpenBao health check retries.
Out of Scope Changes check ✅ Passed The remaining edits are minor wording/comment tweaks in the same migration script and stay within scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@migrations/openbao/entrypoint.sh`:
- Line 66: Add shell-level coverage for the retry loop surrounding the sleep
command in the entrypoint script, verifying that consecutive failed health
checks are separated by RETRY_DELAY; use the repository-native test runner to
execute it before committing, or document in the PR why such timing coverage is
infeasible.
🪄 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: 33fc496a-2240-418e-8aaf-6eef4d9a8737

📥 Commits

Reviewing files that changed from the base of the PR and between 5ef95ed and fab42f0.

📒 Files selected for processing (1)
  • migrations/openbao/entrypoint.sh

Comment thread migrations/openbao/entrypoint.sh
@kristinapathak
kristinapathak requested a review from Max-NV July 21, 2026 17:12
@Max-NV
Max-NV requested a review from nvjmcnamee July 21, 2026 17:43
@mesutoezdil
mesutoezdil force-pushed the mesutoezdil/fix/openbao-healthcheck-retry-sleep branch from fab42f0 to 1bab501 Compare July 21, 2026 17:51
The retry loop logged Retrying in 5 seconds but never slept, so all
5 attempts ran within milliseconds. On a slow OpenBao startup the
loop fell through while the service was still down and the raft
list-peers call failed the migration Job. Add the missing sleep.
Also replace two em dashes in comments and log text to satisfy the
ASCII-only style rule.

Closes NVIDIA#293

Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
@mesutoezdil
mesutoezdil force-pushed the mesutoezdil/fix/openbao-healthcheck-retry-sleep branch from 1bab501 to 1255dff Compare July 21, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openbao migrations: health check retry loop never sleeps

1 participant