From ca5f10ada6a50695e98c27c337fbbea149b42b78 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 24 Jul 2024 11:02:36 -0700 Subject: [PATCH] CTE only does VMM and migration updates We no longer use the "InstanceAndVmmUpdate" CTE to update the instance record, just the VMM record and maybe migration record(s). I've removed the instance update from the CTE and updated its naming and docs to reflect this. --- nexus/db-queries/src/db/datastore/instance.rs | 15 +- nexus/db-queries/src/db/datastore/mod.rs | 1 + nexus/db-queries/src/db/datastore/vmm.rs | 52 +++- nexus/db-queries/src/db/queries/mod.rs | 2 +- .../src/db/queries/{instance.rs => vmm.rs} | 250 ++++-------------- ...stance_and_vmm_update_vmm_and_instance.sql | 55 ---- ...pdate_vmm_instance_and_both_migrations.sql | 119 --------- ...m_update_vmm_instance_and_migration_in.sql | 87 ------ ..._update_vmm_instance_and_migration_out.sql | 87 ------ ...ration_update_vmm_and_both_migrations.sql} | 2 - ...migration_update_vmm_and_migration_in.sql} | 2 - ...igration_update_vmm_and_migration_out.sql} | 2 - ... => vmm_and_migration_update_vmm_only.sql} | 2 +- nexus/src/app/sagas/instance_update/mod.rs | 6 +- 14 files changed, 98 insertions(+), 584 deletions(-) rename nexus/db-queries/src/db/queries/{instance.rs => vmm.rs} (68%) delete mode 100644 nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_instance.sql delete mode 100644 nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_both_migrations.sql delete mode 100644 nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_migration_in.sql delete mode 100644 nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_migration_out.sql rename nexus/db-queries/tests/output/{instance_and_vmm_update_vmm_and_both_migrations.sql => vmm_and_migration_update_vmm_and_both_migrations.sql} (99%) rename nexus/db-queries/tests/output/{instance_and_vmm_update_vmm_and_migration_in.sql => vmm_and_migration_update_vmm_and_migration_in.sql} (98%) rename nexus/db-queries/tests/output/{instance_and_vmm_update_vmm_and_migration_out.sql => vmm_and_migration_update_vmm_and_migration_out.sql} (98%) rename nexus/db-queries/tests/output/{instance_and_vmm_update_vmm_only.sql => vmm_and_migration_update_vmm_only.sql} (87%) diff --git a/nexus/db-queries/src/db/datastore/instance.rs b/nexus/db-queries/src/db/datastore/instance.rs index 39fec43280..e61c2fdca8 100644 --- a/nexus/db-queries/src/db/datastore/instance.rs +++ b/nexus/db-queries/src/db/datastore/instance.rs @@ -36,7 +36,6 @@ use chrono::Utc; use diesel::prelude::*; use nexus_db_model::ApplySledFilterExt; use nexus_db_model::Disk; -use nexus_db_model::VmmRuntimeState; use nexus_types::deployment::SledFilter; use omicron_common::api; use omicron_common::api::external; @@ -49,7 +48,6 @@ 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::internal::nexus::Migrations; use omicron_common::bail_unless; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::InstanceUuid; @@ -243,18 +241,6 @@ pub enum UpdaterLockError { Query(#[from] Error), } -/// The result of an [`DataStore::instance_and_vmm_update_runtime`] call, -/// indicating which records were updated. -#[derive(Copy, Clone, Debug)] -pub struct InstanceUpdateResult { - /// `true` if the instance record was updated, `false` otherwise. - pub instance_updated: bool, - /// `true` if the VMM record was updated, `false` otherwise. - pub vmm_updated: bool, - pub migration_in_updated: bool, - pub migration_out_updated: bool, -} - impl DataStore { /// Idempotently insert a database record for an Instance /// @@ -1201,6 +1187,7 @@ mod tests { use crate::db::lookup::LookupPath; use nexus_db_model::InstanceState; use nexus_db_model::Project; + use nexus_db_model::VmmRuntimeState; use nexus_db_model::VmmState; use nexus_test_utils::db::test_setup_database; use nexus_types::external_api::params; diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index 92cfa24d13..4e4b9e047d 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -121,6 +121,7 @@ pub use sled::SledTransition; pub use sled::TransitionError; pub use switch_port::SwitchPortSettingsCombinedResult; pub use virtual_provisioning_collection::StorageType; +pub use vmm::VmmStateUpdateResult; pub use volume::read_only_resources_associated_with_volume; pub use volume::CrucibleResources; pub use volume::CrucibleTargets; diff --git a/nexus/db-queries/src/db/datastore/vmm.rs b/nexus/db-queries/src/db/datastore/vmm.rs index 308cfc67db..eb788fdc89 100644 --- a/nexus/db-queries/src/db/datastore/vmm.rs +++ b/nexus/db-queries/src/db/datastore/vmm.rs @@ -7,7 +7,6 @@ use super::DataStore; use crate::authz; use crate::context::OpContext; -use crate::db::datastore::instance::InstanceUpdateResult; use crate::db::error::public_error_from_diesel; use crate::db::error::ErrorHandler; use crate::db::model::Vmm; @@ -34,6 +33,22 @@ use omicron_uuid_kinds::PropolisUuid; use std::net::SocketAddr; use uuid::Uuid; +/// The result of an [`DataStore::vmm_and_migration_update_runtime`] call, +/// indicating which records were updated. +#[derive(Copy, Clone, Debug)] +pub struct VmmStateUpdateResult { + /// `true` if the VMM record was updated, `false` otherwise. + pub vmm_updated: bool, + + /// `true` if a migration record was updated for the migration in, false if + /// no update was performed or no migration in was provided. + pub migration_in_updated: bool, + + /// `true` if a migration record was updated for the migration out, false if + /// no update was performed or no migration out was provided. + pub migration_out_updated: bool, +} + impl DataStore { pub async fn vmm_insert( &self, @@ -143,20 +158,44 @@ impl DataStore { Ok(updated) } + /// Updates a VMM record and associated migration record(s) with a single + /// database command. + /// + /// This is intended to be used to apply updates from sled agent that + /// may change a VMM's runtime state (e.g. moving an instance from Running + /// to Stopped) and the state of its current active mgiration in a single + /// transaction. The caller is responsible for ensuring the VMM and + /// migration states are consistent with each other before calling this + /// routine. + /// + /// # Arguments + /// + /// - `vmm_id`: The ID of the VMM to update. + /// - `new_runtime`: The new VMM runtime state to try to write. + /// - `migrations`: The (optional) migration-in and migration-out states to + /// try to write. + /// + /// # Return value + /// + /// - `Ok(`[`VmmStateUpdateResult`]`)` if the query was issued + /// successfully. The returned [`VmmStateUpdateResult`] indicates which + /// database record(s) were updated. Note that an update can fail because + /// it was inapplicable (i.e. the database has state with a newer + /// generation already) or because the relevant record was not found. + /// - `Err` if another error occurred while accessing the database. pub async fn vmm_and_migration_update_runtime( &self, vmm_id: PropolisUuid, new_runtime: &VmmRuntimeState, migrations: Migrations<'_>, - ) -> Result { - let query = crate::db::queries::instance::InstanceAndVmmUpdate::new( + ) -> Result { + let query = crate::db::queries::vmm::VmmAndMigrationUpdate::new( vmm_id, new_runtime.clone(), - None, migrations, ); - // The InstanceAndVmmUpdate query handles and indicates failure to find + // 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 @@ -164,8 +203,7 @@ impl DataStore { .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; - Ok(InstanceUpdateResult { - instance_updated: false, + Ok(VmmStateUpdateResult { vmm_updated: match result.vmm_status { Some(UpdateStatus::Updated) => true, Some(UpdateStatus::NotUpdatedButExists) => false, diff --git a/nexus/db-queries/src/db/queries/mod.rs b/nexus/db-queries/src/db/queries/mod.rs index a1022f9187..46e8a7bc16 100644 --- a/nexus/db-queries/src/db/queries/mod.rs +++ b/nexus/db-queries/src/db/queries/mod.rs @@ -7,8 +7,8 @@ pub mod disk; pub mod external_ip; -pub mod instance; 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/instance.rs b/nexus/db-queries/src/db/queries/vmm.rs similarity index 68% rename from nexus/db-queries/src/db/queries/instance.rs rename to nexus/db-queries/src/db/queries/vmm.rs index c73f7ac680..e8eec47141 100644 --- a/nexus/db-queries/src/db/queries/instance.rs +++ b/nexus/db-queries/src/db/queries/vmm.rs @@ -12,25 +12,22 @@ 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::{ - instance::dsl as instance_dsl, migration::dsl as migration_dsl, - vmm::dsl as vmm_dsl, - }, - Generation, InstanceRuntimeState, MigrationState, VmmRuntimeState, + 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, InstanceUuid, PropolisUuid}; +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 instance and VMM tables in a single +/// 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 ) +// WITH found AS (SELECT FROM T WHERE ) // updated AS (UPDATE T SET RETURNING *) // SELECT // found. @@ -44,48 +41,50 @@ use crate::db::update_and_check::UpdateStatus; // found. = updated.; // // The idea behind this query is to have separate "found" and "updated" -// subqueries for the instance and VMM tables, then use those to create two more +// 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 *), -// instance_found AS (SELECT( -// SELECT id FROM instance WHERE instance.id = id +// migration_in_found AS (SELECT( +// SELECT id FROM migration WHERE migration.id = migration_in_id // ) AS id), -// instance_updated AS (UPDATE instance SET ... RETURNING *), +// 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 // ), -// instance_result AS ( -// SELECT instance_found.id AS found, instance_updated.id AS updated -// FROM instance_found -// LEFT JOIN instance_updated -// ON instance_found.id = instance_updated.id -// ) -// SELECT vmm_result.found, vmm_result.updated, instance_result.found, -// instance_result.updated -// FROM vmm_result, instance_result; -/// -/// If a [`MigrationRuntimeState`] is provided, similar "found" and "update" -/// clauses are also added to join the `migration` record for the instance's -/// active migration, if one exists, and update the migration record. If no -/// migration record is provided, this part of the query is skipped, and the -/// `migration_found` and `migration_updated` portions are always `false`. +// 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 instances and VMMs are used to get a NULL +// 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 InstanceAndVmmUpdate { +pub struct VmmAndMigrationUpdate { vmm_find: Box + Send>, vmm_update: Box + Send>, - instance: Option, migration_in: Option, migration_out: Option, } @@ -99,12 +98,7 @@ struct Update { /// Contains the result of a combined instance-and-VMM update operation. #[derive(Copy, Clone, PartialEq, Debug)] -pub struct InstanceAndVmmUpdateResult { - /// `Some(status)` if the target instance was found; the wrapped - /// `UpdateStatus` indicates whether the row was updated. `None` if the - /// instance was not found. - pub instance_status: RecordUpdateStatus, - +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, @@ -171,11 +165,10 @@ where } } -impl InstanceAndVmmUpdate { +impl VmmAndMigrationUpdate { pub fn new( vmm_id: PropolisUuid, new_vmm_runtime_state: VmmRuntimeState, - instance: Option<(InstanceUuid, InstanceRuntimeState)>, Migrations { migration_in, migration_out }: Migrations<'_>, ) -> Self { let vmm_find = Box::new( @@ -192,32 +185,6 @@ impl InstanceAndVmmUpdate { .set(new_vmm_runtime_state), ); - let instance = instance.map(|(instance_id, new_runtime_state)| { - let instance_id = instance_id.into_untyped_uuid(); - let find = Box::new( - instance_dsl::instance - .filter(instance_dsl::id.eq(instance_id)) - .select(instance_dsl::id), - ); - - let update = Box::new( - diesel::update(instance_dsl::instance) - .filter(instance_dsl::time_deleted.is_null()) - .filter(instance_dsl::id.eq(instance_id)) - .filter( - instance_dsl::state_generation - .lt(new_runtime_state.gen), - ) - .set(new_runtime_state), - ); - Update { - find, - update, - name: "instance", - id: instance_dsl::id::NAME, - } - }); - fn migration_find( migration_id: Uuid, ) -> Box + Send> { @@ -293,21 +260,18 @@ impl InstanceAndVmmUpdate { }, ); - Self { vmm_find, vmm_update, instance, migration_in, migration_out } + Self { vmm_find, vmm_update, migration_in, migration_out } } pub async fn execute_and_check( self, conn: &(impl async_bb8_diesel::AsyncConnection + Sync), - ) -> Result { + ) -> Result { let has_migration_in = self.migration_in.is_some(); let has_migration_out = self.migration_out.is_some(); - let has_instance = self.instance.is_some(); let ( vmm_found, vmm_updated, - instance_found, - instance_updated, migration_in_found, migration_in_updated, migration_out_found, @@ -320,22 +284,12 @@ impl InstanceAndVmmUpdate { Option, Option, Option, - Option, - Option, // WHEW! )>(conn) .await?; let vmm_status = compute_update_status(vmm_found, vmm_updated); - let instance_status = if has_instance { - compute_update_status(instance_found, instance_updated) - .map(RecordUpdateStatus::Found) - .unwrap_or(RecordUpdateStatus::NotFound) - } else { - RecordUpdateStatus::NotProvided - }; - let migration_in_status = if has_migration_in { compute_update_status(migration_in_found, migration_in_updated) .map(RecordUpdateStatus::Found) @@ -352,8 +306,7 @@ impl InstanceAndVmmUpdate { RecordUpdateStatus::NotProvided }; - Ok(InstanceAndVmmUpdateResult { - instance_status, + Ok(VmmAndMigrationUpdateResult { vmm_status, migration_in_status, migration_out_status, @@ -361,12 +314,12 @@ impl InstanceAndVmmUpdate { } } -impl QueryId for InstanceAndVmmUpdate { +impl QueryId for VmmAndMigrationUpdate { type QueryId = (); const HAS_STATIC_QUERY_ID: bool = false; } -impl Query for InstanceAndVmmUpdate { +impl Query for VmmAndMigrationUpdate { type SqlType = ( Nullable, Nullable, @@ -374,12 +327,10 @@ impl Query for InstanceAndVmmUpdate { Nullable, Nullable, Nullable, - Nullable, - Nullable, ); } -impl RunQueryDsl for InstanceAndVmmUpdate {} +impl RunQueryDsl for VmmAndMigrationUpdate {} impl Update { fn push_subqueries<'b>( @@ -422,13 +373,9 @@ impl Update { } } -impl QueryFragment for InstanceAndVmmUpdate { +impl QueryFragment for VmmAndMigrationUpdate { fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, Pg>) -> QueryResult<()> { out.push_sql("WITH "); - if let Some(ref instance) = self.instance { - instance.push_subqueries(&mut out)?; - out.push_sql(", "); - } if let Some(ref m) = self.migration_in { m.push_subqueries(&mut out)?; @@ -474,17 +421,12 @@ impl QueryFragment for InstanceAndVmmUpdate { } out.push_sql("SELECT vmm_result.found, vmm_result.updated, "); - push_select_from_result(self.instance.as_ref(), &mut out); - out.push_sql(", "); 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.instance.is_some() { - out.push_sql(", instance_result"); - } if self.migration_in.is_some() { out.push_sql(", migration_in_result"); } @@ -530,52 +472,19 @@ mod test { } } - fn mk_instance_state() -> (InstanceUuid, InstanceRuntimeState) { - let id = InstanceUuid::nil(); - let state = InstanceRuntimeState { - time_updated: Utc::now(), - gen: Generation::new(), - propolis_id: Some(Uuid::nil()), - dst_propolis_id: Some(Uuid::nil()), - migration_id: Some(Uuid::nil()), - nexus_state: nexus_db_model::InstanceState::Vmm, - }; - (id, state) - } - #[tokio::test] async fn expectorate_query_only_vmm() { let vmm_id = PropolisUuid::nil(); let vmm_state = mk_vmm_state(); - let query = InstanceAndVmmUpdate::new( - vmm_id, - vmm_state, - None, - Migrations::default(), - ); - expectorate_query_contents( - &query, - "tests/output/instance_and_vmm_update_vmm_only.sql", - ) - .await; - } - - #[tokio::test] - async fn expectorate_query_vmm_and_instance() { - let vmm_id = PropolisUuid::nil(); - let vmm_state = mk_vmm_state(); - let instance = mk_instance_state(); - - let query = InstanceAndVmmUpdate::new( + let query = VmmAndMigrationUpdate::new( vmm_id, vmm_state, - Some(instance), Migrations::default(), ); expectorate_query_contents( &query, - "tests/output/instance_and_vmm_update_vmm_and_instance.sql", + "tests/output/vmm_and_migration_update_vmm_only.sql", ) .await; } @@ -586,35 +495,14 @@ mod test { let vmm_state = mk_vmm_state(); let migration = mk_migration_state(); - let query = InstanceAndVmmUpdate::new( - vmm_id, - vmm_state, - None, - Migrations { migration_in: Some(&migration), migration_out: None }, - ); - expectorate_query_contents( - &query, - "tests/output/instance_and_vmm_update_vmm_and_migration_in.sql", - ) - .await; - } - - #[tokio::test] - async fn expectorate_query_vmm_instance_and_migration_in() { - let vmm_id = PropolisUuid::nil(); - let vmm_state = mk_vmm_state(); - let instance = mk_instance_state(); - let migration = mk_migration_state(); - - let query = InstanceAndVmmUpdate::new( + let query = VmmAndMigrationUpdate::new( vmm_id, vmm_state, - Some(instance), Migrations { migration_in: Some(&migration), migration_out: None }, ); expectorate_query_contents( &query, - "tests/output/instance_and_vmm_update_vmm_instance_and_migration_in.sql", + "tests/output/vmm_and_migration_update_vmm_and_migration_in.sql", ) .await; } @@ -625,35 +513,14 @@ mod test { let vmm_state = mk_vmm_state(); let migration = mk_migration_state(); - let query = InstanceAndVmmUpdate::new( - vmm_id, - vmm_state, - None, - Migrations { migration_out: Some(&migration), migration_in: None }, - ); - expectorate_query_contents( - &query, - "tests/output/instance_and_vmm_update_vmm_and_migration_out.sql", - ) - .await; - } - - #[tokio::test] - async fn expectorate_query_vmm_instance_and_migration_out() { - let vmm_id = PropolisUuid::nil(); - let vmm_state = mk_vmm_state(); - let instance = mk_instance_state(); - let migration = mk_migration_state(); - - let query = InstanceAndVmmUpdate::new( + let query = VmmAndMigrationUpdate::new( vmm_id, vmm_state, - Some(instance), Migrations { migration_out: Some(&migration), migration_in: None }, ); expectorate_query_contents( &query, - "tests/output/instance_and_vmm_update_vmm_instance_and_migration_out.sql", + "tests/output/vmm_and_migration_update_vmm_and_migration_out.sql", ) .await; } @@ -665,34 +532,9 @@ mod test { let migration_in = mk_migration_state(); let migration_out = mk_migration_state(); - let query = InstanceAndVmmUpdate::new( - vmm_id, - vmm_state, - None, - Migrations { - migration_in: Some(&migration_in), - migration_out: Some(&migration_out), - }, - ); - expectorate_query_contents( - &query, - "tests/output/instance_and_vmm_update_vmm_and_both_migrations.sql", - ) - .await; - } - - #[tokio::test] - async fn expectorate_query_vmm_instance_and_both_migrations() { - let vmm_id = PropolisUuid::nil(); - let vmm_state = mk_vmm_state(); - let instance = mk_instance_state(); - let migration_in = mk_migration_state(); - let migration_out = mk_migration_state(); - - let query = InstanceAndVmmUpdate::new( + let query = VmmAndMigrationUpdate::new( vmm_id, vmm_state, - Some(instance), Migrations { migration_in: Some(&migration_in), migration_out: Some(&migration_out), @@ -700,7 +542,7 @@ mod test { ); expectorate_query_contents( &query, - "tests/output/instance_and_vmm_update_vmm_instance_and_both_migrations.sql", + "tests/output/vmm_and_migration_update_vmm_and_both_migrations.sql", ) .await; } diff --git a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_instance.sql b/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_instance.sql deleted file mode 100644 index 3014e9068f..0000000000 --- a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_instance.sql +++ /dev/null @@ -1,55 +0,0 @@ -WITH - instance_found AS (SELECT (SELECT instance.id FROM instance WHERE instance.id = $1) AS id), - instance_updated - AS ( - UPDATE - instance - SET - time_state_updated = $2, - state_generation = $3, - active_propolis_id = $4, - target_propolis_id = $5, - migration_id = $6, - state = $7 - WHERE - ((instance.time_deleted IS NULL) AND instance.id = $8) AND instance.state_generation < $9 - RETURNING - id - ), - instance_result - AS ( - SELECT - instance_found.id AS found, instance_updated.id AS updated - FROM - instance_found LEFT JOIN instance_updated ON instance_found.id = instance_updated.id - ), - vmm_found AS (SELECT (SELECT vmm.id FROM vmm WHERE vmm.id = $10) AS id), - vmm_updated - AS ( - UPDATE - vmm - SET - time_state_updated = $11, state_generation = $12, state = $13 - WHERE - ((vmm.time_deleted IS NULL) AND vmm.id = $14) AND vmm.state_generation < $15 - RETURNING - id - ), - 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 - ) -SELECT - vmm_result.found, - vmm_result.updated, - instance_result.found, - instance_result.updated, - NULL, - NULL, - NULL, - NULL -FROM - vmm_result, instance_result diff --git a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_both_migrations.sql b/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_both_migrations.sql deleted file mode 100644 index 52c28f85c3..0000000000 --- a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_both_migrations.sql +++ /dev/null @@ -1,119 +0,0 @@ -WITH - instance_found AS (SELECT (SELECT instance.id FROM instance WHERE instance.id = $1) AS id), - instance_updated - AS ( - UPDATE - instance - SET - time_state_updated = $2, - state_generation = $3, - active_propolis_id = $4, - target_propolis_id = $5, - migration_id = $6, - state = $7 - WHERE - ((instance.time_deleted IS NULL) AND instance.id = $8) AND instance.state_generation < $9 - RETURNING - id - ), - instance_result - AS ( - SELECT - instance_found.id AS found, instance_updated.id AS updated - FROM - instance_found LEFT JOIN instance_updated ON instance_found.id = instance_updated.id - ), - migration_in_found - AS ( - SELECT - ( - SELECT - migration.id - FROM - migration - WHERE - migration.id = $10 AND (migration.time_deleted IS NULL) - ) - AS id - ), - migration_in_updated - AS ( - UPDATE - migration - SET - target_state = $11, time_target_updated = $12, target_gen = $13 - WHERE - (migration.id = $14 AND migration.target_propolis_id = $15) AND migration.target_gen < $16 - RETURNING - 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_found - AS ( - SELECT - ( - SELECT - migration.id - FROM - migration - WHERE - migration.id = $17 AND (migration.time_deleted IS NULL) - ) - AS id - ), - migration_out_updated - AS ( - UPDATE - migration - SET - source_state = $18, time_source_updated = $19, source_gen = $20 - WHERE - (migration.id = $21 AND migration.source_propolis_id = $22) AND migration.source_gen < $23 - RETURNING - id - ), - migration_out_result - AS ( - SELECT - migration_out_found.id AS found, migration_out_updated.id AS updated - FROM - migration_out_found - LEFT JOIN migration_out_updated ON migration_out_found.id = migration_out_updated.id - ), - vmm_found AS (SELECT (SELECT vmm.id FROM vmm WHERE vmm.id = $24) AS id), - vmm_updated - AS ( - UPDATE - vmm - SET - time_state_updated = $25, state_generation = $26, state = $27 - WHERE - ((vmm.time_deleted IS NULL) AND vmm.id = $28) AND vmm.state_generation < $29 - RETURNING - id - ), - 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 - ) -SELECT - vmm_result.found, - vmm_result.updated, - instance_result.found, - instance_result.updated, - migration_in_result.found, - migration_in_result.updated, - migration_out_result.found, - migration_out_result.updated -FROM - vmm_result, instance_result, migration_in_result, migration_out_result diff --git a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_migration_in.sql b/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_migration_in.sql deleted file mode 100644 index e717008617..0000000000 --- a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_migration_in.sql +++ /dev/null @@ -1,87 +0,0 @@ -WITH - instance_found AS (SELECT (SELECT instance.id FROM instance WHERE instance.id = $1) AS id), - instance_updated - AS ( - UPDATE - instance - SET - time_state_updated = $2, - state_generation = $3, - active_propolis_id = $4, - target_propolis_id = $5, - migration_id = $6, - state = $7 - WHERE - ((instance.time_deleted IS NULL) AND instance.id = $8) AND instance.state_generation < $9 - RETURNING - id - ), - instance_result - AS ( - SELECT - instance_found.id AS found, instance_updated.id AS updated - FROM - instance_found LEFT JOIN instance_updated ON instance_found.id = instance_updated.id - ), - migration_in_found - AS ( - SELECT - ( - SELECT - migration.id - FROM - migration - WHERE - migration.id = $10 AND (migration.time_deleted IS NULL) - ) - AS id - ), - migration_in_updated - AS ( - UPDATE - migration - SET - target_state = $11, time_target_updated = $12, target_gen = $13 - WHERE - (migration.id = $14 AND migration.target_propolis_id = $15) AND migration.target_gen < $16 - RETURNING - 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 - ), - vmm_found AS (SELECT (SELECT vmm.id FROM vmm WHERE vmm.id = $17) AS id), - vmm_updated - AS ( - UPDATE - vmm - SET - time_state_updated = $18, state_generation = $19, state = $20 - WHERE - ((vmm.time_deleted IS NULL) AND vmm.id = $21) AND vmm.state_generation < $22 - RETURNING - id - ), - 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 - ) -SELECT - vmm_result.found, - vmm_result.updated, - instance_result.found, - instance_result.updated, - migration_in_result.found, - migration_in_result.updated, - NULL, - NULL -FROM - vmm_result, instance_result, migration_in_result diff --git a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_migration_out.sql b/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_migration_out.sql deleted file mode 100644 index c02b73e4f6..0000000000 --- a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_instance_and_migration_out.sql +++ /dev/null @@ -1,87 +0,0 @@ -WITH - instance_found AS (SELECT (SELECT instance.id FROM instance WHERE instance.id = $1) AS id), - instance_updated - AS ( - UPDATE - instance - SET - time_state_updated = $2, - state_generation = $3, - active_propolis_id = $4, - target_propolis_id = $5, - migration_id = $6, - state = $7 - WHERE - ((instance.time_deleted IS NULL) AND instance.id = $8) AND instance.state_generation < $9 - RETURNING - id - ), - instance_result - AS ( - SELECT - instance_found.id AS found, instance_updated.id AS updated - FROM - instance_found LEFT JOIN instance_updated ON instance_found.id = instance_updated.id - ), - migration_out_found - AS ( - SELECT - ( - SELECT - migration.id - FROM - migration - WHERE - migration.id = $10 AND (migration.time_deleted IS NULL) - ) - AS id - ), - migration_out_updated - AS ( - UPDATE - migration - SET - source_state = $11, time_source_updated = $12, source_gen = $13 - WHERE - (migration.id = $14 AND migration.source_propolis_id = $15) AND migration.source_gen < $16 - RETURNING - id - ), - migration_out_result - AS ( - SELECT - migration_out_found.id AS found, migration_out_updated.id AS updated - FROM - migration_out_found - LEFT JOIN migration_out_updated ON migration_out_found.id = migration_out_updated.id - ), - vmm_found AS (SELECT (SELECT vmm.id FROM vmm WHERE vmm.id = $17) AS id), - vmm_updated - AS ( - UPDATE - vmm - SET - time_state_updated = $18, state_generation = $19, state = $20 - WHERE - ((vmm.time_deleted IS NULL) AND vmm.id = $21) AND vmm.state_generation < $22 - RETURNING - id - ), - 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 - ) -SELECT - vmm_result.found, - vmm_result.updated, - instance_result.found, - instance_result.updated, - NULL, - NULL, - migration_out_result.found, - migration_out_result.updated -FROM - vmm_result, instance_result, migration_out_result diff --git a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_both_migrations.sql b/nexus/db-queries/tests/output/vmm_and_migration_update_vmm_and_both_migrations.sql similarity index 99% rename from nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_both_migrations.sql rename to nexus/db-queries/tests/output/vmm_and_migration_update_vmm_and_both_migrations.sql index 354fc9a403..bb460ff713 100644 --- a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_both_migrations.sql +++ b/nexus/db-queries/tests/output/vmm_and_migration_update_vmm_and_both_migrations.sql @@ -85,8 +85,6 @@ WITH SELECT vmm_result.found, vmm_result.updated, - NULL, - NULL, migration_in_result.found, migration_in_result.updated, migration_out_result.found, diff --git a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_migration_in.sql b/nexus/db-queries/tests/output/vmm_and_migration_update_vmm_and_migration_in.sql similarity index 98% rename from nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_migration_in.sql rename to nexus/db-queries/tests/output/vmm_and_migration_update_vmm_and_migration_in.sql index 870cce4c02..3fec792c6f 100644 --- a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_migration_in.sql +++ b/nexus/db-queries/tests/output/vmm_and_migration_update_vmm_and_migration_in.sql @@ -53,8 +53,6 @@ WITH SELECT vmm_result.found, vmm_result.updated, - NULL, - NULL, migration_in_result.found, migration_in_result.updated, NULL, diff --git a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_migration_out.sql b/nexus/db-queries/tests/output/vmm_and_migration_update_vmm_and_migration_out.sql similarity index 98% rename from nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_migration_out.sql rename to nexus/db-queries/tests/output/vmm_and_migration_update_vmm_and_migration_out.sql index 4dea3779f7..7adeff48da 100644 --- a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_and_migration_out.sql +++ b/nexus/db-queries/tests/output/vmm_and_migration_update_vmm_and_migration_out.sql @@ -55,8 +55,6 @@ SELECT vmm_result.updated, NULL, NULL, - NULL, - NULL, migration_out_result.found, migration_out_result.updated FROM diff --git a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_only.sql b/nexus/db-queries/tests/output/vmm_and_migration_update_vmm_only.sql similarity index 87% rename from nexus/db-queries/tests/output/instance_and_vmm_update_vmm_only.sql rename to nexus/db-queries/tests/output/vmm_and_migration_update_vmm_only.sql index 8f81e662a9..cfe56740fe 100644 --- a/nexus/db-queries/tests/output/instance_and_vmm_update_vmm_only.sql +++ b/nexus/db-queries/tests/output/vmm_and_migration_update_vmm_only.sql @@ -19,6 +19,6 @@ WITH vmm_found LEFT JOIN vmm_updated ON vmm_found.id = vmm_updated.id ) SELECT - vmm_result.found, vmm_result.updated, NULL, NULL, NULL, NULL, NULL, NULL + vmm_result.found, vmm_result.updated, NULL, NULL, NULL, NULL FROM vmm_result diff --git a/nexus/src/app/sagas/instance_update/mod.rs b/nexus/src/app/sagas/instance_update/mod.rs index d93e773304..8b44290d95 100644 --- a/nexus/src/app/sagas/instance_update/mod.rs +++ b/nexus/src/app/sagas/instance_update/mod.rs @@ -258,8 +258,8 @@ use super::{ ACTION_GENERATE_ID, }; use crate::app::db::datastore::instance; -use crate::app::db::datastore::instance::InstanceUpdateResult; use crate::app::db::datastore::InstanceSnapshot; +use crate::app::db::datastore::VmmStateUpdateResult; use crate::app::db::lookup::LookupPath; use crate::app::db::model::ByteCount; use crate::app::db::model::Generation; @@ -302,7 +302,7 @@ mod destroyed; /// Returns `true` if an `instance-update` saga should be executed as a result /// of writing the provided [`SledInstanceState`] to the database with the -/// provided [`InstanceUpdateResult`]. +/// provided [`VmmStateUpdateResult`]. /// /// We determine this only after actually updating the database records, /// because we don't know whether a particular VMM or migration state is @@ -322,7 +322,7 @@ pub fn update_saga_needed( log: &slog::Logger, instance_id: InstanceUuid, state: &SledInstanceState, - result: &InstanceUpdateResult, + result: &VmmStateUpdateResult, ) -> bool { // Currently, an instance-update saga is required if (and only if): //