review feedback

nexus/db-model/src/inventory.rs

@@ -208,12 +208,12 @@ pub struct HwBaseboardId {
     pub serial_number: String,
 }
 
-impl<'a> From<&'a BaseboardId> for HwBaseboardId {
-    fn from(c: &'a BaseboardId) -> Self {
+impl From<BaseboardId> for HwBaseboardId {
+    fn from(c: BaseboardId) -> Self {
         HwBaseboardId {
             id: Uuid::new_v4(),
-            part_number: c.part_number.clone(),
-            serial_number: c.serial_number.clone(),
+            part_number: c.part_number,
+            serial_number: c.serial_number,
         }
     }
 }
@@ -248,14 +248,14 @@ pub struct SwCaboose {
     pub version: String,
 }
 
-impl<'a> From<&'a Caboose> for SwCaboose {
-    fn from(c: &'a Caboose) -> Self {
+impl From<Caboose> for SwCaboose {
+    fn from(c: Caboose) -> Self {
         SwCaboose {
             id: Uuid::new_v4(),
-            board: c.board.clone(),
-            git_commit: c.git_commit.clone(),
-            name: c.name.clone(),
-            version: c.version.clone(),
+            board: c.board,
+            git_commit: c.git_commit,
+            name: c.name,
+            version: c.version,
         }
     }
 }

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

@@ -70,12 +70,12 @@ impl DataStore {
         let baseboards = collection
             .baseboards
             .iter()
-            .map(|b| HwBaseboardId::from(b.as_ref()))
+            .map(|b| HwBaseboardId::from((**b).clone()))
             .collect::<Vec<_>>();
         let cabooses = collection
             .cabooses
             .iter()
-            .map(|s| SwCaboose::from(s.as_ref()))
+            .map(|s| SwCaboose::from((**s).clone()))
             .collect::<Vec<_>>();
         let error_values = collection
             .errors
@@ -85,7 +85,7 @@ impl DataStore {
                 let index = u16::try_from(i).map_err(|e| {
                     Error::internal_error(&format!(
                         "failed to convert error index to u16 (too \
-                                many errors in inventory collection?): {}",
+                        many errors in inventory collection?): {}",
                         e
                     ))
                 })?;
@@ -800,56 +800,6 @@ impl DataStore {
     }
 }
 
-/// A SQL common table expression (CTE) used to insert into `inv_caboose`
-///
-/// Concretely, we have these three tables:
-///
-/// - `hw_baseboard` with an "id" primary key and lookup columns "part_number"
-///    and "serial_number"
-/// - `sw_caboose` with an "id" primary key and lookup columns "board",
-///    "git_commit", "name", and "version"
-/// - `inv_caboose` with foreign keys "hw_baseboard_id", "sw_caboose_id", and
-///    various other columns
-///
-/// We want to INSERT INTO `inv_caboose` a row with:
-///
-/// - hw_baseboard_id (foreign key) the result of looking up an hw_baseboard row
-///   by part number and serial number provided by the caller
-///
-/// - sw_caboose_id (foreign key) the result of looking up a sw_caboose row by
-///   board, git_commit, name, and version provided by the caller
-///
-/// - the other columns being literals provided by the caller
-///
-/// To achieve this, we're going to generate something like:
-///
-/// WITH
-///     my_new_row
-/// AS (
-///     SELECT
-///         hw_baseboard.id, /* `hw_baseboard` foreign key */
-///         sw_caboose.id,   /* `sw_caboose` foreign key */
-///         ...              /* caller-provided literal values for the rest */
-///                          /* of the new inv_caboose row */
-///     FROM
-///         hw_baseboard,
-///         sw_caboose
-///     WHERE
-///         hw_baseboard.part_number = ...   /* caller-provided part number */
-///         hw_baseboard.serial_number = ... /* caller-provided serial number */
-///         sw_caboose.board = ...           /* caller-provided board */
-///         sw_caboose.git_commit = ...      /* caller-provided git_commit */
-///         sw_caboose.name = ...            /* caller-provided name */
-///         sw_caboose.version = ...         /* caller-provided version */
-/// ) INSERT INTO
-///     inv_caboose (... /* inv_caboose columns */)
-///     SELECT * from my_new_row;
-///
-/// The whole point is to avoid back-and-forth between the client and the
-/// database.  Those back-and-forth interactions can significantly increase
-/// latency and the probability of transaction conflicts.  See RFD 192 for
-/// details.
-
 /// Extra interfaces that are not intended (and potentially unsafe) for use in
 /// Nexus, but useful for testing and `omdb`
 pub trait DataStoreInventoryTest: Send + Sync {

nexus/src/app/background/init.rs

@@ -98,7 +98,7 @@ impl BackgroundTasks {
 
         // Background task: inventory collector
         let task_inventory_collection = {
-            let watcher = inventory_collection::InventoryCollector::new(
+            let collector = inventory_collection::InventoryCollector::new(
                 datastore,
                 resolver,
                 &nexus_id.to_string(),
@@ -112,7 +112,7 @@ impl BackgroundTasks {
                     whole system",
                 ),
                 config.inventory.period_secs,
-                Box::new(watcher),
+                Box::new(collector),
                 opctx.child(BTreeMap::new()),
                 vec![],
             );

nexus/src/app/background/inventory_collection.rs

@@ -164,8 +164,11 @@ mod test {
             datastore.clone(),
         );
 
-        // Nexus starts our very background task, so we should find a collection
-        // in the database before too long.
+        // Nexus starts the very background task that we're also testing
+        // manually here.  As a result, we should find a collection in the
+        // database before too long.  Wait for it so that after it appears, we
+        // can assume the rest of the collections came from the instance that
+        // we're testing.
         let mut last_collections =
             poll::wait_for_condition::<_, anyhow::Error, _, _>(
                 || async {