Skip to content

Commit

Permalink
Add clickhouse-cluster-config to omdb blueprint output
Browse files Browse the repository at this point in the history
Print clickhouse cluster config related tables for `blueprint show` and
`blueprint diff` omdb commands.

A bunch of the complexity and duplication here arises from the fact
that  we are diffing not only between blueprints that have identical
structures, but between a blueprints and collections that have
drastically different contents. This is useful, but we probably should
consider a separating the two types of diffs and reworking all the
blueprint diff logic to use some sort of semantic diff between types
such as https://github.com/distil/diffus as recommended by @sunshowers.

This still needs some manual testing, and potentially some expectorate
based testing depending upon what those tests currently do.
  • Loading branch information
andrewjstone committed Nov 7, 2024
1 parent 8bd3149 commit d147f4c
Show file tree
Hide file tree
Showing 3 changed files with 594 additions and 22 deletions.
103 changes: 91 additions & 12 deletions nexus/types/src/deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::internal_api::params::DnsConfigParams;
use crate::inventory::Collection;
pub use crate::inventory::SourceNatConfig;
pub use crate::inventory::ZpoolName;
use blueprint_diff::ClickhouseClusterConfigDiffTables;
use derive_more::From;
use nexus_sled_agent_shared::inventory::OmicronZoneConfig;
use nexus_sled_agent_shared::inventory::OmicronZoneType;
Expand Down Expand Up @@ -268,15 +269,15 @@ impl Blueprint {
self.blueprint_zones.keys().copied()
}

/// Summarize the difference between sleds and zones between two
/// blueprints.
/// Summarize the difference between two blueprints.
///
/// The argument provided is the "before" side, and `self` is the "after"
/// side. This matches the order of arguments to
/// [`Blueprint::diff_since_collection`].
pub fn diff_since_blueprint(&self, before: &Blueprint) -> BlueprintDiff {
BlueprintDiff::new(
DiffBeforeMetadata::Blueprint(Box::new(before.metadata())),
DiffBeforeClickhouseClusterConfig::from(before),
before.sled_state.clone(),
before
.blueprint_zones
Expand All @@ -288,15 +289,11 @@ impl Blueprint {
.iter()
.map(|(sled_id, disks)| (*sled_id, disks.clone().into()))
.collect(),
self.metadata(),
self.sled_state.clone(),
self.blueprint_zones.clone(),
self.blueprint_disks.clone(),
&self,
)
}

/// Summarize the differences in sleds and zones between a collection and a
/// blueprint.
/// Summarize the differences between a collection and a blueprint.
///
/// This gives an idea about what would change about a running system if
/// one were to execute the blueprint.
Expand Down Expand Up @@ -339,13 +336,11 @@ impl Blueprint {

BlueprintDiff::new(
DiffBeforeMetadata::Collection { id: before.id },
DiffBeforeClickhouseClusterConfig::from(before),
before_state,
before_zones,
before_disks,
self.metadata(),
self.sled_state.clone(),
self.blueprint_zones.clone(),
self.blueprint_disks.clone(),
&self,
)
}

Expand Down Expand Up @@ -400,6 +395,30 @@ impl BpSledSubtableData for BlueprintOrCollectionZonesConfig {
}
}

// Useful implementation for printing two column tables stored in a `BTreeMap`, where
// each key and value impl `Display`.
impl<S1, S2> BpSledSubtableData for (Generation, &BTreeMap<S1, S2>)
where
S1: fmt::Display,
S2: fmt::Display,
{
fn bp_generation(&self) -> BpGeneration {
BpGeneration::Value(self.0)
}

fn rows(
&self,
state: BpDiffState,
) -> impl Iterator<Item = BpSledSubtableRow> {
self.1.iter().map(move |(s1, s2)| {
BpSledSubtableRow::from_strings(
state,
vec![s1.to_string(), s2.to_string()],
)
})
}
}

/// Wrapper to allow a [`Blueprint`] to be displayed with information.
///
/// Returned by [`Blueprint::display()`].
Expand Down Expand Up @@ -462,6 +481,30 @@ impl<'a> BlueprintDisplay<'a> {
],
)
}

// Return tables representing a [`ClickhouseClusterConfig`] in a given blueprint
fn make_clickhouse_cluster_config_tables(
&self,
) -> Option<(KvListWithHeading, BpSledSubtable, BpSledSubtable)> {
let Some(config) = &self.blueprint.clickhouse_cluster_config else {
return None;
};

let diff_table =
ClickhouseClusterConfigDiffTables::single_blueprint_table(
BpDiffState::Unchanged,
config,
);

Some((
diff_table.metadata,
diff_table.keepers,
// Safety: The call to
// `ClickhouseClusterConfigDiffTables::single_blueprint_table`
// always returns all tables.
diff_table.servers.unwrap(),
))
}
}

impl<'a> fmt::Display for BlueprintDisplay<'a> {
Expand Down Expand Up @@ -545,6 +588,11 @@ impl<'a> fmt::Display for BlueprintDisplay<'a> {
}
}

if let Some((t1, t2, t3)) = self.make_clickhouse_cluster_config_tables()
{
writeln!(f, "{t1}\n{t2}\n{t3}\n")?;
}

writeln!(f, "{}", self.make_cockroachdb_table())?;
writeln!(f, "{}", self.make_metadata_table())?;

Expand Down Expand Up @@ -1012,6 +1060,37 @@ impl DiffBeforeMetadata {
}
}

/// Data about the "before" version within a [`BlueprintDiff`]
///
/// We only track keepers in inventory collections.
#[derive(Clone, Debug)]
pub enum DiffBeforeClickhouseClusterConfig {
Collection {
id: CollectionUuid,
latest_keeper_membership:
Option<clickhouse_admin_types::ClickhouseKeeperClusterMembership>,
},
Blueprint(Option<ClickhouseClusterConfig>),
}

impl From<&Collection> for DiffBeforeClickhouseClusterConfig {
fn from(value: &Collection) -> Self {
DiffBeforeClickhouseClusterConfig::Collection {
id: value.id,
latest_keeper_membership: value
.latest_clickhouse_keeper_membership(),
}
}
}

impl From<&Blueprint> for DiffBeforeClickhouseClusterConfig {
fn from(value: &Blueprint) -> Self {
DiffBeforeClickhouseClusterConfig::Blueprint(
value.clickhouse_cluster_config.clone(),
)
}
}

/// Single sled's zones config for "before" version within a [`BlueprintDiff`].
#[derive(Clone, Debug)]
pub enum BlueprintOrCollectionZonesConfig {
Expand Down
Loading

0 comments on commit d147f4c

Please sign in to comment.