From 6ca5d12e4bb0b92d6f51f093eabf0d16f8eb6fe0 Mon Sep 17 00:00:00 2001 From: Chet Nichols III Date: Fri, 10 Jul 2026 09:53:14 -0700 Subject: [PATCH] feat: ensure a correct boot config during machine validation Machine validation now ensures the host's boot device config is what it should be, and corrects it when it isn't. While waiting for the host to boot into the validation environment, validation verifies the boot config; if it reads reverted -- however it drifted, whether someone changed it externally, a BIOS quirk, or the boot NIC dropping off the BMC's inventory during the reboot's POST -- validation unlocks the BMC, re-drives the boot-order flow (re-assert the HTTP-boot device, reboot to apply, set and verify the order), re-locks, and resumes from its reboot step. Without this, a drifted boot config was unrecoverable at this stage: the host can't boot into scout, more reboots can't restore a setting that is gone, and the BMC lockdown enabled earlier in HostInit blocks any fix -- so validation paced reboots indefinitely. - Verification lives in the wait-for-reboot path: an intact config keeps the normal reboot pacing; only a reverted one enters correction. - The correction reuses the boot-order flow and the host-boot-repair unlock/re-lock choreography, honoring the expected-machine lockdown policy. - New validation substates (PrepareBootRepair, UnlockForBootRepair, RepairBootConfig, LockAfterBootRepair) surface the correction in state history and metrics. Tests updated! This supports https://github.com/NVIDIA/infra-controller/issues/3365 Signed-off-by: Chet Nichols III --- .../src/tests/common/api_fixtures/mod.rs | 4 + crates/api-core/src/tests/machine_states.rs | 111 ++++++++ crates/api-model/src/machine/mod.rs | 25 ++ .../src/handler/machine_validation.rs | 269 +++++++++++++++++- crates/machine-controller/src/io.rs | 4 + 5 files changed, 410 insertions(+), 3 deletions(-) diff --git a/crates/api-core/src/tests/common/api_fixtures/mod.rs b/crates/api-core/src/tests/common/api_fixtures/mod.rs index a171ef29bd..c8d06f8d2f 100644 --- a/crates/api-core/src/tests/common/api_fixtures/mod.rs +++ b/crates/api-core/src/tests/common/api_fixtures/mod.rs @@ -474,6 +474,10 @@ impl TestEnv { } } MachineValidatingState::RebootHost { .. } => state.clone(), + MachineValidatingState::PrepareBootRepair { .. } + | MachineValidatingState::UnlockForBootRepair { .. } + | MachineValidatingState::RepairBootConfig { .. } + | MachineValidatingState::LockAfterBootRepair { .. } => state.clone(), } } }, diff --git a/crates/api-core/src/tests/machine_states.rs b/crates/api-core/src/tests/machine_states.rs index 8740245110..5fccda3bc6 100644 --- a/crates/api-core/src/tests/machine_states.rs +++ b/crates/api-core/src/tests/machine_states.rs @@ -3151,6 +3151,117 @@ async fn test_set_boot_order_reassert_window_expiry_bounds_reapplies(pool: sqlx: } } +/// A host that cannot return from a validation reboot because its boot config +/// reverted (the boot NIC dropped off the BMC's inventory during POST, so the +/// BIOS reset the HTTP-boot device) gets repaired in place: validation unlocks +/// the BMC, re-drives the boot-order flow (re-assert, reboot to apply, +/// reorder, verify), re-locks, and resumes validation from its reboot step -- +/// instead of pacing reboots that can never succeed. +#[crate::sqlx_test] +async fn test_machine_validation_repairs_reverted_boot_config(pool: sqlx::PgPool) { + let env = create_zero_dpu_test_env(pool).await; + + let mh = create_managed_host_with_config(&env, ManagedHostConfig::zero_dpu()).await; + let host_id = mh.host().id; + let boot_nic_mac = host_inband_nic_mac(&env, host_id).await; + + // The validation reboot's POST reverted the boot config, and HostInit + // enabled lockdown before validation reached this point: BIOS writes + // bounce off the BMC until it is unlocked. + env.redfish_sim.set_is_bios_setup(false); + env.redfish_sim.set_is_boot_order_setup(false); + env.redfish_sim + .set_lockdown(libredfish::EnabledDisabled::Enabled); + + // Park the host mid-validation, waiting on a reboot that cannot land + // (the state is newer than the host's last reported boot). + set_host_controller_state_stuck_in( + &env, + host_id, + &ManagedHostState::Validation { + validation_state: ValidationState::MachineValidation { + machine_validation: MachineValidatingState::MachineValidating { + context: "Discovery".to_string(), + id: MachineValidationId::new(), + completed: 1, + total: 1, + is_enabled: true, + }, + }, + }, + 0, + ) + .await; + + let checkpoint = env.redfish_sim.timepoint(); + + // Detection through re-assert: the flow notices the reverted config, + // unlocks the BMC, and re-asserts the HTTP-boot device. + for _ in 0..6 { + env.run_machine_state_controller_iteration().await; + if env + .redfish_sim + .actions_since(&checkpoint) + .all_hosts() + .iter() + .any(|a| matches!(a, RedfishSimAction::MachineSetup { .. })) + { + break; + } + } + let actions = env.redfish_sim.actions_since(&checkpoint).all_hosts(); + assert!( + actions + .iter() + .any(|a| matches!(a, RedfishSimAction::MachineSetup { .. })), + "validation boot repair should re-assert the reverted HTTP boot device, got: {actions:?}" + ); + assert!( + env.redfish_sim + .lockdown_states() + .contains(&libredfish::EnabledDisabled::Disabled), + "boot repair should unlock the BMC before writing BIOS settings" + ); + + // The repair reboot applies the re-asserted device. + env.redfish_sim.set_is_bios_setup(true); + + // The repair completes (reorder + verify), re-locks the BMC, and resumes + // validation from its reboot step. + let mut resumed = false; + for _ in 0..20 { + env.run_machine_state_controller_iteration().await; + let mut txn = env.db_txn().await; + let host = mh.host().db_machine(&mut txn).await; + if matches!( + host.current_state(), + ManagedHostState::Validation { + validation_state: ValidationState::MachineValidation { + machine_validation: MachineValidatingState::RebootHost { .. }, + }, + } + ) { + resumed = true; + break; + } + } + assert!( + resumed, + "the repaired host should resume validation from its reboot step" + ); + assert!( + env.redfish_sim + .lockdown_states() + .iter() + .all(|l| *l == libredfish::EnabledDisabled::Enabled), + "boot repair should re-lock the BMC once the boot config is repaired" + ); + + // Across the repair, the re-assert targeting the boot NIC preceded the reorder. + let actions = env.redfish_sim.actions_since(&checkpoint).all_hosts(); + assert_machine_setup_precedes_reorder_for(&actions, boot_nic_mac); +} + /// When HostInit/PollingBiosSetup retry budget is exhausted, enter Failed and recover via is_bios_setup. #[crate::sqlx_test] async fn test_polling_bios_setup_exhausted_enters_failed_and_recovers_when_bios_setup_true( diff --git a/crates/api-model/src/machine/mod.rs b/crates/api-model/src/machine/mod.rs index 914b20b5cc..d76e60779c 100644 --- a/crates/api-model/src/machine/mod.rs +++ b/crates/api-model/src/machine/mod.rs @@ -1284,6 +1284,25 @@ pub enum MachineValidatingState { #[serde(default = "default_true")] is_enabled: bool, }, + /// Machine validation ensures the host's boot device config is in place. + /// When it reads reverted -- however it drifted (changed externally, a + /// BIOS quirk, or the boot NIC dropping off the BMC's inventory during a + /// reboot's POST) -- these states correct it, mirroring host boot repair: + /// unlock the BMC, drive the boot-order flow, re-lock, resume validation. + PrepareBootRepair { + validation_id: MachineValidationId, + }, + UnlockForBootRepair { + validation_id: MachineValidationId, + unlock_host_state: UnlockHostState, + }, + RepairBootConfig { + validation_id: MachineValidationId, + set_boot_order_info: SetBootOrderInfo, + }, + LockAfterBootRepair { + validation_id: MachineValidationId, + }, } #[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)] #[serde(tag = "validation_type", rename_all = "lowercase")] @@ -2662,6 +2681,12 @@ pub fn state_sla( MachineValidatingState::RebootHost { .. } => { StateSla::with_sla(slas::VALIDATION, time_in_state) } + MachineValidatingState::PrepareBootRepair { .. } + | MachineValidatingState::UnlockForBootRepair { .. } + | MachineValidatingState::RepairBootConfig { .. } + | MachineValidatingState::LockAfterBootRepair { .. } => { + StateSla::with_sla(slas::VALIDATION, time_in_state) + } }, }, } diff --git a/crates/machine-controller/src/handler/machine_validation.rs b/crates/machine-controller/src/handler/machine_validation.rs index 79f975d0cd..7950f844cd 100644 --- a/crates/machine-controller/src/handler/machine_validation.rs +++ b/crates/machine-controller/src/handler/machine_validation.rs @@ -15,10 +15,10 @@ * limitations under the License. */ use carbide_uuid::machine_validation::MachineValidationId; -use libredfish::SystemPowerControl; +use libredfish::{EnabledDisabled, RedfishError, SystemPowerControl}; use model::machine::{ FailureCause, MachineState, MachineValidatingState, ManagedHostState, ManagedHostStateSnapshot, - ValidationState, + SetBootOrderInfo, SetBootOrderState, UnlockHostState, ValidationState, }; use model::machine_validation::{MachineValidationState, MachineValidationStatus}; use state_controller::state_handler::{ @@ -27,7 +27,30 @@ use state_controller::state_handler::{ use super::{HostHandlerParams, is_machine_validation_requested, machine_validation_completed}; use crate::context::{MachineStateHandlerContextObjects, MachineStateHandlerServices}; -use crate::handler::{handler_host_power_control, rebooted, trigger_reboot_if_needed}; +use crate::handler::{ + RequiredBootInterface, SetBootOrderOutcome, handler_host_power_control, host_power_control, + load_boot_predictions, rebooted, redfish_error, require_boot_interface, set_host_boot_order, + trigger_reboot_if_needed, wait, +}; + +/// The validation flavor of the managed-host state, so the repair arms can +/// transition between validation substates without repeating the wrapper. +fn validating(machine_validation: MachineValidatingState) -> ManagedHostState { + ManagedHostState::Validation { + validation_state: ValidationState::MachineValidation { machine_validation }, + } +} + +/// The starting point for the boot-order flow when validation boot repair +/// re-drives it: the flow itself checks what is already in place and only +/// re-applies what the reboot reverted. +fn boot_repair_start() -> SetBootOrderInfo { + SetBootOrderInfo { + set_boot_order_jid: None, + set_boot_order_state: SetBootOrderState::SetBootOrder, + retry_count: 0, + } +} async fn skip_machine_validation( ctx: &mut StateHandlerContext<'_, MachineStateHandlerContextObjects>, @@ -122,6 +145,46 @@ pub(crate) async fn handle_machine_validation_state( is_enabled, ); if !rebooted(&mh_snapshot.host_snapshot) { + // Ensure the boot config is still what it should be while + // waiting. If it reads reverted -- changed externally, a BIOS + // quirk, or the boot NIC dropping off the BMC's inventory + // during the reboot's POST -- the host can't boot into the + // validation environment, and further reboots can't restore a + // setting that is gone. Correct it instead of pacing forever. + let predictions = load_boot_predictions(ctx, &mh_snapshot.host_snapshot.id).await?; + if let RequiredBootInterface::Ready(boot_interface) = require_boot_interface( + mh_snapshot, + &predictions, + "verifying the boot config during validation", + |message| message, + )? { + let redfish_client = ctx + .services + .create_redfish_client_from_machine(&mh_snapshot.host_snapshot) + .await?; + match boot_interface + .run(|bi| redfish_client.is_bios_setup(Some(bi))) + .await + { + Ok(false) => { + tracing::warn!( + machine_id = %mh_snapshot.host_snapshot.id, + "Boot config reads reverted while waiting for the validation reboot; correcting it via boot repair", + ); + return Ok(StateHandlerOutcome::transition(validating( + MachineValidatingState::PrepareBootRepair { validation_id: *id }, + ))); + } + Ok(true) => {} + Err(e) => { + tracing::warn!( + machine_id = %mh_snapshot.host_snapshot.id, + error = %e, + "Could not verify the boot config while waiting for the validation reboot; will retry", + ); + } + } + } let status = trigger_reboot_if_needed( &mh_snapshot.host_snapshot, mh_snapshot, @@ -174,6 +237,206 @@ pub(crate) async fn handle_machine_validation_state( } Ok(StateHandlerOutcome::do_nothing()) } + MachineValidatingState::PrepareBootRepair { validation_id } => { + // Boot repair writes BIOS settings, and lockdown was enabled + // earlier in HostInit -- writes bounce off a locked BMC. Check and + // unlock first, mirroring host boot repair. + let redfish_client = ctx + .services + .create_redfish_client_from_machine(&mh_snapshot.host_snapshot) + .await?; + + let next = match redfish_client.lockdown_status().await { + Err(RedfishError::NotSupported(_)) => { + tracing::info!( + machine_id = %mh_snapshot.host_snapshot.id, + "BMC vendor does not support checking lockdown status during validation boot repair", + ); + MachineValidatingState::RepairBootConfig { + validation_id: *validation_id, + set_boot_order_info: boot_repair_start(), + } + } + Err(e) => { + tracing::warn!( + machine_id = %mh_snapshot.host_snapshot.id, + error = %e, + "Failed to fetch lockdown status during validation boot repair", + ); + return Ok(StateHandlerOutcome::wait(format!( + "Failed to fetch lockdown status: {e}" + ))); + } + Ok(lockdown_status) if !lockdown_status.is_fully_disabled() => { + tracing::info!( + machine_id = %mh_snapshot.host_snapshot.id, + "Lockdown is enabled during validation boot repair; disabling before boot config writes", + ); + MachineValidatingState::UnlockForBootRepair { + validation_id: *validation_id, + unlock_host_state: UnlockHostState::DisableLockdown, + } + } + Ok(_) => MachineValidatingState::RepairBootConfig { + validation_id: *validation_id, + set_boot_order_info: boot_repair_start(), + }, + }; + + Ok(StateHandlerOutcome::transition(validating(next))) + } + MachineValidatingState::UnlockForBootRepair { + validation_id, + unlock_host_state, + } => { + // Mirror host boot repair's unlock choreography. + let redfish_client = ctx + .services + .create_redfish_client_from_machine(&mh_snapshot.host_snapshot) + .await?; + + let next = match unlock_host_state { + UnlockHostState::DisableLockdown => { + // Tolerate a vendor that reports lockdown status but does + // not support setting it, symmetric with the re-lock step. + match redfish_client.lockdown_bmc(EnabledDisabled::Disabled).await { + Ok(()) => {} + Err(RedfishError::NotSupported(_)) => { + tracing::info!( + machine_id = %mh_snapshot.host_snapshot.id, + "BMC vendor does not support disabling lockdown during validation boot repair", + ); + } + Err(e) => return Err(redfish_error("lockdown_bmc", e)), + } + + let vendor = mh_snapshot.host_snapshot.bmc_vendor(); + if vendor.is_supermicro() { + tracing::info!( + machine_id = %mh_snapshot.host_snapshot.id, + %vendor, + "BMC lockdown disabled; rebooting host so Redfish reflects actual boot state", + ); + MachineValidatingState::UnlockForBootRepair { + validation_id: *validation_id, + unlock_host_state: UnlockHostState::RebootHost, + } + } else { + MachineValidatingState::RepairBootConfig { + validation_id: *validation_id, + set_boot_order_info: boot_repair_start(), + } + } + } + UnlockHostState::RebootHost => { + host_power_control( + redfish_client.as_ref(), + &mh_snapshot.host_snapshot, + SystemPowerControl::ForceRestart, + ctx, + ) + .await + .map_err(|e| { + StateHandlerError::GenericError(eyre::eyre!( + "failed to ForceRestart host after disabling BMC lockdown: {}", + e + )) + })?; + + MachineValidatingState::UnlockForBootRepair { + validation_id: *validation_id, + unlock_host_state: UnlockHostState::WaitForUefiBoot, + } + } + UnlockHostState::WaitForUefiBoot => { + let entered_at = mh_snapshot.host_snapshot.state.version.timestamp(); + if wait( + &entered_at, + host_handler_params.reachability_params.uefi_boot_wait, + ) { + return Ok(StateHandlerOutcome::wait(format!( + "Waiting for UEFI boot to complete on {} after post-unlock reboot", + mh_snapshot.host_snapshot.id + ))); + } + MachineValidatingState::RepairBootConfig { + validation_id: *validation_id, + set_boot_order_info: boot_repair_start(), + } + } + }; + + Ok(StateHandlerOutcome::transition(validating(next))) + } + MachineValidatingState::RepairBootConfig { + validation_id, + set_boot_order_info, + } => { + // Re-drive the boot-order flow: it checks what is already in + // place, re-asserts the reverted HTTP-boot device (rebooting to + // apply it), sets the boot order, and verifies -- the same engine + // the boot-configuration stages use. + let redfish_client = ctx + .services + .create_redfish_client_from_machine(&mh_snapshot.host_snapshot) + .await?; + + match set_host_boot_order( + ctx, + &host_handler_params.reachability_params, + redfish_client.as_ref(), + mh_snapshot, + set_boot_order_info.clone(), + ) + .await? + { + SetBootOrderOutcome::Continue(info) => Ok(StateHandlerOutcome::transition( + validating(MachineValidatingState::RepairBootConfig { + validation_id: *validation_id, + set_boot_order_info: info, + }), + )), + SetBootOrderOutcome::Done => Ok(StateHandlerOutcome::transition(validating( + MachineValidatingState::LockAfterBootRepair { + validation_id: *validation_id, + }, + ))), + SetBootOrderOutcome::WaitingForReboot(reason) + | SetBootOrderOutcome::Wait(reason) => Ok(StateHandlerOutcome::wait(reason)), + } + } + MachineValidatingState::LockAfterBootRepair { validation_id } => { + // Restore the lockdown that boot repair temporarily opened, then + // resume validation from its reboot step. + let redfish_client = ctx + .services + .create_redfish_client_from_machine(&mh_snapshot.host_snapshot) + .await?; + + if mh_snapshot.host_snapshot.host_profile.disable_lockdown { + tracing::info!( + machine_id = %mh_snapshot.host_snapshot.id, + "Skipping lockdown re-enable after validation boot repair per expected-machine config", + ); + } else { + match redfish_client.lockdown_bmc(EnabledDisabled::Enabled).await { + Ok(()) => {} + Err(RedfishError::NotSupported(_)) => { + tracing::info!( + machine_id = %mh_snapshot.host_snapshot.id, + "BMC vendor does not support re-enabling lockdown after validation boot repair", + ); + } + Err(e) => return Err(redfish_error("lockdown_bmc", e)), + } + } + + Ok(StateHandlerOutcome::transition(validating( + MachineValidatingState::RebootHost { + validation_id: *validation_id, + }, + ))) + } } } diff --git a/crates/machine-controller/src/io.rs b/crates/machine-controller/src/io.rs index d514116ff1..e831a4d975 100644 --- a/crates/machine-controller/src/io.rs +++ b/crates/machine-controller/src/io.rs @@ -264,6 +264,10 @@ impl StateControllerIO for MachineStateControllerIO { match validation_state { MachineValidatingState::MachineValidating { .. } => "machinevalidating", MachineValidatingState::RebootHost { .. } => "reboothost", + MachineValidatingState::PrepareBootRepair { .. } => "preparebootrepair", + MachineValidatingState::UnlockForBootRepair { .. } => "unlockforbootrepair", + MachineValidatingState::RepairBootConfig { .. } => "repairbootconfig", + MachineValidatingState::LockAfterBootRepair { .. } => "lockafterbootrepair", } } match state {