[omdb] Show physical disks in blueprints and inventory

This PR changes the construction and display of blueprint diffs
significantly. The need for the changes arose out of the tight coupling
between zones and sleds. When adding disks to the mix we needed to also
take them into account when determininig when a sled was modified. We also
want to display physical disks as tables along with the zone tables
under the sleds. This turned out to be somewhat trickier than necesary,
and so I changed how the tables were constructed and rendered. Hopefully
this will also make it easier to add new tables in the future.

One possibly controversial change is that I changed the way zone
modifications are rendered. They are no longer three lines long.
Currently only `disposition` is allowed to change, and so I used an
arrow from "old" to "new" versions. Additionally, I removed the (added/
modified/removed) suffixes as they seem redundant to me, make the lines
longer, and make things harder to read IMO. I'm open to discussion about
both of these changes. My guess is that eventually, we'll want to be
able to do row filtering, and I plan to also add some colored output,
also in this PR most likely.

It should be noted that the output mechanism is decoupled from the
representation of the tables, `BpSledSubtable`. This allows output in
other formats if desired in the future.

As for the inventory collection output, I added a requirement when
filtering on sled-id, that we also filter on collection-id, because
filtering on sled-id does a full table scan and requires a new index. I
can add the index instead if we feel it's important.

Fixes #5624

clients/sled-agent-client/src/lib.rs

@@ -35,6 +35,7 @@ progenitor::generate_api!(
         PortConfigV1 = { derives = [PartialEq, Eq, Hash, Serialize, Deserialize] },
         RouteConfig = { derives = [PartialEq, Eq, Hash, Serialize, Deserialize] },
         IpNet = { derives = [PartialEq, Eq, Hash, Serialize, Deserialize] },
+        OmicronPhysicalDiskConfig = { derives = [Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord] }
     },
     //TODO trade the manual transformations later in this file for the
     //     replace directives below?
@@ -62,7 +63,6 @@ progenitor::generate_api!(
 // We cannot easily configure progenitor to derive `Eq` on all the client-
 // generated types because some have floats and other types that can't impl
 // `Eq`.  We impl it explicitly for a few types on which we need it.
-impl Eq for types::OmicronPhysicalDiskConfig {}
 impl Eq for types::OmicronPhysicalDisksConfig {}
 impl Eq for types::OmicronZonesConfig {}
 impl Eq for types::OmicronZoneConfig {}

dev-tools/omdb/src/bin/omdb/db.rs

@@ -51,6 +51,7 @@ use nexus_db_model::ExternalIp;
 use nexus_db_model::HwBaseboardId;
 use nexus_db_model::Instance;
 use nexus_db_model::InvCollection;
+use nexus_db_model::InvPhysicalDisk;
 use nexus_db_model::IpAttachState;
 use nexus_db_model::IpKind;
 use nexus_db_model::NetworkInterface;
@@ -98,6 +99,7 @@ use omicron_common::api::external::InstanceState;
 use omicron_common::api::external::MacAddr;
 use omicron_uuid_kinds::CollectionUuid;
 use omicron_uuid_kinds::GenericUuid;
+use omicron_uuid_kinds::SledUuid;
 use sled_agent_client::types::VolumeConstructionRequest;
 use std::borrow::Cow;
 use std::cmp::Ordering;
@@ -381,6 +383,8 @@ enum InventoryCommands {
     Cabooses,
     /// list and show details from particular collections
     Collections(CollectionsArgs),
+    /// show all physical disks every found
+    PhysicalDisks(PhysicalDisksArgs),
     /// list all root of trust pages ever found
     RotPages,
 }
@@ -408,6 +412,15 @@ struct CollectionsShowArgs {
     show_long_strings: bool,
 }
 
+#[derive(Debug, Args, Clone, Copy)]
+struct PhysicalDisksArgs {
+    #[clap(long)]
+    collection_id: Option<CollectionUuid>,
+
+    #[clap(long, requires("collection_id"))]
+    sled_id: Option<SledUuid>,
+}
+
 #[derive(Debug, Args)]
 struct ReconfiguratorSaveArgs {
     /// where to save the output
@@ -2652,6 +2665,9 @@ async fn cmd_db_inventory(
             )
             .await
         }
+        InventoryCommands::PhysicalDisks(args) => {
+            cmd_db_inventory_physical_disks(&conn, limit, args).await
+        }
         InventoryCommands::RotPages => {
             cmd_db_inventory_rot_pages(&conn, limit).await
         }
@@ -2736,6 +2752,63 @@ async fn cmd_db_inventory_cabooses(
     Ok(())
 }
 
+async fn cmd_db_inventory_physical_disks(
+    conn: &DataStoreConnection<'_>,
+    limit: NonZeroU32,
+    args: PhysicalDisksArgs,
+) -> Result<(), anyhow::Error> {
+    #[derive(Tabled)]
+    #[tabled(rename_all = "SCREAMING_SNAKE_CASE")]
+    struct DiskRow {
+        inv_collection_id: Uuid,
+        sled_id: Uuid,
+        slot: i64,
+        vendor: String,
+        model: String,
+        serial: String,
+        variant: String,
+    }
+
+    use db::schema::inv_physical_disk::dsl;
+    let mut query = dsl::inv_physical_disk.into_boxed();
+    query = query.limit(i64::from(u32::from(limit)));
+
+    if let Some(collection_id) = args.collection_id {
+        query = query.filter(
+            dsl::inv_collection_id.eq(collection_id.into_untyped_uuid()),
+        );
+    }
+
+    if let Some(sled_id) = args.sled_id {
+        query = query.filter(dsl::sled_id.eq(sled_id.into_untyped_uuid()));
+    }
+
+    let disks = query
+        .select(InvPhysicalDisk::as_select())
+        .load_async(&**conn)
+        .await
+        .context("loading physical disks")?;
+
+    let rows = disks.into_iter().map(|disk| DiskRow {
+        inv_collection_id: disk.inv_collection_id.into_untyped_uuid(),
+        sled_id: disk.sled_id.into_untyped_uuid(),
+        slot: disk.slot,
+        vendor: disk.vendor,
+        model: disk.model.clone(),
+        serial: disk.model.clone(),
+        variant: format!("{:?}", disk.variant),
+    });
+
+    let table = tabled::Table::new(rows)
+        .with(tabled::settings::Style::empty())
+        .with(tabled::settings::Padding::new(0, 1, 0, 0))
+        .to_string();
+
+    println!("{}", table);
+
+    Ok(())
+}
+
 async fn cmd_db_inventory_rot_pages(
     conn: &DataStoreConnection<'_>,
     limit: NonZeroU32,

dev-tools/omdb/src/bin/omdb/nexus.rs

@@ -1171,7 +1171,7 @@ async fn cmd_nexus_blueprints_diff(
         args.blueprint2_id.resolve_to_blueprint(client),
     )
     .await?;
-    let diff = b2.diff_since_blueprint(&b1).context("diffing blueprints")?;
+    let diff = b2.diff_since_blueprint(&b1);
     println!("{}", diff.display());
     Ok(())
 }

dev-tools/omdb/tests/successes.out

@@ -501,27 +501,38 @@ stdout:
 blueprint  ......<REDACTED_BLUEPRINT_ID>.......
 parent:    <none>
 
-  -----------------------------------------------------------------------------------------
-    zone type         zone ID                                disposition  underlay IP      
-  -----------------------------------------------------------------------------------------
-
-  sled ..........<REDACTED_UUID>...........: blueprint zones at generation 2               
-   (no zones)                                                                              
-
-  sled ..........<REDACTED_UUID>...........: blueprint zones at generation 2               
-    clickhouse        ..........<REDACTED_UUID>...........   in service   ::1              
-    cockroach_db      ..........<REDACTED_UUID>...........   in service   ::1              
-    crucible_pantry   ..........<REDACTED_UUID>...........   in service   ::1              
-    external_dns      ..........<REDACTED_UUID>...........   in service   ::1              
-    internal_dns      ..........<REDACTED_UUID>...........   in service   ::1              
-    nexus             ..........<REDACTED_UUID>...........   in service   ::ffff:127.0.0.1 
-
-METADATA:
-  created by:            nexus-test-utils        
-  created at:            <REDACTED     TIMESTAMP>
-  comment:               initial test blueprint  
-  internal DNS version:  1                       
-  external DNS version:  2                       
+!..........<REDACTED_UUID>...........
+WARNING: Zones exist without physical disks!
+    omicron zones at generation 2:
+    -----------------------------------------------
+    zone type   zone id   disposition   underlay IP
+    -----------------------------------------------
+
+
+
+
+!..........<REDACTED_UUID>...........
+WARNING: Zones exist without physical disks!
+    omicron zones at generation 2:
+    ---------------------------------------------------------------------------------------
+    zone type         zone id                                disposition   underlay IP     
+    ---------------------------------------------------------------------------------------
+    clickhouse        ..........<REDACTED_UUID>...........   in service    ::1             
+    cockroach_db      ..........<REDACTED_UUID>...........   in service    ::1             
+    crucible_pantry   ..........<REDACTED_UUID>...........   in service    ::1             
+    external_dns      ..........<REDACTED_UUID>...........   in service    ::1             
+    internal_dns      ..........<REDACTED_UUID>...........   in service    ::1             
+    nexus             ..........<REDACTED_UUID>...........   in service    ::ffff:127.0.0.1
+
+
+
+ METADATA:
+    created by:::::::::::   nexus-test-utils
+    created at:::::::::::   <REDACTED     TIMESTAMP>
+    comment::::::::::::::   initial test blueprint
+    internal DNS version:   1
+    external DNS version:   2
+
 
 ---------------------------------------------
 stderr:
@@ -534,27 +545,38 @@ stdout:
 blueprint  ......<REDACTED_BLUEPRINT_ID>.......
 parent:    <none>
 
-  -----------------------------------------------------------------------------------------
-    zone type         zone ID                                disposition  underlay IP      
-  -----------------------------------------------------------------------------------------
-
-  sled ..........<REDACTED_UUID>...........: blueprint zones at generation 2               
-   (no zones)                                                                              
-
-  sled ..........<REDACTED_UUID>...........: blueprint zones at generation 2               
-    clickhouse        ..........<REDACTED_UUID>...........   in service   ::1              
-    cockroach_db      ..........<REDACTED_UUID>...........   in service   ::1              
-    crucible_pantry   ..........<REDACTED_UUID>...........   in service   ::1              
-    external_dns      ..........<REDACTED_UUID>...........   in service   ::1              
-    internal_dns      ..........<REDACTED_UUID>...........   in service   ::1              
-    nexus             ..........<REDACTED_UUID>...........   in service   ::ffff:127.0.0.1 
-
-METADATA:
-  created by:            nexus-test-utils        
-  created at:            <REDACTED     TIMESTAMP>
-  comment:               initial test blueprint  
-  internal DNS version:  1                       
-  external DNS version:  2                       
+!..........<REDACTED_UUID>...........
+WARNING: Zones exist without physical disks!
+    omicron zones at generation 2:
+    -----------------------------------------------
+    zone type   zone id   disposition   underlay IP
+    -----------------------------------------------
+
+
+
+
+!..........<REDACTED_UUID>...........
+WARNING: Zones exist without physical disks!
+    omicron zones at generation 2:
+    ---------------------------------------------------------------------------------------
+    zone type         zone id                                disposition   underlay IP     
+    ---------------------------------------------------------------------------------------
+    clickhouse        ..........<REDACTED_UUID>...........   in service    ::1             
+    cockroach_db      ..........<REDACTED_UUID>...........   in service    ::1             
+    crucible_pantry   ..........<REDACTED_UUID>...........   in service    ::1             
+    external_dns      ..........<REDACTED_UUID>...........   in service    ::1             
+    internal_dns      ..........<REDACTED_UUID>...........   in service    ::1             
+    nexus             ..........<REDACTED_UUID>...........   in service    ::ffff:127.0.0.1
+
+
+
+ METADATA:
+    created by:::::::::::   nexus-test-utils
+    created at:::::::::::   <REDACTED     TIMESTAMP>
+    comment::::::::::::::   initial test blueprint
+    internal DNS version:   1
+    external DNS version:   2
+
 
 ---------------------------------------------
 stderr:
@@ -567,23 +589,28 @@ stdout:
 from: blueprint ......<REDACTED_BLUEPRINT_ID>.......
 to:   blueprint ......<REDACTED_BLUEPRINT_ID>.......
 
-  ---------------------------------------------------------------------------------------------------
-     zone type         zone ID                                disposition   underlay IP       status 
-  ---------------------------------------------------------------------------------------------------
-
-  UNCHANGED SLEDS:                                                                                   
-
-   sled ..........<REDACTED_UUID>...........: blueprint zones at generation 2                        
-     clickhouse        ..........<REDACTED_UUID>...........   in service    ::1                      
-     cockroach_db      ..........<REDACTED_UUID>...........   in service    ::1                      
-     crucible_pantry   ..........<REDACTED_UUID>...........   in service    ::1                      
-     external_dns      ..........<REDACTED_UUID>...........   in service    ::1                      
-     internal_dns      ..........<REDACTED_UUID>...........   in service    ::1                      
-     nexus             ..........<REDACTED_UUID>...........   in service    ::ffff:127.0.0.1         
-
-  METADATA:
-    internal DNS version:  1 (unchanged)
-    external DNS version:  2 (unchanged)
+ UNCHANGED SLEDS:
+
+  sled ..........<REDACTED_UUID>...........:
+
+  sled ..........<REDACTED_UUID>...........:
+
+    omicron zones at generation 2:
+    ---------------------------------------------------------------------------------------
+    zone type         zone id                                disposition   underlay IP     
+    ---------------------------------------------------------------------------------------
+    cockroach_db      ..........<REDACTED_UUID>...........   in service    ::1             
+    nexus             ..........<REDACTED_UUID>...........   in service    ::ffff:127.0.0.1
+    clickhouse        ..........<REDACTED_UUID>...........   in service    ::1             
+    external_dns      ..........<REDACTED_UUID>...........   in service    ::1             
+    crucible_pantry   ..........<REDACTED_UUID>...........   in service    ::1             
+    internal_dns      ..........<REDACTED_UUID>...........   in service    ::1             
+
+
+ METADATA:
+    internal DNS version:   1 (unchanged)
+    external DNS version:   2 (unchanged)
+
 
 ---------------------------------------------
 stderr:

dev-tools/reconfigurator-cli/src/main.rs

@@ -780,9 +780,7 @@ fn cmd_blueprint_diff(
     let blueprint1 = sim.blueprint_lookup(blueprint1_id)?;
     let blueprint2 = sim.blueprint_lookup(blueprint2_id)?;
 
-    let sled_diff = blueprint2
-        .diff_since_blueprint(&blueprint1)
-        .context("failed to diff blueprints")?;
+    let sled_diff = blueprint2.diff_since_blueprint(&blueprint1);
     swriteln!(rv, "{}", sled_diff.display());
 
     // Diff'ing DNS is a little trickier.  First, compute what DNS should be for
@@ -897,9 +895,7 @@ fn cmd_blueprint_diff_inventory(
         anyhow!("no such inventory collection: {}", collection_id)
     })?;
     let blueprint = sim.blueprint_lookup(blueprint_id)?;
-    let diff = blueprint
-        .diff_since_collection(&collection)
-        .context("failed to diff blueprint from inventory collection")?;
+    let diff = blueprint.diff_since_collection(&collection);
     Ok(Some(diff.display().to_string()))
 }
 

nexus/db-queries/src/db/datastore/deployment.rs

@@ -1658,7 +1658,7 @@ mod tests {
         let blueprint2 = builder.build();
         let authz_blueprint2 = authz_blueprint_from_id(blueprint2.id);
 
-        let diff = blueprint2.diff_since_blueprint(&blueprint1).unwrap();
+        let diff = blueprint2.diff_since_blueprint(&blueprint1);
         println!("b1 -> b2: {}", diff.display());
         println!("b1 disks: {:?}", blueprint1.blueprint_disks);
         println!("b2 disks: {:?}", blueprint2.blueprint_disks);
@@ -1699,9 +1699,7 @@ mod tests {
             .blueprint_read(&opctx, &authz_blueprint2)
             .await
             .expect("failed to read collection back");
-        let diff = blueprint_read
-            .diff_since_blueprint(&blueprint2)
-            .expect("failed to diff blueprints");
+        let diff = blueprint_read.diff_since_blueprint(&blueprint2);
         println!("diff: {}", diff.display());
         assert_eq!(blueprint2, blueprint_read);
         assert_eq!(blueprint2.internal_dns_version, new_internal_dns_version);