Add support for clickhouse keeper inventory collection (#6810)

I made small change to the types stored in inventory to not collect
OmicronZoneUuids for each keeper. This allows collection based on DNS
names alone. The zone ids were never actually used for anything anyway.

Note that this won't collect anything right now because we won't have
any DNS names for clickhouse-admin-keepers.

Author: andrewjstone

Cargo.lock

@@ -907,7 +907,17 @@ impl KeeperConf {
 /// dropshot servers running in `ClickhouseKeeper` zones. state. We include the
 /// leader committed log index known to the current keeper node (whether or not
 /// it is the leader) to determine which configuration is newest.
-#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
+#[derive(
+    Clone,
+    Debug,
+    PartialEq,
+    Eq,
+    PartialOrd,
+    Ord,
+    Deserialize,
+    Serialize,
+    JsonSchema,
+)]
 #[serde(rename_all = "snake_case")]
 pub struct ClickhouseKeeperClusterMembership {
     /// Keeper ID of the keeper being queried

clickhouse-admin/types/src/lib.rs

@@ -23,5 +23,7 @@ progenitor::generate_api!(
         TypedUuidForOmicronZoneKind = omicron_uuid_kinds::OmicronZoneUuid,
         KeeperConfigurableSettings = clickhouse_admin_api::KeeperConfigurableSettings,
         ServerConfigurableSettings = clickhouse_admin_api::ServerConfigurableSettings,
+        ClickhouseKeeperClusterMembership = clickhouse_admin_types::ClickhouseKeeperClusterMembership,
+        KeeperId = clickhouse_admin_types::KeeperId
     }
 );

clients/clickhouse-admin-client/src/lib.rs

@@ -22,6 +22,7 @@ camino.workspace = true
 camino-tempfile.workspace = true
 clap.workspace = true
 chrono.workspace = true
+clickhouse-admin-client.workspace = true
 cockroach-admin-client.workspace = true
 crucible-agent-client.workspace = true
 crucible-pantry-client.workspace = true

nexus/Cargo.toml

@@ -6,11 +6,12 @@
 
 use crate::omicron_zone_config::{self, OmicronZoneNic};
 use crate::schema::{
-    hw_baseboard_id, inv_caboose, inv_collection, inv_collection_error,
-    inv_dataset, inv_nvme_disk_firmware, inv_omicron_zone,
-    inv_omicron_zone_nic, inv_physical_disk, inv_root_of_trust,
-    inv_root_of_trust_page, inv_service_processor, inv_sled_agent,
-    inv_sled_omicron_zones, inv_zpool, sw_caboose, sw_root_of_trust_page,
+    hw_baseboard_id, inv_caboose, inv_clickhouse_keeper_membership,
+    inv_collection, inv_collection_error, inv_dataset, inv_nvme_disk_firmware,
+    inv_omicron_zone, inv_omicron_zone_nic, inv_physical_disk,
+    inv_root_of_trust, inv_root_of_trust_page, inv_service_processor,
+    inv_sled_agent, inv_sled_omicron_zones, inv_zpool, sw_caboose,
+    sw_root_of_trust_page,
 };
 use crate::typed_uuid::DbTypedUuid;
 use crate::PhysicalDiskKind;
@@ -21,6 +22,7 @@ use crate::{
 use anyhow::{anyhow, bail, Context, Result};
 use chrono::DateTime;
 use chrono::Utc;
+use clickhouse_admin_types::{ClickhouseKeeperClusterMembership, KeeperId};
 use diesel::backend::Backend;
 use diesel::deserialize::{self, FromSql};
 use diesel::expression::AsExpression;
@@ -46,6 +48,7 @@ use omicron_uuid_kinds::ZpoolKind;
 use omicron_uuid_kinds::ZpoolUuid;
 use omicron_uuid_kinds::{CollectionKind, OmicronZoneKind};
 use omicron_uuid_kinds::{CollectionUuid, OmicronZoneUuid};
+use std::collections::BTreeSet;
 use std::net::{IpAddr, SocketAddrV6};
 use thiserror::Error;
 use uuid::Uuid;
@@ -1726,6 +1729,68 @@ impl InvOmicronZoneNic {
     }
 }
 
+#[derive(Queryable, Clone, Debug, Selectable, Insertable)]
+#[diesel(table_name = inv_clickhouse_keeper_membership)]
+pub struct InvClickhouseKeeperMembership {
+    pub inv_collection_id: DbTypedUuid<CollectionKind>,
+    pub queried_keeper_id: i64,
+    pub leader_committed_log_index: i64,
+    pub raft_config: Vec<i64>,
+}
+
+impl TryFrom<InvClickhouseKeeperMembership>
+    for ClickhouseKeeperClusterMembership
+{
+    type Error = anyhow::Error;
+
+    fn try_from(value: InvClickhouseKeeperMembership) -> anyhow::Result<Self> {
+        let err_msg = "clickhouse keeper ID is negative";
+        let mut raft_config = BTreeSet::new();
+        // We are not worried about duplicates here, as each
+        // `clickhouse-admin-keeper` reports about its local, unique keeper.
+        // This uniqueness is guaranteed by the blueprint generation mechanism.
+        for id in value.raft_config {
+            raft_config.insert(KeeperId(id.try_into().context(err_msg)?));
+        }
+        Ok(ClickhouseKeeperClusterMembership {
+            queried_keeper: KeeperId(
+                value.queried_keeper_id.try_into().context(err_msg)?,
+            ),
+            leader_committed_log_index: value
+                .leader_committed_log_index
+                .try_into()
+                .context("log index is negative")?,
+            raft_config,
+        })
+    }
+}
+
+impl InvClickhouseKeeperMembership {
+    pub fn new(
+        inv_collection_id: CollectionUuid,
+        membership: ClickhouseKeeperClusterMembership,
+    ) -> anyhow::Result<InvClickhouseKeeperMembership> {
+        let err_msg = "clickhouse keeper ID > 2^63";
+        let mut raft_config = Vec::with_capacity(membership.raft_config.len());
+        for id in membership.raft_config {
+            raft_config.push(id.0.try_into().context(err_msg)?);
+        }
+        Ok(InvClickhouseKeeperMembership {
+            inv_collection_id: inv_collection_id.into(),
+            queried_keeper_id: membership
+                .queried_keeper
+                .0
+                .try_into()
+                .context(err_msg)?,
+            leader_committed_log_index: membership
+                .leader_committed_log_index
+                .try_into()
+                .context("log index > 2^63")?,
+            raft_config,
+        })
+    }
+}
+
 #[cfg(test)]
 mod test {
     use nexus_types::inventory::NvmeFirmware;

nexus/db-model/src/inventory.rs

@@ -1568,6 +1568,15 @@ table! {
     }
 }
 
+table! {
+    inv_clickhouse_keeper_membership (inv_collection_id, queried_keeper_id) {
+        inv_collection_id -> Uuid,
+        queried_keeper_id -> Int8,
+        leader_committed_log_index -> Int8,
+        raft_config -> Array<Int8>,
+    }
+}
+
 /* blueprints */
 
 table! {

nexus/db-model/src/schema.rs

@@ -17,7 +17,7 @@ use std::collections::BTreeMap;
 ///
 /// This must be updated when you change the database schema.  Refer to
 /// schema/crdb/README.adoc in the root of this repository for details.
-pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(108, 0, 0);
+pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(109, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::new(|| {
         // |  leaving the first copy as an example for the next person.
         // v
         // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
+        KnownVersion::new(109, "inv-clickhouse-keeper-membership"),
         KnownVersion::new(108, "internet-gateway"),
         KnownVersion::new(107, "add-instance-boot-disk"),
         KnownVersion::new(106, "dataset-kinds-update"),

nexus/db-model/src/schema_versions.rs

@@ -16,6 +16,7 @@ use anyhow::Context;
 use async_bb8_diesel::AsyncConnection;
 use async_bb8_diesel::AsyncRunQueryDsl;
 use async_bb8_diesel::AsyncSimpleConnection;
+use clickhouse_admin_types::ClickhouseKeeperClusterMembership;
 use diesel::expression::SelectableHelper;
 use diesel::sql_types::Nullable;
 use diesel::BoolExpressionMethods;
@@ -36,6 +37,7 @@ use nexus_db_model::HwPowerStateEnum;
 use nexus_db_model::HwRotSlot;
 use nexus_db_model::HwRotSlotEnum;
 use nexus_db_model::InvCaboose;
+use nexus_db_model::InvClickhouseKeeperMembership;
 use nexus_db_model::InvCollection;
 use nexus_db_model::InvCollectionError;
 use nexus_db_model::InvDataset;
@@ -244,6 +246,17 @@ impl DataStore {
             })
             .collect::<Result<Vec<InvOmicronZoneNic>, _>>()?;
 
+        let mut inv_clickhouse_keeper_memberships = Vec::new();
+        for membership in &collection.clickhouse_keeper_cluster_membership {
+            inv_clickhouse_keeper_memberships.push(
+                InvClickhouseKeeperMembership::new(
+                    collection_id,
+                    membership.clone(),
+                )
+                .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
@@ -953,6 +966,15 @@ impl DataStore {
                         .await?;
             }
 
+            // Insert the clickhouse keeper memberships we've received
+            {
+                use db::schema::inv_clickhouse_keeper_membership::dsl;
+                diesel::insert_into(dsl::inv_clickhouse_keeper_membership)
+                    .values(inv_clickhouse_keeper_memberships)
+                    .execute_async(&conn)
+                    .await?;
+            }
+
             // Finally, insert the list of errors.
             {
                 use db::schema::inv_collection_error::dsl as errors_dsl;
@@ -1221,6 +1243,7 @@ impl DataStore {
             nnics,
             nzpools,
             nerrors,
+            nclickhouse_keeper_membership,
         ) = conn
             .transaction_async(|conn| async move {
                 // Remove the record describing the collection itself.
@@ -1374,6 +1397,18 @@ impl DataStore {
                         .await?
                     };
 
+                // Remove rows for clickhouse keeper membership
+                let nclickhouse_keeper_membership = {
+                    use db::schema::inv_clickhouse_keeper_membership::dsl;
+                    diesel::delete(
+                        dsl::inv_clickhouse_keeper_membership.filter(
+                            dsl::inv_collection_id.eq(db_collection_id),
+                        ),
+                    )
+                    .execute_async(&conn)
+                    .await?
+                };
+
                 Ok((
                     ncollections,
                     nsps,
@@ -1389,6 +1424,7 @@ impl DataStore {
                     nnics,
                     nzpools,
                     nerrors,
+                    nclickhouse_keeper_membership,
                 ))
             })
             .await
@@ -1415,6 +1451,7 @@ impl DataStore {
             "nnics" => nnics,
             "nzpools" => nzpools,
             "nerrors" => nerrors,
+            "nclickhouse_keeper_membership" => nclickhouse_keeper_membership
         );
 
         Ok(())
@@ -2264,6 +2301,37 @@ impl DataStore {
             map.zones.zones.push(zone);
         }
 
+        // Now load the clickhouse keeper cluster memberships
+        let clickhouse_keeper_cluster_membership = {
+            use db::schema::inv_clickhouse_keeper_membership::dsl;
+            let mut memberships = BTreeSet::new();
+            let mut paginator = Paginator::new(batch_size);
+            while let Some(p) = paginator.next() {
+                let batch = paginated(
+                    dsl::inv_clickhouse_keeper_membership,
+                    dsl::queried_keeper_id,
+                    &p.current_pagparams(),
+                )
+                .filter(dsl::inv_collection_id.eq(db_id))
+                .select(InvClickhouseKeeperMembership::as_select())
+                .load_async(&*conn)
+                .await
+                .map_err(|e| {
+                    public_error_from_diesel(e, ErrorHandler::Server)
+                })?;
+                paginator = p.found_batch(&batch, &|row| row.queried_keeper_id);
+                for membership in batch.into_iter() {
+                    memberships.insert(
+                        ClickhouseKeeperClusterMembership::try_from(membership)
+                            .map_err(|e| {
+                                Error::internal_error(&format!("{e:#}",))
+                            })?,
+                    );
+                }
+            }
+            memberships
+        };
+
         bail_unless!(
             omicron_zone_nics.is_empty(),
             "found extra Omicron zone NICs: {:?}",
@@ -2285,9 +2353,7 @@ impl DataStore {
             rot_pages_found,
             sled_agents,
             omicron_zones,
-            // Currently unused
-            // See: https://github.com/oxidecomputer/omicron/issues/6578
-            clickhouse_keeper_cluster_membership: BTreeMap::new(),
+            clickhouse_keeper_cluster_membership,
         })
     }
 }

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

@@ -11,6 +11,7 @@ workspace = true
 anyhow.workspace = true
 base64.workspace = true
 chrono.workspace = true
+clickhouse-admin-client.workspace = true
 clickhouse-admin-types.workspace = true
 futures.workspace = true
 gateway-client.workspace = true

nexus/inventory/Cargo.toml

@@ -32,7 +32,6 @@ use nexus_types::inventory::ServiceProcessor;
 use nexus_types::inventory::SledAgent;
 use nexus_types::inventory::Zpool;
 use omicron_uuid_kinds::CollectionKind;
-use omicron_uuid_kinds::OmicronZoneUuid;
 use omicron_uuid_kinds::SledUuid;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
@@ -95,7 +94,7 @@ pub struct CollectionBuilder {
     sleds: BTreeMap<SledUuid, SledAgent>,
     omicron_zones: BTreeMap<SledUuid, OmicronZonesFound>,
     clickhouse_keeper_cluster_membership:
-        BTreeMap<OmicronZoneUuid, ClickhouseKeeperClusterMembership>,
+        BTreeSet<ClickhouseKeeperClusterMembership>,
 
     // We just generate one UUID for each collection.
     id_rng: TypedUuidRng<CollectionKind>,
@@ -124,7 +123,7 @@ impl CollectionBuilder {
             rot_pages_found: BTreeMap::new(),
             sleds: BTreeMap::new(),
             omicron_zones: BTreeMap::new(),
-            clickhouse_keeper_cluster_membership: BTreeMap::new(),
+            clickhouse_keeper_cluster_membership: BTreeSet::new(),
             id_rng: TypedUuidRng::from_entropy(),
         }
     }
@@ -571,10 +570,9 @@ impl CollectionBuilder {
     /// clickhouse-admin service running in the keeper zones.
     pub fn found_clickhouse_keeper_cluster_membership(
         &mut self,
-        zone_id: OmicronZoneUuid,
         membership: ClickhouseKeeperClusterMembership,
     ) {
-        self.clickhouse_keeper_cluster_membership.insert(zone_id, membership);
+        self.clickhouse_keeper_cluster_membership.insert(membership);
     }
 }