From 9fddf329dfa9fa58633bb70983c647a32a844e9f Mon Sep 17 00:00:00 2001 From: iliana etaoin Date: Fri, 24 May 2024 04:53:07 +0000 Subject: [PATCH] add crdb settings to blueprint display --- nexus/db-model/src/deployment.rs | 6 + .../output/blueprint_builder_initial_diff.txt | 4 + .../output/planner_basic_add_sled_2_3.txt | 4 + .../output/planner_basic_add_sled_3_5.txt | 4 + .../planner_decommissions_sleds_1_2.txt | 4 + .../planner_decommissions_sleds_bp2.txt | 4 + .../output/planner_nonprovisionable_1_2.txt | 4 + .../output/planner_nonprovisionable_2_2a.txt | 4 + .../output/planner_nonprovisionable_bp2.txt | 4 + nexus/types/src/deployment.rs | 34 +++- nexus/types/src/deployment/blueprint_diff.rs | 153 ++++++++++-------- .../types/src/deployment/blueprint_display.rs | 5 + nexus/types/src/deployment/planning_input.rs | 23 ++- openapi/nexus-internal.json | 11 +- 14 files changed, 191 insertions(+), 73 deletions(-) diff --git a/nexus/db-model/src/deployment.rs b/nexus/db-model/src/deployment.rs index 5b44a2ebed..e6a66543c7 100644 --- a/nexus/db-model/src/deployment.rs +++ b/nexus/db-model/src/deployment.rs @@ -25,6 +25,7 @@ use nexus_types::deployment::BlueprintTarget; use nexus_types::deployment::BlueprintZoneConfig; use nexus_types::deployment::BlueprintZoneDisposition; use nexus_types::deployment::BlueprintZonesConfig; +use nexus_types::deployment::CockroachDbPreserveDowngrade; use omicron_common::api::internal::shared::NetworkInterface; use omicron_common::disk::DiskIdentity; use omicron_uuid_kinds::GenericUuid; @@ -74,6 +75,11 @@ impl From for nexus_types::deployment::BlueprintMetadata { internal_dns_version: *value.internal_dns_version, external_dns_version: *value.external_dns_version, cockroachdb_fingerprint: value.cockroachdb_fingerprint, + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::from_optional_string( + &value.cockroachdb_setting_preserve_downgrade, + ) + .ok(), time_created: value.time_created, creator: value.creator, comment: value.comment, diff --git a/nexus/reconfigurator/planning/tests/output/blueprint_builder_initial_diff.txt b/nexus/reconfigurator/planning/tests/output/blueprint_builder_initial_diff.txt index 8bce7cec98..03e76422e9 100644 --- a/nexus/reconfigurator/planning/tests/output/blueprint_builder_initial_diff.txt +++ b/nexus/reconfigurator/planning/tests/output/blueprint_builder_initial_diff.txt @@ -110,6 +110,10 @@ to: blueprint e4aeb3b3-272f-4967-be34-2d34daa46aa1 nexus 29278a22-1ba1-4117-bfdb-39fcb9ae7fd1 in service fd00:1122:3344:102::22 + COCKROACHDB SETTINGS: ++ state fingerprint::::::::::::::::: (not present in collection) -> (none) ++ cluster.preserve_downgrade_option: (not present in collection) -> (do not modify) + METADATA: + internal DNS version: (not present in collection) -> 1 + external DNS version: (not present in collection) -> 1 diff --git a/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_2_3.txt b/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_2_3.txt index 5b72615bd7..0253baa9f8 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_2_3.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_2_3.txt @@ -138,6 +138,10 @@ to: blueprint 4171ad05-89dd-474b-846b-b007e4346366 + internal_ntp 2d73d30e-ca47-46a8-9c12-917d4ab824b6 in service fd00:1122:3344:104::21 + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: (none) (unchanged) + cluster.preserve_downgrade_option: (do not modify) (unchanged) + METADATA: internal DNS version: 1 (unchanged) external DNS version: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_3_5.txt b/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_3_5.txt index 468303a56a..5a824edf84 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_3_5.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_3_5.txt @@ -148,6 +148,10 @@ to: blueprint f432fcd5-1284-4058-8b4a-9286a3de6163 + crucible f86e19d2-9145-41cf-be89-6aaa34a73873 in service fd00:1122:3344:104::24 + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: (none) (unchanged) + cluster.preserve_downgrade_option: (do not modify) (unchanged) + METADATA: internal DNS version: 1 (unchanged) external DNS version: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_1_2.txt b/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_1_2.txt index b939e69ba1..7219c300b7 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_1_2.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_1_2.txt @@ -126,6 +126,10 @@ to: blueprint 1ac2d88f-27dd-4506-8585-6b2be832528e + nexus c8851a11-a4f7-4b21-9281-6182fd15dc8d in service fd00:1122:3344:102::2d + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: (none) (unchanged) + cluster.preserve_downgrade_option: (do not modify) (unchanged) + METADATA: internal DNS version: 1 (unchanged) external DNS version: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_bp2.txt b/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_bp2.txt index ec94d5d924..3ba829b1d2 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_bp2.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_bp2.txt @@ -97,6 +97,10 @@ WARNING: Zones exist without physical disks! + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: (none) + cluster.preserve_downgrade_option: (do not modify) + METADATA: created by::::::::::: test_blueprint2 created at::::::::::: 1970-01-01T00:00:00.000Z diff --git a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_1_2.txt b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_1_2.txt index c5876b0b41..be2bf3c248 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_1_2.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_1_2.txt @@ -203,6 +203,10 @@ to: blueprint 9f71f5d3-a272-4382-9154-6ea2e171a6c6 + nexus c26b3bda-5561-44a1-a69f-22103fe209a1 in service fd00:1122:3344:101::2f + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: (none) (unchanged) + cluster.preserve_downgrade_option: (do not modify) (unchanged) + METADATA: internal DNS version: 1 (unchanged) external DNS version: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt index fa61fa2758..262bd14811 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt @@ -215,6 +215,10 @@ ERRORS: zone id: 7f4e9f9f-08f8-4d14-885d-e977c05525ad reason: mismatched underlay address: before: fd00:1122:3344:105::21, after: fd01:1122:3344:105::21 + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: (none) (unchanged) + cluster.preserve_downgrade_option: (do not modify) (unchanged) + METADATA: internal DNS version: 1 (unchanged) * external DNS version: 1 -> 2 diff --git a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_bp2.txt b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_bp2.txt index 454ce6779e..f7c0886dde 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_bp2.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_bp2.txt @@ -161,6 +161,10 @@ WARNING: Zones exist without physical disks! + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: (none) + cluster.preserve_downgrade_option: (do not modify) + METADATA: created by::::::::::: test_blueprint2 created at::::::::::: 1970-01-01T00:00:00.000Z diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index 7298da8cbf..4fcd49a254 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -163,8 +163,7 @@ pub struct Blueprint { // on this. pub cockroachdb_fingerprint: String, - /// Whether to set `cluster.preserve_downgrade_option` and what to set it - /// to. + /// Whether to set `cluster.preserve_downgrade_option` and what to set it to pub cockroachdb_setting_preserve_downgrade: CockroachDbPreserveDowngrade, /// when this blueprint was generated (for debugging) @@ -186,6 +185,9 @@ impl Blueprint { internal_dns_version: self.internal_dns_version, external_dns_version: self.external_dns_version, cockroachdb_fingerprint: self.cockroachdb_fingerprint.clone(), + cockroachdb_setting_preserve_downgrade: Some( + self.cockroachdb_setting_preserve_downgrade, + ), time_created: self.time_created, creator: self.creator.clone(), comment: self.comment.clone(), @@ -359,7 +361,28 @@ pub struct BlueprintDisplay<'a> { } impl<'a> BlueprintDisplay<'a> { - pub(super) fn make_metadata_table(&self) -> KvListWithHeading { + fn make_cockroachdb_table(&self) -> KvListWithHeading { + let fingerprint = if self.blueprint.cockroachdb_fingerprint.is_empty() { + NONE_PARENS.to_string() + } else { + self.blueprint.cockroachdb_fingerprint.clone() + }; + + KvListWithHeading::new_unchanged( + COCKROACHDB_HEADING, + vec![ + (COCKROACHDB_FINGERPRINT, fingerprint), + ( + COCKROACHDB_PRESERVE_DOWNGRADE, + self.blueprint + .cockroachdb_setting_preserve_downgrade + .to_string(), + ), + ], + ) + } + + fn make_metadata_table(&self) -> KvListWithHeading { let comment = if self.blueprint.comment.is_empty() { NONE_PARENS.to_string() } else { @@ -459,6 +482,7 @@ impl<'a> fmt::Display for BlueprintDisplay<'a> { } } + writeln!(f, "{}", self.make_cockroachdb_table())?; writeln!(f, "{}", self.make_metadata_table())?; Ok(()) @@ -1013,6 +1037,10 @@ pub struct BlueprintMetadata { pub external_dns_version: Generation, /// CockroachDB state fingerprint when this blueprint was created pub cockroachdb_fingerprint: String, + /// Whether to set `cluster.preserve_downgrade_option` and what to set it to + /// (`None` if this value was retrieved from the database and was invalid) + pub cockroachdb_setting_preserve_downgrade: + Option, /// when this blueprint was generated (for debugging) pub time_created: chrono::DateTime, diff --git a/nexus/types/src/deployment/blueprint_diff.rs b/nexus/types/src/deployment/blueprint_diff.rs index 905dc3dd3d..0ee039b50f 100644 --- a/nexus/types/src/deployment/blueprint_diff.rs +++ b/nexus/types/src/deployment/blueprint_diff.rs @@ -10,7 +10,7 @@ use super::blueprint_display::{ BpSledSubtable, BpSledSubtableColumn, BpSledSubtableData, BpSledSubtableRow, KvListWithHeading, KvPair, }; -use super::zone_sort_key; +use super::{zone_sort_key, CockroachDbPreserveDowngrade}; use omicron_common::api::external::Generation; use omicron_common::disk::DiskIdentity; use omicron_uuid_kinds::OmicronZoneUuid; @@ -662,71 +662,75 @@ impl<'diff> BlueprintDiffDisplay<'diff> { Self { diff } } - pub fn make_metadata_diff_table(&self) -> KvListWithHeading { - let diff = self.diff; - let mut kv = vec![]; - match &diff.before_meta { - DiffBeforeMetadata::Collection { .. } => { - // Collections don't have DNS versions, so this is new. - kv.push(KvPair::new( - BpDiffState::Added, - INTERNAL_DNS_VERSION, - linear_table_modified( - &NOT_PRESENT_IN_COLLECTION_PARENS, - &diff.after_meta.internal_dns_version, - ), - )); - kv.push(KvPair::new( - BpDiffState::Added, - EXTERNAL_DNS_VERSION, - linear_table_modified( - &NOT_PRESENT_IN_COLLECTION_PARENS, - &diff.after_meta.external_dns_version, - ), - )); - } - DiffBeforeMetadata::Blueprint(before) => { - if before.internal_dns_version - != diff.after_meta.internal_dns_version - { - kv.push(KvPair::new( - BpDiffState::Modified, - INTERNAL_DNS_VERSION, - linear_table_modified( - &before.internal_dns_version, - &diff.after_meta.internal_dns_version, - ), - )); - } else { - kv.push(KvPair::new( - BpDiffState::Unchanged, - INTERNAL_DNS_VERSION, - linear_table_unchanged(&before.internal_dns_version), - )); - }; - - if before.external_dns_version - != diff.after_meta.external_dns_version - { - kv.push(KvPair::new( - BpDiffState::Modified, - EXTERNAL_DNS_VERSION, - linear_table_modified( - &before.external_dns_version, - &diff.after_meta.external_dns_version, - ), - )); - } else { - kv.push(KvPair::new( - BpDiffState::Unchanged, - EXTERNAL_DNS_VERSION, - linear_table_unchanged(&before.external_dns_version), - )); - }; - } + pub fn make_metadata_diff_tables( + &self, + ) -> impl IntoIterator { + macro_rules! diff_row { + ($member:ident, $label:expr) => { + diff_row!($member, $label, |value| value) + }; + + ($member:ident, $label:expr, $display:expr) => { + match &self.diff.before_meta { + DiffBeforeMetadata::Collection { .. } => { + // Collections have no metadata, so this is new + KvPair::new( + BpDiffState::Added, + $label, + linear_table_modified( + &NOT_PRESENT_IN_COLLECTION_PARENS, + &$display(&self.diff.after_meta.$member), + ), + ) + } + DiffBeforeMetadata::Blueprint(before) => { + if before.$member == self.diff.after_meta.$member { + KvPair::new( + BpDiffState::Unchanged, + $label, + linear_table_unchanged(&$display( + &self.diff.after_meta.$member, + )), + ) + } else { + KvPair::new( + BpDiffState::Modified, + $label, + linear_table_modified( + &$display(&before.$member), + &$display(&self.diff.after_meta.$member), + ), + ) + } + } + } + }; } - KvListWithHeading::new(METADATA_HEADING, kv) + [ + KvListWithHeading::new( + COCKROACHDB_HEADING, + vec![ + diff_row!( + cockroachdb_fingerprint, + COCKROACHDB_FINGERPRINT, + display_none_if_empty + ), + diff_row!( + cockroachdb_setting_preserve_downgrade, + COCKROACHDB_PRESERVE_DOWNGRADE, + display_optional_preserve_downgrade + ), + ], + ), + KvListWithHeading::new( + METADATA_HEADING, + vec![ + diff_row!(internal_dns_version, INTERNAL_DNS_VERSION), + diff_row!(external_dns_version, EXTERNAL_DNS_VERSION), + ], + ), + ] } /// Write out physical disk and zone tables for a given `sled_id` @@ -847,8 +851,27 @@ impl<'diff> fmt::Display for BlueprintDiffDisplay<'diff> { } // Write out metadata diff table - writeln!(f, "{}", self.make_metadata_diff_table())?; + for table in self.make_metadata_diff_tables() { + writeln!(f, "{}", table)?; + } Ok(()) } } + +fn display_none_if_empty(value: &str) -> &str { + if value.is_empty() { + NONE_PARENS + } else { + value + } +} + +fn display_optional_preserve_downgrade( + value: &Option, +) -> String { + match value { + Some(v) => v.to_string(), + None => INVALID_VALUE_PARENS.to_string(), + } +} diff --git a/nexus/types/src/deployment/blueprint_display.rs b/nexus/types/src/deployment/blueprint_display.rs index fb5c58d513..5d106b6ef3 100644 --- a/nexus/types/src/deployment/blueprint_display.rs +++ b/nexus/types/src/deployment/blueprint_display.rs @@ -18,6 +18,10 @@ pub mod constants { pub(super) const SUB_LAST: &str = "└─"; pub const ARROW: &str = "->"; + pub const COCKROACHDB_HEADING: &str = "COCKROACHDB SETTINGS"; + pub const COCKROACHDB_FINGERPRINT: &str = "state fingerprint"; + pub const COCKROACHDB_PRESERVE_DOWNGRADE: &str = + "cluster.preserve_downgrade_option"; pub const METADATA_HEADING: &str = "METADATA"; pub const CREATED_BY: &str = "created by"; pub const CREATED_AT: &str = "created at"; @@ -29,6 +33,7 @@ pub mod constants { pub const NONE_PARENS: &str = "(none)"; pub const NOT_PRESENT_IN_COLLECTION_PARENS: &str = "(not present in collection)"; + pub const INVALID_VALUE_PARENS: &str = "(invalid value)"; } use constants::*; diff --git a/nexus/types/src/deployment/planning_input.rs b/nexus/types/src/deployment/planning_input.rs index 4410d2d707..bb74c3655e 100644 --- a/nexus/types/src/deployment/planning_input.rs +++ b/nexus/types/src/deployment/planning_input.rs @@ -26,13 +26,12 @@ use omicron_uuid_kinds::OmicronZoneUuid; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::SledUuid; use omicron_uuid_kinds::ZpoolUuid; -use parse_display::Display; -use parse_display::FromStr; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; use std::collections::btree_map::Entry; use std::collections::BTreeMap; +use std::fmt; use strum::IntoEnumIterator; /// Policy and database inputs to the Reconfigurator planner @@ -223,8 +222,8 @@ impl CockroachDbSettings { Eq, PartialOrd, Ord, - Display, - FromStr, + parse_display::Display, + parse_display::FromStr, Deserialize, Serialize, JsonSchema, @@ -299,6 +298,22 @@ impl CockroachDbPreserveDowngrade { } } +impl fmt::Display for CockroachDbPreserveDowngrade { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + CockroachDbPreserveDowngrade::DoNotModify => { + write!(f, "(do not modify)") + } + CockroachDbPreserveDowngrade::AllowUpgrade => { + write!(f, "\"\" (allow upgrade)") + } + CockroachDbPreserveDowngrade::Set(version) => { + write!(f, "\"{}\"", version) + } + } + } +} + impl From for CockroachDbPreserveDowngrade { fn from(value: CockroachDbClusterVersion) -> Self { CockroachDbPreserveDowngrade::Set(value) diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index 7dd58dd501..f9ca60b360 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -1747,7 +1747,7 @@ "type": "string" }, "cockroachdb_setting_preserve_downgrade": { - "description": "Whether to set `cluster.preserve_downgrade_option` and what to set it to.", + "description": "Whether to set `cluster.preserve_downgrade_option` and what to set it to", "allOf": [ { "$ref": "#/components/schemas/CockroachDbPreserveDowngrade" @@ -1824,6 +1824,15 @@ "description": "CockroachDB state fingerprint when this blueprint was created", "type": "string" }, + "cockroachdb_setting_preserve_downgrade": { + "nullable": true, + "description": "Whether to set `cluster.preserve_downgrade_option` and what to set it to (`None` if this value was retrieved from the database and was invalid)", + "allOf": [ + { + "$ref": "#/components/schemas/CockroachDbPreserveDowngrade" + } + ] + }, "comment": { "description": "human-readable string describing why this blueprint was created (for debugging)", "type": "string"