Skip to content

Commit

Permalink
Add instance ID column to migration table
Browse files Browse the repository at this point in the history
  • Loading branch information
hawkw committed Jun 17, 2024
1 parent 064d9ea commit 82345ea
Show file tree
Hide file tree
Showing 9 changed files with 266 additions and 1 deletion.
6 changes: 6 additions & 0 deletions nexus/db-model/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::MigrationState;
use chrono::DateTime;
use chrono::Utc;
use omicron_common::api::internal::nexus;
use omicron_uuid_kinds::{GenericUuid, InstanceUuid};
use serde::Deserialize;
use serde::Serialize;
use uuid::Uuid;
Expand All @@ -32,6 +33,9 @@ pub struct Migration {
/// `instance` table's `migration_id` field.
pub id: Uuid,

/// The instance that was migrated.
pub instance_id: Uuid,

/// The time at which this migration record was created.
pub time_created: DateTime<Utc>,

Expand Down Expand Up @@ -66,11 +70,13 @@ pub struct Migration {
impl Migration {
pub fn new(
migration_id: Uuid,
instance_id: InstanceUuid,
source_propolis_id: Uuid,
target_propolis_id: Uuid,
) -> Self {
Self {
id: migration_id,
instance_id: instance_id.into_untyped_uuid(),
time_created: Utc::now(),
time_deleted: None,
source_state: nexus::MigrationState::Pending.into(),
Expand Down
1 change: 1 addition & 0 deletions nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1769,6 +1769,7 @@ table! {
table! {
migration (id) {
id -> Uuid,
instance_id -> Uuid,
time_created -> Timestamptz,
time_deleted -> Nullable<Timestamptz>,
source_state -> crate::MigrationStateEnum,
Expand Down
7 changes: 6 additions & 1 deletion nexus/db-queries/src/db/datastore/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1343,7 +1343,12 @@ mod tests {
let migration = datastore
.migration_insert(
&opctx,
Migration::new(Uuid::new_v4(), active_vmm.id, target_vmm.id),
Migration::new(
Uuid::new_v4(),
instance_id,
active_vmm.id,
target_vmm.id,
),
)
.await
.expect("migration should be inserted successfully!");
Expand Down
193 changes: 193 additions & 0 deletions nexus/db-queries/src/db/datastore/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,21 @@
//! [`DataStore`] methods on [`Migration`]s.
use super::DataStore;
use crate::authz;
use crate::context::OpContext;
use crate::db::error::public_error_from_diesel;
use crate::db::error::ErrorHandler;
use crate::db::model::{Migration, 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::UpdateStatus;
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::ListResultVec;
use omicron_common::api::external::UpdateResult;
use omicron_common::api::internal::nexus;
use uuid::Uuid;
Expand All @@ -38,6 +42,22 @@ impl DataStore {
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
}

/// List all migrations associated with the provided instance ID.
pub async fn migration_list_by_instance(
&self,
opctx: &OpContext,
authz_instance: &authz::Instance,
pagparams: &DataPageParams<'_, Uuid>,
) -> ListResultVec<Migration> {
paginated(dsl::migration, dsl::id, pagparams)
.filter(dsl::time_deleted.is_null())
.filter(dsl::instance_id.eq(authz_instance.id()))
.select(Migration::as_select())
.load_async(&*self.pool_connection_authorized(opctx).await?)
.await
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
}

/// Marks a migration record as deleted if and only if both sides of the
/// migration are in a terminal state.
pub async fn migration_terminate(
Expand Down Expand Up @@ -90,3 +110,176 @@ impl DataStore {
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::db::datastore::test_utils::datastore_test;
use crate::db::lookup::LookupPath;
use crate::db::model::Instance;
use nexus_db_model::Project;
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;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::InstanceUuid;

async fn create_test_instance(
datastore: &DataStore,
opctx: &OpContext,
) -> authz::Instance {
let silo_id = *nexus_db_fixed_data::silo::DEFAULT_SILO_ID;
let project_id = Uuid::new_v4();
let instance_id = InstanceUuid::new_v4();

let (authz_project, _project) = datastore
.project_create(
&opctx,
Project::new_with_id(
project_id,
silo_id,
params::ProjectCreate {
identity: IdentityMetadataCreateParams {
name: "stuff".parse().unwrap(),
description: "Where I keep my stuff".into(),
},
},
),
)
.await
.expect("project must be created successfully");
let _ = datastore
.project_create_instance(
&opctx,
&authz_project,
Instance::new(
instance_id,
project_id,
&params::InstanceCreate {
identity: IdentityMetadataCreateParams {
name: "myinstance".parse().unwrap(),
description: "It's an instance".into(),
},
ncpus: 2i64.try_into().unwrap(),
memory: ByteCount::from_gibibytes_u32(16),
hostname: "myhostname".try_into().unwrap(),
user_data: Vec::new(),
network_interfaces:
params::InstanceNetworkInterfaceAttachment::None,
external_ips: Vec::new(),
disks: Vec::new(),
ssh_public_keys: None,
start: false,
},
),
)
.await
.expect("instance must be created successfully");

let (.., authz_instance) = LookupPath::new(&opctx, &datastore)
.instance_id(instance_id.into_untyped_uuid())
.lookup_for(authz::Action::Modify)
.await
.expect("instance must exist");
authz_instance
}

#[tokio::test]
async fn test_migration_list_by_instance() {
// 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 instance_id = InstanceUuid::from_untyped_uuid(authz_instance.id());
let migration1 = Migration::new(
Uuid::new_v4(),
instance_id,
Uuid::new_v4(),
Uuid::new_v4(),
);
datastore
.migration_insert(&opctx, migration1.clone())
.await
.expect("must insert migration 1");
let migration2 = Migration::new(
Uuid::new_v4(),
instance_id,
Uuid::new_v4(),
Uuid::new_v4(),
);
datastore
.migration_insert(&opctx, migration2.clone())
.await
.expect("must insert migration 2");

let list = datastore
.migration_list_by_instance(
&opctx,
&authz_instance,
&DataPageParams::max_page(),
)
.await
.expect("must list migrations");
assert_all_migrations_found(&[&migration1, &migration2], &list[..]);

let migration3 = Migration::new(
Uuid::new_v4(),
instance_id,
Uuid::new_v4(),
Uuid::new_v4(),
);
datastore
.migration_insert(&opctx, migration3.clone())
.await
.expect("must insert migration 3");

let list = datastore
.migration_list_by_instance(
&opctx,
&authz_instance,
&DataPageParams::max_page(),
)
.await
.expect("must list migrations");
assert_all_migrations_found(
&[&migration1, &migration2, &migration3],
&list[..],
);

datastore
.migration_mark_deleted(&opctx, migration3.id)
.await
.expect("must delete migration");
let list = datastore
.migration_list_by_instance(
&opctx,
&authz_instance,
&DataPageParams::max_page(),
)
.await
.expect("must list migrations");
assert_all_migrations_found(&[&migration1, &migration2], &list[..]);

// Clean up.
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}

#[track_caller]
fn assert_all_migrations_found(
expected: &[&Migration],
actual: &[Migration],
) {
assert_eq!(expected.len(), actual.len());
for migration in expected {
assert!(
actual.iter().any(|m| m.id == migration.id),
"couldn't find migration {:?} in {actual:#?}",
migration.id,
);
}
}
}
1 change: 1 addition & 0 deletions nexus/src/app/sagas/instance_migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ async fn sim_create_migration_record(
&opctx,
db::model::Migration::new(
migration_id,
InstanceUuid::from_untyped_uuid(params.instance.id()),
source_propolis_id,
target_propolis_id,
),
Expand Down
2 changes: 2 additions & 0 deletions schema/crdb/add-instance-id-to-migrations/up01.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- shake hands with DANGER!
DROP TABLE IF EXISTS omicron.public.migration;
45 changes: 45 additions & 0 deletions schema/crdb/add-instance-id-to-migrations/up02.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
-- A table of the states of current migrations.
CREATE TABLE IF NOT EXISTS omicron.public.migration (
id UUID PRIMARY KEY,

/* The ID of the instance that was migrated */
instance_id UUID NOT NULL,

/* The time this migration record was created. */
time_created TIMESTAMPTZ NOT NULL,

/* The time this migration record was deleted. */
time_deleted TIMESTAMPTZ,

/* The state of the migration source */
source_state omicron.public.migration_state NOT NULL,

/* The ID of the migration source Propolis */
source_propolis_id UUID NOT NULL,

/* Generation number owned and incremented by the source sled-agent */
source_gen INT8 NOT NULL DEFAULT 1,

/* Timestamp of when the source field was last updated.
*
* This is provided by the sled-agent when publishing a migration state
* update.
*/
time_source_updated TIMESTAMPTZ,

/* The state of the migration target */
target_state omicron.public.migration_state NOT NULL,

/* The ID of the migration target Propolis */
target_propolis_id UUID NOT NULL,

/* Generation number owned and incremented by the target sled-agent */
target_gen INT8 NOT NULL DEFAULT 1,

/* Timestamp of when the source field was last updated.
*
* This is provided by the sled-agent when publishing a migration state
* update.
*/
time_target_updated TIMESTAMPTZ
);
4 changes: 4 additions & 0 deletions schema/crdb/add-instance-id-to-migrations/up03.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/* Lookup migrations by instance ID */
CREATE INDEX IF NOT EXISTS lookup_migrations_by_instance_id ON omicron.public.migration (
instance_id
);
8 changes: 8 additions & 0 deletions schema/crdb/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -4090,6 +4090,9 @@ CREATE TYPE IF NOT EXISTS omicron.public.migration_state AS ENUM (
CREATE TABLE IF NOT EXISTS omicron.public.migration (
id UUID PRIMARY KEY,

/* The ID of the instance that was migrated */
instance_id UUID NOT NULL,

/* The time this migration record was created. */
time_created TIMESTAMPTZ NOT NULL,

Expand Down Expand Up @@ -4129,6 +4132,11 @@ CREATE TABLE IF NOT EXISTS omicron.public.migration (
time_target_updated TIMESTAMPTZ
);

/* Lookup migrations by instance ID */
CREATE INDEX IF NOT EXISTS lookup_migrations_by_instance_id ON omicron.public.migration (
instance_id
);

/* Lookup region snapshot by snapshot id */
CREATE INDEX IF NOT EXISTS lookup_region_snapshot_by_snapshot_id on omicron.public.region_snapshot (
snapshot_id
Expand Down

0 comments on commit 82345ea

Please sign in to comment.