Skip to content

Commit

Permalink
Update physical disk test to identify that UUID generation is determi…
Browse files Browse the repository at this point in the history
…nistic now
  • Loading branch information
smklein committed Dec 28, 2023
1 parent d2dd6fe commit 79e6446
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 21 deletions.
38 changes: 17 additions & 21 deletions nexus/db-queries/src/db/datastore/physical_disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,10 @@ mod test {
}

// Only checking some fields:
// - The UUID of the disk may actually not be the same as the upserted one;
// the "vendor/serial/model" value is the more critical unique identifier.
// NOTE: Could we derive a UUID from the VSM values?
// - The 'time' field precision can be modified slightly when inserted into
// the DB.
fn assert_disks_equal_ignore_uuid(lhs: &PhysicalDisk, rhs: &PhysicalDisk) {
fn assert_disks_equal_ignore_time(lhs: &PhysicalDisk, rhs: &PhysicalDisk) {
assert_eq!(lhs.uuid(), rhs.uuid());
assert_eq!(lhs.time_deleted().is_some(), rhs.time_deleted().is_some());
assert_eq!(lhs.vendor, rhs.vendor);
assert_eq!(lhs.serial, rhs.serial);
Expand All @@ -257,9 +255,9 @@ mod test {
}

#[tokio::test]
async fn physical_disk_upsert_different_uuid_idempotent() {
async fn physical_disk_upsert_uuid_generation_deterministic() {
let logctx = dev::test_setup_log(
"physical_disk_upsert_different_uuid_idempotent",
"physical_disk_upsert_uuid_generation_deterministic",
);
let mut db = test_setup_database(&logctx.log).await;
let (opctx, datastore) = datastore_test(&logctx, &db).await;
Expand All @@ -280,7 +278,7 @@ mod test {
.await
.expect("Failed first attempt at upserting disk");
assert_eq!(disk.uuid(), first_observed_disk.uuid());
assert_disks_equal_ignore_uuid(&disk, &first_observed_disk);
assert_disks_equal_ignore_time(&disk, &first_observed_disk);

// Observe the inserted disk
let pagparams = list_disk_params();
Expand All @@ -290,25 +288,27 @@ mod test {
.expect("Failed to list physical disks");
assert_eq!(disks.len(), 1);
assert_eq!(disk.uuid(), disks[0].uuid());
assert_disks_equal_ignore_uuid(&disk, &disks[0]);
assert_disks_equal_ignore_time(&disk, &disks[0]);

// Insert the same disk, with a different UUID primary key
// Insert the same disk, but don't re-state the UUID.
//
// The rest of this test relies on the UUID derivation being
// deterministic.
let disk_again = PhysicalDisk::new(
String::from("Oxide"),
String::from("123"),
String::from("FakeDisk"),
PhysicalDiskKind::U2,
sled_id,
);
// With the same input parameters, the UUID should be deterministic.
assert_eq!(disk.uuid(), disk_again.uuid());

let second_observed_disk = datastore
.physical_disk_upsert(&opctx, disk_again.clone())
.await
.expect("Failed second upsert of physical disk");
// This check is pretty important - note that we return the original
// UUID, not the new one.
assert_ne!(disk_again.uuid(), second_observed_disk.uuid());
assert_eq!(disk_again.id(), second_observed_disk.id());
assert_disks_equal_ignore_uuid(&disk_again, &second_observed_disk);
assert_disks_equal_ignore_time(&disk_again, &second_observed_disk);
assert!(
first_observed_disk.time_modified()
<= second_observed_disk.time_modified()
Expand All @@ -318,13 +318,9 @@ mod test {
.sled_list_physical_disks(&opctx, sled_id, &pagparams)
.await
.expect("Failed to re-list physical disks");

// We'll use the old primary key
assert_eq!(disks.len(), 1);
assert_eq!(disk.uuid(), disks[0].uuid());
assert_ne!(disk_again.uuid(), disks[0].uuid());
assert_disks_equal_ignore_uuid(&disk, &disks[0]);
assert_disks_equal_ignore_uuid(&disk_again, &disks[0]);
assert_disks_equal_ignore_time(&disk, &disks[0]);
assert_disks_equal_ignore_time(&disk_again, &disks[0]);

db.cleanup().await.unwrap();
logctx.cleanup_successful();
Expand Down Expand Up @@ -364,7 +360,7 @@ mod test {
first_observed_disk.time_modified()
<= second_observed_disk.time_modified()
);
assert_disks_equal_ignore_uuid(
assert_disks_equal_ignore_time(
&first_observed_disk,
&second_observed_disk,
);
Expand Down
41 changes: 41 additions & 0 deletions nexus/preprocessed_configs/config.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<!-- This file was generated automatically.
Do not edit it: it is likely to be discarded and generated again before it's read next time.
Files used to generate this file:
config.xml -->

<!-- Config that is used when server is run without config file. --><clickhouse>
<logger>
<level>trace</level>
<console>true</console>
</logger>

<http_port>8123</http_port>
<tcp_port>9000</tcp_port>
<mysql_port>9004</mysql_port>

<path>./</path>

<mlock_executable>true</mlock_executable>

<users>
<default>
<password/>

<networks>
<ip>::/0</ip>
</networks>

<profile>default</profile>
<quota>default</quota>
<access_management>1</access_management>
</default>
</users>

<profiles>
<default/>
</profiles>

<quotas>
<default/>
</quotas>
</clickhouse>

0 comments on commit 79e6446

Please sign in to comment.