diff --git a/crates/api-core/src/tests/dpf/waiting_for_ready.rs b/crates/api-core/src/tests/dpf/waiting_for_ready.rs index 97d070b722..2523c86481 100644 --- a/crates/api-core/src/tests/dpf/waiting_for_ready.rs +++ b/crates/api-core/src/tests/dpf/waiting_for_ready.rs @@ -22,7 +22,7 @@ use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; -use carbide_dpf::{DpuDeploymentType, DpuPhase}; +use carbide_dpf::{DpfError, DpuDeploymentType, DpuPhase}; use carbide_machine_controller::dpf::{DpfOperations, MockDpfOperations}; use carbide_redfish::libredfish::RedfishClientPool; use carbide_redfish::libredfish::test_support::RedfishSimAction; @@ -295,6 +295,155 @@ async fn test_waiting_for_ready_no_reboot(pool: sqlx::PgPool) { ); } +/// Regression for the reprovision race: after `reprovision_dpu` deletes the DPU +/// CR, the old CR lingers (finalizer) with a `deletionTimestamp` while its +/// `status.phase` is still `Ready`. `get_dpu_phase` surfaces that as +/// `DpuPhase::Deleting`; WaitingForReady must keep waiting instead of reading the +/// stale `Ready` and short-circuiting to `DeviceReady`. Once the fresh CR reports +/// `Ready`, it advances. +#[crate::sqlx_test] +async fn test_waiting_for_ready_waits_while_dpu_deleting(pool: sqlx::PgPool) { + let mut mock = MockDpfOperations::new(); + expect_provisioning(&mut mock); + + // True during initial provisioning; flipped to false to simulate the + // terminating old CR observed right after a reprovision delete. + let dpu_ready = Arc::new(AtomicBool::new(true)); + let dr = dpu_ready.clone(); + mock.expect_get_dpu_phase().returning(move |_, _| { + if dr.load(Ordering::SeqCst) { + Ok(DpuPhase::Ready) + } else { + Ok(DpuPhase::Deleting) + } + }); + mock.expect_release_maintenance_hold().returning(|_| Ok(())); + mock.expect_is_reboot_required().returning(|_| Ok(false)); + + let dpf_sdk: Arc = Arc::new(mock); + let mut config = get_config(); + config.dpf = dpf_config(); + + let env = create_test_env_with_overrides( + pool.clone(), + TestEnvOverrides::with_config(config).with_dpf_sdk(dpf_sdk), + ) + .await; + + let mh = timeout(TEST_TIMEOUT, create_managed_host_with_dpf(&env)) + .await + .expect("timed out during initial provisioning"); + + // Simulate the terminating old CR and re-enter WaitingForReady. + dpu_ready.store(false, Ordering::SeqCst); + reset_host_to_waiting_for_ready(&pool, &mh.id, &mh.dpu_ids[0]).await; + + timeout(TEST_TIMEOUT, async { + for _ in 0..5 { + env.run_machine_state_controller_iteration().await; + } + }) + .await + .expect("timed out during state controller iterations"); + + let host = get_host_state(&env, &mh).await; + assert!( + matches!(host, ManagedHostState::DPUInit { .. }) + && !dpf_left_operator_provisioning_substates(&host), + "Host must keep waiting in DPF substates while the DPU CR is Deleting, got: {:?}", + host + ); + + // The operator recreates the DPU; it now reports Ready and provisioning advances. + dpu_ready.store(true, Ordering::SeqCst); + + timeout(TEST_TIMEOUT, async { + env.run_machine_state_controller_iteration().await; + env.run_machine_state_controller_iteration().await; + }) + .await + .expect("timed out during post-ready iterations"); + + let host = get_host_state(&env, &mh).await; + assert!( + dpf_left_operator_provisioning_substates(&host), + "Host should have left DPF operator substates once the fresh DPU CR is Ready, got: {:?}", + host + ); +} + +/// After the operator removes the old DPU CR and before it recreates the new one, +/// `get_dpu_phase` returns `NotFound`. WaitingForReady must treat that window as a +/// wait, not an error, and advance once the fresh CR reports `Ready`. +#[crate::sqlx_test] +async fn test_waiting_for_ready_waits_when_dpu_not_found(pool: sqlx::PgPool) { + let mut mock = MockDpfOperations::new(); + expect_provisioning(&mut mock); + + // True during initial provisioning; flipped to false to simulate the gap + // between the old CR being removed and the new CR being created. + let dpu_ready = Arc::new(AtomicBool::new(true)); + let dr = dpu_ready.clone(); + mock.expect_get_dpu_phase().returning(move |_, _| { + if dr.load(Ordering::SeqCst) { + Ok(DpuPhase::Ready) + } else { + Err(DpfError::not_found("DPU", "node-dpu-001-device-dpu-001")) + } + }); + mock.expect_release_maintenance_hold().returning(|_| Ok(())); + mock.expect_is_reboot_required().returning(|_| Ok(false)); + + let dpf_sdk: Arc = Arc::new(mock); + let mut config = get_config(); + config.dpf = dpf_config(); + + let env = create_test_env_with_overrides( + pool.clone(), + TestEnvOverrides::with_config(config).with_dpf_sdk(dpf_sdk), + ) + .await; + + let mh = timeout(TEST_TIMEOUT, create_managed_host_with_dpf(&env)) + .await + .expect("timed out during initial provisioning"); + + dpu_ready.store(false, Ordering::SeqCst); + reset_host_to_waiting_for_ready(&pool, &mh.id, &mh.dpu_ids[0]).await; + + timeout(TEST_TIMEOUT, async { + for _ in 0..5 { + env.run_machine_state_controller_iteration().await; + } + }) + .await + .expect("timed out during state controller iterations"); + + let host = get_host_state(&env, &mh).await; + assert!( + matches!(host, ManagedHostState::DPUInit { .. }) + && !dpf_left_operator_provisioning_substates(&host), + "Host must keep waiting in DPF substates while the DPU CR is absent, got: {:?}", + host + ); + + dpu_ready.store(true, Ordering::SeqCst); + + timeout(TEST_TIMEOUT, async { + env.run_machine_state_controller_iteration().await; + env.run_machine_state_controller_iteration().await; + }) + .await + .expect("timed out during post-ready iterations"); + + let host = get_host_state(&env, &mh).await; + assert!( + dpf_left_operator_provisioning_substates(&host), + "Host should have left DPF operator substates once the fresh DPU CR is Ready, got: {:?}", + host + ); +} + /// WaitingForReady idempotent reboot: ForceOff is only sent once, /// not on every iteration while waiting for the host to come back. #[crate::sqlx_test] diff --git a/crates/dpf/src/sdk.rs b/crates/dpf/src/sdk.rs index 4a17b23440..a18d9b30c0 100644 --- a/crates/dpf/src/sdk.rs +++ b/crates/dpf/src/sdk.rs @@ -1569,6 +1569,13 @@ impl DpfSdk { return Err(DpfError::not_found("DPU", cr_name)); }; + // A DPU being torn down (e.g. right after reprovision deleted it) still reports + // its old status.phase (often Ready) until the operator's finalizer runs. Treat + // a set deletionTimestamp as authoritative so callers never act on the stale phase. + if dpu.metadata.deletion_timestamp.is_some() { + return Ok(DpuPhase::Deleting); + } + let Some(status) = dpu.status else { return Err(DpfError::InvalidState(format!( "DPU {cr_name} has no status" @@ -2919,6 +2926,94 @@ mod tests { assert_eq!(devices.len(), 1, "DPUDevice should remain"); } + #[tokio::test] + async fn test_get_dpu_phase_reports_deleting_when_terminating() { + use kube::core::ObjectMeta; + + use crate::crds::dpus_generated::{DpuSpec, DpuStatus, DpuStatusPhase}; + + let mock = SdkMock::new(); + let sdk = DpfSdkBuilder::new(mock.clone(), TEST_NAMESPACE, String::new()) + .build_without_resources() + .await + .unwrap(); + + // A DPU that has been deleted (reprovision) but whose finalizer has not yet + // run: it carries a deletionTimestamp while its status.phase is still Ready. + let dpu_name = "node-dpu-001-device-dpu-001"; + let dpu = DPU { + metadata: ObjectMeta { + name: Some(dpu_name.to_string()), + namespace: Some(TEST_NAMESPACE.to_string()), + deletion_timestamp: Some(terminating_timestamp()), + ..Default::default() + }, + spec: DpuSpec { + bfb: Some("bf-bundle".to_string()), + bmc_ip: None, + cluster: None, + dpu_device_name: "dpu-001".to_string(), + dpu_flavor: crate::flavor::DEFAULT_FLAVOR_NAME.to_string(), + dpu_node_name: "node-dpu-001".to_string(), + node_effect: DpuNodeEffect { + apply_on_label_change: None, + custom_action: None, + custom_label: None, + drain: None, + force: None, + hold: None, + no_effect: None, + node_maintenance_additional_requestors: None, + taint: None, + }, + pci_address: None, + serial_number: "SN123".to_string(), + blue_field_software: None, + secure_boot: None, + astra_enabled: None, + }, + status: Some(DpuStatus { + phase: DpuStatusPhase::Ready, + addresses: None, + bf_cfg_file: None, + bfb_file: None, + bfb_version: None, + conditions: None, + dpf_version: None, + dpu_install_interface: None, + dpu_mode: None, + firmware: None, + observed_generation: None, + pci_device: None, + post_provisioning_node_effect: None, + required_reset: None, + agent_last_startup_time: None, + agent_status: None, + dpu_type: None, + operational_conditions: None, + previous_phase: None, + redfish_task_id: None, + secure_boot: None, + deployment_mode: None, + hostless: None, + identity_mode: None, + outdated: None, + reboot_status: None, + }), + }; + mock.dpus + .write() + .unwrap() + .insert(format!("{}/{}", TEST_NAMESPACE, dpu_name), dpu); + + let phase = sdk.get_dpu_phase("dpu-001", "node-dpu-001").await.unwrap(); + assert_eq!( + phase, + DpuPhase::Deleting, + "a DPU with a deletionTimestamp must report Deleting even though its stale status.phase is Ready" + ); + } + #[tokio::test] async fn test_namespace_isolation() { let mock = SdkMock::new(); diff --git a/crates/machine-controller/src/handler/dpf.rs b/crates/machine-controller/src/handler/dpf.rs index f30c100bb8..f65aab6f60 100644 --- a/crates/machine-controller/src/handler/dpf.rs +++ b/crates/machine-controller/src/handler/dpf.rs @@ -436,10 +436,29 @@ async fn handle_dpf_waiting_for_ready( ) -> Result, StateHandlerError> { let node_name = dpu_node_cr_name(&dpf_id(&state.host_snapshot)?); let dpu_device_name = dpf_id(dpu_snapshot)?; - let current_phase = dpf_sdk - .get_dpu_phase(&dpu_device_name, &node_name) - .await - .map_err(dpf_error)?; + let current_phase = match dpf_sdk.get_dpu_phase(&dpu_device_name, &node_name).await { + Ok(phase) => phase, + // The operator briefly removes the old DPU CR before recreating a fresh one + // during reprovision; treat that window as "keep waiting" rather than erroring. + Err(DpfError::NotFound { .. }) => { + return Ok(StateHandlerOutcome::wait( + "DPU CR not yet recreated after reprovision".to_string(), + )); + } + Err(err) => return Err(dpf_error(err)), + }; + + // A DPU with a deletionTimestamp is a terminating old CR (get_dpu_phase maps that to + // Deleting; its status.phase is still stale, often Ready or Error). Do nothing until + // the operator has deleted it and created the fresh CR: don't release the maintenance + // hold, don't trigger a reboot, and don't read Ready/Error off it. This is what lets a + // reprovision of an *errored* DPU proceed instead of immediately re-failing on the old + // CR's stale Error phase. + if current_phase == DpuPhase::Deleting { + return Ok(StateHandlerOutcome::wait( + "DPU CR is being deleted (reprovision in progress); waiting for the new CR".to_string(), + )); + } dpf_sdk .release_maintenance_hold(&node_name)