Skip to content

Commit

Permalink
Add some tests
Browse files Browse the repository at this point in the history
  • Loading branch information
papertigers committed Sep 4, 2024
1 parent fdf5ab4 commit 633cc23
Show file tree
Hide file tree
Showing 4 changed files with 365 additions and 8 deletions.
148 changes: 148 additions & 0 deletions nexus/db-model/src/inventory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1546,3 +1546,151 @@ impl InvOmicronZoneNic {
zone_nic.into_network_interface_for_zone(zone_id)
}
}

#[cfg(test)]
mod test {
use omicron_uuid_kinds::TypedUuid;

use crate::{typed_uuid, InvNvmeDiskFirmware};

#[test]
fn test_inv_nvme_disk_firmware() {
// We can retrieve active_firmware with sane values
let inv_firmware = InvNvmeDiskFirmware {
inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
sled_id: TypedUuid::new_v4().into(),
slot: 1,
active_slot: 1.into(),
next_active_slot: None,
number_of_slots: 1.into(),
slot1_is_read_only: true,
slot_firmware_versions: vec![Some("firmware".to_string())],
};
assert_eq!(inv_firmware.current_version(), Some("firmware"));
assert_eq!(inv_firmware.next_version(), None);

// We can retrieve active_firmware and next_active_firmware with sane
// values
let inv_firmware = InvNvmeDiskFirmware {
inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
sled_id: TypedUuid::new_v4().into(),
slot: 1,
active_slot: 1.into(),
next_active_slot: Some(2.into()),
number_of_slots: 2.into(),
slot1_is_read_only: true,
slot_firmware_versions: vec![
Some("ONE".to_string()),
Some("TWO".to_string()),
],
};
assert_eq!(inv_firmware.current_version(), Some("ONE"));
assert_eq!(inv_firmware.next_version(), Some("TWO"));

// A missing fw version string returns None
let inv_firmware = InvNvmeDiskFirmware {
inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
sled_id: TypedUuid::new_v4().into(),
slot: 1,
active_slot: 1.into(),
next_active_slot: Some(2.into()),
number_of_slots: 2.into(),
slot1_is_read_only: true,
slot_firmware_versions: vec![Some("ONE".to_string()), None],
};
assert_eq!(inv_firmware.current_version(), Some("ONE"));
assert_eq!(inv_firmware.next_version(), None);

// Number of slots and slot firmware versions disagreement returns None
let inv_firmware = InvNvmeDiskFirmware {
inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
sled_id: TypedUuid::new_v4().into(),
slot: 1,
active_slot: 1.into(),
next_active_slot: Some(4.into()),
number_of_slots: 4.into(),
slot1_is_read_only: true,
// number_of_slots reports 4 but the device only gave us two slots
slot_firmware_versions: vec![Some("ONE".to_string()), None],
};
assert_eq!(inv_firmware.current_version(), Some("ONE"));
assert_eq!(inv_firmware.next_version(), None);

// Active slot is below 1
let inv_firmware = InvNvmeDiskFirmware {
inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
sled_id: TypedUuid::new_v4().into(),
slot: 1,
active_slot: 0.into(),
next_active_slot: None,
number_of_slots: 1.into(),
slot1_is_read_only: true,
slot_firmware_versions: vec![Some("ONE".to_string())],
};
assert_eq!(inv_firmware.current_version(), None);
assert_eq!(inv_firmware.next_version(), None);

// Active slot is above 7
let inv_firmware = InvNvmeDiskFirmware {
inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
sled_id: TypedUuid::new_v4().into(),
slot: 1,
active_slot: 8.into(),
next_active_slot: None,
number_of_slots: 8.into(),
slot1_is_read_only: true,
slot_firmware_versions: vec![
None,
None,
None,
None,
None,
None,
None,
Some("EIGHT".to_string()),
],
};
assert_eq!(inv_firmware.current_version(), None);
assert_eq!(inv_firmware.next_version(), None);

// Next active slot is below 1
let inv_firmware = InvNvmeDiskFirmware {
inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
sled_id: TypedUuid::new_v4().into(),
slot: 1,
active_slot: 1.into(),
next_active_slot: Some(0.into()),
number_of_slots: 2.into(),
slot1_is_read_only: true,
slot_firmware_versions: vec![
Some("ONE".to_string()),
Some("TWO".to_string()),
],
};
assert_eq!(inv_firmware.current_version(), Some("ONE"));
assert_eq!(inv_firmware.next_version(), None);

// Next active slot is above 7
let inv_firmware = InvNvmeDiskFirmware {
inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
sled_id: TypedUuid::new_v4().into(),
slot: 1,
active_slot: 1.into(),
next_active_slot: Some(8.into()),
number_of_slots: 8.into(),
slot1_is_read_only: true,
slot_firmware_versions: vec![
None,
None,
None,
None,
None,
None,
None,
Some("EIGHT".to_string()),
],
};
assert_eq!(inv_firmware.current_version(), None);
assert_eq!(inv_firmware.next_version(), None);
}
}
211 changes: 211 additions & 0 deletions nexus/db-queries/src/db/datastore/inventory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2825,4 +2825,215 @@ mod test {
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}

/// While other tests mostly exercise inserting [`PhysicalDisk`]s with
/// NVMe firmware we want additional checks on some constraints that the
/// `inv_nvme_disk_firwmare` table sets.
#[tokio::test]
async fn test_nvme_firmware_inventory_insert() {
use nexus_types::external_api::params::PhysicalDiskKind;
use nexus_types::inventory::{
Collection, NvmeFirmware, PhysicalDisk, PhysicalDiskFirmware,
};

fn add_disk_with_firmware_to_collection(
active_slot: u8,
next_active_slot: Option<u8>,
number_of_slots: u8,
slot1_is_read_only: bool,
slot_firmware_versions: Vec<Option<String>>,
) -> Collection {
let Representative { builder, .. } = representative();
let mut collection = builder.build();

let disk = PhysicalDisk {
identity: omicron_common::disk::DiskIdentity {
vendor: "mike".to_string(),
model: "buffalo".to_string(),
serial: "716".to_string(),
},
variant: PhysicalDiskKind::U2,

slot: 1,
firmware: PhysicalDiskFirmware::Nvme(NvmeFirmware {
active_slot,
next_active_slot,
number_of_slots,
slot1_is_read_only,
slot_firmware_versions,
}),
};

// Remove all disks on the first sled-agent and replace it with our
// new physical disk.
std::mem::swap(&mut collection
.sled_agents
.first_entry()
.expect("collection built from representative should have a sled agent")
.into_mut()
.disks,
&mut vec![disk]);

collection
}

let logctx = dev::test_setup_log("nvme_firmware_inventory_insert");
let mut db = test_setup_database(&logctx.log).await;
let (opctx, datastore) = datastore_test(&logctx, &db).await;

// Verify we don't allow firmware with no firmware slots
let collection = add_disk_with_firmware_to_collection(
1,
None,
0,
true,
vec![Some("TEST".to_string())],
);
datastore
.inventory_insert_collection(&opctx, &collection)
.await
.expect_err("cannot insert firmware with 0 slots");

// Verify we don't allow firmware with too many slots
let collection = add_disk_with_firmware_to_collection(
1,
None,
8,
true,
vec![Some("TEST".to_string())],
);
datastore
.inventory_insert_collection(&opctx, &collection)
.await
.expect_err("cannot insert firmware with 8 slots");

// Verify we don't allow an active_slot of 0
let collection = add_disk_with_firmware_to_collection(
0,
None,
1,
true,
vec![Some("TEST".to_string())],
);
datastore
.inventory_insert_collection(&opctx, &collection)
.await
.expect_err("cannot insert firmware with active_slot 0");

// Verify we don't allow an active_slot larger than 7
let collection = add_disk_with_firmware_to_collection(
8,
None,
1,
true,
vec![Some("TEST".to_string())],
);
datastore
.inventory_insert_collection(&opctx, &collection)
.await
.expect_err("cannot insert firmware with active_slot 8");

// Verify we don't allow an next_active_slot of 0
let collection = add_disk_with_firmware_to_collection(
1,
Some(0),
1,
true,
vec![Some("TEST".to_string())],
);
datastore
.inventory_insert_collection(&opctx, &collection)
.await
.expect_err("cannot insert firmware with next_active_slot 0");

// Verify we don't allow an next_active_slot larger than 7
let collection = add_disk_with_firmware_to_collection(
1,
Some(8),
1,
true,
vec![Some("TEST".to_string())],
);
datastore
.inventory_insert_collection(&opctx, &collection)
.await
.expect_err("cannot insert firmware with next_active_slot 8");

// XXX omicron#6514
// - We would like to deny insertion when a firmware version string is
// larger than the NVMe spec would allow but we have discoverd that our
// version of CRDB will silently truncate a string to size bound rather
// than erroring.
// - The NVMe spec defines the firmware versions as an array of 7
// entries made up of 8 characters (ascii data) so unless the device
// firmware has a bug or the rust libnvme binding has a bug the worst we
// should see is the truncated value.
// - The CRDB STRING type is checking unicode length rather than byte
// length. However, CRDB won't let us use the BYTES type with an ARRAY.
let collection = add_disk_with_firmware_to_collection(
1,
None,
1,
true,
vec![Some("FIRMWARETOOLARGE".to_string())],
);
datastore
.inventory_insert_collection(&opctx, &collection)
.await
.expect("firmware inserted modulo version string truncation");
let collection_read = datastore
.inventory_collection_read(&opctx, collection.id)
.await
.expect("failed to read collection back");
let (_, value) = &collection_read
.sled_agents
.first_key_value()
.expect("we have a sled-agent in the collection");
let disk =
value.disks.first().expect("sled-agent has the disk we inserted");
let PhysicalDiskFirmware::Nvme(ref firmware) = disk.firmware else {
panic!(
"Didn't find the expected NVMe firmware that we just inserted"
)
};
assert_eq!(
firmware.slot_firmware_versions,
vec![Some("FIRMWARE".to_string())],
"Firmware versions contains the truncated string"
);

// Finally lets actually insert a valid configuration and assert that we
// can pull it back out correctly.
//
// - 3 slots
// - 1st slot is active
// - 1st slot is marked read-only
// - 2nd slot is marked active on next reset
// - 3rd slot has no firmware uploaded into it
let collection = add_disk_with_firmware_to_collection(
1,
Some(2),
3,
true,
vec![Some("FOO".to_string()), Some("BAR".to_string()), None],
);
datastore
.inventory_insert_collection(&opctx, &collection)
.await
.expect("firmware inserted");
let collection_read = datastore
.inventory_collection_read(&opctx, collection.id)
.await
.expect("failed to read collection back");
assert_eq!(collection, collection_read);

// NOTE that there is no validation that the bounds checked values
// in the database make sense. For example a device that reports
// a next_active_slot of 2 but only has a value of 1 in the
// number_of_slots field. These configurations are guarded against by
// the `InvNvmeDiskFirmware` type itself.

db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
}
7 changes: 3 additions & 4 deletions schema/crdb/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3187,15 +3187,14 @@ CREATE TABLE IF NOT EXISTS omicron.public.inv_nvme_disk_firmware (

-- total number of firmware slots the device has
number_of_slots INT2 CHECK (number_of_slots BETWEEN 1 AND 7) NOT NULL,
active_slot INT2 CHECK (number_of_slots BETWEEN 1 AND 7) NOT NULL,
active_slot INT2 CHECK (active_slot BETWEEN 1 AND 7) NOT NULL,
-- staged firmware slot to be active on reset
next_active_slot INT2 CHECK (number_of_slots BETWEEN 1 AND 7),
next_active_slot INT2 CHECK (next_active_slot BETWEEN 1 AND 7),
-- slot1 is distinct in the NVMe spec in the sense that it can be read only
slot1_is_read_only BOOLEAN,
-- the firmware version string for each NVMe slot (0 indexed), a NULL means the
-- slot exists but is empty
slot_firmware_versions STRING(8)[],
CONSTRAINT check_slot_firmware_versions_len CHECK (array_length(slot_firmware_versions, 1) BETWEEN 1 AND 7),
slot_firmware_versions STRING(8)[] CHECK (array_length(slot_firmware_versions, 1) BETWEEN 1 AND 7),

-- PK consisting of:
-- - Which collection this was
Expand Down
Loading

0 comments on commit 633cc23

Please sign in to comment.