feat(machine-a-tron): register expected switches and power shelves#2320
feat(machine-a-tron): register expected switches and power shelves#2320poroh wants to merge 1 commit into
Conversation
Register LiteOn power shelves and NVIDIA switches as their expected inventory types instead of expected machines. Preserve switch NVOS MAC/serial data across persistence and adjust mock BMC handling for BMC-only hardware. Signed-off-by: Dmitry Porokh <dporokh@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. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/machine-a-tron/src/machine_a_tron.rs (1)
114-130: ⚡ Quick winConsider documenting the serial number fallback behavior.
The switch registration falls back to
host_info.serialwhenswitch_serial_numberis absent (lines 119-122). While this fallback is reasonable, the implicit behavior may not be obvious to future maintainers or when debugging inventory mismatches.📝 Add a comment documenting the fallback
HostHardwareType::NvidiaSwitchNd5200Ld => { self.app_context .api_client() .add_expected_switch( host_info.bmc_mac_address.to_string(), + // Fall back to host serial if switch-specific serial is unavailable host_info .switch_serial_number .clone() .unwrap_or_else(|| host_info.serial.clone()), host_info .nvos_mac_addresses .iter() .map(|mac| mac.to_string()) .collect(), ) .await }🤖 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/machine-a-tron/src/machine_a_tron.rs` around lines 114 - 130, Document that when registering switches for HostHardwareType::NvidiaSwitchNd5200Ld the call to app_context.api_client().add_expected_switch uses host_info.switch_serial_number and falls back to host_info.serial if switch_serial_number is None; add a concise inline comment above that block (or adjacent to the .unwrap_or_else call) stating the fallback rationale and any assumptions (e.g., host serial represents switch serial when explicit switch_serial_number is missing) and note any implications for inventory matching/debugging.crates/machine-a-tron/src/api_client.rs (1)
564-589: Keepbmc_retain_credentialsdiffering for power shelf vs switch (it matches mock + rotation semantics)
bmc_retain_credentialsis used by site-explorer to skip BMC password rotation and persist the current credentials as-is. The BMC-mock permits factory-default password access only forLiteOnPowerShelf; factory-default credentials are forbidden for other hosts (includingNvidiaSwitchNd5200Ld). That’s whyadd_expected_power_shelfsetsbmc_retain_credentials: Some(true)whileadd_expected_switchleaves it asNone(so the switch path rotates/falls back instead of persisting credentials that the BMC won’t accept).Optional: add a short comment near these two constructor blocks in
crates/machine-a-tron/src/api_client.rsexplaining the coupling betweenbmc_retain_credentials, mock factory-default permission, and the rotation skip behavior.🤖 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/machine-a-tron/src/api_client.rs` around lines 564 - 589, The add_expected_switch constructor must leave bmc_retain_credentials as None (so switches follow rotation/fallback) while add_expected_power_shelf must set bmc_retain_credentials: Some(true) (so LiteOnPowerShelf retains factory creds); update these two constructors in api_client.rs (functions/methods add_expected_switch and add_expected_power_shelf) to ensure those exact values and add a short comment near each constructor explaining the coupling between bmc_retain_credentials, the BMC mock’s factory-default permission, and the credential-rotation/skip behavior.
🤖 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/machine-a-tron/src/api_client.rs`:
- Around line 564-589: The add_expected_switch constructor must leave
bmc_retain_credentials as None (so switches follow rotation/fallback) while
add_expected_power_shelf must set bmc_retain_credentials: Some(true) (so
LiteOnPowerShelf retains factory creds); update these two constructors in
api_client.rs (functions/methods add_expected_switch and
add_expected_power_shelf) to ensure those exact values and add a short comment
near each constructor explaining the coupling between bmc_retain_credentials,
the BMC mock’s factory-default permission, and the credential-rotation/skip
behavior.
In `@crates/machine-a-tron/src/machine_a_tron.rs`:
- Around line 114-130: Document that when registering switches for
HostHardwareType::NvidiaSwitchNd5200Ld the call to
app_context.api_client().add_expected_switch uses host_info.switch_serial_number
and falls back to host_info.serial if switch_serial_number is None; add a
concise inline comment above that block (or adjacent to the .unwrap_or_else
call) stating the fallback rationale and any assumptions (e.g., host serial
represents switch serial when explicit switch_serial_number is missing) and note
any implications for inventory matching/debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c22ddcb1-4acb-4c38-9384-9a6fb7369e2f
📒 Files selected for processing (8)
crates/bmc-mock/src/auth_router.rscrates/bmc-mock/src/machine_info.rscrates/bmc-mock/src/mock_machine_router.rscrates/machine-a-tron/src/api_client.rscrates/machine-a-tron/src/config.rscrates/machine-a-tron/src/host_machine.rscrates/machine-a-tron/src/machine_a_tron.rscrates/machine-a-tron/src/machine_state_machine.rs
1 participant
Description
Register LiteOn power shelves and NVIDIA switches as their expected inventory types instead of expected machines. Preserve switch NVOS MAC/serial data across persistence and adjust mock BMC handling for BMC-only hardware.
Type of Change
Related Issues (Optional)
#1864
Breaking Changes
Testing
Additional Notes