[reconfigurator-cli] Extend target-release test to step through zone updates#8940
Merged
Conversation
davepacheco
approved these changes
Aug 28, 2025
jgallagher
added a commit
that referenced
this pull request
Aug 29, 2025
iliana
pushed a commit
that referenced
this pull request
Sep 2, 2025
…updates (#8940) This is a step toward addressing #8478. It doesn't quite get there: * I stopped at the point where we're ready to update Nexus; the current behavior is wrong, and it didn't seem worth adding that wrong behavior to this test. * Our simulated system doesn't set up all zone types (it's missing boundary NTP, cockroach, oximeter, and multinode clickhouse). I'm not sure the second bullet is _super_ important for this test; we don't really treat different zone types all that differently, other than that some are expunge -> add and others are upgraded in place (and we do have zones of both of those types in our simulated system). But I'd like to leave #8478 open until we can address the first one. In terms of review: the non-expectorate changes are quite small. I'd recommend skimming the expectorate changes and checking the points where there are comments about what just happened or is about to happen more carefully. (As I was doing this I discovered that our simulated system started with 3 pantry zones but during the upgrade test it expunged them without replacing them; that's what led to changing the example system's `target_crucible_pantry_zone_count`. So I think it is worth at least skimming this output to see if there's anything else funky that I missed.)
iliana
pushed a commit
that referenced
this pull request
Sep 2, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a step toward addressing #8478. It doesn't quite get there:
I'm not sure the second bullet is super important for this test; we don't really treat different zone types all that differently, other than that some are expunge -> add and others are upgraded in place (and we do have zones of both of those types in our simulated system). But I'd like to leave #8478 open until we can address the first one.
In terms of review: the non-expectorate changes are quite small. I'd recommend skimming the expectorate changes and checking the points where there are comments about what just happened or is about to happen more carefully. (As I was doing this I discovered that our simulated system started with 3 pantry zones but during the upgrade test it expunged them without replacing them; that's what led to changing the example system's
target_crucible_pantry_zone_count. So I think it is worth at least skimming this output to see if there's anything else funky that I missed.)