feat: Add rack-aware machine-a-tron simulation#3207
Conversation
|
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. |
|
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 ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Summary by CodeRabbit
WalkthroughAdds rack configuration and validation to machine-a-tron, provisions expected racks through the Forge API, associates expected hardware with rack IDs, and adds NVL72 rack integration coverage. ChangesRack Support in Machine-a-tron Simulation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant IntegrationTest
participant MachineATron
participant ForgeAPI
participant Database
IntegrationTest->>MachineATron: run rack-configured scenario
MachineATron->>ForgeAPI: ensure expected rack
MachineATron->>ForgeAPI: register expected hardware with rack_id
ForgeAPI->>Database: persist rack associations
IntegrationTest->>Database: verify rack and machine assignments
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3207.docs.buildwithfern.com/infra-controller |
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
Resolves #1198
Important
Current
mainincludes #3364, which supersedes #3265 with broader admin-segment lock ordering across discovery, primary-interface repair, and teardown paths. This branch is updated tomainat83ea2c434cd45924bb634fa846970921113b6dd0, and the rack lifecycle was revalidated at merge commit0197773cdf3c9452012aecb85edbe99dd29eb329.Short version
This teaches machine-a-tron to declare racks and attach simulated components to them, then exercises the real path with an NVL72-shaped rack containing 18 GB200 compute trays. With current
main's broader #3364 lock-order fix, all 18 trays are registered as expected inventory, discovered as managed machines, associated with the rack, and driven to Ready. Existing rackless configurations keep working.Why this is needed
Before this change, machine-a-tron could create individual simulated machines, switches, and power shelves, but its expected-inventory records always had an empty
rack_id. That meant developers could not use the simulator to exercise rack-based systems: NICo had no expected rack entity to reference, and simulated components could not be associated with one.Issue #1198 defines the minimum useful result as a valid expected rack plus rack-associated simulated hosts, with an 18-tray rack moving through the machine lifecycle. This PR implements and exercises that path.
What changed
rack_profile_id.rack_id. The map supports multiple rack declarations and lets separate machine groups target different racks.racksand a machine-grouprack_idpreserves the previous rackless behavior.Example:
Lock-order dependency status
The 18-tray workload originally exposed a production lock inversion between DPU discovery and admin-address allocation. That fix was split into #3265 because it affects production transactions rather than rack simulation.
Current
mainnow includes #3364 at8024a1589551399d71342c11210851419a7e1695. Its implementation is broader than #3265: it acquires ordered admin-segment locks before machine-interface writes in discovery and primary-interface repair, centralizes ordered segment locking, and protects additional teardown paths. #3265 is therefore superseded rather than a remaining dependency.What the integration test proves
The focused test starts the local NICo API and BMC mock, configures one
NVL72rack, and creates 18WiwynnGB200Nvlhosts with two DPUs each. For the real persisted state it asserts:machine-a-tron-nvl72with profileNVL72;Readyand retains the same rack ID.The rack scenario has its own integration test so its result is independent of the broad parallel integration test's separate cleanup behavior.
How we verified it
Verified revision:
0197773cdf3c9452012aecb85edbe99dd29eb329Merged base: upstream
mainat83ea2c434cd45924bb634fa846970921113b6dd0Environment: Apple Silicon macOS, Docker 28.3.3, PostgreSQL 14.5 Alpine, and the repository's pinned Rust toolchain.
The focused real lifecycle passed twice against separate clean PostgreSQL containers:
The second run had no PostgreSQL deadlock. The first run logged one retryable, unrelated
machinesrow deadlock betweenlast_reboot_requestedand health-report persistence; the test recovered and still reached 18 Ready machines. Neither run reproduced the original admin-segment/interface-row lock inversion.Supporting checks also passed:
How to reproduce the verification
Prerequisites: Docker, the repository Rust toolchain, Vault, curl, and Go.
From a clean checkout:
Expected test result:
1 passed; 0 failed.Inspect the persisted result:
Expected row:
machine-a-tron-nvl72 | NVL72 | 18 | 18 | 18.Scope and non-goals
mainsupplies the broader implementation from fix: AdminForceDeleteMachine can deadlock #3364.machinesrow deadlock outside this PR's rack-simulation scope; the second clean run did not reproduce it.