fix(installer): explain failed OpenIB service - #7152
Conversation
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughStation preparation now emits targeted remediation guidance when ChangesOpenIB remediation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/install-station-openibd-remediation.test.ts (1)
23-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExercise the supported
--applyboundary.
checkFailedUnit()sources the script and callscheck_failed_unitsdirectly, bypassing thebash "$helper" --applypath used byscripts/install.shat Lines 3492-3550. Add at least one test that invokes the script through its supported entrypoint so argument handling and orchestration regressions cannot pass unnoticed.As per path instructions, tests should validate observable behavior through the public boundary rather than lock onto implementation details.
🤖 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 `@test/install-station-openibd-remediation.test.ts` around lines 23 - 25, Extend the tests around check_failed_units to invoke the script through its supported entrypoint with --apply, rather than only sourcing SCRIPT_UNDER_TEST and calling the helper directly. Assert the observable remediation behavior and retain any direct-helper coverage only where needed, ensuring argument parsing and orchestration are exercised through the public boundary.Source: Path instructions
🤖 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 `@test/install-station-openibd-remediation.test.ts`:
- Line 48: Update the assertions for both spawnSync results in the OpenIBD
remediation test to explicitly require result.error to be undefined and
result.status to be non-null before asserting the exit status is nonzero. Apply
this to the assertions near the existing openibd.result.status checks,
preserving the output context in failures.
---
Nitpick comments:
In `@test/install-station-openibd-remediation.test.ts`:
- Around line 23-25: Extend the tests around check_failed_units to invoke the
script through its supported entrypoint with --apply, rather than only sourcing
SCRIPT_UNDER_TEST and calling the helper directly. Assert the observable
remediation behavior and retain any direct-helper coverage only where needed,
ensuring argument parsing and orchestration are exercised through the public
boundary.
🪄 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: 83b6bb34-6fef-481d-9be6-312db11ea636
📒 Files selected for processing (2)
scripts/prepare-dgx-station-host.shtest/install-station-openibd-remediation.test.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: None This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
left a comment
There was a problem hiding this comment.
Approved at exact head 3dc37d6a48c58bc4826b6d402586d071945626aa against current base fce12723d10fcd9ba91da1e8d57704b542aad9e6 after maintainer review. The change remains fail-closed: it adds bounded remediation guidance for a failed openibd.service, does not disable or ignore services, does not mutate systemd/network/driver state, and preserves the metadata-only --force-station-install boundary. Focused tests passed 57/57 locally; exact-head CI reports 43 successes, 6 intentional skips, and no pending or failing checks; all five commits are GitHub Verified; CodeRabbit and both review-advisor lanes are clear; and no review threads remain unresolved. The local maintainer helper has a confirmed fork-association false negative because GitHub returns pull_requests: [] for these otherwise exact-head runs; lead-admin authorization accepts that tooling exception for this revision.
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical `docs/changelog/2026-07-18.mdx` release-prep entry with the exact `## v0.0.88` heading. The entry summarizes every user-visible change on `main` since v0.0.87 and links each release theme to the focused user documentation. ## Changes - Add one parser-safe dated changelog entry for v0.0.88 covering DGX Station preparation, inference health, multi-gateway sandbox operations and recovery, onboarding policy defaults, and rebuild credential reuse. - Reconcile the changelog against the merged v0.0.88-labeled PRs and the complete `v0.0.87..origin/main` commit range. - Source mapping: - [#7152](#7152) -> `docs/changelog/2026-07-18.mdx`: Document RDMA-aware OpenIB service remediation during DGX Station preparation. - [#7155](#7155) -> `docs/changelog/2026-07-18.mdx`: Document stopped-container preservation and fail-closed restart-policy boundaries. - [#7158](#7158) -> `docs/changelog/2026-07-18.mdx`: Document bounded packaged CDI refresh for the exact AI Developer Tools Station profile. - [#7074](#7074) -> `docs/changelog/2026-07-18.mdx`: Document authenticated upstream model probes and precise route-reachability claims. - [#7007](#7007) -> `docs/changelog/2026-07-18.mdx`: Document the explicit serving-process health gap in `status` and `doctor`. - [#7113](#7113) -> `docs/changelog/2026-07-18.mdx`: Document owning-gateway selection for sandbox-scoped status and exec operations. - [#7092](#7092) -> `docs/changelog/2026-07-18.mdx`: Document idempotent recovery for target-owned active port forwards. - [#7133](#7133) -> `docs/changelog/2026-07-18.mdx`: Document web-search-aware policy preset defaults during onboarding. - [#7129](#7129) -> `docs/changelog/2026-07-18.mdx`: Document gateway-registered web-search credential reuse during rebuild preflight. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog contract, exact release heading, and parser-safe MDX structure. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run test/changelog-docs.test.ts` passed 6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — completed successfully with 0 errors and 2 existing Fern warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) — not applicable because native changelog entries use the required parser-safe MDX SPDX comment without frontmatter. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added improved DGX Station preparation workflows. * Enhanced sandbox status and diagnostic reporting for inference health. * Improved state selection and recovery across multiple gateways. * Added safer onboarding defaults for web search policies. * Improved rebuild preflight handling for credential reuse and fail-closed behavior. * **Documentation** * Added release notes for version 0.0.88. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Summary
Station preparation currently reports only a generic failed-unit error when
openibd.serviceblocks installation. This change preserves the fail-closed gate while explaining that NemoClaw does not require RDMA and giving host owners precise disable-or-repair remediation without changing systemd or networking state.Related Issue
Fixes #7151
Changes
openibd.serviceis an unqualified failed unit.Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run test/install-station-openibd-remediation.test.ts test/install-station-host-preparation.test.ts(2 files, 57 tests passed)npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Senthil Ravichandran senthilr@nvidia.com
Summary by CodeRabbit
openibd.service, including conditional next steps based on whether RDMA networking is used.openibd.servicefailures and confirms unrelated failures don’t trigger OpenIB-specific guidance.