Skip to content

Commit

Permalink
[reconfigurator] Planner: mark zones ready_for_cleanup when appropr…
Browse files Browse the repository at this point in the history
…iate (#7588)

This PR implements two related planner changes:

* When expunging a sled, all the zones are immediately marked as
`Expunged { ready_for_cleanup: true, .. }`.
* When there are expunged zones on an in-service sled, we'll consult the
latest inventory collection and flip `ready_to_cleanup` to true if the
zone is gone and the sled's zone config generation is >= the generation
in which the zone was expunged.

No downstream users of `ready_for_cleanup` are included in this PR;
those will come in followups.
  • Loading branch information
jgallagher authored Feb 24, 2025
1 parent 06c17ba commit 6b6ec4e
Show file tree
Hide file tree
Showing 11 changed files with 463 additions and 109 deletions.
43 changes: 42 additions & 1 deletion nexus/reconfigurator/planning/src/blueprint_builder/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,18 @@ impl<'a> BlueprintBuilder<'a> {
}
}

pub fn current_sled_state(
&self,
sled_id: SledUuid,
) -> Result<SledState, Error> {
let editor = self.sled_editors.get(&sled_id).ok_or_else(|| {
Error::Planner(anyhow!(
"tried to get sled state for unknown sled {sled_id}"
))
})?;
Ok(editor.state())
}

/// Set the desired state of the given sled.
pub fn set_sled_decommissioned(
&mut self,
Expand Down Expand Up @@ -935,9 +947,16 @@ impl<'a> BlueprintBuilder<'a> {
// Expunging a disk expunges any datasets and zones that depend on it,
// so expunging all in-service disks should have also expunged all
// datasets and zones. Double-check that that's true.
let mut zones_ready_for_cleanup = Vec::new();
for zone in editor.zones(BlueprintZoneDisposition::any) {
match zone.disposition {
BlueprintZoneDisposition::Expunged { .. } => (),
BlueprintZoneDisposition::Expunged { .. } => {
// Since this is a full sled expungement, we'll never see an
// inventory collection indicating the zones are shut down,
// nor do we need to: go ahead and mark any expunged zones
// as ready for cleanup.
zones_ready_for_cleanup.push(zone.id);
}
BlueprintZoneDisposition::InService => {
return Err(Error::Planner(anyhow!(
"expunged all disks but a zone \
Expand All @@ -957,6 +976,11 @@ impl<'a> BlueprintBuilder<'a> {
}
}
}
for zone_id in zones_ready_for_cleanup {
editor
.mark_expunged_zone_ready_for_cleanup(&zone_id)
.map_err(|err| Error::SledEditError { sled_id, err })?;
}

// If we didn't expunge anything, this sled was presumably expunged in a
// prior planning run. Only note the operation if we did anything.
Expand All @@ -975,6 +999,23 @@ impl<'a> BlueprintBuilder<'a> {
Ok(())
}

/// Mark expunged zones as ready for cleanup.
pub(crate) fn mark_expunged_zones_ready_for_cleanup(
&mut self,
sled_id: SledUuid,
zone_ids: &[OmicronZoneUuid],
) -> Result<(), Error> {
let editor = self.sled_editors.get_mut(&sled_id).ok_or_else(|| {
Error::Planner(anyhow!("tried to expunge unknown sled {sled_id}"))
})?;
for zone_id in zone_ids {
editor
.mark_expunged_zone_ready_for_cleanup(zone_id)
.map_err(|err| Error::SledEditError { sled_id, err })?;
}
Ok(())
}

pub(crate) fn expunge_all_multinode_clickhouse(
&mut self,
sled_id: SledUuid,
Expand Down
23 changes: 23 additions & 0 deletions nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,13 @@ impl SledEditor {
}
}

pub fn state(&self) -> SledState {
match &self.0 {
InnerSledEditor::Active(_) => SledState::Active,
InnerSledEditor::Decommissioned(edited_sled) => edited_sled.state,
}
}

pub fn edit_counts(&self) -> SledEditCounts {
match &self.0 {
InnerSledEditor::Active(editor) => editor.edit_counts(),
Expand Down Expand Up @@ -336,6 +343,13 @@ impl SledEditor {
self.as_active_mut()?.expunge_zone(zone_id)
}

pub fn mark_expunged_zone_ready_for_cleanup(
&mut self,
zone_id: &OmicronZoneUuid,
) -> Result<bool, SledEditError> {
self.as_active_mut()?.mark_expunged_zone_ready_for_cleanup(zone_id)
}

/// Backwards compatibility / test helper: If we're given a blueprint that
/// has zones but wasn't created via `SledEditor`, it might not have
/// datasets for all its zones. This method backfills them.
Expand Down Expand Up @@ -601,6 +615,15 @@ impl ActiveSledEditor {
Ok(did_expunge)
}

pub fn mark_expunged_zone_ready_for_cleanup(
&mut self,
zone_id: &OmicronZoneUuid,
) -> Result<bool, SledEditError> {
let did_mark_ready =
self.zones.mark_expunged_zone_ready_for_cleanup(zone_id)?;
Ok(did_mark_ready)
}

/// Backwards compatibility / test helper: If we're given a blueprint that
/// has zones but wasn't created via `SledEditor`, it might not have
/// datasets for all its zones. This method backfills them.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ pub enum ZonesEditError {
AddDuplicateZoneId { id: OmicronZoneUuid, kind1: ZoneKind, kind2: ZoneKind },
#[error("tried to expunge nonexistent zone {id}")]
ExpungeNonexistentZone { id: OmicronZoneUuid },
#[error("tried to mark a nonexistent zone as ready for cleanup: {id}")]
MarkNonexistentZoneReadyForCleanup { id: OmicronZoneUuid },
#[error("tried to mark a non-expunged zone as ready for cleanup: {id}")]
MarkNonExpungedZoneReadyForCleanup { id: OmicronZoneUuid },
}

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -138,6 +142,41 @@ impl ZonesEditor {
Ok((did_expunge, config.into_ref()))
}

/// Set an expunged zone's `ready_for_cleanup` flag to true.
///
/// Unlike most edit operations, this (alone) will not result in an
/// increased generation when `finalize()` is called: this flag is produced
/// and consumed inside the Reconfigurator system, and is not included in
/// the generation-guarded config send to sled-agents.
///
/// # Errors
///
/// Fails if this zone ID does not exist or is not already in the expunged
/// disposition.
pub fn mark_expunged_zone_ready_for_cleanup(
&mut self,
zone_id: &OmicronZoneUuid,
) -> Result<bool, ZonesEditError> {
let mut config = self.zones.get_mut(zone_id).ok_or_else(|| {
ZonesEditError::MarkNonexistentZoneReadyForCleanup { id: *zone_id }
})?;

match &mut config.disposition {
BlueprintZoneDisposition::InService => {
Err(ZonesEditError::MarkNonExpungedZoneReadyForCleanup {
id: *zone_id,
})
}
BlueprintZoneDisposition::Expunged {
ready_for_cleanup, ..
} => {
let did_mark_ready = !*ready_for_cleanup;
*ready_for_cleanup = true;
Ok(did_mark_ready)
}
}
}

fn expunge_impl(
config: &mut BlueprintZoneConfig,
counts: &mut EditCounts,
Expand Down
Loading

0 comments on commit 6b6ec4e

Please sign in to comment.