Skip to content

Commit dd6e250

Browse files
jgallagherhawkw
jgallagher
authored and
hawkw
committed
BlueprintZoneDisposition::Expunged - track additional properties (#7558)
The `Expunged` zone disposition can now also track: * the config generation in which the zone was expunged * whether we've confirmed that the zone is ready for any post-expungment cleanup Currently `ready_for_cleanup` is always false; the planner will fill it in to `true` in a subsequent PR. While I was modifying this enum: removed the `Quiesced` state, which we don't use yet (and it's not clear how that will need to be represented when we get to it in the future).
1 parent 454b8aa commit dd6e250

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+892
-534
lines changed

‎dev-tools/reconfigurator-cli/tests/output/cmd-expunge-newly-added-stdout‎

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -574,24 +574,24 @@ parent: 3f00b694-1b16-4aaa-8f78-e6b3a527b434
574574

575575

576576
omicron zones at generation 3:
577-
---------------------------------------------------------------------------------------------
578-
zone type zone id disposition underlay IP
579-
---------------------------------------------------------------------------------------------
580-
crucible 0903299a-6296-44f0-8ab9-7c70b5766a05 in service fd00:1122:3344:101::25
581-
crucible 2b6b3cfd-4524-465b-ac6e-10be9ab6d4d4 in service fd00:1122:3344:101::27
582-
crucible 3e0ff677-2ca8-4124-9fef-7c23ac2da6fa in service fd00:1122:3344:101::2d
583-
crucible 512758d0-335f-4c94-9afe-c82f6f127421 in service fd00:1122:3344:101::2a
584-
crucible 8ae67b12-ca74-40d3-a55e-90456cd623ea in service fd00:1122:3344:101::26
585-
crucible 918c8cbc-1c62-4de5-9cf2-c4153a3d1a7e in service fd00:1122:3344:101::2e
586-
crucible be422e7a-16d0-426e-8567-da0aed7200d4 in service fd00:1122:3344:101::28
587-
crucible ea50f438-1b5e-4d61-bd65-46d360f590ee in service fd00:1122:3344:101::2b
588-
crucible f055a042-9d1b-4931-8268-d94cac801d7e in service fd00:1122:3344:101::29
589-
crucible f33e09d1-e3d7-4341-baf2-5f079f3679e5 in service fd00:1122:3344:101::2c
590-
crucible_pantry 100e20aa-1816-4195-8d76-08beace10332 in service fd00:1122:3344:101::24
591-
external_dns 9995de32-dd52-4eb1-b0eb-141eb84bc739 expunged fd00:1122:3344:101::23
592-
internal_dns 171cabf0-8635-42e7-8ea1-2a1d2a2daf55 in service fd00:1122:3344:3::1
593-
internal_ntp 7b4bccad-9869-48da-8320-2dc82752deea in service fd00:1122:3344:101::21
594-
nexus 5b01b0e0-5cdf-4b34-b0a6-c947cbbdb8be in service fd00:1122:3344:101::22
577+
----------------------------------------------------------------------------------------------
578+
zone type zone id disposition underlay IP
579+
----------------------------------------------------------------------------------------------
580+
crucible 0903299a-6296-44f0-8ab9-7c70b5766a05 in service fd00:1122:3344:101::25
581+
crucible 2b6b3cfd-4524-465b-ac6e-10be9ab6d4d4 in service fd00:1122:3344:101::27
582+
crucible 3e0ff677-2ca8-4124-9fef-7c23ac2da6fa in service fd00:1122:3344:101::2d
583+
crucible 512758d0-335f-4c94-9afe-c82f6f127421 in service fd00:1122:3344:101::2a
584+
crucible 8ae67b12-ca74-40d3-a55e-90456cd623ea in service fd00:1122:3344:101::26
585+
crucible 918c8cbc-1c62-4de5-9cf2-c4153a3d1a7e in service fd00:1122:3344:101::2e
586+
crucible be422e7a-16d0-426e-8567-da0aed7200d4 in service fd00:1122:3344:101::28
587+
crucible ea50f438-1b5e-4d61-bd65-46d360f590ee in service fd00:1122:3344:101::2b
588+
crucible f055a042-9d1b-4931-8268-d94cac801d7e in service fd00:1122:3344:101::29
589+
crucible f33e09d1-e3d7-4341-baf2-5f079f3679e5 in service fd00:1122:3344:101::2c
590+
crucible_pantry 100e20aa-1816-4195-8d76-08beace10332 in service fd00:1122:3344:101::24
591+
external_dns 9995de32-dd52-4eb1-b0eb-141eb84bc739 expunged ⏳ fd00:1122:3344:101::23
592+
internal_dns 171cabf0-8635-42e7-8ea1-2a1d2a2daf55 in service fd00:1122:3344:3::1
593+
internal_ntp 7b4bccad-9869-48da-8320-2dc82752deea in service fd00:1122:3344:101::21
594+
nexus 5b01b0e0-5cdf-4b34-b0a6-c947cbbdb8be in service fd00:1122:3344:101::22
595595

596596

597597
COCKROACHDB SETTINGS:
@@ -890,25 +890,25 @@ parent: 366b0b68-d80e-4bc1-abd3-dc69837847e0
890890

891891

892892
omicron zones at generation 4:
893-
---------------------------------------------------------------------------------------------
894-
zone type zone id disposition underlay IP
895-
---------------------------------------------------------------------------------------------
896-
crucible 0903299a-6296-44f0-8ab9-7c70b5766a05 in service fd00:1122:3344:101::25
897-
crucible 2b6b3cfd-4524-465b-ac6e-10be9ab6d4d4 in service fd00:1122:3344:101::27
898-
crucible 3e0ff677-2ca8-4124-9fef-7c23ac2da6fa in service fd00:1122:3344:101::2d
899-
crucible 512758d0-335f-4c94-9afe-c82f6f127421 in service fd00:1122:3344:101::2a
900-
crucible 8ae67b12-ca74-40d3-a55e-90456cd623ea in service fd00:1122:3344:101::26
901-
crucible 918c8cbc-1c62-4de5-9cf2-c4153a3d1a7e in service fd00:1122:3344:101::2e
902-
crucible be422e7a-16d0-426e-8567-da0aed7200d4 in service fd00:1122:3344:101::28
903-
crucible ea50f438-1b5e-4d61-bd65-46d360f590ee in service fd00:1122:3344:101::2b
904-
crucible f055a042-9d1b-4931-8268-d94cac801d7e in service fd00:1122:3344:101::29
905-
crucible f33e09d1-e3d7-4341-baf2-5f079f3679e5 in service fd00:1122:3344:101::2c
906-
crucible_pantry 100e20aa-1816-4195-8d76-08beace10332 in service fd00:1122:3344:101::24
907-
external_dns 9995de32-dd52-4eb1-b0eb-141eb84bc739 expunged fd00:1122:3344:101::23
908-
external_dns d786ef4a-5acb-4f5d-a732-a00addf986b5 in service fd00:1122:3344:101::2f
909-
internal_dns 171cabf0-8635-42e7-8ea1-2a1d2a2daf55 in service fd00:1122:3344:3::1
910-
internal_ntp 7b4bccad-9869-48da-8320-2dc82752deea in service fd00:1122:3344:101::21
911-
nexus 5b01b0e0-5cdf-4b34-b0a6-c947cbbdb8be in service fd00:1122:3344:101::22
893+
----------------------------------------------------------------------------------------------
894+
zone type zone id disposition underlay IP
895+
----------------------------------------------------------------------------------------------
896+
crucible 0903299a-6296-44f0-8ab9-7c70b5766a05 in service fd00:1122:3344:101::25
897+
crucible 2b6b3cfd-4524-465b-ac6e-10be9ab6d4d4 in service fd00:1122:3344:101::27
898+
crucible 3e0ff677-2ca8-4124-9fef-7c23ac2da6fa in service fd00:1122:3344:101::2d
899+
crucible 512758d0-335f-4c94-9afe-c82f6f127421 in service fd00:1122:3344:101::2a
900+
crucible 8ae67b12-ca74-40d3-a55e-90456cd623ea in service fd00:1122:3344:101::26
901+
crucible 918c8cbc-1c62-4de5-9cf2-c4153a3d1a7e in service fd00:1122:3344:101::2e
902+
crucible be422e7a-16d0-426e-8567-da0aed7200d4 in service fd00:1122:3344:101::28
903+
crucible ea50f438-1b5e-4d61-bd65-46d360f590ee in service fd00:1122:3344:101::2b
904+
crucible f055a042-9d1b-4931-8268-d94cac801d7e in service fd00:1122:3344:101::29
905+
crucible f33e09d1-e3d7-4341-baf2-5f079f3679e5 in service fd00:1122:3344:101::2c
906+
crucible_pantry 100e20aa-1816-4195-8d76-08beace10332 in service fd00:1122:3344:101::24
907+
external_dns 9995de32-dd52-4eb1-b0eb-141eb84bc739 expunged ⏳ fd00:1122:3344:101::23
908+
external_dns d786ef4a-5acb-4f5d-a732-a00addf986b5 in service fd00:1122:3344:101::2f
909+
internal_dns 171cabf0-8635-42e7-8ea1-2a1d2a2daf55 in service fd00:1122:3344:3::1
910+
internal_ntp 7b4bccad-9869-48da-8320-2dc82752deea in service fd00:1122:3344:101::21
911+
nexus 5b01b0e0-5cdf-4b34-b0a6-c947cbbdb8be in service fd00:1122:3344:101::22
912912

913913

914914
COCKROACHDB SETTINGS:

‎nexus/db-model/src/deployment.rs‎

Lines changed: 70 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,8 @@ pub struct BpOmicronZone {
467467
pub snat_last_port: Option<SqlU16>,
468468

469469
disposition: DbBpZoneDisposition,
470+
disposition_expunged_as_of_generation: Option<Generation>,
471+
disposition_expunged_ready_for_cleanup: bool,
470472

471473
pub external_ip_id: Option<DbTypedUuid<ExternalIpKind>>,
472474
pub filesystem_pool: Option<DbTypedUuid<ZpoolKind>>,
@@ -483,6 +485,12 @@ impl BpOmicronZone {
483485
.external_networking()
484486
.map(|(ip, _)| ip.id().into());
485487

488+
let DbBpZoneDispositionColumns {
489+
disposition,
490+
expunged_as_of_generation: disposition_expunged_as_of_generation,
491+
expunged_ready_for_cleanup: disposition_expunged_ready_for_cleanup,
492+
} = blueprint_zone.disposition.into();
493+
486494
// Create a dummy record to start, then fill in the rest
487495
let mut bp_omicron_zone = BpOmicronZone {
488496
// Fill in the known fields that don't require inspecting
@@ -495,7 +503,9 @@ impl BpOmicronZone {
495503
.filesystem_pool
496504
.as_ref()
497505
.map(|pool| pool.id().into()),
498-
disposition: to_db_bp_zone_disposition(blueprint_zone.disposition),
506+
disposition,
507+
disposition_expunged_as_of_generation,
508+
disposition_expunged_ready_for_cleanup,
499509
zone_type: blueprint_zone.zone_type.kind().into(),
500510

501511
// Set the remainder of the fields to a default
@@ -879,8 +889,16 @@ impl BpOmicronZone {
879889
}
880890
};
881891

892+
let disposition_cols = DbBpZoneDispositionColumns {
893+
disposition: self.disposition,
894+
expunged_as_of_generation: self
895+
.disposition_expunged_as_of_generation,
896+
expunged_ready_for_cleanup: self
897+
.disposition_expunged_ready_for_cleanup,
898+
};
899+
882900
Ok(BlueprintZoneConfig {
883-
disposition: self.disposition.into(),
901+
disposition: disposition_cols.try_into()?,
884902
id: self.id.into(),
885903
filesystem_pool: self
886904
.filesystem_pool
@@ -907,30 +925,63 @@ impl_enum_type!(
907925

908926
// Enum values
909927
InService => b"in_service"
910-
Quiesced => b"quiesced"
911928
Expunged => b"expunged"
912929
);
913930

914-
/// Converts a [`BlueprintZoneDisposition`] to a version that can be inserted
915-
/// into a database.
916-
pub fn to_db_bp_zone_disposition(
917-
disposition: BlueprintZoneDisposition,
918-
) -> DbBpZoneDisposition {
919-
match disposition {
920-
BlueprintZoneDisposition::InService => DbBpZoneDisposition::InService,
921-
BlueprintZoneDisposition::Quiesced => DbBpZoneDisposition::Quiesced,
922-
BlueprintZoneDisposition::Expunged => DbBpZoneDisposition::Expunged,
931+
struct DbBpZoneDispositionColumns {
932+
disposition: DbBpZoneDisposition,
933+
expunged_as_of_generation: Option<Generation>,
934+
expunged_ready_for_cleanup: bool,
935+
}
936+
937+
impl From<BlueprintZoneDisposition> for DbBpZoneDispositionColumns {
938+
fn from(value: BlueprintZoneDisposition) -> Self {
939+
let (
940+
disposition,
941+
disposition_expunged_as_of_generation,
942+
disposition_expunged_ready_for_cleanup,
943+
) = match value {
944+
BlueprintZoneDisposition::InService => {
945+
(DbBpZoneDisposition::InService, None, false)
946+
}
947+
BlueprintZoneDisposition::Expunged {
948+
as_of_generation,
949+
ready_for_cleanup,
950+
} => (
951+
DbBpZoneDisposition::Expunged,
952+
Some(Generation(as_of_generation)),
953+
ready_for_cleanup,
954+
),
955+
};
956+
Self {
957+
disposition,
958+
expunged_as_of_generation: disposition_expunged_as_of_generation,
959+
expunged_ready_for_cleanup: disposition_expunged_ready_for_cleanup,
960+
}
923961
}
924962
}
925963

926-
impl From<DbBpZoneDisposition> for BlueprintZoneDisposition {
927-
fn from(disposition: DbBpZoneDisposition) -> Self {
928-
match disposition {
929-
DbBpZoneDisposition::InService => {
930-
BlueprintZoneDisposition::InService
964+
impl TryFrom<DbBpZoneDispositionColumns> for BlueprintZoneDisposition {
965+
type Error = anyhow::Error;
966+
967+
fn try_from(
968+
value: DbBpZoneDispositionColumns,
969+
) -> Result<Self, Self::Error> {
970+
match (value.disposition, value.expunged_as_of_generation) {
971+
(DbBpZoneDisposition::InService, None) => Ok(Self::InService),
972+
(DbBpZoneDisposition::Expunged, Some(as_of_generation)) => {
973+
Ok(Self::Expunged {
974+
as_of_generation: *as_of_generation,
975+
ready_for_cleanup: value.expunged_ready_for_cleanup,
976+
})
931977
}
932-
DbBpZoneDisposition::Quiesced => BlueprintZoneDisposition::Quiesced,
933-
DbBpZoneDisposition::Expunged => BlueprintZoneDisposition::Expunged,
978+
(DbBpZoneDisposition::InService, Some(_))
979+
| (DbBpZoneDisposition::Expunged, None) => Err(anyhow!(
980+
"illegal database state (CHECK constraint broken?!): \
981+
disposition {:?}, disposition_expunged_as_of_generation {:?}",
982+
value.disposition,
983+
value.expunged_as_of_generation,
984+
)),
934985
}
935986
}
936987
}
@@ -1086,64 +1137,3 @@ impl BpClickhouseServerZoneIdToNodeId {
10861137
})
10871138
}
10881139
}
1089-
1090-
mod diesel_util {
1091-
use crate::{
1092-
schema::bp_omicron_zone::disposition, to_db_bp_zone_disposition,
1093-
DbBpZoneDisposition,
1094-
};
1095-
use diesel::{
1096-
helper_types::EqAny, prelude::*, query_dsl::methods::FilterDsl,
1097-
};
1098-
use nexus_types::deployment::{
1099-
BlueprintZoneDisposition, BlueprintZoneFilter,
1100-
};
1101-
1102-
/// An extension trait to apply a [`BlueprintZoneFilter`] to a Diesel
1103-
/// expression.
1104-
///
1105-
/// This is applicable to any Diesel expression which includes the
1106-
/// `bp_omicron_zone` table.
1107-
///
1108-
/// This needs to live here, rather than in `nexus-db-queries`, because it
1109-
/// names the `DbBpZoneDisposition` type which is private to this crate.
1110-
pub trait ApplyBlueprintZoneFilterExt {
1111-
type Output;
1112-
1113-
/// Applies a [`BlueprintZoneFilter`] to a Diesel expression.
1114-
fn blueprint_zone_filter(
1115-
self,
1116-
filter: BlueprintZoneFilter,
1117-
) -> Self::Output;
1118-
}
1119-
1120-
impl<E> ApplyBlueprintZoneFilterExt for E
1121-
where
1122-
E: FilterDsl<BlueprintZoneFilterQuery>,
1123-
{
1124-
type Output = E::Output;
1125-
1126-
fn blueprint_zone_filter(
1127-
self,
1128-
filter: BlueprintZoneFilter,
1129-
) -> Self::Output {
1130-
// This is only boxed for ease of reference above.
1131-
let all_matching_dispositions: BoxedIterator<DbBpZoneDisposition> =
1132-
Box::new(
1133-
BlueprintZoneDisposition::all_matching(filter)
1134-
.map(to_db_bp_zone_disposition),
1135-
);
1136-
1137-
FilterDsl::filter(
1138-
self,
1139-
disposition.eq_any(all_matching_dispositions),
1140-
)
1141-
}
1142-
}
1143-
1144-
type BoxedIterator<T> = Box<dyn Iterator<Item = T>>;
1145-
type BlueprintZoneFilterQuery =
1146-
EqAny<disposition, BoxedIterator<DbBpZoneDisposition>>;
1147-
}
1148-
1149-
pub use diesel_util::ApplyBlueprintZoneFilterExt;

‎nexus/db-model/src/schema.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1753,6 +1753,8 @@ table! {
17531753
snat_first_port -> Nullable<Int4>,
17541754
snat_last_port -> Nullable<Int4>,
17551755
disposition -> crate::DbBpZoneDispositionEnum,
1756+
disposition_expunged_as_of_generation -> Nullable<Int8>,
1757+
disposition_expunged_ready_for_cleanup -> Bool,
17561758
external_ip_id -> Nullable<Uuid>,
17571759
filesystem_pool -> Nullable<Uuid>,
17581760
}

‎nexus/db-model/src/schema_versions.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use std::collections::BTreeMap;
1717
///
1818
/// This must be updated when you change the database schema. Refer to
1919
/// schema/crdb/README.adoc in the root of this repository for details.
20-
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(124, 0, 0);
20+
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(125, 0, 0);
2121

2222
/// List of all past database schema versions, in *reverse* order
2323
///
@@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::new(|| {
2929
// | leaving the first copy as an example for the next person.
3030
// v
3131
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
32+
KnownVersion::new(125, "blueprint-disposition-expunged-cleanup"),
3233
KnownVersion::new(124, "support-read-only-region-replacement"),
3334
KnownVersion::new(123, "vpc-subnet-contention"),
3435
KnownVersion::new(122, "tuf-artifact-replication"),

‎nexus/db-queries/src/db/datastore/support_bundle.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -983,7 +983,10 @@ mod test {
983983
for zones in bp.blueprint_zones.values_mut() {
984984
for mut zone in &mut zones.zones {
985985
if zone.id == bundle.assigned_nexus.unwrap().into() {
986-
zone.disposition = BlueprintZoneDisposition::Expunged;
986+
zone.disposition = BlueprintZoneDisposition::Expunged {
987+
as_of_generation: *Generation::new(),
988+
ready_for_cleanup: false,
989+
};
987990
}
988991
}
989992
}

‎nexus/db-queries/src/db/datastore/vpc.rs‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ use crate::db::collection_insert::DatastoreCollection;
1616
use crate::db::error::public_error_from_diesel;
1717
use crate::db::error::ErrorHandler;
1818
use crate::db::identity::Resource;
19-
use crate::db::model::ApplyBlueprintZoneFilterExt;
2019
use crate::db::model::ApplySledFilterExt;
2120
use crate::db::model::IncompleteVpc;
2221
use crate::db::model::InstanceNetworkInterface;
@@ -56,13 +55,13 @@ use nexus_db_fixed_data::vpc::SERVICES_INTERNET_GATEWAY_DEFAULT_ROUTE_V4;
5655
use nexus_db_fixed_data::vpc::SERVICES_INTERNET_GATEWAY_DEFAULT_ROUTE_V6;
5756
use nexus_db_fixed_data::vpc::SERVICES_INTERNET_GATEWAY_ID;
5857
use nexus_db_fixed_data::vpc::SERVICES_VPC_ID;
58+
use nexus_db_model::DbBpZoneDisposition;
5959
use nexus_db_model::ExternalIp;
6060
use nexus_db_model::InternetGateway;
6161
use nexus_db_model::InternetGatewayIpAddress;
6262
use nexus_db_model::InternetGatewayIpPool;
6363
use nexus_db_model::IpPoolRange;
6464
use nexus_db_model::NetworkInterfaceKind;
65-
use nexus_types::deployment::BlueprintZoneFilter;
6665
use nexus_types::deployment::SledFilter;
6766
use omicron_common::api::external::http_pagination::PaginatedBy;
6867
use omicron_common::api::external::CreateResult;
@@ -799,8 +798,11 @@ impl DataStore {
799798
)
800799
// Filter out services that are expunged and shouldn't be resolved
801800
// here.
802-
.blueprint_zone_filter(
803-
BlueprintZoneFilter::ShouldDeployVpcFirewallRules,
801+
//
802+
// TODO: We should reference a rendezvous table instead of filtering
803+
// for in-service zones.
804+
.filter(
805+
bp_omicron_zone::disposition.eq(DbBpZoneDisposition::InService),
804806
)
805807
.filter(service_network_interface::vpc_id.eq(vpc_id))
806808
.filter(service_network_interface::time_deleted.is_null())
@@ -3356,31 +3358,24 @@ mod tests {
33563358
.expect("failed to undo ineligible sleds");
33573359
assert_service_sled_ids(&datastore, &sled_ids).await;
33583360

3359-
// Make a new blueprint marking one of the zones as quiesced and one as
3360-
// expunged. Ensure that the sled with *quiesced* zone is returned by
3361-
// vpc_resolve_to_sleds, but the sled with the *expunged* zone is not.
3362-
// (But other services are still running.)
3361+
// Make a new blueprint marking one of the zones as expunged. Ensure
3362+
// that the sled the expunged zone is not returned by
3363+
// vpc_resolve_to_sleds. (But other services are still running.)
33633364
let bp4 = {
33643365
let mut bp4 = bp3.clone();
33653366
bp4.id = BlueprintUuid::new_v4();
33663367
bp4.parent_blueprint_id = Some(bp3.id);
33673368

3368-
// Sled index 2's Nexus is quiesced (should be included).
3369-
let sled2 = bp4
3370-
.blueprint_zones
3371-
.get_mut(&sled_ids[2])
3372-
.expect("zones for sled");
3373-
sled2.zones.iter_mut().next().unwrap().disposition =
3374-
BlueprintZoneDisposition::Quiesced;
3375-
sled2.generation = sled2.generation.next();
3376-
33773369
// Sled index 3's zone is expunged (should be excluded).
33783370
let sled3 = bp4
33793371
.blueprint_zones
33803372
.get_mut(&sled_ids[3])
33813373
.expect("zones for sled");
33823374
sled3.zones.iter_mut().next().unwrap().disposition =
3383-
BlueprintZoneDisposition::Expunged;
3375+
BlueprintZoneDisposition::Expunged {
3376+
as_of_generation: Generation::new(),
3377+
ready_for_cleanup: false,
3378+
};
33843379
sled3.generation = sled3.generation.next();
33853380

33863381
bp4

0 commit comments

Comments
 (0)