Skip to content

Commit e4f7ca0

Browse files
jgallagherjgallagher
authored
Clean up a variety of tests that create blueprints (#9393)
I'd like to add a couple new fields to `BlueprintSledConfig`, and doing so broke a variety of tests in annoying ways. Many of those tests have required annoying changes in the past, too. This PR doesn't add any new fields, but instead attempts to clean up those tests so changes to Reconfigurator structures are less likely to cause problems (and hopefully any problems will be easier to fix). A couple techniques: * Some tests that were creating raw blueprints by hand now use either `BlueprintBuilder` or `ExampleSystemBuilder` * Replaces `BlueprintBuilder::build_empty_with_sleds{,_seeded}` with `BlueprintBuilder::build_empty{,_seeded}`. The former added some sleds with nothing on them; the latter adds no sleds at all. I updated all the callers of `build_empty_with_sleds` to either use `build_empty` (actually fine for most of them) or the example system infrastructure. This does add a few new helper methods on real types; e.g., `BlueprintBuilder::sled_add_zone_boundary_ntp()` (for directly adding a boundary NTP instead of promoting an existing internal NTP zone). But currently only tests call these new methods, and I don't think they're prone to misuse; e.g., `sled_add_zone_boundary_ntp()` checks and fails if called on a sled that already has an in-service NTP zone of either flavor.
1 parent 2f86dda commit e4f7ca0

File tree

12 files changed

+708
-1104
lines changed

12 files changed

+708
-1104
lines changed

‎dev-tools/reconfigurator-cli/tests/output/cmds-blueprint-history-stdout‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@ blueprint df06bb57-ad42-4431-9206-abff322896c7 created from blueprint af934083-5
2626
> # which does not include any of those blueprints.
2727
> blueprint-history
2828
TIME BLUEPRINT
29-
<REDACTED_TIMESTAMP> 184f10b3-61cb-41ef-9b93-3489b2bac559 starting blueprint with 3 empty sleds
29+
<REDACTED_TIMESTAMP> 184f10b3-61cb-41ef-9b93-3489b2bac559 starting blueprint (empty)
3030
<REDACTED_TIMESTAMP> dbcbd3d6-41ff-48ae-ac0b-1becc9b2fd21
3131

3232

3333

3434
> # You can give it a specific blueprint.
3535
> blueprint-history 8da82a8e-bf97-4fbd-8ddd-9f6462732cf1
3636
TIME BLUEPRINT
37-
<REDACTED_TIMESTAMP> 184f10b3-61cb-41ef-9b93-3489b2bac559 starting blueprint with 3 empty sleds
37+
<REDACTED_TIMESTAMP> 184f10b3-61cb-41ef-9b93-3489b2bac559 starting blueprint (empty)
3838
<REDACTED_TIMESTAMP> dbcbd3d6-41ff-48ae-ac0b-1becc9b2fd21
3939
<REDACTED_TIMESTAMP> 8da82a8e-bf97-4fbd-8ddd-9f6462732cf1
4040

@@ -43,7 +43,7 @@ TIME BLUEPRINT
4343
> # Running it from the latest blueprint should report all of them.
4444
> blueprint-history latest
4545
TIME BLUEPRINT
46-
<REDACTED_TIMESTAMP> 184f10b3-61cb-41ef-9b93-3489b2bac559 starting blueprint with 3 empty sleds
46+
<REDACTED_TIMESTAMP> 184f10b3-61cb-41ef-9b93-3489b2bac559 starting blueprint (empty)
4747
<REDACTED_TIMESTAMP> dbcbd3d6-41ff-48ae-ac0b-1becc9b2fd21
4848
<REDACTED_TIMESTAMP> 8da82a8e-bf97-4fbd-8ddd-9f6462732cf1
4949
<REDACTED_TIMESTAMP> 58d5e830-0884-47d8-a7cd-b2b3751adeb4
@@ -64,21 +64,21 @@ TIME BLUEPRINT
6464
> # Show diffs, too.
6565
> blueprint-history --diff latest
6666
TIME BLUEPRINT
67-
<REDACTED_TIMESTAMP> 184f10b3-61cb-41ef-9b93-3489b2bac559 starting blueprint with 3 empty sleds
67+
<REDACTED_TIMESTAMP> 184f10b3-61cb-41ef-9b93-3489b2bac559 starting blueprint (empty)
6868
<REDACTED_TIMESTAMP> dbcbd3d6-41ff-48ae-ac0b-1becc9b2fd21
6969
from: blueprint 184f10b3-61cb-41ef-9b93-3489b2bac559
7070
to: blueprint dbcbd3d6-41ff-48ae-ac0b-1becc9b2fd21
7171

72-
MODIFIED SLEDS:
72+
ADDED SLEDS:
7373

74-
sled 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c (active, config generation 1 -> 2):
74+
sled 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c (active, config generation 2):
7575

7676
host phase 2 contents:
7777
------------------------
7878
slot boot image source
7979
------------------------
80-
A current contents
81-
B current contents
80+
+ A current contents
81+
+ B current contents
8282

8383

8484
physical disks:
@@ -184,14 +184,14 @@ to: blueprint dbcbd3d6-41ff-48ae-ac0b-1becc9b2fd21
184184
+ nexus 466a9f29-62bf-4e63-924a-b9efdb86afec install dataset in service fd00:1122:3344:102::22
185185

186186

187-
sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6 (active, config generation 1 -> 2):
187+
sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6 (active, config generation 2):
188188

189189
host phase 2 contents:
190190
------------------------
191191
slot boot image source
192192
------------------------
193-
A current contents
194-
B current contents
193+
+ A current contents
194+
+ B current contents
195195

196196

197197
physical disks:
@@ -294,14 +294,14 @@ to: blueprint dbcbd3d6-41ff-48ae-ac0b-1becc9b2fd21
294294
+ nexus 0c71b3b2-6ceb-4e8f-b020-b08675e83038 install dataset in service fd00:1122:3344:101::22
295295

296296

297-
sled d81c6a84-79b8-4958-ae41-ea46c9b19763 (active, config generation 1 -> 2):
297+
sled d81c6a84-79b8-4958-ae41-ea46c9b19763 (active, config generation 2):
298298

299299
host phase 2 contents:
300300
------------------------
301301
slot boot image source
302302
------------------------
303-
A current contents
304-
B current contents
303+
+ A current contents
304+
+ B current contents
305305

306306

307307
physical disks:

‎dev-tools/reconfigurator-cli/tests/output/cmds-example-stdout‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -694,16 +694,16 @@ external DNS:
694694
from: blueprint 02697f74-b14a-4418-90f0-c28b2a3a6aa9
695695
to: blueprint 86db3308-f817-4626-8838-4085949a6a41
696696

697-
MODIFIED SLEDS:
697+
ADDED SLEDS:
698698

699-
sled 89d02b1b-478c-401a-8e28-7a26f74fa41b (active, config generation 1 -> 2):
699+
sled 89d02b1b-478c-401a-8e28-7a26f74fa41b (active, config generation 2):
700700

701701
host phase 2 contents:
702702
------------------------
703703
slot boot image source
704704
------------------------
705-
A current contents
706-
B current contents
705+
+ A current contents
706+
+ B current contents
707707

708708

709709
physical disks:
@@ -914,16 +914,16 @@ external DNS:
914914
from: blueprint 86db3308-f817-4626-8838-4085949a6a41
915915
to: blueprint 02697f74-b14a-4418-90f0-c28b2a3a6aa9
916916

917-
MODIFIED SLEDS:
917+
REMOVED SLEDS:
918918

919-
sled 89d02b1b-478c-401a-8e28-7a26f74fa41b (active, config generation 2 -> 1):
919+
sled 89d02b1b-478c-401a-8e28-7a26f74fa41b (was active, config generation 2):
920920

921921
host phase 2 contents:
922922
------------------------
923923
slot boot image source
924924
------------------------
925-
A current contents
926-
B current contents
925+
- A current contents
926+
- B current contents
927927

928928

929929
physical disks:

‎nexus/db-queries/src/db/datastore/deployment.rs‎

Lines changed: 37 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -3146,20 +3146,16 @@ mod tests {
31463146
use nexus_types::deployment::BlueprintHostPhase2DesiredContents;
31473147
use nexus_types::deployment::BlueprintHostPhase2DesiredSlots;
31483148
use nexus_types::deployment::BlueprintPhysicalDiskDisposition;
3149-
use nexus_types::deployment::BlueprintZoneConfig;
31503149
use nexus_types::deployment::BlueprintZoneDisposition;
31513150
use nexus_types::deployment::BlueprintZoneImageSource;
3152-
use nexus_types::deployment::BlueprintZoneType;
31533151
use nexus_types::deployment::ExpectedActiveRotSlot;
3154-
use nexus_types::deployment::OmicronZoneExternalFloatingIp;
31553152
use nexus_types::deployment::PendingMgsUpdate;
31563153
use nexus_types::deployment::PlanningInput;
31573154
use nexus_types::deployment::PlanningInputBuilder;
31583155
use nexus_types::deployment::SledDetails;
31593156
use nexus_types::deployment::SledDisk;
31603157
use nexus_types::deployment::SledFilter;
31613158
use nexus_types::deployment::SledResources;
3162-
use nexus_types::deployment::blueprint_zone_type;
31633159
use nexus_types::external_api::views::PhysicalDiskPolicy;
31643160
use nexus_types::external_api::views::PhysicalDiskState;
31653161
use nexus_types::external_api::views::SledPolicy;
@@ -3168,36 +3164,24 @@ mod tests {
31683164
use nexus_types::inventory::Collection;
31693165
use omicron_common::address::IpRange;
31703166
use omicron_common::address::Ipv6Subnet;
3171-
use omicron_common::api::external::MacAddr;
3172-
use omicron_common::api::external::Name;
31733167
use omicron_common::api::external::TufArtifactMeta;
31743168
use omicron_common::api::external::TufRepoDescription;
31753169
use omicron_common::api::external::TufRepoMeta;
3176-
use omicron_common::api::external::Vni;
3177-
use omicron_common::api::internal::shared::NetworkInterface;
3178-
use omicron_common::api::internal::shared::NetworkInterfaceKind;
31793170
use omicron_common::disk::DiskIdentity;
31803171
use omicron_common::disk::M2Slot;
31813172
use omicron_common::update::ArtifactId;
3182-
use omicron_common::zpool_name::ZpoolName;
31833173
use omicron_test_utils::dev;
31843174
use omicron_test_utils::dev::poll::CondCheckError;
31853175
use omicron_test_utils::dev::poll::wait_for_condition;
3186-
use omicron_uuid_kinds::ExternalIpUuid;
31873176
use omicron_uuid_kinds::OmicronZoneUuid;
31883177
use omicron_uuid_kinds::PhysicalDiskUuid;
31893178
use omicron_uuid_kinds::SledUuid;
31903179
use omicron_uuid_kinds::ZpoolUuid;
3191-
use oxnet::IpNet;
31923180
use pretty_assertions::assert_eq;
31933181
use rand::Rng;
31943182
use std::collections::BTreeSet;
31953183
use std::mem;
3196-
use std::net::IpAddr;
3197-
use std::net::Ipv4Addr;
31983184
use std::net::Ipv6Addr;
3199-
use std::net::SocketAddrV6;
3200-
use std::str::FromStr;
32013185
use std::sync::Arc;
32023186
use std::sync::LazyLock;
32033187
use std::sync::atomic::AtomicBool;
@@ -3311,10 +3295,7 @@ mod tests {
33113295
let (opctx, datastore) = (db.opctx(), db.datastore());
33123296

33133297
// Create an empty blueprint from it
3314-
let blueprint1 = BlueprintBuilder::build_empty_with_sleds(
3315-
std::iter::empty(),
3316-
"test",
3317-
);
3298+
let blueprint1 = BlueprintBuilder::build_empty("test");
33183299
let authz_blueprint = authz_blueprint_from_id(blueprint1.id);
33193300

33203301
// Trying to read it from the database should fail with the relevant
@@ -4039,10 +4020,7 @@ mod tests {
40394020
// Create three blueprints:
40404021
// * `blueprint1` has no parent
40414022
// * `blueprint2` and `blueprint3` both have `blueprint1` as parent
4042-
let blueprint1 = BlueprintBuilder::build_empty_with_sleds(
4043-
std::iter::empty(),
4044-
"test1",
4045-
);
4023+
let blueprint1 = BlueprintBuilder::build_empty("test1");
40464024
let blueprint2 = BlueprintBuilder::new_based_on(
40474025
&logctx.log,
40484026
&blueprint1,
@@ -4190,10 +4168,7 @@ mod tests {
41904168
let (opctx, datastore) = (db.opctx(), db.datastore());
41914169

41924170
// Create an initial blueprint and a child.
4193-
let blueprint1 = BlueprintBuilder::build_empty_with_sleds(
4194-
std::iter::empty(),
4195-
"test1",
4196-
);
4171+
let blueprint1 = BlueprintBuilder::build_empty("test1");
41974172
let blueprint2 = BlueprintBuilder::new_based_on(
41984173
&logctx.log,
41994174
&blueprint1,
@@ -4292,104 +4267,46 @@ mod tests {
42924267
logctx.cleanup_successful();
42934268
}
42944269

4295-
async fn create_blueprint_with_external_ip(
4296-
datastore: &DataStore,
4297-
opctx: &OpContext,
4298-
) -> Blueprint {
4299-
// Create an initial blueprint and a child.
4300-
let sled_id = SledUuid::new_v4();
4301-
let mut blueprint = BlueprintBuilder::build_empty_with_sleds(
4302-
[sled_id].into_iter(),
4303-
"test1",
4304-
);
4305-
4306-
// To observe realistic database behavior, we need the invocation of
4307-
// "blueprint_ensure_external_networking_resources" to actually write something
4308-
// back to the database.
4309-
//
4310-
// While this is *mostly* made-up blueprint contents, the part that matters
4311-
// is that it's provisioning a zone (Nexus) which does have resources
4312-
// to be allocated.
4313-
let ip_range = IpRange::try_from((
4314-
Ipv4Addr::new(10, 0, 0, 1),
4315-
Ipv4Addr::new(10, 0, 0, 10),
4316-
))
4317-
.unwrap();
4318-
let (service_authz_ip_pool, service_ip_pool) = datastore
4319-
.ip_pools_service_lookup(&opctx, IpVersion::V4)
4320-
.await
4321-
.expect("lookup service ip pool");
4322-
datastore
4323-
.ip_pool_add_range(
4324-
&opctx,
4325-
&service_authz_ip_pool,
4326-
&service_ip_pool,
4327-
&ip_range,
4328-
)
4329-
.await
4330-
.expect("add range to service ip pool");
4331-
let zone_id = OmicronZoneUuid::new_v4();
4332-
blueprint
4333-
.sleds
4334-
.get_mut(&sled_id)
4335-
.unwrap()
4336-
.zones
4337-
.insert_unique(BlueprintZoneConfig {
4338-
disposition: BlueprintZoneDisposition::InService,
4339-
id: zone_id,
4340-
filesystem_pool: ZpoolName::new_external(ZpoolUuid::new_v4()),
4341-
zone_type: BlueprintZoneType::Nexus(
4342-
blueprint_zone_type::Nexus {
4343-
internal_address: SocketAddrV6::new(
4344-
Ipv6Addr::LOCALHOST,
4345-
0,
4346-
0,
4347-
0,
4348-
),
4349-
lockstep_port: 0,
4350-
external_ip: OmicronZoneExternalFloatingIp {
4351-
id: ExternalIpUuid::new_v4(),
4352-
ip: "10.0.0.1".parse().unwrap(),
4353-
},
4354-
nic: NetworkInterface {
4355-
id: Uuid::new_v4(),
4356-
kind: NetworkInterfaceKind::Service {
4357-
id: *zone_id.as_untyped_uuid(),
4358-
},
4359-
name: Name::from_str("mynic").unwrap(),
4360-
ip: "172.30.2.6".parse().unwrap(),
4361-
mac: MacAddr::random_system(),
4362-
subnet: IpNet::host_net(IpAddr::V6(
4363-
Ipv6Addr::LOCALHOST,
4364-
)),
4365-
vni: Vni::random(),
4366-
primary: true,
4367-
slot: 1,
4368-
transit_ips: vec![],
4369-
},
4370-
external_tls: false,
4371-
external_dns_servers: vec![],
4372-
nexus_generation: Generation::new(),
4373-
},
4374-
),
4375-
image_source: BlueprintZoneImageSource::InstallDataset,
4376-
})
4377-
.expect("freshly generated zone IDs are unique");
4378-
4379-
blueprint
4380-
}
4381-
43824270
#[tokio::test]
43834271
async fn test_ensure_external_networking_works_with_good_target() {
4272+
const TEST_NAME: &str =
4273+
"test_ensure_external_networking_works_with_good_target";
43844274
// Setup
4385-
let logctx = dev::test_setup_log(
4386-
"test_ensure_external_networking_works_with_good_target",
4387-
);
4275+
let logctx = dev::test_setup_log(TEST_NAME);
43884276
let db = TestDatabase::new_with_datastore(&logctx.log).await;
43894277
let (opctx, datastore) = (db.opctx(), db.datastore());
43904278

4391-
let blueprint =
4392-
create_blueprint_with_external_ip(&datastore, &opctx).await;
4279+
let (example, mut blueprint) =
4280+
ExampleSystemBuilder::new(&opctx.log, TEST_NAME).build();
4281+
4282+
// Insert the IP pool ranges used by our example system.
4283+
for pool_range in
4284+
example.system.external_ip_policy().clone().into_raw_ranges()
4285+
{
4286+
// This looks up the pool again for each range; we only need at most
4287+
// two (one V4, one V6), but our example system doesn't have many
4288+
// ranges so this should be fine.
4289+
let (service_authz_ip_pool, service_ip_pool) = datastore
4290+
.ip_pools_service_lookup(&opctx, pool_range.version().into())
4291+
.await
4292+
.expect("lookup service ip pool");
4293+
datastore
4294+
.ip_pool_add_range(
4295+
&opctx,
4296+
&service_authz_ip_pool,
4297+
&service_ip_pool,
4298+
&pool_range,
4299+
)
4300+
.await
4301+
.expect("add range to service IP pool");
4302+
}
4303+
4304+
// `ExampleSystemBuilder` returns a blueprint that has an empty parent.
4305+
// To make `blueprint` the target, we have to either insert that parent
4306+
// and make it the target first, or modify `blueprint` to make it look
4307+
// like it's the original. The latter is shorter.
4308+
blueprint.parent_blueprint_id = None;
4309+
43934310
datastore.blueprint_insert(&opctx, &blueprint).await.unwrap();
43944311

43954312
let bp_target = BlueprintTarget {

0 commit comments

Comments
 (0)