[reconfigurator] Avoid deploying expunged datasets to sled agents

[smklein]

If a blueprint has marked a dataset as expunged, stop telling sled agents to deploy it.

Fixes #7304

[smklein]

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.

[smklein]

This is the actual fix - basically, converting from "Blueprint dataset" -> "Deployable dataset" format will filter out expunged datasets.

If this feels too magical, I can remove impl From here and instead create:

impl BlueprintDatasetsConfig {
  fn into_in_service_datasets(self) -> DatasetsConfig {
     ...
  }
}

(the implementation would be the same, but this could make it more explicit that we're filtering)

[sunshowers]

I like your suggestion, seems like a good refactor :)

[smklein]

Done in 0bca90f

[davepacheco]

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.

[smklein]

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

[davepacheco]

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.

[smklein]

So, given the extra context in #7500 , I was wrong about this not impacting the underlying system.

In cases where a dataset is expunged, but the disk remains, then yes, this has no effect.

However, in cases where a dataset is expunged from a disk which is also expunged, then it's critical to omit these datasets when handing them to a sled agent. Otherwise, the reconfigurator can ask the sled agent to deploy a dataset that should be expunged, on a disk that should be expunged. This appears to hang reconfigurator execution.