diff --git a/nexus/db-model/src/migration.rs b/nexus/db-model/src/migration.rs index 79eb20708c..2abb6e3ee0 100644 --- a/nexus/db-model/src/migration.rs +++ b/nexus/db-model/src/migration.rs @@ -30,8 +30,6 @@ pub struct Migration { /// The time at which this migration record was deleted, pub time_deleted: Option>, - /// The time at which the source VMM state was last updated. - /// The state of the migration source VMM. pub source_state: MigrationState, diff --git a/nexus/db-queries/src/db/datastore/instance.rs b/nexus/db-queries/src/db/datastore/instance.rs index 07d09ebb5c..e090210cbe 100644 --- a/nexus/db-queries/src/db/datastore/instance.rs +++ b/nexus/db-queries/src/db/datastore/instance.rs @@ -142,6 +142,25 @@ 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, + /// Indicates whether a migration record for this instance was updated, if a + /// [`MigrationRuntimeState`] was provided to + /// [`DataStore::instance_and_vmm_update_runtime`]. + /// + /// - `Some(true)` if a migration record was updated + /// - `Some(false)` if a [`MigrationRuntimeState`] was provided, but the + /// migration record was not updated + /// - `None` if no [`MigrationRuntimeState`] was provided + pub migration_updated: Option, +} + impl DataStore { /// Idempotently insert a database record for an Instance /// @@ -373,12 +392,11 @@ impl DataStore { /// /// # Return value /// - /// - `Ok((instance_updated, vmm_updated))` if the query was issued - /// successfully. `instance_updated` and `vmm_updated` are each true if - /// the relevant item was updated and false otherwise. 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. + /// - `Ok(`[`InstanceUpdateResult`]`)` if the query was issued + /// successfully. The returned [`InstanceUpdateResult`] 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 instance_and_vmm_update_runtime( &self, @@ -387,7 +405,7 @@ impl DataStore { vmm_id: &Uuid, new_vmm: &VmmRuntimeState, migration: &Option, - ) -> Result<(bool, bool, Option), Error> { + ) -> Result { let query = crate::db::queries::instance::InstanceAndVmmUpdate::new( *instance_id, new_instance.clone(), @@ -427,7 +445,11 @@ impl DataStore { None }; - Ok((instance_updated, vmm_updated, migration_updated)) + Ok(InstanceUpdateResult { + instance_updated, + vmm_updated, + migration_updated, + }) } /// Lists all instances on in-service sleds with active Propolis VMM diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index ef90925421..98b4ea91e2 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -25,6 +25,7 @@ use nexus_db_queries::authn; use nexus_db_queries::authz; use nexus_db_queries::context::OpContext; use nexus_db_queries::db; +use nexus_db_queries::db::datastore::instance::InstanceUpdateResult; use nexus_db_queries::db::datastore::InstanceAndActiveVmm; use nexus_db_queries::db::identity::Resource; use nexus_db_queries::db::lookup; @@ -563,28 +564,27 @@ impl super::Nexus { // outright fails, this operation fails. If the operation nominally // succeeds but nothing was updated, this action is outdated and the // caller should not proceed with migration. - let (updated, _, _) = match instance_put_result { - Ok(state) => { - self.write_returned_instance_state(&instance_id, state).await? - - // TODO(eliza): this is where we would also want to insert to the - // migration table... - } - Err(e) => { - if e.instance_unhealthy() { - let _ = self - .mark_instance_failed( - &instance_id, - &prev_instance_runtime, - &e, - ) - .await; + let InstanceUpdateResult { instance_updated, .. } = + match instance_put_result { + Ok(state) => { + self.write_returned_instance_state(&instance_id, state) + .await? } - return Err(e.into()); - } - }; + Err(e) => { + if e.instance_unhealthy() { + let _ = self + .mark_instance_failed( + &instance_id, + &prev_instance_runtime, + &e, + ) + .await; + } + return Err(e.into()); + } + }; - if updated { + if instance_updated { Ok(self .db_datastore .instance_refetch(opctx, &authz_instance) @@ -1324,7 +1324,7 @@ impl super::Nexus { &self, instance_id: &Uuid, state: Option, - ) -> Result<(bool, bool, Option), Error> { + ) -> Result { slog::debug!(&self.log, "writing instance state returned from sled agent"; "instance_id" => %instance_id, @@ -1350,7 +1350,13 @@ impl super::Nexus { update_result } else { - Ok((false, false, None)) + // There was no instance state to write back, so --- perhaps + // obviously --- nothing happened. + Ok(InstanceUpdateResult { + instance_updated: false, + vmm_updated: false, + migration_updated: None, + }) } } @@ -1958,16 +1964,6 @@ impl super::Nexus { } } -/// Records what aspects of an instance's state were actually changed in a -/// [`notify_instance_updated`] call. -/// -/// This is (presently) used for debugging purposes only. -#[derive(Copy, Clone)] -pub(crate) struct InstanceUpdated { - pub instance_updated: bool, - pub vmm_updated: bool, -} - /// Invoked by a sled agent to publish an updated runtime state for an /// Instance. #[allow(clippy::too_many_arguments)] // :( @@ -1980,7 +1976,7 @@ pub(crate) async fn notify_instance_updated( instance_id: &Uuid, new_runtime_state: &nexus::SledInstanceState, v2p_notification_tx: tokio::sync::watch::Sender<()>, -) -> Result, Error> { +) -> Result, Error> { let propolis_id = new_runtime_state.propolis_id; info!(log, "received new runtime state from sled agent"; @@ -2118,14 +2114,14 @@ pub(crate) async fn notify_instance_updated( } match result { - Ok((instance_updated, vmm_updated, migration_updated)) => { + Ok(result) => { info!(log, "instance and vmm updated by sled agent"; "instance_id" => %instance_id, "propolis_id" => %propolis_id, - "instance_updated" => instance_updated, - "vmm_updated" => vmm_updated, - "migration_updated" => ?migration_updated); - Ok(Some(InstanceUpdated { instance_updated, vmm_updated })) + "instance_updated" => result.instance_updated, + "vmm_updated" => result.vmm_updated, + "migration_updated" => ?result.migration_updated); + Ok(Some(result)) } // The update command should swallow object-not-found errors and