diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs index 643b71f8984..8f41eb47ccd 100644 --- a/nexus/src/app/sagas/instance_create.rs +++ b/nexus/src/app/sagas/instance_create.rs @@ -960,7 +960,7 @@ async fn sic_instance_ensure( } #[cfg(test)] -mod test { +pub mod test { use crate::{ app::saga::create_saga_dag, app::sagas::instance_create::Params, app::sagas::instance_create::SagaInstanceCreate, @@ -1124,7 +1124,9 @@ mod test { && sled_agent.disk_count().await == 0 } - async fn verify_clean_clate(cptestctx: &ControlPlaneTestContext) { + pub(crate) async fn verify_clean_slate( + cptestctx: &ControlPlaneTestContext, + ) { let sled_agent = &cptestctx.sled_agent.sled_agent; let datastore = cptestctx.server.apictx.nexus.datastore(); @@ -1178,7 +1180,7 @@ mod test { .await .expect_err("Saga should have failed"); - verify_clean_clate(&cptestctx).await; + verify_clean_slate(&cptestctx).await; } } @@ -1245,7 +1247,7 @@ mod test { .await .expect_err("Saga should have failed"); - verify_clean_clate(&cptestctx).await; + verify_clean_slate(&cptestctx).await; } } @@ -1308,6 +1310,6 @@ mod test { // This is important to ensure that our original saga didn't // double-allocate during repeated actions. destroy_instance(&cptestctx).await; - verify_clean_clate(&cptestctx).await; + verify_clean_slate(&cptestctx).await; } } diff --git a/nexus/src/app/sagas/instance_delete.rs b/nexus/src/app/sagas/instance_delete.rs index bae309434ca..e96d3c38534 100644 --- a/nexus/src/app/sagas/instance_delete.rs +++ b/nexus/src/app/sagas/instance_delete.rs @@ -8,6 +8,7 @@ use super::NexusSaga; use crate::app::sagas::declare_saga_actions; use crate::context::OpContext; use crate::{authn, authz}; +use omicron_common::api::external::{Error, ResourceType}; use serde::Deserialize; use serde::Serialize; use steno::ActionError; @@ -70,6 +71,16 @@ async fn sid_delete_instance_record( .datastore() .project_delete_instance(&opctx, ¶ms.authz_instance) .await + .or_else(|err| { + // Necessary for idempotency + match err { + Error::ObjectNotFound { + type_name: ResourceType::Instance, + lookup_type: _, + } => Ok(()), + _ => Err(err), + } + }) .map_err(ActionError::action_failed)?; Ok(()) } @@ -104,3 +115,193 @@ async fn sid_deallocate_external_ip( .map_err(ActionError::action_failed)?; Ok(()) } + +#[cfg(test)] +mod test { + use crate::{ + app::saga::create_saga_dag, + app::sagas::instance_create::test::verify_clean_slate, + app::sagas::instance_delete::Params, + app::sagas::instance_delete::SagaInstanceDelete, + authn::saga::Serialized, context::OpContext, db, + db::lookup::LookupPath, external_api::params, + }; + use dropshot::test_util::ClientTestContext; + use nexus_test_utils::resource_helpers::create_disk; + use nexus_test_utils::resource_helpers::create_organization; + use nexus_test_utils::resource_helpers::create_project; + use nexus_test_utils::resource_helpers::populate_ip_pool; + use nexus_test_utils::resource_helpers::DiskTest; + use nexus_test_utils_macros::nexus_test; + use nexus_types::identity::Resource; + use omicron_common::api::external::{ + ByteCount, IdentityMetadataCreateParams, InstanceCpuCount, Name, + }; + use std::num::NonZeroU32; + use uuid::Uuid; + + type ControlPlaneTestContext = + nexus_test_utils::ControlPlaneTestContext; + + const INSTANCE_NAME: &str = "my-instance"; + const ORG_NAME: &str = "test-org"; + const PROJECT_NAME: &str = "springfield-squidport"; + const DISK_NAME: &str = "my-disk"; + + async fn create_org_project_and_disk(client: &ClientTestContext) -> Uuid { + populate_ip_pool(&client, "default", None).await; + create_organization(&client, ORG_NAME).await; + let project = create_project(client, ORG_NAME, PROJECT_NAME).await; + create_disk(&client, ORG_NAME, PROJECT_NAME, DISK_NAME).await; + project.identity.id + } + + async fn new_test_params( + cptestctx: &ControlPlaneTestContext, + instance_id: Uuid, + ) -> Params { + let opctx = test_opctx(&cptestctx); + let datastore = cptestctx.server.apictx.nexus.datastore(); + + let (.., authz_instance, _instance) = + LookupPath::new(&opctx, &datastore) + .instance_id(instance_id) + .fetch() + .await + .expect("Failed to lookup instance"); + Params { + serialized_authn: Serialized::for_opctx(&opctx), + authz_instance, + } + } + + // Helper for creating instance create parameters + fn new_instance_create_params() -> params::InstanceCreate { + params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: INSTANCE_NAME.parse().unwrap(), + description: "My instance".to_string(), + }, + ncpus: InstanceCpuCount::try_from(2).unwrap(), + memory: ByteCount::from_gibibytes_u32(4), + hostname: String::from("inst"), + user_data: vec![], + network_interfaces: + params::InstanceNetworkInterfaceAttachment::Default, + external_ips: vec![params::ExternalIpCreate::Ephemeral { + pool_name: None, + }], + disks: vec![params::InstanceDiskAttachment::Attach( + params::InstanceDiskAttach { name: DISK_NAME.parse().unwrap() }, + )], + start: false, + } + } + + pub fn test_opctx(cptestctx: &ControlPlaneTestContext) -> OpContext { + OpContext::for_tests( + cptestctx.logctx.log.new(o!()), + cptestctx.server.apictx.nexus.datastore().clone(), + ) + } + + #[nexus_test(server = crate::Server)] + async fn test_saga_basic_usage_succeeds( + cptestctx: &ControlPlaneTestContext, + ) { + DiskTest::new(cptestctx).await; + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + create_org_project_and_disk(&client).await; + + // Build the saga DAG with the provided test parameters + let dag = create_saga_dag::( + new_test_params( + &cptestctx, + create_instance(&cptestctx, new_instance_create_params()) + .await + .id(), + ) + .await, + ) + .unwrap(); + let runnable_saga = nexus.create_runnable_saga(dag).await.unwrap(); + + // Actually run the saga + nexus + .run_saga(runnable_saga) + .await + .expect("Saga should have succeeded"); + } + + async fn create_instance( + cptestctx: &ControlPlaneTestContext, + params: params::InstanceCreate, + ) -> db::model::Instance { + let nexus = &cptestctx.server.apictx.nexus; + let opctx = test_opctx(&cptestctx); + + let project_selector = params::ProjectSelector { + organization_selector: Some( + Name::try_from(ORG_NAME.to_string()).unwrap().into(), + ), + project: PROJECT_NAME.to_string().try_into().unwrap(), + }; + let project_lookup = + nexus.project_lookup(&opctx, &project_selector).unwrap(); + nexus + .project_create_instance(&opctx, &project_lookup, ¶ms) + .await + .unwrap() + } + + #[nexus_test(server = crate::Server)] + async fn test_actions_succeed_idempotently( + cptestctx: &ControlPlaneTestContext, + ) { + DiskTest::new(cptestctx).await; + + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + create_org_project_and_disk(&client).await; + + // Build the saga DAG with the provided test parameters + let dag = create_saga_dag::( + new_test_params( + &cptestctx, + create_instance(&cptestctx, new_instance_create_params()) + .await + .id(), + ) + .await, + ) + .unwrap(); + + let runnable_saga = + nexus.create_runnable_saga(dag.clone()).await.unwrap(); + + // Cause all actions to run twice. The saga should succeed regardless! + for node in dag.get_nodes() { + nexus + .sec() + .saga_inject_repeat( + runnable_saga.id(), + node.index(), + steno::RepeatInjected { + action: NonZeroU32::new(2).unwrap(), + undo: NonZeroU32::new(1).unwrap(), + }, + ) + .await + .unwrap(); + } + + // Verify that the saga's execution succeeded. + nexus + .run_saga(runnable_saga) + .await + .expect("Saga should have succeeded"); + + verify_clean_slate(&cptestctx).await; + } +}