New BlueprintDiff implementation

[andrewjstone]

This commit introduces an automated blueprint diff mechanism based on diffus along with the visitor pattern for traversing heterogeneous diff trees. It builds on several prior commits that introduced diffus support and added visitors for types:

#7261 #7336 #7362 #7366

The new BlueprintDiffer implements the relevant visitors and accumulates change state from visitor method callbacks. The accumulated state is the BlueprintDiff which replaces the old BlueprintDiff. The new structure intentionally groups resources by sleds, as the 4 primary maps in a Blueprint (sled_state, blueprint_zones, blueprint_disks, blueprint_datasets) are going to be collapsed into a single map. This allows us to modify the visitors to traverse the new blueprint diffs in one place and not have to worry about modifying the BlueprintDiff itself or any consumers. More details about why we are collapsing these maps can be found in #7078.

We primarily use BlueprintDiffs for testing and omdb output, and therefore the printed representation is absolutely critical for us. This commit reuses the types and formatting contained in nexus/ types/src/deployment/ blueprint_display.rs and provides a new type, BpDiffPrintable that can be created from a BlueprintDiff. BpDiffPrintable contains our formatted tables ready to be used by BlueprintDiffDisplay along with the orignal blueprints to render the diff. It's possible that we can collapse BlueprintDiffDisplay and BpDiffPrintable into one type and simplify a bit. I just haven't done that yet.

The printable output of the diffs has maintained backwards compatibility in all cases except for errors and warnings. Those have been specifically adapted to work with our visitors and be accumulated while walking the diffus diffs. This only resulted in the change to one test output file, which should leave us confident in the correctness of this new implementation. An optional show_unchanged flag was added at key points in the code and in the future we plan to change the default to only show actual changes. We didn't do that here so that we could ensure the output of the new implementation matches the existing code. We also plan to add more columns, such as disposition for datasets.

Usage going forward

This is a significant change to our blueprint diff code, and in some cases it may take slightly more boilerplate in order to diff newly added fields or structs, than in the older code. However, there are a few things this new style has going for it. Figuring out the differences between blueprints is now done automatically and completely. There is no need to compute these differences, which is both error prone, and complicated. See the old BpDiffZones code for what finding the differences for just zones in a blueprint looked like.

With automated diffs output from diffus, we now need to write code to expose the necessary change information to consumers. We want to do this in a unified and rigorous manner, and one that composes nicely. We chose a visitor pattern for this. For each somewhat complex type (use your judgement), we provide a visitor trait that walks the heterogeneous diff tree and calls trait methods when a change is found. User code implements only the visitor methods that it cares about and then can accumulate its own internal state based on which callbacks fire. We provide a top-level VisitBlueprint trait to detect all changes in a blueprint and implement it in BlueprintDiffer to construct a BlueprintDiff in a format we can use.

There are a few important things to note about our visitors. While diffus provides a complete tree of all changes as well as fields and variants that have not changed, the visitors as currently implemented only expose changes. These changes are unified for simplicity into a Change type. We also don't currently expose all changes. For most fields that we don't expect to ever change, there is not a change callback. Mostly this is because the visitors were written before I had to use them :). It turns out it may actually be useful to expose these fields so that we can report errors or warnings in our diffs if they do change unexpectedly. You can see an example of error tracking in the BlueprintDiffer::visit_zone_type_change method and its test output in nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt.

It is tedious to write visitor traits to walk a diffus diff and then implement these traits to accumulate the state we want. It can reasonably be asked why we wouldn't just walk the tree and generate the state we want directly without the visitors. Indeed, this is quite reasonable for small one off tasks. But we are building a foundation that will grow over time in terms of type structure and number of developers working on the system. For such a long term project, it's usefulf to decouple the tree walking from the state accumulation. For one thing, it makes each bit easier to read and write on its own. For another, it allows us to build different implementations of the visitors for different use cases, such as testing. Remember, users only have to implement the methods they care about. Therefore if they only want to see if a zone changed, they don't have to worry about parsing diffus types, but can just implement a callback. An example of this is the TestVisitor in live-tests/tests/test_nexus_add_remove.rs.

I anticipate we will find ways to make implementing and using diff visitors more ergonomic over time while maintaining the compositional rigor that the separation of concerns provides in this model.

Testing

I mainly have run planning tests locally to ensure diffs work as expected. I still need to run the live tests and play with omdb.

[andrewjstone]

TLDR: I'm setting this PR to a draft for now and am leaning towards closing it in favor of a simpler solution using daft.

---

On Friday evening I successfully got blueprint diffs working with automated diffs based on diffus as described in the PR description.

Almost immediately after opening this pull request I had an uneasy feeling that the approach taken in this PR was the wrong long term choice. It's certainly hard to argue against automated diff generation in terms of correctness and completeness. However, the more I thought about it, the more unreasonable it seemed to require hand writing visitors for each diff. This is super tedious and also can introduce its own errors. Hell, I didn't even finish writing them all for this PR. The ClickhouseClusterConfig is still hand diffed!

Something @sunshowers mentioned about autogenerated visitors hit a nerve, and I thought about how I could write a proc macro to generate visitors for generated diffus types However, looking at the hand crafted ones I created showed that they were not purely mechanical. I was explicitly making a choice to change the return types and not explicitly visit the diffus types. The visitors provided "cleaner" types that users could then implement and accumulate into the actual desired user level diff, such as BlueprintDiff. Autogenerating visitors would likely be strictly worse than just operating on the diffus types directly.

Then it hit me! What this PR is doing is taking a diffus Diff, handwriting visitors, and accumulating state into a Diff type that we want to work with. What if we could skip the latter two steps and generate a diff in the format we want, or at least one close to that? Well, we certainly aren't going to get that with Diffus. So I stepped back and wrote daft over the weekend. It's much simpler than diffus and suits our needs better. It should also make for a much smaller diff when integrated with omicron than this PR. We own it, and we can change it to suit our needs, if we have to. We can autogenerate visitors for daft types also, although I don't think it is necessary.

I'm leaving this PR open, and in draft, because I still think it's worth looking at. There are bits of it that should be merged in and will be incorporated into a daft based diff PR. I'm going to immediately proceed to that PR, which will be much easier to implement than what exists here, and much less code. We can then compare and contrast the two.

I know that I've been hammering on diffs for a while, but I don't want to merge this in just because I got it working. It took building and using it to realize it's probably the wrong direction.