Skip to content

[nexus] Make it 'more default' for Debug datasets to exist in test env #7982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 25, 2025
Merged

[nexus] Make it 'more default' for Debug datasets to exist in test env #7982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
12f461b
[nexus] Make it 'more default' for Debug datasets to exist in test en…
smklein Apr 15, 2025
1af91c6
test patching
smklein Apr 15, 2025
73b4975
Try compiling
smklein Apr 15, 2025
47819c0
Patching tests more
smklein Apr 15, 2025
ab14729
Merge branch 'main' into omicron-dev-disk-test
smklein Apr 23, 2025
7fb1168
Merge branch 'main' into omicron-dev-disk-test
smklein Apr 25, 2025
36d2d04
Make datasets private, add helpers to access them
smklein Apr 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions dev-tools/omicron-dev/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use futures::StreamExt;
use libc::SIGINT;
use nexus_config::NexusConfig;
use nexus_test_interface::NexusServer;
use nexus_test_utils::resource_helpers::DiskTest;
use signal_hook_tokio::Signals;

#[tokio::main]
Expand Down Expand Up @@ -78,6 +79,18 @@ impl RunAllArgs {
>(&mut config, 0)
.await
.context("error setting up services")?;

println!("omicron-dev: Adding disks to first sled agent");

// This is how our integration tests are identifying that "disks exist"
// within the database.
//
// This inserts:
// - DEFAULT_ZPOOL_COUNT zpools, each of which contains:
// - A crucible dataset
// - A debug dataset
DiskTest::new(&cptestctx).await;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an example of where this is useful:

#7972

We can create (and store) support bundles on the simulated system.


println!("omicron-dev: services are running.");

// Print out basic information about what was started.
Expand Down
7 changes: 3 additions & 4 deletions nexus/src/app/background/tasks/read_only_region_replacement_start.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,15 +209,14 @@ mod test {
datastore.clone(),
);

// Record which datasets map to which zpools for later
// Record which crucible datasets map to which zpools for later

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

for zpool in disk_test.zpools() {
for dataset in &zpool.datasets {
dataset_to_zpool.insert(zpool.id, dataset.id);
}
let dataset = zpool.crucible_dataset();
dataset_to_zpool.insert(zpool.id, dataset.id);
}

let mut task =
Expand Down
30 changes: 15 additions & 15 deletions nexus/src/app/background/tasks/region_snapshot_replacement_start.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,20 +451,19 @@ mod test {
BTreeMap::default();

for zpool in disk_test.zpools() {
for dataset in &zpool.datasets {
dataset_to_zpool
.insert(zpool.id.to_string(), dataset.id.to_string());

datastore
.region_snapshot_create(RegionSnapshot::new(
dataset.id,
region_id,
snapshot_id,
String::from("[fd00:1122:3344::101]:12345"),
))
.await
.unwrap();
}
let dataset = zpool.crucible_dataset();
dataset_to_zpool
.insert(zpool.id.to_string(), dataset.id.to_string());

datastore
.region_snapshot_create(RegionSnapshot::new(
dataset.id,
region_id,
snapshot_id,
String::from("[fd00:1122:3344::101]:12345"),
))
.await
.unwrap();
}

// Create the fake snapshot
Expand Down Expand Up @@ -631,7 +630,8 @@ mod test {

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

let dataset_id = disk_test.zpools().next().unwrap().datasets[0].id;
let dataset_id =
disk_test.zpools().next().unwrap().crucible_dataset().id;

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

Expand Down
25 changes: 10 additions & 15 deletions nexus/src/app/sagas/disk_create.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1010,28 +1010,23 @@ pub(crate) mod test {
test: &DiskTest<'_>,
) -> bool {
for zpool in test.zpools() {
for dataset in &zpool.datasets {
if datastore
.regions_total_reserved_size(dataset.id)
.await
.unwrap()
!= 0
{
return false;
}
let dataset = zpool.crucible_dataset();
if datastore.regions_total_reserved_size(dataset.id).await.unwrap()
!= 0
{
return false;
}
}
true
}

fn no_regions_ensured(sled_agent: &SledAgent, test: &DiskTest<'_>) -> bool {
for zpool in test.zpools() {
for dataset in &zpool.datasets {
let crucible_dataset =
sled_agent.get_crucible_dataset(zpool.id, dataset.id);
if !crucible_dataset.is_empty() {
return false;
}
let dataset = zpool.crucible_dataset();
let crucible_dataset =
sled_agent.get_crucible_dataset(zpool.id, dataset.id);
if !crucible_dataset.is_empty() {
return false;
}
}
true
Expand Down
28 changes: 13 additions & 15 deletions nexus/src/app/sagas/region_replacement_start.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,8 @@ pub(crate) mod test {
nexus_test_utils::ControlPlaneTestContext<crate::Server>;
type DiskTest<'a> =
nexus_test_utils::resource_helpers::DiskTest<'a, crate::Server>;
type DiskTestBuilder<'a> =
nexus_test_utils::resource_helpers::DiskTestBuilder<'a, crate::Server>;

const DISK_NAME: &str = "my-disk";
const PROJECT_NAME: &str = "springfield-squidport";
Expand All @@ -800,8 +802,8 @@ pub(crate) mod test {
async fn test_region_replacement_start_saga(
cptestctx: &ControlPlaneTestContext,
) {
let mut disk_test = DiskTest::new(cptestctx).await;
disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't strictly necessary, but IMO it's a little clearer -- we're calling "add_zpool_with_dataset" to make a fourth zpool, on top of the three which already exist.

let _disk_test =
DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await;

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

for zpool in test.zpools() {
for dataset in &zpool.datasets {
if datastore
.regions_total_reserved_size(dataset.id)
.await
.unwrap()
!= 0
{
count += 1;
}
let dataset = zpool.crucible_dataset();
if datastore.regions_total_reserved_size(dataset.id).await.unwrap()
!= 0
{
count += 1;
}
}

Expand Down Expand Up @@ -1119,8 +1117,8 @@ pub(crate) mod test {
async fn test_action_failure_can_unwind_idempotently(
cptestctx: &ControlPlaneTestContext,
) {
let mut disk_test = DiskTest::new(cptestctx).await;
disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await;
let disk_test =
DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await;

let log = &cptestctx.logctx.log;

Expand Down Expand Up @@ -1196,8 +1194,8 @@ pub(crate) mod test {
async fn test_actions_succeed_idempotently(
cptestctx: &ControlPlaneTestContext,
) {
let mut disk_test = DiskTest::new(cptestctx).await;
disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await;
let _disk_test =
DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await;

let client = &cptestctx.external_client;
let nexus = &cptestctx.server.server_context().nexus;
Expand Down
30 changes: 14 additions & 16 deletions nexus/src/app/sagas/region_snapshot_replacement_start.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1259,8 +1259,8 @@ pub(crate) mod test {

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

let mut disk_test = DiskTest::new(cptestctx).await;
disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await;
let disk_test =
DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await;

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

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

for zpool in test.zpools() {
for dataset in &zpool.datasets {
let crucible_dataset =
sled_agent.get_crucible_dataset(zpool.id, dataset.id);
for region in crucible_dataset.list() {
match region.state {
crucible_agent_client::types::State::Tombstoned
| crucible_agent_client::types::State::Destroyed => {
// ok
}

_ => {
non_destroyed_regions_from_agent
.push(region.clone());
}
let dataset = zpool.crucible_dataset();
let crucible_dataset =
sled_agent.get_crucible_dataset(zpool.id, dataset.id);
for region in crucible_dataset.list() {
match region.state {
crucible_agent_client::types::State::Tombstoned
| crucible_agent_client::types::State::Destroyed => {
// ok
}

_ => {
non_destroyed_regions_from_agent.push(region.clone());
}
}
}
Expand Down
Loading
Loading