[4/n] [reconfigurator] more control over ExampleSystem zone allocation #6785
Conversation
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
sunshowers
requested review from
andrewjstone,
davepacheco,
jgallagher and
smklein
| test_name: String, | ||
| nsleds: usize, | ||
| ndisks_per_sled: u8, | ||
| // None means nsleds |
There was a problem hiding this comment.
I know this is a builder, but I think we'd be more likely to maintain the completeness of the ExampleSystem if we took a Policy as input, rather than duplicating what gets built by hand. I think it will also make the use of the ExampleSystem more consistent with production by driving things from Policy rather than another similar, but not quite the same interface.
We could then copy the policy into the PlanningInput when we build the ExampleSystem.
There was a problem hiding this comment.
That makes sense!
There was a problem hiding this comment.
Hmm, I agree in principle but it's a bit hard to express the current default of 1 Nexus zone per sled in a Policy struct. We could change the default after auditing that our tests all set up the right number of Nexus zones, though.
How about I file a followup issue to do this? I think once the 1-zone-per-sled issue is resolved, we can make the builder interface and Policy work together. We could in principle do something like with_policy(|policy| { policy.target_nexus_zone_count = 1; }).
There was a problem hiding this comment.
Hmm, I agree in principle but it's a bit hard to express the current default of 1 Nexus zone per sled in a
Policystruct. We could change the default after auditing that our tests all set up the right number of Nexus zones, though.
I guess I assumed if there was enough sleds the planner would guarantee this, or that it would be the default.
How about I file a followup issue to do this? I think once the 1-zone-per-sled issue is resolved, we can make the builder interface and
Policywork together. We could in principle do something likewith_policy(|policy| { policy.target_nexus_zone_count = 1; }).
A follow up seems fine to me.
There was a problem hiding this comment.
I'll file an issue, thanks!
nexus/reconfigurator/planning/src/blueprint_builder/external_networking.rs
Outdated
Show resolved
Hide resolved
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
sunshowers
changed the base branch from
sunshowers/spr/main.4n-reconfigurator-more-control-over-examplesystem-zone-allocation
to
main
Created using spr 1.3.6-beta.1
sunshowers
deleted the
sunshowers/spr/4n-reconfigurator-more-control-over-examplesystem-zone-allocation
branch
ExampleSysteminstances now allow for controlling the number of Nexus and DNSzones.
There was one test that manually poked at the
ExampleSystemearlier -- thattest can now switch to using this scheme. (I didn't realize it at the time but
we were allocating just one Nexus zone per sled -- I thought we were doing 3
total. Well, now it's clear how many zones are being allocated.)
Also add tests to ensure zone allocation works properly.
Depends on: