Skip to content

feat: Add rack-aware machine-a-tron simulation#3207

Open
kfelternv wants to merge 10 commits into
NVIDIA:mainfrom
kfelternv:machine-a-tron-racks
Open

feat: Add rack-aware machine-a-tron simulation#3207
kfelternv wants to merge 10 commits into
NVIDIA:mainfrom
kfelternv:machine-a-tron-racks

Conversation

@kfelternv

@kfelternv kfelternv commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Resolves #1198

Important

Current main includes #3364, which supersedes #3265 with broader admin-segment lock ordering across discovery, primary-interface repair, and teardown paths. This branch is updated to main at 83ea2c434cd45924bb634fa846970921113b6dd0, and the rack lifecycle was revalidated at merge commit 0197773cdf3c9452012aecb85edbe99dd29eb329.

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

  • Machine-a-tron config accepts optional rack declarations keyed by rack ID. Each declaration supplies a typed rack_profile_id.
  • Each machine group can optionally reference one declared rack_id. The map supports multiple rack declarations and lets separate machine groups target different racks.
  • Config validation runs before API work. It rejects empty rack IDs, empty profile IDs, and machine groups that reference undeclared racks.
  • Expected racks are registered before expected components. Reusing an existing rack is idempotent when its profile matches; a conflicting profile is reported as a configuration error.
  • The selected rack ID is copied into expected machine, switch, and power-shelf records created from that machine group.
  • Omitting both racks and a machine-group rack_id preserves the previous rackless behavior.

Example:

[racks.rack-001]
rack_profile_id = "NVL72"

[machines.compute-trays]
rack_id = "rack-001"
host_count = 18
dpu_per_host_count = 2

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 main now includes #3364 at 8024a1589551399d71342c11210851419a7e1695. 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 NVL72 rack, and creates 18 WiwynnGB200Nvl hosts with two DPUs each. For the real persisted state it asserts:

  • the expected rack exists as machine-a-tron-nvl72 with profile NVL72;
  • all 18 expected machines reference that rack;
  • each managed machine reaches Ready and 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: 0197773cdf3c9452012aecb85edbe99dd29eb329

Merged base: upstream main at 83ea2c434cd45924bb634fa846970921113b6dd0

Environment: 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:

run 1: test result: ok. 1 passed; finished in 79.49s
run 2: test result: ok. 1 passed; finished in 79.67s
machine-a-tron-nvl72 | NVL72 | 18 expected | 18 managed | 18 Ready

The second run had no PostgreSQL deadlock. The first run logged one retryable, unrelated machines row deadlock between last_reboot_requested and 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:

carbide-machine-a-tron: 5 unit tests passed
carbide-machine-a-tron: 1 doctest passed
final PR diff against current main: git diff --check passed
signed merge commit: valid Kyle Felter signature

How to reproduce the verification

Prerequisites: Docker, the repository Rust toolchain, Vault, curl, and Go.

From a clean checkout:

git fetch origin pull/3207/head:pr-3207
git checkout pr-3207
git rev-parse HEAD
# expected: 0197773cdf3c9452012aecb85edbe99dd29eb329

mkdir -p ../pr3207-tools/bin
GOBIN="$PWD/../pr3207-tools/bin" \
  go install github.com/fullstorydev/grpcurl/cmd/grpcurl@v1.8.7
export PATH="$PWD/../pr3207-tools/bin:$PATH"

docker run --detach --name nico-rack-pr3207-postgres \
  --env POSTGRES_HOST_AUTH_METHOD=trust \
  --publish 127.0.0.1:55434:5432 \
  postgres:14.5-alpine
docker exec nico-rack-pr3207-postgres pg_isready -U postgres

REPO_ROOT="$PWD" \
DATABASE_URL=postgresql://postgres@127.0.0.1:55434 \
cargo test --release -p carbide-api-integration-tests --test lib \
  test_machine_a_tron_rack_integration -- --exact --nocapture

Expected test result: 1 passed; 0 failed.

Inspect the persisted result:

docker exec nico-rack-pr3207-postgres psql -U postgres \
  -d api_server_test_machine_a_tron_rack_integration \
  -c "SELECT er.rack_id, er.rack_profile_id,
      (SELECT COUNT(*) FROM expected_machines em WHERE em.rack_id = er.rack_id) AS expected,
      (SELECT COUNT(*) FROM machines m WHERE m.rack_id = er.rack_id) AS managed,
      (SELECT COUNT(*) FROM machines m WHERE m.rack_id = er.rack_id AND m.controller_state->>'state' = 'ready') AS ready
    FROM expected_racks er
    WHERE er.rack_id = 'machine-a-tron-nvl72';"

Expected row: machine-a-tron-nvl72 | NVL72 | 18 | 18 | 18.

Scope and non-goals

  • This PR covers the minimum rack-simulation lifecycle requested by Support rack systems in simulation (machine-a-tron) #1198. It does not add the optional firmware-update flow.
  • It does not change existing rackless simulator configurations.
  • It does not add a production lock-order fix; current main supplies the broader implementation from fix: AdminForceDeleteMachine can deadlock #3364.
  • It does not change general machine-controller concurrency. One of two verification runs observed a retryable machines row deadlock outside this PR's rack-simulation scope; the second clean run did not reproduce it.

@copy-pr-bot

copy-pr-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown

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.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0e3c5f0e-cb52-484d-bf2d-785b2b0c03ed

📥 Commits

Reviewing files that changed from the base of the PR and between 43763aa and 0197773.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • AGENTS.md
✅ Files skipped from review due to trivial changes (1)
  • AGENTS.md

Summary by CodeRabbit

  • New Features

    • Added rack-aware machine configuration, including rack profiles and machine-to-rack assignment.
    • Expected machines, switches, and power shelves can now be associated with racks.
    • Added automatic creation and validation of expected rack inventory.
    • Added support for NVL72 rack profiles.
  • Bug Fixes

    • Configuration is now validated before machine provisioning begins.
  • Tests

    • Added integration coverage for rack-based machine provisioning and assignment.

Walkthrough

Adds 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.

Changes

Rack Support in Machine-a-tron Simulation

Layer / File(s) Summary
Rack configuration and validation
crates/machine-a-tron/src/config.rs, crates/machine-a-tron/src/lib.rs, crates/machine-a-tron/config/mat.toml, crates/machine-a-tron/Cargo.toml
Adds rack definitions, optional machine rack references, validation rules, public re-exports, sample configuration, and validation tests.
Forge rack provisioning
crates/machine-a-tron/src/api_client.rs
Propagates rack IDs to expected hardware requests and adds idempotent expected-rack creation with profile validation.
Rack-aware machine orchestration
crates/machine-a-tron/src/machine_a_tron.rs, crates/machine-a-tron/src/main.rs, crates/api-test-helper/src/machine_a_tron.rs
Validates configuration before execution, provisions configured racks, and passes rack IDs through expected inventory registration.
Rack integration test coverage
crates/api-integration-tests/tests/lib.rs, crates/api-test-helper/src/api_server.rs, crates/api-integration-tests/Cargo.toml, AGENTS.md
Adds the NVL72 rack profile, rack-aware test harness setup, rack assignment assertions, and updates existing harness call sites.

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
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: making machine-a-tron rack-aware.
Description check ✅ Passed The description is directly about the rack-aware simulation and integration test changes in this PR.
Linked Issues check ✅ Passed The PR meets #1198 by creating expected racks, attaching rack_id to expected machines, and adding an 18-tray rack lifecycle test.
Out of Scope Changes check ✅ Passed No clearly unrelated code changes are present; the edits all support rack-aware machine-a-tron simulation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

kfelternv added 6 commits July 7, 2026 16:58
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>
@kfelternv kfelternv marked this pull request as ready for review July 8, 2026 15:32
@kfelternv kfelternv requested a review from a team as a code owner July 8, 2026 15:32
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Comment thread crates/api-core/src/handlers/machine_discovery.rs Outdated
poroh
poroh previously approved these changes Jul 8, 2026
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
boot-artifacts-aarch64 3 0 0 3 0 0
boot-artifacts-x86_64 3 0 0 3 0 0
forge-admin-cli-x86_64 271 13 34 91 7 126
machine-validation-runner 804 40 237 296 36 195
machine_validation 804 40 237 296 36 195
machine_validation-aarch64 804 40 237 296 36 195
nvmetal-carbide 804 40 237 296 36 195
TOTAL 3493 173 982 1281 151 906

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

@kfelternv kfelternv marked this pull request as draft July 8, 2026 18:53
@kfelternv

Copy link
Copy Markdown
Contributor Author

The production DPU discovery lock-order fix has been split into #3265. This PR is back in draft and must wait for #3265 to merge; after it lands, I will update the branch and rerun the focused 18-tray lifecycle before requesting review again.

Comment thread crates/api-test-helper/src/api_server.rs Outdated
@kfelternv kfelternv marked this pull request as ready for review July 11, 2026 02:30
@kfelternv kfelternv requested a review from kensimon July 11, 2026 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support rack systems in simulation (machine-a-tron)

4 participants