diff --git a/nexus/db-queries/src/db/datastore/migration.rs b/nexus/db-queries/src/db/datastore/migration.rs index 8a7d1c645bd..128239503cb 100644 --- a/nexus/db-queries/src/db/datastore/migration.rs +++ b/nexus/db-queries/src/db/datastore/migration.rs @@ -6,12 +6,16 @@ use super::DataStore; use crate::context::OpContext; +use crate::db; use crate::db::error::public_error_from_diesel; use crate::db::error::ErrorHandler; -use crate::db::model::{Migration, MigrationState}; +use crate::db::model::Generation; +use crate::db::model::Migration; +use crate::db::model::MigrationState; use crate::db::pagination::paginated; use crate::db::schema::migration::dsl; use crate::db::update_and_check::UpdateAndCheck; +use crate::db::update_and_check::UpdateAndQueryResult; use crate::db::update_and_check::UpdateStatus; use async_bb8_diesel::AsyncRunQueryDsl; use chrono::Utc; @@ -23,6 +27,7 @@ use omicron_common::api::external::UpdateResult; use omicron_common::api::internal::nexus; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::InstanceUuid; +use omicron_uuid_kinds::PropolisUuid; use uuid::Uuid; impl DataStore { @@ -123,6 +128,50 @@ impl DataStore { }) .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + + pub(crate) async fn migration_update_source_on_connection( + &self, + conn: &async_bb8_diesel::Connection, + vmm_id: &PropolisUuid, + migration: &nexus::MigrationRuntimeState, + ) -> Result, diesel::result::Error> { + let generation = Generation(migration.r#gen); + diesel::update(dsl::migration) + .filter(dsl::id.eq(migration.migration_id)) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::source_gen.lt(generation)) + .filter(dsl::source_propolis_id.eq(vmm_id.into_untyped_uuid())) + .set(( + dsl::source_state.eq(MigrationState(migration.state)), + dsl::source_gen.eq(generation), + dsl::time_source_updated.eq(migration.time_updated), + )) + .check_if_exists::(migration.migration_id) + .execute_and_check(conn) + .await + } + + pub(crate) async fn migration_update_target_on_connection( + &self, + conn: &async_bb8_diesel::Connection, + vmm_id: &PropolisUuid, + migration: &nexus::MigrationRuntimeState, + ) -> Result, diesel::result::Error> { + let generation = Generation(migration.r#gen); + diesel::update(dsl::migration) + .filter(dsl::id.eq(migration.migration_id)) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::target_gen.lt(generation)) + .filter(dsl::target_propolis_id.eq(vmm_id.into_untyped_uuid())) + .set(( + dsl::target_state.eq(MigrationState(migration.state)), + dsl::target_gen.eq(generation), + dsl::time_target_updated.eq(migration.time_updated), + )) + .check_if_exists::(migration.migration_id) + .execute_and_check(conn) + .await + } } #[cfg(test)] diff --git a/nexus/db-queries/src/db/datastore/vmm.rs b/nexus/db-queries/src/db/datastore/vmm.rs index c5988468618..14a922fcf21 100644 --- a/nexus/db-queries/src/db/datastore/vmm.rs +++ b/nexus/db-queries/src/db/datastore/vmm.rs @@ -7,6 +7,7 @@ use super::DataStore; use crate::authz; use crate::context::OpContext; +use crate::db; use crate::db::error::public_error_from_diesel; use crate::db::error::ErrorHandler; use crate::db::model::Vmm; @@ -15,18 +16,22 @@ use crate::db::model::VmmState as DbVmmState; use crate::db::pagination::paginated; use crate::db::schema::vmm::dsl; use crate::db::update_and_check::UpdateAndCheck; +use crate::db::update_and_check::UpdateAndQueryResult; use crate::db::update_and_check::UpdateStatus; +use crate::transaction_retry::OptionalError; use async_bb8_diesel::AsyncRunQueryDsl; use chrono::Utc; use diesel::prelude::*; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::Error; +use omicron_common::api::external::InternalContext; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::LookupType; use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; +use omicron_common::api::internal::nexus; use omicron_common::api::internal::nexus::Migrations; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::PropolisUuid; @@ -133,29 +138,41 @@ impl DataStore { vmm_id: &PropolisUuid, new_runtime: &VmmRuntimeState, ) -> Result { - let updated = diesel::update(dsl::vmm) + self.vmm_update_runtime_on_connection( + &*self.pool_connection_unauthorized().await?, + vmm_id, + new_runtime, + ) + .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()), + ), + ) + }) + } + + async fn vmm_update_runtime_on_connection( + &self, + conn: &async_bb8_diesel::Connection, + vmm_id: &PropolisUuid, + new_runtime: &VmmRuntimeState, + ) -> Result, diesel::result::Error> { + diesel::update(dsl::vmm) .filter(dsl::time_deleted.is_null()) .filter(dsl::id.eq(vmm_id.into_untyped_uuid())) .filter(dsl::state_generation.lt(new_runtime.gen)) .set(new_runtime.clone()) .check_if_exists::(vmm_id.into_untyped_uuid()) - .execute_and_check(&*self.pool_connection_unauthorized().await?) + .execute_and_check(conn) .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()), - ), - ) - })?; - - Ok(updated) } /// Updates a VMM record and associated migration record(s) with a single @@ -185,33 +202,94 @@ impl DataStore { /// - `Err` if another error occurred while accessing the database. pub async fn vmm_and_migration_update_runtime( &self, + opctx: &OpContext, vmm_id: PropolisUuid, new_runtime: &VmmRuntimeState, - migrations: Migrations<'_>, + Migrations { migration_in, migration_out }: Migrations<'_>, ) -> Result { - let query = crate::db::queries::vmm::VmmAndMigrationUpdate::new( - vmm_id, - new_runtime.clone(), - migrations, - ); - - // The VmmAndMigrationUpdate query handles and indicates failure to find - // either the VMM or the migration, so a query failure here indicates - // some kind of internal error and not a failed lookup. - let result = query - .execute_and_check(&*self.pool_connection_unauthorized().await?) + fn migration_id( + m: Option<&nexus::MigrationRuntimeState>, + ) -> Option { + m.as_ref().map(|m| m.migration_id) + } + + if migration_id(migration_in) == migration_id(migration_out) { + return Err(Error::conflict( + "migrating from a VMM to itself is nonsensical", + )) + .internal_context(format!("migration_in: {migration_in:?}; migration_out: {migration_out:?}")); + } + + let err = OptionalError::new(); + let conn = self.pool_connection_authorized(opctx).await?; + + self.transaction_retry_wrapper("vmm_and_migration_update_runtime") + .transaction(&conn, |conn| { + let err = err.clone(); + async move { + let vmm_updated = self + .vmm_update_runtime_on_connection( + &conn, + &vmm_id, + new_runtime, + ) + .await.map(|r| match r.status { UpdateStatus::Updated => true, UpdateStatus::NotUpdatedButExists => false })?; + let migration_out_updated = match migration_out { + Some(migration) => { + let r = self.migration_update_source_on_connection( + &conn, &vmm_id, migration, + ) + .await?; + match r.status { + UpdateStatus::Updated => true, + UpdateStatus::NotUpdatedButExists => match r.found { + m if m.time_deleted.is_some() => return Err(err.bail(Error::Gone)), + m if m.source_propolis_id != vmm_id.into_untyped_uuid() => { + return Err(err.bail(Error::invalid_value( + "source propolis UUID", + format!("{vmm_id} is not the source VMM of this migration"), + ))); + } + // Not updated, generation has advanced. + _ => false + }, + } + }, + None => false, + }; + let migration_in_updated = match migration_in { + Some(migration) => { + let r = self.migration_update_target_on_connection( + &conn, &vmm_id, migration, + ) + .await?; + match r.status { + UpdateStatus::Updated => true, + UpdateStatus::NotUpdatedButExists => match r.found { + m if m.time_deleted.is_some() => return Err(err.bail(Error::Gone)), + m if m.target_propolis_id != vmm_id.into_untyped_uuid() => { + return Err(err.bail(Error::invalid_value( + "target propolis UUID", + format!("{vmm_id} is not the target VMM of this migration"), + ))); + } + // Not updated, generation has advanced. + _ => false + }, + } + }, + None => false, + }; + Ok(VmmStateUpdateResult { + vmm_updated, + migration_in_updated, + migration_out_updated, + }) + }}) .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; - - Ok(VmmStateUpdateResult { - vmm_updated: match result.vmm_status { - Some(UpdateStatus::Updated) => true, - Some(UpdateStatus::NotUpdatedButExists) => false, - None => false, - }, - migration_in_updated: result.migration_in_status.was_updated(), - migration_out_updated: result.migration_out_status.was_updated(), - }) + .map_err(|e| { + err.take().unwrap_or_else(|| public_error_from_diesel(e, ErrorHandler::Server)) + }) } /// Forcibly overwrites the Propolis IP/Port in the supplied VMM's record with @@ -392,6 +470,7 @@ mod tests { }; datastore .vmm_and_migration_update_runtime( + &opctx, PropolisUuid::from_untyped_uuid(vmm1.id), &VmmRuntimeState { time_state_updated: Utc::now(), @@ -413,6 +492,7 @@ mod tests { }; datastore .vmm_and_migration_update_runtime( + &opctx, PropolisUuid::from_untyped_uuid(vmm2.id), &VmmRuntimeState { time_state_updated: Utc::now(), @@ -499,6 +579,7 @@ mod tests { }; datastore .vmm_and_migration_update_runtime( + &opctx, PropolisUuid::from_untyped_uuid(vmm2.id), &VmmRuntimeState { time_state_updated: Utc::now(), @@ -522,6 +603,7 @@ mod tests { }; datastore .vmm_and_migration_update_runtime( + &opctx, PropolisUuid::from_untyped_uuid(vmm3.id), &VmmRuntimeState { time_state_updated: Utc::now(), @@ -569,7 +651,7 @@ mod tests { .find(|m| m.id == migration2.id) .expect("query must include migration2"); assert_eq!( - new_db_migration2.source_state, + db_migration2.source_state, db::model::MigrationState::COMPLETED ); assert_eq!( diff --git a/nexus/db-queries/src/db/queries/mod.rs b/nexus/db-queries/src/db/queries/mod.rs index 46e8a7bc163..f88b8fab6d8 100644 --- a/nexus/db-queries/src/db/queries/mod.rs +++ b/nexus/db-queries/src/db/queries/mod.rs @@ -8,7 +8,6 @@ pub mod disk; pub mod external_ip; pub mod ip_pool; -pub mod vmm; #[macro_use] mod next_item; pub mod network_interface; diff --git a/nexus/db-queries/src/db/queries/vmm.rs b/nexus/db-queries/src/db/queries/vmm.rs deleted file mode 100644 index e8eec47141d..00000000000 --- a/nexus/db-queries/src/db/queries/vmm.rs +++ /dev/null @@ -1,549 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! Implement a query for updating an instance and VMM in a single CTE. - -use async_bb8_diesel::AsyncRunQueryDsl; -use diesel::prelude::QueryResult; -use diesel::query_builder::{Query, QueryFragment, QueryId}; -use diesel::result::Error as DieselError; -use diesel::sql_types::{Nullable, Uuid as SqlUuid}; -use diesel::{pg::Pg, query_builder::AstPass}; -use diesel::{Column, ExpressionMethods, QueryDsl, RunQueryDsl}; -use nexus_db_model::{ - schema::{migration::dsl as migration_dsl, vmm::dsl as vmm_dsl}, - Generation, MigrationState, VmmRuntimeState, -}; -use omicron_common::api::internal::nexus::{MigrationRuntimeState, Migrations}; -use omicron_uuid_kinds::{GenericUuid, PropolisUuid}; -use uuid::Uuid; - -use crate::db::pool::DbConnection; -use crate::db::update_and_check::UpdateStatus; - -/// A CTE that checks and updates the VMM and migration tables in a single -/// atomic operation. -// -// The single-table update-and-check CTE has the following form: -// -// WITH found AS (SELECT FROM T WHERE ) -// updated AS (UPDATE T SET RETURNING *) -// SELECT -// found. -// updated. -// found.* -// FROM -// found -// LEFT JOIN -// updated -// ON -// found. = updated.; -// -// The idea behind this query is to have separate "found" and "updated" -// subqueries for the VMM and migration tables, then use those to create two more -// subqueries that perform the joins and yield the results, along the following -// lines: -// -// WITH vmm_found AS (SELECT(SELECT id FROM vmm WHERE vmm.id = id) AS id), -// vmm_updated AS (UPDATE vmm SET ... RETURNING *), -// migration_in_found AS (SELECT( -// SELECT id FROM migration WHERE migration.id = migration_in_id -// ) AS id), -// migration_in_updated AS (UPDATE migration SET ... RETURNING *), -// migration_out_found AS (SELECT( -// SELECT id FROM migration WHERE migration.id = migration_out_id -// ) AS id), -// migration_out_updated AS (UPDATE migration SET ... RETURNING *), -// vmm_result AS ( -// SELECT vmm_found.id AS found, vmm_updated.id AS updated -// FROM vmm_found -// LEFT JOIN vmm_updated -// ON vmm_found.id = vmm_updated.id -// ), -// migration_in_result AS ( -// SELECT migration_in_found.id AS found, migration_in_updated.id AS updated -// FROM migration_in_found -// LEFT JOIN migration_in_updated -// ON migration_in_found.id = migration_in_updated.id -// ), -// migration_out_result AS ( .. ) -// SELECT vmm_result.found, vmm_result.updated, migration_in_result.found, -// migration_in_result.updated, migration_out_result.found, -// migration_out_result.updated, -// FROM vmm_result, migration_in_result, migration_out_result; -// -// Depending on whether a migration in, migration out, both, or neither were -// provided, the structure of the query will differ somewhat. -// -// The "wrapper" SELECTs when finding migrations and VMMs are used to get a NULL -// result in the final output instead of failing the entire query if the target -// object is missing. This maximizes Nexus's flexibility when dealing with -// updates from sled agent that refer to one valid and one deleted object. (This -// can happen if, e.g., sled agent sends a message indicating that a retired VMM -// has finally been destroyed when its instance has since been deleted.) -pub struct VmmAndMigrationUpdate { - vmm_find: Box + Send>, - vmm_update: Box + Send>, - migration_in: Option, - migration_out: Option, -} - -struct Update { - name: &'static str, - id: &'static str, - find: Box + Send>, - update: Box + Send>, -} - -/// Contains the result of a combined instance-and-VMM update operation. -#[derive(Copy, Clone, PartialEq, Debug)] -pub struct VmmAndMigrationUpdateResult { - /// `Some(status)` if the target VMM was found; the wrapped `UpdateStatus` - /// indicates whether the row was updated. `None` if the VMM was not found. - pub vmm_status: Option, - - /// Indicates whether a migration-in update was performed. - pub migration_in_status: RecordUpdateStatus, - - /// Indicates whether a migration-out update was performed. - pub migration_out_status: RecordUpdateStatus, -} - -#[derive(Copy, Clone, PartialEq, Debug)] -pub enum RecordUpdateStatus { - /// No record was found for the provided ID. - NotFound, - /// No record for this table was provided as part of the update. - NotProvided, - /// An update for this record was provided, and a a record matching the - /// provided ID exists. - Found(UpdateStatus), -} - -impl RecordUpdateStatus { - pub fn was_updated(self) -> bool { - matches!(self, Self::Found(UpdateStatus::Updated)) - } -} - -/// Computes the update status to return from the results of queries that find -/// and update an object with an ID of type `T`. -fn compute_update_status( - found: Option, - updated: Option, -) -> Option -where - T: PartialEq + std::fmt::Display, -{ - match (found, updated) { - // If both the "find" and "update" prongs returned an ID, the row was - // updated. The IDs should match in this case (if they don't then the - // query was constructed very strangely!). - (Some(found_id), Some(updated_id)) if found_id == updated_id => { - Some(UpdateStatus::Updated) - } - // If the "find" prong returned an ID but the "update" prong didn't, the - // row exists but wasn't updated. - (Some(_), None) => Some(UpdateStatus::NotUpdatedButExists), - // If neither prong returned anything, indicate the row is missing. - (None, None) => None, - // If both prongs returned an ID, but they don't match, something - // terrible has happened--the prongs must have referred to different - // IDs! - (Some(found_id), Some(mismatched_id)) => unreachable!( - "updated ID {} didn't match found ID {}", - mismatched_id, found_id - ), - // Similarly, if the target ID was not found but something was updated - // anyway, then something is wrong with the update query--either it has - // the wrong ID or did not filter rows properly. - (None, Some(updated_id)) => unreachable!( - "ID {} was updated but no found ID was supplied", - updated_id - ), - } -} - -impl VmmAndMigrationUpdate { - pub fn new( - vmm_id: PropolisUuid, - new_vmm_runtime_state: VmmRuntimeState, - Migrations { migration_in, migration_out }: Migrations<'_>, - ) -> Self { - let vmm_find = Box::new( - vmm_dsl::vmm - .filter(vmm_dsl::id.eq(vmm_id.into_untyped_uuid())) - .select(vmm_dsl::id), - ); - - let vmm_update = Box::new( - diesel::update(vmm_dsl::vmm) - .filter(vmm_dsl::time_deleted.is_null()) - .filter(vmm_dsl::id.eq(vmm_id.into_untyped_uuid())) - .filter(vmm_dsl::state_generation.lt(new_vmm_runtime_state.gen)) - .set(new_vmm_runtime_state), - ); - - fn migration_find( - migration_id: Uuid, - ) -> Box + Send> { - Box::new( - migration_dsl::migration - .filter(migration_dsl::id.eq(migration_id)) - .filter(migration_dsl::time_deleted.is_null()) - .select(migration_dsl::id), - ) - } - - let migration_in = migration_in.cloned().map( - |MigrationRuntimeState { - migration_id, - state, - gen, - time_updated, - }| { - let state = MigrationState::from(state); - let gen = Generation::from(gen); - let update = Box::new( - diesel::update(migration_dsl::migration) - .filter(migration_dsl::id.eq(migration_id)) - .filter( - migration_dsl::target_propolis_id - .eq(vmm_id.into_untyped_uuid()), - ) - .filter(migration_dsl::target_gen.lt(gen)) - .set(( - migration_dsl::target_state.eq(state), - migration_dsl::time_target_updated.eq(time_updated), - migration_dsl::target_gen.eq(gen), - )), - ); - Update { - find: migration_find(migration_id), - update, - name: "migration_in", - id: migration_dsl::id::NAME, - } - }, - ); - - let migration_out = migration_out.cloned().map( - |MigrationRuntimeState { - migration_id, - state, - gen, - time_updated, - }| { - let state = MigrationState::from(state); - let gen = Generation::from(gen); - let update = Box::new( - diesel::update(migration_dsl::migration) - .filter(migration_dsl::id.eq(migration_id)) - .filter( - migration_dsl::source_propolis_id - .eq(vmm_id.into_untyped_uuid()), - ) - .filter(migration_dsl::source_gen.lt(gen)) - .set(( - migration_dsl::source_state.eq(state), - migration_dsl::time_source_updated.eq(time_updated), - migration_dsl::source_gen.eq(gen), - )), - ); - Update { - find: migration_find(migration_id), - update, - name: "migration_out", - id: migration_dsl::id::NAME, - } - }, - ); - - Self { vmm_find, vmm_update, migration_in, migration_out } - } - - pub async fn execute_and_check( - self, - conn: &(impl async_bb8_diesel::AsyncConnection + Sync), - ) -> Result { - let has_migration_in = self.migration_in.is_some(); - let has_migration_out = self.migration_out.is_some(); - let ( - vmm_found, - vmm_updated, - migration_in_found, - migration_in_updated, - migration_out_found, - migration_out_updated, - ) = self - .get_result_async::<( - Option, - Option, - Option, - Option, - Option, - Option, - // WHEW! - )>(conn) - .await?; - - let vmm_status = compute_update_status(vmm_found, vmm_updated); - - let migration_in_status = if has_migration_in { - compute_update_status(migration_in_found, migration_in_updated) - .map(RecordUpdateStatus::Found) - .unwrap_or(RecordUpdateStatus::NotFound) - } else { - RecordUpdateStatus::NotProvided - }; - - let migration_out_status = if has_migration_out { - compute_update_status(migration_out_found, migration_out_updated) - .map(RecordUpdateStatus::Found) - .unwrap_or(RecordUpdateStatus::NotFound) - } else { - RecordUpdateStatus::NotProvided - }; - - Ok(VmmAndMigrationUpdateResult { - vmm_status, - migration_in_status, - migration_out_status, - }) - } -} - -impl QueryId for VmmAndMigrationUpdate { - type QueryId = (); - const HAS_STATIC_QUERY_ID: bool = false; -} - -impl Query for VmmAndMigrationUpdate { - type SqlType = ( - Nullable, - Nullable, - Nullable, - Nullable, - Nullable, - Nullable, - ); -} - -impl RunQueryDsl for VmmAndMigrationUpdate {} - -impl Update { - fn push_subqueries<'b>( - &'b self, - out: &mut AstPass<'_, 'b, Pg>, - ) -> QueryResult<()> { - out.push_sql(self.name); - out.push_sql("_found AS (SELECT ("); - self.find.walk_ast(out.reborrow())?; - out.push_sql(") AS ID), "); - out.push_sql(self.name); - out.push_sql("_updated AS ("); - self.update.walk_ast(out.reborrow())?; - out.push_sql("RETURNING id), "); - out.push_sql(self.name); - out.push_sql("_result AS (SELECT "); - out.push_sql(self.name); - out.push_sql("_found."); - out.push_identifier(self.id)?; - out.push_sql(" AS found, "); - out.push_sql(self.name); - out.push_sql("_updated."); - out.push_identifier(self.id)?; - out.push_sql(" AS updated"); - out.push_sql(" FROM "); - out.push_sql(self.name); - out.push_sql("_found LEFT JOIN "); - out.push_sql(self.name); - out.push_sql("_updated ON "); - out.push_sql(self.name); - out.push_sql("_found."); - out.push_identifier(self.id)?; - out.push_sql("= "); - out.push_sql(self.name); - out.push_sql("_updated."); - out.push_identifier(self.id)?; - out.push_sql(")"); - - Ok(()) - } -} - -impl QueryFragment for VmmAndMigrationUpdate { - fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, Pg>) -> QueryResult<()> { - out.push_sql("WITH "); - - if let Some(ref m) = self.migration_in { - m.push_subqueries(&mut out)?; - out.push_sql(", "); - } - - if let Some(ref m) = self.migration_out { - m.push_subqueries(&mut out)?; - out.push_sql(", "); - } - - out.push_sql("vmm_found AS (SELECT ("); - self.vmm_find.walk_ast(out.reborrow())?; - out.push_sql(") AS id), "); - - out.push_sql("vmm_updated AS ("); - self.vmm_update.walk_ast(out.reborrow())?; - out.push_sql(" RETURNING id), "); - out.push_sql("vmm_result AS ("); - out.push_sql("SELECT vmm_found."); - out.push_identifier(vmm_dsl::id::NAME)?; - out.push_sql(" AS found, vmm_updated."); - out.push_identifier(vmm_dsl::id::NAME)?; - out.push_sql(" AS updated"); - out.push_sql(" FROM vmm_found LEFT JOIN vmm_updated ON vmm_found."); - out.push_identifier(vmm_dsl::id::NAME)?; - out.push_sql(" = vmm_updated."); - out.push_identifier(vmm_dsl::id::NAME)?; - out.push_sql(") "); - - fn push_select_from_result( - update: Option<&Update>, - out: &mut AstPass<'_, '_, Pg>, - ) { - if let Some(update) = update { - out.push_sql(update.name); - out.push_sql("_result.found, "); - out.push_sql(update.name); - out.push_sql("_result.updated"); - } else { - out.push_sql("NULL, NULL") - } - } - - out.push_sql("SELECT vmm_result.found, vmm_result.updated, "); - push_select_from_result(self.migration_in.as_ref(), &mut out); - out.push_sql(", "); - push_select_from_result(self.migration_out.as_ref(), &mut out); - out.push_sql(" "); - - out.push_sql("FROM vmm_result"); - if self.migration_in.is_some() { - out.push_sql(", migration_in_result"); - } - - if self.migration_out.is_some() { - out.push_sql(", migration_out_result"); - } - - Ok(()) - } -} - -#[cfg(test)] -mod test { - use super::*; - use crate::db::model::Generation; - use crate::db::model::VmmState; - use crate::db::raw_query_builder::expectorate_query_contents; - use chrono::Utc; - use omicron_common::api::internal::nexus::MigrationRuntimeState; - use omicron_common::api::internal::nexus::MigrationState; - use uuid::Uuid; - - // These tests are a bit of a "change detector", but they're here to help - // with debugging too. If you change this query, it can be useful to see - // exactly how the output SQL has been altered. - - fn mk_vmm_state() -> VmmRuntimeState { - VmmRuntimeState { - time_state_updated: Utc::now(), - gen: Generation::new(), - state: VmmState::Starting, - } - } - - fn mk_migration_state() -> MigrationRuntimeState { - let migration_id = Uuid::nil(); - MigrationRuntimeState { - migration_id, - state: MigrationState::Pending, - gen: Generation::new().into(), - time_updated: Utc::now(), - } - } - - #[tokio::test] - async fn expectorate_query_only_vmm() { - let vmm_id = PropolisUuid::nil(); - let vmm_state = mk_vmm_state(); - - let query = VmmAndMigrationUpdate::new( - vmm_id, - vmm_state, - Migrations::default(), - ); - expectorate_query_contents( - &query, - "tests/output/vmm_and_migration_update_vmm_only.sql", - ) - .await; - } - - #[tokio::test] - async fn expectorate_query_vmm_and_migration_in() { - let vmm_id = PropolisUuid::nil(); - let vmm_state = mk_vmm_state(); - let migration = mk_migration_state(); - - let query = VmmAndMigrationUpdate::new( - vmm_id, - vmm_state, - Migrations { migration_in: Some(&migration), migration_out: None }, - ); - expectorate_query_contents( - &query, - "tests/output/vmm_and_migration_update_vmm_and_migration_in.sql", - ) - .await; - } - - #[tokio::test] - async fn expectorate_query_vmm_and_migration_out() { - let vmm_id = PropolisUuid::nil(); - let vmm_state = mk_vmm_state(); - let migration = mk_migration_state(); - - let query = VmmAndMigrationUpdate::new( - vmm_id, - vmm_state, - Migrations { migration_out: Some(&migration), migration_in: None }, - ); - expectorate_query_contents( - &query, - "tests/output/vmm_and_migration_update_vmm_and_migration_out.sql", - ) - .await; - } - - #[tokio::test] - async fn expectorate_query_vmm_and_both_migrations() { - let vmm_id = PropolisUuid::nil(); - let vmm_state = mk_vmm_state(); - let migration_in = mk_migration_state(); - let migration_out = mk_migration_state(); - - let query = VmmAndMigrationUpdate::new( - vmm_id, - vmm_state, - Migrations { - migration_in: Some(&migration_in), - migration_out: Some(&migration_out), - }, - ); - expectorate_query_contents( - &query, - "tests/output/vmm_and_migration_update_vmm_and_both_migrations.sql", - ) - .await; - } -} diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 29726f5a784..2368d83ecfe 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -1879,6 +1879,7 @@ pub(crate) async fn notify_instance_updated( let result = datastore .vmm_and_migration_update_runtime( + &opctx, propolis_id, // TODO(eliza): probably should take this by value... &new_runtime_state.vmm_state.clone().into(), diff --git a/nexus/src/app/sagas/instance_update/mod.rs b/nexus/src/app/sagas/instance_update/mod.rs index 311d7db78a1..7a9fdfb39f9 100644 --- a/nexus/src/app/sagas/instance_update/mod.rs +++ b/nexus/src/app/sagas/instance_update/mod.rs @@ -2290,6 +2290,7 @@ mod test { .nexus .datastore() .vmm_and_migration_update_runtime( + &self.opctx, vmm_id, &new_runtime, migrations, @@ -2346,6 +2347,7 @@ mod test { .nexus .datastore() .vmm_and_migration_update_runtime( + &self.opctx, vmm_id, &new_runtime, migrations,