From d1e716505fb8cd224f68224dbf1d2b3df726ce55 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 28 Aug 2024 13:46:49 -0700 Subject: [PATCH] okay FINE (i really hope this is safe) --- nexus/db-model/src/vmm_state.rs | 2 +- nexus/db-queries/src/db/datastore/vmm.rs | 40 +++++++++++++++++++++--- nexus/src/app/sagas/instance_common.rs | 27 ---------------- nexus/src/app/sagas/instance_migrate.rs | 19 +++++------ nexus/src/app/sagas/instance_start.rs | 18 +++++------ 5 files changed, 54 insertions(+), 52 deletions(-) diff --git a/nexus/db-model/src/vmm_state.rs b/nexus/db-model/src/vmm_state.rs index 8d3c3f2d9c4..6ba29331b8c 100644 --- a/nexus/db-model/src/vmm_state.rs +++ b/nexus/db-model/src/vmm_state.rs @@ -48,7 +48,7 @@ impl VmmState { /// States in which it is safe to deallocate a VMM's sled resources and mark /// it as deleted. pub const DESTROYABLE_STATES: &'static [Self] = - &[Self::Destroyed, Self::SagaUnwound]; + &[Self::Destroyed, Self::Failed, Self::SagaUnwound]; pub const TERMINAL_STATES: &'static [Self] = &[Self::Destroyed, Self::Failed]; diff --git a/nexus/db-queries/src/db/datastore/vmm.rs b/nexus/db-queries/src/db/datastore/vmm.rs index 089a2914be2..285cb33f25d 100644 --- a/nexus/db-queries/src/db/datastore/vmm.rs +++ b/nexus/db-queries/src/db/datastore/vmm.rs @@ -82,11 +82,9 @@ impl DataStore { opctx: &OpContext, vmm_id: &PropolisUuid, ) -> UpdateResult { - let valid_states = vec![DbVmmState::Destroyed, DbVmmState::Failed]; - let updated = diesel::update(dsl::vmm) .filter(dsl::id.eq(vmm_id.into_untyped_uuid())) - .filter(dsl::state.eq_any(valid_states)) + .filter(dsl::state.eq_any(DbVmmState::DESTROYABLE_STATES)) .filter(dsl::time_deleted.is_null()) .set(dsl::time_deleted.eq(Utc::now())) .check_if_exists::(vmm_id.into_untyped_uuid()) @@ -307,6 +305,40 @@ impl DataStore { }) } + /// Transitions a VMM to the `SagaUnwound` state. + /// + /// This may *only* be called by the saga that created a VMM record. + pub async fn vmm_mark_saga_unwound( + &self, + opctx: &OpContext, + vmm_id: &PropolisUuid, + ) -> Result { + diesel::update(dsl::vmm) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::id.eq(vmm_id.into_untyped_uuid())) + .set(( + dsl::state.eq(DbVmmState::SagaUnwound), + dsl::time_state_updated.eq(chrono::Utc::now()), + dsl::state_generation.eq(dsl::state_generation + 1), + )) + .check_if_exists::(vmm_id.into_untyped_uuid()) + .execute_and_check(&*self.pool_connection_authorized(opctx).await?) + .await + .map(|r| match r.status { + UpdateStatus::Updated => true, + UpdateStatus::NotUpdatedButExists => false, + }) + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Vmm, + LookupType::ById(vmm_id.into_untyped_uuid()), + ), + ) + }) + } + /// Forcibly overwrites the Propolis IP/Port in the supplied VMM's record with /// the supplied Propolis IP. /// @@ -357,7 +389,7 @@ impl DataStore { paginated(dsl::vmm, dsl::id, pagparams) // In order to be considered "abandoned", a VMM must be: - // - in the `Destroyed` or `SagaUnwound` state + // - in the `Destroyed`, `SagaUnwound`, or `Failed` states .filter(dsl::state.eq_any(DbVmmState::DESTROYABLE_STATES)) // - not deleted yet .filter(dsl::time_deleted.is_null()) diff --git a/nexus/src/app/sagas/instance_common.rs b/nexus/src/app/sagas/instance_common.rs index ca793fc1238..48662bd7160 100644 --- a/nexus/src/app/sagas/instance_common.rs +++ b/nexus/src/app/sagas/instance_common.rs @@ -7,7 +7,6 @@ use std::net::{IpAddr, Ipv6Addr}; use crate::Nexus; -use chrono::Utc; use nexus_db_model::{ ByteCount, ExternalIp, InstanceState, IpAttachState, Ipv4NatEntry, SledReservationConstraints, SledResource, VmmState, @@ -115,32 +114,6 @@ pub async fn create_and_insert_vmm_record( Ok(vmm) } -/// Given a previously-inserted VMM record, set its state to `SagaUnwound` and -/// then delete it. -/// -/// This function succeeds idempotently if called with the same parameters, -/// provided that the VMM record was not changed by some other actor after the -/// calling saga inserted it. -/// -/// This function is intended to be called when a saga which created a VMM -/// record unwinds. -pub async fn unwind_vmm_record( - datastore: &DataStore, - opctx: &OpContext, - prev_record: &db::model::Vmm, -) -> Result<(), anyhow::Error> { - let new_runtime = db::model::VmmRuntimeState { - state: db::model::VmmState::SagaUnwound, - time_state_updated: Utc::now(), - gen: prev_record.runtime.gen.next().into(), - }; - - let prev_id = PropolisUuid::from_untyped_uuid(prev_record.id); - datastore.vmm_update_runtime(&prev_id, &new_runtime).await?; - datastore.vmm_mark_deleted(&opctx, &prev_id).await?; - Ok(()) -} - /// Allocates a new IPv6 address for a propolis instance that will run on the /// supplied sled. pub(super) async fn allocate_vmm_ipv6( diff --git a/nexus/src/app/sagas/instance_migrate.rs b/nexus/src/app/sagas/instance_migrate.rs index 4cda7ebe943..a484d7d8250 100644 --- a/nexus/src/app/sagas/instance_migrate.rs +++ b/nexus/src/app/sagas/instance_migrate.rs @@ -342,18 +342,15 @@ async fn sim_destroy_vmm_record( ¶ms.serialized_authn, ); - let vmm = sagactx - .lookup::(REGISTERED_VMM_RECORD) - .or_else(|_| sagactx.lookup::(INITIAL_VMM_RECORD))?; - info!(osagactx.log(), "destroying vmm record for migration unwind"; - "propolis_id" => %vmm.id); + let propolis_id = sagactx.lookup::("dst_propolis_id")?; + info!( + osagactx.log(), + "destroying vmm record for migration unwind"; + "propolis_id" => %propolis_id, + ); - super::instance_common::unwind_vmm_record( - osagactx.datastore(), - &opctx, - &vmm, - ) - .await + osagactx.datastore().vmm_mark_saga_unwound(&opctx, &propolis_id).await?; + Ok(()) } async fn sim_set_migration_ids( diff --git a/nexus/src/app/sagas/instance_start.rs b/nexus/src/app/sagas/instance_start.rs index a67ca58e701..66d4448f5a0 100644 --- a/nexus/src/app/sagas/instance_start.rs +++ b/nexus/src/app/sagas/instance_start.rs @@ -217,15 +217,15 @@ async fn sis_destroy_vmm_record( ¶ms.serialized_authn, ); - let vmm = sagactx - .lookup::(REGISTERED_VMM_RECORD) - .or_else(|_| sagactx.lookup::(INITIAL_VMM_RECORD))?; - super::instance_common::unwind_vmm_record( - osagactx.datastore(), - &opctx, - &vmm, - ) - .await + let propolis_id = sagactx.lookup::("propolis_id")?; + info!( + osagactx.log(), + "destroying vmm record for start saga unwind"; + "propolis_id" => %propolis_id, + ); + + osagactx.datastore().vmm_mark_saga_unwound(&opctx, &propolis_id).await?; + Ok(()) } async fn sis_move_to_starting(