From 45987cf17ea1bd55f166e97962f92240c9cb7185 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 27 Apr 2026 17:43:31 -0700 Subject: [PATCH 1/9] [nexus] is your computer running? better go catch it --- nexus/db-model/src/sled.rs | 4 + nexus/db-queries/src/db/datastore/mod.rs | 2 +- nexus/db-queries/src/db/datastore/sled.rs | 41 ++++ nexus/db-schema/src/schema.rs | 2 + nexus/src/app/background/init.rs | 1 + .../app/background/tasks/instance_watcher.rs | 176 ++++++++++++++++-- 6 files changed, 208 insertions(+), 18 deletions(-) diff --git a/nexus/db-model/src/sled.rs b/nexus/db-model/src/sled.rs index 47631f3ac09..40d716ddfb7 100644 --- a/nexus/db-model/src/sled.rs +++ b/nexus/db-model/src/sled.rs @@ -121,6 +121,10 @@ impl Sled { &self.serial_number } + pub fn revision(&self) -> u32 { + self.revision.into() + } + pub fn part_number(&self) -> &str { &self.part_number } diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index bcf16ba016b..dfe213c8d4e 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -129,7 +129,7 @@ mod silo; mod silo_auth_settings; mod silo_group; mod silo_user; -mod sled; +pub mod sled; mod sled_instance; mod snapshot; mod ssh_key; diff --git a/nexus/db-queries/src/db/datastore/sled.rs b/nexus/db-queries/src/db/datastore/sled.rs index 1910c625c80..5a8f40ecbd0 100644 --- a/nexus/db-queries/src/db/datastore/sled.rs +++ b/nexus/db-queries/src/db/datastore/sled.rs @@ -18,6 +18,7 @@ use crate::db::model::Sled; use crate::db::model::SledResourceVmm; use crate::db::model::SledState; use crate::db::model::SledUpdate; +use crate::db::model::SqlU16; use crate::db::model::to_db_sled_policy; use crate::db::model::to_db_typed_uuid; use crate::db::pagination::Paginator; @@ -48,6 +49,7 @@ use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; +use omicron_common::api::external::LookupResult; use omicron_common::api::external::ResourceType; use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_uuid_kinds::DatasetUuid; @@ -1615,6 +1617,45 @@ impl DataStore { Ok(old_state) } + + pub async fn sled_try_to_find_slot( + &self, + opctx: &OpContext, + sled: &Sled, + ) -> LookupResult { + use nexus_db_schema::schema::hw_baseboard_id::dsl as baseboard_dsl; + use nexus_db_schema::schema::inv_service_processor::dsl as sp_dsl; + + let conn = self.pool_connection_authorized(opctx).await?; + + let slot = baseboard_dsl::hw_baseboard_id + .filter( + baseboard_dsl::serial_number + .eq(sled.serial_number().to_owned()), + ) + .filter( + baseboard_dsl::part_number.eq(sled.part_number().to_owned()), + ) + .inner_join( + sp_dsl::inv_service_processor + .on(sp_dsl::hw_baseboard_id.eq(baseboard_dsl::id)), + ) + .select(sp_dsl::sp_slot) + .order_by(sp_dsl::time_collected.desc()) + .first_async::(&*conn) + .await + .optional() + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))? + .map(|slot| SledLocation::Slot(slot.into())) + .unwrap_or(SledLocation::Unknown); + Ok(slot) + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SledLocation { + Unknown, + Slot(u16), } // --- diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 0a66531be52..0dbcf131108 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -1640,6 +1640,8 @@ table! { } } +allow_tables_to_appear_in_same_query!(hw_baseboard_id, inv_service_processor); + table! { sw_caboose (id) { id -> Uuid, diff --git a/nexus/src/app/background/init.rs b/nexus/src/app/background/init.rs index e9cc7ed3b83..76b563766dc 100644 --- a/nexus/src/app/background/init.rs +++ b/nexus/src/app/background/init.rs @@ -790,6 +790,7 @@ impl BackgroundTasksInitializer { datastore.clone(), sagas.clone(), producer_registry, + resolver.clone(), instance_watcher::WatcherIdentity { nexus_id, rack_id }, ); driver.register(TaskDefinition { diff --git a/nexus/src/app/background/tasks/instance_watcher.rs b/nexus/src/app/background/tasks/instance_watcher.rs index b251601ce99..e1c9886b8a1 100644 --- a/nexus/src/app/background/tasks/instance_watcher.rs +++ b/nexus/src/app/background/tasks/instance_watcher.rs @@ -8,6 +8,7 @@ use crate::app::background::BackgroundTask; use crate::app::instance::SledAgentInstanceError; use crate::app::saga::StartSaga; use futures::{FutureExt, future::BoxFuture}; +use gateway_client::types::PowerState; use nexus_db_model::Instance; use nexus_db_model::Project; use nexus_db_model::Sled; @@ -15,6 +16,7 @@ use nexus_db_model::Vmm; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::DataStore; use nexus_db_queries::db::pagination::Paginator; +use nexus_networking::GatewayClient; use nexus_types::external_api::sled::SledPolicy; use nexus_types::identity::Asset; use nexus_types::identity::Resource; @@ -45,6 +47,7 @@ use virtual_machine::VirtualMachine; pub(crate) struct InstanceWatcher { datastore: Arc, sagas: Arc, + resolver: internal_dns_resolver::Resolver, metrics: Arc>, id: WatcherIdentity, } @@ -74,18 +77,20 @@ impl InstanceWatcher { datastore: Arc, sagas: Arc, producer_registry: &ProducerRegistry, + resolver: internal_dns_resolver::Resolver, id: WatcherIdentity, ) -> Self { let metrics = Arc::new(Mutex::new(metrics::Metrics::default())); producer_registry .register_producer(metrics::Producer(metrics.clone())) .unwrap(); - Self { datastore, sagas, metrics, id } + Self { datastore, sagas, metrics, id, resolver } } fn check_instance( &self, opctx: &OpContext, + gateways: &Arc<[GatewayClient]>, client: SledAgentClient, target: VirtualMachine, vmm: Vmm, @@ -93,6 +98,7 @@ impl InstanceWatcher { ) -> impl Future + Send + 'static + use<> { let datastore = self.datastore.clone(); let sagas = self.sagas.clone(); + let gateways = gateways.clone(); let vmm_id = PropolisUuid::from_untyped_uuid(target.vmm_id); let opctx = { @@ -103,6 +109,10 @@ impl InstanceWatcher { ); meta.insert("vmm_id".to_string(), vmm_id.to_string()); meta.insert("sled_id".to_string(), sled.id().to_string()); + meta.insert( + "sled_serial".to_string(), + sled.serial_number().to_string(), + ); opctx.child(meta) }; @@ -201,19 +211,61 @@ impl InstanceWatcher { Err(SledAgentInstanceError( ClientError::CommunicationError(e), )) => { - // TODO(eliza): eventually, we may want to transition the - // instance to the `Failed` state if the sled-agent has been - // unreachable for a while. We may also want to take other - // corrective actions or alert an operator in this case. - // - // TODO(eliza): because we have the purported IP address - // of the instance's VMM from our database query, we could - // also ask the VMM directly when the sled-agent is - // unreachable. We should start doing that here at some - // point. - slog::info!(opctx.log, "sled agent is unreachable"; InlineErrorChain::new(&e)); - check.result = Err(Incomplete::SledAgentUnreachable); - return check; + slog::info!( + opctx.log, + "sled-agent is unreachable"; + InlineErrorChain::new(&e), + ); + + // Is your computer running? + match is_computer_on( + &opctx, &datastore, &gateways, &sled, + ) + .await + { + // ...better go catch it! + Err(error) => { + let error = InlineErrorChain::new(&*error); + warn!( + opctx.log, + "sled-agent is unreachable, cannot \ + determine if your computer is running"; + error + ); + check.result = + Err(Incomplete::SledAgentUnreachable); + return check; + } + Ok(PowerState::A0) => { + check.result = + Err(Incomplete::SledAgentUnreachable); + return check; + } + Ok(state) => { + slog::info!( + opctx.log, + "instance is assigned to a VMM on a sled \ + that is not in A0, marking it as Failed"; + "sled_power_state" => ?state, + ); + check.outcome = + CheckOutcome::Failure(Failure::SledOff); + // TODO(eliza): it would be nicer if this used the same + // code path as `mark_instance_failed`... + SledVmmState { + vmm_state: instance::VmmRuntimeState { + generation: vmm.generation.0.next(), + state: instance::VmmState::Failed, + time_updated: chrono::Utc::now(), + }, + // It's fine to synthesize `None`s here because a `None` + // just means "don't update the migration state", not + // "there is no migration". + migration_in: None, + migration_out: None, + } + } + } } Err(SledAgentInstanceError(e)) => { slog::warn!( @@ -280,6 +332,71 @@ impl InstanceWatcher { } } +/// An implementation of the `is_computer_on()` function [originally defined +/// in the BeOS C library][1]. +/// +/// [1]: https://web.archive.org/web/20260309055003/https://www.haiku-os.org/legacy-docs/bebook/TheKernelKit_SystemInfo.html +async fn is_computer_on( + opctx: &OpContext, + datastore: &DataStore, + gateways: &[GatewayClient], + sled: &Sled, +) -> anyhow::Result { + use gateway_types::component::SpType; + use nexus_db_queries::db::datastore::sled::SledLocation; + + let SledLocation::Slot(slot) = + datastore.sled_try_to_find_slot(opctx, sled).await? + else { + anyhow::bail!( + "could not determine sled location (it may not yet be in the \ + inventory?)" + ); + }; + + // Ask MGS whether the computer is on. If trying to talk to one of the + // gateways fails for whatever reason, ask any others we were able to + // resolve as well. + for GatewayClient { client, addr } in gateways { + let state = match client.sp_get(&SpType::Sled, slot).await { + Ok(response) => response.into_inner(), + Err(error) => { + let error = InlineErrorChain::new(&error); + slog::warn!( + &opctx.log, + "requesting sled SP status from MGS failed"; + "gateway_addr" => %addr, + "error" => &error, + ); + continue; + } + }; + + let part_number = sled.part_number(); + let rev = sled.revision(); + let sled_serial = sled.serial_number(); + if state.serial_number != sled.serial_number() { + // Well, this is surprising; is our inventory stale? In any case, if + // we can't find the actual location of the sled, we can't determine + // whether or not it's turned on... + anyhow::bail!( + "expected sled {part_number}:{rev}:{sled_serial} to be in \ + slot {slot}, but found sled {}:{}:{} instead", + state.model, + state.revision, + state.serial_number, + ); + } + + return Ok(state.power_state); + } + + Err(anyhow::anyhow!( + "no MGS could determine if the sled is on (asked {})", + gateways.len() + )) +} + /// The identity of the process performing the health check, for distinguishing /// health check metrics emitted by different Nexus instances. /// @@ -382,6 +499,10 @@ enum Failure { /// The instance was assigned to a sled that was expunged. Its VMM has been /// marked as `Failed`, since the sled is no longer present. SledExpunged, + /// The instance was assigned to a sled that is not currently in A0. + /// Its VMM has been marked as `Failed`, since the computer it's on is not, + /// you know...turned on. + SledOff, } impl Failure { @@ -389,6 +510,7 @@ impl Failure { match self { Self::NoSuchInstance => "no_such_instance".into(), Self::SledExpunged => "sled_expunged".into(), + Self::SledOff => "sled_off".into(), } } } @@ -402,8 +524,8 @@ enum Incomplete { /// The sled-agent for the sled on which the instance is running was /// unreachable. /// - /// This may indicate a network partition between us and that sled, that - /// the sled-agent process has crashed, or that the sled is down. + /// This may indicate a network partition between us and that sled, or that + /// the sled-agent process has crashed. SledAgentUnreachable, /// The sled-agent responded with an unexpected HTTP error. SledAgentHttpError(u16), @@ -448,6 +570,26 @@ impl BackgroundTask for InstanceWatcher { MAX_CONCURRENT_CHECKS, ); + let gateways = match GatewayClient::resolve_all_gateways(&opctx.log, &self.resolver).await { + Ok(gateways) => gateways.collect::>(), + Err(err) => { + // This is a bit sad, but talking to MGS is only necessary + // to check if a sled is powered on in the event that the + // sled-agent is unreachable. If we don't see communication + // errors talking to sled-agents, we can confirm that VMMs + // are, or are not, healthy, even if we are unable to talk + // to MGS. + slog::warn!( + opctx.log, + "failed to resolve any management gateways; we will \ + not be checking if sleds are powered on, should \ + sled-agents be unreachable"; + "error" => InlineErrorChain::new(&*err), + ); + Arc::new([]) + }, + }; + let mut update_sagas_queued: usize = 0; let mut instance_states: BTreeMap = BTreeMap::new(); @@ -508,7 +650,7 @@ impl BackgroundTask for InstanceWatcher { }; let target = VirtualMachine::new(self.id, &sled, &instance, &vmm, &project); - tasks.spawn(self.check_instance(opctx, client, target ,vmm ,sled)).await + tasks.spawn(self.check_instance(opctx, &gateways, client, target, vmm, sled)).await } else { // If there are no remaining instances to check, wait for // all previously spawned check to complete. From be5efe90cb55ed83ed5ec5be5502d032f0aa0378 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 28 Apr 2026 13:56:44 -0700 Subject: [PATCH 2/9] refactor some stuff --- .../app/background/tasks/instance_watcher.rs | 296 +++++++++--------- 1 file changed, 141 insertions(+), 155 deletions(-) diff --git a/nexus/src/app/background/tasks/instance_watcher.rs b/nexus/src/app/background/tasks/instance_watcher.rs index e1c9886b8a1..86e538cd8a6 100644 --- a/nexus/src/app/background/tasks/instance_watcher.rs +++ b/nexus/src/app/background/tasks/instance_watcher.rs @@ -118,7 +118,7 @@ impl InstanceWatcher { async move { slog::trace!( - opctx.log, "checking on VMM"; "propolis_id" => %vmm_id + opctx.log, "checking on VMM"; "propolis_id" => %vmm.id ); let mut check = Check { @@ -128,156 +128,13 @@ impl InstanceWatcher { update_saga_queued: false, }; - let state = if sled.policy() == SledPolicy::Expunged { - // If the sled has been expunged, any VMMs still on that sled - // should be marked as `Failed`. - slog::info!( - opctx.log, - "instance is assigned to a VMM on an Expunged sled; \ - marking it as Failed"; - ); - check.outcome = CheckOutcome::Failure(Failure::SledExpunged); - // TODO(eliza): it would be nicer if this used the same - // code path as `mark_instance_failed`... - SledVmmState { - vmm_state: instance::VmmRuntimeState { - generation: vmm.generation.0.next(), - state: instance::VmmState::Failed, - time_updated: chrono::Utc::now(), - }, - // It's fine to synthesize `None`s here because a `None` - // just means "don't update the migration state", not - // "there is no migration". - migration_in: None, - migration_out: None, - } - } else { - // Otherwise, ask the sled-agent what it has to say for itself. - let rsp = client - .vmm_get_state(&vmm_id) - .await - .map_err(SledAgentInstanceError); - match rsp { - Ok(rsp) => { - let state = rsp.into_inner(); - check.outcome = - CheckOutcome::Success(state.vmm_state.state.into()); - state - } - // Oh, this error indicates that the VMM should transition to - // `Failed`. Let's synthesize a `SledInstanceState` that does - // that. - Err(e) if e.vmm_gone() => { - slog::info!( - opctx.log, - "sled-agent error indicates that this instance's \ - VMM has failed!"; - "error" => %e, - ); - check.outcome = - CheckOutcome::Failure(Failure::NoSuchInstance); - // TODO(eliza): it would be nicer if this used the same - // code path as `mark_instance_failed`... - SledVmmState { - vmm_state: instance::VmmRuntimeState { - generation: vmm.generation.0.next(), - state: instance::VmmState::Failed, - time_updated: chrono::Utc::now(), - }, - // It's fine to synthesize `None`s here because a `None` - // just means "don't update the migration state", not - // "there is no migration". - migration_in: None, - migration_out: None, - } - } - Err(SledAgentInstanceError( - ClientError::ErrorResponse(rsp), - )) => { - let status = rsp.status(); - if status.is_client_error() { - slog::warn!(opctx.log, "check incomplete due to client error"; - "status" => ?status, "error" => ?rsp.into_inner()); - } else { - slog::info!(opctx.log, "check incomplete due to server error"; - "status" => ?status, "error" => ?rsp.into_inner()); - } - - check.result = Err(Incomplete::SledAgentHttpError( - status.as_u16(), - )); - return check; - } - Err(SledAgentInstanceError( - ClientError::CommunicationError(e), - )) => { - slog::info!( - opctx.log, - "sled-agent is unreachable"; - InlineErrorChain::new(&e), - ); - - // Is your computer running? - match is_computer_on( - &opctx, &datastore, &gateways, &sled, - ) - .await - { - // ...better go catch it! - Err(error) => { - let error = InlineErrorChain::new(&*error); - warn!( - opctx.log, - "sled-agent is unreachable, cannot \ - determine if your computer is running"; - error - ); - check.result = - Err(Incomplete::SledAgentUnreachable); - return check; - } - Ok(PowerState::A0) => { - check.result = - Err(Incomplete::SledAgentUnreachable); - return check; - } - Ok(state) => { - slog::info!( - opctx.log, - "instance is assigned to a VMM on a sled \ - that is not in A0, marking it as Failed"; - "sled_power_state" => ?state, - ); - check.outcome = - CheckOutcome::Failure(Failure::SledOff); - // TODO(eliza): it would be nicer if this used the same - // code path as `mark_instance_failed`... - SledVmmState { - vmm_state: instance::VmmRuntimeState { - generation: vmm.generation.0.next(), - state: instance::VmmState::Failed, - time_updated: chrono::Utc::now(), - }, - // It's fine to synthesize `None`s here because a `None` - // just means "don't update the migration state", not - // "there is no migration". - migration_in: None, - migration_out: None, - } - } - } - } - Err(SledAgentInstanceError(e)) => { - slog::warn!( - opctx.log, - "error checking up on instance"; - "error" => ?e, - "status" => ?e.status(), - ); - check.result = Err(Incomplete::ClientError); - return check; - } - } + let Some(state) = check + .run(&opctx, &datastore, &sled, &vmm, &gateways, &client) + .await + else { + // Check did not result in an updated state, nothing else to + // do for this one! + return check; }; debug!( @@ -286,10 +143,7 @@ impl InstanceWatcher { "state" => ?state, ); match crate::app::instance::process_vmm_update( - &datastore, - &opctx, - PropolisUuid::from_untyped_uuid(target.vmm_id), - &state, + &datastore, &opctx, vmm_id, &state, ) .await { @@ -332,6 +186,138 @@ impl InstanceWatcher { } } +impl Check { + async fn run( + &mut self, + opctx: &OpContext, + datastore: &DataStore, + sled: &Sled, + vmm: &Vmm, + gateways: &[GatewayClient], + client: &SledAgentClient, + ) -> Option { + let mk_failed = || { + // TODO(eliza): it would be nicer if this used the same + // code path as `mark_instance_failed`... + Some(SledVmmState { + vmm_state: instance::VmmRuntimeState { + generation: vmm.generation.0.next(), + state: instance::VmmState::Failed, + time_updated: chrono::Utc::now(), + }, + // It's fine to synthesize `None`s here because a `None` + // just means "don't update the migration state", not + // "there is no migration". + migration_in: None, + migration_out: None, + }) + }; + + if sled.policy() == SledPolicy::Expunged { + // If the sled has been expunged, any VMMs still on that sled + // should be marked as `Failed`. + slog::info!( + opctx.log, + "instance is assigned to a VMM on an Expunged sled; \ + marking it as Failed"; + ); + self.outcome = CheckOutcome::Failure(Failure::SledExpunged); + return mk_failed(); + } + + // Ask the sled-agent what it has to say for itself. + let rsp = client + .vmm_get_state(&PropolisUuid::from_untyped_uuid(vmm.id)) + .await + .map_err(SledAgentInstanceError); + match rsp { + Ok(rsp) => { + let state = rsp.into_inner(); + self.outcome = + CheckOutcome::Success(state.vmm_state.state.into()); + Some(state) + } + // Oh, this error indicates that the VMM should transition to + // `Failed`. Let's synthesize a `SledInstanceState` that does + // that. + Err(e) if e.vmm_gone() => { + slog::info!( + opctx.log, + "sled-agent error indicates that this instance's \ + VMM has failed!"; + "error" => %e, + ); + self.outcome = CheckOutcome::Failure(Failure::NoSuchInstance); + mk_failed() + } + Err(SledAgentInstanceError(ClientError::ErrorResponse(rsp))) => { + let status = rsp.status(); + if status.is_client_error() { + slog::warn!(opctx.log, "check incomplete due to client error"; + "status" => ?status, "error" => ?rsp.into_inner()); + } else { + slog::info!(opctx.log, "check incomplete due to server error"; + "status" => ?status, "error" => ?rsp.into_inner()); + } + + self.result = + Err(Incomplete::SledAgentHttpError(status.as_u16())); + None + } + Err(SledAgentInstanceError(ClientError::CommunicationError(e))) => { + slog::info!( + opctx.log, + "sled-agent is unreachable"; + InlineErrorChain::new(&e), + ); + + // Is your computer running? + match is_computer_on(&opctx, &datastore, &gateways, &sled).await + { + // ...impossible to tell! Results are inconclusive. + Err(error) => { + let error = InlineErrorChain::new(&*error); + warn!( + opctx.log, + "sled-agent is unreachable, but we cannot \ + determine if your computer is running"; + error + ); + } + + // ...better go catch it! + Ok(PowerState::A0) => {} + + // It is not running, the instance can't possibly be there. + Ok(state) => { + slog::info!( + opctx.log, + "instance is assigned to a VMM on a sled \ + that is not in A0, marking it as Failed"; + "sled_power_state" => ?state, + ); + self.outcome = CheckOutcome::Failure(Failure::SledOff); + return mk_failed(); + } + } + + self.result = Err(Incomplete::SledAgentUnreachable); + None + } + Err(SledAgentInstanceError(e)) => { + slog::warn!( + opctx.log, + "error checking up on instance"; + "error" => ?e, + "status" => ?e.status(), + ); + self.result = Err(Incomplete::ClientError); + None + } + } + } +} + /// An implementation of the `is_computer_on()` function [originally defined /// in the BeOS C library][1]. /// From 29d5624e84ef46b3d288bd7e2080e5585ec37e0d Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 28 Apr 2026 14:15:57 -0700 Subject: [PATCH 3/9] get SP slots from inventory instead --- nexus/db-queries/src/db/datastore/sled.rs | 39 -------------- nexus/src/app/background/init.rs | 1 + .../app/background/tasks/instance_watcher.rs | 53 ++++++++++++------- 3 files changed, 35 insertions(+), 58 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/sled.rs b/nexus/db-queries/src/db/datastore/sled.rs index 5a8f40ecbd0..5391123374b 100644 --- a/nexus/db-queries/src/db/datastore/sled.rs +++ b/nexus/db-queries/src/db/datastore/sled.rs @@ -1617,45 +1617,6 @@ impl DataStore { Ok(old_state) } - - pub async fn sled_try_to_find_slot( - &self, - opctx: &OpContext, - sled: &Sled, - ) -> LookupResult { - use nexus_db_schema::schema::hw_baseboard_id::dsl as baseboard_dsl; - use nexus_db_schema::schema::inv_service_processor::dsl as sp_dsl; - - let conn = self.pool_connection_authorized(opctx).await?; - - let slot = baseboard_dsl::hw_baseboard_id - .filter( - baseboard_dsl::serial_number - .eq(sled.serial_number().to_owned()), - ) - .filter( - baseboard_dsl::part_number.eq(sled.part_number().to_owned()), - ) - .inner_join( - sp_dsl::inv_service_processor - .on(sp_dsl::hw_baseboard_id.eq(baseboard_dsl::id)), - ) - .select(sp_dsl::sp_slot) - .order_by(sp_dsl::time_collected.desc()) - .first_async::(&*conn) - .await - .optional() - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))? - .map(|slot| SledLocation::Slot(slot.into())) - .unwrap_or(SledLocation::Unknown); - Ok(slot) - } -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum SledLocation { - Unknown, - Slot(u16), } // --- diff --git a/nexus/src/app/background/init.rs b/nexus/src/app/background/init.rs index 76b563766dc..f7d231c5166 100644 --- a/nexus/src/app/background/init.rs +++ b/nexus/src/app/background/init.rs @@ -791,6 +791,7 @@ impl BackgroundTasksInitializer { sagas.clone(), producer_registry, resolver.clone(), + inventory_load_watcher.clone(), instance_watcher::WatcherIdentity { nexus_id, rack_id }, ); driver.register(TaskDefinition { diff --git a/nexus/src/app/background/tasks/instance_watcher.rs b/nexus/src/app/background/tasks/instance_watcher.rs index 86e538cd8a6..f39ea1eb260 100644 --- a/nexus/src/app/background/tasks/instance_watcher.rs +++ b/nexus/src/app/background/tasks/instance_watcher.rs @@ -20,6 +20,7 @@ use nexus_networking::GatewayClient; use nexus_types::external_api::sled::SledPolicy; use nexus_types::identity::Asset; use nexus_types::identity::Resource; +use nexus_types::inventory; use omicron_common::api::external::Error; use omicron_common::api::external::InstanceState; use omicron_uuid_kinds::GenericUuid; @@ -31,6 +32,7 @@ use parallel_task_set::ParallelTaskSet; use sled_agent_client::Client as SledAgentClient; use sled_agent_types::instance; use sled_agent_types::instance::SledVmmState; +use sled_hardware_types::BaseboardId; use slog_error_chain::InlineErrorChain; use std::borrow::Cow; use std::collections::BTreeMap; @@ -38,6 +40,7 @@ use std::collections::VecDeque; use std::future::Future; use std::sync::Arc; use std::sync::Mutex; +use tokio::sync::watch; use uuid::Uuid; oximeter::use_timeseries!("vm-health-check.toml"); @@ -49,6 +52,7 @@ pub(crate) struct InstanceWatcher { sagas: Arc, resolver: internal_dns_resolver::Resolver, metrics: Arc>, + inv_rx: watch::Receiver>>, id: WatcherIdentity, } @@ -78,13 +82,14 @@ impl InstanceWatcher { sagas: Arc, producer_registry: &ProducerRegistry, resolver: internal_dns_resolver::Resolver, + inv_rx: watch::Receiver>>, id: WatcherIdentity, ) -> Self { let metrics = Arc::new(Mutex::new(metrics::Metrics::default())); producer_registry .register_producer(metrics::Producer(metrics.clone())) .unwrap(); - Self { datastore, sagas, metrics, id, resolver } + Self { datastore, sagas, metrics, id, resolver, inv_rx } } fn check_instance( @@ -99,6 +104,7 @@ impl InstanceWatcher { let datastore = self.datastore.clone(); let sagas = self.sagas.clone(); let gateways = gateways.clone(); + let inv_rx = self.inv_rx.clone(); let vmm_id = PropolisUuid::from_untyped_uuid(target.vmm_id); let opctx = { @@ -129,7 +135,7 @@ impl InstanceWatcher { }; let Some(state) = check - .run(&opctx, &datastore, &sled, &vmm, &gateways, &client) + .run(&opctx, inv_rx, &sled, &vmm, &gateways, &client) .await else { // Check did not result in an updated state, nothing else to @@ -190,7 +196,7 @@ impl Check { async fn run( &mut self, opctx: &OpContext, - datastore: &DataStore, + inv_rx: watch::Receiver>>, sled: &Sled, vmm: &Vmm, gateways: &[GatewayClient], @@ -272,8 +278,7 @@ impl Check { ); // Is your computer running? - match is_computer_on(&opctx, &datastore, &gateways, &sled).await - { + match is_computer_on(&opctx, &inv_rx, &gateways, &sled).await { // ...impossible to tell! Results are inconclusive. Err(error) => { let error = InlineErrorChain::new(&*error); @@ -324,19 +329,31 @@ impl Check { /// [1]: https://web.archive.org/web/20260309055003/https://www.haiku-os.org/legacy-docs/bebook/TheKernelKit_SystemInfo.html async fn is_computer_on( opctx: &OpContext, - datastore: &DataStore, + inv_rx: &watch::Receiver>>, gateways: &[GatewayClient], sled: &Sled, ) -> anyhow::Result { - use gateway_types::component::SpType; - use nexus_db_queries::db::datastore::sled::SledLocation; + let Some(inv) = inv_rx.borrow().clone() else { + anyhow::bail!( + "cannot find sled to ask if it's turned on, since there is no \ + inventory collection yet" + ); + }; - let SledLocation::Slot(slot) = - datastore.sled_try_to_find_slot(opctx, sled).await? - else { + let part_number = sled.part_number(); + let rev = sled.revision(); + let serial_number = sled.serial_number(); + + // TODO(eliza): this sucks, i would rather not have to memcpy the strings + // here...this would be nicer if we iterated over sled-agents from the + // inventory in the main loop. change that! + let bb = BaseboardId { + part_number: part_number.to_owned(), + serial_number: serial_number.to_owned(), + }; + let Some(sp) = inv.sps.get(&bb) else { anyhow::bail!( - "could not determine sled location (it may not yet be in the \ - inventory?)" + "SP for sled {part_number}:{rev}:{serial_number} is not in the inventory" ); }; @@ -344,7 +361,7 @@ async fn is_computer_on( // gateways fails for whatever reason, ask any others we were able to // resolve as well. for GatewayClient { client, addr } in gateways { - let state = match client.sp_get(&SpType::Sled, slot).await { + let state = match client.sp_get(&sp.sp_type, sp.sp_slot).await { Ok(response) => response.into_inner(), Err(error) => { let error = InlineErrorChain::new(&error); @@ -358,16 +375,14 @@ async fn is_computer_on( } }; - let part_number = sled.part_number(); - let rev = sled.revision(); - let sled_serial = sled.serial_number(); if state.serial_number != sled.serial_number() { // Well, this is surprising; is our inventory stale? In any case, if // we can't find the actual location of the sled, we can't determine // whether or not it's turned on... anyhow::bail!( - "expected sled {part_number}:{rev}:{sled_serial} to be in \ - slot {slot}, but found sled {}:{}:{} instead", + "expected sled {part_number}:{rev}:{serial_number} to be in \ + slot {}, but found sled {}:{}:{} instead", + sp.sp_slot, state.model, state.revision, state.serial_number, From 8cc9b798adccffb4cfd937aeef8170196c43aff5 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 28 Apr 2026 14:18:13 -0700 Subject: [PATCH 4/9] tidy --- nexus/db-queries/src/db/datastore/sled.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/sled.rs b/nexus/db-queries/src/db/datastore/sled.rs index 5391123374b..1910c625c80 100644 --- a/nexus/db-queries/src/db/datastore/sled.rs +++ b/nexus/db-queries/src/db/datastore/sled.rs @@ -18,7 +18,6 @@ use crate::db::model::Sled; use crate::db::model::SledResourceVmm; use crate::db::model::SledState; use crate::db::model::SledUpdate; -use crate::db::model::SqlU16; use crate::db::model::to_db_sled_policy; use crate::db::model::to_db_typed_uuid; use crate::db::pagination::Paginator; @@ -49,7 +48,6 @@ use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; -use omicron_common::api::external::LookupResult; use omicron_common::api::external::ResourceType; use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_uuid_kinds::DatasetUuid; From 07c8ba8733a777fad56e559621793e144c30328f Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 29 Apr 2026 11:11:26 -0700 Subject: [PATCH 5/9] rm unneeded schema change --- nexus/db-schema/src/schema.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 0dbcf131108..0a66531be52 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -1640,8 +1640,6 @@ table! { } } -allow_tables_to_appear_in_same_query!(hw_baseboard_id, inv_service_processor); - table! { sw_caboose (id) { id -> Uuid, From dda4998eb707e913e7a524b38094d69d4652adf5 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 29 Apr 2026 11:38:56 -0700 Subject: [PATCH 6/9] commentary tweaks --- .../app/background/tasks/instance_watcher.rs | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/nexus/src/app/background/tasks/instance_watcher.rs b/nexus/src/app/background/tasks/instance_watcher.rs index f39ea1eb260..87459d0a628 100644 --- a/nexus/src/app/background/tasks/instance_watcher.rs +++ b/nexus/src/app/background/tasks/instance_watcher.rs @@ -237,6 +237,11 @@ impl Check { .await .map_err(SledAgentInstanceError); match rsp { + // We received a response from the sled-agent. We shall update the + // instance's state to match. + // + // Note that this does not always mean that the *VMM* is healthy, + // but only that we successfully got its state from the sled-agent. Ok(rsp) => { let state = rsp.into_inner(); self.outcome = @@ -256,6 +261,11 @@ impl Check { self.outcome = CheckOutcome::Failure(Failure::NoSuchInstance); mk_failed() } + // We were able to contact the sled-agent, but it responded with an + // error which does *not* tell us that the VMM has failed. Either + // the sled-agent is unhealthy, or we sent an invalid request for + // some reason. In either case, the check is inconclusive and the + // instance's state will not change. Err(SledAgentInstanceError(ClientError::ErrorResponse(rsp))) => { let status = rsp.status(); if status.is_client_error() { @@ -270,6 +280,13 @@ impl Check { Err(Incomplete::SledAgentHttpError(status.as_u16())); None } + // We were unable to communicate with the sled-agent. This could be + // due to a network partition between us and the sled-agent, or + // because the sled is powered off or not present. We will use the + // management network to check whether the sled is present and + // powered on. If we determine conclusively that it is not present + // and in A0, the instance is moved to `Failed`. Otherwise, the + // check is inconclusive and the instance's state will not change. Err(SledAgentInstanceError(ClientError::CommunicationError(e))) => { slog::info!( opctx.log, @@ -309,6 +326,7 @@ impl Check { self.result = Err(Incomplete::SledAgentUnreachable); None } + // Any other errors mean that the check is inconclusive. Err(SledAgentInstanceError(e)) => { slog::warn!( opctx.log, @@ -323,8 +341,8 @@ impl Check { } } -/// An implementation of the `is_computer_on()` function [originally defined -/// in the BeOS C library][1]. +/// An implementation of the `is_computer_on()` function originally defined +/// [in the BeOS C library][1]. /// /// [1]: https://web.archive.org/web/20260309055003/https://www.haiku-os.org/legacy-docs/bebook/TheKernelKit_SystemInfo.html async fn is_computer_on( From a54661082123b5f700688a0fe66f885f36c0f54f Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 29 Apr 2026 12:13:09 -0700 Subject: [PATCH 7/9] string wrapping fixes --- nexus/src/app/background/tasks/instance_watcher.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nexus/src/app/background/tasks/instance_watcher.rs b/nexus/src/app/background/tasks/instance_watcher.rs index 87459d0a628..8ccf683a1e5 100644 --- a/nexus/src/app/background/tasks/instance_watcher.rs +++ b/nexus/src/app/background/tasks/instance_watcher.rs @@ -342,7 +342,7 @@ impl Check { } /// An implementation of the `is_computer_on()` function originally defined -/// [in the BeOS C library][1]. +/// in the BeOS C library][1]. /// /// [1]: https://web.archive.org/web/20260309055003/https://www.haiku-os.org/legacy-docs/bebook/TheKernelKit_SystemInfo.html async fn is_computer_on( @@ -371,7 +371,8 @@ async fn is_computer_on( }; let Some(sp) = inv.sps.get(&bb) else { anyhow::bail!( - "SP for sled {part_number}:{rev}:{serial_number} is not in the inventory" + "SP for sled {part_number}:{rev}:{serial_number} is not in the \ + inventory" ); }; From e59be439d4a01dc756e0914044e7862f08676615 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 29 Apr 2026 12:18:52 -0700 Subject: [PATCH 8/9] fix bad line wrapping in old code i moved around --- .../src/app/background/tasks/instance_watcher.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/nexus/src/app/background/tasks/instance_watcher.rs b/nexus/src/app/background/tasks/instance_watcher.rs index 8ccf683a1e5..a27b338e2ed 100644 --- a/nexus/src/app/background/tasks/instance_watcher.rs +++ b/nexus/src/app/background/tasks/instance_watcher.rs @@ -269,11 +269,19 @@ impl Check { Err(SledAgentInstanceError(ClientError::ErrorResponse(rsp))) => { let status = rsp.status(); if status.is_client_error() { - slog::warn!(opctx.log, "check incomplete due to client error"; - "status" => ?status, "error" => ?rsp.into_inner()); + slog::warn!( + opctx.log, + "check incomplete due to client error"; + "status" => ?status, + "error" => ?rsp.into_inner() + ); } else { - slog::info!(opctx.log, "check incomplete due to server error"; - "status" => ?status, "error" => ?rsp.into_inner()); + slog::info!( + opctx.log, + "check incomplete due to server error"; + "status" => ?status, + "error" => ?rsp.into_inner(), + ); } self.result = From 014fe6a0feb054321c999264e073e234c1a10c2e Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 29 Apr 2026 12:28:04 -0700 Subject: [PATCH 9/9] update all the old code to use `InlineErrorChain` --- .../app/background/tasks/instance_watcher.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/nexus/src/app/background/tasks/instance_watcher.rs b/nexus/src/app/background/tasks/instance_watcher.rs index a27b338e2ed..1895e7091f1 100644 --- a/nexus/src/app/background/tasks/instance_watcher.rs +++ b/nexus/src/app/background/tasks/instance_watcher.rs @@ -252,11 +252,12 @@ impl Check { // `Failed`. Let's synthesize a `SledInstanceState` that does // that. Err(e) if e.vmm_gone() => { + let error = InlineErrorChain::new(&e); slog::info!( opctx.log, "sled-agent error indicates that this instance's \ VMM has failed!"; - "error" => %e, + error, ); self.outcome = CheckOutcome::Failure(Failure::NoSuchInstance); mk_failed() @@ -267,20 +268,28 @@ impl Check { // some reason. In either case, the check is inconclusive and the // instance's state will not change. Err(SledAgentInstanceError(ClientError::ErrorResponse(rsp))) => { + // This is a bit goofy: we destructure the error because + // `ResponseValue` has a `status()` which is not optional (so we + // don't have to unwrap it), but then we re-construct the + // `progenitor_client::Error` because we would like to format it + // with `InlineErrorChain`. This looks silly but there's nothing + // actually *wrong* with it... let status = rsp.status(); + let error = ClientError::ErrorResponse(rsp); + let error = InlineErrorChain::new(&error); if status.is_client_error() { slog::warn!( opctx.log, "check incomplete due to client error"; "status" => ?status, - "error" => ?rsp.into_inner() + "error" => error, ); } else { slog::info!( opctx.log, "check incomplete due to server error"; "status" => ?status, - "error" => ?rsp.into_inner(), + "error" => error, ); } @@ -339,7 +348,7 @@ impl Check { slog::warn!( opctx.log, "error checking up on instance"; - "error" => ?e, + "error" => InlineErrorChain::new(&e), "status" => ?e.status(), ); self.result = Err(Incomplete::ClientError);