Skip to content

Commit 20e3202

Browse files
smkleinsmklein
authored
[nexus] Make it 'more default' for Debug datasets to exist in test env (#7982)
Before this PR: - `DiskTest::new` did not create any debug datasets - `omicron-dev run-all` did not initialize any disks - Any tests wanting debug datasets would need to explicitly request them, then call `disk_test.propagate_datasets_to_sleds()`. - Additionally, in this environment, many tests assume "the only tests which exist are crucible datasets". After this PR: - `DiskTest::new` does creates debug datasets on all zpools - `omicron-dev run-all` initializes disks on the "first sled agent", with datasets - Any tests using `DiskTest::new` will get them by default - Tests are adjusted to cope with multiple dataset types
1 parent 3675075 commit 20e3202

File tree

12 files changed

+217
-217
lines changed

12 files changed

+217
-217
lines changed

‎dev-tools/omicron-dev/src/main.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use futures::StreamExt;
88
use libc::SIGINT;
99
use nexus_config::NexusConfig;
1010
use nexus_test_interface::NexusServer;
11+
use nexus_test_utils::resource_helpers::DiskTest;
1112
use signal_hook_tokio::Signals;
1213

1314
#[tokio::main]
@@ -78,6 +79,18 @@ impl RunAllArgs {
7879
>(&mut config, 0)
7980
.await
8081
.context("error setting up services")?;
82+
83+
println!("omicron-dev: Adding disks to first sled agent");
84+
85+
// This is how our integration tests are identifying that "disks exist"
86+
// within the database.
87+
//
88+
// This inserts:
89+
// - DEFAULT_ZPOOL_COUNT zpools, each of which contains:
90+
// - A crucible dataset
91+
// - A debug dataset
92+
DiskTest::new(&cptestctx).await;
93+
8194
println!("omicron-dev: services are running.");
8295

8396
// Print out basic information about what was started.

‎nexus/src/app/background/tasks/read_only_region_replacement_start.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,15 +209,14 @@ mod test {
209209
datastore.clone(),
210210
);
211211

212-
// Record which datasets map to which zpools for later
212+
// Record which crucible datasets map to which zpools for later
213213

214214
let mut dataset_to_zpool: BTreeMap<ZpoolUuid, DatasetUuid> =
215215
BTreeMap::default();
216216

217217
for zpool in disk_test.zpools() {
218-
for dataset in &zpool.datasets {
219-
dataset_to_zpool.insert(zpool.id, dataset.id);
220-
}
218+
let dataset = zpool.crucible_dataset();
219+
dataset_to_zpool.insert(zpool.id, dataset.id);
221220
}
222221

223222
let mut task =

‎nexus/src/app/background/tasks/region_snapshot_replacement_start.rs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -451,20 +451,19 @@ mod test {
451451
BTreeMap::default();
452452

453453
for zpool in disk_test.zpools() {
454-
for dataset in &zpool.datasets {
455-
dataset_to_zpool
456-
.insert(zpool.id.to_string(), dataset.id.to_string());
457-
458-
datastore
459-
.region_snapshot_create(RegionSnapshot::new(
460-
dataset.id,
461-
region_id,
462-
snapshot_id,
463-
String::from("[fd00:1122:3344::101]:12345"),
464-
))
465-
.await
466-
.unwrap();
467-
}
454+
let dataset = zpool.crucible_dataset();
455+
dataset_to_zpool
456+
.insert(zpool.id.to_string(), dataset.id.to_string());
457+
458+
datastore
459+
.region_snapshot_create(RegionSnapshot::new(
460+
dataset.id,
461+
region_id,
462+
snapshot_id,
463+
String::from("[fd00:1122:3344::101]:12345"),
464+
))
465+
.await
466+
.unwrap();
468467
}
469468

470469
// Create the fake snapshot
@@ -631,7 +630,8 @@ mod test {
631630

632631
let disk_test = DiskTest::new(cptestctx).await;
633632

634-
let dataset_id = disk_test.zpools().next().unwrap().datasets[0].id;
633+
let dataset_id =
634+
disk_test.zpools().next().unwrap().crucible_dataset().id;
635635

636636
// Add a region snapshot replacement request for a fake region snapshot
637637

‎nexus/src/app/sagas/disk_create.rs‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,28 +1010,23 @@ pub(crate) mod test {
10101010
test: &DiskTest<'_>,
10111011
) -> bool {
10121012
for zpool in test.zpools() {
1013-
for dataset in &zpool.datasets {
1014-
if datastore
1015-
.regions_total_reserved_size(dataset.id)
1016-
.await
1017-
.unwrap()
1018-
!= 0
1019-
{
1020-
return false;
1021-
}
1013+
let dataset = zpool.crucible_dataset();
1014+
if datastore.regions_total_reserved_size(dataset.id).await.unwrap()
1015+
!= 0
1016+
{
1017+
return false;
10221018
}
10231019
}
10241020
true
10251021
}
10261022

10271023
fn no_regions_ensured(sled_agent: &SledAgent, test: &DiskTest<'_>) -> bool {
10281024
for zpool in test.zpools() {
1029-
for dataset in &zpool.datasets {
1030-
let crucible_dataset =
1031-
sled_agent.get_crucible_dataset(zpool.id, dataset.id);
1032-
if !crucible_dataset.is_empty() {
1033-
return false;
1034-
}
1025+
let dataset = zpool.crucible_dataset();
1026+
let crucible_dataset =
1027+
sled_agent.get_crucible_dataset(zpool.id, dataset.id);
1028+
if !crucible_dataset.is_empty() {
1029+
return false;
10351030
}
10361031
}
10371032
true

‎nexus/src/app/sagas/region_replacement_start.rs‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,8 @@ pub(crate) mod test {
792792
nexus_test_utils::ControlPlaneTestContext<crate::Server>;
793793
type DiskTest<'a> =
794794
nexus_test_utils::resource_helpers::DiskTest<'a, crate::Server>;
795+
type DiskTestBuilder<'a> =
796+
nexus_test_utils::resource_helpers::DiskTestBuilder<'a, crate::Server>;
795797

796798
const DISK_NAME: &str = "my-disk";
797799
const PROJECT_NAME: &str = "springfield-squidport";
@@ -800,8 +802,8 @@ pub(crate) mod test {
800802
async fn test_region_replacement_start_saga(
801803
cptestctx: &ControlPlaneTestContext,
802804
) {
803-
let mut disk_test = DiskTest::new(cptestctx).await;
804-
disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await;
805+
let _disk_test =
806+
DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await;
805807

806808
let client = &cptestctx.external_client;
807809
let nexus = &cptestctx.server.server_context().nexus;
@@ -1026,15 +1028,11 @@ pub(crate) mod test {
10261028
let mut count = 0;
10271029

10281030
for zpool in test.zpools() {
1029-
for dataset in &zpool.datasets {
1030-
if datastore
1031-
.regions_total_reserved_size(dataset.id)
1032-
.await
1033-
.unwrap()
1034-
!= 0
1035-
{
1036-
count += 1;
1037-
}
1031+
let dataset = zpool.crucible_dataset();
1032+
if datastore.regions_total_reserved_size(dataset.id).await.unwrap()
1033+
!= 0
1034+
{
1035+
count += 1;
10381036
}
10391037
}
10401038

@@ -1119,8 +1117,8 @@ pub(crate) mod test {
11191117
async fn test_action_failure_can_unwind_idempotently(
11201118
cptestctx: &ControlPlaneTestContext,
11211119
) {
1122-
let mut disk_test = DiskTest::new(cptestctx).await;
1123-
disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await;
1120+
let disk_test =
1121+
DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await;
11241122

11251123
let log = &cptestctx.logctx.log;
11261124

@@ -1196,8 +1194,8 @@ pub(crate) mod test {
11961194
async fn test_actions_succeed_idempotently(
11971195
cptestctx: &ControlPlaneTestContext,
11981196
) {
1199-
let mut disk_test = DiskTest::new(cptestctx).await;
1200-
disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await;
1197+
let _disk_test =
1198+
DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await;
12011199

12021200
let client = &cptestctx.external_client;
12031201
let nexus = &cptestctx.server.server_context().nexus;

‎nexus/src/app/sagas/region_snapshot_replacement_start.rs‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,8 +1259,8 @@ pub(crate) mod test {
12591259

12601260
assert_eq!(region_allocations(&datastore).await, 0);
12611261

1262-
let mut disk_test = DiskTest::new(cptestctx).await;
1263-
disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await;
1262+
let disk_test =
1263+
DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await;
12641264

12651265
assert_eq!(region_allocations(&datastore).await, 0);
12661266

@@ -1500,20 +1500,18 @@ pub(crate) mod test {
15001500
let mut non_destroyed_regions_from_agent = vec![];
15011501

15021502
for zpool in test.zpools() {
1503-
for dataset in &zpool.datasets {
1504-
let crucible_dataset =
1505-
sled_agent.get_crucible_dataset(zpool.id, dataset.id);
1506-
for region in crucible_dataset.list() {
1507-
match region.state {
1508-
crucible_agent_client::types::State::Tombstoned
1509-
| crucible_agent_client::types::State::Destroyed => {
1510-
// ok
1511-
}
1512-
1513-
_ => {
1514-
non_destroyed_regions_from_agent
1515-
.push(region.clone());
1516-
}
1503+
let dataset = zpool.crucible_dataset();
1504+
let crucible_dataset =
1505+
sled_agent.get_crucible_dataset(zpool.id, dataset.id);
1506+
for region in crucible_dataset.list() {
1507+
match region.state {
1508+
crucible_agent_client::types::State::Tombstoned
1509+
| crucible_agent_client::types::State::Destroyed => {
1510+
// ok
1511+
}
1512+
1513+
_ => {
1514+
non_destroyed_regions_from_agent.push(region.clone());
15171515
}
15181516
}
15191517
}

0 commit comments

Comments
 (0)