diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 47cbb4ba35..3a89d0273c 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -173,12 +173,58 @@ impl super::Nexus { // TODO-robustness We need to figure out what to do with Destroyed // instances? Presumably we need to clean them up at some point, but // not right away so that callers can see that they've been destroyed. - let (.., authz_instance) = LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) - .instance_name(instance_name) - .lookup_for(authz::Action::Delete) + let (.., authz_instance, db_instance) = + LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .instance_name(instance_name) + .fetch() + .await?; + + opctx.authorize(authz::Action::Delete, &authz_instance).await?; + + match db_instance.runtime_state.state.state() { + InstanceState::Stopped | InstanceState::Failed => { + // ok + } + + state => { + return Err(Error::InvalidRequest { + message: format!( + "instance cannot be deleted in state \"{}\"", + state, + ), + }); + } + } + + // Detach all attached disks + let disks = self + .instance_list_disks( + opctx, + organization_name, + project_name, + instance_name, + &DataPageParams { + marker: None, + direction: dropshot::PaginationOrder::Ascending, + limit: std::num::NonZeroU32::new(MAX_DISKS_PER_INSTANCE) + .unwrap(), + }, + ) .await?; + + for disk in &disks { + self.instance_detach_disk( + opctx, + organization_name, + project_name, + instance_name, + &disk.name(), + ) + .await?; + } + self.db_datastore.project_delete_instance(opctx, &authz_instance).await } diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index a6b2ba3e46..51b0dddacd 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -1024,6 +1024,7 @@ impl DataStore { ErrorHandler::NotFoundByResource(authz_instance), ) })?; + match result.status { UpdateStatus::Updated => Ok(()), UpdateStatus::NotUpdatedButExists => { diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index 3b2ead4b79..dcead06384 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -1527,6 +1527,154 @@ async fn test_cannot_attach_faulted_disks(cptestctx: &ControlPlaneTestContext) { } } +// Test that disks are detached when instance is destroyed +#[nexus_test] +async fn test_disks_detached_when_instance_destroyed( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + const ORGANIZATION_NAME: &str = "bobs-barrel-of-bytes"; + const PROJECT_NAME: &str = "bit-barrel"; + + // Test pre-reqs + DiskTest::new(&cptestctx).await; + create_organization(&client, ORGANIZATION_NAME).await; + create_project(client, ORGANIZATION_NAME, PROJECT_NAME).await; + + // Make 8 disks + for i in 0..8 { + create_disk( + &client, + ORGANIZATION_NAME, + PROJECT_NAME, + &format!("probablydata{}", i,), + ) + .await; + } + + // Assert we created 8 disks + let url_project_disks = format!( + "/organizations/{}/projects/{}/disks", + ORGANIZATION_NAME, PROJECT_NAME, + ); + let disks: Vec = NexusRequest::iter_collection_authn( + client, + &url_project_disks, + "", + None, + ) + .await + .expect("failed to list disks") + .all_items; + + assert_eq!(disks.len(), 8); + for disk in &disks { + assert_eq!(disk.state, DiskState::Detached); + } + + // Boot the instance + let instance_params = params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("nfs")).unwrap(), + description: String::from("probably serving data"), + }, + ncpus: InstanceCpuCount::try_from(2).unwrap(), + memory: ByteCount::from_mebibytes_u32(4), + hostname: String::from("nfs"), + user_data: vec![], + network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, + disks: (0..8) + .map(|i| { + params::InstanceDiskAttachment::Attach( + params::InstanceDiskAttach { + name: Name::try_from( + format!("probablydata{}", i).to_string(), + ) + .unwrap(), + }, + ) + }) + .collect(), + }; + + let url_instances = format!( + "/organizations/{}/projects/{}/instances", + ORGANIZATION_NAME, PROJECT_NAME + ); + + let builder = + RequestBuilder::new(client, http::Method::POST, &url_instances) + .body(Some(&instance_params)) + .expect_status(Some(http::StatusCode::CREATED)); + + let _response = NexusRequest::new(builder) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("expected instance creation!"); + + // Assert disks are attached + let url_project_disks = format!( + "/organizations/{}/projects/{}/disks", + ORGANIZATION_NAME, PROJECT_NAME, + ); + let disks: Vec = NexusRequest::iter_collection_authn( + client, + &url_project_disks, + "", + None, + ) + .await + .expect("failed to list disks") + .all_items; + + assert_eq!(disks.len(), 8); + for disk in &disks { + assert!(matches!(disk.state, DiskState::Attached(_))); + } + + // Stop and delete instance + let instance_url = format!( + "/organizations/{}/projects/{}/instances/nfs", + ORGANIZATION_NAME, PROJECT_NAME + ); + + let instance = + instance_post(&client, &instance_url, InstanceOp::Stop).await; + let apictx = &cptestctx.server.apictx; + let nexus = &apictx.nexus; + instance_simulate(nexus, &instance.identity.id).await; + let instance = instance_get(&client, &instance_url).await; + assert_eq!(instance.runtime.run_state, InstanceState::Stopped); + + NexusRequest::object_delete(&client, &instance_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); + + // Assert disks are detached + let url_project_disks = format!( + "/organizations/{}/projects/{}/disks", + ORGANIZATION_NAME, PROJECT_NAME, + ); + let disks: Vec = NexusRequest::iter_collection_authn( + client, + &url_project_disks, + "", + None, + ) + .await + .expect("failed to list disks") + .all_items; + + assert_eq!(disks.len(), 8); + for disk in &disks { + assert_eq!(disk.state, DiskState::Detached); + } +} + async fn instance_get( client: &ClientTestContext, instance_url: &str,