[reconfigurator] Stop reading dataset UUIDs from datastore #6645
Comments
jgallagher
added a commit
that referenced
this issue
Dec 4, 2024
jgallagher
added a commit
that referenced
this issue
Dec 4, 2024
jgallagher
added a commit
that referenced
this issue
Dec 4, 2024
jgallagher
added a commit
that referenced
this issue
Dec 4, 2024
jgallagher
added a commit
that referenced
this issue
Dec 5, 2024
…uilder` (#7204) This is a step on the road to #7078. The current `BlueprintBuilder` internals would not at all be amenable to squishing the disparate maps in `Blueprint` together, so this PR tries to rework those internals. `BlueprintBuilder` now does its own map squishing (inside `new_based_on()`), combining state+zones+disks+datasets into one helper (`SledEditor`). All modifications are preformed via those editors, then `BlueprintBuilder::build()` breaks the combined editors' results back out into four maps for `Blueprint`. There should be only one functional change in this PR (marked with a `TODO-john`): previously, expunging a zone checked that that zone had not been modified in the current planning cycle. I'm not sure that's really necessary; it felt like some defensiveness born out of the complexity of the builder itself, maybe? I think we could put it back (inside `ZonesEditor`, presumably) if folks think it would be good to keep. My intention was to change the tests as little as possible to ensure I didn't break anything as I was moving functionality around, so `new_based_on()` and `build()` have some arguably-pretty-gross concessions to behave exactly the way the builder did before these changes. I'd like to _remove_ those concessions, but those will be nontrivial behavioral changes that I don't want to try to roll in with all of this cleanup. I think I landed this pretty well; there are only a few expectorate changes (due to the slightly reworked `ZonesEditor` producing a different ordering for a few tests), and none in `omdb` (which is where I've often seen incidental planner changes bubble out). Apologies for the size of the PR. I'd lightly recommend that the new `blueprint_editor/` module and its subcomponents should be pretty quickly reviewable and should be looked at first; they're _supposed_ to be simple and obvious, and are largely ported over from the prior storage / disks / datasets / zones editors. I did rework how we're handling #6645 backwards compat to try to reduce how much we need to pass `SledResources` around, so that complicates things in `DatasetsEditor` some, but not too bad IMO. (I also added a test for this, since I changed it and I don't think we had one before.) The `BlueprintBuilter` changes should then be more or less the natural way one would use a collection of `SledEditor`s without changing its API or behavior (yet!).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In #6229 , nexus begins managing datasets via the reconfigurator. As a part of this logic, when ensuring a dataset exists, the reconfigurator first checks for the dataset within the datastore, to ensure that if it already exists, the UUID is re-used.
As an example:
xyzon a physical diskIf the reconfigurator allocated a new UUID for Crucible, we'd end up with two Crucible datasets on the sled - this would be wrong. Instead, it looks for any "Crucible kind" datasets on the zpool within that disk, and re-uses the existing UUID.
If the reconfigurator was confident that "all existing datasets existed in the blueprint", this logic would be unnecessary -- instead of reading from the datastore, the reconfigurator could look for old UUIDs by scanning the parent blueprint.
The text was updated successfully, but these errors were encountered: