From 3c98b77414900f9f113b66a0c7c559748c26a358 Mon Sep 17 00:00:00 2001 From: Chet Nichols III Date: Fri, 31 Jul 2026 18:48:06 -0700 Subject: [PATCH] feat: detect verified boot configuration drift Once a host's desired boot interface is verified, later manual or firmware changes in Redfish do not change its database version. That meant a `Ready` host had no reason to look again, so the database could still say the target was verified after the BMC had drifted. This adds a read-only check as the final work for `Ready` and for `Assigned` hosts whose `InstanceState` is `Ready`: - `boot_interface_observation_interval` waits 10 minutes after a successful read instead of contacting every healthy BMC on each 30-second controller pass. - A match refreshes `observed_at`; drift reopens the exact `Pair` or `MacOnly` target as one pending generation, but only if operator intent has not changed while Redfish was being read. - `Ready` reuses `BootConfiguring`, while `Assigned` keeps the pending work until release. - Failed reads change nothing and are tried again on the next controller pass. Managed-DPU hosts wait for current network observations, and locked Supermicro hosts skip the read because their boot-order view is stale until the existing unlock/reboot flow. The observation path itself never changes Redfish. This supports https://github.com/NVIDIA/infra-controller/issues/4248 Tests added! Signed-off-by: Chet Nichols III --- crates/api-core/src/cfg/README.md | 13 + crates/api-core/src/cfg/file.rs | 19 + .../src/cfg/test_data/full_config.toml | 1 + .../test_data/full_config_post_migration.toml | 1 + .../src/cfg/test_data/site_config.toml | 1 + crates/api-core/src/tests/machine_states.rs | 410 +++++++++++++++++- .../src/machine_desired_boot_interface.rs | 172 ++++++++ .../src/config/controller.rs | 33 ++ crates/machine-controller/src/handler.rs | 13 +- .../src/handler/boot_interface_observation.rs | 317 ++++++++++++++ 10 files changed, 977 insertions(+), 3 deletions(-) create mode 100644 crates/machine-controller/src/handler/boot_interface_observation.rs diff --git a/crates/api-core/src/cfg/README.md b/crates/api-core/src/cfg/README.md index ddefbb17c0..9ebe50eb0e 100644 --- a/crates/api-core/src/cfg/README.md +++ b/crates/api-core/src/cfg/README.md @@ -386,8 +386,21 @@ Extends `StateControllerConfig` with: | `uefi_boot_wait` | `Duration` | `5m` | Wait time for UEFI boot completion after host reboot. | | `max_bios_config_retries` | `u32` | `3` | Shared retry budget for automated host boot-configuration convergence across BIOS recovery and boot-order verification. | | `polling_bios_setup_stuck_threshold` | `Duration` | `15m` | Time in PollingBiosSetup with `is_bios_setup == false` before recovery escalation. | +| `boot_interface_observation_interval` | `Duration` | `10m` | Positive time between successful Redfish observations of an already-verified boot interface. | | `controller` | `StateControllerConfig` | *(default)* | Common state controller timing (see [StateControllerConfig](#statecontrollerconfig)). | +The Redfish observation is read-only. A successful match refreshes the last +observation timestamp. A mismatch records a new pending generation for the same +desired target: Ready enters the existing boot-configuration flow on its next +controller sweep, while Assigned defers remediation until release. Failed reads +and skipped observations preserve the last successful observation and retry on +a later controller iteration. + +The controller skips periodic observation for locked Supermicro hosts because +their reported boot-order view remains stale until lockdown is disabled and the +host is rebooted. Profiles configured with `disable_lockdown = true` use the +normal observation path. + ### `NetworkSegmentStateControllerConfig` Extends `StateControllerConfig` with: diff --git a/crates/api-core/src/cfg/file.rs b/crates/api-core/src/cfg/file.rs index ca602339d7..acebfd4b57 100644 --- a/crates/api-core/src/cfg/file.rs +++ b/crates/api-core/src/cfg/file.rs @@ -3975,6 +3975,7 @@ mod tests { uefi_boot_wait: Duration::minutes(5), max_bios_config_retries: 3, polling_bios_setup_stuck_threshold: Duration::minutes(15), + boot_interface_observation_interval: Duration::hours(2), }; let config_str = serde_json::to_string(&input).unwrap(); @@ -3995,6 +3996,7 @@ mod tests { fn deserialize_machine_controller_config() { let config = r#"{"dpu_wait_time": "20m","power_down_wait":"10s", "failure_retry_time":"1h30m", "dpu_up_threshold": "1w", + "boot_interface_observation_interval": "2h", "controller": {"iteration_time": "33s", "max_object_handling_time": "63s", "max_concurrency": 13}}"#; let config: MachineStateControllerConfig = serde_json::from_str(config).unwrap(); @@ -4021,6 +4023,7 @@ mod tests { uefi_boot_wait: Duration::minutes(5), max_bios_config_retries: 3, polling_bios_setup_stuck_threshold: Duration::minutes(15), + boot_interface_observation_interval: Duration::hours(2), } ); } @@ -4044,10 +4047,23 @@ mod tests { uefi_boot_wait: Duration::minutes(5), max_bios_config_retries: 3, polling_bios_setup_stuck_threshold: Duration::minutes(15), + boot_interface_observation_interval: Duration::minutes(10), } ); } + #[test] + fn reject_nonpositive_boot_interface_observation_intervals() { + for invalid_interval in ["0s", "-1s"] { + let config_json = + format!(r#"{{"boot_interface_observation_interval": "{invalid_interval}"}}"#); + assert!( + serde_json::from_str::(&config_json).is_err(), + "boot_interface_observation_interval={invalid_interval} must be rejected", + ); + } + } + #[test] fn deserialize_network_segment_state_controller_config() { let config = r#"{"network_segment_drain_time": "21m", @@ -4469,6 +4485,7 @@ mod tests { uefi_boot_wait: Duration::minutes(5), max_bios_config_retries: 3, polling_bios_setup_stuck_threshold: Duration::minutes(15), + boot_interface_observation_interval: Duration::hours(2), } ); assert_eq!( @@ -4710,6 +4727,7 @@ mod tests { uefi_boot_wait: Duration::minutes(5), max_bios_config_retries: 3, polling_bios_setup_stuck_threshold: Duration::minutes(15), + boot_interface_observation_interval: Duration::minutes(10), } ); assert_eq!( @@ -5094,6 +5112,7 @@ mod tests { uefi_boot_wait: Duration::minutes(5), max_bios_config_retries: 3, polling_bios_setup_stuck_threshold: Duration::minutes(15), + boot_interface_observation_interval: Duration::hours(2), } ); assert_eq!( diff --git a/crates/api-core/src/cfg/test_data/full_config.toml b/crates/api-core/src/cfg/test_data/full_config.toml index b065cde731..5a92610a04 100644 --- a/crates/api-core/src/cfg/test_data/full_config.toml +++ b/crates/api-core/src/cfg/test_data/full_config.toml @@ -95,6 +95,7 @@ power_down_wait = "13s" failure_retry_time = "31m" dpu_up_threshold = "33m" scout_reporting_timeout = "20m" +boot_interface_observation_interval = "10m" [host_health] prevent_allocations_on_stale_dpu_agent_version = true diff --git a/crates/api-core/src/cfg/test_data/full_config_post_migration.toml b/crates/api-core/src/cfg/test_data/full_config_post_migration.toml index e722a37355..ec360162b0 100644 --- a/crates/api-core/src/cfg/test_data/full_config_post_migration.toml +++ b/crates/api-core/src/cfg/test_data/full_config_post_migration.toml @@ -56,6 +56,7 @@ power_down_wait = "13s" failure_retry_time = "31m" dpu_up_threshold = "33m" scout_reporting_timeout = "20m" +boot_interface_observation_interval = "10m" [host_health] prevent_allocations_on_stale_dpu_agent_version = true diff --git a/crates/api-core/src/cfg/test_data/site_config.toml b/crates/api-core/src/cfg/test_data/site_config.toml index be2b2f7526..39dace90cd 100644 --- a/crates/api-core/src/cfg/test_data/site_config.toml +++ b/crates/api-core/src/cfg/test_data/site_config.toml @@ -41,6 +41,7 @@ dpu_wait_time = "7m" power_down_wait = "17s" failure_retry_time = "70m" dpu_up_threshold = "77m" +boot_interface_observation_interval = "2h" [machine_state_controller.controller] iteration_time = "3m" max_object_handling_time = "11s" diff --git a/crates/api-core/src/tests/machine_states.rs b/crates/api-core/src/tests/machine_states.rs index 141fdde1a6..a21764f589 100644 --- a/crates/api-core/src/tests/machine_states.rs +++ b/crates/api-core/src/tests/machine_states.rs @@ -24,7 +24,7 @@ use base64::prelude::*; use carbide_machine_controller::context::MachineStateHandlerContextObjects; use carbide_machine_controller::handler::{MachineStateHandlerBuilder, handler_host_power_control}; use carbide_machine_controller::metrics::MachineMetrics; -use carbide_redfish::libredfish::test_support::RedfishSimAction; +use carbide_redfish::libredfish::test_support::{RedfishSimAction, RedfishSimPlatformAction}; use carbide_site_explorer::MachineCreator; use carbide_site_explorer::config::SiteExplorerConfig; use carbide_utils::arch::CpuArchitecture; @@ -2988,6 +2988,89 @@ async fn set_pending_boot_interface( pending } +/// Makes a verified target eligible for periodic observation without changing +/// its desired or verified generation. +async fn backdate_boot_interface_observation( + env: &TestEnv, + managed_host: &TestManagedHost, +) -> model::machine::Machine { + let mut txn = env.db_txn().await; + let host_before = managed_host.host().db_machine(&mut txn).await; + let desired_boot_interface = host_before + .config + .desired_boot_interface + .as_ref() + .expect("fixture should have a desired boot interface"); + let verified_observation = host_before + .status + .boot_interface_status_observation + .as_ref() + .expect("fixture should have a verified boot interface"); + assert_eq!( + verified_observation.config_version, desired_boot_interface.version, + "periodic observation requires an already-verified generation" + ); + + let backdate_update = sqlx::query( + "UPDATE machine_boot_interfaces \ + SET observed_at = CURRENT_TIMESTAMP - INTERVAL '100 years' \ + WHERE machine_id = $1 AND desired_version = verified_version", + ) + .bind(host_before.id) + .execute(txn.as_mut()) + .await + .expect("backdating the verified boot-interface observation should succeed"); + assert_eq!(backdate_update.rows_affected(), 1); + let backdated_host = managed_host.host().db_machine(&mut txn).await; + txn.commit() + .await + .expect("backdated boot-interface observation should commit"); + backdated_host +} + +/// Verifies that periodic observation stayed on the read-only Redfish boundary. +fn assert_read_only_boot_interface_observation(redfish_actions: &[RedfishSimAction]) { + assert!( + redfish_actions + .iter() + .any(|action| matches!(action, RedfishSimAction::IsBootOrderSetup { .. })), + "the controller should read the boot order: {redfish_actions:?}" + ); + assert!( + redfish_actions.iter().all(|action| !matches!( + action, + RedfishSimAction::MachineSetup { .. } + | RedfishSimAction::SetBootOrderDpuFirst { .. } + | RedfishSimAction::Power(_) + )), + "periodic observation must not mutate Redfish: {redfish_actions:?}" + ); +} + +/// Verifies that periodic observation was skipped before any Redfish read or +/// mutation reached the host or its platform. +fn assert_no_boot_interface_observation( + redfish_actions: &[RedfishSimAction], + platform_actions: &[RedfishSimPlatformAction], +) { + assert!( + redfish_actions.iter().all(|action| !matches!( + action, + RedfishSimAction::IsBootOrderSetup { .. } + | RedfishSimAction::MachineSetup { .. } + | RedfishSimAction::SetBootOrderDpuFirst { .. } + | RedfishSimAction::Power(_) + )), + "periodic observation must skip Redfish entirely: {redfish_actions:?}" + ); + assert!( + platform_actions + .iter() + .all(|action| !matches!(action, RedfishSimPlatformAction::IsBiosSetup { .. })), + "periodic observation must not inspect host BIOS: {platform_actions:?}" + ); +} + /// Replaces the fixture's DMI vendor so controller vendor branches can be /// exercised after ordinary ingestion has completed. async fn set_host_hardware_vendor(env: &TestEnv, mh: &TestManagedHost, vendor: &str) { @@ -3525,6 +3608,331 @@ async fn test_ready_boot_config_skips_unlock_when_already_correct(pool: sqlx::Pg ); } +/// A periodic observation of an unchanged target refreshes only its observation +/// metadata. It does not create a new desired generation, enter convergence, +/// or issue any Redfish mutation. +#[crate::sqlx_test] +async fn test_ready_periodic_boot_interface_observation_refreshes_verified_observation( + pool: sqlx::PgPool, +) { + let env = create_zero_dpu_test_env(pool).await; + let managed_host = create_managed_host_with_config(&env, ManagedHostConfig::zero_dpu()).await; + let host_before = backdate_boot_interface_observation(&env, &managed_host).await; + let desired_before = host_before + .config + .desired_boot_interface + .as_ref() + .expect("fixture should have desired boot intent") + .clone(); + let observed_at_before = host_before + .status + .boot_interface_status_observation + .as_ref() + .expect("fixture should have a verified boot interface") + .observed_at; + let expected_mac = desired_before.value.mac_address().to_string(); + + env.redfish_sim.set_is_bios_setup(true); + env.redfish_sim.set_is_boot_order_setup(true); + let redfish_checkpoint = env.redfish_sim.timepoint(); + + env.run_machine_state_controller_iteration().await; + + let mut txn = env.db_txn().await; + let host_after = managed_host.host().db_machine(&mut txn).await; + assert_eq!(host_after.current_state(), &ManagedHostState::Ready); + assert_eq!(host_after.current_version(), host_before.current_version()); + assert_eq!(host_after.version, host_before.version); + let desired_after = host_after + .config + .desired_boot_interface + .as_ref() + .expect("a successful observation must retain desired intent"); + assert_eq!(desired_after.version, desired_before.version); + assert_eq!(desired_after.value, desired_before.value); + let refreshed_observation = host_after + .status + .boot_interface_status_observation + .as_ref() + .expect("the periodically observed target should remain verified"); + assert_eq!(refreshed_observation.config_version, desired_before.version); + assert!( + refreshed_observation.observed_at > observed_at_before, + "a successful periodic read should refresh its observation timestamp" + ); + assert!(!refreshed_observation.assumed); + drop(txn); + + let redfish_actions = env + .redfish_sim + .actions_since(&redfish_checkpoint) + .all_hosts(); + assert!(redfish_actions.iter().any(|action| matches!( + action, + RedfishSimAction::IsBootOrderSetup { boot_interface_mac } + if boot_interface_mac == &expected_mac + ))); + assert_read_only_boot_interface_observation(&redfish_actions); + + let redfish_client_count_after = env.redfish_sim.create_client_calls().len(); + env.run_machine_state_controller_iteration().await; + assert_eq!( + env.redfish_sim.create_client_calls().len(), + redfish_client_count_after, + "a fresh observation should suppress another Redfish read" + ); +} + +/// A managed DPU can disappear from the host's Redfish boot view during a +/// restart. Periodic observation skips Redfish until every DPU has a network +/// observation from the current host state inside the freshness threshold. +#[crate::sqlx_test] +async fn test_ready_periodic_boot_interface_observation_waits_for_fresh_dpu_network_observations( + pool: sqlx::PgPool, +) { + let env = create_test_env(pool).await; + let managed_host = create_managed_host_with_config(&env, ManagedHostConfig::default()).await; + backdate_boot_interface_observation(&env, &managed_host).await; + set_host_controller_state_stuck_in(&env, managed_host.host().id, &ManagedHostState::Ready, 0) + .await; + + let mut txn = env.db_txn().await; + let snapshot_before = managed_host.snapshot(&mut txn).await; + assert!(!snapshot_before.dpu_snapshots.is_empty()); + assert!(snapshot_before.dpu_snapshots.iter().all(|dpu_snapshot| { + dpu_snapshot + .network_status_observation + .as_ref() + .is_none_or(|dpu_network_observation| { + dpu_network_observation.observed_at + < snapshot_before.host_snapshot.state.version.timestamp() + }) + })); + let desired_before = snapshot_before + .host_snapshot + .config + .desired_boot_interface + .as_ref() + .expect("fixture should have desired boot intent") + .clone(); + let observation_before = snapshot_before + .host_snapshot + .status + .boot_interface_status_observation + .clone(); + drop(txn); + + env.redfish_sim.set_is_bios_setup(true); + env.redfish_sim.set_is_boot_order_setup(false); + let platform_action_count_before = env.redfish_sim.platform_actions().len(); + let redfish_checkpoint = env.redfish_sim.timepoint(); + + env.run_machine_state_controller_iteration().await; + + let mut txn = env.db_txn().await; + let host_after = managed_host.host().db_machine(&mut txn).await; + assert_eq!(host_after.current_state(), &ManagedHostState::Ready); + let desired_after = host_after + .config + .desired_boot_interface + .as_ref() + .expect("skipped observation must retain desired boot intent"); + assert_eq!(desired_after.version, desired_before.version); + assert_eq!(desired_after.value, desired_before.value); + assert_eq!( + host_after.status.boot_interface_status_observation, + observation_before + ); + drop(txn); + + let redfish_actions = env + .redfish_sim + .actions_since(&redfish_checkpoint) + .all_hosts(); + assert_no_boot_interface_observation( + &redfish_actions, + &env.redfish_sim.platform_actions()[platform_action_count_before..], + ); +} + +/// A periodic mismatch opens a fresh generation for the exact same target. +/// Ready records the drift as a new pending generation, then enters the +/// existing convergence state on its next controller sweep. +#[crate::sqlx_test] +async fn test_ready_periodic_boot_interface_drift_reenters_convergence(pool: sqlx::PgPool) { + let env = create_zero_dpu_test_env(pool).await; + let managed_host = create_managed_host_with_config(&env, ManagedHostConfig::zero_dpu()).await; + let host_before = backdate_boot_interface_observation(&env, &managed_host).await; + let desired_before = host_before + .config + .desired_boot_interface + .as_ref() + .expect("fixture should have desired boot intent") + .clone(); + let observation_before = host_before + .status + .boot_interface_status_observation + .as_ref() + .expect("fixture should have a verified boot interface") + .clone(); + + env.redfish_sim.set_is_bios_setup(true); + env.redfish_sim.set_is_boot_order_setup(false); + let redfish_checkpoint = env.redfish_sim.timepoint(); + + env.run_machine_state_controller_iteration().await; + + let mut txn = env.db_txn().await; + let host_after = managed_host.host().db_machine(&mut txn).await; + assert_eq!(host_after.current_state(), &ManagedHostState::Ready); + assert_eq!(host_after.current_version(), host_before.current_version()); + let pending_desired = host_after + .config + .desired_boot_interface + .as_ref() + .expect("drift should reopen desired boot intent") + .clone(); + assert_ne!(pending_desired.version, desired_before.version); + assert_eq!( + pending_desired.value, desired_before.value, + "drift must preserve the exact operator-selected target" + ); + assert_eq!( + host_after.status.boot_interface_status_observation.as_ref(), + Some(&observation_before), + "the last successful observation remains valid historical status" + ); + assert_eq!( + host_after.pending_boot_interface_config_version(), + Some(pending_desired.version) + ); + drop(txn); + + let redfish_actions = env + .redfish_sim + .actions_since(&redfish_checkpoint) + .all_hosts(); + assert_read_only_boot_interface_observation(&redfish_actions); + + env.run_machine_state_controller_iteration().await; + + let mut txn = env.db_txn().await; + let host_in_convergence = managed_host.host().db_machine(&mut txn).await; + assert_eq!( + host_in_convergence.current_state(), + &ManagedHostState::BootConfiguring { + desired_version: pending_desired.version, + desired_boot_interface: pending_desired.value, + post_lock_verification_retry_count: 0, + boot_config_state: ReadyBootConfigState::Prepare, + }, + "the next Ready sweep should hand the pending target to existing convergence" + ); +} + +/// Assigned hosts record the same drift without entering a disruptive repair +/// flow. The pending generation remains for Ready to converge after release. +#[crate::sqlx_test] +async fn test_assigned_periodic_boot_interface_drift_defers_convergence(pool: sqlx::PgPool) { + let (env, managed_host) = zero_dpu_host_with_instance(pool).await; + set_assigned_state(&env, &managed_host.host().id, InstanceState::Ready).await; + let host_before = backdate_boot_interface_observation(&env, &managed_host).await; + let desired_before = host_before + .config + .desired_boot_interface + .as_ref() + .expect("fixture should have desired boot intent") + .clone(); + + env.redfish_sim.set_is_bios_setup(true); + env.redfish_sim.set_is_boot_order_setup(false); + let redfish_checkpoint = env.redfish_sim.timepoint(); + + env.run_machine_state_controller_iteration().await; + + let mut txn = env.db_txn().await; + let host_after = managed_host.host().db_machine(&mut txn).await; + assert_eq!( + host_after.current_state(), + &ManagedHostState::Assigned { + instance_state: InstanceState::Ready, + } + ); + assert_eq!(host_after.current_version(), host_before.current_version()); + let pending_desired = host_after + .config + .desired_boot_interface + .as_ref() + .expect("assigned drift should persist pending intent"); + assert_eq!( + pending_desired.value, desired_before.value, + "assigned drift must preserve the exact operator-selected target" + ); + assert_ne!(pending_desired.version, desired_before.version); + assert_eq!( + host_after.pending_boot_interface_config_version(), + Some(pending_desired.version) + ); + drop(txn); + + let redfish_actions = env + .redfish_sim + .actions_since(&redfish_checkpoint) + .all_hosts(); + assert_read_only_boot_interface_observation(&redfish_actions); +} + +/// Locked Supermicro reports a stale boot-order view until an unlock and +/// reboot. Periodic observation must therefore leave both Ready and its +/// verified status untouched instead of disrupting the host or publishing a +/// false result. +#[crate::sqlx_test] +async fn test_locked_supermicro_periodic_boot_interface_observation_is_skipped(pool: sqlx::PgPool) { + let env = create_zero_dpu_test_env(pool).await; + let mut supermicro_host_config = ManagedHostConfig::zero_dpu(); + supermicro_host_config.vendor = Some(bmc_vendor::BMCVendor::Supermicro); + let managed_host = create_managed_host_with_config(&env, supermicro_host_config).await; + set_host_hardware_vendor(&env, &managed_host, "Supermicro").await; + let host_before = backdate_boot_interface_observation(&env, &managed_host).await; + let desired_before = host_before + .config + .desired_boot_interface + .as_ref() + .expect("fixture should have desired boot intent") + .clone(); + let observation_before = host_before.status.boot_interface_status_observation.clone(); + let platform_action_count_before = env.redfish_sim.platform_actions().len(); + let redfish_checkpoint = env.redfish_sim.timepoint(); + + env.run_machine_state_controller_iteration().await; + + let mut txn = env.db_txn().await; + let host_after = managed_host.host().db_machine(&mut txn).await; + assert_eq!(host_after.current_state(), &ManagedHostState::Ready); + assert_eq!(host_after.current_version(), host_before.current_version()); + let desired_after = host_after + .config + .desired_boot_interface + .as_ref() + .expect("skipped observation must retain desired boot intent"); + assert_eq!(desired_after.version, desired_before.version); + assert_eq!(desired_after.value, desired_before.value); + assert_eq!( + host_after.status.boot_interface_status_observation, + observation_before + ); + drop(txn); + + let redfish_actions = env + .redfish_sim + .actions_since(&redfish_checkpoint) + .all_hosts(); + assert_no_boot_interface_observation( + &redfish_actions, + &env.redfish_sim.platform_actions()[platform_action_count_before..], + ); +} + /// Hosts whose lifecycle profile intentionally leaves lockdown disabled still /// take the already-correct fast path. LockHost has no policy restoration to /// perform for them, but it still re-observes the exact target before marking diff --git a/crates/api-db/src/machine_desired_boot_interface.rs b/crates/api-db/src/machine_desired_boot_interface.rs index 106ad0309e..cbd8f022c2 100644 --- a/crates/api-db/src/machine_desired_boot_interface.rs +++ b/crates/api-db/src/machine_desired_boot_interface.rs @@ -574,6 +574,71 @@ pub async fn enrich_interface_id( })) } +/// Tries to reopen an inspected target as a new pending generation after +/// Redfish drift. +/// +/// The parent-machine lock and exact desired-generation check make this an +/// observation result, not a blind `force_set`: newer operator intent wins. +/// The verified-version check also makes replay a no-op once a generation is +/// already pending. `None` means either condition changed before this result +/// could be persisted. +pub async fn try_reopen_after_observed_drift( + txn: &mut PgConnection, + machine_id: &MachineId, + inspected_boot_interface: &Versioned, +) -> Result>, DatabaseError> { + validate_machine_id(machine_id)?; + validate_target(&inspected_boot_interface.value)?; + + let desired_boot_interface_row = load_for_update(txn, machine_id).await?; + let current_machine_version = desired_boot_interface_row.machine_version; + let Some(current_desired_boot_interface) = desired_boot_interface_row.decode(machine_id)? + else { + return Ok(None); + }; + if current_desired_boot_interface.version != inspected_boot_interface.version + || current_desired_boot_interface.value != inspected_boot_interface.value + { + return Ok(None); + } + + // Read and lock the child status only after `load_for_update` has acquired + // the parent-machine lock. Every desired/status writer uses this same + // parent-first order, so this is both a fresh status read and deadlock-safe. + let verified_version_query = r#" + SELECT verified_version + FROM machine_boot_interfaces + WHERE machine_id = $1 + FOR UPDATE + "#; + let verified_version: Option = sqlx::query_scalar(verified_version_query) + .bind(machine_id) + .fetch_one(&mut *txn) + .await + .map_err(|error| DatabaseError::query(verified_version_query, error))?; + if verified_version != Some(current_desired_boot_interface.version) { + return Ok(None); + } + + let Some(reopened_version) = update( + txn, + machine_id, + current_machine_version, + Some(current_desired_boot_interface.version), + ¤t_desired_boot_interface.value, + VerificationPolicy::Pending, + ) + .await? + else { + return Ok(None); + }; + + Ok(Some(Versioned { + value: current_desired_boot_interface.value, + version: reopened_version, + })) +} + /// Records a Redfish observation only if the desired boot-interface version /// still matches the version the caller observed. /// @@ -849,6 +914,113 @@ mod tests { Ok(()) } + /// A drift result for an old generation cannot overwrite newer operator intent. + #[crate::sqlx_test] + async fn observation_results_reject_a_newer_desired_generation( + pool: PgPool, + ) -> Result<(), Box> { + let mut txn = pool.begin().await?; + let machine_id = machine_id(MachineType::Host, 44); + seed_machine(txn.as_mut(), &machine_id).await?; + let inspected_target = MachineBootInterfaceTarget::Pair(MachineBootInterface { + mac_address: MacAddress::new([2, 0, 0, 0, 4, 4]), + interface_id: "NIC.Slot.4-1-1".to_string(), + }); + let inspected_desired = set(txn.as_mut(), &machine_id, &inspected_target).await?; + let observed_at = + DateTime::from_timestamp(1_722_000_300, 123_000_000).expect("fixture timestamp"); + assert!( + mark_verified( + txn.as_mut(), + &machine_id, + inspected_desired.version, + observed_at, + ) + .await? + ); + + let operator_target = + MachineBootInterfaceTarget::MacOnly(MacAddress::new([2, 0, 0, 0, 4, 5])); + let operator_desired = set(txn.as_mut(), &machine_id, &operator_target).await?; + assert!( + try_reopen_after_observed_drift(txn.as_mut(), &machine_id, &inspected_desired) + .await? + .is_none() + ); + let persisted_desired = get(txn.as_mut(), &machine_id) + .await? + .expect("operator-selected target"); + assert_target(&persisted_desired, &operator_desired.value); + assert_eq!(persisted_desired.version, operator_desired.version); + assert_eq!( + status_observation(txn.as_mut(), &machine_id).await?, + (Some(inspected_desired.version), Some(observed_at), false,), + ); + + Ok(()) + } + + /// Exact Pair drift opens one pending generation for the same target. + #[crate::sqlx_test] + async fn observation_drift_reopens_the_exact_pair_once( + pool: PgPool, + ) -> Result<(), Box> { + let mut txn = pool.begin().await?; + let machine_id = machine_id(MachineType::Host, 45); + let initial_machine_version = seed_machine(txn.as_mut(), &machine_id).await?; + let inspected_target = MachineBootInterfaceTarget::Pair(MachineBootInterface { + mac_address: MacAddress::new([2, 0, 0, 0, 4, 6]), + interface_id: "NIC.Slot.4-1-2".to_string(), + }); + let inspected_desired = set(txn.as_mut(), &machine_id, &inspected_target).await?; + let observed_at = + DateTime::from_timestamp(1_722_000_400, 123_000_000).expect("fixture timestamp"); + assert!( + mark_verified( + txn.as_mut(), + &machine_id, + inspected_desired.version, + observed_at, + ) + .await? + ); + + let pending_desired = + try_reopen_after_observed_drift(txn.as_mut(), &machine_id, &inspected_desired) + .await? + .expect("fresh pending generation"); + assert_target(&pending_desired, &inspected_target); + assert_eq!( + pending_desired.version.version_nr(), + inspected_desired.version.version_nr() + 1 + ); + assert_eq!( + status_observation(txn.as_mut(), &machine_id).await?, + (Some(inspected_desired.version), Some(observed_at), false,), + "drift keeps the last factual observation while the new generation is pending", + ); + let (machine_version_after_reopen, desired_version_after_reopen) = + versions(txn.as_mut(), &machine_id).await?; + assert_eq!( + machine_version_after_reopen.version_nr(), + initial_machine_version.version_nr() + 2, + ); + assert_eq!(desired_version_after_reopen, Some(pending_desired.version)); + + assert!( + try_reopen_after_observed_drift(txn.as_mut(), &machine_id, &pending_desired) + .await? + .is_none(), + "an already-pending generation must not be reopened", + ); + assert_eq!( + versions(txn.as_mut(), &machine_id).await?, + (machine_version_after_reopen, desired_version_after_reopen), + ); + + Ok(()) + } + #[crate::sqlx_test] #[allow(txn_held_across_await)] // Intentionally hold a row lock while exercising concurrency. async fn concurrent_set_and_mark_verified_use_parent_first_lock_order( diff --git a/crates/machine-controller/src/config/controller.rs b/crates/machine-controller/src/config/controller.rs index 267a0050e0..aaa227b87c 100644 --- a/crates/machine-controller/src/config/controller.rs +++ b/crates/machine-controller/src/config/controller.rs @@ -21,6 +21,21 @@ use chrono::Duration; use duration_str::deserialize_duration_chrono; use serde::{Deserialize, Serialize}; +/// Deserializes a recurring interval and rejects zero or negative durations. +fn deserialize_positive_duration_chrono<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + let duration = deserialize_duration_chrono(deserializer)?; + if duration <= Duration::zero() { + return Err(serde::de::Error::custom( + "duration must be greater than zero", + )); + } + + Ok(duration) +} + /// MachineStateController related config. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct MachineStateControllerConfig { @@ -92,6 +107,14 @@ pub struct MachineStateControllerConfig { serialize_with = "as_duration" )] pub polling_bios_setup_stuck_threshold: Duration, + /// How long a verified desired boot interface may go before the controller + /// compares it with Redfish again. + #[serde( + default = "MachineStateControllerConfig::boot_interface_observation_interval_default", + deserialize_with = "deserialize_positive_duration_chrono", + serialize_with = "as_duration" + )] + pub boot_interface_observation_interval: Duration, } impl MachineStateControllerConfig { @@ -110,6 +133,9 @@ impl MachineStateControllerConfig { ), polling_bios_setup_stuck_threshold: MachineStateControllerConfig::polling_bios_setup_stuck_threshold_default(), + // Keep periodic Redfish reads out of unrelated controller tests. + // Focused tests explicitly age the observation they exercise. + boot_interface_observation_interval: Duration::weeks(52), } } @@ -148,6 +174,11 @@ impl MachineStateControllerConfig { pub fn polling_bios_setup_stuck_threshold_default() -> Duration { Duration::minutes(15) } + + /// Default cadence for rechecking an already-verified boot interface. + pub fn boot_interface_observation_interval_default() -> Duration { + Duration::minutes(10) + } } impl Default for MachineStateControllerConfig { @@ -167,6 +198,8 @@ impl Default for MachineStateControllerConfig { ), polling_bios_setup_stuck_threshold: MachineStateControllerConfig::polling_bios_setup_stuck_threshold_default(), + boot_interface_observation_interval: + MachineStateControllerConfig::boot_interface_observation_interval_default(), } } } diff --git a/crates/machine-controller/src/handler.rs b/crates/machine-controller/src/handler.rs index 562cf81847..475e3763c4 100644 --- a/crates/machine-controller/src/handler.rs +++ b/crates/machine-controller/src/handler.rs @@ -119,6 +119,7 @@ use crate::{MeasuringOutcome, get_measuring_prerequisites, handle_measuring_stat pub mod attestation; mod bios_config; +mod boot_interface_observation; mod dpf; mod dpu_uefi_rotation; mod firmware_artifact; @@ -1119,7 +1120,9 @@ impl MachineStateHandler { )); } - Ok(StateHandlerOutcome::do_nothing()) + // Periodic BMC observation is deliberately Ready's final work, + // so it cannot preempt lifecycle or operator-requested actions. + boot_interface_observation::observe_verified_boot_interface(ctx, mh_snapshot).await } ManagedHostState::BootConfiguring { @@ -7999,9 +8002,15 @@ impl StateHandler for InstanceStateHandler { Ok(StateHandlerOutcome::transition(next_state).with_txn(txn)) } else if let Some(txn) = txn_opt { + // Commit extension cleanup before the observer performs + // Redfish I/O in a separate attempt. Ok(StateHandlerOutcome::do_nothing().with_txn(txn)) } else { - Ok(StateHandlerOutcome::do_nothing()) + boot_interface_observation::observe_verified_boot_interface( + ctx, + mh_snapshot, + ) + .await } } InstanceState::HostPlatformConfiguration { diff --git a/crates/machine-controller/src/handler/boot_interface_observation.rs b/crates/machine-controller/src/handler/boot_interface_observation.rs new file mode 100644 index 0000000000..c8cd18a512 --- /dev/null +++ b/crates/machine-controller/src/handler/boot_interface_observation.rs @@ -0,0 +1,317 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +//! Periodic observation of an already-verified host boot interface. +//! +//! This path only reads Redfish and records what it found. A mismatch opens a +//! new pending generation for the same target; the existing Ready reconciler +//! owns every later unlock, write, reboot, and verification step. + +use carbide_redfish::boot_interface::BootInterfaceTarget; +use chrono::{DateTime, Duration, Utc}; +use config_version::Versioned; +use model::machine::{Machine, ManagedHostState, ManagedHostStateSnapshot}; +use model::machine_boot_interface::MachineBootInterfaceTarget; +use state_controller::state_handler::{ + StateHandlerContext, StateHandlerError, StateHandlerOutcome, +}; + +use super::host_boot_config::{ + HostBootConfigDecision, decide_host_boot_config, inspect_host_boot_config, +}; +use crate::context::MachineStateHandlerContextObjects; + +/// Returns the verified desired target after its observation interval elapses. +fn periodic_observation_target( + host: &Machine, + now: DateTime, + observation_interval: Duration, +) -> Option<&Versioned> { + let desired_boot_interface = host.config.desired_boot_interface.as_ref()?; + let boot_interface_observation = host.status.boot_interface_status_observation.as_ref()?; + + (desired_boot_interface.version == boot_interface_observation.config_version + && now.signed_duration_since(boot_interface_observation.observed_at) + >= observation_interval) + .then_some(desired_boot_interface) +} + +/// Returns whether a managed DPU has reported recently enough to trust the +/// host's Redfish boot view and after the host entered its current state. +fn dpu_observation_is_current( + managed_host_snapshot: &ManagedHostStateSnapshot, + dpu_snapshot: &Machine, + now: DateTime, + dpu_up_threshold: Duration, +) -> bool { + super::is_dpu_up(managed_host_snapshot, dpu_snapshot) + && dpu_snapshot + .network_status_observation + .as_ref() + .is_some_and(|dpu_network_observation| { + now.signed_duration_since(dpu_network_observation.observed_at) <= dpu_up_threshold + }) +} + +/// Observes an eligible verified target without mutating Redfish. +/// +/// A BMC failure leaves the last successful `observed_at` unchanged, so every +/// later Ready or Assigned sweep retries it. The configured interval applies +/// between successful observations, not failed attempts. After a mismatch, +/// Ready enters `BootConfiguring` on its next sweep, while Assigned keeps the +/// pending generation until the host is unassigned. +pub(super) async fn observe_verified_boot_interface( + ctx: &mut StateHandlerContext<'_, MachineStateHandlerContextObjects>, + managed_host_snapshot: &ManagedHostStateSnapshot, +) -> Result, StateHandlerError> { + let host = &managed_host_snapshot.host_snapshot; + let controller_config = &ctx.services.site_config.machine_state_controller; + let now = Utc::now(); + let Some(desired_boot_interface) = periodic_observation_target( + host, + now, + controller_config.boot_interface_observation_interval, + ) else { + return Ok(StateHandlerOutcome::do_nothing()); + }; + + // Locked Supermicro reports a stale boot-order view. The explicit + // `BootConfiguring` flow can safely unlock and reboot an unassigned host, + // but a periodic observer must remain non-disruptive (especially while an + // instance is assigned). Profiles that intentionally leave lockdown off + // can use the ordinary read-only path. + if host.bmc_vendor().is_supermicro() && !host.host_profile.disable_lockdown { + return Ok(StateHandlerOutcome::do_nothing()); + } + + // A managed DPU can disappear from the host's Redfish boot view during a + // DPU restart. Do not turn that transient view into drift until every DPU + // has reported network state since the host entered its current state and + // the report remains inside its health window. + if managed_host_snapshot + .dpu_snapshots + .iter() + .any(|dpu_snapshot| { + !dpu_observation_is_current( + managed_host_snapshot, + dpu_snapshot, + now, + controller_config.dpu_up_threshold, + ) + }) + { + return Ok(StateHandlerOutcome::do_nothing()); + } + + let redfish_client = match ctx.services.create_redfish_client_from_machine(host).await { + Ok(redfish_client) => redfish_client, + Err(error) => { + tracing::warn!( + machine_id = %host.id, + desired_version = %desired_boot_interface.version, + operation = "create_redfish_client", + error = %error, + "Failed to observe verified host boot configuration", + ); + return Ok(StateHandlerOutcome::do_nothing()); + } + }; + let redfish_target: BootInterfaceTarget = desired_boot_interface.value.clone().into(); + let boot_config_inspection = match inspect_host_boot_config( + redfish_client.as_ref(), + managed_host_snapshot, + &redfish_target, + ) + .await + { + Ok(boot_config_inspection) => boot_config_inspection, + Err(error) => { + tracing::warn!( + machine_id = %host.id, + desired_version = %desired_boot_interface.version, + operation = "inspect_host_boot_config", + error = %error, + "Failed to observe verified host boot configuration", + ); + return Ok(StateHandlerOutcome::do_nothing()); + } + }; + + let boot_config_decision = decide_host_boot_config(boot_config_inspection); + let mut observation_txn = ctx.services.db_pool.begin().await?; + match boot_config_decision { + HostBootConfigDecision::Complete => { + let observation_recorded = db::machine_desired_boot_interface::mark_verified( + observation_txn.as_mut(), + &host.id, + desired_boot_interface.version, + Utc::now(), + ) + .await?; + if observation_recorded { + tracing::debug!( + machine_id = %host.id, + desired_version = %desired_boot_interface.version, + "Verified periodic host boot configuration observation", + ); + } else { + tracing::debug!( + machine_id = %host.id, + desired_version = %desired_boot_interface.version, + "Discarded stale host boot configuration observation", + ); + } + } + required_action @ (HostBootConfigDecision::ConfigureBios + | HostBootConfigDecision::SetBootOrder) => { + // BIOS inspection covers NICo's whole managed boot profile, not just + // the boot-order entry. Reopening this target sends either kind of + // drift through the existing whole-profile flow. + let pending_boot_interface = + db::machine_desired_boot_interface::try_reopen_after_observed_drift( + observation_txn.as_mut(), + &host.id, + desired_boot_interface, + ) + .await?; + if let Some(pending_boot_interface) = pending_boot_interface { + tracing::warn!( + machine_id = %host.id, + desired_version = %pending_boot_interface.version, + ?required_action, + repair_deferred_until_unassigned = matches!( + &managed_host_snapshot.managed_state, + ManagedHostState::Assigned { .. } + ), + "Host boot configuration drift detected", + ); + } else { + tracing::debug!( + machine_id = %host.id, + desired_version = %desired_boot_interface.version, + "Discarded stale host boot configuration drift observation", + ); + } + } + } + + Ok(StateHandlerOutcome::do_nothing().with_txn(observation_txn)) +} + +#[cfg(test)] +mod tests { + use carbide_test_support::value_scenarios; + use chrono::Duration; + use config_version::{ConfigVersion, Versioned}; + use mac_address::MacAddress; + use model::machine_boot_interface::{ + BootInterfaceStatusObservation, MachineBootInterfaceTarget, + }; + use model::test_support::machine_snapshot::{host_machine, managed_host_state_snapshot}; + + use super::*; + + #[test] + fn observation_requires_elapsed_interval_and_verified_intent() { + struct PeriodicObservationInput { + desired_version: Option, + observed_version: Option, + observation_age: Duration, + } + + let now = DateTime::from_timestamp(1_722_000_000, 0).expect("fixture timestamp"); + let observation_interval = Duration::minutes(10); + let observed_version = ConfigVersion::initial(); + + value_scenarios!( + run = |input: PeriodicObservationInput| { + let mut host = host_machine(); + host.config.desired_boot_interface = input.desired_version.map(|version| { + Versioned::new( + MachineBootInterfaceTarget::MacOnly(MacAddress::new([2, 0, 0, 0, 0, 1])), + version, + ) + }); + host.status.boot_interface_status_observation = + input.observed_version.map(|config_version| { + BootInterfaceStatusObservation { + config_version, + observed_at: now - input.observation_age, + assumed: false, + } + }); + + periodic_observation_target(&host, now, observation_interval).is_some() + }; + "eligible after the interval" { + PeriodicObservationInput { + desired_version: Some(observed_version), + observed_version: Some(observed_version), + observation_age: observation_interval, + } => true, + } + "ineligible" { + PeriodicObservationInput { + desired_version: Some(observed_version), + observed_version: Some(observed_version), + observation_age: observation_interval - Duration::seconds(1), + } => false, + PeriodicObservationInput { + desired_version: Some(observed_version.increment()), + observed_version: Some(observed_version), + observation_age: observation_interval, + } => false, + PeriodicObservationInput { + desired_version: None, + observed_version: Some(observed_version), + observation_age: observation_interval, + } => false, + PeriodicObservationInput { + desired_version: Some(observed_version), + observed_version: None, + observation_age: observation_interval, + } => false, + } + ); + } + + /// A report from the current state is still unsafe after its health window. + #[test] + fn managed_dpu_observation_must_also_be_recent() { + let managed_host_snapshot = managed_host_state_snapshot(); + let dpu_snapshot = &managed_host_snapshot.dpu_snapshots[0]; + let dpu_observation_timestamp = dpu_snapshot + .network_status_observation + .as_ref() + .expect("fixture DPU should have a network observation") + .observed_at; + let dpu_up_threshold = Duration::minutes(5); + + assert!(dpu_observation_is_current( + &managed_host_snapshot, + dpu_snapshot, + dpu_observation_timestamp + dpu_up_threshold, + dpu_up_threshold, + )); + assert!(!dpu_observation_is_current( + &managed_host_snapshot, + dpu_snapshot, + dpu_observation_timestamp + dpu_up_threshold + Duration::seconds(1), + dpu_up_threshold, + )); + } +}