Trying to collect timesync status in inventory

Author: smklein

Cargo.lock

@@ -64,6 +64,7 @@ nexus-mgs-updates.workspace = true
 nexus-networking.workspace = true
 nexus-saga-recovery.workspace = true
 nexus-test-interface.workspace = true
+ntp-admin-client.workspace = true
 num-integer.workspace = true
 omicron-cockroach-metrics.workspace = true
 openssl.workspace = true

nexus/Cargo.toml

@@ -34,7 +34,7 @@ use nexus_db_schema::schema::{
     inv_last_reconciliation_disk_result,
     inv_last_reconciliation_orphaned_dataset,
     inv_last_reconciliation_zone_result, inv_mupdate_override_non_boot,
-    inv_nvme_disk_firmware, inv_omicron_sled_config,
+    inv_ntp_timesync, inv_nvme_disk_firmware, inv_omicron_sled_config,
     inv_omicron_sled_config_dataset, inv_omicron_sled_config_disk,
     inv_omicron_sled_config_zone, inv_omicron_sled_config_zone_nic,
     inv_physical_disk, inv_root_of_trust, inv_root_of_trust_page,
@@ -61,7 +61,7 @@ use nexus_sled_agent_shared::inventory::{
 };
 use nexus_types::inventory::{
     BaseboardId, Caboose, CockroachStatus, Collection, NvmeFirmware,
-    PowerState, RotPage, RotSlot,
+    PowerState, RotPage, RotSlot, TimeSync,
 };
 use omicron_common::api::external;
 use omicron_common::api::internal::shared::NetworkInterface;
@@ -2788,6 +2788,38 @@ impl TryFrom<InvCockroachStatus> for CockroachStatus {
     }
 }
 
+#[derive(Queryable, Clone, Debug, Selectable, Insertable)]
+#[diesel(table_name = inv_ntp_timesync)]
+pub struct InvNtpTimesync {
+    pub inv_collection_id: DbTypedUuid<CollectionKind>,
+    pub ip: ipv6::Ipv6Addr,
+    pub port: SqlU16,
+    pub synced: bool,
+}
+
+impl InvNtpTimesync {
+    pub fn new(
+        inv_collection_id: CollectionUuid,
+        timesync: &TimeSync,
+    ) -> Result<Self, anyhow::Error> {
+        Ok(Self {
+            inv_collection_id: inv_collection_id.into(),
+            ip: timesync.addr.ip().into(),
+            port: timesync.addr.port().into(),
+            synced: timesync.synced,
+        })
+    }
+}
+
+impl From<InvNtpTimesync> for nexus_types::inventory::TimeSync {
+    fn from(value: InvNtpTimesync) -> Self {
+        Self {
+            addr: SocketAddrV6::new(*value.ip, *value.port, 0, 0),
+            synced: value.synced,
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use nexus_types::inventory::NvmeFirmware;

nexus/db-model/src/inventory.rs

@@ -42,6 +42,7 @@ use nexus_db_model::InvLastReconciliationDatasetResult;
 use nexus_db_model::InvLastReconciliationDiskResult;
 use nexus_db_model::InvLastReconciliationOrphanedDataset;
 use nexus_db_model::InvLastReconciliationZoneResult;
+use nexus_db_model::InvNtpTimesync;
 use nexus_db_model::InvNvmeDiskFirmware;
 use nexus_db_model::InvOmicronSledConfig;
 use nexus_db_model::InvOmicronSledConfigDataset;
@@ -93,6 +94,7 @@ use nexus_types::inventory::CockroachStatus;
 use nexus_types::inventory::Collection;
 use nexus_types::inventory::PhysicalDiskFirmware;
 use nexus_types::inventory::SledAgent;
+use nexus_types::inventory::TimeSync;
 use omicron_cockroach_metrics::NodeId as CockroachNodeId;
 use omicron_common::api::external::Error;
 use omicron_common::api::external::InternalContext;
@@ -385,6 +387,13 @@ impl DataStore {
             .collect::<Result<Vec<_>, _>>()
             .map_err(|e| Error::internal_error(&e.to_string()))?;
 
+        let inv_ntp_timesync_records: Vec<InvNtpTimesync> = collection
+            .ntp_timesync
+            .iter()
+            .map(|timesync| InvNtpTimesync::new(collection_id, timesync))
+            .collect::<Result<Vec<_>, _>>()
+            .map_err(|e| Error::internal_error(&e.to_string()))?;
+
         // This implementation inserts all records associated with the
         // collection in one transaction.  This is primarily for simplicity.  It
         // means we don't have to worry about other readers seeing a
@@ -1424,6 +1433,14 @@ impl DataStore {
                     .await?;
             }
 
+            // Insert the NTP info we've observed
+            if !inv_ntp_timesync_records.is_empty() {
+                use nexus_db_schema::schema::inv_ntp_timesync::dsl;
+                diesel::insert_into(dsl::inv_ntp_timesync)
+                    .values(inv_ntp_timesync_records)
+                    .execute_async(&conn)
+                    .await?;
+            }
 
             // Finally, insert the list of errors.
             {
@@ -1714,6 +1731,7 @@ impl DataStore {
             nerrors: usize,
             nclickhouse_keeper_membership: usize,
             ncockroach_status: usize,
+            nntp_timesync: usize,
         }
 
         let NumRowsDeleted {
@@ -1744,6 +1762,7 @@ impl DataStore {
             nerrors,
             nclickhouse_keeper_membership,
             ncockroach_status,
+            nntp_timesync,
         } =
             self.transaction_retry_wrapper("inventory_delete_collection")
                 .transaction(&conn, |conn| async move {
@@ -2000,6 +2019,17 @@ impl DataStore {
                         .execute_async(&conn)
                         .await?
                     };
+                    // Remove rows for NTP timesync
+                    let nntp_timesync = {
+                        use nexus_db_schema::schema::inv_ntp_timesync::dsl;
+                        diesel::delete(
+                            dsl::inv_ntp_timesync.filter(
+                                dsl::inv_collection_id.eq(db_collection_id),
+                            ),
+                        )
+                        .execute_async(&conn)
+                        .await?
+                    };
 
                     Ok(NumRowsDeleted {
                         ncollections,
@@ -2029,6 +2059,7 @@ impl DataStore {
                         nerrors,
                         nclickhouse_keeper_membership,
                         ncockroach_status,
+                        nntp_timesync,
                     })
                 })
                 .await
@@ -2069,6 +2100,7 @@ impl DataStore {
             "nerrors" => nerrors,
             "nclickhouse_keeper_membership" => nclickhouse_keeper_membership,
             "ncockroach_status" => ncockroach_status,
+            "nntp_timesync" => nntp_timesync,
         );
 
         Ok(())
@@ -3435,6 +3467,25 @@ impl DataStore {
                 .collect::<Result<BTreeMap<_, _>, Error>>()?
         };
 
+        // Load TimeSync statuses
+        let ntp_timesync: IdOrdMap<TimeSync> = {
+            use nexus_db_schema::schema::inv_ntp_timesync::dsl;
+
+            let records: Vec<InvNtpTimesync> = dsl::inv_ntp_timesync
+                .filter(dsl::inv_collection_id.eq(db_id))
+                .select(InvNtpTimesync::as_select())
+                .load_async(&*conn)
+                .await
+                .map_err(|e| {
+                    public_error_from_diesel(e, ErrorHandler::Server)
+                })?;
+
+            records
+                .into_iter()
+                .map(|record| TimeSync::from(record))
+                .collect::<IdOrdMap<_>>()
+        };
+
         // Finally, build up the sled-agent map using the sled agent and
         // omicron zone rows. A for loop is easier to understand than into_iter
         // + filter_map + return Result + collect.
@@ -3681,6 +3732,7 @@ impl DataStore {
             sled_agents,
             clickhouse_keeper_cluster_membership,
             cockroach_status,
+            ntp_timesync,
         })
     }
 }

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

@@ -1899,6 +1899,15 @@ table! {
     }
 }
 
+table! {
+    inv_ntp_timesync (inv_collection_id, ip, port) {
+        inv_collection_id -> Uuid,
+        ip -> Inet,
+        port -> Int4,
+        synced -> Bool,
+    }
+}
+
 /* blueprints */
 
 table! {

nexus/db-schema/src/schema.rs

@@ -23,6 +23,7 @@ id-map.workspace = true
 iddqd.workspace = true
 nexus-sled-agent-shared.workspace = true
 nexus-types.workspace = true
+ntp-admin-client.workspace = true
 omicron-common.workspace = true
 omicron-uuid-kinds.workspace = true
 reqwest.workspace = true
@@ -31,6 +32,7 @@ sled-agent-client.workspace = true
 sled-agent-types.workspace = true
 sled-agent-zone-images-examples.workspace = true
 slog.workspace = true
+slog-error-chain.workspace = true
 strum.workspace = true
 swrite.workspace = true
 thiserror.workspace = true

nexus/inventory/Cargo.toml

@@ -31,6 +31,7 @@ use nexus_types::inventory::RotPageWhich;
 use nexus_types::inventory::RotState;
 use nexus_types::inventory::ServiceProcessor;
 use nexus_types::inventory::SledAgent;
+use nexus_types::inventory::TimeSync;
 use nexus_types::inventory::Zpool;
 use omicron_cockroach_metrics::CockroachMetric;
 use omicron_cockroach_metrics::NodeId;
@@ -39,6 +40,7 @@ use omicron_uuid_kinds::CollectionKind;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::hash::Hash;
+use std::net::SocketAddrV6;
 use std::sync::Arc;
 use thiserror::Error;
 use typed_rng::TypedUuidRng;
@@ -120,6 +122,7 @@ pub struct CollectionBuilder {
     clickhouse_keeper_cluster_membership:
         BTreeSet<ClickhouseKeeperClusterMembership>,
     cockroach_status: BTreeMap<NodeId, CockroachStatus>,
+    ntp_timesync: IdOrdMap<TimeSync>,
     // CollectionBuilderRng is taken by value, rather than passed in as a
     // mutable ref, to encourage a tree-like structure where each RNG is
     // generally independent.
@@ -150,6 +153,7 @@ impl CollectionBuilder {
             sleds: IdOrdMap::new(),
             clickhouse_keeper_cluster_membership: BTreeSet::new(),
             cockroach_status: BTreeMap::new(),
+            ntp_timesync: IdOrdMap::new(),
             rng: CollectionBuilderRng::from_entropy(),
         }
     }
@@ -173,6 +177,7 @@ impl CollectionBuilder {
             clickhouse_keeper_cluster_membership: self
                 .clickhouse_keeper_cluster_membership,
             cockroach_status: self.cockroach_status,
+            ntp_timesync: self.ntp_timesync,
         }
     }
 
@@ -550,6 +555,20 @@ impl CollectionBuilder {
         self.clickhouse_keeper_cluster_membership.insert(membership);
     }
 
+    /// Record information about timesync
+    pub fn found_ntp_timesync(
+        &mut self,
+        addr: SocketAddrV6,
+        timesync: ntp_admin_client::types::TimeSync,
+    ) -> Result<(), anyhow::Error> {
+        self.ntp_timesync
+            .insert_unique(TimeSync { addr, synced: timesync.sync })
+            .map_err(|err| err.into_owned())
+            .with_context(|| {
+                format!("NTP service {addr} reported time multiple times")
+            })
+    }
+
     /// Record metrics from a CockroachDB node
     pub fn found_cockroach_metrics(
         &mut self,
@@ -621,6 +640,8 @@ mod test {
         assert!(collection.cabooses_found.is_empty());
         assert!(collection.rot_pages_found.is_empty());
         assert!(collection.clickhouse_keeper_cluster_membership.is_empty());
+        assert!(collection.cockroach_status.is_empty());
+        assert!(collection.ntp_timesync.is_empty());
     }
 
     // Simple test of a single, fairly typical collection that contains just