From 5047ad7b7b5ad7c974ac8c635e2b915256278db5 Mon Sep 17 00:00:00 2001 From: Chet Nichols III Date: Fri, 31 Jul 2026 12:14:28 -0700 Subject: [PATCH] fix(machine-controller): wait for host power-off before power-on After #4430 fixed the stale Machine-a-Tron action queue, the rack integration test exposed a second ordering problem: the controller could enter the `On` phase before the BMC had actually applied `ForceOff`. So, this gives the observed power-off its own persisted `WaitingForPlatformPowerOff` phase. `ForceOff` is issued once, stale power readings stay parked there, and the existing idempotent `On` phase remains restart-safe if the process exits between the Redfish request and state commit. Tests added! This supports https://github.com/NVIDIA/infra-controller/issues/4446 Signed-off-by: Chet Nichols III --- crates/api-core/src/tests/machine_history.rs | 1 + crates/api-model/src/machine/mod.rs | 19 ++- crates/machine-controller/src/handler.rs | 45 +++++- crates/machine-controller/src/io.rs | 5 +- .../tests/integration/power_management.rs | 137 +++++++++++++++++- 5 files changed, 195 insertions(+), 12 deletions(-) diff --git a/crates/api-core/src/tests/machine_history.rs b/crates/api-core/src/tests/machine_history.rs index 05cb7af345..b4d2e524a2 100644 --- a/crates/api-core/src/tests/machine_history.rs +++ b/crates/api-core/src/tests/machine_history.rs @@ -44,6 +44,7 @@ async fn test_machine_state_history(pool: sqlx::PgPool) -> Result<(), Box { + self.handle_waiting_for_platform_power_off(state, ctx).await + } DpuInitState::WaitingForPlatformPowercycle { substate: PerformPowerOperation::On, } => { @@ -5006,6 +5007,32 @@ impl DpuMachineStateHandler { Ok(StateHandlerOutcome::transition(next_state)) } + + /// Waits for the one host-wide `ForceOff` to become visible through Redfish. + /// + /// Normal dispatch calls this before walking individual DPUs so a + /// multi-DPU host performs one BMC read per controller iteration. + async fn handle_waiting_for_platform_power_off( + &self, + state: &ManagedHostStateSnapshot, + ctx: &mut StateHandlerContext<'_, MachineStateHandlerContextObjects>, + ) -> Result, StateHandlerError> { + // Redfish power actions are asynchronous. Persist this wait before + // trusting a new reading so stale `On` cannot skip the power cycle. + if !is_host_powered_off(state, ctx).await? { + return Ok(StateHandlerOutcome::wait(format!( + "Waiting for host {} to power off before powering it on", + state.host_snapshot.id, + ))); + } + + let next_state = DpuInitState::WaitingForPlatformPowercycle { + substate: PerformPowerOperation::On, + } + .next_state_with_all_dpus_updated(&state.managed_state)?; + + Ok(StateHandlerOutcome::transition(next_state)) + } } #[async_trait::async_trait] @@ -5029,6 +5056,16 @@ impl StateHandler for DpuMachineStateHandler { }; Ok(StateHandlerOutcome::transition(next_state)) } else { + if let ManagedHostState::DPUInit { dpu_states } = &state.managed_state + && !dpu_states.states.is_empty() + && dpu_states + .states + .values() + .all(|state| matches!(state, DpuInitState::WaitingForPlatformPowerOff)) + { + return self.handle_waiting_for_platform_power_off(state, ctx).await; + } + for dpu_snapshot in &state.dpu_snapshots { state_handler_outcome = self.handle_dpuinit_state(state, dpu_snapshot, ctx).await?; diff --git a/crates/machine-controller/src/io.rs b/crates/machine-controller/src/io.rs index e4695decb9..8b8f7e57ad 100644 --- a/crates/machine-controller/src/io.rs +++ b/crates/machine-controller/src/io.rs @@ -172,7 +172,10 @@ impl StateControllerIO for MachineStateControllerIO { DpuInitState::WaitingForNetworkConfig => "waitingfornetworkconfig", DpuInitState::WaitingForPlatformConfiguration => "waitingforplatformconfiguration", DpuInitState::PollingBiosSetup => "pollingbiossetup", - DpuInitState::WaitingForPlatformPowercycle { .. } => "waitingforplatformpowercycle", + // The observed-Off wait remains part of the existing + // operator-facing power-cycle metric. + DpuInitState::WaitingForPlatformPowercycle { .. } + | DpuInitState::WaitingForPlatformPowerOff => "waitingforplatformpowercycle", DpuInitState::DpfStates { .. } => "dpfstates", } } diff --git a/crates/machine-controller/tests/integration/power_management.rs b/crates/machine-controller/tests/integration/power_management.rs index 963b88e517..c3c66d1653 100644 --- a/crates/machine-controller/tests/integration/power_management.rs +++ b/crates/machine-controller/tests/integration/power_management.rs @@ -18,10 +18,16 @@ use std::sync::Arc; use carbide_redfish::libredfish::RedfishClientPool as _; +use carbide_redfish::libredfish::test_support::RedfishSimAction; use carbide_test_harness::prelude::*; -use carbide_test_harness::test_support::fixture_config::FixtureDefault as _; +use carbide_test_harness::test_support::fixture_config::{ + FixtureDefault as _, ManagedHostConfigExt as _, +}; use carbide_utils::redfish::BmcAccessInfo; -use model::machine::{MachineMaintenanceOperation, ManagedHostState}; +use model::machine::{ + DpuInitState, DpuInitStates, MachineMaintenanceOperation, ManagedHostState, + PerformPowerOperation, +}; use model::power_manager::PowerState; use model::test_support::ManagedHostConfig; use rpc::forge::{ @@ -55,6 +61,14 @@ impl TestContext { } async fn from_env(env: Env) -> Self { + Self::from_env_with_config(env, ManagedHostConfig::default()).await + } + + /// Builds the fixture with an explicit hardware layout. + /// + /// Power-cycle coverage uses this to exercise one host with multiple DPUs + /// without changing the default fixture used by the rest of this module. + async fn from_env_with_config(env: Env, config: ManagedHostConfig) -> Self { let domain = env.test_harness.test_domain().await; let network_controller = env.test_harness.network_controller(); let underlay_segment = network_controller.create_underlay_segment(&domain).await; @@ -63,7 +77,7 @@ impl TestContext { let mh = env .test_harness .managed_host_builder(&site_explorer, underlay_segment) - .with_config(ManagedHostConfig::default()) + .with_config(config) .build() .await .0; @@ -129,6 +143,123 @@ impl TestManagedHostPowerExt for TestManagedHost { } } +#[sqlx_test] +async fn dpu_init_power_cycle_waits_for_observed_host_power_off( + pool: PgPool, +) -> Result<(), Box> { + let TestContext { mut env, mh } = TestContext::from_env_with_config( + Env::builder(pool).build().await, + ManagedHostConfig::default().with_dpu_count(2), + ) + .await; + let dpu_init_state = |dpu_state: DpuInitState| ManagedHostState::DPUInit { + dpu_states: DpuInitStates { + states: mh + .dpus + .iter() + .map(|dpu| (dpu.id, dpu_state.clone())) + .collect(), + }, + }; + let powering_off = dpu_init_state(DpuInitState::WaitingForPlatformPowercycle { + substate: PerformPowerOperation::Off, + }); + let waiting_for_power_off = dpu_init_state(DpuInitState::WaitingForPlatformPowerOff); + let powering_on = dpu_init_state(DpuInitState::WaitingForPlatformPowercycle { + substate: PerformPowerOperation::On, + }); + let configuring = dpu_init_state(DpuInitState::WaitingForPlatformConfiguration); + mh.advance_state(powering_off).await; + + let bmc_access_info = mh.bmc_access_info().await; + let redfish_client = env.redfish_sim.client_by_info(&bmc_access_info).await?; + assert_eq!( + redfish_client.get_power_state().await?, + libredfish::PowerState::On, + ); + + // Keep the power-off observation in its own persisted phase. A delayed BMC + // can then report stale `On` without letting the controller skip ahead. + let redfish_timepoint = env.redfish_sim.timepoint(); + env.run_single_iteration().await; + + assert_eq!( + mh.host.machine().await.current_state(), + &waiting_for_power_off, + ); + assert_eq!( + env.redfish_sim + .actions_since(&redfish_timepoint) + .for_host(&bmc_access_info.host), + vec![RedfishSimAction::Power( + libredfish::SystemPowerControl::ForceOff, + )], + ); + + redfish_client + .power(libredfish::SystemPowerControl::On) + .await?; + let redfish_timepoint = env.redfish_sim.timepoint(); + env.run_single_iteration().await; + + assert_eq!( + mh.host.machine().await.current_state(), + &waiting_for_power_off, + ); + assert!( + env.redfish_sim + .actions_since(&redfish_timepoint) + .for_host(&bmc_access_info.host) + .is_empty(), + ); + + // Once `Off` is observed, persist the power-on phase before issuing `On`. + redfish_client + .power(libredfish::SystemPowerControl::ForceOff) + .await?; + let redfish_timepoint = env.redfish_sim.timepoint(); + env.run_single_iteration().await; + + assert_eq!(mh.host.machine().await.current_state(), &powering_on); + assert!( + env.redfish_sim + .actions_since(&redfish_timepoint) + .for_host(&bmc_access_info.host) + .is_empty(), + ); + + let redfish_timepoint = env.redfish_sim.timepoint(); + env.run_single_iteration().await; + + assert_eq!(mh.host.machine().await.current_state(), &configuring); + assert_eq!( + env.redfish_sim + .actions_since(&redfish_timepoint) + .for_host(&bmc_access_info.host), + vec![RedfishSimAction::Power(libredfish::SystemPowerControl::On)], + ); + assert_eq!( + redfish_client.get_power_state().await?, + libredfish::PowerState::On, + ); + + // If the process exits after `On` succeeds but before the transition is + // committed, the persisted power-on phase must still resume cleanly. + mh.advance_state(powering_on).await; + let redfish_timepoint = env.redfish_sim.timepoint(); + env.run_single_iteration().await; + + assert_eq!(mh.host.machine().await.current_state(), &configuring); + assert!( + env.redfish_sim + .actions_since(&redfish_timepoint) + .for_host(&bmc_access_info.host) + .is_empty(), + ); + + Ok(()) +} + #[sqlx_test] async fn desired_on_polls_powered_off_machine( pool: PgPool,