feat(scale-up-fabric): support RMS v2 fabric manager configuration#4158
Open
jayzhudev wants to merge 1 commit into
Open
feat(scale-up-fabric): support RMS v2 fabric manager configuration#4158jayzhudev wants to merge 1 commit into
jayzhudev wants to merge 1 commit into
Conversation
jayzhudev
added
the
rack lifecycle
Contributor
Summary by CodeRabbit
WalkthroughThe change adds selectable RMS V1/V2 scale-up fabric manager workflows. V2 submits rack-wide asynchronous jobs, polls completion, validates returned fabric and service status, persists results, and extends certificate configuration with explicit ChangesRMS v2 fabric manager workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MaintenanceHandler
participant CertificateWorkflow
participant FabricWorkflow
participant RMS
participant StatusVerification
MaintenanceHandler->>CertificateWorkflow: submit V2 certificate jobs
CertificateWorkflow->>FabricWorkflow: transition after completion
FabricWorkflow->>RMS: submit rack-wide fabric configuration
RMS-->>FabricWorkflow: return asynchronous job_id
FabricWorkflow->>RMS: poll job status
RMS-->>FabricWorkflow: return completion state
FabricWorkflow->>StatusVerification: verify completed configuration
StatusVerification->>RMS: retrieve fabric and switch statuses
RMS-->>StatusVerification: return observed statuses
StatusVerification-->>MaintenanceHandler: persist status and complete maintenance
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
jayzhudev
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
crates/api-core/src/tests/rack_state_controller/handler.rs (1)
3681-3722: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCompare the returned node IDs, not just counts. The node payloads include IDs, so these assertions should compare the sorted IDs from each request against
switch_ids; count-only checks still pass for duplicated or wrong switches. Apply the same check tostatus_nodesandfabric_manager_status_nodes.🤖 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 `@crates/api-core/src/tests/rack_state_controller/handler.rs` around lines 3681 - 3722, The assertions in the test should validate node identities rather than only lengths. In the test block containing configure_nodes, status_nodes, and fabric_manager_status_nodes, extract each payload’s node IDs, sort them, and compare them with the sorted switch_ids for all three requests while preserving the existing request-count assertions.crates/rack-controller/src/maintenance.rs (1)
1704-1714: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the RMS client-resolution preamble.
This "prefer the NMX-configure client, fall back to the service client" sequence now appears verbatim at lines 1554-1563 and here, alongside the older
if let/elsevariants at lines 536-553, 2698-2714 and 2845-2861. A single helper returning the resolved client (with theArcclone where the caller must outlive a later&mut ctxborrow, as this call site requires for line 1785) would collapse five near-copies and make the lifetime distinction deliberate rather than incidental.Entirely optional — the current code is correct as written.
🤖 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 `@crates/rack-controller/src/maintenance.rs` around lines 1704 - 1714, Extract the repeated RMS client-resolution logic into a shared helper, including NMX-configured client preference and service-client fallback. Update the variants around this preamble and the older if-let branches to use the helper, preserving an owned Arc clone where callers such as the current maintenance flow need the client to outlive a later mutable ctx borrow.crates/rack-controller/src/fabric_manager.rs (1)
671-733: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpress this as a
check_valuestable rather than a hand-rolled loop.
observe_primary_casealready reduces the fallible call to a totalPrimaryObservation, so this is a textbook input→output mapping.check_valuesis imported at line 465 and used bytest_fabric_manager_status_from_entryin this same module; aligning keeps scenario reporting uniform across the suite.As per coding guidelines: "Use table-driven tests for functions mapping inputs to outputs or errors. Use
scenarios!withOutcomefor fallible operations,value_scenarios!for total operations, and directcheck_cases/check_valueswhen macros obscure intent."♻️ Proposed refactor to the established table-driven form
#[test] fn observed_primary_switch_classifies_status_responses() { let second_id = SwitchId::new(SwitchIdSource::Tpm, [2; 32], SwitchType::NvLink).to_string(); - for (scenario, input, expect) in [ - ( - "one enabled switch is the observed primary", - PrimaryResponseCase::Selected, - PrimaryObservation::Primary(second_id), - ), - ( - "RMS failure can converge", - PrimaryResponseCase::RmsFailure, - PrimaryObservation::Retryable, - ), - ( - "invalid RMS status violates the contract", - PrimaryResponseCase::InvalidStatus, - PrimaryObservation::Terminal, - ), - ( - "missing fabric status violates the contract", - PrimaryResponseCase::MissingFabricStatus, - PrimaryObservation::Terminal, - ), - ( - "unexpected switch violates the contract", - PrimaryResponseCase::UnexpectedSwitch, - PrimaryObservation::Terminal, - ), - ( - "duplicate switch violates the contract", - PrimaryResponseCase::DuplicateSwitch, - PrimaryObservation::Terminal, - ), - ( - "per-switch inspection failure can recover", - PrimaryResponseCase::InspectionFailure, - PrimaryObservation::Retryable, - ), - ( - "omitted switch violates the contract", - PrimaryResponseCase::OmittedSwitch, - PrimaryObservation::Terminal, - ), - ( - "no enabled switch can converge", - PrimaryResponseCase::NoPrimary, - PrimaryObservation::Retryable, - ), - ( - "multiple enabled switches can converge", - PrimaryResponseCase::MultiplePrimaries, - PrimaryObservation::Retryable, - ), - ( - "invalid primary ID violates the contract", - PrimaryResponseCase::InvalidPrimaryId, - PrimaryObservation::Terminal, - ), - ] { - assert_eq!(observe_primary_case(input), expect, "{scenario}"); - } + check_values( + [ + Check { + scenario: "one enabled switch is the observed primary", + input: PrimaryResponseCase::Selected, + expect: PrimaryObservation::Primary(second_id), + }, + Check { + scenario: "RMS failure can converge", + input: PrimaryResponseCase::RmsFailure, + expect: PrimaryObservation::Retryable, + }, + Check { + scenario: "invalid RMS status violates the contract", + input: PrimaryResponseCase::InvalidStatus, + expect: PrimaryObservation::Terminal, + }, + Check { + scenario: "missing fabric status violates the contract", + input: PrimaryResponseCase::MissingFabricStatus, + expect: PrimaryObservation::Terminal, + }, + Check { + scenario: "unexpected switch violates the contract", + input: PrimaryResponseCase::UnexpectedSwitch, + expect: PrimaryObservation::Terminal, + }, + Check { + scenario: "duplicate switch violates the contract", + input: PrimaryResponseCase::DuplicateSwitch, + expect: PrimaryObservation::Terminal, + }, + Check { + scenario: "per-switch inspection failure can recover", + input: PrimaryResponseCase::InspectionFailure, + expect: PrimaryObservation::Retryable, + }, + Check { + scenario: "omitted switch violates the contract", + input: PrimaryResponseCase::OmittedSwitch, + expect: PrimaryObservation::Terminal, + }, + Check { + scenario: "no enabled switch can converge", + input: PrimaryResponseCase::NoPrimary, + expect: PrimaryObservation::Retryable, + }, + Check { + scenario: "multiple enabled switches can converge", + input: PrimaryResponseCase::MultiplePrimaries, + expect: PrimaryObservation::Retryable, + }, + Check { + scenario: "invalid primary ID violates the contract", + input: PrimaryResponseCase::InvalidPrimaryId, + expect: PrimaryObservation::Terminal, + }, + ], + observe_primary_case, + ); }🤖 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 `@crates/rack-controller/src/fabric_manager.rs` around lines 671 - 733, Replace the hand-rolled loop in observed_primary_switch_classifies_status_responses with the imported check_values table-driven assertion, preserving all existing scenario inputs, expected PrimaryObservation values, and scenario labels. Keep the test focused on the total observe_primary_case input-to-output mapping and follow the check_values usage established by test_fabric_manager_status_from_entry.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@crates/api-core/src/tests/rack_state_controller/handler.rs`:
- Around line 3681-3722: The assertions in the test should validate node
identities rather than only lengths. In the test block containing
configure_nodes, status_nodes, and fabric_manager_status_nodes, extract each
payload’s node IDs, sort them, and compare them with the sorted switch_ids for
all three requests while preserving the existing request-count assertions.
In `@crates/rack-controller/src/fabric_manager.rs`:
- Around line 671-733: Replace the hand-rolled loop in
observed_primary_switch_classifies_status_responses with the imported
check_values table-driven assertion, preserving all existing scenario inputs,
expected PrimaryObservation values, and scenario labels. Keep the test focused
on the total observe_primary_case input-to-output mapping and follow the
check_values usage established by test_fabric_manager_status_from_entry.
In `@crates/rack-controller/src/maintenance.rs`:
- Around line 1704-1714: Extract the repeated RMS client-resolution logic into a
shared helper, including NMX-configured client preference and service-client
fallback. Update the variants around this preamble and the older if-let branches
to use the helper, preserving an owned Arc clone where callers such as the
current maintenance flow need the client to outlive a later mutable ctx borrow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 458ee8ce-7f6e-46a3-ab4b-eaef103da77f
📒 Files selected for processing (7)
crates/api-core/src/cfg/README.mdcrates/api-core/src/tests/rack_state_controller/handler.rscrates/api-model/src/rack.rscrates/rack-controller/src/config.rscrates/rack-controller/src/fabric_manager.rscrates/rack-controller/src/maintenance.rscrates/rack/src/rms_client.rs
jayzhudev
force-pushed
the
feat/nmx-c-configure-v2
branch
from
July 27, 2026 05:30
dd153de to
c5baf83
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
crates/rack-controller/src/fabric_manager.rs (1)
613-617: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe table collapses five distinct failure reasons into a single
None.
observed_primary_switchproduces five carefully differentiated diagnostics (RMS failure, missing fabric status, no primary, multiple primaries, primary outside the rack), yet.ok()discards all of them, so a regression that swaps two of those branches would still pass. Mapping theResulttoResult<String, String>and asserting the expected message keeps the table honest about the contract it documents.🤖 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 `@crates/rack-controller/src/fabric_manager.rs` around lines 613 - 617, Update the table closure around observed_primary_switch to preserve its differentiated error diagnostics instead of discarding them with .ok(). Convert the Result to Result<String, String> and assert the expected error message for each failure case, while retaining the existing primary-to-string mapping for successful cases.crates/api-model/src/rack.rs (1)
537-540: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider surfacing the selected API version in the
ConfigureCertificatesdisplay.The state string is what the rack controller logs on transition (
next_state = %configure_nmx_cluster), so discardingscale_up_fabric_manager_api_versionvia..hides which of the two workflows a rack is actually executing — precisely the fact an operator debugging a stuck rack wants first.♻️ Suggested rendering
ConfigureNmxClusterState::ConfigureCertificates { + scale_up_fabric_manager_api_version, configure_certificate, - .. - } => write!(f, "ConfigureCertificates({configure_certificate})"), + } => write!( + f, + "ConfigureCertificates({scale_up_fabric_manager_api_version:?}, {configure_certificate})" + ),🤖 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 `@crates/api-model/src/rack.rs` around lines 537 - 540, Update the ConfigureNmxClusterState::ConfigureCertificates Display implementation to retain and include scale_up_fabric_manager_api_version in the ConfigureCertificates output. Ensure the rendered state distinguishes the API-version workflow while preserving the existing configure_certificate information.crates/rack-controller/src/config.rs (1)
105-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider expressing the accepted-version cases as a table-driven scenario.
The manual
forloop duplicates whatvalue_scenarios!/check_valuesexpress declaratively, and a failing iteration reports no scenario label. As per coding guidelines, "Use table-driven tests for functions mapping inputs to outputs, errors, or observable results. Use ...value_scenarios!for total operations".♻️ Suggested table-driven form
- #[test] - fn rms_config_accepts_supported_scale_up_fabric_manager_apis() { - for (value, expected) in [ - ("v1", ScaleUpFabricManagerApiVersion::V1), - ("v2", ScaleUpFabricManagerApiVersion::V2), - ] { - let config: RmsConfig = serde_json::from_value(serde_json::json!({ - "scale_up_fabric_manager_api_version": value - })) - .expect("supported RMS API config should deserialize"); - - assert_eq!(config.scale_up_fabric_manager_api_version, expected); - } - } + #[test] + fn rms_config_accepts_supported_scale_up_fabric_manager_apis() { + carbide_test_support::value_scenarios!( + run = |value: &str| { + serde_json::from_value::<RmsConfig>(serde_json::json!({ + "scale_up_fabric_manager_api_version": value + })) + .ok() + .map(|config| config.scale_up_fabric_manager_api_version) + }; + "supported api versions" { + "v1" => Some(ScaleUpFabricManagerApiVersion::V1), + "v2" => Some(ScaleUpFabricManagerApiVersion::V2), + } + ); + }🤖 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 `@crates/rack-controller/src/config.rs` around lines 105 - 118, Rewrite the test rms_config_accepts_supported_scale_up_fabric_manager_apis to use the repository’s value_scenarios! and check_values table-driven helpers instead of the manual for loop. Represent each supported API version and expected ScaleUpFabricManagerApiVersion as a labeled scenario, while preserving the existing deserialization assertion and equality checks.Source: Coding guidelines
crates/rack-controller/src/maintenance.rs (1)
1544-1599: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared "load complete fabric + resolve node identity" preamble.
configure_scale_up_fabric_manager_v2andverify_scale_up_fabric_manager_v2(lines 1805-1840) repeat the same five steps verbatim: load the fabric inventory, skip on empty, validate NVOS credentials, resolve the profile, and derive the switch node identity. Two copies of a five-branch precondition sequence will drift. A small helper returning(RackSwitchFirmwareInventory, RmsNodeIdentity)— or an outcome enum for the skip/error branches — would keep both V2 entry points on one contract.🤖 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 `@crates/rack-controller/src/maintenance.rs` around lines 1544 - 1599, Extract the repeated fabric-manager precondition flow from configure_scale_up_fabric_manager_v2 and verify_scale_up_fabric_manager_v2 into a shared helper. Have it load and validate inventory, preserve the empty-inventory skip and profile-resolution error outcomes, resolve rack_hardware_topology, and derive switch_node_identity_for_profile, returning the inventory and RmsNodeIdentity (or an equivalent outcome type); update both entry points to use this helper.
🤖 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/rack-controller/src/maintenance.rs`:
- Around line 1873-1884: Update the observed_primary handling around
observed_primary_switch so transient transport/read failures continue returning
StateHandlerOutcome::wait, while semantic verification failures (no enabled
switch, multiple enabled switches, or a primary outside the submitted rack) use
bounded retries and then call transition_to_rack_error with an actionable cause.
Preserve retry behavior for genuinely transient failures.
---
Nitpick comments:
In `@crates/api-model/src/rack.rs`:
- Around line 537-540: Update the
ConfigureNmxClusterState::ConfigureCertificates Display implementation to retain
and include scale_up_fabric_manager_api_version in the ConfigureCertificates
output. Ensure the rendered state distinguishes the API-version workflow while
preserving the existing configure_certificate information.
In `@crates/rack-controller/src/config.rs`:
- Around line 105-118: Rewrite the test
rms_config_accepts_supported_scale_up_fabric_manager_apis to use the
repository’s value_scenarios! and check_values table-driven helpers instead of
the manual for loop. Represent each supported API version and expected
ScaleUpFabricManagerApiVersion as a labeled scenario, while preserving the
existing deserialization assertion and equality checks.
In `@crates/rack-controller/src/fabric_manager.rs`:
- Around line 613-617: Update the table closure around observed_primary_switch
to preserve its differentiated error diagnostics instead of discarding them with
.ok(). Convert the Result to Result<String, String> and assert the expected
error message for each failure case, while retaining the existing
primary-to-string mapping for successful cases.
In `@crates/rack-controller/src/maintenance.rs`:
- Around line 1544-1599: Extract the repeated fabric-manager precondition flow
from configure_scale_up_fabric_manager_v2 and verify_scale_up_fabric_manager_v2
into a shared helper. Have it load and validate inventory, preserve the
empty-inventory skip and profile-resolution error outcomes, resolve
rack_hardware_topology, and derive switch_node_identity_for_profile, returning
the inventory and RmsNodeIdentity (or an equivalent outcome type); update both
entry points to use this helper.
🪄 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: f38a2df3-435a-4592-ad25-1d30fcd85fc2
📒 Files selected for processing (16)
crates/api-core/src/cfg/README.mdcrates/api-core/src/handlers/component_manager.rscrates/api-core/src/tests/rack_state_controller/handler.rscrates/api-core/src/tests/rack_state_controller/mod.rscrates/api-model/src/rack.rscrates/component-manager/src/component_manager.rscrates/component-manager/src/mock.rscrates/component-manager/src/nsm.rscrates/component-manager/src/nv_switch_manager.rscrates/component-manager/src/rms.rscrates/rack-controller/src/config.rscrates/rack-controller/src/fabric_manager.rscrates/rack-controller/src/maintenance.rscrates/rack-controller/src/nmx_certificate.rscrates/rack/src/rms_client.rscrates/switch-controller/src/certificate.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/api-core/src/tests/rack_state_controller/handler.rs
Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
jayzhudev
force-pushed
the
feat/nmx-c-configure-v2
branch
from
July 27, 2026 06:21
c5baf83 to
cc97a04
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/rack-controller/src/fabric_manager.rs (1)
423-423: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd missing scenarios for the two newly introduced failure branches.
The scenario table exercises RMS failure, missing fabric status, zero/multiple enabled switches, and an enabled switch outside the submitted rack, but doesn't cover:
- An enabled switch whose
node_idfailsSwitchId::from_strparsing (Lines 363-371 ofobserved_primary_switch).- An enabled, submitted switch that reports a non-empty per-switch
error_message(Lines 383-388).Both branches gate whether a candidate primary switch is accepted before being persisted as the rack's primary, so it's worth locking down their behavior with the same table-driven approach already used here.
♻️ Sketch of additional scenarios
let switch_status = |node_id: &str, enabled| rms::ScaleUpFabricSwitchStatus { node_id: node_id.to_string(), enabled, ..Default::default() }; + + let switch_status_with_error = + |node_id: &str, enabled, error_message: &str| rms::ScaleUpFabricSwitchStatus { + node_id: node_id.to_string(), + enabled, + error_message: error_message.to_string(), + ..Default::default() + };Add corresponding
Checkentries for:enabled_switch.node_idset to a non-SwitchIdstring, and an enabled/submitted switch withswitch_status_with_error(&first_id, true, "inspection failed"), both expectingNone.Also applies to: 534-620
🤖 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 `@crates/rack-controller/src/fabric_manager.rs` at line 423, Add two table-driven Check scenarios to the existing scenario table around observed_primary_switch: one with an enabled switch whose node_id is an invalid SwitchId string, and one using switch_status_with_error(&first_id, true, "inspection failed") for an enabled submitted switch. Set both expected results to None, preserving the existing scenario structure and helpers.
🤖 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.
Nitpick comments:
In `@crates/rack-controller/src/fabric_manager.rs`:
- Line 423: Add two table-driven Check scenarios to the existing scenario table
around observed_primary_switch: one with an enabled switch whose node_id is an
invalid SwitchId string, and one using switch_status_with_error(&first_id, true,
"inspection failed") for an enabled submitted switch. Set both expected results
to None, preserving the existing scenario structure and helpers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a035281f-0074-498e-ad74-cc806551c406
📒 Files selected for processing (16)
crates/api-core/src/cfg/README.mdcrates/api-core/src/handlers/component_manager.rscrates/api-core/src/tests/rack_state_controller/handler.rscrates/api-core/src/tests/rack_state_controller/mod.rscrates/api-model/src/rack.rscrates/component-manager/src/component_manager.rscrates/component-manager/src/mock.rscrates/component-manager/src/nsm.rscrates/component-manager/src/nv_switch_manager.rscrates/component-manager/src/rms.rscrates/rack-controller/src/config.rscrates/rack-controller/src/fabric_manager.rscrates/rack-controller/src/maintenance.rscrates/rack-controller/src/nmx_certificate.rscrates/rack/src/rms_client.rscrates/switch-controller/src/certificate.rs
🚧 Files skipped from review as they are similar to previous changes (15)
- crates/component-manager/src/mock.rs
- crates/component-manager/src/nsm.rs
- crates/api-core/src/tests/rack_state_controller/mod.rs
- crates/component-manager/src/component_manager.rs
- crates/api-core/src/cfg/README.md
- crates/rack-controller/src/nmx_certificate.rs
- crates/api-core/src/handlers/component_manager.rs
- crates/switch-controller/src/certificate.rs
- crates/component-manager/src/nv_switch_manager.rs
- crates/api-model/src/rack.rs
- crates/rack-controller/src/config.rs
- crates/rack/src/rms_client.rs
- crates/component-manager/src/rms.rs
- crates/api-core/src/tests/rack_state_controller/handler.rs
- crates/rack-controller/src/maintenance.rs
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.
Add support for the
librms v0.10.0ConfigureScaleUpFabricManagerV2async RPC.The V1 requires NICo to coordinate multiple RMS RPCs: disable the cluster state across all switches, query switch tray indexes to select a primary, synchronously configure that primary, and then query fabric status separately. V2 moves this coordination into RMS as a scale-up fabric-wide desired-state reconciliation job.
With the V2 RPC, NICo gains a stronger completion contract: RMS selects the primary, reconciles the scale-up fabric, waits for NMX-C to reach configured state, and verifies final switch and topology readback before completing the job. NICo submits the group of switches and desired topology, polls the job, then reads and persists the RMS-selected primary and per-switch Fabric Manager status.
V1 remains the default for compatibility.
To enable V2 under the RMS configuration:
If unset,
v1is used.Related issues
Resolves #1923
Type of Change
Breaking Changes
V1 remains the default, so existing deployments retain current behavior.
Testing
Manual Testing
Validated against two switches using RMS and the V2 selector.
GetScaleUpFabricStatusafter completion and persisted the observed primary and Fabric Manager status.BatchSetScaleUpFabricStatecalls were observed.Selected redacted logs:
Trimmed completed job response:
{ "executionState": "JOB_EXECUTION_STATE_COMPLETED", "stateDescription": "Completed", "result": { "topology_type": "gb200_nvl36r1_c2g4_topology", "switches": [ { "node_id": "<primary-switch-id>", "enabled": true, "fabric_manager_status": "ok" }, { "node_id": "<non-primary-switch-id>", "enabled": false, "fabric_manager_status": "" } ] } }Persisted NICo state:
Repeat reconciliation evidence:
Additional notes:
REFUSED_STREAMduring final readback. RMS rolled both switches back to disabled; retrying the same desired state passed.Simulation Testing
Tested scenarios:
scale_up_fabric_manager_api_version = "v2"and verified selection of the V2 workflow.BatchSetScaleUpFabricState(false)and device-info preparation calls.ConfigureScaleUpFabricManagerV2 request containing the selected topology, all rack switches, and no primary-switch override.GetJobStatus, and continued only after job completion.GetScaleUpFabricStatusafter completion, accepted the sole primary selected by RMS, and replaced the previously stored primary.