feat(installer): add Spark express vLLM profile option - #8512
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (45)
📝 WalkthroughWalkthroughDGX Spark Express now supports automatic managed-vLLM selection and a fixed catalog-backed vLLM profile. Managed host-local vLLM uses validated bridge networking and separate inference and validation endpoints. Onboarding, sandbox sync, documentation, and physical E2E qualification were updated. ChangesDGX Spark vLLM onboarding and qualification
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-8512.docs.buildwithfern.com/nemoclaw |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
5 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
1 additional E2E selection from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
cv
left a comment
There was a problem hiding this comment.
Product scope and security review are incomplete. This draft adds a second DGX Spark Express option, fixed vLLM profile, installer behavior, public documentation, and a physical qualification target without a linked accepted issue or design decision. Record the decision that defines ownership, lifecycle, compatibility, security, and hardware validation for this supported surface, and complete the required sensitive-path review and exact DGX Spark evidence before marking the PR ready. Refresh onto current main and rerun all required checks afterward.
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
DGX Spark hardware evidence — final source treeTested on a physical DGX Spark with NVIDIA GB10 and 124,610 MiB RAM. The final hardware run used source commit OpenShell 0.0.101 passing scope
The earlier OpenShell 0.0.99 target passed 1/1 in 5m58s. A separate source-checkout onboarding on that version passed all eight phases, returned exactly Host-specific SSH findingThis machine has a pre-existing custom wildcard With those settings active, the OpenShell 0.0.101 run completed vLLM readiness and sandbox creation, but the primary dashboard forward was refused. Onboarding exited 1, the target stopped before inference assertions, and its exact-identity cleanup passed. A temporary first-match One earlier failed diagnostic relay attempt left a stale OpenShell PID record until that exact record was moved aside. Clean stops removed their records. Track that stale-record behavior separately with OpenShell; it does not establish a DGX Spark requirement or block this PR. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Make CLI tests independent of scheduler timing and host OpenShell installation state. Increase CLI coverage from 8 to 12 shards; the measured run model lowers the slowest shard from 586.3 seconds to 344.3 seconds while issue #8669 tracks the underlying slow tests. ## Related Issue Relates #8669. Follow-up to #8512. ## Changes - Inject a deterministic monotonic clock into the stale deadline-generation lifecycle-lock test. - Keep the stale-rebuild fixture on supported OpenShell 0.0.101 binaries and directly verify that rebuild does not invoke the installer. - Give the legacy Hermes compatibility fixture setup a 30-second hook timeout so an overloaded coverage shard does not fail Vitest’s default 10-second timeout. - Run PR and main CLI coverage with 12 shards, merge 12 reports, and report failures from every shard. - Exercise shard 12 as valid input, reject shard 13, and verify failed-shard links for shard 12. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: This changes internal CI parallelism and deterministic test fixtures only; no supported product surface changes. - [ ] 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: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: - The workflow changes only increase internal CLI coverage parallelism and update failed-shard validation. - The test changes make existing CI and lifecycle fixtures deterministic, including direct verification that rebuild does not invoke the OpenShell installer. - The legacy Hermes compatibility change only extends the heavy test-fixture setup timeout; it does not change production behavior. - No production command, configuration, user workflow, output contract, or supported product surface changes. - Agent: Codex Desktop <!-- docs-review-head-sha: 01dc6e2 --> <!-- docs-review-agents-blob-sha: c4923a3 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable; `scripts/prepare-dgx-station-host.sh` is unchanged. - Result: Not applicable - Supporting evidence: Not applicable ## Verification - [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 validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — the two formerly flaky files passed five runs, 225 test executions; the updated installer probe passed five additional runs; the workflow contract passed 5/5; the source-shape budget passed. The legacy Hermes compatibility suite passed 18/18 tests in each of five normal runs and 18/18 once with V8 coverage; that coverage command exited only on unrelated whole-repository coverage thresholds. - [ ] Applicable broad gate passed — `npm run check` passed every pre-commit check. Its CLI coverage lane completed 23,741 passing tests and found 27 unrelated DGX Spark host-state failures: an installed user service, a group-writable Codex workspace parent, and ARM64 Python executed in an amd64 container. An isolated HOME cleared all 24 service-related failures. - [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) - [ ] 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) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Expanded CLI coverage testing from 8 to 12 parallel shards. * Updated coverage validation and reporting to include all 12 shards. * Improved test reliability with deterministic timing and extended setup timeouts. * Verified rebuilds avoid unnecessary installation attempts and related messaging. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Completes the accepted DGX Spark Express option 2 capability in #8379:
127.0.0.1:8000and the current privateopenshell-dockerbridge gateway on port8000.https://inference.localwhile host validation remains on loopback.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/get-started/prerequisites.mdxdocs/inference/choose-local-inference-server.mdxdocs/inference/set-up-vllm-on-two-dgx-sparks.mdxdocs/inference/set-up-vllm.mdxdocs/reference/platform-support.mdxdocs/resources/prompt-assets/dgx-spark.mddocs/resources/starter-prompt.mdtest/e2e/README.mdVerification
64cf09fd3spark-express-vllmtarget: OpenShell 0.0.99 passed in 5m58s; final source tree with OpenShell 0.0.101 passed in 5m46sDGX Spark evidence
28cbe0e85088a22b4031298f39b812fbc0627b9d; final rebased PR head7f1d958562bbc2915704d551f97b1132991c62f2has the identical Git tree544ea15a168b5091c9b21437fb7036d4f600dfa8.127.0.0.1:8000and172.18.0.1:8000; no wildcard or additional binding was present.Host-specific SSH finding
This machine has a pre-existing custom wildcard
Host *SSH stanza that appliesCompression yes,ControlMaster auto, a socketControlPath, andControlPersist 600to the OpenShellsandboxhost. These are machine-specific settings, not DGX Spark defaults. With them active, both OpenShell 0.0.99 and 0.0.101 failed the primary dashboard forward. A temporary first-matchHost sandboxoverride set compression, multiplexing, and persistence off; the final OpenShell 0.0.101 target then passed onboarding, runtime checks, inference, denial, and cleanup. The original SSH file was restored byte-for-byte.One earlier failed diagnostic relay attempt left stale OpenShell PID state until the exact record was moved aside. Track that behavior separately with OpenShell. It does not establish a DGX Spark requirement or block this PR's source-checkout qualification.
Signed-off-by: Aaron Erickson aerickson@nvidia.com
Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation