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.

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

smklein
Copy link
Collaborator

@smklein smklein commented Apr 15, 2025

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

// - 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.

@@ -799,8 +801,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.

@smklein smklein marked this pull request as ready for review April 16, 2025 16:18
@smklein smklein requested review from davepacheco and jmpesp April 16, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant