fix(nmxc-browser): resolve NMX-C endpoint by rack ID as well. - #4475
Closed
rtamma-nv wants to merge 2 commits into
Closed
fix(nmxc-browser): resolve NMX-C endpoint by rack ID as well.#4475rtamma-nv wants to merge 2 commits into
rtamma-nv wants to merge 2 commits into
Conversation
Add rack_id as an alternative to chassis_serial in NmxcBrowseRequest. When rack_id is provided the endpoint is resolved from the rack's ready control-plane switch NVOS IP via resolve_nmx_c_endpoint_url, removing the requirement for a nvlink_nmxc_endpoints row. Signed-off-by: Roopesh Tamma <rtamma@nvidia.com>
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Summary by CodeRabbit
WalkthroughNMX-C browsing now accepts either a chassis serial or rack ID. CLI and web inputs validate the selectors. The API resolves rack endpoints through the ready control-plane switch NVOS IP. ChangesNMX-C rack endpoint browsing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant nmxc_browser
participant nmxc_browse
participant resolve_nmx_c_endpoint_url
Browser->>nmxc_browser: Submit chassis serial or rack ID
nmxc_browser->>nmxc_browse: Send validated selector
nmxc_browse->>resolve_nmx_c_endpoint_url: Resolve NMX-C endpoint
resolve_nmx_c_endpoint_url-->>nmxc_browse: Return endpoint URL
nmxc_browse-->>nmxc_browser: Return browse result
nmxc_browser-->>Browser: Render query result
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-08-01 09:38:34 UTC | Commit: de7bc75 |
![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 `@crates/api-core/src/handlers/nmxc_browse.rs`:
- Around line 155-171: Add table-driven tests for the selector validation logic
in nmxc_browse, covering rack-only, chassis-only, both-provided, and
neither-provided inputs. Use the existing scenarios! or check_cases pattern, and
assert the expected ManagedHostGroupType or validation error for each case.
In `@crates/rpc/proto/forge.proto`:
- Around line 8037-8045: Update the field comments for chassis_serial and
rack_id to document that the request must provide at least one of these fields,
while preserving the existing mutual-exclusion and endpoint-resolution
descriptions.
🪄 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: 4bdba87d-9d62-4bac-b681-284a23cc0db4
📒 Files selected for processing (7)
crates/admin-cli/src/browse/nmxc/args.rscrates/admin-cli/src/browse/nmxc/cmd.rscrates/admin-cli/src/machine/nvlink_info/cmd.rscrates/api-core/src/handlers/nmxc_browse.rscrates/api-web/src/nmxc_browser.rscrates/api-web/templates/nmxc_browser.htmlcrates/rpc/proto/forge.proto
Signed-off-by: Roopesh Tamma <rtamma@nvidia.com>
10 tasks
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.
Currently, NMX-C browser expects chassis_serial->nmx_c_endpoint mapping to be available in nvlink_nmx_c_endpoint table. In sites where there are ingested racks, nmx_c_endpoint table is not required to be populated.
Add rack_id as an alternative to chassis_serial in NmxcBrowseRequest. When rack_id is provided the endpoint is resolved from the rack's ready control-plane switch NVOS IP via resolve_nmx_c_endpoint_url, removing the requirement for a nvlink_nmxc_endpoints row.
Related issues
](#4474)
Type of Change
Breaking Changes
Testing
Additional Notes