diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index d2cdf2ddb56..717c34c4d83 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -2645,11 +2645,11 @@ async fn cmd_db_disk_info( datastore: &DataStore, args: &DiskInfoArgs, ) -> Result<(), anyhow::Error> { + let conn = datastore.pool_connection_for_tests().await?; + let disk = { use nexus_db_schema::schema::disk::dsl; - let conn = datastore.pool_connection_for_tests().await?; - dsl::disk .filter(dsl::id.eq(args.uuid)) .select(nexus_db_model::Disk::as_select()) @@ -2658,7 +2658,7 @@ async fn cmd_db_disk_info( .context("failed to find disk")? }; - match datastore.disk_get_with_model(opctx, disk).await? { + match datastore.disk_get_with_model(&conn, disk).await? { Disk::Crucible(disk) => { crucible_disk_info(opctx, datastore, disk).await } diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index a8eaeae3c91..d3078823712 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -65,6 +65,7 @@ use nexus_types::internal_api::background::IncompleteBootstoreConfigReport; use nexus_types::internal_api::background::InstanceReincarnationStatus; use nexus_types::internal_api::background::InstanceUpdaterStatus; use nexus_types::internal_api::background::InventoryLoadStatus; +use nexus_types::internal_api::background::LocalStorageDeleteStatus; use nexus_types::internal_api::background::LookupRegionPortStatus; use nexus_types::internal_api::background::PhysicalDiskAdoptionStatus; use nexus_types::internal_api::background::ProbeDistributorStatus; @@ -90,6 +91,7 @@ use nexus_types::internal_api::background::TufArtifactReplicationCounters; use nexus_types::internal_api::background::TufArtifactReplicationRequest; use nexus_types::internal_api::background::TufArtifactReplicationStatus; use nexus_types::internal_api::background::TufRepoPrunerStatus; +use nexus_types::internal_api::background::VolumeDeleteStatus; use nexus_types::internal_api::background::fm_rendezvous; use omicron_uuid_kinds::BlueprintUuid; use omicron_uuid_kinds::CollectionUuid; @@ -1400,6 +1402,12 @@ fn print_task_details( "switch_port_config_manager" => { print_task_switch_port_settings_manager(details); } + "volume_delete" => { + print_task_volume_delete(details); + } + "local_storage_delete" => { + print_task_local_storage_delete(details); + } _ => { println!( "warning: unknown background task: {:?} \ @@ -4198,6 +4206,81 @@ fn print_task_physical_disk_adoption(details: &serde_json::Value) { } } +fn print_task_volume_delete(details: &serde_json::Value) { + match serde_json::from_value::(details.clone()) { + Err(error) => eprintln!( + "warning: failed to interpret task details: {:?}: {:?}", + error, details + ), + + Ok(status) => { + let VolumeDeleteStatus { + region_results, + running_snapshot_results, + snapshot_results, + volumes_deleted, + errors, + } = &status; + + println!(" result of deleting regions:"); + for result in region_results { + println!(" > {result}"); + } + + println!(" result of deleting running snapshots:"); + for result in running_snapshot_results { + println!(" > {result}"); + } + + println!(" result of deleting snapshots:"); + for result in snapshot_results { + println!(" > {result}"); + } + + println!(" volumes deleted:"); + for id in volumes_deleted { + println!(" > {id}"); + } + + println!(" errors: {}", errors.len()); + for error in errors { + println!(" > {error}"); + } + } + } +} + +fn print_task_local_storage_delete(details: &serde_json::Value) { + match serde_json::from_value::(details.clone()) { + Err(error) => eprintln!( + "warning: failed to interpret task details: {:?}: {:?}", + error, details + ), + + Ok(status) => { + let LocalStorageDeleteStatus { + delete_results, + deallocate_results, + errors, + } = &status; + + println!(" result of deleting local storage:"); + for result in delete_results { + println!(" > {result}"); + } + + println!(" result of deallocating local storage:"); + for result in deallocate_results { + println!(" > {result}"); + } + + println!(" errors: {}", errors.len()); + for error in errors { + println!(" > {error}"); + } + } + } +} const ERRICON: &str = "/!\\"; fn warn_if_nonzero(n: usize) -> &'static str { diff --git a/dev-tools/omdb/tests/env.out b/dev-tools/omdb/tests/env.out index 4712e4f3343..b07756ed122 100644 --- a/dev-tools/omdb/tests/env.out +++ b/dev-tools/omdb/tests/env.out @@ -155,6 +155,10 @@ task: "inventory_loader" loads the latest inventory collection from the DB +task: "local_storage_delete" + delete resources for disks backed by local storage + + task: "lookup_region_port" fill in missing ports for region records @@ -268,6 +272,10 @@ task: "v2p_manager" manages opte v2p mappings for vpc networking +task: "volume_delete" + delete resources from soft-deleted volumes + + task: "vpc_route_manager" propagates updated VPC routes to all OPTE ports @@ -428,6 +436,10 @@ task: "inventory_loader" loads the latest inventory collection from the DB +task: "local_storage_delete" + delete resources for disks backed by local storage + + task: "lookup_region_port" fill in missing ports for region records @@ -541,6 +553,10 @@ task: "v2p_manager" manages opte v2p mappings for vpc networking +task: "volume_delete" + delete resources from soft-deleted volumes + + task: "vpc_route_manager" propagates updated VPC routes to all OPTE ports @@ -688,6 +704,10 @@ task: "inventory_loader" loads the latest inventory collection from the DB +task: "local_storage_delete" + delete resources for disks backed by local storage + + task: "lookup_region_port" fill in missing ports for region records @@ -801,6 +821,10 @@ task: "v2p_manager" manages opte v2p mappings for vpc networking +task: "volume_delete" + delete resources from soft-deleted volumes + + task: "vpc_route_manager" propagates updated VPC routes to all OPTE ports diff --git a/dev-tools/omdb/tests/successes.out b/dev-tools/omdb/tests/successes.out index 27dbe0059db..f0c276e9678 100644 --- a/dev-tools/omdb/tests/successes.out +++ b/dev-tools/omdb/tests/successes.out @@ -390,6 +390,10 @@ task: "inventory_loader" loads the latest inventory collection from the DB +task: "local_storage_delete" + delete resources for disks backed by local storage + + task: "lookup_region_port" fill in missing ports for region records @@ -503,6 +507,10 @@ task: "v2p_manager" manages opte v2p mappings for vpc networking +task: "volume_delete" + delete resources from soft-deleted volumes + + task: "vpc_route_manager" propagates updated VPC routes to all OPTE ports @@ -863,6 +871,14 @@ task: "inventory_loader" loaded latest inventory collection as of : collection ....................., taken at +task: "local_storage_delete" + configured period: every h m s + last completed activation: , triggered by + started at (s ago) and ran for ms + result of deleting local storage: + result of deallocating local storage: + errors: 0 + task: "lookup_region_port" configured period: every m last completed activation: , triggered by @@ -1096,6 +1112,16 @@ task: "v2p_manager" started at (s ago) and ran for ms warning: unknown background task: "v2p_manager" (don't know how to interpret details: Object {}) +task: "volume_delete" + configured period: every h m s + last completed activation: , triggered by + started at (s ago) and ran for ms + result of deleting regions: + result of deleting running snapshots: + result of deleting snapshots: + volumes deleted: + errors: 0 + task: "vpc_route_manager" configured period: every s last completed activation: , triggered by @@ -1585,6 +1611,14 @@ task: "inventory_loader" loaded latest inventory collection as of : collection ....................., taken at +task: "local_storage_delete" + configured period: every h m s + last completed activation: , triggered by + started at (s ago) and ran for ms + result of deleting local storage: + result of deallocating local storage: + errors: 0 + task: "lookup_region_port" configured period: every m last completed activation: , triggered by @@ -1818,6 +1852,16 @@ task: "v2p_manager" started at (s ago) and ran for ms warning: unknown background task: "v2p_manager" (don't know how to interpret details: Object {}) +task: "volume_delete" + configured period: every h m s + last completed activation: , triggered by + started at (s ago) and ran for ms + result of deleting regions: + result of deleting running snapshots: + result of deleting snapshots: + volumes deleted: + errors: 0 + task: "vpc_route_manager" configured period: every s last completed activation: , triggered by diff --git a/nexus-config/src/nexus_config.rs b/nexus-config/src/nexus_config.rs index 4711aeb78ba..27ef8c97e23 100644 --- a/nexus-config/src/nexus_config.rs +++ b/nexus-config/src/nexus_config.rs @@ -481,6 +481,10 @@ pub struct BackgroundTaskConfig { pub audit_log_cleanup: AuditLogCleanupConfig, /// configuration for populate switch ports task pub populate_switch_ports: PopulateSwitchPortsConfig, + /// configuration for volume delete task + pub volume_delete: VolumeDeleteConfig, + /// configuration for local storage delete task + pub local_storage_delete: LocalStorageDeleteConfig, } #[serde_as] @@ -1108,6 +1112,22 @@ pub struct TrustQuorumConfig { pub period_secs: Duration, } +#[serde_as] +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct VolumeDeleteConfig { + /// period (in seconds) for periodic activations of this background task + #[serde_as(as = "DurationSeconds")] + pub period_secs: Duration, +} + +#[serde_as] +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct LocalStorageDeleteConfig { + /// period (in seconds) for periodic activations of this background task + #[serde_as(as = "DurationSeconds")] + pub period_secs: Duration, +} + /// Configuration for a nexus server #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct PackageConfig { @@ -1399,6 +1419,8 @@ mod test { audit_log_cleanup.retention_days = 90 audit_log_cleanup.max_deleted_per_activation = 10000 populate_switch_ports.period_secs = 31 + volume_delete.period_secs = 30 + local_storage_delete.period_secs = 30 [default_region_allocation_strategy] type = "random" seed = 0 @@ -1685,6 +1707,14 @@ mod test { populate_switch_ports: PopulateSwitchPortsConfig { period_secs: Duration::from_secs(31), }, + volume_delete: + VolumeDeleteConfig { + period_secs: Duration::from_secs(30), + }, + local_storage_delete: + LocalStorageDeleteConfig { + period_secs: Duration::from_secs(30), + }, }, multicast: MulticastConfig { enabled: false }, default_region_allocation_strategy: @@ -1802,6 +1832,8 @@ mod test { audit_log_cleanup.retention_days = 90 audit_log_cleanup.max_deleted_per_activation = 10000 populate_switch_ports.period_secs = 31 + volume_delete.period_secs = 30 + local_storage_delete.period_secs = 30 [default_region_allocation_strategy] type = "random" diff --git a/nexus/background-task-interface/src/init.rs b/nexus/background-task-interface/src/init.rs index 197bc057bec..8e0ee20036f 100644 --- a/nexus/background-task-interface/src/init.rs +++ b/nexus/background-task-interface/src/init.rs @@ -64,6 +64,8 @@ pub struct BackgroundTasks { pub task_attached_subnet_manager: Activator, pub task_session_cleanup: Activator, pub task_populate_switch_ports: Activator, + pub task_volume_delete: Activator, + pub task_local_storage_delete: Activator, // Handles to activate background tasks that do not get used by Nexus // at-large. These background tasks are implementation details as far as diff --git a/nexus/db-queries/src/db/datastore/disk.rs b/nexus/db-queries/src/db/datastore/disk.rs index f1fb522c526..921654a9df3 100644 --- a/nexus/db-queries/src/db/datastore/disk.rs +++ b/nexus/db-queries/src/db/datastore/disk.rs @@ -500,7 +500,9 @@ impl DataStore { let (.., disk) = LookupPath::new(opctx, self).disk_id(disk_id).fetch().await?; - self.disk_get_with_model(opctx, disk).await + let conn = self.pool_connection_authorized(opctx).await?; + + self.disk_get_with_model(&conn, disk).await } /// Return a `datastore::Disk` given a `model::Disk` @@ -508,15 +510,15 @@ impl DataStore { /// Note: basically all of Nexus should _not_ be using this, and should be /// using `disk_get` instead: this version of the function bypasses the /// LookupPath induced permissions check and should only called from omdb. + /// Code that is looking up deleted disks should also use this method, as + /// `LookupPath` will not return deleted resources. pub async fn disk_get_with_model( &self, - opctx: &OpContext, + conn: &async_bb8_diesel::Connection, disk: model::Disk, ) -> LookupResult { let disk_id = disk.id(); - let conn = self.pool_connection_authorized(opctx).await?; - let disk = match disk.disk_type { db::model::DiskType::Crucible => { use nexus_db_schema::schema::disk_type_crucible::dsl; @@ -524,7 +526,7 @@ impl DataStore { let disk_type_crucible = dsl::disk_type_crucible .filter(dsl::disk_id.eq(disk_id)) .select(DiskTypeCrucible::as_select()) - .first_async(&*conn) + .first_async(conn) .await .map_err(|e| { public_error_from_diesel(e, ErrorHandler::Server) @@ -542,7 +544,7 @@ impl DataStore { let disk_type_local_storage = dsl::disk_type_local_storage .filter(dsl::disk_id.eq(disk_id)) .select(DiskTypeLocalStorage::as_select()) - .first_async(&*conn) + .first_async(conn) .await .map_err(|e| { public_error_from_diesel(e, ErrorHandler::Server) diff --git a/nexus/db-queries/src/db/datastore/local_storage.rs b/nexus/db-queries/src/db/datastore/local_storage.rs index 7ba521d2807..0385a2e29aa 100644 --- a/nexus/db-queries/src/db/datastore/local_storage.rs +++ b/nexus/db-queries/src/db/datastore/local_storage.rs @@ -9,10 +9,12 @@ use crate::authz; use crate::context::OpContext; use crate::db::collection_insert::AsyncInsertError; use crate::db::collection_insert::DatastoreCollection; +use crate::db::datastore; use crate::db::datastore::DbConnection; use crate::db::datastore::LocalStorageAllocation; use crate::db::datastore::LocalStorageDisk; use crate::db::datastore::SQL_BATCH_SIZE; +use crate::db::model; use crate::db::model::LocalStorageDatasetAllocation; use crate::db::model::LocalStorageUnencryptedDatasetAllocation; use crate::db::model::RendezvousLocalStorageDataset; @@ -451,4 +453,61 @@ impl DataStore { }) .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + + /// Return all deleted disks that have undeleted local storage allocations + pub async fn deleted_disks_with_undeleted_local_storage( + &self, + opctx: &OpContext, + ) -> Result, Error> { + opctx.check_complex_operations_allowed()?; + + let conn = self.pool_connection_authorized(opctx).await?; + + use nexus_db_schema::schema::disk::dsl; + use nexus_db_schema::schema::disk_type_local_storage::dsl as dtls_dsl; + use nexus_db_schema::schema::local_storage_unencrypted_dataset_allocation::dsl as lsuda_dsl; + + // Find all deleted disks where the unencrypted local storage allocation + // is not yet deleted. + let found_disks: Vec = dsl::disk + .inner_join( + dtls_dsl::disk_type_local_storage + .on(dsl::id.eq(dtls_dsl::disk_id)), + ) + .inner_join( + lsuda_dsl::local_storage_unencrypted_dataset_allocation.on( + dtls_dsl::local_storage_unencrypted_dataset_allocation_id + .eq(lsuda_dsl::id.nullable()), + ), + ) + .filter(lsuda_dsl::time_deleted.is_null()) + .filter(dsl::time_deleted.is_not_null()) + .select(model::Disk::as_select()) + .load_async(&*conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + let mut disks = Vec::with_capacity(found_disks.len()); + + for found_disk in found_disks { + match self.disk_get_with_model(&conn, found_disk).await? { + datastore::Disk::Crucible(crucible_disk) => { + // The query above joins the disk table with the + // disk_type_local_storage table, meaning the higher level + // Disk can never be the Crucible type, unless there's a + // serious problem. Return an error instead of panicking. + return Err(Error::internal_error(&format!( + "disk {} should be the local storage, not crucible", + crucible_disk.id(), + ))); + } + + datastore::Disk::LocalStorage(local_storage_disk) => { + disks.push(local_storage_disk); + } + } + } + + Ok(disks) + } } diff --git a/nexus/db-queries/src/db/datastore/volume.rs b/nexus/db-queries/src/db/datastore/volume.rs index a3192f69f31..a10c708551f 100644 --- a/nexus/db-queries/src/db/datastore/volume.rs +++ b/nexus/db-queries/src/db/datastore/volume.rs @@ -1765,6 +1765,40 @@ impl DataStore { }) } + pub async fn get_soft_deleted_volumes( + &self, + opctx: &OpContext, + ) -> ListResultVec { + opctx.check_complex_operations_allowed()?; + + let mut volumes = Vec::new(); + let mut paginator = Paginator::new( + SQL_BATCH_SIZE, + dropshot::PaginationOrder::Ascending, + ); + let conn = self.pool_connection_authorized(opctx).await?; + + while let Some(p) = paginator.next() { + use nexus_db_schema::schema::volume::dsl; + + let mut page = + paginated(dsl::volume, dsl::id, &p.current_pagparams()) + .filter(dsl::time_deleted.is_not_null()) + .select(Volume::as_select()) + .get_results_async::(&*conn) + .await + .map_err(|e| { + public_error_from_diesel(e, ErrorHandler::Server) + })?; + + paginator = p.found_batch(&page, &|r| *r.id().as_untyped_uuid()); + + volumes.append(&mut page); + } + + Ok(volumes) + } + async fn volume_remove_rop_in_txn( conn: &async_bb8_diesel::Connection, err: OptionalError, diff --git a/nexus/examples/config-second.toml b/nexus/examples/config-second.toml index 166afa72ba0..c2af4bb9907 100644 --- a/nexus/examples/config-second.toml +++ b/nexus/examples/config-second.toml @@ -217,6 +217,8 @@ audit_log_cleanup.period_secs = 600 audit_log_cleanup.retention_days = 90 audit_log_cleanup.max_deleted_per_activation = 10000 populate_switch_ports.period_secs = 30 +volume_delete.period_secs = 30 +local_storage_delete.period_secs = 30 [default_region_allocation_strategy] # allocate region on 3 random distinct zpools, on 3 random distinct sleds. diff --git a/nexus/examples/config.toml b/nexus/examples/config.toml index eeb4fa52666..2848524d8e7 100644 --- a/nexus/examples/config.toml +++ b/nexus/examples/config.toml @@ -201,6 +201,8 @@ audit_log_cleanup.period_secs = 600 audit_log_cleanup.retention_days = 90 audit_log_cleanup.max_deleted_per_activation = 10000 populate_switch_ports.period_secs = 30 +volume_delete.period_secs = 30 +local_storage_delete.period_secs = 30 [default_region_allocation_strategy] # allocate region on 3 random distinct zpools, on 3 random distinct sleds. diff --git a/nexus/src/app/background/init.rs b/nexus/src/app/background/init.rs index 8233ea5db11..b990b06ed11 100644 --- a/nexus/src/app/background/init.rs +++ b/nexus/src/app/background/init.rs @@ -118,6 +118,7 @@ use super::tasks::instance_updater; use super::tasks::instance_watcher; use super::tasks::inventory_collection; use super::tasks::inventory_load; +use super::tasks::local_storage_delete::*; use super::tasks::lookup_region_port; use super::tasks::metrics_producer_gc; use super::tasks::multicast::MulticastGroupReconciler; @@ -143,6 +144,7 @@ use super::tasks::trust_quorum; use super::tasks::tuf_artifact_replication; use super::tasks::tuf_repo_pruner; use super::tasks::v2p_mappings::V2PManager; +use super::tasks::volume_delete::*; use super::tasks::vpc_routes; use super::tasks::webhook_deliverator; use crate::Nexus; @@ -282,6 +284,8 @@ impl BackgroundTasksInitializer { task_attached_subnet_manager: Activator::new(), task_session_cleanup: Activator::new(), task_populate_switch_ports: Activator::new(), + task_volume_delete: Activator::new(), + task_local_storage_delete: Activator::new(), // Handles to activate background tasks that do not get used by Nexus // at-large. These background tasks are implementation details as far as @@ -379,6 +383,8 @@ impl BackgroundTasksInitializer { task_audit_log_timeout_incomplete, task_audit_log_cleanup, task_populate_switch_ports, + task_volume_delete, + task_local_storage_delete, // Add new background tasks here. Be sure to use this binding in a // call to `Driver::register()` below. That's what actually wires // up the Activator to the corresponding background task. @@ -1328,6 +1334,26 @@ impl BackgroundTasksInitializer { activator: task_populate_switch_ports, }); + driver.register(TaskDefinition { + name: "volume_delete", + description: "delete resources from soft-deleted volumes", + period: config.volume_delete.period_secs, + task_impl: Box::new(VolumeDeleter::new(datastore.clone())), + opctx: opctx.child(BTreeMap::new()), + watchers: vec![], + activator: task_volume_delete, + }); + + driver.register(TaskDefinition { + name: "local_storage_delete", + description: "delete resources for disks backed by local storage", + period: config.local_storage_delete.period_secs, + task_impl: Box::new(LocalStorageDeleter::new(datastore.clone())), + opctx: opctx.child(BTreeMap::new()), + watchers: vec![], + activator: task_local_storage_delete, + }); + driver } } diff --git a/nexus/src/app/background/tasks/local_storage_delete.rs b/nexus/src/app/background/tasks/local_storage_delete.rs new file mode 100644 index 00000000000..7898fa8516f --- /dev/null +++ b/nexus/src/app/background/tasks/local_storage_delete.rs @@ -0,0 +1,276 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! When the higher level disks backed by local storage are deleted, this +//! background task will delete any allocated local storage, and will delete +//! local storage allocation records when those resources have been cleand up. + +use crate::app::background::BackgroundTask; +use futures::FutureExt; +use futures::future::BoxFuture; +use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::DataStore; +use nexus_db_queries::db::datastore::LocalStorageAllocation; +use nexus_db_queries::db::model::LocalStorageUnencryptedDatasetAllocation; +use nexus_types::internal_api::background::LocalStorageDeleteStatus; +use serde_json::json; +use sled_agent_client::types::LocalStorageDatasetDeleteRequest; +use slog::Logger; +use std::sync::Arc; + +pub struct LocalStorageDeleter { + datastore: Arc, + reqwest_client: reqwest::Client, +} + +/// Functions that poll for an expected change will either return that the +/// change occurred, or that the current activation of this task has to wait for +/// the change to occur in a future activations of this task. +#[derive(PartialEq)] +enum DeleteResult { + Deleted, + + WaitForNextActivation, +} + +impl LocalStorageDeleter { + pub fn new(datastore: Arc) -> Self { + LocalStorageDeleter { + datastore, + reqwest_client: reqwest::Client::new(), + } + } + + async fn delete_unencrypted_allocation( + &self, + log: &Logger, + opctx: &OpContext, + allocation: &LocalStorageUnencryptedDatasetAllocation, + status: &mut LocalStorageDeleteStatus, + ) -> DeleteResult { + let sled_id = allocation.sled_id(); + let zpool_id = allocation.pool_id().upcast(); + + // Check if either the sled or disk backing the zpool was expunged. If + // we can't determine then bail and wait for the next task activation. + + let sled_in_service = + match self.datastore.check_sled_in_service(&opctx, sled_id).await { + Ok(sled_in_service) => sled_in_service, + + Err(e) => { + let s = format!( + "error calling check_sled_in_service for sled \ + {sled_id}: {e}", + ); + + error!(log, "{s}"); + status.errors.push(s); + + return DeleteResult::WaitForNextActivation; + } + }; + + if !sled_in_service { + // Sled's been expunged, so consider the local storage deleted. + return DeleteResult::Deleted; + } + + let zpool_in_service = + match self.datastore.check_zpool_in_service(&opctx, zpool_id).await + { + Ok(zpool_in_service) => zpool_in_service, + + Err(e) => { + let s = format!( + "error calling check_zpool_in_service for zpool \ + {zpool_id}: {e}", + ); + + error!(log, "{s}"); + status.errors.push(s); + + return DeleteResult::WaitForNextActivation; + } + }; + + if !zpool_in_service { + // The disk backing the zpool's been expunged, so consider the local + // storage deleted. + return DeleteResult::Deleted; + } + + // Now that all checks are done, get a sled agent client and make the + // delete request. + + let request = LocalStorageDatasetDeleteRequest { + zpool_id: allocation.pool_id(), + dataset_id: allocation.id(), + encrypted_at_rest: false, + }; + + let sled_agent_client = match nexus_networking::sled_client_ext( + &self.datastore, + opctx, + sled_id, + log, + self.reqwest_client.clone(), + ) + .await + { + Ok(client) => client, + + Err(e) => { + let s = format!( + "error calling sled_client_ext for allocation {}: {e}", + allocation.id(), + ); + + error!(log, "{s}"); + status.errors.push(s); + + return DeleteResult::WaitForNextActivation; + } + }; + + match sled_agent_client.local_storage_dataset_delete(&request).await { + Ok(_) => DeleteResult::Deleted, + + Err(e) => { + let s = + format!("error sending local_storage_dataset_delete: {e}"); + + error!(log, "{s}"); + status.errors.push(s); + + DeleteResult::WaitForNextActivation + } + } + } +} + +impl BackgroundTask for LocalStorageDeleter { + fn activate<'a>( + &'a mut self, + opctx: &'a OpContext, + ) -> BoxFuture<'a, serde_json::Value> { + async { + let log = &opctx.log; + let mut status = LocalStorageDeleteStatus::default(); + + let disks_needing_clean_up = match self + .datastore + .deleted_disks_with_undeleted_local_storage(opctx) + .await + { + Ok(v) => v, + + Err(e) => { + let s = format!( + "error calling \ + unencrypted_allocations_for_deleted_disks: {e}" + ); + + error!(log, "{s}"); + status.errors.push(s); + + return json!(status); + } + }; + + for disk in disks_needing_clean_up { + let Some(allocation) = &disk.local_storage_dataset_allocation + else { + // No allocation was made for this disk + continue; + }; + + // Attempt deleting the local storage before removing the + // database record. If the delete does not succeed, try again in + // the next task activation. + + match allocation { + LocalStorageAllocation::Unencrypted(allocation) => { + match self + .delete_unencrypted_allocation( + log, + opctx, + &allocation, + &mut status, + ) + .await + { + DeleteResult::Deleted => { + let s = format!( + "deleted disk {} allocation {}", + disk.id(), + allocation.id(), + ); + + info!(log, "{s}"); + status.delete_results.push(s); + + // Drop through to deallocation once deletion + // succeeds. + } + + DeleteResult::WaitForNextActivation => { + // Cannot deallocate the record until deletion + // succeeds. + continue; + } + } + } + + LocalStorageAllocation::Encrypted(allocation) => { + // Until encrypted local storage is supported, seeing a + // request to clean up disks of that type should be + // noted as a error. + let s = format!( + "request to delete disk {} encrypted allocation {}", + disk.id(), + allocation.id(), + ); + + error!(log, "{s}"); + status.errors.push(s); + + continue; + } + } + + match self + .datastore + .delete_local_storage_dataset_allocations(opctx, &disk) + .await + { + Ok(()) => { + let s = format!( + "deallocated disk {} allocation {}", + disk.id(), + allocation.id(), + ); + info!(log, "{s}"); + status.deallocate_results.push(s); + } + + Err(e) => { + let s = format!( + "error calling \ + delete_local_storage_dataset_allocations: {e}" + ); + + error!(log, "{s}"); + status.errors.push(s); + + continue; + } + } + } + + json!(status) + } + .boxed() + } +} diff --git a/nexus/src/app/background/tasks/mod.rs b/nexus/src/app/background/tasks/mod.rs index 1058d0cd638..9213d36e96b 100644 --- a/nexus/src/app/background/tasks/mod.rs +++ b/nexus/src/app/background/tasks/mod.rs @@ -31,6 +31,7 @@ pub mod instance_updater; pub mod instance_watcher; pub mod inventory_collection; pub mod inventory_load; +pub mod local_storage_delete; pub mod lookup_region_port; pub mod metrics_producer_gc; pub mod multicast; @@ -58,5 +59,6 @@ pub mod trust_quorum; pub mod tuf_artifact_replication; pub mod tuf_repo_pruner; pub mod v2p_mappings; +pub mod volume_delete; pub mod vpc_routes; pub mod webhook_deliverator; diff --git a/nexus/src/app/background/tasks/volume_delete.rs b/nexus/src/app/background/tasks/volume_delete.rs new file mode 100644 index 00000000000..99e27f56e2b --- /dev/null +++ b/nexus/src/app/background/tasks/volume_delete.rs @@ -0,0 +1,1221 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Nexus is responsible for telling Crucible Agent(s) when to clean up +//! resources - those Agents do not have any idea of what volumes are +//! constructed, currently active, etc. Plus, volumes can (and will) change +//! during their lifetime. Operations like growing a disk, removing a read-only +//! parent after a scrub has completed, or re-encrypting a disk will all change +//! the volume that backs a disk. +//! +//! Nexus has to account for all the Crucible resources it is using, and count +//! how many volumes are using those resources. Only when that count drops to +//! zero is it valid to clean up the appropriate Crucible resource. +//! +//! Complicating things is the fact that ZFS datasets cannot be deleted if there +//! are snapshots of that dataset. Nexus' resource accounting must take this +//! dependency into account. Note that ZFS snapshots can layer, but any snapshot +//! can be deleted without the requirement of (for example) deleting the +//! snapshots in a certain order. +//! +//! Multiple Nexus will be running this background task and will result in many +//! identical requests being sent to the downstream Crucible agents. Each +//! Crucible agent's endpoint is idempotent and should handle this without any +//! issues. + +use crate::app::background::BackgroundTask; +use crucible_agent_client::Client as CrucibleAgentClient; +use crucible_agent_client::types::GetSnapshotResponse; +use crucible_agent_client::types::Region; +use crucible_agent_client::types::RegionId; +use crucible_agent_client::types::State as RegionState; +use futures::FutureExt; +use futures::future::BoxFuture; +use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::DataStore; +use nexus_db_queries::db::datastore::CrucibleResources; +use nexus_db_queries::db::datastore::FreedCrucibleResources; +use nexus_db_queries::db::model::CrucibleDataset; +use nexus_types::identity::Asset; +use nexus_types::internal_api::background::VolumeDeleteStatus; +use omicron_common::api::external::Error; +use omicron_uuid_kinds::DatasetUuid; +use omicron_uuid_kinds::VolumeUuid; +use serde_json::json; +use slog::Logger; +use std::sync::Arc; +use uuid::Uuid; + +pub struct VolumeDeleter { + datastore: Arc, + reqwest_client: reqwest::Client, +} + +/// Almost all endpoints that the Crucible Agent exposes are requests for +/// something asynchronous to occur. Functions that poll for the expected change +/// will either return that the change occurred, or that the current activation +/// of this task has to wait for the change to occur in a future activations of +/// this task. +#[derive(PartialEq)] +enum DeleteResult { + Deleted, + + WaitForNextActivation, +} + +// The functions in this impl can roughly be separated into two categories: +// +// 1. those that aceept a `status` argument and return a `DeleteResult` +// 2. those that do not accept a `status` argument and return a Result +// +// This separation attempts to prevent duplicates in the status' errors field. +// Functions that accept a status argument are responsible for adding to it, +// particularly adding to the errors field the Err results of functions that do +// not accept a status argument. Any function can write to the log. + +impl VolumeDeleter { + pub fn new(datastore: Arc) -> Self { + VolumeDeleter { datastore, reqwest_client: reqwest::Client::new() } + } + + fn crucible_agent_client_for_dataset( + &self, + dataset: &CrucibleDataset, + ) -> CrucibleAgentClient { + CrucibleAgentClient::new_with_client( + &format!("http://{}", dataset.address()), + self.reqwest_client.clone(), + ) + } + + /// Return true if the Crucible agent is gone, and false if it's expected to + /// be there and answer Nexus. + async fn crucible_agent_is_gone( + &self, + dataset_id: DatasetUuid, + ) -> Result { + let on_in_service_physical_disk = self + .datastore + .crucible_dataset_physical_disk_in_service(dataset_id) + .await?; + + let is_gone = !on_in_service_physical_disk; + + Ok(is_gone) + } + + /// Returns a Ok(Some(Region)) if a region with id {region_id} exists, + /// Ok(None) if it does not (a 404 was seen), and Err otherwise. + async fn maybe_get_crucible_region( + &self, + log: &Logger, + dataset: &CrucibleDataset, + region_id: Uuid, + ) -> Result, Error> { + let client = self.crucible_agent_client_for_dataset(dataset); + let dataset_id = dataset.id(); + + if self.crucible_agent_is_gone(dataset_id).await? { + return Err(Error::Gone); + } + + match client.region_get(&RegionId(region_id.to_string())).await { + Ok(v) => Ok(Some(v.into_inner())), + + Err(e) => { + if is_not_found(&e) { + // A 404 Not Found is ok for this function, just return None + Ok(None) + } else { + error!( + log, + "region_get saw {:?}", + e; + "region_id" => %region_id, + "dataset_id" => %dataset_id, + ); + + Err(into_external_error(&e)) + } + } + } + } + + /// Send a region deletion request + async fn request_crucible_region_delete( + &self, + log: &Logger, + dataset: &CrucibleDataset, + region_id: Uuid, + ) -> Result<(), Error> { + let client = self.crucible_agent_client_for_dataset(dataset); + let dataset_id = dataset.id(); + + if self.crucible_agent_is_gone(dataset_id).await? { + return Err(Error::Gone); + } + + match client.region_delete(&RegionId(region_id.to_string())).await { + Ok(_) => Ok(()), + + Err(e) => { + error!( + log, + "region_delete saw {:?}", + e; + "region_id" => %region_id, + "dataset_id" => %dataset.id(), + ); + + Err(into_external_error(&e)) + } + } + } + + /// Call out to a Crucible agent to delete a region. Poll to see if that + /// region's state has changed and indicated it has been deleted. + async fn delete_crucible_region( + &self, + log: &Logger, + dataset: &CrucibleDataset, + region_id: Uuid, + ) -> Result { + // If the region never existed, then a `GET` will return 404, and so + // will a `DELETE`. Catch this case, and return Ok if the region never + // existed. This can occur if ensuring all datasets and regions in a + // region set partially fails, and the entire region set is being + // deleted. + + match self.maybe_get_crucible_region(log, dataset, region_id).await { + Ok(Some(_)) => { + // region found, proceed with deleting + } + + Ok(None) => { + // region never exited, return Ok + return Ok(DeleteResult::Deleted); + } + + // Return Ok if the dataset's agent is gone, no delete call + // is required. + Err(Error::Gone) => { + warn!( + log, + "dataset is gone"; + "dataset_id" => %dataset.id(), + ); + + return Ok(DeleteResult::Deleted); + } + + Err(e) => return Err(e), + } + + // Past here, the region exists (or existed at some point): ensure it is + // deleted. Request the deletion (which is idempotent), then query for + // the appropriate state change. + + self.request_crucible_region_delete(log, dataset, region_id).await?; + + let region = + match self.maybe_get_crucible_region(log, dataset, region_id).await + { + // Previous to the delete request, we queried and found this + // region, and now it's gone. + Ok(None) => Err(Error::internal_error(format!( + "dataset {} region {region_id} is missing now!", + dataset.id(), + ))), + + Ok(Some(region)) => Ok(region), + + // Return Ok if the dataset's agent is gone, no state check is + // required. + Err(Error::Gone) => { + warn!( + log, + "dataset is gone"; + "dataset_id" => %dataset.id(), + ); + + return Ok(DeleteResult::Deleted); + } + + Err(e) => Err(e), + }?; + + // If this state change query shows that the region was deleted, + // proceed, otherwise the overall task needs to wait. + + match region.state { + RegionState::Tombstoned => Ok(DeleteResult::WaitForNextActivation), + + RegionState::Destroyed => Ok(DeleteResult::Deleted), + + RegionState::Failed => { + // If the delete failed, Nexus can re-request that the region be + // deleted, and it will move back to Tombstoned. This will occur + // on the next invocation of this task. + + Ok(DeleteResult::WaitForNextActivation) + } + + RegionState::Requested | RegionState::Created => { + // It's unexpected that the region be in this state after a + // deletion request. We successfully requested the region + // deletion before entering this check, and the Crucible agent + // should prevent the state transition from Tombstoned to either + // of these states. + + Err(Error::internal_error(format!( + "region is {:?} after successful deletion request!", + region.state, + ))) + } + } + } + + async fn delete_crucible_regions( + &self, + log: &Logger, + crucible_resources_to_delete: &CrucibleResources, + status: &mut VolumeDeleteStatus, + ) -> DeleteResult { + let datasets_and_regions = match self + .datastore + .regions_to_delete(&crucible_resources_to_delete) + .await + { + Ok(datasets_and_regions) => datasets_and_regions, + + Err(e) => { + let s = format!("error calling regions_to_delete: {e}"); + error!(log, "{s}"); + status.errors.push(s); + + // Try again next time! + return DeleteResult::WaitForNextActivation; + } + }; + + let request_count = datasets_and_regions.len(); + if request_count == 0 { + return DeleteResult::Deleted; + } + + // Send DELETE calls to the corresponding Crucible agents + let mut all_deleted = true; + + for (dataset, region) in &datasets_and_regions { + match self.delete_crucible_region(log, &dataset, region.id()).await + { + Ok(DeleteResult::Deleted) => { + status + .region_results + .push(format!("{}: deleted", region.id())); + } + + Ok(DeleteResult::WaitForNextActivation) => { + status.region_results.push(format!( + "{}: requested deletion, waiting", + region.id() + )); + + all_deleted = false; + } + + Err(e) => { + status.errors.push(format!("{e}")); + + all_deleted = false; + } + } + } + + // If any of the operations require waiting for the next activation, + // then return that, otherwise return that all were deleted. + + if all_deleted { + // When all crucible resources are cleaned up, hard delete the + // region records. This also re-computes the crucible_dataset + // size_used column for those region's datasets. + + let region_ids_to_delete = + datasets_and_regions.iter().map(|(_, r)| r.id()).collect(); + + match self + .datastore + .regions_hard_delete(log, region_ids_to_delete) + .await + { + Ok(()) => DeleteResult::Deleted, + + Err(e) => { + let s = format!("error calling regions_hard_delete: {e}"); + error!(log, "{s}"); + status.errors.push(s); + + // More work is required next task activation + DeleteResult::WaitForNextActivation + } + } + } else { + // More work is required next task activation + DeleteResult::WaitForNextActivation + } + } + + async fn request_crucible_running_snapshot_delete( + &self, + log: &Logger, + dataset: &CrucibleDataset, + region_id: Uuid, + snapshot_id: Uuid, + ) -> Result<(), Error> { + let client = self.crucible_agent_client_for_dataset(dataset); + let dataset_id = dataset.id(); + + if self.crucible_agent_is_gone(dataset_id).await? { + return Err(Error::Gone); + } + + match client + .region_delete_running_snapshot( + &RegionId(region_id.to_string()), + &snapshot_id.to_string(), + ) + .await + { + Ok(_) => Ok(()), + + Err(e) => { + error!( + log, + "region_delete_running_snapshot saw {:?}", + e; + "dataset_id" => %dataset.id(), + "region_id" => %region_id, + "snapshot_id" => %snapshot_id, + ); + + Err(into_external_error(&e)) + } + } + } + + async fn get_crucible_region_snapshots( + &self, + log: &Logger, + dataset: &CrucibleDataset, + region_id: Uuid, + ) -> Result { + let client = self.crucible_agent_client_for_dataset(dataset); + let dataset_id = dataset.id(); + + if self.crucible_agent_is_gone(dataset_id).await? { + return Err(Error::Gone); + } + + match client + .region_get_snapshots(&RegionId(region_id.to_string())) + .await + { + Ok(v) => Ok(v.into_inner()), + + Err(e) => { + error!( + log, + "region_get_snapshots saw {:?}", + e; + "dataset_id" => %dataset.id(), + "region_id" => %region_id, + ); + + Err(into_external_error(&e)) + } + } + } + + async fn delete_crucible_running_snapshot( + &self, + log: &Logger, + dataset: &CrucibleDataset, + region_id: Uuid, + snapshot_id: Uuid, + ) -> Result { + // Request the deletion (which is idempotent), then query for the + // appropriate state change. + + match self + .request_crucible_running_snapshot_delete( + log, + dataset, + region_id, + snapshot_id, + ) + .await + { + Ok(()) => { + // ok + } + + Err(Error::Gone) => { + warn!( + log, + "dataset is gone"; + "dataset_id" => %dataset.id(), + ); + + return Ok(DeleteResult::Deleted); + } + + Err(e) => { + return Err(e); + } + } + + let response = match self + .get_crucible_region_snapshots(log, dataset, region_id) + .await + { + Ok(v) => v, + + // Return Ok if the dataset's agent is gone, no + // delete call is required. + Err(Error::Gone) => { + warn!( + log, + "dataset is gone"; + "dataset_id" => %dataset.id(), + ); + + return Ok(DeleteResult::Deleted); + } + + Err(e) => { + return Err(e); + } + }; + + match response.running_snapshots.get(&snapshot_id.to_string()) { + Some(running_snapshot) => match running_snapshot.state { + RegionState::Tombstoned => { + Ok(DeleteResult::WaitForNextActivation) + } + + RegionState::Destroyed => Ok(DeleteResult::Deleted), + + _ => { + warn!( + log, + "running_snapshot is Some, state is {}", + running_snapshot.state.to_string(); + "region_id" => %region_id, + "snapshot_id" => %snapshot_id, + ); + + Ok(DeleteResult::WaitForNextActivation) + } + }, + + None => { + // It's possible that the running snapshot record was GCed, and + // it won't come back - consider it deleted. + + info!( + log, + "running_snapshot is None"; + "region_id" => %region_id, + "snapshot_id" => %snapshot_id, + ); + + Ok(DeleteResult::Deleted) + } + } + } + + async fn delete_crucible_running_snapshots( + &self, + log: &Logger, + crucible_resources_to_delete: &CrucibleResources, + status: &mut VolumeDeleteStatus, + ) -> DeleteResult { + let datasets_and_snapshots = match self + .datastore + .snapshots_to_delete(&crucible_resources_to_delete) + .await + { + Ok(datasets_and_snapshots) => datasets_and_snapshots, + + Err(e) => { + let s = format!("error calling snapshots_to_delete: {e}"); + error!(log, "{s}"); + status.errors.push(s); + + // Try again next time! + return DeleteResult::WaitForNextActivation; + } + }; + + let request_count = datasets_and_snapshots.len(); + if request_count == 0 { + return DeleteResult::Deleted; + } + + // Send DELETE calls to the corresponding Crucible agents + let mut all_deleted = true; + + for (dataset, region_snapshot) in datasets_and_snapshots { + let region_id = region_snapshot.region_id; + let snapshot_id = region_snapshot.snapshot_id; + + match self + .delete_crucible_running_snapshot( + log, + &dataset, + region_id, + snapshot_id, + ) + .await + { + Ok(DeleteResult::Deleted) => { + status.running_snapshot_results.push(format!( + "{} / {}: deleted", + region_id, snapshot_id + )); + } + + Ok(DeleteResult::WaitForNextActivation) => { + status.running_snapshot_results.push(format!( + "{} / {}: requested deletion, waiting", + region_id, snapshot_id, + )); + + all_deleted = false; + } + + Err(e) => { + status.errors.push(format!("{e}")); + + all_deleted = false; + } + } + } + + // If any of the operations require waiting for the next activation, + // then return that, otherwise return that all were deleted. + + if all_deleted { + DeleteResult::Deleted + } else { + DeleteResult::WaitForNextActivation + } + } + + async fn request_crucible_snapshot_delete( + &self, + log: &Logger, + dataset: &CrucibleDataset, + region_id: Uuid, + snapshot_id: Uuid, + ) -> Result<(), Error> { + let client = self.crucible_agent_client_for_dataset(dataset); + let dataset_id = dataset.id(); + + if self.crucible_agent_is_gone(dataset_id).await? { + return Err(Error::Gone); + } + + match client + .region_delete_snapshot( + &RegionId(region_id.to_string()), + &snapshot_id.to_string(), + ) + .await + { + Ok(_) => Ok(()), + + Err(e) => { + error!( + log, + "region_delete_snapshot saw {:?}", + e; + "dataset_id" => %dataset_id, + "region_id" => %region_id, + "snapshot_id" => %snapshot_id, + ); + + Err(into_external_error(&e)) + } + } + } + + async fn delete_crucible_snapshot( + &self, + log: &Logger, + dataset: &CrucibleDataset, + region_id: Uuid, + snapshot_id: Uuid, + ) -> Result { + // Unlike other Crucible agent endpoints, this one is synchronous in + // that it is not only a request to the Crucible agent: `zfs destroy` is + // performed right away. However this is still a request to illumos that + // may not take effect right away. Wait until the snapshot no longer + // appears in the list of region snapshots, meaning it was not returned + // from `zfs list`. + + // Request the deletion (which is idempotent), then query for the + // appropriate state change. + + match self + .request_crucible_snapshot_delete( + log, + dataset, + region_id, + snapshot_id, + ) + .await + { + Ok(()) => { + // ok + } + + Err(Error::Gone) => { + return Ok(DeleteResult::Deleted); + } + + Err(e) => { + return Err(e); + } + } + + let response = match self + .get_crucible_region_snapshots(log, dataset, region_id) + .await + { + Ok(v) => v, + + // Return Ok if the dataset's agent is gone, no + // delete call is required. + Err(Error::Gone) => { + warn!( + log, + "dataset is gone"; + "dataset_id" => %dataset.id(), + ); + + return Ok(DeleteResult::Deleted); + } + + Err(e) => { + return Err(e); + } + }; + + // If the snapshot is still returned in the list of snapshots, wait for + // it to be deleted. + + if response.snapshots.iter().any(|x| x.name == snapshot_id.to_string()) + { + Ok(DeleteResult::WaitForNextActivation) + } else { + Ok(DeleteResult::Deleted) + } + } + + async fn delete_crucible_snapshots( + &self, + log: &Logger, + crucible_resources_to_delete: &CrucibleResources, + status: &mut VolumeDeleteStatus, + ) -> DeleteResult { + let datasets_and_snapshots = match self + .datastore + .snapshots_to_delete(&crucible_resources_to_delete) + .await + { + Ok(datasets_and_snapshots) => datasets_and_snapshots, + + Err(e) => { + let s = format!("error calling snapshots_to_delete: {e}"); + error!(log, "{s}"); + status.errors.push(s); + + // Try again next time! + return DeleteResult::WaitForNextActivation; + } + }; + + let request_count = datasets_and_snapshots.len(); + if request_count == 0 { + return DeleteResult::Deleted; + } + + // Send DELETE calls to the corresponding Crucible agents + let mut all_deleted = true; + + for (dataset, region_snapshot) in &datasets_and_snapshots { + let region_id = region_snapshot.region_id; + let snapshot_id = region_snapshot.snapshot_id; + + match self + .delete_crucible_snapshot(log, &dataset, region_id, snapshot_id) + .await + { + Ok(DeleteResult::Deleted) => { + status.snapshot_results.push(format!( + "{} / {}: deleted", + region_id, snapshot_id + )); + } + + Ok(DeleteResult::WaitForNextActivation) => { + status.snapshot_results.push(format!( + "{} / {}: requested deletion, waiting", + region_id, snapshot_id, + )); + + all_deleted = false; + } + + Err(e) => { + status.errors.push(format!("{e}")); + + all_deleted = false; + } + } + } + + // If any of the operations require waiting for the next activation, + // then return that, otherwise return that all were deleted. + + if all_deleted { + // When all crucible resources are cleaned up, hard delete the + // region snapshot records. + + let mut result = DeleteResult::Deleted; + + for (_, region_snapshot) in datasets_and_snapshots { + if let Err(e) = self + .datastore + .region_snapshot_remove( + region_snapshot.dataset_id.into(), + region_snapshot.region_id, + region_snapshot.snapshot_id, + ) + .await + { + let s = + format!("error calling region_snapshot_remove: {e}"); + error!(log, "{s}"); + status.errors.push(s); + + // More work is required next task activation + result = DeleteResult::WaitForNextActivation + } + } + + result + } else { + DeleteResult::WaitForNextActivation + } + } + + async fn clean_up_volume_resources( + &self, + log: &Logger, + crucible_resources_to_delete: &CrucibleResources, + status: &mut VolumeDeleteStatus, + ) -> DeleteResult { + // For any resource that is no longer referenced due to the soft-delete + // of a volume, do the following in order: + // + // - delete top level regions + // - delete running snapshots (read: read-only regions backed by + // snapshots) + // - delete region snapshots + // + // Running snapshots have to be deleted before snapshots are, but + // attempt deleting both regions and running snapshots each task + // activation as they are independent. + + let delete_regions_result = self + .delete_crucible_regions(log, crucible_resources_to_delete, status) + .await; + + let delete_region_snapshots_result = self + .delete_crucible_running_snapshots( + log, + crucible_resources_to_delete, + status, + ) + .await; + + if delete_regions_result == DeleteResult::Deleted + && delete_region_snapshots_result == DeleteResult::Deleted + { + // We're able to continue on with deleting snapshots + } else { + // Otherwise, more work is required from the next task activation. + return DeleteResult::WaitForNextActivation; + } + + // Past this point, running snapshots ar deleted, so delete any + // snapshots. If this is successful, then all crucible resources to + // delete have been cleaned up + + self.delete_crucible_snapshots( + log, + crucible_resources_to_delete, + status, + ) + .await + } + + async fn conditionally_hard_delete_volume_record( + &self, + log: &Logger, + volume_id: VolumeUuid, + status: &mut VolumeDeleteStatus, + ) { + // Do not hard delete the volume record if there are lingering regions + // associated with them. This occurs when a region snapshot hasn't been + // deleted, which means we can't delete the region. Later on, deleting + // the region snapshot when its reference count goes to zero will free + // up the region(s) to be deleted (by delete_freed_crucible_regions). + + let allocated_regions = + match self.datastore.get_allocated_regions(volume_id).await { + Ok(allocated_regions) => allocated_regions, + + Err(e) => { + let s = format!( + "failed to get_allocated_regions for {volume_id}: {e}" + ); + error!(log, "{s}"); + status.errors.push(s); + return; + } + }; + + if !allocated_regions.is_empty() { + info!( + &log, + "allocated regions for {volume_id} is not-empty, skipping \ + hard delete", + ); + + return; + } + + if let Err(e) = self.datastore.volume_hard_delete(volume_id).await { + let s = + format!("failed to volume_hard_delete for {volume_id}: {e}"); + error!(log, "{s}"); + status.errors.push(s); + return; + } + + status.volumes_deleted.push(volume_id.to_string()); + } + + /// Deleting region snapshots may have freed up regions that were deleted in + /// the DB but couldn't previously be deleted by the Crucible Agent because + /// a snapshot existed. Look for those here. + /// + /// It's insufficient to rely on the struct of CrucibleResources to clean + /// up: imagine a disk that is composed of three regions (a subset of the + /// VolumeConstructionRequest is shown here): + /// + /// ```json + /// { + /// "type": "volume", + /// "id": "6b353c87-afac-4ee2-b71a-6fe35fcf9e46", + /// "sub_volumes": [ + /// { + /// "type": "region", + /// "opts": { + /// "targets": [ + /// "[fd00:1122:3344:101::5]:1000", + /// "[fd00:1122:3344:102::9]:1000", + /// "[fd00:1122:3344:103::2]:1000" + /// ], + /// "read_only": false + /// } + /// } + /// ], + /// "read_only_parent": null, + /// } + /// ``` + /// + /// Taking a snapshot of this will produce the following volume: + /// + /// ```json + /// { + /// "type": "volume", + /// "id": "1ef7282e-a3fb-4222-85a8-b16d3fbfd738", <-- new UUID + /// "sub_volumes": [ + /// { + /// "type": "region", + /// "opts": { + /// "targets": [ + /// "[fd00:1122:3344:101::5]:1001", <-- port changed + /// "[fd00:1122:3344:102::9]:1001", <-- port changed + /// "[fd00:1122:3344:103::2]:1001" <-- port changed + /// ], + /// "read_only": true <-- read_only now true + /// } + /// } + /// ], + /// "read_only_parent": null, + /// } + /// ``` + /// + /// The snapshot targets will use the same IP but different port: snapshots + /// are initially located on the same filesystem as their region. + /// + /// The disk's volume has no read only resources, while the snapshot's + /// volume does. The disk volume's targets are all regions (backed by + /// downstairs that are read/write) while the snapshot volume's targets are + /// all snapshots (backed by downstairs that are read-only). The two volumes + /// are linked in the sense that the snapshots from the second are contained + /// *within* the regions of the first, reflecting the resource nesting from + /// ZFS. This is also reflected in the REST endpoint that the Crucible agent + /// uses: + /// + /// /crucible/0/regions/{id}/snapshots/{name} + /// + /// If the disk is then deleted, the CrucibleResources struct returned as + /// from the soft-delete function will contain *nothing* to clean up: the + /// regions contain snapshots that are part of other volumes and cannot be + /// deleted, and the disk's volume doesn't reference any read-only + /// resources. + /// + /// This is expected and normal: regions are "leaked" all the time due to + /// snapshots preventing their deletion. This function detects when those + /// regions can be cleaned up. + /// + /// Note: each delete of a snapshot could trigger another delete of a + /// region, if that region's use has gone to zero. A snapshot delete will + /// never trigger another snapshot delete. + async fn delete_freed_crucible_regions( + &self, + log: &Logger, + status: &mut VolumeDeleteStatus, + ) { + // Find regions freed up for deletion by a previous delete of region + // snapshots. + let freed_datasets_regions_and_volumes = + match self.datastore.find_deleted_volume_regions().await { + Ok(freed_datasets_regions_and_volumes) => { + freed_datasets_regions_and_volumes + } + + Err(e) => { + let s = + format!("failed to find_deleted_volume_regions: {e}"); + error!(log, "{s}"); + status.errors.push(s); + + // Try again next time! + return; + } + }; + + if freed_datasets_regions_and_volumes.is_empty() { + return; + } + + let FreedCrucibleResources { datasets_and_regions, volumes } = + freed_datasets_regions_and_volumes; + + let mut all_deleted = true; + + for (dataset, region) in &datasets_and_regions { + match self.delete_crucible_region(log, &dataset, region.id()).await + { + Ok(DeleteResult::Deleted) => { + status + .region_results + .push(format!("{}: deleted", region.id())); + } + + Ok(DeleteResult::WaitForNextActivation) => { + status.region_results.push(format!( + "{}: requested deletion, waiting", + region.id() + )); + + all_deleted = false; + } + + Err(e) => { + all_deleted = false; + + let s = format!( + "failed delete_crucible_region for {}: {e}", + region.id(), + ); + error!(log, "{s}"); + status.errors.push(s); + } + } + } + + if all_deleted { + // When all crucible resources are cleaned up, hard delete the + // region records. This also re-computes the crucible_dataset + // size_used column for those region's datasets. + + let region_ids_to_delete = + datasets_and_regions.iter().map(|(_, r)| r.id()).collect(); + + match self + .datastore + .regions_hard_delete(log, region_ids_to_delete) + .await + { + Ok(()) => { + // ok + } + + Err(e) => { + let s = format!("error calling regions_hard_delete: {e}"); + error!(log, "{s}"); + status.errors.push(s); + + // More work is required next task activation + return; + } + } + + for volume_id in volumes { + // A Volume returned by `find_deleted_volume_regions` will not + // have read/write regions, so it is safe to delete without + // checking. + + if let Err(e) = + self.datastore.volume_hard_delete(volume_id).await + { + let s = format!( + "error calling volume_hard_delete for {volume_id}: {e}" + ); + error!(log, "{s}"); + status.errors.push(s); + } + } + } else { + // More work is required next task activation + } + } +} + +impl BackgroundTask for VolumeDeleter { + fn activate<'a>( + &'a mut self, + opctx: &'a OpContext, + ) -> BoxFuture<'a, serde_json::Value> { + async { + let log = &opctx.log; + let mut status = VolumeDeleteStatus::default(); + + let soft_deleted_volumes = + match self.datastore.get_soft_deleted_volumes(opctx).await { + Ok(v) => v, + Err(e) => { + let s = format!( + "error calling get_soft_deleted_volumes: {e}" + ); + error!(log, "{s}"); + status.errors.push(s); + return json!(status); + } + }; + + for volume in soft_deleted_volumes { + let Some(resources_to_clean_up) = &volume.resources_to_clean_up + else { + let s = format!( + "volume {} has no resources to clean up", + volume.id(), + ); + error!(log, "{s}"); + status.errors.push(s); + continue; + }; + + let resources_to_clean_up: CrucibleResources = + match serde_json::from_str(&resources_to_clean_up) { + Ok(v) => v, + Err(e) => { + let s = format!( + "volume {} resources to clean up did not \ + deserialize: {e}", + volume.id(), + ); + error!(log, "{s}"); + status.errors.push(s); + continue; + } + }; + + match self + .clean_up_volume_resources( + log, + &resources_to_clean_up, + &mut status, + ) + .await + { + DeleteResult::Deleted => { + self.conditionally_hard_delete_volume_record( + log, + volume.id(), + &mut status, + ) + .await; + } + + DeleteResult::WaitForNextActivation => { + continue; + } + } + } + + self.delete_freed_crucible_regions(log, &mut status).await; + + json!(status) + } + .boxed() + } +} + +fn is_not_found( + e: &crucible_agent_client::Error, +) -> bool { + match e { + crucible_agent_client::Error::ErrorResponse(rv) => match rv.status() { + http::StatusCode::NOT_FOUND => true, + _ => false, + }, + + _ => false, + } +} + +fn into_external_error( + e: &crucible_agent_client::Error, +) -> Error { + match e { + crucible_agent_client::Error::ErrorResponse(rv) => { + if rv.status().is_client_error() { + Error::invalid_request(&rv.message) + } else { + Error::internal_error(&rv.message) + } + } + + _ => Error::internal_error(format!("unexpected failure: {e}")), + } +} diff --git a/nexus/src/app/disk.rs b/nexus/src/app/disk.rs index 7fe564a6b5b..42bb2246220 100644 --- a/nexus/src/app/disk.rs +++ b/nexus/src/app/disk.rs @@ -371,13 +371,23 @@ impl super::Nexus { let saga_params = sagas::disk_delete::Params { serialized_authn: authn::saga::Serialized::for_opctx(opctx), project_id: project.id(), - disk, + disk: disk.clone(), }; self.sagas .saga_execute::(saga_params) .await?; + match disk { + datastore::Disk::Crucible(_) => { + self.background_tasks.task_volume_delete.activate(); + } + + datastore::Disk::LocalStorage(_) => { + self.background_tasks.task_local_storage_delete.activate(); + } + } + Ok(()) } @@ -400,6 +410,8 @@ impl super::Nexus { datastore::Disk::Crucible(disk) => { self.volume_remove_read_only_parent(&opctx, disk.volume_id()) .await?; + + self.background_tasks.task_volume_delete.activate(); } datastore::Disk::LocalStorage(_) => { diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 76ab0725dab..16d5b6b394e 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -192,6 +192,8 @@ impl super::Nexus { .saga_execute::(saga_params) .await?; + self.background_tasks.task_volume_delete.activate(); + Ok(()) } diff --git a/nexus/src/app/sagas/disk_create.rs b/nexus/src/app/sagas/disk_create.rs index 307c6825d86..349adfa75e5 100644 --- a/nexus/src/app/sagas/disk_create.rs +++ b/nexus/src/app/sagas/disk_create.rs @@ -1073,6 +1073,7 @@ pub(crate) mod test { use nexus_db_queries::context::OpContext; use nexus_db_queries::db; use nexus_db_queries::db::datastore::DataStore; + use nexus_test_utils::background::wait_for_all_volume_deletes; use nexus_test_utils::resource_helpers; use nexus_test_utils::resource_helpers::create_project; use nexus_test_utils_macros::nexus_test; @@ -1608,6 +1609,11 @@ pub(crate) mod test { .await; destroy_disk(&cptestctx).await; + wait_for_all_volume_deletes( + nexus.datastore(), + &cptestctx.lockstep_client, + ) + .await; verify_clean_slate(&cptestctx, &test).await; } diff --git a/nexus/src/app/sagas/disk_delete.rs b/nexus/src/app/sagas/disk_delete.rs index 271852fe0ca..5070522f324 100644 --- a/nexus/src/app/sagas/disk_delete.rs +++ b/nexus/src/app/sagas/disk_delete.rs @@ -5,27 +5,16 @@ use super::ActionRegistry; use super::NexusActionContext; use super::NexusSaga; -use crate::app::InlineErrorChain; -use crate::app::sagas::SagaInitError; use crate::app::sagas::declare_saga_actions; -use crate::app::sagas::sled_out_of_service_gone_check; -use crate::app::sagas::volume_delete; -use crate::app::sagas::zpool_out_of_service_gone_check; use nexus_db_queries::authn; use nexus_db_queries::db; use nexus_db_queries::db::datastore; use nexus_types::saga::saga_action_failed; use omicron_common::api::external::DiskState; use omicron_common::api::external::Error; -use omicron_common::backoff::backon_retry_policy_internal_service; -use progenitor_extras::retry::{ - GoneCheckResult, retry_operation_while_indefinitely, -}; use serde::Deserialize; use serde::Serialize; -use sled_agent_client::types::LocalStorageDatasetDeleteRequest; use steno::ActionError; -use steno::Node; use uuid::Uuid; // disk delete saga: input parameters @@ -49,11 +38,8 @@ declare_saga_actions! { + sdd_account_space - sdd_account_space_undo } - DELETE_LOCAL_STORAGE -> "delete_local_storage" { - + sdd_delete_local_storage - } - DEALLOCATE_LOCAL_STORAGE -> "deallocate_local_storage" { - + sdd_deallocate_local_storage + SOFT_DELETE_CRUCIBLE_VOLUME -> "soft_delete_volume" { + + sdd_soft_delete_volume } } @@ -77,47 +63,13 @@ impl NexusSaga for SagaDiskDelete { builder.append(space_account_action()); match ¶ms.disk { - datastore::Disk::Crucible(disk) => { - let subsaga_params = volume_delete::Params { - serialized_authn: params.serialized_authn.clone(), - volume_id: disk.volume_id(), - }; - - let subsaga_dag = { - let subsaga_builder = - steno::DagBuilder::new(steno::SagaName::new( - volume_delete::SagaVolumeDelete::NAME, - )); - volume_delete::SagaVolumeDelete::make_saga_dag( - &subsaga_params, - subsaga_builder, - )? - }; - - builder.append(Node::constant( - "params_for_volume_delete_subsaga", - serde_json::to_value(&subsaga_params).map_err(|e| { - SagaInitError::SerializeError( - "params_for_volume_delete_subsaga".to_string(), - e, - ) - })?, - )); - - builder.append(Node::subsaga( - "volume_delete_subsaga_no_result", - subsaga_dag, - "params_for_volume_delete_subsaga", - )); + datastore::Disk::Crucible(_) => { + builder.append(soft_delete_crucible_volume_action()); } datastore::Disk::LocalStorage(_) => { - // Attempt deleting the local storage before removing the - // database record. If the delete does not succeed, at least the - // user can re-request the deletion. - - builder.append(delete_local_storage_action()); - builder.append(deallocate_local_storage_action()); + // Local storage clean up is done entirely in the + // local_storage_delete background task } } @@ -208,127 +160,27 @@ async fn sdd_account_space_undo( Ok(()) } -async fn sdd_delete_local_storage( +async fn sdd_soft_delete_volume( sagactx: NexusActionContext, ) -> Result<(), ActionError> { - let osagactx = sagactx.user_data(); let params = sagactx.saga_params::()?; - let opctx = crate::context::op_context_for_saga_action( - &sagactx, - ¶ms.serialized_authn, - ); - - let datastore::Disk::LocalStorage(disk) = params.disk else { - unreachable!( - "check during `make_saga_dag` should have ensured disk type is \ - local storage" - ); - }; - - let Some(allocation) = disk.local_storage_dataset_allocation else { - // Nothing to do! - return Ok(()); - }; - - let sled_id = allocation.sled_id(); - let zpool_id = allocation.pool_id().upcast(); - - let request = LocalStorageDatasetDeleteRequest { - zpool_id: allocation.pool_id(), - dataset_id: allocation.id(), - encrypted_at_rest: allocation.encrypted_at_rest(), - }; - - // Get a sled agent client - - let sled_agent_client = osagactx - .nexus() - .sled_client(&sled_id) - .await - .map_err(saga_action_failed)?; - - // Ensure that the local storage is deleted - - let delete_operation = || async { - sled_agent_client.local_storage_dataset_delete(&request).await - }; - - // Bail out of the retry loop if either the disk or sled is no longer - // in-service. - let gone_check = || async { - match sled_out_of_service_gone_check( - osagactx.datastore(), - &opctx, - sled_id, - ) - .await? - { - GoneCheckResult::StillAvailable => { - // proceed to zpool check - } - - GoneCheckResult::Gone => { - return Ok(GoneCheckResult::Gone); - } - } - - zpool_out_of_service_gone_check(osagactx.datastore(), &opctx, zpool_id) - .await - }; - - let log = osagactx.log().clone(); - let result = retry_operation_while_indefinitely( - backon_retry_policy_internal_service(), - delete_operation, - gone_check, - |notification| { - slog::warn!( - log, - "failed to delete local storage dataset, retrying in {:?}", - notification.delay; - InlineErrorChain::new(¬ification.error), - ); - }, - ) - .await; - - match result { - Ok(_) => Ok(()), - - // In this case, if the particular disk hosting this local storage was - // expunged, or if the sled was expunged, then proceed with the rest of - // the saga. - Err(e) if e.is_gone() => Ok(()), - - Err(e) => Err(saga_action_failed(Error::internal_error(&format!( - "failed to delete local storage: {}", - InlineErrorChain::new(&e) - )))), - } -} - -async fn sdd_deallocate_local_storage( - sagactx: NexusActionContext, -) -> Result<(), ActionError> { let osagactx = sagactx.user_data(); - let params = sagactx.saga_params::()?; - let opctx = crate::context::op_context_for_saga_action( - &sagactx, - ¶ms.serialized_authn, - ); - let datastore::Disk::LocalStorage(disk) = params.disk else { + let datastore::Disk::Crucible(disk) = params.disk else { unreachable!( "check during `make_saga_dag` should have ensured disk type is \ - local storage" + crucible" ); }; - osagactx - .datastore() - .delete_local_storage_dataset_allocations(&opctx, &disk) - .await - .map_err(saga_action_failed)?; + osagactx.datastore().soft_delete_volume(disk.volume_id()).await.map_err( + |e| { + saga_action_failed(Error::internal_error(&format!( + "failed to soft_delete_volume: {:?}", + e, + ))) + }, + )?; Ok(()) } @@ -353,6 +205,8 @@ pub(crate) mod test { use nexus_db_queries::authz; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::datastore::Disk; + use nexus_test_utils::background::wait_for_all_local_storage_deletes; + use nexus_test_utils::background::wait_for_all_volume_deletes; use nexus_test_utils::resource_helpers::DiskTest; use nexus_test_utils::resource_helpers::create_project; use nexus_test_utils_macros::nexus_test; @@ -477,6 +331,12 @@ pub(crate) mod test { ) .await; + wait_for_all_volume_deletes( + nexus.datastore(), + &cptestctx.lockstep_client, + ) + .await; + crate::app::sagas::disk_create::test::verify_clean_slate( &cptestctx, &test, ) @@ -641,6 +501,16 @@ pub(crate) mod test { let nexus = &self.cptestctx.server.server_context().nexus; let datastore = nexus.datastore(); + // Run the local storage delete background task to completion + + wait_for_all_local_storage_deletes( + &datastore, + &self.cptestctx.lockstep_client, + ) + .await; + + // Then check all allocations were deleted + let conn = datastore.pool_connection_for_tests().await.unwrap(); use nexus_db_schema::schema::local_storage_unencrypted_dataset_allocation::dsl; diff --git a/nexus/src/app/sagas/image_delete.rs b/nexus/src/app/sagas/image_delete.rs index 7ace0f5ef88..b6c06074761 100644 --- a/nexus/src/app/sagas/image_delete.rs +++ b/nexus/src/app/sagas/image_delete.rs @@ -3,15 +3,14 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use super::{ActionRegistry, NexusActionContext, NexusSaga}; -use crate::app::sagas; use crate::app::sagas::declare_saga_actions; use nexus_db_queries::{authn, authz, db}; use nexus_types::saga::saga_action_failed; +use omicron_common::api::external::Error; use omicron_uuid_kinds::VolumeUuid; use serde::Deserialize; use serde::Serialize; use steno::ActionError; -use steno::Node; #[derive(Debug, Deserialize, Serialize)] pub(crate) enum ImageParam { @@ -38,6 +37,9 @@ pub(crate) struct Params { declare_saga_actions! { image_delete; + SOFT_DELETE_VOLUME -> "soft_delete_volume" { + + sid_soft_delete_volume + } DELETE_IMAGE_RECORD -> "no_result1" { + sid_delete_image_record } @@ -54,38 +56,11 @@ impl NexusSaga for SagaImageDelete { } fn make_saga_dag( - params: &Self::Params, + _params: &Self::Params, mut builder: steno::DagBuilder, ) -> Result { builder.append(delete_image_record_action()); - - const DELETE_VOLUME_PARAMS: &'static str = "delete_volume_params"; - - let volume_delete_params = sagas::volume_delete::Params { - serialized_authn: params.serialized_authn.clone(), - volume_id: params.image_param.volume_id(), - }; - builder.append(Node::constant( - DELETE_VOLUME_PARAMS, - serde_json::to_value(&volume_delete_params).map_err(|e| { - super::SagaInitError::SerializeError( - String::from("volume_id"), - e, - ) - })?, - )); - - let make_volume_delete_dag = || { - let subsaga_builder = steno::DagBuilder::new(steno::SagaName::new( - sagas::volume_delete::SagaVolumeDelete::NAME, - )); - sagas::volume_delete::create_dag(subsaga_builder) - }; - builder.append(steno::Node::subsaga( - "delete_volume", - make_volume_delete_dag()?, - DELETE_VOLUME_PARAMS, - )); + builder.append(soft_delete_volume_action()); Ok(builder.build()?) } @@ -123,3 +98,23 @@ async fn sid_delete_image_record( Ok(()) } + +async fn sid_soft_delete_volume( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let params = sagactx.saga_params::()?; + let osagactx = sagactx.user_data(); + + osagactx + .datastore() + .soft_delete_volume(params.image_param.volume_id()) + .await + .map_err(|e| { + saga_action_failed(Error::internal_error(&format!( + "failed to soft_delete_volume: {:?}", + e, + ))) + })?; + + Ok(()) +} diff --git a/nexus/src/app/sagas/mod.rs b/nexus/src/app/sagas/mod.rs index 6460974475d..0005fb07ff2 100644 --- a/nexus/src/app/sagas/mod.rs +++ b/nexus/src/app/sagas/mod.rs @@ -59,7 +59,6 @@ pub mod snapshot_delete; pub mod subnet_attach; pub mod subnet_detach; pub mod test_saga; -pub mod volume_delete; pub mod volume_remove_rop; pub mod vpc_create; pub mod vpc_subnet_create; @@ -193,7 +192,6 @@ fn make_action_registry() -> ActionRegistry { snapshot_delete::SagaSnapshotDelete, subnet_attach::SagaSubnetAttach, subnet_detach::SagaSubnetDetach, - volume_delete::SagaVolumeDelete, volume_remove_rop::SagaVolumeRemoveROP, vpc_create::SagaVpcCreate, vpc_subnet_create::SagaVpcSubnetCreate, diff --git a/nexus/src/app/sagas/region_replacement_finish.rs b/nexus/src/app/sagas/region_replacement_finish.rs index 73edff9c71c..1dfe55ccc84 100644 --- a/nexus/src/app/sagas/region_replacement_finish.rs +++ b/nexus/src/app/sagas/region_replacement_finish.rs @@ -23,8 +23,7 @@ //! It will set itself as the "operating saga" for a region replacement request, //! change the state to "Completing", and: //! -//! 1. Call the Volume delete saga for the fake Volume that points to the old -//! region. +//! 1. Soft-delete the fake Volume that points to the old region. //! //! 2. Clear the operating saga id from the request record, and change the state //! to Completed. @@ -35,9 +34,9 @@ use super::{ SagaInitError, }; use crate::app::sagas::declare_saga_actions; -use crate::app::sagas::volume_delete; use crate::app::{authn, db}; use nexus_types::saga::saga_action_failed; +use omicron_common::api::external::Error; use omicron_uuid_kinds::VolumeUuid; use serde::Deserialize; use serde::Serialize; @@ -65,6 +64,9 @@ declare_saga_actions! { + srrf_set_saga_id - srrf_set_saga_id_undo } + SOFT_DELETE_VOLUME -> "soft_delete_volume" { + + srrf_soft_delete_volume + } UPDATE_REQUEST_RECORD -> "unused_2" { + srrf_update_request_record } @@ -83,7 +85,7 @@ impl NexusSaga for SagaRegionReplacementFinish { } fn make_saga_dag( - params: &Self::Params, + _params: &Self::Params, mut builder: steno::DagBuilder, ) -> Result { builder.append(Node::action( @@ -93,38 +95,7 @@ impl NexusSaga for SagaRegionReplacementFinish { )); builder.append(set_saga_id_action()); - - let subsaga_params = volume_delete::Params { - serialized_authn: params.serialized_authn.clone(), - volume_id: params.region_volume_id, - }; - - let subsaga_dag = { - let subsaga_builder = steno::DagBuilder::new(steno::SagaName::new( - volume_delete::SagaVolumeDelete::NAME, - )); - volume_delete::SagaVolumeDelete::make_saga_dag( - &subsaga_params, - subsaga_builder, - )? - }; - - builder.append(Node::constant( - "params_for_volume_delete_subsaga", - serde_json::to_value(&subsaga_params).map_err(|e| { - SagaInitError::SerializeError( - "params_for_volume_delete_subsaga".to_string(), - e, - ) - })?, - )); - - builder.append(Node::subsaga( - "volume_delete_subsaga_no_result", - subsaga_dag, - "params_for_volume_delete_subsaga", - )); - + builder.append(soft_delete_volume_action()); builder.append(update_request_record_action()); Ok(builder.build()?) @@ -181,6 +152,26 @@ async fn srrf_set_saga_id_undo( Ok(()) } +async fn srrf_soft_delete_volume( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let params = sagactx.saga_params::()?; + let osagactx = sagactx.user_data(); + + osagactx + .datastore() + .soft_delete_volume(params.region_volume_id) + .await + .map_err(|e| { + saga_action_failed(Error::internal_error(&format!( + "failed to soft_delete_volume: {:?}", + e, + ))) + })?; + + Ok(()) +} + async fn srrf_update_request_record( sagactx: NexusActionContext, ) -> Result<(), ActionError> { @@ -218,6 +209,7 @@ pub(crate) mod test { use nexus_db_model::RegionReplacementState; use nexus_db_queries::authn::saga::Serialized; use nexus_db_queries::context::OpContext; + use nexus_test_utils::background::wait_for_all_volume_deletes; use nexus_test_utils_macros::nexus_test; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::GenericUuid; @@ -349,7 +341,11 @@ pub(crate) mod test { assert_eq!(result.replacement_state, RegionReplacementState::Complete); assert!(result.operating_saga_id.is_none()); - // Validate the Volume was deleted + // Run the volume delete background task and validate the Volume was + // deleted. + wait_for_all_volume_deletes(datastore, &cptestctx.lockstep_client) + .await; + assert!( datastore.volume_get(old_region_volume_id).await.unwrap().is_none() ); diff --git a/nexus/src/app/sagas/region_snapshot_replacement_finish.rs b/nexus/src/app/sagas/region_snapshot_replacement_finish.rs index d61cf11f25d..4b8bce12bc2 100644 --- a/nexus/src/app/sagas/region_snapshot_replacement_finish.rs +++ b/nexus/src/app/sagas/region_snapshot_replacement_finish.rs @@ -23,10 +23,9 @@ //! ``` //! //! The first thing this saga does is set itself as the "operating saga" for the -//! request, and change the state to "Completing". Then, it performs the volume -//! delete sub-saga for the new region volume. Finally, it updates the region -//! snapshot replacement request by clearing the operating saga id and changing -//! the state to "Complete". +//! request, and change the state to "Completing". Then, it soft-deletes the new +//! region volume. Finally, it updates the region snapshot replacement request +//! by clearing the operating saga id and changing the state to "Complete". //! //! Any unwind will place the state back into Running. @@ -35,9 +34,9 @@ use super::{ SagaInitError, }; use crate::app::sagas::declare_saga_actions; -use crate::app::sagas::volume_delete; use crate::app::{authn, db}; use nexus_types::saga::saga_action_failed; +use omicron_common::api::external::Error; use serde::Deserialize; use serde::Serialize; use steno::ActionError; @@ -60,6 +59,9 @@ declare_saga_actions! { + rsrfs_set_saga_id - rsrfs_set_saga_id_undo } + SOFT_DELETE_VOLUME -> "soft_delete_volume" { + + rsrfs_soft_delete_volume + } UPDATE_REQUEST_RECORD -> "unused_4" { + rsrfs_update_request_record } @@ -78,7 +80,7 @@ impl NexusSaga for SagaRegionSnapshotReplacementFinish { } fn make_saga_dag( - params: &Self::Params, + _params: &Self::Params, mut builder: steno::DagBuilder, ) -> Result { builder.append(Node::action( @@ -88,42 +90,7 @@ impl NexusSaga for SagaRegionSnapshotReplacementFinish { )); builder.append(set_saga_id_action()); - - if let Some(new_region_volume_id) = - params.request.new_region_volume_id() - { - let subsaga_params = volume_delete::Params { - serialized_authn: params.serialized_authn.clone(), - volume_id: new_region_volume_id, - }; - - let subsaga_dag = { - let subsaga_builder = steno::DagBuilder::new( - steno::SagaName::new(volume_delete::SagaVolumeDelete::NAME), - ); - volume_delete::SagaVolumeDelete::make_saga_dag( - &subsaga_params, - subsaga_builder, - )? - }; - - builder.append(Node::constant( - "params_for_volume_delete_subsaga", - serde_json::to_value(&subsaga_params).map_err(|e| { - SagaInitError::SerializeError( - "params_for_volume_delete_subsaga".to_string(), - e, - ) - })?, - )); - - builder.append(Node::subsaga( - "volume_delete_subsaga_no_result", - subsaga_dag, - "params_for_volume_delete_subsaga", - )); - } - + builder.append(soft_delete_volume_action()); builder.append(update_request_record_action()); Ok(builder.build()?) @@ -184,6 +151,28 @@ async fn rsrfs_set_saga_id_undo( Ok(()) } +async fn rsrfs_soft_delete_volume( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let params = sagactx.saga_params::()?; + let osagactx = sagactx.user_data(); + + if let Some(new_region_volume_id) = params.request.new_region_volume_id() { + osagactx + .datastore() + .soft_delete_volume(new_region_volume_id) + .await + .map_err(|e| { + saga_action_failed(Error::internal_error(&format!( + "failed to soft_delete_volume: {:?}", + e, + ))) + })?; + } + + Ok(()) +} + async fn rsrfs_update_request_record( sagactx: NexusActionContext, ) -> Result<(), ActionError> { diff --git a/nexus/src/app/sagas/region_snapshot_replacement_garbage_collect.rs b/nexus/src/app/sagas/region_snapshot_replacement_garbage_collect.rs index d1ebdd4cc3e..c01932af6eb 100644 --- a/nexus/src/app/sagas/region_snapshot_replacement_garbage_collect.rs +++ b/nexus/src/app/sagas/region_snapshot_replacement_garbage_collect.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -//! Clean up the volume that stashes the target replaced during the region +//! Soft-delete up the volume that stashes the target replaced during the region //! snapshot replacement start saga. After that's done, change the region //! snapshot replacement state to Running. This saga handles the following //! region snapshot replacement request state transitions: @@ -29,9 +29,9 @@ use super::{ SagaInitError, }; use crate::app::sagas::declare_saga_actions; -use crate::app::sagas::volume_delete; use crate::app::{authn, db}; use nexus_types::saga::saga_action_failed; +use omicron_common::api::external::Error; use omicron_uuid_kinds::VolumeUuid; use serde::Deserialize; use serde::Serialize; @@ -59,6 +59,9 @@ declare_saga_actions! { + rsrgs_set_saga_id - rsrgs_set_saga_id_undo } + SOFT_DELETE_VOLUME -> "soft_delete_volume" { + + rsrgs_soft_delete_volume + } UPDATE_REQUEST_RECORD -> "unused_2" { + rsrgs_update_request_record } @@ -77,7 +80,7 @@ impl NexusSaga for SagaRegionSnapshotReplacementGarbageCollect { } fn make_saga_dag( - params: &Self::Params, + _params: &Self::Params, mut builder: steno::DagBuilder, ) -> Result { builder.append(Node::action( @@ -87,38 +90,7 @@ impl NexusSaga for SagaRegionSnapshotReplacementGarbageCollect { )); builder.append(set_saga_id_action()); - - let subsaga_params = volume_delete::Params { - serialized_authn: params.serialized_authn.clone(), - volume_id: params.old_snapshot_volume_id, - }; - - let subsaga_dag = { - let subsaga_builder = steno::DagBuilder::new(steno::SagaName::new( - volume_delete::SagaVolumeDelete::NAME, - )); - volume_delete::SagaVolumeDelete::make_saga_dag( - &subsaga_params, - subsaga_builder, - )? - }; - - builder.append(Node::constant( - "params_for_volume_delete_subsaga", - serde_json::to_value(&subsaga_params).map_err(|e| { - SagaInitError::SerializeError( - "params_for_volume_delete_subsaga".to_string(), - e, - ) - })?, - )); - - builder.append(Node::subsaga( - "volume_delete_subsaga_no_result", - subsaga_dag, - "params_for_volume_delete_subsaga", - )); - + builder.append(soft_delete_volume_action()); builder.append(update_request_record_action()); Ok(builder.build()?) @@ -180,6 +152,26 @@ async fn rsrgs_set_saga_id_undo( Ok(()) } +async fn rsrgs_soft_delete_volume( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let params = sagactx.saga_params::()?; + let osagactx = sagactx.user_data(); + + osagactx + .datastore() + .soft_delete_volume(params.old_snapshot_volume_id) + .await + .map_err(|e| { + saga_action_failed(Error::internal_error(&format!( + "failed to soft_delete_volume: {:?}", + e, + ))) + })?; + + Ok(()) +} + async fn rsrgs_update_request_record( sagactx: NexusActionContext, ) -> Result<(), ActionError> { @@ -218,6 +210,7 @@ pub(crate) mod test { use nexus_db_model::RegionSnapshotReplacementState; use nexus_db_queries::authn::saga::Serialized; use nexus_db_queries::context::OpContext; + use nexus_test_utils::background::wait_for_all_volume_deletes; use nexus_test_utils_macros::nexus_test; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::GenericUuid; @@ -319,7 +312,11 @@ pub(crate) mod test { RegionSnapshotReplacementState::Running ); - // Validate the Volume was deleted + // Run the volume delete background task and validate the Volume was + // deleted. + wait_for_all_volume_deletes(datastore, &cptestctx.lockstep_client) + .await; + assert!( datastore .volume_get(old_snapshot_volume_id) diff --git a/nexus/src/app/sagas/region_snapshot_replacement_step_garbage_collect.rs b/nexus/src/app/sagas/region_snapshot_replacement_step_garbage_collect.rs index c508fb01a8e..e97d248b4dc 100644 --- a/nexus/src/app/sagas/region_snapshot_replacement_step_garbage_collect.rs +++ b/nexus/src/app/sagas/region_snapshot_replacement_step_garbage_collect.rs @@ -2,20 +2,19 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -//! Delete the volume that stashes the target replaced during a region snapshot -//! replacement step saga. After that's done, change the region snapshot -//! replacement step's state to "VolumeDeleted". +//! Soft-delete the volume that stashes the target replaced during a region +//! snapshot replacement step saga. After that's done, change the region +//! snapshot replacement step's state to "VolumeDeleted". use super::{ActionRegistry, NexusActionContext, NexusSaga, SagaInitError}; use crate::app::sagas::declare_saga_actions; -use crate::app::sagas::volume_delete; use crate::app::{authn, db}; use nexus_types::saga::saga_action_failed; +use omicron_common::api::external::Error; use omicron_uuid_kinds::VolumeUuid; use serde::Deserialize; use serde::Serialize; use steno::ActionError; -use steno::Node; // region snapshot replacement step garbage collect saga: input parameters @@ -33,6 +32,9 @@ pub(crate) struct Params { declare_saga_actions! { region_snapshot_replacement_step_garbage_collect; + SOFT_DELETE_VOLUME -> "soft_delete_volume" { + + srsgs_soft_delete_volume + } UPDATE_REQUEST_RECORD -> "unused_1" { + srsgs_update_request_record } @@ -54,40 +56,10 @@ impl NexusSaga for SagaRegionSnapshotReplacementStepGarbageCollect { } fn make_saga_dag( - params: &Self::Params, + _params: &Self::Params, mut builder: steno::DagBuilder, ) -> Result { - let subsaga_params = volume_delete::Params { - serialized_authn: params.serialized_authn.clone(), - volume_id: params.old_snapshot_volume_id, - }; - - let subsaga_dag = { - let subsaga_builder = steno::DagBuilder::new(steno::SagaName::new( - volume_delete::SagaVolumeDelete::NAME, - )); - volume_delete::SagaVolumeDelete::make_saga_dag( - &subsaga_params, - subsaga_builder, - )? - }; - - builder.append(Node::constant( - "params_for_volume_delete_subsaga", - serde_json::to_value(&subsaga_params).map_err(|e| { - SagaInitError::SerializeError( - "params_for_volume_delete_subsaga".to_string(), - e, - ) - })?, - )); - - builder.append(Node::subsaga( - "volume_delete_subsaga_no_result", - subsaga_dag, - "params_for_volume_delete_subsaga", - )); - + builder.append(soft_delete_volume_action()); builder.append(update_request_record_action()); Ok(builder.build()?) @@ -96,6 +68,26 @@ impl NexusSaga for SagaRegionSnapshotReplacementStepGarbageCollect { // region snapshot replacement step garbage collect saga: action implementations +async fn srsgs_soft_delete_volume( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let params = sagactx.saga_params::()?; + let osagactx = sagactx.user_data(); + + osagactx + .datastore() + .soft_delete_volume(params.old_snapshot_volume_id) + .await + .map_err(|e| { + saga_action_failed(Error::internal_error(&format!( + "failed to soft_delete_volume: {:?}", + e, + ))) + })?; + + Ok(()) +} + async fn srsgs_update_request_record( sagactx: NexusActionContext, ) -> Result<(), ActionError> { @@ -130,6 +122,7 @@ pub(crate) mod test { use nexus_db_queries::authn::saga::Serialized; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::datastore::region_snapshot_replacement; + use nexus_test_utils::background::wait_for_all_volume_deletes; use nexus_test_utils_macros::nexus_test; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::VolumeUuid; @@ -243,7 +236,11 @@ pub(crate) mod test { RegionSnapshotReplacementStepState::VolumeDeleted ); - // Validate the Volume was deleted + // Run the volume delete background task and validate the Volume was + // deleted. + wait_for_all_volume_deletes(datastore, &cptestctx.lockstep_client) + .await; + assert!( datastore .volume_get(old_snapshot_volume_id) diff --git a/nexus/src/app/sagas/snapshot_create.rs b/nexus/src/app/sagas/snapshot_create.rs index 21fa49b903c..f5f1a3fa342 100644 --- a/nexus/src/app/sagas/snapshot_create.rs +++ b/nexus/src/app/sagas/snapshot_create.rs @@ -1784,6 +1784,7 @@ mod test { use nexus_db_queries::db::DataStore; use nexus_db_queries::db::datastore::Disk; use nexus_db_queries::db::datastore::InstanceAndActiveVmm; + use nexus_test_utils::background::wait_for_all_volume_deletes; use nexus_test_utils::resource_helpers::create_default_ip_pools; use nexus_test_utils::resource_helpers::create_disk; use nexus_test_utils::resource_helpers::create_project; @@ -2410,6 +2411,10 @@ mod test { } delete_disk(client, PROJECT_NAME, DISK_NAME).await; + wait_for_all_volume_deletes( + nexus.datastore(), + &cptestctx.lockstep_client, + ).await; verify_clean_slate(cptestctx, &test).await; }) }, diff --git a/nexus/src/app/sagas/snapshot_delete.rs b/nexus/src/app/sagas/snapshot_delete.rs index a9e2f2f42af..605390e1c40 100644 --- a/nexus/src/app/sagas/snapshot_delete.rs +++ b/nexus/src/app/sagas/snapshot_delete.rs @@ -3,14 +3,13 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use super::{ActionRegistry, NexusActionContext, NexusSaga}; -use crate::app::sagas; use crate::app::sagas::declare_saga_actions; use nexus_db_queries::{authn, authz, db}; use nexus_types::saga::saga_action_failed; +use omicron_common::api::external::Error; use serde::Deserialize; use serde::Serialize; use steno::ActionError; -use steno::Node; #[derive(Debug, Deserialize, Serialize)] pub(crate) struct Params { @@ -27,8 +26,11 @@ declare_saga_actions! { SPACE_ACCOUNT -> "no_result2" { + ssd_account_space } - NOOP -> "no_result3" { - + ssd_noop + SOFT_DELETE_VOLUME -> "soft_delete_volume" { + + ssd_soft_delete_volume + } + SOFT_DELETE_DEST_VOLUME -> "soft_delete_dest_volume" { + + ssd_soft_delete_dest_volume } } @@ -43,65 +45,13 @@ impl NexusSaga for SagaSnapshotDelete { } fn make_saga_dag( - params: &Self::Params, + _params: &Self::Params, mut builder: steno::DagBuilder, ) -> Result { builder.append(delete_snapshot_record_action()); builder.append(space_account_action()); - - const DELETE_VOLUME_PARAMS: &'static str = "delete_volume_params"; - const DELETE_VOLUME_DESTINATION_PARAMS: &'static str = - "delete_volume_destination_params"; - - let volume_delete_params = sagas::volume_delete::Params { - serialized_authn: params.serialized_authn.clone(), - volume_id: params.snapshot.volume_id(), - }; - builder.append(Node::constant( - DELETE_VOLUME_PARAMS, - serde_json::to_value(&volume_delete_params).map_err(|e| { - super::SagaInitError::SerializeError( - String::from("volume_id"), - e, - ) - })?, - )); - - let volume_delete_params = sagas::volume_delete::Params { - serialized_authn: params.serialized_authn.clone(), - volume_id: params.snapshot.destination_volume_id(), - }; - builder.append(Node::constant( - DELETE_VOLUME_DESTINATION_PARAMS, - serde_json::to_value(&volume_delete_params).map_err(|e| { - super::SagaInitError::SerializeError( - String::from("destination_volume_id"), - e, - ) - })?, - )); - - let make_volume_delete_dag = || { - let subsaga_builder = steno::DagBuilder::new(steno::SagaName::new( - sagas::volume_delete::SagaVolumeDelete::NAME, - )); - sagas::volume_delete::create_dag(subsaga_builder) - }; - - builder.append_parallel(vec![ - steno::Node::subsaga( - "delete_volume", - make_volume_delete_dag()?, - DELETE_VOLUME_PARAMS, - ), - steno::Node::subsaga( - "delete_destination_volume", - make_volume_delete_dag()?, - DELETE_VOLUME_DESTINATION_PARAMS, - ), - ]); - - builder.append(noop_action()); + builder.append(soft_delete_volume_action()); + builder.append(soft_delete_dest_volume_action()); Ok(builder.build()?) } @@ -158,7 +108,42 @@ async fn ssd_account_space( Ok(()) } -// Sagas must end in one node, not parallel -async fn ssd_noop(_sagactx: NexusActionContext) -> Result<(), ActionError> { +async fn ssd_soft_delete_volume( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let params = sagactx.saga_params::()?; + let osagactx = sagactx.user_data(); + + osagactx + .datastore() + .soft_delete_volume(params.snapshot.volume_id()) + .await + .map_err(|e| { + saga_action_failed(Error::internal_error(&format!( + "failed to soft_delete_volume: {:?}", + e, + ))) + })?; + + Ok(()) +} + +async fn ssd_soft_delete_dest_volume( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let params = sagactx.saga_params::()?; + let osagactx = sagactx.user_data(); + + osagactx + .datastore() + .soft_delete_volume(params.snapshot.destination_volume_id()) + .await + .map_err(|e| { + saga_action_failed(Error::internal_error(&format!( + "failed to soft_delete_volume: {:?}", + e, + ))) + })?; + Ok(()) } diff --git a/nexus/src/app/sagas/volume_delete.rs b/nexus/src/app/sagas/volume_delete.rs deleted file mode 100644 index b02e7184b8d..00000000000 --- a/nexus/src/app/sagas/volume_delete.rs +++ /dev/null @@ -1,541 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! Nexus is responsible for telling Crucible Agent(s) when to clean up -//! resources - those Agents do not have any idea of what volumes are -//! constructed, currently active, etc. Plus, volumes can (and will) change -//! during their lifetime. Operations like growing a disk, removing a read-only -//! parent after a scrub has completed, or re-encrypting a disk will all change -//! the volume that backs a disk. -//! -//! Nexus has to account for all the Crucible resources it is using, and count -//! how many volumes are using those resources. Only when that count drops to -//! zero is it valid to clean up the appropriate Crucible resource. -//! -//! Complicating things is the fact that ZFS datasets cannot be deleted if there -//! are snapshots of that dataset. Nexus' resource accounting must take this -//! dependency into account. Note that ZFS snapshots can layer, but any snapshot -//! can be deleted without the requirement of (for example) deleting the -//! snapshots in a certain order. -//! -//! One problem to solve is doing this idempotently. Volumes reference Crucible -//! resources, and when they are inserted or deleted the accounting needs to -//! change. Saga nodes must be idempotent in order to work correctly. - -use super::ActionRegistry; -use super::NexusActionContext; -use super::NexusSaga; -use crate::app::sagas::declare_saga_actions; -use nexus_db_queries::authn; -use nexus_db_queries::db::datastore::CrucibleResources; -use nexus_db_queries::db::datastore::FreedCrucibleResources; -use nexus_types::saga::saga_action_failed; -use omicron_common::api::external::Error; -use omicron_uuid_kinds::VolumeUuid; -use serde::Deserialize; -use serde::Serialize; -use steno::ActionError; - -// volume delete saga: input parameters - -#[derive(Debug, Deserialize, Serialize)] -pub(crate) struct Params { - pub serialized_authn: authn::saga::Serialized, - pub volume_id: VolumeUuid, -} - -// volume delete saga: actions - -declare_saga_actions! { - volume_delete; - DECREASE_CRUCIBLE_RESOURCE_COUNT -> "crucible_resources_to_delete" { - + svd_decrease_crucible_resource_count - } - DELETE_CRUCIBLE_REGIONS -> "no_result_1" { - + svd_delete_crucible_regions - } - DELETE_CRUCIBLE_RUNNING_SNAPSHOTS -> "no_result_2" { - + svd_delete_crucible_running_snapshots - } - DELETE_CRUCIBLE_SNAPSHOTS -> "no_result_3" { - + svd_delete_crucible_snapshots - } - DELETE_CRUCIBLE_SNAPSHOT_RECORDS -> "no_result_4" { - + svd_delete_crucible_snapshot_records - } - FIND_FREED_CRUCIBLE_REGIONS -> "freed_crucible_regions" { - + svd_find_freed_crucible_regions - } - DELETE_FREED_CRUCIBLE_REGIONS -> "no_result_5" { - + svd_delete_freed_crucible_regions - } - HARD_DELETE_VOLUME_RECORD -> "volume_hard_deleted" { - + svd_hard_delete_volume_record - } -} - -// volume delete saga: definition - -pub fn create_dag( - mut builder: steno::DagBuilder, -) -> Result { - builder.append(decrease_crucible_resource_count_action()); - builder.append_parallel(vec![ - // clean up top level regions for volume - delete_crucible_regions_action(), - // clean up running snapshots no longer referenced by any volume - delete_crucible_running_snapshots_action(), - ]); - // clean up snapshots no longer referenced by any volume - builder.append(delete_crucible_snapshots_action()); - // remove snapshot db records - builder.append(delete_crucible_snapshot_records_action()); - // clean up regions that were freed by deleting snapshots - builder.append(find_freed_crucible_regions_action()); - builder.append(delete_freed_crucible_regions_action()); - builder.append(hard_delete_volume_record_action()); - - Ok(builder.build()?) -} - -#[derive(Debug)] -pub(crate) struct SagaVolumeDelete; -impl NexusSaga for SagaVolumeDelete { - const NAME: &'static str = "volume-delete"; - type Params = Params; - - fn register_actions(registry: &mut ActionRegistry) { - volume_delete_register_actions(registry); - } - - fn make_saga_dag( - _params: &Self::Params, - builder: steno::DagBuilder, - ) -> Result { - create_dag(builder) - } -} - -// volume delete saga: action implementations - -/// Decrease Crucible resource accounting for this volume, and return Crucible -/// resources to delete. -async fn svd_decrease_crucible_resource_count( - sagactx: NexusActionContext, -) -> Result { - let osagactx = sagactx.user_data(); - let params = sagactx.saga_params::()?; - - let crucible_resources = osagactx - .datastore() - .soft_delete_volume(params.volume_id) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to soft_delete_volume: {:?}", - e, - ))) - })?; - - Ok(crucible_resources) -} - -/// Clean up regions associated with this volume. -async fn svd_delete_crucible_regions( - sagactx: NexusActionContext, -) -> Result<(), ActionError> { - let log = sagactx.user_data().log(); - let osagactx = sagactx.user_data(); - - let crucible_resources_to_delete = - sagactx.lookup::("crucible_resources_to_delete")?; - - // Send DELETE calls to the corresponding Crucible agents - let datasets_and_regions = osagactx - .datastore() - .regions_to_delete( - &crucible_resources_to_delete, - ) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to get datasets_and_regions from crucible resources ({:?}): {:?}", - crucible_resources_to_delete, - e, - ))) - })?; - - osagactx - .nexus() - .delete_crucible_regions(log, datasets_and_regions.clone()) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to delete_crucible_regions: {:?}", - e, - ))) - })?; - - // Remove DB records - let region_ids_to_delete = - datasets_and_regions.iter().map(|(_, r)| r.id()).collect(); - - osagactx - .datastore() - .regions_hard_delete(log, region_ids_to_delete) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to regions_hard_delete: {:?}", - e, - ))) - })?; - - Ok(()) -} - -/// Clean up running read-only downstairs corresponding to snapshots freed up -/// for deletion by deleting this volume. -/// -/// This Volume may have referenced read-only downstairs (and their snapshots), -/// and deleting it will remove the references - this may free up those -/// resources for deletion, which this Saga node does. -async fn svd_delete_crucible_running_snapshots( - sagactx: NexusActionContext, -) -> Result<(), ActionError> { - let log = sagactx.user_data().log(); - let osagactx = sagactx.user_data(); - - let crucible_resources_to_delete = - sagactx.lookup::("crucible_resources_to_delete")?; - - // Send DELETE calls to the corresponding Crucible agents - let datasets_and_snapshots = osagactx - .datastore() - .snapshots_to_delete( - &crucible_resources_to_delete, - ) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to get datasets_and_snapshots from crucible resources ({:?}): {:?}", - crucible_resources_to_delete, - e, - ))) - })?; - - osagactx - .nexus() - .delete_crucible_running_snapshots(log, datasets_and_snapshots.clone()) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to delete_crucible_running_snapshots: {:?}", - e, - ))) - })?; - - Ok(()) -} - -/// Clean up snapshots freed up for deletion by deleting this volume. -/// -/// This Volume may have referenced read-only downstairs (and their snapshots), -/// and deleting it will remove the references - this may free up those -/// resources for deletion, which this Saga node does. -async fn svd_delete_crucible_snapshots( - sagactx: NexusActionContext, -) -> Result<(), ActionError> { - let log = sagactx.user_data().log(); - let osagactx = sagactx.user_data(); - - let crucible_resources_to_delete = - sagactx.lookup::("crucible_resources_to_delete")?; - - // Send DELETE calls to the corresponding Crucible agents - let datasets_and_snapshots = osagactx - .datastore() - .snapshots_to_delete( - &crucible_resources_to_delete, - ) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to get datasets_and_snapshots from crucible resources ({:?}): {:?}", - crucible_resources_to_delete, - e, - ))) - })?; - - osagactx - .nexus() - .delete_crucible_snapshots(log, datasets_and_snapshots.clone()) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to delete_crucible_snapshots: {:?}", - e, - ))) - })?; - - Ok(()) -} - -/// Remove records for deleted snapshots -async fn svd_delete_crucible_snapshot_records( - sagactx: NexusActionContext, -) -> Result<(), ActionError> { - let osagactx = sagactx.user_data(); - - let crucible_resources_to_delete = - sagactx.lookup::("crucible_resources_to_delete")?; - - // Remove DB records - let datasets_and_snapshots = osagactx - .datastore() - .snapshots_to_delete( - &crucible_resources_to_delete, - ) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to get datasets_and_snapshots from crucible resources ({:?}): {:?}", - crucible_resources_to_delete, - e, - ))) - })?; - - for (_, region_snapshot) in datasets_and_snapshots { - osagactx - .datastore() - .region_snapshot_remove( - region_snapshot.dataset_id.into(), - region_snapshot.region_id, - region_snapshot.snapshot_id, - ) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to region_snapshot_remove {} {} {}: {:?}", - region_snapshot.dataset_id, - region_snapshot.region_id, - region_snapshot.snapshot_id, - e, - ))) - })?; - } - - Ok(()) -} - -/// Deleting region snapshots in a previous saga node may have freed up regions -/// that were deleted in the DB but couldn't be deleted by the Crucible Agent -/// because a snapshot existed. Look for those here. These will be a different -/// volume id (i.e. for a previously deleted disk) than the one in this saga's -/// params struct. -/// -/// It's insufficient to rely on the struct of CrucibleResources to clean up -/// that is returned as part of svd_decrease_crucible_resource_count. Imagine a -/// disk that is composed of three regions (a subset of -/// [`sled_agent_client::VolumeConstructionRequest`] is shown here): -/// -/// ```json -/// { -/// "type": "volume", -/// "id": "6b353c87-afac-4ee2-b71a-6fe35fcf9e46", -/// "sub_volumes": [ -/// { -/// "type": "region", -/// "opts": { -/// "targets": [ -/// "[fd00:1122:3344:101::5]:1000", -/// "[fd00:1122:3344:102::9]:1000", -/// "[fd00:1122:3344:103::2]:1000" -/// ], -/// "read_only": false -/// } -/// } -/// ], -/// "read_only_parent": null, -/// } -/// ``` -/// -/// Taking a snapshot of this will produce the following volume: -/// -/// ```json -/// { -/// "type": "volume", -/// "id": "1ef7282e-a3fb-4222-85a8-b16d3fbfd738", <-- new UUID -/// "sub_volumes": [ -/// { -/// "type": "region", -/// "opts": { -/// "targets": [ -/// "[fd00:1122:3344:101::5]:1001", <-- port changed -/// "[fd00:1122:3344:102::9]:1001", <-- port changed -/// "[fd00:1122:3344:103::2]:1001" <-- port changed -/// ], -/// "read_only": true <-- read_only now true -/// } -/// } -/// ], -/// "read_only_parent": null, -/// } -/// ``` -/// -/// The snapshot targets will use the same IP but different port: snapshots are -/// initially located on the same filesystem as their region. -/// -/// The disk's volume has no read only resources, while the snapshot's volume -/// does. The disk volume's targets are all regions (backed by downstairs that -/// are read/write) while the snapshot volume's targets are all snapshots -/// (backed by downstairs that are read-only). The two volumes are linked in the -/// sense that the snapshots from the second are contained *within* the regions -/// of the first, reflecting the resource nesting from ZFS. This is also -/// reflected in the REST endpoint that the Crucible agent uses: -/// -/// /crucible/0/regions/{id}/snapshots/{name} -/// -/// If the disk is then deleted, the volume delete saga will run for the first -/// volume shown here. The CrucibleResources struct returned as part of -/// [`svd_decrease_crucible_resource_count`] will contain *nothing* to clean up: -/// the regions contain snapshots that are part of other volumes and cannot be -/// deleted, and the disk's volume doesn't reference any read-only resources. -/// -/// This is expected and normal: regions are "leaked" all the time due to -/// snapshots preventing their deletion. This part of the saga detects when -/// those regions can be cleaned up - it must be stored in the output of this -/// saga node as deleting volume records will affect what is returned by -/// `find_deleted_volume_regions`. -/// -/// Note: each delete of a snapshot could trigger another delete of a region, if -/// that region's use has gone to zero. A snapshot delete will never trigger -/// another snapshot delete. -async fn svd_find_freed_crucible_regions( - sagactx: NexusActionContext, -) -> Result { - let osagactx = sagactx.user_data(); - - // Find regions freed up for deletion by a previous saga node deleting the - // region snapshots. - let freed_datasets_regions_and_volumes = - osagactx.datastore().find_deleted_volume_regions().await.map_err( - |e| { - saga_action_failed(Error::internal_error(&format!( - "failed to find_deleted_volume_regions: {:?}", - e, - ))) - }, - )?; - - Ok(freed_datasets_regions_and_volumes) -} - -async fn svd_delete_freed_crucible_regions( - sagactx: NexusActionContext, -) -> Result<(), ActionError> { - let log = sagactx.user_data().log(); - let osagactx = sagactx.user_data(); - - // Find regions freed up for deletion by a previous saga node deleting the - // region snapshots. - let freed_datasets_regions_and_volumes = - sagactx.lookup::("freed_crucible_regions")?; - - for (dataset, region) in - &freed_datasets_regions_and_volumes.datasets_and_regions - { - // Send DELETE calls to the corresponding Crucible agents - osagactx - .nexus() - .delete_crucible_regions( - log, - vec![(dataset.clone(), region.clone())], - ) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to delete_crucible_regions: {:?}", - e, - ))) - })?; - - // Remove region DB record - osagactx - .datastore() - .regions_hard_delete(log, vec![region.id()]) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to regions_hard_delete: {:?}", - e, - ))) - })?; - } - - for volume_id in &freed_datasets_regions_and_volumes.volumes { - osagactx.datastore().volume_hard_delete(*volume_id).await.map_err( - |e| { - saga_action_failed(Error::internal_error(&format!( - "failed to volume_hard_delete {}: {:?}", - volume_id, e, - ))) - }, - )?; - } - - Ok(()) -} - -/// Hard delete the volume record -async fn svd_hard_delete_volume_record( - sagactx: NexusActionContext, -) -> Result { - let osagactx = sagactx.user_data(); - let params = sagactx.saga_params::()?; - - // Do not hard delete the volume record if there are lingering regions - // associated with them. This occurs when a region snapshot hasn't been - // deleted, which means we can't delete the region. Later on, deleting the - // region snapshot will free up the region(s) to be deleted (this occurs in - // svd_delete_freed_crucible_regions). - let allocated_regions = osagactx - .datastore() - .get_allocated_regions(params.volume_id) - .await - .map_err(|e| { - saga_action_failed(Error::internal_error(&format!( - "failed to get_allocated_regions for {}: {:?}", - params.volume_id, e, - ))) - })?; - - let log = sagactx.user_data().log(); - - if !allocated_regions.is_empty() { - info!( - &log, - "allocated regions for {} is not-empty, skipping volume_hard_delete", - params.volume_id, - ); - return Ok(false); - } - - info!( - &log, - "allocated regions for {} is empty, calling volume_hard_delete", - params.volume_id, - ); - - osagactx.datastore().volume_hard_delete(params.volume_id).await.map_err( - |e| { - saga_action_failed(Error::internal_error(&format!( - "failed to volume_hard_delete {}: {:?}", - params.volume_id, e, - ))) - }, - )?; - - Ok(true) -} diff --git a/nexus/src/app/sagas/volume_remove_rop.rs b/nexus/src/app/sagas/volume_remove_rop.rs index ed189db5d09..e6456a7f58f 100644 --- a/nexus/src/app/sagas/volume_remove_rop.rs +++ b/nexus/src/app/sagas/volume_remove_rop.rs @@ -3,10 +3,10 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use super::{ActionRegistry, NexusActionContext, NexusSaga, SagaInitError}; -use crate::app::sagas; use crate::app::sagas::declare_saga_actions; use nexus_db_queries::authn; use nexus_types::saga::saga_action_failed; +use omicron_common::api::external::Error; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::VolumeUuid; use serde::Deserialize; @@ -48,10 +48,13 @@ declare_saga_actions! { + svr_create_temp_volume - svr_create_temp_volume_undo } - // remove the read_only_parent, attach it to the temp volume. + // remove the read_only_parent, attach it to the temp volume. REMOVE_READ_ONLY_PARENT -> "no_result_1" { + svr_remove_read_only_parent } + SOFT_DELETE_VOLUME -> "soft_delete_volume" { + + svr_soft_delete_volume + } } // volume remove read only parent saga: definition @@ -67,25 +70,11 @@ impl NexusSaga for SagaVolumeRemoveROP { } fn make_saga_dag( - params: &Self::Params, + _params: &Self::Params, mut builder: steno::DagBuilder, ) -> Result { // Generate the temp volume ID this saga will use. let temp_volume_id = VolumeUuid::new_v4(); - // Generate the params for the subsaga called at the end. - let subsaga_params = sagas::volume_delete::Params { - serialized_authn: params.serialized_authn.clone(), - volume_id: temp_volume_id, - }; - let subsaga_dag = { - let subsaga_builder = steno::DagBuilder::new(steno::SagaName::new( - sagas::volume_delete::SagaVolumeDelete::NAME, - )); - sagas::volume_delete::SagaVolumeDelete::make_saga_dag( - &subsaga_params, - subsaga_builder, - )? - }; // Add the temp_volume_id to the saga. builder.append(Node::constant( @@ -97,26 +86,12 @@ impl NexusSaga for SagaVolumeRemoveROP { // Create the temporary volume builder.append(create_temp_volume_action()); + // Remove the read only parent, attach to temp volume builder.append(remove_read_only_parent_action()); - // Build the params for the subsaga to delete the temp volume - builder.append(Node::constant( - "params_for_delete_subsaga", - serde_json::to_value(&subsaga_params).map_err(|e| { - SagaInitError::SerializeError( - String::from("params_for_delete_subsaga"), - e, - ) - })?, - )); - - // Call the subsaga to delete the temp volume - builder.append(Node::subsaga( - "final_no_result", - subsaga_dag, - "params_for_delete_subsaga", - )); + // Soft-delete the temp volume + builder.append(soft_delete_volume_action()); Ok(builder.build()?) } @@ -179,3 +154,22 @@ async fn svr_remove_read_only_parent( .map_err(saga_action_failed)?; Ok(()) } + +async fn svr_soft_delete_volume( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let osagactx = sagactx.user_data(); + + let temp_volume_id = sagactx.lookup::("temp_volume_id")?; + + osagactx.datastore().soft_delete_volume(temp_volume_id).await.map_err( + |e| { + saga_action_failed(Error::internal_error(&format!( + "failed to soft_delete_volume: {:?}", + e, + ))) + }, + )?; + + Ok(()) +} diff --git a/nexus/src/app/snapshot.rs b/nexus/src/app/snapshot.rs index b17b32b8db0..2c1bdb86bae 100644 --- a/nexus/src/app/snapshot.rs +++ b/nexus/src/app/snapshot.rs @@ -204,6 +204,8 @@ impl super::Nexus { ) .await?; + self.background_tasks.task_volume_delete.activate(); + Ok(()) } } diff --git a/nexus/test-utils/src/background.rs b/nexus/test-utils/src/background.rs index 7580749b19d..ddc91409777 100644 --- a/nexus/test-utils/src/background.rs +++ b/nexus/test-utils/src/background.rs @@ -6,12 +6,17 @@ use crate::http_testing::NexusRequest; use dropshot::test_util::ClientTestContext; +use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::DataStore; use nexus_lockstep_client::types::BackgroundTask; use nexus_lockstep_client::types::CurrentStatus; use nexus_lockstep_client::types::LastResult; +use nexus_types::identity::Asset; use nexus_types::internal_api::background::*; use omicron_test_utils::dev::poll::{CondCheckError, wait_for_condition}; use slog::info; +use slog::o; +use std::sync::Arc; use std::time::Duration; /// Given the name of a background task, wait for it to complete if it's @@ -57,7 +62,7 @@ pub async fn wait_background_task( /// /// The `timeout` parameter controls how long to wait for the task to go idle /// before activating it, and how long to wait for it to complete after -/// activation. Defaults to 10 seconds if not specified. +/// activation. Defaults to 30 seconds if not specified. pub async fn activate_background_task( lockstep_client: &ClientTestContext, task_name: &str, @@ -65,7 +70,7 @@ pub async fn activate_background_task( activate_background_task_with_timeout( lockstep_client, task_name, - Duration::from_secs(10), + Duration::from_secs(30), ) .await } @@ -581,3 +586,166 @@ pub async fn run_blueprint_rendezvous(lockstep_client: &ClientTestContext) { ) .unwrap(); } + +/// Run the volume_delete background task, and assert that there are no reported +/// errors. +pub async fn run_volume_delete(internal_client: &ClientTestContext) { + let status = run_volume_delete_return_status(internal_client).await; + assert!(status.errors.is_empty()); +} + +/// Run the volume_delete background task and return the status. +pub async fn run_volume_delete_return_status( + internal_client: &ClientTestContext, +) -> VolumeDeleteStatus { + let last_background_task = + activate_background_task(&internal_client, "volume_delete").await; + + let LastResult::Completed(last_result_completed) = + last_background_task.last + else { + panic!( + "unexpected {:?} returned from volume_delete task", + last_background_task.last, + ); + }; + + serde_json::from_value::(last_result_completed.details) + .unwrap() +} + +pub async fn wait_for_all_volume_deletes( + datastore: &Arc, + lockstep_client: &ClientTestContext, +) { + wait_for_condition( + || { + let datastore = datastore.clone(); + let opctx = OpContext::for_tests( + lockstep_client.client_log.new(o!()), + datastore.clone(), + ); + + async move { + // Trigger the volume delete background task. Bail out of this + // loop only when there's no more soft-deleted volumes. + // + // Be careful not to check if the background tasks performed any + // actions: the fixed point that we're waiting for is for all + // resources to be cleaned up. + + run_volume_delete(lockstep_client).await; + + let mut soft_deleted_volumes_left = + datastore.get_soft_deleted_volumes(&opctx).await.unwrap(); + + // Filter out volumes that have allocated regions left, these + // will not be deleted. + let soft_deleted_volumes_left = { + let mut result = + Vec::with_capacity(soft_deleted_volumes_left.len()); + + while let Some(volume) = soft_deleted_volumes_left.pop() { + let allocated_regions = datastore + .get_allocated_regions(volume.id()) + .await + .unwrap(); + + if allocated_regions.is_empty() { + result.push(volume); + } + } + + result.len() + }; + + if soft_deleted_volumes_left > 0 { + info!( + &lockstep_client.client_log, + "wait_for_all_volume_deletes: \ + {soft_deleted_volumes_left} soft-deleted volumes left", + ); + + return Err(CondCheckError::<()>::NotYet { status: None }); + } + + Ok(()) + } + }, + &std::time::Duration::from_millis(50), + &std::time::Duration::from_secs(260), + ) + .await + .expect("all deletes finished"); +} + +/// Run the local_storage_delete background task and return the status. +pub async fn run_local_storage_delete(internal_client: &ClientTestContext) { + let last_background_task = + activate_background_task(&internal_client, "local_storage_delete") + .await; + + let LastResult::Completed(last_result_completed) = + last_background_task.last + else { + panic!( + "unexpected {:?} returned from volume_delete task", + last_background_task.last, + ); + }; + + let status = serde_json::from_value::( + last_result_completed.details, + ) + .unwrap(); + + assert!(status.errors.is_empty()); +} + +pub async fn wait_for_all_local_storage_deletes( + datastore: &Arc, + lockstep_client: &ClientTestContext, +) { + wait_for_condition( + || { + let datastore = datastore.clone(); + let opctx = OpContext::for_tests( + lockstep_client.client_log.new(o!()), + datastore.clone(), + ); + + async move { + // Trigger the local storage delete background task. Bail out of + // this loop only when there's no more allocations to clean up. + // + // Be careful not to check if the background tasks performed any + // actions: the fixed point that we're waiting for is for all + // resources to be cleaned up. + + run_local_storage_delete(lockstep_client).await; + + let disks_requiring_work = datastore + .deleted_disks_with_undeleted_local_storage(&opctx) + .await + .unwrap(); + + if !disks_requiring_work.is_empty() { + info!( + &lockstep_client.client_log, + "wait_for_all_local_storage_deletes: {} disks \ + requiring work left", + disks_requiring_work.len(), + ); + + return Err(CondCheckError::<()>::NotYet { status: None }); + } + + Ok(()) + } + }, + &std::time::Duration::from_millis(50), + &std::time::Duration::from_secs(260), + ) + .await + .expect("all deletes finished"); +} diff --git a/nexus/tests/config.test.toml b/nexus/tests/config.test.toml index 567291b21a3..ee564db766a 100644 --- a/nexus/tests/config.test.toml +++ b/nexus/tests/config.test.toml @@ -237,6 +237,8 @@ audit_log_cleanup.period_secs = 600 audit_log_cleanup.retention_days = 90 audit_log_cleanup.max_deleted_per_activation = 10000 populate_switch_ports.period_secs = 30 +volume_delete.period_secs = 10000 +local_storage_delete.period_secs = 10000 [multicast] # Enable multicast functionality for tests (disabled by default in production) diff --git a/nexus/tests/integration_tests/common.rs b/nexus/tests/integration_tests/common.rs new file mode 100644 index 00000000000..daa72618b73 --- /dev/null +++ b/nexus/tests/integration_tests/common.rs @@ -0,0 +1,18 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use nexus_test_utils::ControlPlaneTestContext; +use nexus_test_utils::background::wait_for_all_volume_deletes; +use nexus_test_utils::resource_helpers::DiskTest; + +pub(crate) async fn assert_all_crucible_resources_deleted<'a>( + cptestctx: &ControlPlaneTestContext, + disk_test: &DiskTest<'a, omicron_nexus::Server>, +) { + let nexus = &cptestctx.server.server_context().nexus; + let datastore = nexus.datastore(); + let lockstep_client = &cptestctx.lockstep_client; + wait_for_all_volume_deletes(&datastore, &lockstep_client).await; + assert!(disk_test.crucible_resources_deleted().await); +} diff --git a/nexus/tests/integration_tests/crucible_replacements.rs b/nexus/tests/integration_tests/crucible_replacements.rs index de4796aaccf..5ef7e6bed13 100644 --- a/nexus/tests/integration_tests/crucible_replacements.rs +++ b/nexus/tests/integration_tests/crucible_replacements.rs @@ -4,6 +4,7 @@ //! Tests related to region and region snapshot replacement +use crate::integration_tests::common::assert_all_crucible_resources_deleted; use async_bb8_diesel::AsyncRunQueryDsl; use diesel::ExpressionMethods; use diesel::QueryDsl; @@ -122,6 +123,13 @@ pub(crate) async fn wait_for_all_replacements( run_all_crucible_replacement_tasks(lockstep_client).await; + // Also, run the volume delete background task. This is + // important because some of the checks for moving replacement + // requests along will check if volumes are soft or hard + // deleted. + + run_volume_delete(lockstep_client).await; + let ro_left_to_do = datastore .find_read_only_regions_on_expunged_physical_disks(&opctx) .await @@ -299,6 +307,7 @@ mod region_replacement { client: ClientTestContext, lockstep_client: ClientTestContext, replacement_request_id: Uuid, + cptestctx: &'a ControlPlaneTestContext, } impl<'a> DeletedVolumeTest<'a> { @@ -372,6 +381,7 @@ mod region_replacement { client: client.clone(), lockstep_client: lockstep_client.clone(), replacement_request_id, + cptestctx, } } @@ -416,8 +426,12 @@ mod region_replacement { RegionReplacementState::Complete, ); - // Assert there are no more Crucible resources - assert!(self.disk_test.crucible_resources_deleted().await); + // Assert there are no undeleted Crucible resources + assert_all_crucible_resources_deleted( + &self.cptestctx, + &self.disk_test, + ) + .await; } async fn wait_for_request_state( @@ -1050,6 +1064,8 @@ async fn test_racing_replacements_for_soft_deleted_disk_volume( let snapshot_id = snapshot.identity.id; async move { + run_volume_delete(&lockstep_client).await; + let region_snapshot = datastore .region_snapshot_get(dataset_id, region_id, snapshot_id) .await @@ -1304,7 +1320,7 @@ async fn test_racing_replacements_for_soft_deleted_disk_volume( // Now, assert that all crucible resources are cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } mod region_snapshot_replacement { @@ -1328,6 +1344,7 @@ mod region_snapshot_replacement { lockstep_client: ClientTestContext, replacement_request_id: Uuid, snapshot_socket_addr: SocketAddr, + cptestctx: &'a ControlPlaneTestContext, } impl<'a> DeletedVolumeTest<'a> { @@ -1468,6 +1485,7 @@ mod region_snapshot_replacement { lockstep_client: lockstep_client.clone(), replacement_request_id, snapshot_socket_addr, + cptestctx, } } @@ -1508,8 +1526,8 @@ mod region_snapshot_replacement { /// completion /// - this harness' region snapshot replacement request has transitioned /// to Complete - /// - there are no more volumes that reference the request's region - /// snapshot + /// - there are no non-deleted volumes that reference the request's + /// region snapshot pub async fn finish_test(&self) { // Make sure that all the background tasks can run to completion. @@ -1532,40 +1550,34 @@ mod region_snapshot_replacement { RegionSnapshotReplacementState::Complete, ); - // Assert no volumes are referencing the snapshot address + // Wait for all volume deletes, then assert no non-deleted volumes + // are referencing the snapshot address - let mut counter = 1; - loop { - let volumes = self - .datastore - .find_volumes_referencing_socket_addr( - &self.opctx(), - self.snapshot_socket_addr, - ) - .await - .unwrap(); + wait_for_all_volume_deletes(&self.datastore, &self.lockstep_client) + .await; - if !volumes.is_empty() { - eprintln!( - "Volume should be gone, try {counter} {:?}", - volumes - ); - tokio::time::sleep(std::time::Duration::from_secs(5)).await; - counter += 1; - if counter > 200 { - panic!( - "Tried 200 times, and still this did not finish" - ); - } - } else { - break; - } - } + let volumes: Vec<_> = self + .datastore + .find_volumes_referencing_socket_addr( + &self.opctx(), + self.snapshot_socket_addr, + ) + .await + .unwrap() + .into_iter() + .filter(|volume| volume.time_deleted.is_none()) + .collect(); + + assert!(volumes.is_empty()); } /// Assert no Crucible resources are leaked pub async fn assert_no_crucible_resources_leaked(&self) { - assert!(self.disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted( + &self.cptestctx, + &self.disk_test, + ) + .await; } async fn wait_for_request_state( @@ -1738,6 +1750,8 @@ mod region_snapshot_replacement { ); let mut i = 1; loop { + run_volume_delete(&self.lockstep_client).await; + let region_snapshot_replace_request = self .datastore .get_region_snapshot_replacement_request_by_id( @@ -2763,6 +2777,8 @@ async fn test_replacement_sanity_twice_after_snapshot_delete( // Delete the snapshot delete_snapshot(&client, PROJECT_NAME, "snap").await; + wait_for_all_volume_deletes(&datastore, &lockstep_client).await; + // Assert snapshot volume is gone let db_snapshot = datastore .snapshot_get(&opctx, snapshot.identity.id) @@ -2834,4 +2850,6 @@ async fn test_replacement_sanity_twice_after_snapshot_delete( wait_for_all_replacements(&datastore, &lockstep_client).await; } + + wait_for_all_volume_deletes(&datastore, &lockstep_client).await; } diff --git a/nexus/tests/integration_tests/disks.rs b/nexus/tests/integration_tests/disks.rs index dbc6f0ecc66..2f571e7f2c0 100644 --- a/nexus/tests/integration_tests/disks.rs +++ b/nexus/tests/integration_tests/disks.rs @@ -5,6 +5,7 @@ //! Tests basic disk support in the API use super::instances::instance_wait_for_state; +use crate::integration_tests::common::assert_all_crucible_resources_deleted; use dropshot::HttpErrorResponseBody; use dropshot::test_util::ClientTestContext; use http::StatusCode; @@ -19,6 +20,8 @@ use nexus_db_queries::db::datastore::RegionAllocationFor; use nexus_db_queries::db::datastore::RegionAllocationParameters; use nexus_db_queries::db::fixed_data::FLEET_ID; use nexus_test_utils::SLED_AGENT_UUID; +use nexus_test_utils::background::run_volume_delete_return_status; +use nexus_test_utils::background::wait_for_all_volume_deletes; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; @@ -59,7 +62,6 @@ use sled_agent_client::VolumeConstructionRequest; use std::collections::HashSet; use std::collections::VecDeque; use std::sync::Arc; -use tokio::sync::oneshot; use uuid::Uuid; type ControlPlaneTestContext = @@ -1321,11 +1323,10 @@ async fn test_disk_virtual_provisioning_collection( ); } +/// Confirm that region deletes can happen async to the higher user-level +/// resources #[nexus_test] -async fn test_disk_virtual_provisioning_collection_failed_delete( - cptestctx: &ControlPlaneTestContext, -) { - // Confirm that there's no panic deleting a project if a disk deletion fails +async fn test_async_region_delete(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; let nexus = &cptestctx.server.server_context().nexus; let datastore = nexus.datastore(); @@ -1387,50 +1388,24 @@ async fn test_disk_virtual_provisioning_collection_failed_delete( .get_crucible_dataset(zpool.id, dataset.id) .set_region_deletion_error(true); - // Delete the disk - expect this to fail + // Delete the disk - this will succeed but the third region won't be + // properly deleted yet NexusRequest::new( RequestBuilder::new(client, Method::DELETE, &disk_url) - .expect_status(Some(StatusCode::INTERNAL_SERVER_ERROR)), + .expect_status(Some(StatusCode::NO_CONTENT)), ) .authn_as(AuthnMode::PrivilegedUser) .execute() .await - .expect("unexpected success deleting 1 GiB disk"); + .expect("unexpected error deleting 1 GiB disk"); - // The virtual provisioning collection numbers haven't changed - let virtual_provisioning_collection = datastore - .virtual_provisioning_collection_get(&opctx, project_id1) - .await - .unwrap(); - assert_eq!( - virtual_provisioning_collection.virtual_disk_bytes_provisioned.0, - disk_size - ); + // Assert running the volume delete task deletes the two other regions but + // errors when trying to delete the third. + let status = + run_volume_delete_return_status(&cptestctx.lockstep_client).await; - // And the disk is now faulted. The name will have changed due to the - // "undelete and fault" function. - let disk_url = format!( - "/v1/disks/deleted-{}?project={}", - disk.identity.id, PROJECT_NAME - ); - let disk = disk_get(&client, &disk_url).await; - assert_eq!(disk.state, DiskState::Faulted); - - // Set the third agent to respond normally - cptestctx - .first_sled_agent() - .get_crucible_dataset(zpool.id, dataset.id) - .set_region_deletion_error(false); - - // Request disk delete again - NexusRequest::new( - RequestBuilder::new(client, Method::DELETE, &disk_url) - .expect_status(Some(StatusCode::NO_CONTENT)), - ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .expect("unexpected failure deleting 1 GiB disk"); + assert!(!status.errors.is_empty()); + assert_eq!(status.region_results.len(), 2); // Delete the project's default VPC subnet and VPC let subnet_url = @@ -1448,7 +1423,7 @@ async fn test_disk_virtual_provisioning_collection_failed_delete( .await .expect("failed to make request"); - // The project can be deleted now + // Assert the project can be deleted even if the region isn't yet. let url = format!("/v1/projects/{}", PROJECT_NAME); NexusRequest::new( RequestBuilder::new(client, Method::DELETE, &url) @@ -1458,6 +1433,16 @@ async fn test_disk_virtual_provisioning_collection_failed_delete( .execute() .await .expect("unexpected failure deleting project"); + + // Set the third agent to respond normally + cptestctx + .first_sled_agent() + .get_crucible_dataset(zpool.id, dataset.id) + .set_region_deletion_error(false); + + // Wait for all crucible resources to be cleaned up + + assert_all_crucible_resources_deleted(&cptestctx, &disk_test).await; } #[nexus_test] @@ -1591,6 +1576,7 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; let nexus = &cptestctx.server.server_context().nexus; let datastore = nexus.datastore(); + let lockstep_client = &cptestctx.lockstep_client; // Create three zpools, each with one dataset. let test = DiskTest::new(&cptestctx).await; @@ -1698,6 +1684,8 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { .await .expect("unexpected failure deleting 7 GiB disk"); + wait_for_all_volume_deletes(&datastore, &lockstep_client).await; + // Total occupied size should be 0 for zpool in test.zpools() { let dataset = zpool.crucible_dataset(); @@ -2326,29 +2314,27 @@ async fn test_no_halt_disk_delete_one_region_on_expunged_agent( cptestctx.first_sled_agent().drop_dataset(zpool.id, dataset.id); - // Spawn a task that tries to delete the disk + // Ensure the disk delete does not hang even when a Crucible agent isn't + // responding + let disk_url = get_disk_url(DISK_NAME); - let client = client.clone(); - let (task_started_tx, task_started_rx) = oneshot::channel(); + NexusRequest::object_delete(&client, &disk_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to delete disk"); - let jh = tokio::spawn(async move { - task_started_tx.send(()).unwrap(); + // Run the volume delete task, and assert two regions are deleted. - NexusRequest::object_delete(&client, &disk_url) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .expect("failed to delete disk"); - }); + let status = + run_volume_delete_return_status(&cptestctx.lockstep_client).await; - // Wait until the task starts - task_started_rx.await.unwrap(); - - // It won't finish until the dataset is expunged. - assert!(!jh.is_finished()); + assert!(!status.errors.is_empty()); + assert_eq!(status.region_results.len(), 2); // Expunge the physical disk + let (_, db_zpool) = LookupPath::new(&opctx, datastore) .zpool_id(zpool.id) .fetch() @@ -2364,11 +2350,19 @@ async fn test_no_halt_disk_delete_one_region_on_expunged_agent( .await .unwrap(); - // Now, the delete call will finish Ok - jh.await.unwrap(); + // Rerun the volume delete task and assert the last region was deleted. This + // test asserts against 3 regions being deleted because the volume record + // won't be hard-deleted, and the background task will re-attempt each + // delete. + + let status = + run_volume_delete_return_status(&cptestctx.lockstep_client).await; - // Ensure that the disk was properly deleted and all the regions are gone - - // Nexus should hard delete the region records in this case. + assert!(status.errors.is_empty()); + assert_eq!(status.region_results.len(), 3); + + // Assert that all the regions are gone - Nexus should hard delete the + // region records in this case. let datasets_and_regions = datastore.get_allocated_regions(db_disk.volume_id()).await.unwrap(); @@ -2616,6 +2610,12 @@ async fn test_zpool_control_plane_storage_buffer( .await .expect("failed to delete disk"); + // This test is testing the control plane storage buffer, not the background + // volume deletion. Wait for all those to complete, otherwise later on the + // test will see unrelated INSUFFICIENT_STORAGE errors. + + wait_for_all_volume_deletes(&datastore, &cptestctx.lockstep_client).await; + // For any of the zpools, set the control plane storage buffer to 2G. This // should prevent the disk's region allocation from succeeding (as the // reserved sizes of 10G + 5G plus the storage buffer of 2G is 1G over the diff --git a/nexus/tests/integration_tests/mod.rs b/nexus/tests/integration_tests/mod.rs index dc1fc84c2b4..f07bb1bb3c6 100644 --- a/nexus/tests/integration_tests/mod.rs +++ b/nexus/tests/integration_tests/mod.rs @@ -18,6 +18,7 @@ mod bfd; mod certificates; mod cockroach; mod commands; +mod common; mod console_api; mod crucible_replacements; mod data_migrations; diff --git a/nexus/tests/integration_tests/snapshots.rs b/nexus/tests/integration_tests/snapshots.rs index a13eec1a5ec..82ffdf6dff7 100644 --- a/nexus/tests/integration_tests/snapshots.rs +++ b/nexus/tests/integration_tests/snapshots.rs @@ -4,6 +4,7 @@ //! Tests basic snapshot support in the API +use crate::integration_tests::common::assert_all_crucible_resources_deleted; use crate::integration_tests::instances::instance_simulate; use chrono::Utc; use dropshot::test_util::ClientTestContext; @@ -995,7 +996,7 @@ async fn test_snapshot_unwind(cptestctx: &ControlPlaneTestContext) { .expect("failed to delete disk"); // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } // Test that the code that Saga nodes call is idempotent diff --git a/nexus/tests/integration_tests/volume_management.rs b/nexus/tests/integration_tests/volume_management.rs index 700c0dc3a00..e605dc71426 100644 --- a/nexus/tests/integration_tests/volume_management.rs +++ b/nexus/tests/integration_tests/volume_management.rs @@ -5,6 +5,7 @@ //! Tests that Nexus properly manages and cleans up Crucible resources //! associated with Volumes +use crate::integration_tests::common::assert_all_crucible_resources_deleted; use crate::integration_tests::crucible_replacements::wait_for_all_replacements; use crate::integration_tests::sleds::sleds_list; use async_bb8_diesel::AsyncRunQueryDsl; @@ -37,6 +38,7 @@ use nexus_db_queries::db::datastore::SourceVolume; use nexus_db_queries::db::datastore::VolumeReplaceResult; use nexus_db_queries::db::datastore::VolumeToDelete; use nexus_db_queries::db::datastore::VolumeWithTarget; +use nexus_test_utils::background::wait_for_all_volume_deletes; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; @@ -210,7 +212,7 @@ async fn test_snapshot_then_delete_disk(cptestctx: &ControlPlaneTestContext) { .expect("failed to delete snapshot"); // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test] @@ -274,7 +276,7 @@ async fn test_delete_snapshot_then_disk(cptestctx: &ControlPlaneTestContext) { .expect("failed to delete disk"); // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test] @@ -342,7 +344,7 @@ async fn test_multiple_snapshots(cptestctx: &ControlPlaneTestContext) { } // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test] @@ -353,6 +355,11 @@ async fn test_snapshot_prevents_other_disk( // allocation. let client = &cptestctx.external_client; + let lockstep_client = &cptestctx.lockstep_client; + let apictx = &cptestctx.server.server_context(); + let nexus = &apictx.nexus; + let datastore = nexus.datastore(); + let disk_test = DiskTest::new(&cptestctx).await; let disks_url = get_disks_url(); let base_disk_name: Name = "base-disk".parse().unwrap(); @@ -427,6 +434,11 @@ async fn test_snapshot_prevents_other_disk( .await .expect("failed to delete snapshot"); + // Wait for the volume delete background task, else the next disk creation + // may fail with INSUFFICIENT_STORAGE + + wait_for_all_volume_deletes(&datastore, &lockstep_client).await; + // Disk allocation will work now let _next_disk: external::Disk = NexusRequest::new( RequestBuilder::new(client, Method::POST, &disks_url) @@ -451,7 +463,7 @@ async fn test_snapshot_prevents_other_disk( delete_image(client, PROJECT_NAME, "not-alpine").await; // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test] @@ -590,7 +602,7 @@ async fn test_multiple_disks_multiple_snapshots_order_1( .expect("failed to delete snapshot"); // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test] @@ -729,7 +741,7 @@ async fn test_multiple_disks_multiple_snapshots_order_2( .expect("failed to delete snapshot"); // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } async fn prepare_for_test_multiple_layers_of_snapshots( @@ -908,7 +920,7 @@ async fn test_multiple_layers_of_snapshots_delete_all_disks_first( } // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test] @@ -946,7 +958,7 @@ async fn test_multiple_layers_of_snapshots_delete_all_snapshots_first( } // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test] @@ -1010,7 +1022,7 @@ async fn test_multiple_layers_of_snapshots_random_delete_order( } // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test] @@ -1160,7 +1172,7 @@ async fn test_create_image_from_snapshot_delete( .expect("failed to delete image"); // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } enum DeleteImageTestParam { @@ -1287,7 +1299,7 @@ async fn delete_image_test( } // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } // Make sure that whatever order disks, images, and snapshots are deleted, the @@ -2512,7 +2524,7 @@ async fn test_disk_create_saga_unwinds_correctly( .unwrap(); // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test] @@ -2586,7 +2598,7 @@ async fn test_snapshot_create_saga_unwinds_correctly( .expect("failed to delete disk"); // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } // Test function that creates a VolumeConstructionRequest::Region With gen, @@ -4144,6 +4156,8 @@ async fn test_read_only_region_reference_counting( .await .expect("failed to delete disk"); + wait_for_all_volume_deletes(&datastore, &lockstep_client).await; + let usage = datastore .volume_usage_records_for_resource( VolumeResourceUsage::ReadOnlyRegion { @@ -4185,7 +4199,7 @@ async fn test_read_only_region_reference_counting( assert!(region_destroyed); // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } /// Assert that a snapshot of a volume with a read-only region is properly @@ -4440,7 +4454,7 @@ async fn test_read_only_region_reference_counting_layers( .expect("failed to delete disk"); // Assert everything was cleaned up - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test] @@ -5137,7 +5151,8 @@ async fn test_double_layer_with_read_only_region_delete( .await .expect("failed to delete another-disk-from-snapshot"); - assert!(disk_test.crucible_resources_deleted().await); + // Assert everything was cleaned up + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test(extra_sled_agents = 3)] @@ -5316,8 +5331,7 @@ async fn test_double_layer_snapshot_with_read_only_region_delete_2( .expect("failed to delete disk-from-snapshot"); // Assert everything was cleaned up - - assert!(disk_test.crucible_resources_deleted().await); + assert_all_crucible_resources_deleted(cptestctx, &disk_test).await; } #[nexus_test(extra_sled_agents = 3)] diff --git a/nexus/types/src/internal_api/background.rs b/nexus/types/src/internal_api/background.rs index 2a352a7923a..5982ed09099 100644 --- a/nexus/types/src/internal_api/background.rs +++ b/nexus/types/src/internal_api/background.rs @@ -1365,6 +1365,24 @@ pub struct PhysicalDiskAdoptionStatus { pub errors: Vec, } +/// The status of a `volume_delete` background task activation +#[derive(Serialize, Deserialize, Default, Debug, PartialEq, Eq)] +pub struct VolumeDeleteStatus { + pub region_results: Vec, + pub running_snapshot_results: Vec, + pub snapshot_results: Vec, + pub volumes_deleted: Vec, + pub errors: Vec, +} + +/// The status of a `local_storage_delete` background task activation +#[derive(Serialize, Deserialize, Default, Debug, PartialEq, Eq)] +pub struct LocalStorageDeleteStatus { + pub delete_results: Vec, + pub deallocate_results: Vec, + pub errors: Vec, +} + #[cfg(test)] mod test { use super::TufRepoInfo; diff --git a/smf/nexus/multi-sled/config-partial.toml b/smf/nexus/multi-sled/config-partial.toml index bfe9dbad187..879fe3a147f 100644 --- a/smf/nexus/multi-sled/config-partial.toml +++ b/smf/nexus/multi-sled/config-partial.toml @@ -132,6 +132,8 @@ audit_log_cleanup.period_secs = 600 audit_log_cleanup.retention_days = 90 audit_log_cleanup.max_deleted_per_activation = 10000 populate_switch_ports.period_secs = 30 +volume_delete.period_secs = 30 +local_storage_delete.period_secs = 30 [default_region_allocation_strategy] # by default, allocate across 3 distinct sleds diff --git a/smf/nexus/single-sled/config-partial.toml b/smf/nexus/single-sled/config-partial.toml index 8e36b8bd4ec..ff9b16efc25 100644 --- a/smf/nexus/single-sled/config-partial.toml +++ b/smf/nexus/single-sled/config-partial.toml @@ -132,6 +132,8 @@ audit_log_cleanup.period_secs = 600 audit_log_cleanup.retention_days = 90 audit_log_cleanup.max_deleted_per_activation = 10000 populate_switch_ports.period_secs = 30 +volume_delete.period_secs = 30 +local_storage_delete.period_secs = 30 [default_region_allocation_strategy] # by default, allocate without requirement for distinct sleds.