From 98bf63546a17c881bdfeb0a7c0e1499b782645bf Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 1 Jul 2024 16:26:52 -0700 Subject: [PATCH] Allow Nexus to specify Zone filesystem location (#5931) This is a newer version of https://github.com/oxidecomputer/omicron/pull/5050 , focused on a slightly smaller scope. This PR adds an optional `filesystem_pool` to Sled Agent's API, and to Nexus' blueprint for zone construction. This allows Nexus to have control over which zpool is being used for zone provisioning, rather than letting the Sled Agent control this decision as it used to. By giving more control to Nexus, Nexus can better understand the impact of faults (e.g., will pulling a physical disk cause a zone to fail?) and can have more control over zone-reallocation. This field is optional largely for backwards compatibility, but in the future, will eventually become mandatory. Fixes https://github.com/oxidecomputer/omicron/issues/5048 Part of https://github.com/oxidecomputer/omicron/issues/5929 --- clients/nexus-client/src/lib.rs | 1 + .../reconfigurator-cli/tests/test_basic.rs | 20 +++ nexus/db-model/src/deployment.rs | 9 +- nexus/db-model/src/inventory.rs | 5 + nexus/db-model/src/omicron_zone_config.rs | 14 +- nexus/db-model/src/schema.rs | 2 + nexus/db-model/src/schema_versions.rs | 4 +- nexus/db-queries/src/db/datastore/rack.rs | 21 ++- nexus/inventory/src/collector.rs | 4 + .../reconfigurator/execution/src/datasets.rs | 1 + nexus/reconfigurator/execution/src/dns.rs | 24 +++ .../execution/src/external_networking.rs | 11 ++ .../execution/src/omicron_zones.rs | 13 +- .../planning/src/blueprint_builder/builder.rs | 156 +++++++++++------- .../planning/src/blueprint_builder/zones.rs | 25 ++- nexus/reconfigurator/planning/src/planner.rs | 2 +- .../background/tasks/blueprint_execution.rs | 6 +- .../tasks/crdb_node_id_collector.rs | 7 +- nexus/test-utils/src/lib.rs | 7 + nexus/test-utils/src/resource_helpers.rs | 63 +++++++ nexus/types/src/deployment.rs | 3 + nexus/types/src/deployment/planning_input.rs | 2 + nexus/types/src/deployment/zone_type.rs | 39 +---- openapi/nexus-internal.json | 8 + openapi/sled-agent.json | 9 + schema/all-zones-requests.json | 11 ++ .../crdb/add-nullable-filesystem-pool/up1.sql | 1 + .../crdb/add-nullable-filesystem-pool/up2.sql | 1 + schema/crdb/dbinit.sql | 10 +- schema/rss-service-plan-v3.json | 11 ++ sled-agent/src/params.rs | 8 + sled-agent/src/rack_setup/plan/service.rs | 39 ++++- sled-agent/src/services.rs | 61 ++++--- sled-agent/src/sim/server.rs | 24 ++- 34 files changed, 479 insertions(+), 143 deletions(-) create mode 100644 schema/crdb/add-nullable-filesystem-pool/up1.sql create mode 100644 schema/crdb/add-nullable-filesystem-pool/up2.sql diff --git a/clients/nexus-client/src/lib.rs b/clients/nexus-client/src/lib.rs index 9043592414..fc64c48a63 100644 --- a/clients/nexus-client/src/lib.rs +++ b/clients/nexus-client/src/lib.rs @@ -43,6 +43,7 @@ progenitor::generate_api!( TypedUuidForUpstairsKind = omicron_uuid_kinds::TypedUuid, TypedUuidForUpstairsRepairKind = omicron_uuid_kinds::TypedUuid, TypedUuidForUpstairsSessionKind = omicron_uuid_kinds::TypedUuid, + TypedUuidForZpoolKind = omicron_uuid_kinds::TypedUuid, }, patch = { SledAgentInfo = { derives = [PartialEq, Eq] }, diff --git a/dev-tools/reconfigurator-cli/tests/test_basic.rs b/dev-tools/reconfigurator-cli/tests/test_basic.rs index 1ae78487a3..efba4ee4f4 100644 --- a/dev-tools/reconfigurator-cli/tests/test_basic.rs +++ b/dev-tools/reconfigurator-cli/tests/test_basic.rs @@ -8,6 +8,7 @@ use expectorate::assert_contents; use nexus_db_queries::authn; use nexus_db_queries::authz; use nexus_db_queries::context::OpContext; +use nexus_test_utils::resource_helpers::DiskTestBuilder; use nexus_test_utils::SLED_AGENT_UUID; use nexus_test_utils_macros::nexus_test; use nexus_types::deployment::Blueprint; @@ -20,6 +21,7 @@ use omicron_test_utils::dev::test_cmds::path_to_executable; use omicron_test_utils::dev::test_cmds::redact_variable; use omicron_test_utils::dev::test_cmds::run_command; use omicron_test_utils::dev::test_cmds::EXIT_SUCCESS; +use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::SledUuid; use slog::debug; use std::io::BufReader; @@ -56,6 +58,24 @@ type ControlPlaneTestContext = #[nexus_test] async fn test_blueprint_edit(cptestctx: &ControlPlaneTestContext) { // Setup + // + // Add a zpool to both sleds, just to ensure that all new zones can find + // a transient filesystem wherever they end up being placed. + let _sled_agent_zpools = DiskTestBuilder::new(&cptestctx) + .on_sled(SledUuid::from_untyped_uuid( + cptestctx.sled_agent.sled_agent.id, + )) + .with_zpool_count(1) + .build() + .await; + let _sled_agent2_zpools = DiskTestBuilder::new(&cptestctx) + .on_sled(SledUuid::from_untyped_uuid( + cptestctx.sled_agent2.sled_agent.id, + )) + .with_zpool_count(1) + .build() + .await; + let nexus = &cptestctx.server.server_context().nexus; let datastore = nexus.datastore(); let log = &cptestctx.logctx.log; diff --git a/nexus/db-model/src/deployment.rs b/nexus/db-model/src/deployment.rs index e6a66543c7..c398f6d11c 100644 --- a/nexus/db-model/src/deployment.rs +++ b/nexus/db-model/src/deployment.rs @@ -31,7 +31,7 @@ use omicron_common::disk::DiskIdentity; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::SledUuid; use omicron_uuid_kinds::ZpoolUuid; -use omicron_uuid_kinds::{ExternalIpKind, SledKind}; +use omicron_uuid_kinds::{ExternalIpKind, SledKind, ZpoolKind}; use uuid::Uuid; /// See [`nexus_types::deployment::Blueprint`]. @@ -248,6 +248,7 @@ pub struct BpOmicronZone { disposition: DbBpZoneDisposition, pub external_ip_id: Option>, + pub filesystem_pool: Option>, } impl BpOmicronZone { @@ -264,6 +265,7 @@ impl BpOmicronZone { sled_id, blueprint_zone.id.into_untyped_uuid(), blueprint_zone.underlay_address, + blueprint_zone.filesystem_pool.as_ref().map(|pool| pool.id()), &blueprint_zone.zone_type.clone().into(), external_ip_id, )?; @@ -291,6 +293,10 @@ impl BpOmicronZone { snat_last_port: zone.snat_last_port, disposition: to_db_bp_zone_disposition(blueprint_zone.disposition), external_ip_id: zone.external_ip_id.map(From::from), + filesystem_pool: blueprint_zone + .filesystem_pool + .as_ref() + .map(|pool| pool.id().into()), }) } @@ -302,6 +308,7 @@ impl BpOmicronZone { sled_id: self.sled_id.into(), id: self.id, underlay_address: self.underlay_address, + filesystem_pool: self.filesystem_pool.map(|id| id.into()), zone_type: self.zone_type, primary_service_ip: self.primary_service_ip, primary_service_port: self.primary_service_port, diff --git a/nexus/db-model/src/inventory.rs b/nexus/db-model/src/inventory.rs index 4abd7fe927..14c4684e1e 100644 --- a/nexus/db-model/src/inventory.rs +++ b/nexus/db-model/src/inventory.rs @@ -37,6 +37,7 @@ use omicron_uuid_kinds::CollectionUuid; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::SledKind; use omicron_uuid_kinds::SledUuid; +use omicron_uuid_kinds::ZpoolKind; use omicron_uuid_kinds::ZpoolUuid; use uuid::Uuid; @@ -1025,6 +1026,7 @@ pub struct InvOmicronZone { pub snat_ip: Option, pub snat_first_port: Option, pub snat_last_port: Option, + pub filesystem_pool: Option>, } impl InvOmicronZone { @@ -1039,6 +1041,7 @@ impl InvOmicronZone { sled_id, zone.id, zone.underlay_address, + zone.filesystem_pool.as_ref().map(|pool| pool.id()), &zone.zone_type, external_ip_id, )?; @@ -1064,6 +1067,7 @@ impl InvOmicronZone { snat_ip: zone.snat_ip, snat_first_port: zone.snat_first_port, snat_last_port: zone.snat_last_port, + filesystem_pool: zone.filesystem_pool.map(|id| id.into()), }) } @@ -1075,6 +1079,7 @@ impl InvOmicronZone { sled_id: self.sled_id.into(), id: self.id, underlay_address: self.underlay_address, + filesystem_pool: self.filesystem_pool.map(|id| id.into()), zone_type: self.zone_type, primary_service_ip: self.primary_service_ip, primary_service_port: self.primary_service_port, diff --git a/nexus/db-model/src/omicron_zone_config.rs b/nexus/db-model/src/omicron_zone_config.rs index 3b18a749a7..bb3eac7046 100644 --- a/nexus/db-model/src/omicron_zone_config.rs +++ b/nexus/db-model/src/omicron_zone_config.rs @@ -23,8 +23,9 @@ use nexus_types::deployment::{ }; use nexus_types::inventory::{NetworkInterface, OmicronZoneType}; use omicron_common::api::internal::shared::NetworkInterfaceKind; +use omicron_common::zpool_name::ZpoolName; use omicron_uuid_kinds::{ - ExternalIpUuid, GenericUuid, OmicronZoneUuid, SledUuid, + ExternalIpUuid, GenericUuid, OmicronZoneUuid, SledUuid, ZpoolUuid, }; use std::net::{IpAddr, Ipv6Addr, SocketAddr, SocketAddrV6}; use uuid::Uuid; @@ -34,6 +35,7 @@ pub(crate) struct OmicronZone { pub(crate) sled_id: SledUuid, pub(crate) id: Uuid, pub(crate) underlay_address: ipv6::Ipv6Addr, + pub(crate) filesystem_pool: Option, pub(crate) zone_type: ZoneType, pub(crate) primary_service_ip: ipv6::Ipv6Addr, pub(crate) primary_service_port: SqlU16, @@ -60,6 +62,7 @@ impl OmicronZone { sled_id: SledUuid, zone_id: Uuid, zone_underlay_address: Ipv6Addr, + filesystem_pool: Option, zone_type: &nexus_types::inventory::OmicronZoneType, external_ip_id: Option, ) -> anyhow::Result { @@ -201,6 +204,7 @@ impl OmicronZone { sled_id, id, underlay_address, + filesystem_pool, zone_type, primary_service_ip, primary_service_port, @@ -365,6 +369,9 @@ impl OmicronZone { disposition, id: OmicronZoneUuid::from_untyped_uuid(common.id), underlay_address: std::net::Ipv6Addr::from(common.underlay_address), + filesystem_pool: common + .filesystem_pool + .map(|id| ZpoolName::new_external(id)), zone_type, }) } @@ -468,6 +475,9 @@ impl OmicronZone { Ok(nexus_types::inventory::OmicronZoneConfig { id: common.id, underlay_address: std::net::Ipv6Addr::from(common.underlay_address), + filesystem_pool: common + .filesystem_pool + .map(|id| ZpoolName::new_external(id)), zone_type, }) } @@ -558,6 +568,7 @@ impl OmicronZone { Ok(ZoneConfigCommon { id: self.id, underlay_address: self.underlay_address, + filesystem_pool: self.filesystem_pool, zone_type: self.zone_type, primary_service_address, snat_ip: self.snat_ip, @@ -582,6 +593,7 @@ impl OmicronZone { struct ZoneConfigCommon { id: Uuid, underlay_address: ipv6::Ipv6Addr, + filesystem_pool: Option, zone_type: ZoneType, primary_service_address: SocketAddrV6, snat_ip: Option, diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index 7fa07aa131..990aab151c 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -1476,6 +1476,7 @@ table! { snat_ip -> Nullable, snat_first_port -> Nullable, snat_last_port -> Nullable, + filesystem_pool -> Nullable, } } @@ -1592,6 +1593,7 @@ table! { snat_last_port -> Nullable, disposition -> crate::DbBpZoneDispositionEnum, external_ip_id -> Nullable, + filesystem_pool -> Nullable, } } diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 6f537bb522..8255d684cb 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -17,7 +17,8 @@ use std::collections::BTreeMap; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(80, 0, 0); +pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(81, 0, 0); + /// List of all past database schema versions, in *reverse* order /// /// If you want to change the Omicron database schema, you must update this. @@ -28,6 +29,7 @@ static KNOWN_VERSIONS: Lazy> = Lazy::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(81, "add-nullable-filesystem-pool"), KnownVersion::new(80, "add-instance-id-to-migrations"), KnownVersion::new(79, "nic-spoof-allow"), KnownVersion::new(78, "vpc-subnet-routing"), diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index 627f1f60ab..dac1c2847d 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -1035,6 +1035,7 @@ mod test { IdentityMetadataCreateParams, MacAddr, Vni, }; use omicron_common::api::internal::shared::SourceNatConfig; + use omicron_common::zpool_name::ZpoolName; use omicron_test_utils::dev; use omicron_uuid_kinds::{ExternalIpUuid, OmicronZoneUuid}; use omicron_uuid_kinds::{GenericUuid, ZpoolUuid}; @@ -1287,6 +1288,10 @@ mod test { fn_to_get_all!(ip_pool_range, IpPoolRange); fn_to_get_all!(dataset, Dataset); + fn random_zpool() -> ZpoolName { + ZpoolName::new_external(ZpoolUuid::new_v4()) + } + fn random_dataset() -> OmicronZoneDataset { OmicronZoneDataset { pool_name: illumos_utils::zpool::ZpoolName::new_external( @@ -1361,6 +1366,7 @@ mod test { let mut macs = MacAddr::iter_system(); let mut blueprint_zones = BTreeMap::new(); + let dataset = random_dataset(); blueprint_zones.insert( SledUuid::from_untyped_uuid(sled1.id()), BlueprintZonesConfig { @@ -1370,9 +1376,10 @@ mod test { disposition: BlueprintZoneDisposition::InService, id: external_dns_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(dataset.pool_name.clone()), zone_type: BlueprintZoneType::ExternalDns( blueprint_zone_type::ExternalDns { - dataset: random_dataset(), + dataset, http_address: "[::1]:80".parse().unwrap(), dns_address: OmicronZoneExternalFloatingAddr { id: ExternalIpUuid::new_v4(), @@ -1399,6 +1406,7 @@ mod test { disposition: BlueprintZoneDisposition::InService, id: ntp1_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(random_zpool()), zone_type: BlueprintZoneType::BoundaryNtp( blueprint_zone_type::BoundaryNtp { address: "[::1]:80".parse().unwrap(), @@ -1441,6 +1449,7 @@ mod test { disposition: BlueprintZoneDisposition::InService, id: nexus_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(random_zpool()), zone_type: BlueprintZoneType::Nexus( blueprint_zone_type::Nexus { internal_address: "[::1]:80".parse().unwrap(), @@ -1473,6 +1482,7 @@ mod test { disposition: BlueprintZoneDisposition::InService, id: ntp2_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(random_zpool()), zone_type: BlueprintZoneType::BoundaryNtp( blueprint_zone_type::BoundaryNtp { address: "[::1]:80".parse().unwrap(), @@ -1514,6 +1524,7 @@ mod test { disposition: BlueprintZoneDisposition::InService, id: ntp3_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(random_zpool()), zone_type: BlueprintZoneType::InternalNtp( blueprint_zone_type::InternalNtp { address: "[::1]:80".parse().unwrap(), @@ -1696,6 +1707,7 @@ mod test { disposition: BlueprintZoneDisposition::InService, id: nexus_id1, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(random_zpool()), zone_type: BlueprintZoneType::Nexus( blueprint_zone_type::Nexus { internal_address: "[::1]:80".parse().unwrap(), @@ -1728,6 +1740,7 @@ mod test { disposition: BlueprintZoneDisposition::InService, id: nexus_id2, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(random_zpool()), zone_type: BlueprintZoneType::Nexus( blueprint_zone_type::Nexus { internal_address: "[::1]:80".parse().unwrap(), @@ -1969,6 +1982,7 @@ mod test { disposition: BlueprintZoneDisposition::InService, id: nexus_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(random_zpool()), zone_type: BlueprintZoneType::Nexus( blueprint_zone_type::Nexus { internal_address: "[::1]:80".parse().unwrap(), @@ -2067,6 +2081,7 @@ mod test { let mut macs = MacAddr::iter_system(); let mut blueprint_zones = BTreeMap::new(); + let dataset = random_dataset(); blueprint_zones.insert( SledUuid::from_untyped_uuid(sled.id()), BlueprintZonesConfig { @@ -2076,9 +2091,10 @@ mod test { disposition: BlueprintZoneDisposition::InService, id: external_dns_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(dataset.pool_name.clone()), zone_type: BlueprintZoneType::ExternalDns( blueprint_zone_type::ExternalDns { - dataset: random_dataset(), + dataset, http_address: "[::1]:80".parse().unwrap(), dns_address: OmicronZoneExternalFloatingAddr { id: ExternalIpUuid::new_v4(), @@ -2105,6 +2121,7 @@ mod test { disposition: BlueprintZoneDisposition::InService, id: nexus_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(random_zpool()), zone_type: BlueprintZoneType::Nexus( blueprint_zone_type::Nexus { internal_address: "[::1]:80".parse().unwrap(), diff --git a/nexus/inventory/src/collector.rs b/nexus/inventory/src/collector.rs index a64d092e1c..b65d87505f 100644 --- a/nexus/inventory/src/collector.rs +++ b/nexus/inventory/src/collector.rs @@ -379,7 +379,9 @@ mod test { use gateway_messages::SpPort; use nexus_types::inventory::Collection; use omicron_common::api::external::Generation; + use omicron_common::zpool_name::ZpoolName; use omicron_sled_agent::sim; + use omicron_uuid_kinds::ZpoolUuid; use std::fmt::Write; use std::net::Ipv6Addr; use std::net::SocketAddrV6; @@ -547,6 +549,7 @@ mod test { let sled_url = format!("http://{}/", agent.http_server.local_addr()); let client = sled_agent_client::Client::new(&sled_url, log); + let filesystem_pool = ZpoolName::new_external(ZpoolUuid::new_v4()); let zone_address = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0); client .omicron_zones_put(&sled_agent_client::types::OmicronZonesConfig { @@ -558,6 +561,7 @@ mod test { sled_agent_client::types::OmicronZoneType::Oximeter { address: zone_address.to_string(), }, + filesystem_pool: Some(filesystem_pool), }], }) .await diff --git a/nexus/reconfigurator/execution/src/datasets.rs b/nexus/reconfigurator/execution/src/datasets.rs index c4f5cbae82..e007c2528e 100644 --- a/nexus/reconfigurator/execution/src/datasets.rs +++ b/nexus/reconfigurator/execution/src/datasets.rs @@ -278,6 +278,7 @@ mod tests { disposition: BlueprintZoneDisposition::InService, id: OmicronZoneUuid::new_v4(), underlay_address: "::1".parse().unwrap(), + filesystem_pool: Some(ZpoolName::new_external(new_zpool_id)), zone_type: BlueprintZoneType::Crucible( blueprint_zone_type::Crucible { address: "[::1]:0".parse().unwrap(), diff --git a/nexus/reconfigurator/execution/src/dns.rs b/nexus/reconfigurator/execution/src/dns.rs index ef4996db54..2dbb3eff5c 100644 --- a/nexus/reconfigurator/execution/src/dns.rs +++ b/nexus/reconfigurator/execution/src/dns.rs @@ -471,6 +471,7 @@ mod test { use nexus_reconfigurator_planning::example::example; use nexus_reconfigurator_preparation::PlanningInputFromDb; use nexus_test_utils::resource_helpers::create_silo; + use nexus_test_utils::resource_helpers::DiskTestBuilder; use nexus_test_utils_macros::nexus_test; use nexus_types::deployment::Blueprint; use nexus_types::deployment::BlueprintTarget; @@ -499,9 +500,12 @@ mod test { use omicron_common::address::SLED_PREFIX; use omicron_common::api::external::Generation; use omicron_common::api::external::IdentityMetadataCreateParams; + use omicron_common::zpool_name::ZpoolName; use omicron_test_utils::dev::test_setup_log; use omicron_uuid_kinds::ExternalIpUuid; + use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::OmicronZoneUuid; + use omicron_uuid_kinds::ZpoolUuid; use sled_agent_client::ZoneKind; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -621,6 +625,9 @@ mod test { disposition: BlueprintZoneDisposition::Quiesced, id: out_of_service_id, underlay_address: out_of_service_addr, + filesystem_pool: Some(ZpoolName::new_external( + ZpoolUuid::new_v4(), + )), zone_type: BlueprintZoneType::Oximeter( blueprint_zone_type::Oximeter { address: SocketAddrV6::new( @@ -1134,6 +1141,23 @@ mod test { async fn test_silos_external_dns_end_to_end( cptestctx: &ControlPlaneTestContext, ) { + // Add a zpool to both sleds, just to ensure that all new zones can find + // a transient filesystem wherever they end up being placed. + let _sled_agent_zpools = DiskTestBuilder::new(&cptestctx) + .on_sled(SledUuid::from_untyped_uuid( + cptestctx.sled_agent.sled_agent.id, + )) + .with_zpool_count(1) + .build() + .await; + let _sled_agent2_zpools = DiskTestBuilder::new(&cptestctx) + .on_sled(SledUuid::from_untyped_uuid( + cptestctx.sled_agent2.sled_agent.id, + )) + .with_zpool_count(1) + .build() + .await; + let nexus = &cptestctx.server.server_context().nexus; let datastore = nexus.datastore(); let log = &cptestctx.logctx.log; diff --git a/nexus/reconfigurator/execution/src/external_networking.rs b/nexus/reconfigurator/execution/src/external_networking.rs index 3ac1de96d5..a451eeda0f 100644 --- a/nexus/reconfigurator/execution/src/external_networking.rs +++ b/nexus/reconfigurator/execution/src/external_networking.rs @@ -445,7 +445,9 @@ mod tests { use omicron_common::address::NUM_SOURCE_NAT_PORTS; use omicron_common::api::external::MacAddr; use omicron_common::api::external::Vni; + use omicron_common::zpool_name::ZpoolName; use omicron_uuid_kinds::ExternalIpUuid; + use omicron_uuid_kinds::ZpoolUuid; use oxnet::IpNet; use std::net::IpAddr; use std::net::Ipv6Addr; @@ -593,6 +595,9 @@ mod tests { disposition: BlueprintZoneDisposition::InService, id: self.nexus_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(ZpoolName::new_external( + ZpoolUuid::new_v4(), + )), zone_type: BlueprintZoneType::Nexus( blueprint_zone_type::Nexus { internal_address: "[::1]:0".parse().unwrap(), @@ -607,6 +612,9 @@ mod tests { disposition: BlueprintZoneDisposition::InService, id: self.dns_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(ZpoolName::new_external( + ZpoolUuid::new_v4(), + )), zone_type: BlueprintZoneType::ExternalDns( blueprint_zone_type::ExternalDns { dataset: OmicronZoneDataset { @@ -624,6 +632,9 @@ mod tests { disposition: BlueprintZoneDisposition::InService, id: self.ntp_id, underlay_address: Ipv6Addr::LOCALHOST, + filesystem_pool: Some(ZpoolName::new_external( + ZpoolUuid::new_v4(), + )), zone_type: BlueprintZoneType::BoundaryNtp( blueprint_zone_type::BoundaryNtp { address: "[::1]:0".parse().unwrap(), diff --git a/nexus/reconfigurator/execution/src/omicron_zones.rs b/nexus/reconfigurator/execution/src/omicron_zones.rs index a40d65411b..08d41928c9 100644 --- a/nexus/reconfigurator/execution/src/omicron_zones.rs +++ b/nexus/reconfigurator/execution/src/omicron_zones.rs @@ -104,8 +104,10 @@ mod test { }; use nexus_types::inventory::OmicronZoneDataset; use omicron_common::api::external::Generation; + use omicron_common::zpool_name::ZpoolName; use omicron_uuid_kinds::OmicronZoneUuid; use omicron_uuid_kinds::SledUuid; + use omicron_uuid_kinds::ZpoolUuid; use std::collections::BTreeMap; use std::net::SocketAddr; use uuid::Uuid; @@ -183,19 +185,17 @@ mod test { // the full set of zones that must be running. // See `rack_setup::service::ServiceInner::run` for more details. fn make_zones() -> BlueprintZonesConfig { + let zpool = ZpoolName::new_external(ZpoolUuid::new_v4()); BlueprintZonesConfig { generation: Generation::new(), zones: vec![BlueprintZoneConfig { disposition: BlueprintZoneDisposition::InService, id: OmicronZoneUuid::new_v4(), underlay_address: "::1".parse().unwrap(), + filesystem_pool: Some(zpool.clone()), zone_type: BlueprintZoneType::InternalDns( blueprint_zone_type::InternalDns { - dataset: OmicronZoneDataset { - pool_name: format!("oxp_{}", Uuid::new_v4()) - .parse() - .unwrap(), - }, + dataset: OmicronZoneDataset { pool_name: zpool }, dns_address: "[::1]:0".parse().unwrap(), gz_address: "::1".parse().unwrap(), gz_address_index: 0, @@ -295,6 +295,9 @@ mod test { disposition, id: OmicronZoneUuid::new_v4(), underlay_address: "::1".parse().unwrap(), + filesystem_pool: Some(ZpoolName::new_external( + ZpoolUuid::new_v4(), + )), zone_type: BlueprintZoneType::InternalNtp( blueprint_zone_type::InternalNtp { address: "[::1]:0".parse().unwrap(), diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs index 1609a6c0cd..4177d4884f 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs @@ -6,7 +6,6 @@ use crate::ip_allocator::IpAllocator; use crate::planner::zone_needs_expungement; -use crate::planner::DiscretionaryOmicronZone; use crate::planner::ZoneExpungeReason; use anyhow::anyhow; use internal_dns::config::Host; @@ -631,7 +630,6 @@ impl<'a> BlueprintBuilder<'a> { let sled_subnet = sled_info.subnet; let ip = self.sled_alloc_ip(sled_id)?; let ntp_address = SocketAddrV6::new(ip, NTP_PORT, 0, 0); - // Construct the list of internal DNS servers. // // It'd be tempting to get this list from the other internal NTP @@ -659,18 +657,22 @@ impl<'a> BlueprintBuilder<'a> { }) .collect(); + let zone_type = + BlueprintZoneType::InternalNtp(blueprint_zone_type::InternalNtp { + address: ntp_address, + ntp_servers, + dns_servers, + domain: None, + }); + let filesystem_pool = + self.sled_select_zpool(sled_id, zone_type.kind())?; + let zone = BlueprintZoneConfig { disposition: BlueprintZoneDisposition::InService, id: self.rng.zone_rng.next(), underlay_address: ip, - zone_type: BlueprintZoneType::InternalNtp( - blueprint_zone_type::InternalNtp { - address: ntp_address, - ntp_servers, - dns_servers, - domain: None, - }, - ), + filesystem_pool: Some(filesystem_pool), + zone_type, }; self.sled_add_zone(sled_id, zone)?; @@ -713,16 +715,19 @@ impl<'a> BlueprintBuilder<'a> { let ip = self.sled_alloc_ip(sled_id)?; let port = omicron_common::address::CRUCIBLE_PORT; let address = SocketAddrV6::new(ip, port, 0, 0); + let zone_type = + BlueprintZoneType::Crucible(blueprint_zone_type::Crucible { + address, + dataset: OmicronZoneDataset { pool_name: pool_name.clone() }, + }); + let filesystem_pool = pool_name; + let zone = BlueprintZoneConfig { disposition: BlueprintZoneDisposition::InService, id: self.rng.zone_rng.next(), underlay_address: ip, - zone_type: BlueprintZoneType::Crucible( - blueprint_zone_type::Crucible { - address, - dataset: OmicronZoneDataset { pool_name }, - }, - ), + filesystem_pool: Some(filesystem_pool), + zone_type, }; self.sled_add_zone(sled_id, zone)?; @@ -835,19 +840,23 @@ impl<'a> BlueprintBuilder<'a> { let ip = self.sled_alloc_ip(sled_id)?; let port = omicron_common::address::NEXUS_INTERNAL_PORT; let internal_address = SocketAddrV6::new(ip, port, 0, 0); + let zone_type = + BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { + internal_address, + external_ip, + nic, + external_tls, + external_dns_servers: external_dns_servers.clone(), + }); + let filesystem_pool = + self.sled_select_zpool(sled_id, zone_type.kind())?; + let zone = BlueprintZoneConfig { disposition: BlueprintZoneDisposition::InService, id: nexus_id, underlay_address: ip, - zone_type: BlueprintZoneType::Nexus( - blueprint_zone_type::Nexus { - internal_address, - external_ip, - nic, - external_tls, - external_dns_servers: external_dns_servers.clone(), - }, - ), + filesystem_pool: Some(filesystem_pool), + zone_type, }; self.sled_add_zone(sled_id, zone)?; } @@ -884,22 +893,26 @@ impl<'a> BlueprintBuilder<'a> { for _ in 0..num_crdb_to_add { let zone_id = self.rng.zone_rng.next(); let underlay_ip = self.sled_alloc_ip(sled_id)?; - let pool_name = self.sled_alloc_zpool( - sled_id, - DiscretionaryOmicronZone::CockroachDb, - )?; + let pool_name = + self.sled_select_zpool(sled_id, ZoneKind::CockroachDb)?; let port = omicron_common::address::COCKROACH_PORT; let address = SocketAddrV6::new(underlay_ip, port, 0, 0); + let zone_type = BlueprintZoneType::CockroachDb( + blueprint_zone_type::CockroachDb { + address, + dataset: OmicronZoneDataset { + pool_name: pool_name.clone(), + }, + }, + ); + let filesystem_pool = pool_name; + let zone = BlueprintZoneConfig { disposition: BlueprintZoneDisposition::InService, id: zone_id, underlay_address: underlay_ip, - zone_type: BlueprintZoneType::CockroachDb( - blueprint_zone_type::CockroachDb { - address, - dataset: OmicronZoneDataset { pool_name }, - }, - ), + filesystem_pool: Some(filesystem_pool), + zone_type, }; self.sled_add_zone(sled_id, zone)?; } @@ -963,44 +976,40 @@ impl<'a> BlueprintBuilder<'a> { allocator.alloc().ok_or(Error::OutOfAddresses { sled_id }) } - fn sled_alloc_zpool( - &mut self, + // Selects a zpools for this zone type. + // + // This zpool may be used for either durable storage or transient + // storage - the usage is up to the caller. + // + // If `zone_kind` already exists on `sled_id`, it is prevented + // from using the same zpool as exisitng zones with the same kind. + fn sled_select_zpool( + &self, sled_id: SledUuid, - kind: DiscretionaryOmicronZone, + zone_kind: ZoneKind, ) -> Result { - let resources = self.sled_resources(sled_id)?; + let all_in_service_zpools = + self.sled_resources(sled_id)?.all_zpools(ZpoolFilter::InService); - // We refuse to choose a zpool for a zone of a given `kind` if this - // sled already has a zone of that kind on the same zpool. Build up a - // set of invalid zpools for this sled/kind pair. + // We refuse to choose a zpool for a zone of a given `zone_kind` if this + // sled already has a durable zone of that kind on the same zpool. Build + // up a set of invalid zpools for this sled/kind pair. let mut skip_zpools = BTreeSet::new(); for zone_config in self.current_sled_zones(sled_id) { - match (kind, &zone_config.zone_type) { - ( - DiscretionaryOmicronZone::Nexus, - BlueprintZoneType::Nexus(_), - ) => { - // TODO handle this case once we track transient datasets + if let Some(zpool) = zone_config.zone_type.durable_zpool() { + if zone_kind == zone_config.zone_type.kind() { + skip_zpools.insert(zpool); } - ( - DiscretionaryOmicronZone::CockroachDb, - BlueprintZoneType::CockroachDb(crdb), - ) => { - skip_zpools.insert(&crdb.dataset.pool_name); - } - (DiscretionaryOmicronZone::Nexus, _) - | (DiscretionaryOmicronZone::CockroachDb, _) => (), } } - for &zpool_id in resources.all_zpools(ZpoolFilter::InService) { + for &zpool_id in all_in_service_zpools { let zpool_name = ZpoolName::new_external(zpool_id); if !skip_zpools.contains(&zpool_name) { return Ok(zpool_name); } } - - Err(Error::NoAvailableZpool { sled_id, kind: kind.into() }) + Err(Error::NoAvailableZpool { sled_id, kind: zone_kind }) } fn sled_resources( @@ -1298,12 +1307,14 @@ pub mod test { // On any given zpool, we should have at most one zone of any given // kind. + // + // TODO: we may want a similar check for non-durable datasets? let mut kinds_by_zpool: BTreeMap< ZpoolUuid, BTreeMap, > = BTreeMap::new(); for (_, zone) in blueprint.all_omicron_zones(BlueprintZoneFilter::All) { - if let Some(dataset) = zone.zone_type.dataset() { + if let Some(dataset) = zone.zone_type.durable_dataset() { let kind = zone.zone_type.kind(); if let Some(previous) = kinds_by_zpool .entry(dataset.pool_name.id()) @@ -1647,6 +1658,31 @@ pub mod test { logctx.cleanup_successful(); } + // Tests that provisioning zones with durable zones co-locates their zone filesystems. + #[test] + fn test_zone_filesystem_zpool_colocated() { + static TEST_NAME: &str = + "blueprint_builder_test_zone_filesystem_zpool_colocated"; + let logctx = test_setup_log(TEST_NAME); + let (_, _, blueprint) = + example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS); + + for (_, zone_config) in &blueprint.blueprint_zones { + for zone in &zone_config.zones { + // The pool should only be optional for backwards compatibility. + let filesystem_pool = zone + .filesystem_pool + .as_ref() + .expect("Should have filesystem pool"); + + if let Some(durable_pool) = zone.zone_type.durable_zpool() { + assert_eq!(durable_pool, filesystem_pool); + } + } + } + logctx.cleanup_successful(); + } + #[test] fn test_add_nexus_with_no_existing_nexus_zones() { static TEST_NAME: &str = diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs b/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs index 68e2b9c2a2..6cb76539ec 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs @@ -190,6 +190,9 @@ mod tests { }; use maplit::btreeset; + use nexus_types::deployment::SledDisk; + use nexus_types::external_api::views::PhysicalDiskPolicy; + use nexus_types::external_api::views::PhysicalDiskState; use nexus_types::{ deployment::{ blueprint_zone_type, BlueprintZoneType, SledDetails, SledFilter, @@ -198,7 +201,11 @@ mod tests { external_api::views::{SledPolicy, SledState}, }; use omicron_common::address::Ipv6Subnet; + use omicron_common::disk::DiskIdentity; + use omicron_common::zpool_name::ZpoolName; use omicron_test_utils::dev::test_setup_log; + use omicron_uuid_kinds::PhysicalDiskUuid; + use omicron_uuid_kinds::ZpoolUuid; use crate::{ blueprint_builder::{ @@ -233,7 +240,19 @@ mod tests { subnet: Ipv6Subnet::new( "fd00:1::".parse().unwrap(), ), - zpools: BTreeMap::new(), + zpools: BTreeMap::from([( + ZpoolUuid::new_v4(), + SledDisk { + disk_identity: DiskIdentity { + vendor: String::from("fake-vendor"), + serial: String::from("fake-serial"), + model: String::from("fake-model"), + }, + disk_id: PhysicalDiskUuid::new_v4(), + policy: PhysicalDiskPolicy::InService, + state: PhysicalDiskState::Active, + }, + )]), }, }, ) @@ -280,11 +299,15 @@ mod tests { let change = builder.zones.change_sled_zones(existing_sled_id); let new_zone_id = OmicronZoneUuid::new_v4(); + // NOTE: This pool doesn't actually exist on the sled, but nothing is + // checking for that in this test? + let filesystem_pool = ZpoolName::new_external(ZpoolUuid::new_v4()); change .add_zone(BlueprintZoneConfig { disposition: BlueprintZoneDisposition::InService, id: new_zone_id, underlay_address: Ipv6Addr::UNSPECIFIED, + filesystem_pool: Some(filesystem_pool), zone_type: BlueprintZoneType::Oximeter( blueprint_zone_type::Oximeter { address: SocketAddrV6::new( diff --git a/nexus/reconfigurator/planning/src/planner.rs b/nexus/reconfigurator/planning/src/planner.rs index 4e288e8d8a..a4756f8405 100644 --- a/nexus/reconfigurator/planning/src/planner.rs +++ b/nexus/reconfigurator/planning/src/planner.rs @@ -672,7 +672,7 @@ pub(crate) fn zone_needs_expungement( } // Should we expunge the zone because durable storage is gone? - if let Some(durable_storage_zpool) = zone_config.zone_type.zpool() { + if let Some(durable_storage_zpool) = zone_config.zone_type.durable_zpool() { let zpool_id = durable_storage_zpool.id(); if !sled_details.resources.zpool_is_provisionable(&zpool_id) { return Some(ZoneExpungeReason::DiskExpunged); diff --git a/nexus/src/app/background/tasks/blueprint_execution.rs b/nexus/src/app/background/tasks/blueprint_execution.rs index 253a89a18d..451317f42a 100644 --- a/nexus/src/app/background/tasks/blueprint_execution.rs +++ b/nexus/src/app/background/tasks/blueprint_execution.rs @@ -129,9 +129,11 @@ mod test { use nexus_types::external_api::views::SledState; use nexus_types::inventory::OmicronZoneDataset; use omicron_common::api::external::Generation; + use omicron_common::zpool_name::ZpoolName; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::OmicronZoneUuid; use omicron_uuid_kinds::SledUuid; + use omicron_uuid_kinds::ZpoolUuid; use serde::Deserialize; use serde_json::json; use std::collections::BTreeMap; @@ -260,16 +262,18 @@ mod test { fn make_zones( disposition: BlueprintZoneDisposition, ) -> BlueprintZonesConfig { + let pool_id = ZpoolUuid::new_v4(); BlueprintZonesConfig { generation: Generation::new(), zones: vec![BlueprintZoneConfig { disposition, id: OmicronZoneUuid::new_v4(), underlay_address: "::1".parse().unwrap(), + filesystem_pool: Some(ZpoolName::new_external(pool_id)), zone_type: BlueprintZoneType::InternalDns( blueprint_zone_type::InternalDns { dataset: OmicronZoneDataset { - pool_name: format!("oxp_{}", Uuid::new_v4()) + pool_name: format!("oxp_{}", pool_id) .parse() .unwrap(), }, diff --git a/nexus/src/app/background/tasks/crdb_node_id_collector.rs b/nexus/src/app/background/tasks/crdb_node_id_collector.rs index 2a0e1c6d3d..0da411699e 100644 --- a/nexus/src/app/background/tasks/crdb_node_id_collector.rs +++ b/nexus/src/app/background/tasks/crdb_node_id_collector.rs @@ -241,8 +241,10 @@ mod tests { use nexus_test_utils::db::test_setup_database; use nexus_types::deployment::BlueprintZoneConfig; use nexus_types::deployment::BlueprintZoneDisposition; + use omicron_common::zpool_name::ZpoolName; use omicron_test_utils::dev; use omicron_uuid_kinds::SledUuid; + use omicron_uuid_kinds::ZpoolUuid; use std::collections::BTreeMap; use std::iter; use std::net::SocketAddr; @@ -265,16 +267,18 @@ mod tests { .get_mut(&sled_id) .expect("found entry for test sled"); + let zpool_id = ZpoolUuid::new_v4(); let make_crdb_zone_config = |disposition, id, addr: SocketAddrV6| BlueprintZoneConfig { disposition, id, underlay_address: *addr.ip(), + filesystem_pool: Some(ZpoolName::new_external(zpool_id)), zone_type: BlueprintZoneType::CockroachDb( blueprint_zone_type::CockroachDb { address: addr, dataset: nexus_types::inventory::OmicronZoneDataset { - pool_name: format!("oxp_{}", Uuid::new_v4()) + pool_name: format!("oxp_{}", zpool_id) .parse() .unwrap(), }, @@ -312,6 +316,7 @@ mod tests { disposition: BlueprintZoneDisposition::InService, id: OmicronZoneUuid::new_v4(), underlay_address: "::1".parse().unwrap(), + filesystem_pool: Some(ZpoolName::new_external(ZpoolUuid::new_v4())), zone_type: BlueprintZoneType::CruciblePantry( blueprint_zone_type::CruciblePantry { address: "[::1]:0".parse().unwrap(), diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index 7d69e6b3b0..b90f86285f 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -53,6 +53,7 @@ use omicron_common::api::internal::nexus::ProducerKind; use omicron_common::api::internal::shared::NetworkInterface; use omicron_common::api::internal::shared::NetworkInterfaceKind; use omicron_common::api::internal::shared::SwitchLocation; +use omicron_common::zpool_name::ZpoolName; use omicron_sled_agent::sim; use omicron_test_utils::dev; use omicron_uuid_kinds::ExternalIpUuid; @@ -421,6 +422,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { disposition: BlueprintZoneDisposition::InService, id: OmicronZoneUuid::from_untyped_uuid(dataset_id), underlay_address: *address.ip(), + filesystem_pool: Some(ZpoolName::new_external(zpool_id)), zone_type: BlueprintZoneType::CockroachDb( blueprint_zone_type::CockroachDb { address, @@ -473,6 +475,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { disposition: BlueprintZoneDisposition::InService, id: OmicronZoneUuid::from_untyped_uuid(dataset_id), underlay_address: *address.ip(), + filesystem_pool: Some(ZpoolName::new_external(zpool_id)), zone_type: BlueprintZoneType::Clickhouse( blueprint_zone_type::Clickhouse { address, @@ -661,6 +664,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { disposition: BlueprintZoneDisposition::InService, id: nexus_id, underlay_address: *address.ip(), + filesystem_pool: Some(ZpoolName::new_external(ZpoolUuid::new_v4())), zone_type: BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { external_dns_servers: self .config @@ -983,6 +987,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { disposition: BlueprintZoneDisposition::InService, id: zone_id, underlay_address: *address.ip(), + filesystem_pool: Some(ZpoolName::new_external(ZpoolUuid::new_v4())), zone_type: BlueprintZoneType::CruciblePantry( blueprint_zone_type::CruciblePantry { address }, ), @@ -1024,6 +1029,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { disposition: BlueprintZoneDisposition::InService, id: zone_id, underlay_address: *dropshot_address.ip(), + filesystem_pool: Some(ZpoolName::new_external(zpool_id)), zone_type: BlueprintZoneType::ExternalDns( blueprint_zone_type::ExternalDns { dataset: OmicronZoneDataset { pool_name }, @@ -1086,6 +1092,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { disposition: BlueprintZoneDisposition::InService, id: zone_id, underlay_address: *http_address.ip(), + filesystem_pool: Some(ZpoolName::new_external(zpool_id)), zone_type: BlueprintZoneType::InternalDns( blueprint_zone_type::InternalDns { dataset: OmicronZoneDataset { pool_name }, diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index ccebffd197..48eff399c7 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -45,6 +45,7 @@ use omicron_sled_agent::sim::SledAgent; use omicron_test_utils::dev::poll::wait_for_condition; use omicron_test_utils::dev::poll::CondCheckError; use omicron_uuid_kinds::GenericUuid; +use omicron_uuid_kinds::SledUuid; use omicron_uuid_kinds::ZpoolUuid; use oxnet::Ipv4Net; use oxnet::Ipv6Net; @@ -807,6 +808,53 @@ pub struct TestZpool { pub datasets: Vec, } +pub struct DiskTestBuilder<'a, N: NexusServer> { + cptestctx: &'a ControlPlaneTestContext, + sled_agent: Option>, + zpool_count: u32, +} + +impl<'a, N: NexusServer> DiskTestBuilder<'a, N> { + pub fn new(cptestctx: &'a ControlPlaneTestContext) -> Self { + Self { + cptestctx, + sled_agent: Some(cptestctx.sled_agent.sled_agent.clone()), + zpool_count: DiskTest::DEFAULT_ZPOOL_COUNT, + } + } + + /// Chooses a specific sled where disks should be added + pub fn on_sled(mut self, sled_id: SledUuid) -> Self { + // This list is hardcoded, because ControlPlaneTestContext is + // hardcoded to use two sled agents. + let sleds = [&self.cptestctx.sled_agent, &self.cptestctx.sled_agent2]; + + self.sled_agent = sleds.into_iter().find_map(|server| { + if server.sled_agent.id == sled_id.into_untyped_uuid() { + Some(server.sled_agent.clone()) + } else { + None + } + }); + self + } + + /// Selects a specific number of zpools to be created + pub fn with_zpool_count(mut self, count: u32) -> Self { + self.zpool_count = count; + self + } + + pub async fn build(self) -> DiskTest { + DiskTest::new_from_builder( + self.cptestctx, + self.sled_agent.expect("Sled Agent does not exist"), + self.zpool_count, + ) + .await + } +} + pub struct DiskTest { pub sled_agent: Arc, pub zpools: Vec, @@ -840,6 +888,21 @@ impl DiskTest { disk_test } + pub async fn new_from_builder( + cptestctx: &ControlPlaneTestContext, + sled_agent: Arc, + zpool_count: u32, + ) -> Self { + let mut disk_test = Self { sled_agent, zpools: vec![] }; + + // Create three Zpools, each 10 GiB, each with one Crucible dataset. + for _ in 0..zpool_count { + disk_test.add_zpool_with_dataset(cptestctx).await; + } + + disk_test + } + pub async fn add_zpool_with_dataset( &mut self, cptestctx: &ControlPlaneTestContext, diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index d64cde2d06..4f1984db7f 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -623,6 +623,7 @@ pub struct BlueprintZoneConfig { pub id: OmicronZoneUuid, pub underlay_address: Ipv6Addr, + pub filesystem_pool: Option, pub zone_type: BlueprintZoneType, } @@ -873,6 +874,7 @@ impl BlueprintZoneConfig { disposition, id: OmicronZoneUuid::from_untyped_uuid(config.id), underlay_address: config.underlay_address, + filesystem_pool: config.filesystem_pool, zone_type, }) } @@ -883,6 +885,7 @@ impl From for OmicronZoneConfig { Self { id: z.id.into_untyped_uuid(), underlay_address: z.underlay_address, + filesystem_pool: z.filesystem_pool, zone_type: z.zone_type.into(), } } diff --git a/nexus/types/src/deployment/planning_input.rs b/nexus/types/src/deployment/planning_input.rs index 028f2301ba..3e8bc9aa2c 100644 --- a/nexus/types/src/deployment/planning_input.rs +++ b/nexus/types/src/deployment/planning_input.rs @@ -402,6 +402,8 @@ pub struct SledResources { /// /// (used to allocate storage for control plane zones with persistent /// storage) + // NOTE: I'd really like to make this private, to make it harder to + // accidentally pick a zpool that is not in-service. pub zpools: BTreeMap, /// the IPv6 subnet of this sled on the underlay network diff --git a/nexus/types/src/deployment/zone_type.rs b/nexus/types/src/deployment/zone_type.rs index dc0fd98129..4c40bfc1de 100644 --- a/nexus/types/src/deployment/zone_type.rs +++ b/nexus/types/src/deployment/zone_type.rs @@ -35,33 +35,10 @@ pub enum BlueprintZoneType { impl BlueprintZoneType { /// Returns the zpool being used by this zone, if any. - pub fn zpool(&self) -> Option<&omicron_common::zpool_name::ZpoolName> { - match self { - BlueprintZoneType::ExternalDns( - blueprint_zone_type::ExternalDns { dataset, .. }, - ) - | BlueprintZoneType::Clickhouse( - blueprint_zone_type::Clickhouse { dataset, .. }, - ) - | BlueprintZoneType::ClickhouseKeeper( - blueprint_zone_type::ClickhouseKeeper { dataset, .. }, - ) - | BlueprintZoneType::CockroachDb( - blueprint_zone_type::CockroachDb { dataset, .. }, - ) - | BlueprintZoneType::Crucible(blueprint_zone_type::Crucible { - dataset, - .. - }) - | BlueprintZoneType::InternalDns( - blueprint_zone_type::InternalDns { dataset, .. }, - ) => Some(&dataset.pool_name), - BlueprintZoneType::BoundaryNtp(_) - | BlueprintZoneType::InternalNtp(_) - | BlueprintZoneType::Nexus(_) - | BlueprintZoneType::Oximeter(_) - | BlueprintZoneType::CruciblePantry(_) => None, - } + pub fn durable_zpool( + &self, + ) -> Option<&omicron_common::zpool_name::ZpoolName> { + self.durable_dataset().map(|dataset| &dataset.pool_name) } pub fn external_networking( @@ -141,12 +118,8 @@ impl BlueprintZoneType { } } - // Returns the dataset associated with this zone. - // - // TODO-cleanup This currently returns `None` for zones that only have - // transient datasets. This should change to a non-optional value once Nexus - // is aware of them. - pub fn dataset(&self) -> Option<&OmicronZoneDataset> { + /// Returns a durable dataset associated with this zone, if any exists. + pub fn durable_dataset(&self) -> Option<&OmicronZoneDataset> { match self { BlueprintZoneType::Clickhouse( blueprint_zone_type::Clickhouse { dataset, .. }, diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index 9d495a726c..6d380891aa 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -1967,6 +1967,14 @@ } ] }, + "filesystem_pool": { + "nullable": true, + "allOf": [ + { + "$ref": "#/components/schemas/ZpoolName" + } + ] + }, "id": { "$ref": "#/components/schemas/TypedUuidForOmicronZoneKind" }, diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index be13ba7a8b..8165cfa9d6 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -3732,6 +3732,15 @@ "description": "Describes one Omicron-managed zone running on a sled", "type": "object", "properties": { + "filesystem_pool": { + "nullable": true, + "description": "The pool on which we'll place this zone's filesystem.\n\nNote that this is transient -- the sled agent is permitted to destroy the zone's dataset on this pool each time the zone is initialized.", + "allOf": [ + { + "$ref": "#/components/schemas/ZpoolName" + } + ] + }, "id": { "type": "string", "format": "uuid" diff --git a/schema/all-zones-requests.json b/schema/all-zones-requests.json index 20b99b2064..910feb8c74 100644 --- a/schema/all-zones-requests.json +++ b/schema/all-zones-requests.json @@ -240,6 +240,17 @@ "zone_type" ], "properties": { + "filesystem_pool": { + "description": "The pool on which we'll place this zone's filesystem.\n\nNote that this is transient -- the sled agent is permitted to destroy the zone's dataset on this pool each time the zone is initialized.", + "anyOf": [ + { + "$ref": "#/definitions/ZpoolName" + }, + { + "type": "null" + } + ] + }, "id": { "type": "string", "format": "uuid" diff --git a/schema/crdb/add-nullable-filesystem-pool/up1.sql b/schema/crdb/add-nullable-filesystem-pool/up1.sql new file mode 100644 index 0000000000..53c58db51c --- /dev/null +++ b/schema/crdb/add-nullable-filesystem-pool/up1.sql @@ -0,0 +1 @@ +ALTER TABLE omicron.public.inv_omicron_zone ADD COLUMN IF NOT EXISTS filesystem_pool UUID; diff --git a/schema/crdb/add-nullable-filesystem-pool/up2.sql b/schema/crdb/add-nullable-filesystem-pool/up2.sql new file mode 100644 index 0000000000..f1a3b71e30 --- /dev/null +++ b/schema/crdb/add-nullable-filesystem-pool/up2.sql @@ -0,0 +1 @@ +ALTER TABLE omicron.public.bp_omicron_zone ADD COLUMN IF NOT EXISTS filesystem_pool UUID; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 990b1047a2..57f6838a55 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -3247,6 +3247,10 @@ CREATE TABLE IF NOT EXISTS omicron.public.inv_omicron_zone ( snat_last_port INT4 CHECK (snat_last_port IS NULL OR snat_last_port BETWEEN 0 AND 65535), + -- TODO: This is nullable for backwards compatibility. + -- Eventually, that nullability should be removed. + filesystem_pool UUID, + PRIMARY KEY (inv_collection_id, id) ); @@ -3493,6 +3497,10 @@ CREATE TABLE IF NOT EXISTS omicron.public.bp_omicron_zone ( -- created yet. external_ip_id UUID, + -- TODO: This is nullable for backwards compatibility. + -- Eventually, that nullability should be removed. + filesystem_pool UUID, + PRIMARY KEY (blueprint_id, id) ); @@ -4129,7 +4137,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '80.0.0', NULL) + (TRUE, NOW(), NOW(), '81.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/rss-service-plan-v3.json b/schema/rss-service-plan-v3.json index 481c92cc36..fd4b9c7064 100644 --- a/schema/rss-service-plan-v3.json +++ b/schema/rss-service-plan-v3.json @@ -397,6 +397,17 @@ "zone_type" ], "properties": { + "filesystem_pool": { + "description": "The pool on which we'll place this zone's filesystem.\n\nNote that this is transient -- the sled agent is permitted to destroy the zone's dataset on this pool each time the zone is initialized.", + "anyOf": [ + { + "$ref": "#/definitions/ZpoolName" + }, + { + "type": "null" + } + ] + }, "id": { "type": "string", "format": "uuid" diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index 9575e08c17..465a4abb56 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -341,6 +341,13 @@ impl From for sled_agent_client::types::OmicronZonesConfig { pub struct OmicronZoneConfig { pub id: Uuid, pub underlay_address: Ipv6Addr, + + /// The pool on which we'll place this zone's filesystem. + /// + /// Note that this is transient -- the sled agent is permitted to + /// destroy the zone's dataset on this pool each time the zone is + /// initialized. + pub filesystem_pool: Option, pub zone_type: OmicronZoneType, } @@ -349,6 +356,7 @@ impl From for sled_agent_client::types::OmicronZoneConfig { Self { id: local.id, underlay_address: local.underlay_address, + filesystem_pool: local.filesystem_pool, zone_type: local.zone_type.into(), } } diff --git a/sled-agent/src/rack_setup/plan/service.rs b/sled-agent/src/rack_setup/plan/service.rs index f13c15723c..39235b91eb 100644 --- a/sled-agent/src/rack_setup/plan/service.rs +++ b/sled-agent/src/rack_setup/plan/service.rs @@ -31,6 +31,7 @@ use omicron_common::backoff::{ }; use omicron_common::ledger::{self, Ledger, Ledgerable}; use omicron_uuid_kinds::{GenericUuid, OmicronZoneUuid, SledUuid, ZpoolUuid}; +use rand::prelude::SliceRandom; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use sled_agent_client::{ @@ -404,7 +405,8 @@ impl Plan { ) .unwrap(); let dataset_name = - sled.alloc_from_u2_zpool(DatasetKind::InternalDns)?; + sled.alloc_dataset_from_u2s(DatasetKind::InternalDns)?; + let filesystem_pool = Some(dataset_name.pool().clone()); sled.request.zones.push(OmicronZoneConfig { // TODO-cleanup use TypedUuid everywhere @@ -419,6 +421,7 @@ impl Plan { gz_address: dns_subnet.gz_address(), gz_address_index: i.try_into().expect("Giant indices?"), }, + filesystem_pool, }); } @@ -442,7 +445,8 @@ impl Plan { ) .unwrap(); let dataset_name = - sled.alloc_from_u2_zpool(DatasetKind::CockroachDb)?; + sled.alloc_dataset_from_u2s(DatasetKind::CockroachDb)?; + let filesystem_pool = Some(dataset_name.pool().clone()); sled.request.zones.push(OmicronZoneConfig { // TODO-cleanup use TypedUuid everywhere id: id.into_untyped_uuid(), @@ -453,6 +457,7 @@ impl Plan { }, address, }, + filesystem_pool, }); } @@ -485,7 +490,8 @@ impl Plan { let dns_port = omicron_common::address::DNS_PORT; let dns_address = SocketAddr::new(external_ip, dns_port); let dataset_kind = DatasetKind::ExternalDns; - let dataset_name = sled.alloc_from_u2_zpool(dataset_kind)?; + let dataset_name = sled.alloc_dataset_from_u2s(dataset_kind)?; + let filesystem_pool = Some(dataset_name.pool().clone()); sled.request.zones.push(OmicronZoneConfig { // TODO-cleanup use TypedUuid everywhere @@ -499,6 +505,7 @@ impl Plan { dns_address, nic, }, + filesystem_pool, }); } @@ -520,6 +527,7 @@ impl Plan { ) .unwrap(); let (nic, external_ip) = svc_port_builder.next_nexus(id)?; + let filesystem_pool = Some(sled.alloc_zpool_from_u2s()?); sled.request.zones.push(OmicronZoneConfig { // TODO-cleanup use TypedUuid everywhere id: id.into_untyped_uuid(), @@ -542,6 +550,7 @@ impl Plan { external_tls: !config.external_certificates.is_empty(), external_dns_servers: config.dns_servers.clone(), }, + filesystem_pool, }); } @@ -563,6 +572,7 @@ impl Plan { omicron_common::address::OXIMETER_PORT, ) .unwrap(); + let filesystem_pool = Some(sled.alloc_zpool_from_u2s()?); sled.request.zones.push(OmicronZoneConfig { // TODO-cleanup use TypedUuid everywhere id: id.into_untyped_uuid(), @@ -575,6 +585,7 @@ impl Plan { 0, ), }, + filesystem_pool, }) } @@ -599,7 +610,8 @@ impl Plan { ) .unwrap(); let dataset_name = - sled.alloc_from_u2_zpool(DatasetKind::Clickhouse)?; + sled.alloc_dataset_from_u2s(DatasetKind::Clickhouse)?; + let filesystem_pool = Some(dataset_name.pool().clone()); sled.request.zones.push(OmicronZoneConfig { // TODO-cleanup use TypedUuid everywhere id: id.into_untyped_uuid(), @@ -610,6 +622,7 @@ impl Plan { pool_name: dataset_name.pool().clone(), }, }, + filesystem_pool, }); } @@ -636,7 +649,8 @@ impl Plan { ) .unwrap(); let dataset_name = - sled.alloc_from_u2_zpool(DatasetKind::ClickhouseKeeper)?; + sled.alloc_dataset_from_u2s(DatasetKind::ClickhouseKeeper)?; + let filesystem_pool = Some(dataset_name.pool().clone()); sled.request.zones.push(OmicronZoneConfig { // TODO-cleanup use TypedUuid everywhere id: id.into_untyped_uuid(), @@ -647,6 +661,7 @@ impl Plan { pool_name: dataset_name.pool().clone(), }, }, + filesystem_pool, }); } @@ -661,6 +676,7 @@ impl Plan { let address = sled.addr_alloc.next().expect("Not enough addrs"); let port = omicron_common::address::CRUCIBLE_PANTRY_PORT; let id = OmicronZoneUuid::new_v4(); + let filesystem_pool = Some(sled.alloc_zpool_from_u2s()?); dns_builder .host_zone_with_one_backend( id, @@ -676,6 +692,7 @@ impl Plan { zone_type: OmicronZoneType::CruciblePantry { address: SocketAddrV6::new(address, port, 0, 0), }, + filesystem_pool, }); } @@ -704,6 +721,7 @@ impl Plan { address, dataset: OmicronZoneDataset { pool_name: pool.clone() }, }, + filesystem_pool: Some(pool.clone()), }); } } @@ -716,6 +734,7 @@ impl Plan { let id = OmicronZoneUuid::new_v4(); let address = sled.addr_alloc.next().expect("Not enough addrs"); let ntp_address = SocketAddrV6::new(address, NTP_PORT, 0, 0); + let filesystem_pool = Some(sled.alloc_zpool_from_u2s()?); let (zone_type, svcname) = if idx < BOUNDARY_NTP_COUNT { boundary_ntp_servers @@ -753,6 +772,7 @@ impl Plan { id: id.into_untyped_uuid(), underlay_address: address, zone_type, + filesystem_pool, }); } @@ -878,9 +898,16 @@ impl SledInfo { } } + fn alloc_zpool_from_u2s(&self) -> Result { + self.u2_zpools + .choose(&mut rand::thread_rng()) + .map(|z| z.clone()) + .ok_or_else(|| PlanError::NotEnoughSleds) + } + /// Allocates a dataset of the specified type from one of the U.2 pools on /// this Sled - fn alloc_from_u2_zpool( + fn alloc_dataset_from_u2s( &mut self, kind: DatasetKind, ) -> Result { diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index fb57990f1b..f4e9f8da0a 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -2098,6 +2098,7 @@ impl ServiceManager { }, underlay_address, id, + .. }, .. }) => { @@ -3291,13 +3292,13 @@ impl ServiceManager { ) -> Result { let name = zone.zone_name(); - // For each new zone request, we pick a U.2 to store the zone - // filesystem. Note: This isn't known to Nexus right now, so it's a - // local-to-sled decision. + // If the caller has requested a specific durable dataset, + // ensure that it is encrypted and that it exists. // - // Currently, the zone filesystem should be destroyed between - // reboots, so it's fine to make this decision locally. - let root = if let Some(dataset) = zone.dataset_name() { + // Typically, the transient filesystem pool will be placed on the same + // zpool as the durable dataset (to reduce the fault domain), but that + // decision belongs to Nexus, and is not enforced here. + if let Some(dataset) = zone.dataset_name() { // Check that the dataset is actually ready to be used. let [zoned, canmount, encryption] = illumos_utils::zfs::Zfs::get_values( @@ -3327,11 +3328,6 @@ impl ServiceManager { check_property("encryption", encryption, "aes-256-gcm")?; } - // If the zone happens to already manage a dataset, then - // we co-locate the zone dataset on the same zpool. - // - // This slightly reduces the underlying fault domain for the - // service. let data_pool = dataset.pool(); if !all_u2_pools.contains(&data_pool) { warn!( @@ -3344,20 +3340,35 @@ impl ServiceManager { device: format!("zpool: {data_pool}"), }); } - data_pool.dataset_mountpoint(&mount_config.root, ZONE_DATASET) - } else { - // If the zone it not coupled to other datsets, we pick one - // arbitrarily. - let mut rng = rand::thread_rng(); - all_u2_pools - .choose(&mut rng) - .map(|pool| { - pool.dataset_mountpoint(&mount_config.root, ZONE_DATASET) - }) + } + + let filesystem_pool = match (&zone.filesystem_pool, zone.dataset_name()) + { + // If a pool was explicitly requested, use it. + (Some(pool), _) => pool.clone(), + // NOTE: The following cases are for backwards compatibility. + // + // If no pool was selected, prefer to use the same pool as the + // durable dataset. Otherwise, pick one randomly. + (None, Some(dataset)) => dataset.pool().clone(), + (None, None) => all_u2_pools + .choose(&mut rand::thread_rng()) .ok_or_else(|| Error::U2NotFound)? - .clone() + .clone(), }; - Ok(root) + + if !all_u2_pools.contains(&filesystem_pool) { + warn!( + self.inner.log, + "zone filesystem dataset requested on a zpool which doesn't exist"; + "zone" => &name, + "zpool" => %filesystem_pool + ); + return Err(Error::MissingDevice { + device: format!("zpool: {filesystem_pool}"), + }); + } + Ok(filesystem_pool.dataset_mountpoint(&mount_config.root, ZONE_DATASET)) } pub async fn cockroachdb_initialize(&self) -> Result<(), Error> { @@ -4361,6 +4372,7 @@ mod test { id, underlay_address: Ipv6Addr::LOCALHOST, zone_type, + filesystem_pool: None, }], }, Some(&tmp_dir), @@ -4392,6 +4404,7 @@ mod test { dns_servers: vec![], domain: None, }, + filesystem_pool: None, }], }, Some(&tmp_dir), @@ -4808,6 +4821,7 @@ mod test { dns_servers: vec![], domain: None, }, + filesystem_pool: None, }]; let tmp_dir = String::from(test_config.config_dir.path().as_str()); @@ -4836,6 +4850,7 @@ mod test { dns_servers: vec![], domain: None, }, + filesystem_pool: None, }); // Now try to apply that list with an older generation number. This diff --git a/sled-agent/src/sim/server.rs b/sled-agent/src/sim/server.rs index 5b66342a1a..215cb7d5f4 100644 --- a/sled-agent/src/sim/server.rs +++ b/sled-agent/src/sim/server.rs @@ -356,19 +356,27 @@ pub async fn run_standalone_server( let internal_dns_version = Generation::try_from(dns_config.generation) .expect("invalid internal dns version"); + let all_u2_zpools = server.sled_agent.get_zpools().await; + let get_random_zpool = || { + use rand::seq::SliceRandom; + let pool = all_u2_zpools + .choose(&mut rand::thread_rng()) + .expect("No external zpools found, but we need one"); + ZpoolName::new_external(ZpoolUuid::from_untyped_uuid(pool.id)) + }; + // Record the internal DNS server as though RSS had provisioned it so // that Nexus knows about it. let http_bound = match dns.dropshot_server.local_addr() { SocketAddr::V4(_) => panic!("did not expect v4 address"), SocketAddr::V6(a) => a, }; + let pool_name = ZpoolName::new_external(ZpoolUuid::new_v4()); let mut zones = vec![OmicronZoneConfig { id: Uuid::new_v4(), underlay_address: *http_bound.ip(), zone_type: OmicronZoneType::InternalDns { - dataset: OmicronZoneDataset { - pool_name: ZpoolName::new_external(ZpoolUuid::new_v4()), - }, + dataset: OmicronZoneDataset { pool_name: pool_name.clone() }, http_address: http_bound, dns_address: match dns.dns_server.local_address() { SocketAddr::V4(_) => panic!("did not expect v4 address"), @@ -377,6 +385,8 @@ pub async fn run_standalone_server( gz_address: Ipv6Addr::LOCALHOST, gz_address_index: 0, }, + // Co-locate the filesystem pool with the dataset + filesystem_pool: Some(pool_name), }]; let mut internal_services_ip_pool_ranges = vec![]; @@ -415,6 +425,7 @@ pub async fn run_standalone_server( external_tls: false, external_dns_servers: vec![], }, + filesystem_pool: Some(get_random_zpool()), }); internal_services_ip_pool_ranges.push(match ip { @@ -432,13 +443,12 @@ pub async fn run_standalone_server( { let ip = *external_dns_internal_addr.ip(); let id = Uuid::new_v4(); + let pool_name = ZpoolName::new_external(ZpoolUuid::new_v4()); zones.push(OmicronZoneConfig { id, underlay_address: ip, zone_type: OmicronZoneType::ExternalDns { - dataset: OmicronZoneDataset { - pool_name: ZpoolName::new_external(ZpoolUuid::new_v4()), - }, + dataset: OmicronZoneDataset { pool_name: pool_name.clone() }, http_address: external_dns_internal_addr, dns_address: SocketAddr::V6(external_dns_internal_addr), nic: nexus_types::inventory::NetworkInterface { @@ -457,6 +467,8 @@ pub async fn run_standalone_server( transit_ips: vec![], }, }, + // Co-locate the filesystem pool with the dataset + filesystem_pool: Some(pool_name), }); internal_services_ip_pool_ranges