[reconfigurator] Planner: Don't omit expunged sleds from blueprint_datasets
#7613
Conversation
| assert_eq!(summary.sleds_modified.len(), 4); | ||
| assert_eq!(summary.sleds_unchanged.len(), 1); | ||
| assert_eq!(summary.sleds_modified.len(), 3); | ||
| assert_eq!(summary.sleds_unchanged.len(), 2); |
There was a problem hiding this comment.
I believe this test was "wrong" (in that a sled was showing up as modified only because we were dropping its datasets): The sled that moved from modified to unchanged is sled 68d24ac5-f341-49ea-a92a-0381b52ab387 in nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_1_2.txt. The changes in the diff prior to this PR are exclusively "all the datasets were removed"; now that that isn't happening, the sled is unchanged.
| kind: SledKind::MultimapInconsistency( | ||
| MultimapInconsistency::PresentInZonesNotDisks, | ||
| MultimapInconsistency::PresentInZonesNotDatasets, | ||
| ), | ||
| }, | ||
| }, | ||
| Note { | ||
| severity: Severity::BackwardsCompatibility, |
There was a problem hiding this comment.
Should this still remain a BackwardsCompatibility severity, now that it's actually incorrect to have this inconsistency? Are we keeping it purely for older blueprints?
| @@ -95,6 +95,95 @@ to: blueprint 9f71f5d3-a272-4382-9154-6ea2e171a6c6 | |||
| nexus 88602518-f176-49a1-af12-02fac36214c3 in service fd00:1122:3344:105::22 | |||
|
|
|||
|
|
|||
| sled 68d24ac5-f341-49ea-a92a-0381b52ab387 (decommissioned): | |||
There was a problem hiding this comment.
I don't think there's anything that needs changing in this PR, but this test drives me a bit nuts. Output that shows a decommissioned sled and in-service disks and datasets really makes no sense. Perhaps we should consider simplifying or removing this test in the future.
This removes the last "for backwards compatibility..." bit from
BlueprintBuilder::build()where we potentially drop entries from one of the four blueprint maps. The code changes are small:and the bulk of the diffs are expectorate changes.