Skip to content

Commit

Permalink
Restore initial planning input population of external networking reco…
Browse files Browse the repository at this point in the history
…rds (#6742)

Recapping from the update sync:

* #6483 moved the code restored by this PR into a helper method
(`update_network_resources_from_blueprint()`), and also added calls to
that method to a handful of tests to make them pass
* In the external DNS work, we discovered that those tests should not
have needed that, and the reason they did was because of a different bug
* #6599 fixed that bug and removed
`update_network_resources_from_blueprint()` and all its callers, but
neglected to restore this code, meaning the `PlanningInput`s our tests
create from their initial system descriptions never had any external
networking records
  • Loading branch information
jgallagher authored Oct 2, 2024
1 parent c50cf01 commit 041e406
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 28 deletions.
34 changes: 33 additions & 1 deletion nexus/reconfigurator/planning/src/example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@ use crate::system::SledBuilder;
use crate::system::SystemDescription;
use nexus_types::deployment::Blueprint;
use nexus_types::deployment::BlueprintZoneFilter;
use nexus_types::deployment::OmicronZoneNic;
use nexus_types::deployment::PlanningInput;
use nexus_types::deployment::SledFilter;
use nexus_types::inventory::Collection;
use omicron_common::policy::INTERNAL_DNS_REDUNDANCY;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::SledKind;
use omicron_uuid_kinds::VnicUuid;
use typed_rng::TypedUuidRng;

pub struct ExampleSystem {
Expand Down Expand Up @@ -44,7 +47,7 @@ impl ExampleSystem {
let _ = system.sled(SledBuilder::new().id(*sled_id)).unwrap();
}

let input_builder = system
let mut input_builder = system
.to_planning_input_builder()
.expect("failed to make planning input builder");
let base_input = input_builder.clone().build();
Expand Down Expand Up @@ -102,6 +105,35 @@ impl ExampleSystem {
system.to_collection_builder().expect("failed to build collection");
builder.set_rng_seed((test_name, "ExampleSystem collection"));

for sled_id in blueprint.sleds() {
let Some(zones) = blueprint.blueprint_zones.get(&sled_id) else {
continue;
};
for zone in zones.zones.iter() {
let service_id = zone.id;
if let Some((external_ip, nic)) =
zone.zone_type.external_networking()
{
input_builder
.add_omicron_zone_external_ip(service_id, external_ip)
.expect("failed to add Omicron zone external IP");
input_builder
.add_omicron_zone_nic(
service_id,
OmicronZoneNic {
// TODO-cleanup use `TypedUuid` everywhere
id: VnicUuid::from_untyped_uuid(nic.id),
mac: nic.mac,
ip: nic.ip,
slot: nic.slot,
primary: nic.primary,
},
)
.expect("failed to add Omicron zone NIC");
}
}
}

for (sled_id, zones) in &blueprint.blueprint_zones {
builder
.found_sled_omicron_zones(
Expand Down
55 changes: 28 additions & 27 deletions nexus/src/app/background/tasks/blueprint_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,34 @@ mod test {
// sleds to CRDB.
let mut s1 = httptest::Server::run();
let mut s2 = httptest::Server::run();

// The only sled-agent endpoint we care about in this test is `PUT
// /omicron-zones`. Add a closure to avoid repeating it multiple times
// below. We don't do a careful check of the _contents_ of what's being
// sent; for that, see the tests in nexus-reconfigurator-execution.
let match_put_omicron_zones =
|| request::method_path("PUT", "/omicron-zones");

// Helper for our mock sled-agent http servers to blanket ignore and
// return 200 OK for anything _except_ `PUT /omciron-zones`, which is
// the endpoint we care about in this test.
//
// Other Nexus background tasks created by our test context may notice
// the sled-agent records we're about to insert into CRDB and query them
// (e.g., for inventory, vpc routing info, ...). We don't want those to
// cause spurious test failures, so just tell our sled-agents to accept
// any number of them.
let mock_server_ignore_spurious_http_requests =
|s: &mut httptest::Server| {
s.expect(
Expectation::matching(not(match_put_omicron_zones()))
.times(..)
.respond_with(status_code(200)),
);
};
mock_server_ignore_spurious_http_requests(&mut s1);
mock_server_ignore_spurious_http_requests(&mut s2);

let sled_id1 = SledUuid::new_v4();
let sled_id2 = SledUuid::new_v4();
let rack_id = Uuid::new_v4();
Expand Down Expand Up @@ -410,33 +438,6 @@ mod test {
)
.await;

// The only sled-agent endpoint we care about in this test is `PUT
// /omicron-zones`. Add a closure to avoid repeating it multiple times
// below. We don't do a careful check of the _contents_ of what's being
// sent; for that, see the tests in nexus-reconfigurator-execution.
let match_put_omicron_zones =
|| request::method_path("PUT", "/omicron-zones");

// Helper for our mock sled-agent http servers to blanket ignore and
// return 200 OK for anything _except_ `PUT /omciron-zones`, which is
// the endpoint we care about in this test.
//
// Other Nexus background tasks created by our test context may notice
// the sled-agent records we're about to insert into CRDB and query them
// (e.g., for inventory, vpc routing info, ...). We don't want those to
// cause spurious test failures, so just tell our sled-agents to accept
// any number of them.
let mock_server_ignore_spurious_http_requests =
|s: &mut httptest::Server| {
s.expect(
Expectation::matching(not(match_put_omicron_zones()))
.times(..)
.respond_with(status_code(200)),
);
};
mock_server_ignore_spurious_http_requests(&mut s1);
mock_server_ignore_spurious_http_requests(&mut s2);

// Insert records for the zpools backing the datasets in these zones.
for (sled_id, config) in
blueprint.1.all_omicron_zones(BlueprintZoneFilter::All)
Expand Down

0 comments on commit 041e406

Please sign in to comment.