Skip to content

fix(CICD, machine-controller): wait for host power-off before power-on #4454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chet merged 1 commit into from
Jul 31, 2026
Merged

fix(CICD, machine-controller): wait for host power-off before power-on #4454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/api-core/src/tests/machine_history.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ async fn test_machine_state_history(pool: sqlx::PgPool) -> Result<(), Box<dyn st
{"state": "dpuinit", "dpu_states": {"states": {&dpu_machine_id_string: {"dpustate": "installdpuos", "substate": {"installdpuosstate": "waitforinstallcomplete", "progress": "0", "task_id": "0"}}}}},
{"state": "dpuinit", "dpu_states": {"states": {&dpu_machine_id_string: {"dpustate": "init"}}}},
{"state": "dpuinit", "dpu_states": {"states": {&dpu_machine_id_string: {"dpustate": "waitingforplatformpowercycle", "substate": {"state": "off"}}}}},
{"state": "dpuinit", "dpu_states": {"states": {&dpu_machine_id_string: {"dpustate": "waitingforplatformpoweroff"}}}},
{"state": "dpuinit", "dpu_states": {"states": {&dpu_machine_id_string: {"dpustate": "waitingforplatformpowercycle", "substate": {"state": "on"}}}}},
{"state": "dpuinit", "dpu_states": {"states": {&dpu_machine_id_string: {"dpustate": "waitingforplatformconfiguration"}}}},
{"state": "dpuinit", "dpu_states": {"states": {&dpu_machine_id_string: {"dpustate": "pollingbiossetup"}}}},
Expand Down
19 changes: 15 additions & 4 deletions crates/api-model/src/machine/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1900,14 +1900,25 @@ pub enum SetSecureBootState {
WaitCertificateUpload { task_id: String },
}

// Since order is derived, Enum members must be in initial to last state sequence.
// Derived ordering gates states through `Init` and selects the least-advanced
// DPU for SLA and status reporting. Host-wide power-cycle phases transition
// every DPU together, so their relative ordering is not observed.
#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq, PartialOrd, Ord)]
#[serde(tag = "dpustate", rename_all = "lowercase")]
pub enum DpuInitState {
InstallDpuOs { substate: InstallDpuOsState },
DpfStates { state: DpfState },
InstallDpuOs {
substate: InstallDpuOsState,
},
DpfStates {
state: DpfState,
},
Init,
WaitingForPlatformPowercycle { substate: PerformPowerOperation },
WaitingForPlatformPowercycle {
substate: PerformPowerOperation,
},
/// Waits for Redfish to confirm the platform is `Off` before the
/// idempotent power-on phase begins.
WaitingForPlatformPowerOff,
WaitingForPlatformConfiguration,
PollingBiosSetup,
WaitingForNetworkConfig,
Expand Down
45 changes: 41 additions & 4 deletions crates/machine-controller/src/handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4358,13 +4358,14 @@ impl DpuMachineStateHandler {

handler_host_power_control(state, ctx, SystemPowerControl::ForceOff).await?;

let next_state = DpuInitState::WaitingForPlatformPowercycle {
substate: PerformPowerOperation::On,
}
.next_state_with_all_dpus_updated(&state.managed_state)?;
let next_state = DpuInitState::WaitingForPlatformPowerOff
.next_state_with_all_dpus_updated(&state.managed_state)?;

Ok(StateHandlerOutcome::transition(next_state))
}
DpuInitState::WaitingForPlatformPowerOff => {
self.handle_waiting_for_platform_power_off(state, ctx).await
}
DpuInitState::WaitingForPlatformPowercycle {
substate: PerformPowerOperation::On,
} => {
Expand Down Expand Up @@ -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<StateHandlerOutcome<ManagedHostState>, 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))
}
Comment on lines +5010 to +5035

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== all_dpu_states_in_sync ==="
rg -n -A 25 'fn all_dpu_states_in_sync' --type rust

echo "=== next_state_with_all_dpus_updated ==="
rg -n -A 25 'fn next_state_with_all_dpus_updated' --type rust

echo "=== is_host_powered_off ==="
rg -n -A 25 'fn is_host_powered_off' --type rust

echo "=== callers of handle_waiting_for_platform_power_off ==="
rg -n -B2 -A2 'handle_waiting_for_platform_power_off' --type rust

Repository: NVIDIA/infra-controller

Length of output: 192

🏁 Script executed:

set -euo pipefail

echo "=== candidate files ==="
git ls-files | rg '(^|/)(handler|.*state.*|.*managed.*)\.(rs|rs\.in)$' | head -200

echo "=== symbol references ==="
rg -n --hidden --glob '*.rs' \
  'all_dpu_states_in_sync|next_state_with_all_dpus_updated|is_host_powered_off|handle_waiting_for_platform_power_off|WaitingForPlatformPowercycle|WaitingForPlatformPowerOff' .

Repository: NVIDIA/infra-controller

Length of output: 14242

🏁 Script executed:

set -euo pipefail

echo "=== helper trait and implementations ==="
cat -n crates/machine-controller/src/handler/helpers.rs | sed -n '1,180p'
cat -n crates/machine-controller/src/handler/helpers.rs | sed -n '190,285p'
cat -n crates/machine-controller/src/handler/helpers.rs | sed -n '470,610p'

echo "=== DPU state dispatch and power transitions ==="
cat -n crates/machine-controller/src/handler.rs | sed -n '4200,4410p'
cat -n crates/machine-controller/src/handler.rs | sed -n '5000,5080p'

echo "=== power-state helper ==="
cat -n crates/machine-controller/src/handler.rs | sed -n '11200,11265p'

Repository: NVIDIA/infra-controller

Length of output: 36349

🏁 Script executed:

set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

handler = Path("crates/machine-controller/src/handler.rs").read_text()
helpers = Path("crates/machine-controller/src/handler/helpers.rs").read_text()

start = handler.index("async fn handle_waiting_for_platform_power_off")
end = handler.index("\n    }\n}", start) + len("\n    }")
body = handler[start:end]

print("=== power-off handler guard check ===")
print("calls all_dpu_states_in_sync:", "all_dpu_states_in_sync" in body)
print("calls is_host_powered_off:", "is_host_powered_off" in body)
print("calls next_state_with_all_dpus_updated:", "next_state_with_all_dpus_updated" in body)

start = helpers.index("fn next_state_with_all_dpus_updated(\n        self,\n        current_state: &ManagedHostState,")
end = helpers.index("\n    }\n}", start) + len("\n    }")
body = helpers[start:end]

print("=== DpuInitState bulk transition behavior ===")
print("matches current DPUInit state:", "ManagedHostState::DPUInit { dpu_states }" in body)
print("checks current DPU values before replacement:",
      bool(re.search(r"states\\.values\\(\\)|all_dpu_states_in_sync|matches!\\(", body)))
print("replaces every DPU value:", ".states\n                    .keys()" in body)
PY

echo "=== all_equal implementation ==="
rg -n -A 20 '^fn all_equal|^pub fn all_equal|all_equal' crates/machine-controller/src/handler/helpers.rs

echo "=== power-management tests around convergence ==="
cat -n crates/machine-controller/tests/integration/power_management.rs | sed -n '130,205p'

Repository: NVIDIA/infra-controller

Length of output: 1218

🏁 Script executed:

set -euo pipefail

python3 - <<'PY'
from pathlib import Path

helpers = Path("crates/machine-controller/src/handler/helpers.rs").read_text()

start = helpers.index(
    "fn next_state_with_all_dpus_updated(\n        self,\n        current_state: &ManagedHostState,"
)
end = helpers.index("\n    }\n}", start) + len("\n    }")
body = helpers[start:end]

print("=== DpuInitState bulk transition behavior ===")
print("checks DPU state values before replacement:",
      "states.values()" in body or "all_dpu_states_in_sync" in body or "matches!" in body)
print("iterates all DPU keys:", ".states" in body and ".keys()" in body)
print("constructs DPUInit state:", "ManagedHostState::DPUInit" in body)

print("=== mixed-state model ===")
states = {"dpu-a": "WaitingForPlatformPowerOff", "dpu-b": "Init"}
print("before:", states)
if not ("states.values()" in body or "all_dpu_states_in_sync" in body or "matches!" in body):
    states = {key: "WaitingForPlatformPowercycle(On)" for key in states}
print("after bulk transition:", states)
PY

echo "=== all_equal implementation and usages ==="
rg -n -A 25 'all_equal' crates/machine-controller/src/handler/helpers.rs

echo "=== power-management tests around convergence ==="
cat -n crates/machine-controller/tests/integration/power_management.rs | sed -n '130,205p'

Repository: NVIDIA/infra-controller

Length of output: 5766

Add an all-DPU convergence guard before the host power-off check.

next_state_with_all_dpus_updated replaces every DPU state without validating the current states. The per-DPU path can therefore advance all DPUs to WaitingForPlatformPowercycle { On } while another DPU remains in a different state. Return wait unless state.managed_state.all_dpu_states_in_sync()? is true.

🤖 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-controller/src/handler.rs` around lines 5010 - 5035, Add an
all-DPU convergence check at the start of handle_waiting_for_platform_power_off,
returning StateHandlerOutcome::wait unless
state.managed_state.all_dpu_states_in_sync()? is true. Only perform
is_host_powered_off and transition via next_state_with_all_dpus_updated after
synchronization is confirmed.

Source: Path instructions

}

#[async_trait::async_trait]
Expand All @@ -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?;

Expand Down
5 changes: 4 additions & 1 deletion crates/machine-controller/src/io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
}
Expand Down
137 changes: 134 additions & 3 deletions crates/machine-controller/tests/integration/power_management.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<dyn std::error::Error>> {
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,
Expand Down
Loading