-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support_bundle_fail_expunged should not look at expunged datasets, zones #7319
Comments
FYI @davepacheco , I was looking for other users of omicron/nexus/reconfigurator/execution/src/sagas.rs Lines 26 to 45 in f63ed09
Is this problematic, for the same reasons @jgallagher mentioned? Namely, would the following be possible:
If this is a real issue, we could possibly resolve this similarly to how I resolved #7325 :
(This would treat "expunged" and "pruned" Nexus zones identically) |
I believe that when we talked about this in the context of #5136, the plan was that pruning an expunged Nexus zone from the blueprint would be gated on having verified that the zone was actually gone and we'd finished reassigning its sagas. I think the idea was to use inventory for the first part and the put the saga counts per-nexus into the planning input, like we do for networking and maybe disk resources? I'm not sure -- it wasn't fully fleshed out because we don't do pruning yet.
I think this would work provided that the query that attempts to re-assign sagas in this way is conditional on the target blueprint not having changed. I don't feel as confident about it and would want to think through that more but that's a plausible path, too. |
Originally posted by @jgallagher in #7063 (comment)
support_bundle_fail_expunged
could be implemented without looking at explicitly expunged datasets and zones -- it could consider the set of "all zones and datasets that are in-service", and consider anything outside of that (whether expunged, or pruned, or whatever) as "gone".Related: I should take a look at other checks for
BlueprintZoneFilter::Expunged
, and see if anyone else has fallen into this expunged vs pruned issue.The text was updated successfully, but these errors were encountered: