[sled-agent] Destroy orphaned datasets (PR 3/2) #8323
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Base automatically changed from
john/sled-agent-config-reconciler-report-orphaned-datasets-inventory
to
main
June 12, 2025 13:33
jgallagher
force-pushed
the
john/sled-agent-destroy-orphaned-datasets
branch
from
f3ac7a8 to
fce9033
Compare
andrewjstone
approved these changes
Jun 12, 2025
| /// control "chicken switches" (potentially-destructive sled-agent behavior | ||
| /// that can be toggled on or off via `omdb`) | ||
| #[clap(subcommand)] | ||
| ChickenSwitch(ChickenSwitchCommands), |
| List, | ||
| enum ChickenSwitchCommands { | ||
| /// interact with the "destroy orphaned datasets" chicken switch | ||
| DestroyOrphans(DestroyOrphansArgs), |
There was a problem hiding this comment.
I can't get over the name of this variant.
| @@ -65,6 +66,7 @@ pub(crate) fn spawn<T: SledAgentFacilities>( | |||
| currently_managed_zpools_tx: watch::Sender<Arc<CurrentlyManagedZpools>>, | |||
| external_disks_tx: watch::Sender<HashSet<Disk>>, | |||
| raw_disks_rx: RawDisksReceiver, | |||
| destroy_orphans: Arc<AtomicBool>, | |||
There was a problem hiding this comment.
I've said it before. All you need is an AtomicBool!
| .datasets_report_orphans( | ||
| datasets.clone(), | ||
| currently_managed_zpools, | ||
| self.destroy_orphans.load(Ordering::Relaxed), |
There was a problem hiding this comment.
Yay, proper usage of ordering!
|
Tested enabling this on One case that would have failed without this is this internal DNS dataset, which we expunged and replaced on the same zpool: Looking at the zpool history, we see when RSS created the initial dataset, and when sled-agent destroyed it and replaced it with the new one: |
|
Also tested not turning this switch on, expunging a zone, and confirming datasets are not deleted. On and the dataset still exists on disk as expected: |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds on #8302. This allows the config-reconciler to destroy durable datasets it believes are orphaned due to expunged Omicron zones. This is disabled by default, and can be enabled on a sled-by-sled case via a new
omdb sled-agent chicken-switch destroy-orphans enablesubcommand (with related commands togetanddisablethe same).We don't want to ship automatic dataset deletion before R17 (R16 should only ship "report orphaned datasets"), but we need to be able to turn it on for upgrade testing in the meantime. All of this
chicken-switchstuff should be removeable after R16, once we're comfortable enabling deletion in general.