diff --git a/nexus/db-model/src/migration.rs b/nexus/db-model/src/migration.rs index 2abb6e3ee0..5739122a46 100644 --- a/nexus/db-model/src/migration.rs +++ b/nexus/db-model/src/migration.rs @@ -14,7 +14,15 @@ use uuid::Uuid; /// The state of a migration as understood by Nexus. #[derive( - Clone, Debug, Queryable, Insertable, Selectable, Serialize, Deserialize, + Clone, + Debug, + Queryable, + Insertable, + Selectable, + Serialize, + Deserialize, + Eq, + PartialEq, )] #[diesel(table_name = migration)] pub struct Migration { diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index 4c07f6065b..6f0f992df1 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -1776,6 +1776,7 @@ table! { } allow_tables_to_appear_in_same_query!(instance, migration); +allow_tables_to_appear_in_same_query!(migration, vmm); joinable!(instance -> migration (migration_id)); allow_tables_to_appear_in_same_query!( diff --git a/nexus/db-model/src/vmm.rs b/nexus/db-model/src/vmm.rs index cfa1d43759..ceaef2e709 100644 --- a/nexus/db-model/src/vmm.rs +++ b/nexus/db-model/src/vmm.rs @@ -21,7 +21,14 @@ use uuid::Uuid; /// An individual VMM process that incarnates a specific instance. #[derive( - Clone, Queryable, Debug, Selectable, Serialize, Deserialize, Insertable, + Clone, + Queryable, + Debug, + Selectable, + Serialize, + Deserialize, + Insertable, + PartialEq, )] #[diesel(table_name = vmm)] pub struct Vmm { @@ -101,6 +108,7 @@ impl Vmm { Queryable, Serialize, Deserialize, + PartialEq, )] #[diesel(table_name = vmm)] pub struct VmmRuntimeState { diff --git a/nexus/db-queries/src/db/datastore/instance.rs b/nexus/db-queries/src/db/datastore/instance.rs index e090210cbe..50e70b47df 100644 --- a/nexus/db-queries/src/db/datastore/instance.rs +++ b/nexus/db-queries/src/db/datastore/instance.rs @@ -21,6 +21,7 @@ use crate::db::lookup::LookupPath; use crate::db::model::Generation; use crate::db::model::Instance; use crate::db::model::InstanceRuntimeState; +use crate::db::model::Migration; use crate::db::model::Name; use crate::db::model::Project; use crate::db::model::Sled; @@ -119,6 +120,26 @@ impl From for omicron_common::api::external::Instance { } } +/// A complete snapshot of the database records describing the current state of +/// an instance: the [`Instance`] record itself, along with its active [`Vmm`], +/// target [`Vmm`], and current [`Migration`], if they exist. +/// +/// This is returned by [`DataStore::instance_fetch_all`]. +#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] +pub struct InstanceSnapshot { + /// The instance record. + pub instance: Instance, + /// The [`Vmm`] record pointed to by the instance's `active_propolis_id`, if + /// it is set. + pub active_vmm: Option, + /// The [`Vmm`] record pointed to by the instance's `target_propolis_id`, if + /// it is set. + pub target_vmm: Option, + /// The [`Migration`] record pointed to by the instance's `migration_id`, if + /// it is set. + pub migration: Option, +} + /// A token which represents that a saga holds the instance-updater lock on a /// particular instance. /// @@ -330,6 +351,123 @@ impl DataStore { Ok(InstanceAndActiveVmm { instance, vmm }) } + /// Fetches all database records describing the state of the provided + /// instance in a single atomic query. + /// + /// If an instance with the provided UUID exists, this method returns an + /// [`InstanceSnapshot`], which contains the following: + /// + /// - The [`Instance`] record itself, + /// - The instance's active [`Vmm`] record, if the `active_propolis_id` + /// column is not null, + /// - The instance's target [`Vmm`] record, if the `target_propolis_id` + /// column is not null, + /// - The instance's current active [`Migration`], if the `migration_id` + /// column is not null. + pub async fn instance_fetch_all( + &self, + opctx: &OpContext, + authz_instance: &authz::Instance, + ) -> LookupResult { + opctx.authorize(authz::Action::Read, authz_instance).await?; + + use db::schema::instance::dsl as instance_dsl; + use db::schema::migration::dsl as migration_dsl; + use db::schema::vmm::dsl as vmm_dsl; + + let mut results = instance_dsl::instance + .filter(instance_dsl::id.eq(authz_instance.id())) + .filter(instance_dsl::time_deleted.is_null()) + .left_join( + vmm_dsl::vmm.on((vmm_dsl::id + .nullable() + .eq(instance_dsl::active_propolis_id) + .or(vmm_dsl::id + .nullable() + .eq(instance_dsl::target_propolis_id))) + .and(vmm_dsl::time_deleted.is_null())), + ) + .left_join( + migration_dsl::migration.on(migration_dsl::id + .nullable() + .eq(instance_dsl::migration_id) + .and(migration_dsl::time_deleted.is_null())), + ) + .select(( + Instance::as_select(), + Option::::as_select(), + Option::::as_select(), + )) + .load_async::<(Instance, Option, Option)>( + &*self.pool_connection_authorized(opctx).await?, + ) + .await + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Instance, + LookupType::ById(authz_instance.id()), + ), + ) + })? + .into_iter(); + + // Because we join with the `vmm` table on VMMs whose ID is the active + // *or* target VMM ID in the instance record, this query may return a + // result set with multiple rows. If it does, we have to iterate over + // them and determine which VMM result is the active VMM and which is the + // target. The instance and migration records should be identical, so we + // only need the first ones. + // + // Yes, this code is a bit unfortunate, but Diesel doesn't like the idea + // of joining with the same table twice on different columns...so, this, + // at least, works. + let (instance, vmm, migration) = results.next().ok_or_else(|| { + LookupType::ById(authz_instance.id()) + .into_not_found(ResourceType::Instance) + })?; + let mut snapshot = InstanceSnapshot { + instance, + migration, + active_vmm: None, + target_vmm: None, + }; + + impl InstanceSnapshot { + fn add_vmm(&mut self, vmm: Vmm) { + match vmm.id { + id if self.instance.runtime_state.propolis_id == Some(id) => { + self.active_vmm = Some(vmm) + } + id if self.instance.runtime_state.dst_propolis_id == Some(id) => { + self.target_vmm = Some(vmm) + } + _ => debug_assert!( + false, + "tried to add a VMM to an instance snapshot that was neither \ + the active nor target VMM...was this VMM actually returned by \ + the instance snapshot query?", + ), + } + } + } + if let Some(vmm) = vmm { + snapshot.add_vmm(vmm); + } + if let Some((_, Some(vmm), _)) = results.next() { + snapshot.add_vmm(vmm); + } + + debug_assert!( + results.next().is_none(), + "instance snapshot query should not return more than two rows, as \ + as an instance has 0-1 active and 0-1 target VMMs" + ); + + Ok(snapshot) + } + // TODO-design It's tempting to return the updated state of the Instance // here because it's convenient for consumers and by using a RETURNING // clause, we could ensure that the "update" and "fetch" are atomic. @@ -821,14 +959,16 @@ mod tests { use super::*; use crate::db::datastore::test_utils::datastore_test; use crate::db::lookup::LookupPath; + use nexus_db_model::InstanceState; use nexus_db_model::Project; + use nexus_db_model::VmmState; use nexus_test_utils::db::test_setup_database; use nexus_types::external_api::params; use omicron_common::api::external::ByteCount; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_test_utils::dev; - async fn test_setup( + async fn create_test_instance( datastore: &DataStore, opctx: &OpContext, ) -> authz::Instance { @@ -852,7 +992,6 @@ mod tests { ) .await .expect("project must be created successfully"); - let _ = datastore .project_create_instance( &opctx, @@ -897,7 +1036,7 @@ mod tests { let (opctx, datastore) = datastore_test(&logctx, &db).await; let saga1 = Uuid::new_v4(); let saga2 = Uuid::new_v4(); - let authz_instance = test_setup(&datastore, &opctx).await; + let authz_instance = create_test_instance(&datastore, &opctx).await; macro_rules! assert_locked { ($id:expr) => {{ @@ -971,7 +1110,7 @@ mod tests { dev::test_setup_log("test_instance_updater_lock_is_idempotent"); let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; - let authz_instance = test_setup(&datastore, &opctx).await; + let authz_instance = create_test_instance(&datastore, &opctx).await; let saga1 = Uuid::new_v4(); // attempt to lock the instance once. @@ -1029,7 +1168,7 @@ mod tests { ); let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; - let authz_instance = test_setup(&datastore, &opctx).await; + let authz_instance = create_test_instance(&datastore, &opctx).await; let saga1 = Uuid::new_v4(); let saga2 = Uuid::new_v4(); @@ -1109,4 +1248,170 @@ mod tests { db.cleanup().await.unwrap(); logctx.cleanup_successful(); } + + #[tokio::test] + async fn test_instance_fetch_all() { + // Setup + let logctx = dev::test_setup_log("test_instance_fetch_all"); + let mut db = test_setup_database(&logctx.log).await; + let (opctx, datastore) = datastore_test(&logctx, &db).await; + let authz_instance = create_test_instance(&datastore, &opctx).await; + let snapshot = + dbg!(datastore.instance_fetch_all(&opctx, &authz_instance).await) + .expect("instance fetch must succeed"); + + assert_eq!( + dbg!(snapshot.instance.id()), + dbg!(authz_instance.id()), + "must have fetched the correct instance" + ); + assert_eq!( + dbg!(snapshot.active_vmm), + None, + "instance does not have an active VMM" + ); + assert_eq!( + dbg!(snapshot.target_vmm), + None, + "instance does not have a target VMM" + ); + assert_eq!( + dbg!(snapshot.migration), + None, + "instance does not have a migration" + ); + + let active_vmm = datastore + .vmm_insert( + &opctx, + Vmm { + id: Uuid::new_v4(), + time_created: Utc::now(), + time_deleted: None, + instance_id: authz_instance.id(), + sled_id: Uuid::new_v4(), + propolis_ip: "10.1.9.32".parse().unwrap(), + propolis_port: 420.into(), + runtime: VmmRuntimeState { + time_state_updated: Utc::now(), + gen: Generation::new(), + state: VmmState::Running, + }, + }, + ) + .await + .expect("active VMM should be inserted successfully!"); + datastore + .instance_update_runtime( + &authz_instance.id(), + &InstanceRuntimeState { + time_updated: Utc::now(), + gen: Generation( + snapshot.instance.runtime_state.gen.0.next(), + ), + nexus_state: InstanceState::Vmm, + propolis_id: Some(active_vmm.id), + ..snapshot.instance.runtime_state.clone() + }, + ) + .await + .expect("instance update should work"); + let snapshot = + dbg!(datastore.instance_fetch_all(&opctx, &authz_instance).await) + .expect("instance fetch must succeed"); + + assert_eq!( + dbg!(snapshot.instance.id()), + dbg!(authz_instance.id()), + "must have fetched the correct instance" + ); + assert_eq!( + dbg!(snapshot.active_vmm.map(|vmm| vmm.id)), + Some(dbg!(active_vmm.id)), + "fetched active VMM must be the instance's active VMM" + ); + assert_eq!( + dbg!(snapshot.target_vmm), + None, + "instance does not have a target VMM" + ); + assert_eq!( + dbg!(snapshot.migration), + None, + "instance does not have a migration" + ); + + let target_vmm = datastore + .vmm_insert( + &opctx, + Vmm { + id: Uuid::new_v4(), + time_created: Utc::now(), + time_deleted: None, + instance_id: authz_instance.id(), + sled_id: Uuid::new_v4(), + propolis_ip: "10.1.9.42".parse().unwrap(), + propolis_port: 666.into(), + runtime: VmmRuntimeState { + time_state_updated: Utc::now(), + gen: Generation::new(), + state: VmmState::Running, + }, + }, + ) + .await + .expect("target VMM should be inserted successfully!"); + let migration = datastore + .migration_insert( + &opctx, + Migration::new(Uuid::new_v4(), active_vmm.id, target_vmm.id), + ) + .await + .expect("migration should be inserted successfully!"); + datastore + .instance_update_runtime( + &authz_instance.id(), + &InstanceRuntimeState { + time_updated: Utc::now(), + gen: Generation( + snapshot.instance.runtime_state.gen.0.next(), + ), + nexus_state: InstanceState::Vmm, + propolis_id: Some(active_vmm.id), + dst_propolis_id: Some(target_vmm.id), + migration_id: Some(migration.id), + ..snapshot.instance.runtime_state.clone() + }, + ) + .await + .expect("instance update should work"); + let snapshot = + dbg!(datastore.instance_fetch_all(&opctx, &authz_instance).await) + .expect("instance fetch must succeed"); + + assert_eq!( + dbg!(snapshot.instance.id()), + dbg!(authz_instance.id()), + "must have fetched the correct instance" + ); + assert_eq!( + dbg!(snapshot.active_vmm.map(|vmm| vmm.id)), + Some(dbg!(active_vmm.id)), + "fetched active VMM must be the instance's active VMM" + ); + assert_eq!( + dbg!(snapshot.target_vmm.map(|vmm| vmm.id)), + Some(dbg!(target_vmm.id)), + "fetched target VMM must be the instance's target VMM" + ); + assert_eq!( + dbg!(snapshot.migration.map(|m| m.id)), + Some(dbg!(migration.id)), + "fetched migration must be the instance's migration" + ); + + // Clean up. + db.cleanup().await.unwrap(); + logctx.cleanup_successful(); + } }