Skip to content

Commit

Permalink
CTE only does VMM and migration updates
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hawkw committed Jul 24, 2024
1 parent 49b9bc6 commit ca5f10a
Show file tree
Hide file tree
Showing 14 changed files with 98 additions and 584 deletions.
15 changes: 1 addition & 14 deletions nexus/db-queries/src/db/datastore/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
///
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions nexus/db-queries/src/db/datastore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
52 changes: 45 additions & 7 deletions nexus/db-queries/src/db/datastore/vmm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -143,29 +158,52 @@ 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<InstanceUpdateResult, Error> {
let query = crate::db::queries::instance::InstanceAndVmmUpdate::new(
) -> Result<VmmStateUpdateResult, Error> {
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
.execute_and_check(&*self.pool_connection_unauthorized().await?)
.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,
Expand Down
2 changes: 1 addition & 1 deletion nexus/db-queries/src/db/queries/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit ca5f10a

Please sign in to comment.