This does not resolve #6177 , and the issues raised by @davepacheco there are still open (namely, the ordering between {dataset,zone} {add,remove} which could cause issues during zone expungement). However, this solves the more scope-limited issue raised by #7304 : If a blueprint thinks a dataset is expunged, we stop telling the sled agents to instantiate it.

Given the stuff mentioned in #7313: are we comfortable landing this before having fixed #7309? I think it should be fine. But I don't think it improves things without also doing #6177. And I think it makes the system more complicated to reason about in the meantime.

Given the stuff mentioned in #7313: are we comfortable landing this before having fixed #7309? I think it should be fine. But I don't think it improves things without also doing #6177.

I agree that this shouldn't change the net effect of the system, without subsequent PRs.

And I think it makes the system more complicated to reason about in the meantime.

Not sure I agree with this one. IMO this PR inches us closer to "more correct" behavior because it makes the reconfigurator do the right thing with the APIs it has. I think it'll have the same impact on the Sled Agent as "not requesting expunged datasets" because they aren't getting deleted regardless.

I think it would make sense to do the following:

• Fix reconfigurator does not try to remove expunged datasets #7304 (aka, stop requesting datasets that aren't in-service)
• Fix Reconfigurator "deploy datasets then zones" ordering will break for zone expungement #7309 (aka, make an API in the Sled Agent that accepts datasets + zones + disks)

At this point: The reconfigurator would be sending a set of info to the sled agents that's correct, but not being carried out correctly, because things aren't being deleted.

Then we could:

• Fix "datasets ensure" API in sled agent should remove unmatched datasets (eventually) #6177 (aka, actually delete unwanted datasets)
• Which would make planner may need to work around durable datasets never being destroyed #7312 obsolete

IMO this PR inches us closer to "more correct" behavior because it makes the reconfigurator do the right thing with the APIs it has. I think it'll have the same impact on the Sled Agent as "not requesting expunged datasets" because they aren't getting deleted regardless.

Agreed!

I think it would make sense to do the following:

I agree that sequence should work. My worry here stems mainly from various recent discussions (like the R12/#7265 one) where I feel like we struggled to accurately predict how the system would behave in this area, particularly across various releases. #7309 itself is an example of a non-obvious consequence of a bunch of other reasonable-seeming things. I guess another way to say that is that if we land this fix, then a bug (6177) becomes load-bearing for zone expungement to keep working, which feels like a weird place to be.