From ec69e001ef833c61060d63d80bea25256c91dea8 Mon Sep 17 00:00:00 2001 From: iliana etaoin Date: Fri, 24 May 2024 18:10:44 -0700 Subject: [PATCH] manage cockroachdb cluster version with blueprints (#5603) To upgrade CockroachDB, we'll need to manage the `cluster.preserve_downgrade_option` cluster setting to give ourselves the opportunity to roll back an upgrade. I was initially planning to manage this with database migrations, but `SET CLUSTER SETTING` cannot be run as part of a multi-statement transaction. In the limit, the Reconfigurator will need to do this anyway as it performs rolling upgrades of CockroachDB nodes, so we may as well teach it to manage cluster settings today. --- dev-tools/omdb/tests/successes.out | 12 + docs/crdb-upgrades.adoc | 115 +++++++++ nexus/db-model/src/deployment.rs | 13 + nexus/db-model/src/schema.rs | 3 + nexus/db-model/src/schema_versions.rs | 3 +- .../src/db/datastore/cockroachdb_settings.rs | 229 +++++++++++++++++ .../db-queries/src/db/datastore/deployment.rs | 17 ++ nexus/db-queries/src/db/datastore/mod.rs | 1 + nexus/db-queries/src/db/datastore/rack.rs | 16 ++ .../execution/src/cockroachdb.rs | 113 +++++++++ nexus/reconfigurator/execution/src/dns.rs | 14 +- nexus/reconfigurator/execution/src/lib.rs | 8 + .../execution/src/omicron_physical_disks.rs | 5 +- .../execution/src/omicron_zones.rs | 6 +- .../planning/src/blueprint_builder/builder.rs | 21 ++ nexus/reconfigurator/planning/src/planner.rs | 224 +++++++++++++++++ nexus/reconfigurator/planning/src/system.rs | 9 + .../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/reconfigurator/preparation/src/lib.rs | 13 + .../src/app/background/blueprint_execution.rs | 5 +- nexus/src/app/background/blueprint_load.rs | 7 +- nexus/src/app/deployment.rs | 8 + nexus/src/app/rack.rs | 29 +++ nexus/test-utils/src/lib.rs | 4 + nexus/types/src/deployment.rs | 45 +++- nexus/types/src/deployment/blueprint_diff.rs | 153 +++++++----- .../types/src/deployment/blueprint_display.rs | 5 + nexus/types/src/deployment/planning_input.rs | 235 ++++++++++++++++++ openapi/nexus-internal.json | 89 +++++++ .../blueprint-crdb-preserve-downgrade/up1.sql | 3 + .../blueprint-crdb-preserve-downgrade/up2.sql | 2 + schema/crdb/dbinit.sql | 17 +- sled-agent/src/rack_setup/service.rs | 7 +- 40 files changed, 1388 insertions(+), 75 deletions(-) create mode 100644 docs/crdb-upgrades.adoc create mode 100644 nexus/db-queries/src/db/datastore/cockroachdb_settings.rs create mode 100644 nexus/reconfigurator/execution/src/cockroachdb.rs create mode 100644 schema/crdb/blueprint-crdb-preserve-downgrade/up1.sql create mode 100644 schema/crdb/blueprint-crdb-preserve-downgrade/up2.sql diff --git a/dev-tools/omdb/tests/successes.out b/dev-tools/omdb/tests/successes.out index 0f588069e4..07ebeb10bf 100644 --- a/dev-tools/omdb/tests/successes.out +++ b/dev-tools/omdb/tests/successes.out @@ -542,6 +542,10 @@ WARNING: Zones exist without physical disks! + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: d4d87aa2ad877a4cc2fddd0573952362739110de + cluster.preserve_downgrade_option: "22.1" + METADATA: created by::::::::::: nexus-test-utils created at::::::::::: @@ -576,6 +580,10 @@ WARNING: Zones exist without physical disks! + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: d4d87aa2ad877a4cc2fddd0573952362739110de + cluster.preserve_downgrade_option: "22.1" + METADATA: created by::::::::::: nexus-test-utils created at::::::::::: @@ -613,6 +621,10 @@ to: blueprint ............. nexus ..................... in service ::ffff:127.0.0.1 + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: d4d87aa2ad877a4cc2fddd0573952362739110de (unchanged) + cluster.preserve_downgrade_option: "22.1" (unchanged) + METADATA: internal DNS version: 1 (unchanged) external DNS version: 2 (unchanged) diff --git a/docs/crdb-upgrades.adoc b/docs/crdb-upgrades.adoc new file mode 100644 index 0000000000..eecfa9194e --- /dev/null +++ b/docs/crdb-upgrades.adoc @@ -0,0 +1,115 @@ +:showtitle: +:numbered: +:toc: left + += So You Want To Upgrade CockroachDB + +CockroachDB has a number of overlapping things called "versions": + +1. The `cockroachdb` executable is built from a particular version, such + as v22.2.19. We'll call this the *executable version*. +2. The executable version is made up of three components: a number + representing the release year, a number representing which release + it was within that year, and a patch release number. The first two + components constitute the *major version* (such as v22.2). +3. There is also a version for the on-disk data format that CockroachDB + writes and manages. This is called the *cluster version*. When + you create a new cluster while running major version v22.2, it + is initialized at cluster version `22.2`. Each major version of + CockroachDB can operate on both its own associated cluster version, + and the previous major version's cluster version, to facilitate + rolling upgrades. + +By default the cluster version is upgraded and _finalized_ once +all nodes in the cluster have upgraded to a new major version +(the CockroachDB docs refer to this as "auto-finalization"). +<> However, it is not possible to downgrade the +cluster version. To mitigate the risk of one-way upgrades, we use a +CockroachDB cluster setting named `cluster.preserve_downgrade_option` +to prevent auto-finalization and... preserve our option to downgrade in +a future release, as the option name would suggest. We then perform an +upgrade to the next major version across at least two releases, which we +refer to as a tick-tock cycle: + +- In a *tick release*, we upgrade the executable versions across the + cluster. +- In a *tock release*, we release our downgrade option and allow + CockroachDB to perform the cluster upgrade automatically. When the + upgrade is complete, we configure the "preserve downgrade option" + setting again to prepare for the next tick release. + +(This is not strictly speaking a "tick-tock" cycle, because any number +of releases may occur between a tick and a tock, and between a tock and +a tick, but they must occur in that order.) + +== Process for a tick release + +. Determine whether to move to the next major release of CockroachDB. + We have generally avoided being early adopters of new major releases + and prefer to see the rate of https://www.cockroachlabs.com/docs/advisories/[technical + advisories] that solely affect the new major version drop off. (This + generally won't stop you from working on building and testing the + next major version, however, as the build process sometimes changes + significantly from release to release.) +. Build a new version of CockroachDB for illumos. You will want to + update the https://github.com/oxidecomputer/garbage-compactor/tree/master/cockroach[build + scripts in garbage-compactor]. +. In your local Omicron checkout on a Helios machine, unpack the + resulting tarball to `out/cockroachdb`, and update `tools/cockroachdb_version` + to the version you've built. +. Add an enum variant for the new version to `CockroachDbClusterVersion` + in `nexus/types/src/deployment/planning_input.rs`, and change the + associated constant `NEWLY_INITIALIZED` to that value. +. Run the test suite, which should catch any unexpected SQL + compatibility issues between releases and help validate that your + build works. + * You will need to run the `test_omdb_success_cases` test from + omicron-omdb with `EXPECTORATE=overwrite`; this file contains the + expected output of various omdb commands, including a fingerprint of + CockroachDB's cluster state. +. Submit a PR for your changes to garbage-compactor; when merged, + publish the final build to the `oxide-cockroachdb-build` S3 bucket. +. Update `tools/cockroachdb_checksums`. For non-illumos checksums, use + the https://www.cockroachlabs.com/docs/releases/[official releases] + matching the version you built. +. Submit a PR with your changes (including `tools/cockroachdb_version` + and `tools/cockroachdb_checksums`) to Omicron. + +== Process for a tock release + +. Change the associated constant `CockroachDbClusterVersion::POLICY` in + `nexus/types/src/deployment/planning_input.rs` from the previous major + version to the current major version. + +== What Nexus does + +The Reconfigurator collects the current cluster version, and compares +this to the desired cluster version set by policy (which we update in +tock releases). + +If they do not match, CockroachDB ensures the +`cluster.preserve_downgrade_option` setting is the default value (an +empty string), which allows CockroachDB to perform the upgrade to the +desired version. The end result of this upgrade is that the current and +desired cluster versions will match. + +When they match, Nexus ensures that the +`cluster.preserve_downgrade_option` setting is set to the current +cluster version, to prevent automatic cluster upgrades when CockroachDB +is next upgraded to a new major version. + +Because blueprints are serialized and continue to run even if the +underlying state has changed, Nexus needs to ensure its view of the +world is not out-of-date. Nexus saves a fingerprint of the current +cluster state in the blueprint (intended to be opaque, but ultimately +a hash of the cluster version and executable version of the node we're +currently connected to). When setting CockroachDB options, it verifies +this fingerprint in a way that causes an error instead of setting the +option. + +[bibliography] +== External References + +- [[[crdb-tn-upgrades]]] Cockroach Labs. Cluster versions and upgrades. + November 2023. + https://github.com/cockroachdb/cockroach/blob/53262957399e6e0fccd63c91add57a510b86dc9a/docs/tech-notes/version_upgrades.md diff --git a/nexus/db-model/src/deployment.rs b/nexus/db-model/src/deployment.rs index 0b766f9e9b..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; @@ -41,6 +42,8 @@ pub struct Blueprint { pub parent_blueprint_id: Option, pub internal_dns_version: Generation, pub external_dns_version: Generation, + pub cockroachdb_fingerprint: String, + pub cockroachdb_setting_preserve_downgrade: Option, pub time_created: DateTime, pub creator: String, pub comment: String, @@ -53,6 +56,10 @@ impl From<&'_ nexus_types::deployment::Blueprint> for Blueprint { parent_blueprint_id: bp.parent_blueprint_id, internal_dns_version: Generation(bp.internal_dns_version), external_dns_version: Generation(bp.external_dns_version), + cockroachdb_fingerprint: bp.cockroachdb_fingerprint.clone(), + cockroachdb_setting_preserve_downgrade: bp + .cockroachdb_setting_preserve_downgrade + .to_optional_string(), time_created: bp.time_created, creator: bp.creator.clone(), comment: bp.comment.clone(), @@ -67,6 +74,12 @@ impl From for nexus_types::deployment::BlueprintMetadata { parent_blueprint_id: value.parent_blueprint_id, 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/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index deeca970c7..94e699443c 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -1504,6 +1504,9 @@ table! { internal_dns_version -> Int8, external_dns_version -> Int8, + cockroachdb_fingerprint -> Text, + + cockroachdb_setting_preserve_downgrade -> Nullable, } } diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 5ceaf3167a..75e1d7e440 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.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(65, 0, 0); +pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(66, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy> = 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(66, "blueprint-crdb-preserve-downgrade"), KnownVersion::new(65, "region-replacement"), KnownVersion::new(64, "add-view-for-v2p-mappings"), KnownVersion::new(63, "remove-producer-base-route-column"), diff --git a/nexus/db-queries/src/db/datastore/cockroachdb_settings.rs b/nexus/db-queries/src/db/datastore/cockroachdb_settings.rs new file mode 100644 index 0000000000..177cf673e7 --- /dev/null +++ b/nexus/db-queries/src/db/datastore/cockroachdb_settings.rs @@ -0,0 +1,229 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Datastore methods involving CockroachDB settings, which are managed by the +//! Reconfigurator. + +use super::DataStore; +use crate::context::OpContext; +use crate::db::error::public_error_from_diesel; +use crate::db::error::ErrorHandler; +use crate::db::raw_query_builder::QueryBuilder; +use async_bb8_diesel::AsyncRunQueryDsl; +use diesel::deserialize::Queryable; +use diesel::sql_types; +use nexus_types::deployment::CockroachDbSettings; +use omicron_common::api::external::Error; +use omicron_common::api::external::LookupResult; + +/// This bit of SQL calculates a "state fingerprint" for the CockroachDB +/// cluster. `DataStore::cockroachdb_settings` calculates the fingerprint and +/// returns it to the caller. `DataStore::cockroach_setting_set_*` requires the +/// caller send the fingerprint, and it verifies it against the current state of +/// the cluster. +/// +/// This is done to help prevent TOCTOU-class bugs that arise from blueprint +/// planning taking place before blueprint execution. Here are the ones we're +/// aware of, which guide the contents of this fingerprint: +/// +/// - If the cluster version has changed, we are probably in the middle of +/// an upgrade. We should not be setting any settings and should re-plan. +/// (`crdb_internal.active_version()`) +/// - If the major version of CockroachDB has changed, we should not trust +/// the blueprint's value for the `cluster.preserve_downgrade_option` +/// setting; if set to an empty string and we've just upgraded the software +/// to the next major version, this will result in unwanted finalization. +/// (`crdb_internal.node_executable_version()`) +/// +/// Because these are run as part of a gadget that allows CockroachDB to verify +/// the fingerprint during a `SET CLUSTER SETTING` statement, which cannot +/// be run as part of a multi-transaction statement or CTE, we are limited to +/// values that can be returned from built-in functions and operators. +/// +/// This fingerprint should return a STRING value. It is safe to modify how this +/// fingerprint is calculated between Nexus releases; the stale fingerprint in +/// the previous blueprint will be rejected. +const STATE_FINGERPRINT_SQL: &str = r#" + encode(digest( + crdb_internal.active_version() + || crdb_internal.node_executable_version() + , 'sha1'), 'hex') +"#; + +impl DataStore { + /// Get the current CockroachDB settings. + pub async fn cockroachdb_settings( + &self, + opctx: &OpContext, + ) -> LookupResult { + #[derive(Debug, Queryable)] + struct QueryOutput { + state_fingerprint: String, + version: String, + preserve_downgrade: String, + } + type QueryRow = (sql_types::Text, sql_types::Text, sql_types::Text); + + let conn = self.pool_connection_authorized(opctx).await?; + let output: QueryOutput = QueryBuilder::new() + .sql("SELECT ") + .sql(STATE_FINGERPRINT_SQL) + .sql(", * FROM ") + .sql("[SHOW CLUSTER SETTING version], ") + .sql("[SHOW CLUSTER SETTING cluster.preserve_downgrade_option]") + .query::() + .get_result_async(&*conn) + .await + .map_err(|err| { + public_error_from_diesel(err, ErrorHandler::Server) + })?; + Ok(CockroachDbSettings { + state_fingerprint: output.state_fingerprint, + version: output.version, + preserve_downgrade: output.preserve_downgrade, + }) + } + + /// Set a CockroachDB setting with a `String` value. + /// + /// This cannot be run in a multi-statement transaction. + pub async fn cockroachdb_setting_set_string( + &self, + opctx: &OpContext, + state_fingerprint: String, + setting: &'static str, + value: String, + ) -> Result<(), Error> { + let conn = self.pool_connection_authorized(opctx).await?; + QueryBuilder::new() + .sql("SET CLUSTER SETTING ") + .sql(setting) + // `CASE` is the one conditional statement we get out of the + // CockroachDB grammar for `SET CLUSTER SETTING`. + .sql(" = CASE ") + .sql(STATE_FINGERPRINT_SQL) + .sql(" = ") + .param() + .sql(" WHEN TRUE THEN ") + .param() + // This is the gadget that allows us to reject changing a setting + // if the fingerprint doesn't match. CockroachDB settings are typed, + // but none of them are nullable, and NULL cannot be coerced into + // any of them, so this branch returns an error if it's hit (tested + // below in `test_state_fingerprint`). + .sql(" ELSE NULL END") + .bind::(state_fingerprint) + .bind::(value.clone()) + .query::<()>() + .execute_async(&*conn) + .await + .map_err(|err| { + public_error_from_diesel(err, ErrorHandler::Server) + })?; + info!( + opctx.log, + "set cockroachdb setting"; + "setting" => setting, + "value" => &value, + ); + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::{CockroachDbSettings, OpContext}; + use nexus_test_utils::db::test_setup_database; + use nexus_types::deployment::CockroachDbClusterVersion; + use omicron_common::api::external::Error; + use omicron_test_utils::dev; + use std::sync::Arc; + + #[tokio::test] + async fn test_preserve_downgrade() { + let logctx = dev::test_setup_log("test_preserve_downgrade"); + let mut db = test_setup_database(&logctx.log).await; + let (_, datastore) = + crate::db::datastore::test_utils::datastore_test(&logctx, &db) + .await; + let opctx = + OpContext::for_tests(logctx.log.new(o!()), Arc::clone(&datastore)); + + let settings = datastore.cockroachdb_settings(&opctx).await.unwrap(); + // With a fresh cluster, this is the expected state + let version = CockroachDbClusterVersion::NEWLY_INITIALIZED.to_string(); + assert_eq!(settings.version, version); + assert_eq!(settings.preserve_downgrade, ""); + + // Verify that if a fingerprint is wrong, we get the expected SQL error + // back. + let Err(Error::InternalError { internal_message }) = datastore + .cockroachdb_setting_set_string( + &opctx, + String::new(), + "cluster.preserve_downgrade_option", + version.clone(), + ) + .await + else { + panic!("should have returned an internal error"); + }; + assert_eq!( + internal_message, + "unexpected database error: \ + cannot use unknown tree.dNull value for string setting" + ); + // And ensure that the state didn't change. + assert_eq!( + settings, + datastore.cockroachdb_settings(&opctx).await.unwrap() + ); + + // Test setting it (twice, to verify doing it again doesn't trigger + // an error) + for _ in 0..2 { + datastore + .cockroachdb_setting_set_string( + &opctx, + settings.state_fingerprint.clone(), + "cluster.preserve_downgrade_option", + version.clone(), + ) + .await + .unwrap(); + assert_eq!( + datastore.cockroachdb_settings(&opctx).await.unwrap(), + CockroachDbSettings { + state_fingerprint: settings.state_fingerprint.clone(), + version: version.clone(), + preserve_downgrade: version.clone(), + } + ); + } + + // Test resetting it (twice, same reason) + for _ in 0..2 { + datastore + .cockroachdb_setting_set_string( + &opctx, + settings.state_fingerprint.clone(), + "cluster.preserve_downgrade_option", + String::new(), + ) + .await + .unwrap(); + assert_eq!( + datastore.cockroachdb_settings(&opctx).await.unwrap(), + CockroachDbSettings { + state_fingerprint: settings.state_fingerprint.clone(), + version: version.clone(), + preserve_downgrade: String::new(), + } + ); + } + + db.cleanup().await.unwrap(); + logctx.cleanup_successful(); + } +} diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 003b64fd78..790dc0d72c 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -47,6 +47,7 @@ use nexus_types::deployment::BlueprintMetadata; use nexus_types::deployment::BlueprintPhysicalDisksConfig; use nexus_types::deployment::BlueprintTarget; use nexus_types::deployment::BlueprintZonesConfig; +use nexus_types::deployment::CockroachDbPreserveDowngrade; use nexus_types::external_api::views::SledState; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::Error; @@ -283,6 +284,8 @@ impl DataStore { parent_blueprint_id, internal_dns_version, external_dns_version, + cockroachdb_fingerprint, + cockroachdb_setting_preserve_downgrade, time_created, creator, comment, @@ -306,11 +309,23 @@ impl DataStore { blueprint.parent_blueprint_id, *blueprint.internal_dns_version, *blueprint.external_dns_version, + blueprint.cockroachdb_fingerprint, + blueprint.cockroachdb_setting_preserve_downgrade, blueprint.time_created, blueprint.creator, blueprint.comment, ) }; + let cockroachdb_setting_preserve_downgrade = + CockroachDbPreserveDowngrade::from_optional_string( + &cockroachdb_setting_preserve_downgrade, + ) + .map_err(|_| { + Error::internal_error(&format!( + "unrecognized cluster version {:?}", + cockroachdb_setting_preserve_downgrade + )) + })?; // Load the sled states for this blueprint. let sled_state: BTreeMap = { @@ -611,6 +626,8 @@ impl DataStore { parent_blueprint_id, internal_dns_version, external_dns_version, + cockroachdb_fingerprint, + cockroachdb_setting_preserve_downgrade, time_created, creator, comment, diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index a69e91dff4..b5cb749162 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -53,6 +53,7 @@ mod bfd; mod bgp; mod bootstore; mod certificate; +mod cockroachdb_settings; mod console_session; mod dataset; mod db_metadata; diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index b8275b56d4..d836185d87 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -1013,6 +1013,7 @@ mod test { }; use nexus_test_utils::db::test_setup_database; use nexus_types::deployment::BlueprintZonesConfig; + use nexus_types::deployment::CockroachDbPreserveDowngrade; use nexus_types::deployment::{ BlueprintZoneConfig, OmicronZoneExternalFloatingAddr, OmicronZoneExternalFloatingIp, @@ -1056,9 +1057,12 @@ mod test { blueprint_zones: BTreeMap::new(), blueprint_disks: BTreeMap::new(), sled_state: BTreeMap::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, parent_blueprint_id: None, internal_dns_version: *Generation::new(), external_dns_version: *Generation::new(), + cockroachdb_fingerprint: String::new(), time_created: Utc::now(), creator: "test suite".to_string(), comment: "test suite".to_string(), @@ -1525,9 +1529,12 @@ mod test { sled_state: sled_states_active(blueprint_zones.keys().copied()), blueprint_zones, blueprint_disks: BTreeMap::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, parent_blueprint_id: None, internal_dns_version: *Generation::new(), external_dns_version: *Generation::new(), + cockroachdb_fingerprint: String::new(), time_created: now_db_precision(), creator: "test suite".to_string(), comment: "test blueprint".to_string(), @@ -1779,9 +1786,12 @@ mod test { sled_state: sled_states_active(blueprint_zones.keys().copied()), blueprint_zones, blueprint_disks: BTreeMap::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, parent_blueprint_id: None, internal_dns_version: *Generation::new(), external_dns_version: *Generation::new(), + cockroachdb_fingerprint: String::new(), time_created: now_db_precision(), creator: "test suite".to_string(), comment: "test blueprint".to_string(), @@ -1988,9 +1998,12 @@ mod test { sled_state: sled_states_active(blueprint_zones.keys().copied()), blueprint_zones, blueprint_disks: BTreeMap::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, parent_blueprint_id: None, internal_dns_version: *Generation::new(), external_dns_version: *Generation::new(), + cockroachdb_fingerprint: String::new(), time_created: now_db_precision(), creator: "test suite".to_string(), comment: "test blueprint".to_string(), @@ -2123,9 +2136,12 @@ mod test { sled_state: sled_states_active(blueprint_zones.keys().copied()), blueprint_zones, blueprint_disks: BTreeMap::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, parent_blueprint_id: None, internal_dns_version: *Generation::new(), external_dns_version: *Generation::new(), + cockroachdb_fingerprint: String::new(), time_created: now_db_precision(), creator: "test suite".to_string(), comment: "test blueprint".to_string(), diff --git a/nexus/reconfigurator/execution/src/cockroachdb.rs b/nexus/reconfigurator/execution/src/cockroachdb.rs new file mode 100644 index 0000000000..101a7372c5 --- /dev/null +++ b/nexus/reconfigurator/execution/src/cockroachdb.rs @@ -0,0 +1,113 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Ensures CockroachDB settings are set + +use anyhow::Context; +use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::DataStore; +use nexus_types::deployment::Blueprint; + +pub(crate) async fn ensure_settings( + opctx: &OpContext, + datastore: &DataStore, + blueprint: &Blueprint, +) -> anyhow::Result<()> { + if let Some(value) = + blueprint.cockroachdb_setting_preserve_downgrade.to_optional_string() + { + datastore + .cockroachdb_setting_set_string( + opctx, + blueprint.cockroachdb_fingerprint.clone(), + "cluster.preserve_downgrade_option", + value, + ) + .await + .context("failed to set cluster.preserve_downgrade_option")?; + } + Ok(()) +} + +#[cfg(test)] +mod test { + use super::*; + use crate::overridables::Overridables; + use nexus_db_queries::authn; + use nexus_db_queries::authz; + use nexus_test_utils_macros::nexus_test; + use nexus_types::deployment::CockroachDbClusterVersion; + use std::sync::Arc; + + type ControlPlaneTestContext = + nexus_test_utils::ControlPlaneTestContext; + + #[nexus_test] + async fn test_ensure_preserve_downgrade_option( + cptestctx: &ControlPlaneTestContext, + ) { + let nexus = &cptestctx.server.server_context().nexus; + let datastore = nexus.datastore(); + let log = &cptestctx.logctx.log; + let opctx = OpContext::for_background( + log.clone(), + Arc::new(authz::Authz::new(log)), + authn::Context::internal_api(), + datastore.clone(), + ); + + // Fetch the initial CockroachDB settings. + let settings = datastore + .cockroachdb_settings(&opctx) + .await + .expect("failed to get cockroachdb settings"); + // Fetch the initial blueprint installed during rack initialization. + let (_blueprint_target, blueprint) = datastore + .blueprint_target_get_current_full(&opctx) + .await + .expect("failed to get blueprint from datastore"); + eprintln!("blueprint: {}", blueprint.display()); + // The initial blueprint should already have these filled in. + assert_eq!( + blueprint.cockroachdb_fingerprint, + settings.state_fingerprint + ); + assert_eq!( + blueprint.cockroachdb_setting_preserve_downgrade, + CockroachDbClusterVersion::NEWLY_INITIALIZED.into() + ); + // The cluster version, preserve downgrade setting, and + // `NEWLY_INITIALIZED` should all match. + assert_eq!( + settings.version, + CockroachDbClusterVersion::NEWLY_INITIALIZED.to_string() + ); + assert_eq!( + settings.preserve_downgrade, + CockroachDbClusterVersion::NEWLY_INITIALIZED.to_string() + ); + // Execute the initial blueprint. + let overrides = Overridables::for_test(cptestctx); + crate::realize_blueprint_with_overrides( + &opctx, + datastore, + &blueprint, + "test-suite", + &overrides, + ) + .await + .expect("failed to execute initial blueprint"); + // The CockroachDB settings should not have changed. + assert_eq!( + settings, + datastore + .cockroachdb_settings(&opctx) + .await + .expect("failed to get cockroachdb settings") + ); + + // TODO(iliana): when we upgrade to v22.2, test an actual cluster + // upgrade when crdb-seed is run with the old CockroachDB + } +} diff --git a/nexus/reconfigurator/execution/src/dns.rs b/nexus/reconfigurator/execution/src/dns.rs index 4223652b00..ec48a35cbe 100644 --- a/nexus/reconfigurator/execution/src/dns.rs +++ b/nexus/reconfigurator/execution/src/dns.rs @@ -477,6 +477,9 @@ mod test { use nexus_types::deployment::BlueprintZoneConfig; use nexus_types::deployment::BlueprintZoneDisposition; use nexus_types::deployment::BlueprintZonesConfig; + use nexus_types::deployment::CockroachDbClusterVersion; + use nexus_types::deployment::CockroachDbPreserveDowngrade; + use nexus_types::deployment::CockroachDbSettings; use nexus_types::deployment::SledFilter; use nexus_types::external_api::params; use nexus_types::external_api::shared; @@ -596,9 +599,12 @@ mod test { blueprint_zones, blueprint_disks: BTreeMap::new(), sled_state, + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, parent_blueprint_id: None, internal_dns_version: initial_dns_generation, external_dns_version: Generation::new(), + cockroachdb_fingerprint: String::new(), time_created: now_db_precision(), creator: "test-suite".to_string(), comment: "test blueprint".to_string(), @@ -1147,11 +1153,14 @@ mod test { .expect("fetching initial external DNS"); // Fetch the initial blueprint installed during rack initialization. - let (_blueprint_target, blueprint) = datastore + let (_blueprint_target, mut blueprint) = datastore .blueprint_target_get_current_full(&opctx) .await .expect("failed to read current target blueprint"); eprintln!("blueprint: {}", blueprint.display()); + // Override the CockroachDB settings so that we don't try to set them. + blueprint.cockroachdb_setting_preserve_downgrade = + CockroachDbPreserveDowngrade::DoNotModify; // Now, execute the initial blueprint. let overrides = Overridables::for_test(cptestctx); @@ -1222,9 +1231,12 @@ mod test { .into(), // These are not used because we're not actually going through // the planner. + cockroachdb_settings: &CockroachDbSettings::empty(), external_ip_rows: &[], service_nic_rows: &[], target_nexus_zone_count: NEXUS_REDUNDANCY, + target_cockroachdb_cluster_version: + CockroachDbClusterVersion::POLICY, log, } .build() diff --git a/nexus/reconfigurator/execution/src/lib.rs b/nexus/reconfigurator/execution/src/lib.rs index 8ac8bc4399..63bb4b24f0 100644 --- a/nexus/reconfigurator/execution/src/lib.rs +++ b/nexus/reconfigurator/execution/src/lib.rs @@ -24,6 +24,7 @@ use slog_error_chain::InlineErrorChain; use std::collections::BTreeMap; use std::net::SocketAddrV6; +mod cockroachdb; mod datasets; mod dns; mod external_networking; @@ -214,5 +215,12 @@ where ) .await?; + // This is likely to error if any cluster upgrades are in progress (which + // can take some time), so it should remain at the end so that other parts + // of the blueprint can progress normally. + cockroachdb::ensure_settings(&opctx, datastore, blueprint) + .await + .map_err(|err| vec![err])?; + Ok(()) } diff --git a/nexus/reconfigurator/execution/src/omicron_physical_disks.rs b/nexus/reconfigurator/execution/src/omicron_physical_disks.rs index ab0c5cab45..d7d8604e7e 100644 --- a/nexus/reconfigurator/execution/src/omicron_physical_disks.rs +++ b/nexus/reconfigurator/execution/src/omicron_physical_disks.rs @@ -109,7 +109,7 @@ mod test { use nexus_test_utils_macros::nexus_test; use nexus_types::deployment::{ Blueprint, BlueprintPhysicalDiskConfig, BlueprintPhysicalDisksConfig, - BlueprintTarget, + BlueprintTarget, CockroachDbPreserveDowngrade, }; use omicron_common::api::external::Generation; use omicron_common::disk::DiskIdentity; @@ -137,9 +137,12 @@ mod test { blueprint_zones: BTreeMap::new(), blueprint_disks, sled_state: BTreeMap::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, parent_blueprint_id: None, internal_dns_version: Generation::new(), external_dns_version: Generation::new(), + cockroachdb_fingerprint: String::new(), time_created: chrono::Utc::now(), creator: "test".to_string(), comment: "test blueprint".to_string(), diff --git a/nexus/reconfigurator/execution/src/omicron_zones.rs b/nexus/reconfigurator/execution/src/omicron_zones.rs index 68c1455ee4..a40d65411b 100644 --- a/nexus/reconfigurator/execution/src/omicron_zones.rs +++ b/nexus/reconfigurator/execution/src/omicron_zones.rs @@ -95,7 +95,8 @@ mod test { use nexus_db_queries::context::OpContext; use nexus_test_utils_macros::nexus_test; use nexus_types::deployment::{ - blueprint_zone_type, BlueprintZoneType, OmicronZonesConfig, + blueprint_zone_type, BlueprintZoneType, CockroachDbPreserveDowngrade, + OmicronZonesConfig, }; use nexus_types::deployment::{ Blueprint, BlueprintTarget, BlueprintZoneConfig, @@ -127,9 +128,12 @@ mod test { blueprint_zones, blueprint_disks: BTreeMap::new(), sled_state: BTreeMap::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, parent_blueprint_id: None, internal_dns_version: Generation::new(), external_dns_version: Generation::new(), + cockroachdb_fingerprint: String::new(), time_created: chrono::Utc::now(), creator: "test".to_string(), comment: "test blueprint".to_string(), diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs index 1efefb9817..7e98b3906d 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs @@ -20,6 +20,7 @@ use nexus_types::deployment::BlueprintZoneDisposition; use nexus_types::deployment::BlueprintZoneFilter; use nexus_types::deployment::BlueprintZoneType; use nexus_types::deployment::BlueprintZonesConfig; +use nexus_types::deployment::CockroachDbPreserveDowngrade; use nexus_types::deployment::DiskFilter; use nexus_types::deployment::OmicronZoneDataset; use nexus_types::deployment::OmicronZoneExternalFloatingIp; @@ -146,6 +147,7 @@ pub struct BlueprintBuilder<'a> { pub(super) zones: BlueprintZonesBuilder<'a>, disks: BlueprintDisksBuilder<'a>, sled_state: BTreeMap, + cockroachdb_setting_preserve_downgrade: CockroachDbPreserveDowngrade, creator: String, comments: Vec, @@ -208,6 +210,9 @@ impl<'a> BlueprintBuilder<'a> { parent_blueprint_id: None, internal_dns_version: Generation::new(), external_dns_version: Generation::new(), + cockroachdb_fingerprint: String::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, time_created: now_db_precision(), creator: creator.to_owned(), comment: format!("starting blueprint with {num_sleds} empty sleds"), @@ -264,6 +269,8 @@ impl<'a> BlueprintBuilder<'a> { zones: BlueprintZonesBuilder::new(parent_blueprint), disks: BlueprintDisksBuilder::new(parent_blueprint), sled_state, + cockroachdb_setting_preserve_downgrade: parent_blueprint + .cockroachdb_setting_preserve_downgrade, creator: creator.to_owned(), comments: Vec::new(), rng: BlueprintBuilderRng::new(), @@ -302,6 +309,13 @@ impl<'a> BlueprintBuilder<'a> { parent_blueprint_id: Some(self.parent_blueprint.id), internal_dns_version: self.input.internal_dns_version(), external_dns_version: self.input.external_dns_version(), + cockroachdb_fingerprint: self + .input + .cockroachdb_settings() + .state_fingerprint + .clone(), + cockroachdb_setting_preserve_downgrade: self + .cockroachdb_setting_preserve_downgrade, time_created: now_db_precision(), creator: self.creator, comment: self.comments.join(", "), @@ -735,6 +749,13 @@ impl<'a> BlueprintBuilder<'a> { Ok(EnsureMultiple::Added(num_nexus_to_add)) } + pub fn cockroachdb_preserve_downgrade( + &mut self, + version: CockroachDbPreserveDowngrade, + ) { + self.cockroachdb_setting_preserve_downgrade = version; + } + fn sled_add_zone( &mut self, sled_id: SledUuid, diff --git a/nexus/reconfigurator/planning/src/planner.rs b/nexus/reconfigurator/planning/src/planner.rs index 3708d212ec..6ed81cbb63 100644 --- a/nexus/reconfigurator/planning/src/planner.rs +++ b/nexus/reconfigurator/planning/src/planner.rs @@ -13,6 +13,9 @@ use crate::blueprint_builder::Error; use crate::planner::omicron_zone_placement::PlacementError; use nexus_types::deployment::Blueprint; use nexus_types::deployment::BlueprintZoneDisposition; +use nexus_types::deployment::CockroachDbClusterVersion; +use nexus_types::deployment::CockroachDbPreserveDowngrade; +use nexus_types::deployment::CockroachDbSettings; use nexus_types::deployment::PlanningInput; use nexus_types::deployment::SledFilter; use nexus_types::deployment::ZpoolFilter; @@ -25,6 +28,7 @@ use slog::{info, warn, Logger}; use std::collections::BTreeMap; use std::collections::BTreeSet; use std::hash::Hash; +use std::str::FromStr; use self::omicron_zone_placement::DiscretionaryOmicronZone; use self::omicron_zone_placement::OmicronZonePlacement; @@ -90,6 +94,7 @@ impl<'a> Planner<'a> { self.do_plan_expunge()?; self.do_plan_add()?; self.do_plan_decommission()?; + self.do_plan_cockroachdb_settings(); Ok(()) } @@ -455,6 +460,100 @@ impl<'a> Planner<'a> { Ok(()) } + + fn do_plan_cockroachdb_settings(&mut self) { + // Figure out what we should set the CockroachDB "preserve downgrade + // option" setting to based on the planning input. + // + // CockroachDB version numbers look like SemVer but are not. Major + // version numbers consist of the first *two* components, which + // represent the year and the Nth release that year. So the major + // version in "22.2.7" is "22.2". + // + // A given major version of CockroachDB is backward compatible with the + // storage format of the previous major version of CockroachDB. This is + // shown by the `version` setting, which displays the current storage + // format version. When `version` is '22.2', versions v22.2.x or v23.1.x + // can be used to run a node. This allows for rolling upgrades of nodes + // within the cluster and also preserves the ability to rollback until + // the new software version can be validated. + // + // By default, when all nodes of a cluster are upgraded to a new major + // version, the upgrade is "auto-finalized"; `version` is changed to the + // new major version, and rolling back to a previous major version of + // CockroachDB is no longer possible. + // + // The `cluster.preserve_downgrade_option` setting can be used to + // control this. This setting can only be set to the current value + // of the `version` setting, and when it is set, CockroachDB will not + // perform auto-finalization. To perform finalization and finish the + // upgrade, a client must reset the "preserve downgrade option" setting. + // Finalization occurs in the background, and the "preserve downgrade + // option" setting should not be changed again until finalization + // completes. + // + // We determine the appropriate value for `preserve_downgrade_option` + // based on: + // + // 1. the _target_ cluster version from the `Policy` (what we want to + // be running) + // 2. the `version` setting reported by CockroachDB (what we're + // currently running) + // + // by saying: + // + // - If we don't recognize the `version` CockroachDB reports, we will + // do nothing. + // - If our target version is _equal to_ what CockroachDB reports, + // we will ensure `preserve_downgrade_option` is set to the current + // `version`. This prevents auto-finalization when we deploy the next + // major version of CockroachDB as part of an update. + // - If our target version is _older than_ what CockroachDB reports, we + // will also ensure `preserve_downgrade_option` is set to the current + // `version`. (This will happen on newly-initialized clusters when + // we deploy a version of CockroachDB that is newer than our current + // policy.) + // - If our target version is _newer than_ what CockroachDB reports, we + // will ensure `preserve_downgrade_option` is set to the default value + // (the empty string). This will trigger finalization. + + let policy = self.input.target_cockroachdb_cluster_version(); + let CockroachDbSettings { version, .. } = + self.input.cockroachdb_settings(); + let value = match CockroachDbClusterVersion::from_str(version) { + // The current version is known to us. + Ok(version) => { + if policy > version { + // Ensure `cluster.preserve_downgrade_option` is reset so we + // can upgrade. + CockroachDbPreserveDowngrade::AllowUpgrade + } else { + // The cluster version is equal to or newer than the + // version we want by policy. In either case, ensure + // `cluster.preserve_downgrade_option` is set. + CockroachDbPreserveDowngrade::Set(version) + } + } + // The current version is unknown to us; we are likely in the middle + // of an cluster upgrade. + Err(_) => CockroachDbPreserveDowngrade::DoNotModify, + }; + self.blueprint.cockroachdb_preserve_downgrade(value); + info!( + &self.log, + "will ensure cockroachdb setting"; + "setting" => "cluster.preserve_downgrade_option", + "value" => ?value, + ); + + // Hey! Listen! + // + // If we need to manage more CockroachDB settings, we should ensure + // that no settings will be modified if we don't recognize the current + // cluster version -- we're likely in the middle of an upgrade! + // + // https://www.cockroachlabs.com/docs/stable/cluster-settings#change-a-cluster-setting + } } /// Returns `Some(reason)` if the sled needs its zones to be expunged, @@ -508,6 +607,9 @@ mod test { use nexus_types::deployment::BlueprintZoneDisposition; use nexus_types::deployment::BlueprintZoneFilter; use nexus_types::deployment::BlueprintZoneType; + use nexus_types::deployment::CockroachDbClusterVersion; + use nexus_types::deployment::CockroachDbPreserveDowngrade; + use nexus_types::deployment::CockroachDbSettings; use nexus_types::deployment::OmicronZoneNetworkResources; use nexus_types::external_api::views::SledPolicy; use nexus_types::external_api::views::SledProvisionPolicy; @@ -1365,4 +1467,126 @@ mod test { logctx.cleanup_successful(); } + + #[test] + fn test_ensure_preserve_downgrade_option() { + static TEST_NAME: &str = "planner_ensure_preserve_downgrade_option"; + let logctx = test_setup_log(TEST_NAME); + + let (collection, input, bp1) = example(&logctx.log, TEST_NAME, 0); + let mut builder = input.into_builder(); + assert!(bp1.cockroachdb_fingerprint.is_empty()); + assert_eq!( + bp1.cockroachdb_setting_preserve_downgrade, + CockroachDbPreserveDowngrade::DoNotModify + ); + + // If `preserve_downgrade_option` is unset and the current cluster + // version matches `POLICY`, we ensure it is set. + builder.set_cockroachdb_settings(CockroachDbSettings { + state_fingerprint: "bp2".to_owned(), + version: CockroachDbClusterVersion::POLICY.to_string(), + preserve_downgrade: String::new(), + }); + let bp2 = Planner::new_based_on( + logctx.log.clone(), + &bp1, + &builder.clone().build(), + "initial settings", + &collection, + ) + .expect("failed to create planner") + .with_rng_seed((TEST_NAME, "bp2")) + .plan() + .expect("failed to plan"); + assert_eq!(bp2.cockroachdb_fingerprint, "bp2"); + assert_eq!( + bp2.cockroachdb_setting_preserve_downgrade, + CockroachDbClusterVersion::POLICY.into() + ); + + // If `preserve_downgrade_option` is unset and the current cluster + // version is known to us and _newer_ than `POLICY`, we still ensure + // it is set. (During a "tock" release, `POLICY == NEWLY_INITIALIZED` + // and this won't be materially different than the above test, but it + // shouldn't need to change when moving to a "tick" release.) + builder.set_cockroachdb_settings(CockroachDbSettings { + state_fingerprint: "bp3".to_owned(), + version: CockroachDbClusterVersion::NEWLY_INITIALIZED.to_string(), + preserve_downgrade: String::new(), + }); + let bp3 = Planner::new_based_on( + logctx.log.clone(), + &bp1, + &builder.clone().build(), + "initial settings", + &collection, + ) + .expect("failed to create planner") + .with_rng_seed((TEST_NAME, "bp3")) + .plan() + .expect("failed to plan"); + assert_eq!(bp3.cockroachdb_fingerprint, "bp3"); + assert_eq!( + bp3.cockroachdb_setting_preserve_downgrade, + CockroachDbClusterVersion::NEWLY_INITIALIZED.into() + ); + + // When we run the planner again after setting the setting, the inputs + // will change; we should still be ensuring the setting. + builder.set_cockroachdb_settings(CockroachDbSettings { + state_fingerprint: "bp4".to_owned(), + version: CockroachDbClusterVersion::NEWLY_INITIALIZED.to_string(), + preserve_downgrade: CockroachDbClusterVersion::NEWLY_INITIALIZED + .to_string(), + }); + let bp4 = Planner::new_based_on( + logctx.log.clone(), + &bp1, + &builder.clone().build(), + "after ensure", + &collection, + ) + .expect("failed to create planner") + .with_rng_seed((TEST_NAME, "bp4")) + .plan() + .expect("failed to plan"); + assert_eq!(bp4.cockroachdb_fingerprint, "bp4"); + assert_eq!( + bp4.cockroachdb_setting_preserve_downgrade, + CockroachDbClusterVersion::NEWLY_INITIALIZED.into() + ); + + // When `version` isn't recognized, do nothing regardless of the value + // of `preserve_downgrade`. + for preserve_downgrade in [ + String::new(), + CockroachDbClusterVersion::NEWLY_INITIALIZED.to_string(), + "definitely not a real cluster version".to_owned(), + ] { + builder.set_cockroachdb_settings(CockroachDbSettings { + state_fingerprint: "bp5".to_owned(), + version: "definitely not a real cluster version".to_owned(), + preserve_downgrade: preserve_downgrade.clone(), + }); + let bp5 = Planner::new_based_on( + logctx.log.clone(), + &bp1, + &builder.clone().build(), + "unknown version", + &collection, + ) + .expect("failed to create planner") + .with_rng_seed((TEST_NAME, format!("bp5-{}", preserve_downgrade))) + .plan() + .expect("failed to plan"); + assert_eq!(bp5.cockroachdb_fingerprint, "bp5"); + assert_eq!( + bp5.cockroachdb_setting_preserve_downgrade, + CockroachDbPreserveDowngrade::DoNotModify + ); + } + + logctx.cleanup_successful(); + } } diff --git a/nexus/reconfigurator/planning/src/system.rs b/nexus/reconfigurator/planning/src/system.rs index e28b96dda5..74c9313e05 100644 --- a/nexus/reconfigurator/planning/src/system.rs +++ b/nexus/reconfigurator/planning/src/system.rs @@ -10,6 +10,8 @@ use gateway_client::types::RotState; use gateway_client::types::SpState; use indexmap::IndexMap; use nexus_inventory::CollectionBuilder; +use nexus_types::deployment::CockroachDbClusterVersion; +use nexus_types::deployment::CockroachDbSettings; use nexus_types::deployment::PlanningInputBuilder; use nexus_types::deployment::Policy; use nexus_types::deployment::SledDetails; @@ -74,6 +76,7 @@ pub struct SystemDescription { available_non_scrimlet_slots: BTreeSet, available_scrimlet_slots: BTreeSet, target_nexus_zone_count: usize, + target_cockroachdb_cluster_version: CockroachDbClusterVersion, service_ip_pool_ranges: Vec, internal_dns_version: Generation, external_dns_version: Generation, @@ -121,6 +124,8 @@ impl SystemDescription { // Policy defaults let target_nexus_zone_count = NEXUS_REDUNDANCY; + let target_cockroachdb_cluster_version = + CockroachDbClusterVersion::POLICY; // IPs from TEST-NET-1 (RFC 5737) let service_ip_pool_ranges = vec![IpRange::try_from(( "192.0.2.2".parse::().unwrap(), @@ -135,6 +140,7 @@ impl SystemDescription { available_non_scrimlet_slots, available_scrimlet_slots, target_nexus_zone_count, + target_cockroachdb_cluster_version, service_ip_pool_ranges, internal_dns_version: Generation::new(), external_dns_version: Generation::new(), @@ -301,11 +307,14 @@ impl SystemDescription { let policy = Policy { service_ip_pool_ranges: self.service_ip_pool_ranges.clone(), target_nexus_zone_count: self.target_nexus_zone_count, + target_cockroachdb_cluster_version: self + .target_cockroachdb_cluster_version, }; let mut builder = PlanningInputBuilder::new( policy, self.internal_dns_version, self.external_dns_version, + CockroachDbSettings::empty(), ); for sled in self.sleds.values() { 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/reconfigurator/preparation/src/lib.rs b/nexus/reconfigurator/preparation/src/lib.rs index 305644bc93..24e9afddf8 100644 --- a/nexus/reconfigurator/preparation/src/lib.rs +++ b/nexus/reconfigurator/preparation/src/lib.rs @@ -16,6 +16,8 @@ use nexus_db_queries::db::pagination::Paginator; use nexus_db_queries::db::DataStore; use nexus_types::deployment::Blueprint; use nexus_types::deployment::BlueprintMetadata; +use nexus_types::deployment::CockroachDbClusterVersion; +use nexus_types::deployment::CockroachDbSettings; use nexus_types::deployment::OmicronZoneExternalIp; use nexus_types::deployment::OmicronZoneNic; use nexus_types::deployment::PlanningInput; @@ -58,8 +60,10 @@ pub struct PlanningInputFromDb<'a> { pub external_ip_rows: &'a [nexus_db_model::ExternalIp], pub service_nic_rows: &'a [nexus_db_model::ServiceNetworkInterface], pub target_nexus_zone_count: usize, + pub target_cockroachdb_cluster_version: CockroachDbClusterVersion, pub internal_dns_version: nexus_db_model::Generation, pub external_dns_version: nexus_db_model::Generation, + pub cockroachdb_settings: &'a CockroachDbSettings, pub log: &'a Logger, } @@ -70,11 +74,14 @@ impl PlanningInputFromDb<'_> { let policy = Policy { service_ip_pool_ranges, target_nexus_zone_count: self.target_nexus_zone_count, + target_cockroachdb_cluster_version: self + .target_cockroachdb_cluster_version, }; let mut builder = PlanningInputBuilder::new( policy, self.internal_dns_version.into(), self.external_dns_version.into(), + self.cockroachdb_settings.clone(), ); let mut zpools_by_sled_id = { @@ -217,17 +224,23 @@ pub async fn reconfigurator_state_load( .await .context("fetching external DNS version")? .version; + let cockroachdb_settings = datastore + .cockroachdb_settings(opctx) + .await + .context("fetching cockroachdb settings")?; let planning_input = PlanningInputFromDb { sled_rows: &sled_rows, zpool_rows: &zpool_rows, ip_pool_range_rows: &ip_pool_range_rows, target_nexus_zone_count: NEXUS_REDUNDANCY, + target_cockroachdb_cluster_version: CockroachDbClusterVersion::POLICY, external_ip_rows: &external_ip_rows, service_nic_rows: &service_nic_rows, log: &opctx.log, internal_dns_version, external_dns_version, + cockroachdb_settings: &cockroachdb_settings, } .build() .context("assembling planning_input")?; diff --git a/nexus/src/app/background/blueprint_execution.rs b/nexus/src/app/background/blueprint_execution.rs index 2ac1b3fd35..69725acf1d 100644 --- a/nexus/src/app/background/blueprint_execution.rs +++ b/nexus/src/app/background/blueprint_execution.rs @@ -123,7 +123,7 @@ mod test { use nexus_types::deployment::{ blueprint_zone_type, Blueprint, BlueprintPhysicalDisksConfig, BlueprintTarget, BlueprintZoneConfig, BlueprintZoneDisposition, - BlueprintZoneType, BlueprintZonesConfig, + BlueprintZoneType, BlueprintZonesConfig, CockroachDbPreserveDowngrade, }; use nexus_types::external_api::views::SledState; use nexus_types::inventory::OmicronZoneDataset; @@ -165,9 +165,12 @@ mod test { blueprint_zones, blueprint_disks, sled_state, + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, parent_blueprint_id: None, internal_dns_version: dns_version, external_dns_version: dns_version, + cockroachdb_fingerprint: String::new(), time_created: chrono::Utc::now(), creator: "test".to_string(), comment: "test blueprint".to_string(), diff --git a/nexus/src/app/background/blueprint_load.rs b/nexus/src/app/background/blueprint_load.rs index cda1d07fcb..baf86d655f 100644 --- a/nexus/src/app/background/blueprint_load.rs +++ b/nexus/src/app/background/blueprint_load.rs @@ -188,7 +188,9 @@ mod test { use crate::app::background::common::BackgroundTask; use nexus_inventory::now_db_precision; use nexus_test_utils_macros::nexus_test; - use nexus_types::deployment::{Blueprint, BlueprintTarget}; + use nexus_types::deployment::{ + Blueprint, BlueprintTarget, CockroachDbPreserveDowngrade, + }; use omicron_common::api::external::Generation; use serde::Deserialize; use std::collections::BTreeMap; @@ -212,9 +214,12 @@ mod test { blueprint_zones: BTreeMap::new(), blueprint_disks: BTreeMap::new(), sled_state: BTreeMap::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, parent_blueprint_id: Some(parent_blueprint_id), internal_dns_version: Generation::new(), external_dns_version: Generation::new(), + cockroachdb_fingerprint: String::new(), time_created: now_db_precision(), creator: "test".to_string(), comment: "test blueprint".to_string(), diff --git a/nexus/src/app/deployment.rs b/nexus/src/app/deployment.rs index 98f1f84744..280f4306c7 100644 --- a/nexus/src/app/deployment.rs +++ b/nexus/src/app/deployment.rs @@ -13,6 +13,7 @@ use nexus_types::deployment::Blueprint; use nexus_types::deployment::BlueprintMetadata; use nexus_types::deployment::BlueprintTarget; use nexus_types::deployment::BlueprintTargetSet; +use nexus_types::deployment::CockroachDbClusterVersion; use nexus_types::deployment::PlanningInput; use nexus_types::deployment::SledFilter; use nexus_types::inventory::Collection; @@ -162,6 +163,10 @@ impl super::Nexus { "fetching external DNS version for blueprint planning", )? .version; + let cockroachdb_settings = + datastore.cockroachdb_settings(opctx).await.internal_context( + "fetching cockroachdb settings for blueprint planning", + )?; let planning_input = PlanningInputFromDb { sled_rows: &sled_rows, @@ -170,9 +175,12 @@ impl super::Nexus { external_ip_rows: &external_ip_rows, service_nic_rows: &service_nic_rows, target_nexus_zone_count: NEXUS_REDUNDANCY, + target_cockroachdb_cluster_version: + CockroachDbClusterVersion::POLICY, log: &opctx.log, internal_dns_version, external_dns_version, + cockroachdb_settings: &cockroachdb_settings, } .build()?; diff --git a/nexus/src/app/rack.rs b/nexus/src/app/rack.rs index c766446f38..1327558dd4 100644 --- a/nexus/src/app/rack.rs +++ b/nexus/src/app/rack.rs @@ -24,6 +24,7 @@ use nexus_reconfigurator_execution::silo_dns_name; use nexus_types::deployment::blueprint_zone_type; use nexus_types::deployment::BlueprintZoneFilter; use nexus_types::deployment::BlueprintZoneType; +use nexus_types::deployment::CockroachDbClusterVersion; use nexus_types::deployment::SledFilter; use nexus_types::external_api::params::Address; use nexus_types::external_api::params::AddressConfig; @@ -53,6 +54,7 @@ use omicron_common::api::external::BgpPeer; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::Error; use omicron_common::api::external::IdentityMetadataCreateParams; +use omicron_common::api::external::InternalContext; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::Name; @@ -228,6 +230,33 @@ impl super::Nexus { let mut blueprint = request.blueprint; blueprint.external_dns_version = blueprint.external_dns_version.next(); + // Fill in the CockroachDB metadata for the initial blueprint, and set + // the `cluster.preserve_downgrade_option` setting ahead of blueprint + // execution. + let cockroachdb_settings = self + .datastore() + .cockroachdb_settings(opctx) + .await + .internal_context( + "fetching cockroachdb settings for rack initialization", + )?; + self.datastore() + .cockroachdb_setting_set_string( + opctx, + cockroachdb_settings.state_fingerprint.clone(), + "cluster.preserve_downgrade_option", + CockroachDbClusterVersion::NEWLY_INITIALIZED.to_string(), + ) + .await + .internal_context( + "setting `cluster.preserve_downgrade_option` \ + for rack initialization", + )?; + blueprint.cockroachdb_fingerprint = + cockroachdb_settings.state_fingerprint; + blueprint.cockroachdb_setting_preserve_downgrade = + CockroachDbClusterVersion::NEWLY_INITIALIZED.into(); + // Administrators of the Recovery Silo are automatically made // administrators of the Fleet. let mapped_fleet_roles = BTreeMap::from([( diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index d4af109849..deb43c42b6 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -30,6 +30,7 @@ use nexus_types::deployment::BlueprintZoneConfig; use nexus_types::deployment::BlueprintZoneDisposition; use nexus_types::deployment::BlueprintZoneType; use nexus_types::deployment::BlueprintZonesConfig; +use nexus_types::deployment::CockroachDbPreserveDowngrade; use nexus_types::deployment::OmicronZoneExternalFloatingAddr; use nexus_types::deployment::OmicronZoneExternalFloatingIp; use nexus_types::external_api::params::UserId; @@ -790,6 +791,9 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { .try_into() .expect("bad internal DNS generation"), external_dns_version: Generation::new(), + cockroachdb_fingerprint: String::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, time_created: Utc::now(), creator: "nexus-test-utils".to_string(), comment: "initial test blueprint".to_string(), diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index f1be32f258..4fcd49a254 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -62,6 +62,9 @@ pub use network_resources::OmicronZoneExternalSnatIp; pub use network_resources::OmicronZoneNetworkResources; pub use network_resources::OmicronZoneNic; pub use network_resources::OmicronZoneNicEntry; +pub use planning_input::CockroachDbClusterVersion; +pub use planning_input::CockroachDbPreserveDowngrade; +pub use planning_input::CockroachDbSettings; pub use planning_input::DiskFilter; pub use planning_input::PlanningInput; pub use planning_input::PlanningInputBuildError; @@ -155,6 +158,14 @@ pub struct Blueprint { // See blueprint execution for more on this. pub external_dns_version: Generation, + /// CockroachDB state fingerprint when this blueprint was created + // See `nexus/db-queries/src/db/datastore/cockroachdb_settings.rs` for more + // on this. + pub cockroachdb_fingerprint: String, + + /// 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) pub time_created: chrono::DateTime, /// identity of the component that generated the blueprint (for debugging) @@ -173,6 +184,10 @@ impl Blueprint { parent_blueprint_id: self.parent_blueprint_id, 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(), @@ -346,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 { @@ -446,6 +482,7 @@ impl<'a> fmt::Display for BlueprintDisplay<'a> { } } + writeln!(f, "{}", self.make_cockroachdb_table())?; writeln!(f, "{}", self.make_metadata_table())?; Ok(()) @@ -998,6 +1035,12 @@ pub struct BlueprintMetadata { pub internal_dns_version: Generation, /// external DNS version when this blueprint was created 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 c8cdeec15b..bb74c3655e 100644 --- a/nexus/types/src/deployment/planning_input.rs +++ b/nexus/types/src/deployment/planning_input.rs @@ -26,10 +26,12 @@ use omicron_uuid_kinds::OmicronZoneUuid; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::SledUuid; use omicron_uuid_kinds::ZpoolUuid; +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 @@ -59,6 +61,9 @@ pub struct PlanningInput { /// current external DNS version external_dns_version: Generation, + /// current CockroachDB settings + cockroachdb_settings: CockroachDbSettings, + /// per-sled policy and resources sleds: BTreeMap, @@ -67,18 +72,31 @@ pub struct PlanningInput { } impl PlanningInput { + /// current internal DNS version pub fn internal_dns_version(&self) -> Generation { self.internal_dns_version } + /// current external DNS version pub fn external_dns_version(&self) -> Generation { self.external_dns_version } + /// current CockroachDB settings + pub fn cockroachdb_settings(&self) -> &CockroachDbSettings { + &self.cockroachdb_settings + } + pub fn target_nexus_zone_count(&self) -> usize { self.policy.target_nexus_zone_count } + pub fn target_cockroachdb_cluster_version( + &self, + ) -> CockroachDbClusterVersion { + self.policy.target_cockroachdb_cluster_version + } + pub fn service_ip_pool_ranges(&self) -> &[IpRange] { &self.policy.service_ip_pool_ranges } @@ -130,12 +148,178 @@ impl PlanningInput { policy: self.policy, internal_dns_version: self.internal_dns_version, external_dns_version: self.external_dns_version, + cockroachdb_settings: self.cockroachdb_settings, sleds: self.sleds, network_resources: self.network_resources, } } } +/// Describes the current values for any CockroachDB settings that we care +/// about. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct CockroachDbSettings { + /// A fingerprint representing the current state of the cluster. This must + /// be recorded in a blueprint and passed to the `DataStore` function when + /// changing settings. + pub state_fingerprint: String, + + /// `version` + /// + /// WARNING: This value should _not_ be used to set the + /// `cluster.preserve_downgrade_option` setting. It can potentially reflect + /// an internal, intermediate upgrade version (e.g. "22.1-12"). + pub version: String, + /// `cluster.preserve_downgrade_option` + pub preserve_downgrade: String, +} + +impl CockroachDbSettings { + pub const fn empty() -> CockroachDbSettings { + CockroachDbSettings { + state_fingerprint: String::new(), + version: String::new(), + preserve_downgrade: String::new(), + } + } +} + +/// CockroachDB cluster versions we are aware of. +/// +/// CockroachDB can be upgraded from one major version to the next, e.g. v22.1 +/// -> v22.2. Each major version introduces changes in how it stores data on +/// disk to support new features, and each major version has support for reading +/// the previous version's data so that it can perform an upgrade. The version +/// of the data format is called the "cluster version", which is distinct from +/// but related to the software version that's being run. +/// +/// While software version v22.2 is using cluster version v22.1, it's possible +/// to downgrade back to v22.1. Once the cluster version is upgraded, there's no +/// going back. +/// +/// To give us some time to evaluate new versions of the software while +/// retaining a downgrade path, we currently deploy new versions of CockroachDB +/// across two releases of the Oxide software, in a "tick-tock" model: +/// +/// - In "tick" releases, we upgrade the version of the +/// CockroachDB software to a new major version, and update +/// `CockroachDbClusterVersion::NEWLY_INITIALIZED`. On upgraded racks, the new +/// version is running with the previous cluster version; on newly-initialized +/// racks, the new version is running with the new cluser version. +/// - In "tock" releases, we change `CockroachDbClusterVersion::POLICY` to the +/// major version we upgraded to in the last "tick" release. This results in a +/// new blueprint that upgrades the cluster version, destroying the downgrade +/// path but allowing us to eventually upgrade to the next release. +/// +/// These presently describe major versions of CockroachDB. The order of these +/// must be maintained in the correct order (the first variant must be the +/// earliest version). +#[derive( + Debug, + Clone, + Copy, + PartialEq, + Eq, + PartialOrd, + Ord, + parse_display::Display, + parse_display::FromStr, + Deserialize, + Serialize, + JsonSchema, +)] +pub enum CockroachDbClusterVersion { + #[display("22.1")] + V22_1, +} + +impl CockroachDbClusterVersion { + /// The hardcoded CockroachDB cluster version we want to be on, used in + /// [`Policy`]. + /// + /// /!\ WARNING: If you change this, there is no going back. /!\ + pub const POLICY: CockroachDbClusterVersion = + CockroachDbClusterVersion::V22_1; + + /// The CockroachDB cluster version created as part of newly-initialized + /// racks. + /// + /// CockroachDB knows how to create a new cluster with the current cluster + /// version, and how to upgrade the cluster version from the previous major + /// release, but it does not have any ability to create a new cluster with + /// the previous major release's cluster version. + /// + /// During "tick" releases, newly-initialized racks will be running + /// this cluster version, which will be one major version newer than the + /// version specified by `CockroachDbClusterVersion::POLICY`. During "tock" + /// releases, these versions are the same. + pub const NEWLY_INITIALIZED: CockroachDbClusterVersion = + CockroachDbClusterVersion::V22_1; +} + +/// Whether to set `cluster.preserve_downgrade_option` and what to set it to. +#[derive( + Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize, JsonSchema, +)] +#[serde(tag = "action", content = "data", rename_all = "snake_case")] +pub enum CockroachDbPreserveDowngrade { + /// Do not modify the setting. + DoNotModify, + /// Ensure the setting is set to an empty string. + AllowUpgrade, + /// Ensure the setting is set to a given cluster version. + Set(CockroachDbClusterVersion), +} + +impl CockroachDbPreserveDowngrade { + pub fn from_optional_string( + value: &Option, + ) -> Result { + Ok(match value { + Some(version) => { + if version.is_empty() { + CockroachDbPreserveDowngrade::AllowUpgrade + } else { + CockroachDbPreserveDowngrade::Set(version.parse()?) + } + } + None => CockroachDbPreserveDowngrade::DoNotModify, + }) + } + + pub fn to_optional_string(self) -> Option { + match self { + CockroachDbPreserveDowngrade::DoNotModify => None, + CockroachDbPreserveDowngrade::AllowUpgrade => Some(String::new()), + CockroachDbPreserveDowngrade::Set(version) => { + Some(version.to_string()) + } + } + } +} + +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) + } +} + /// Describes a single disk already managed by the sled. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SledDisk { @@ -447,6 +631,11 @@ pub struct Policy { /// desired total number of deployed Nexus zones pub target_nexus_zone_count: usize, + + /// desired CockroachDB `cluster.preserve_downgrade_option` setting. + /// at present this is hardcoded based on the version of CockroachDB we + /// presently ship and the tick-tock pattern described in RFD 469. + pub target_cockroachdb_cluster_version: CockroachDbClusterVersion, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -483,6 +672,7 @@ pub struct PlanningInputBuilder { policy: Policy, internal_dns_version: Generation, external_dns_version: Generation, + cockroachdb_settings: CockroachDbSettings, sleds: BTreeMap, network_resources: OmicronZoneNetworkResources, } @@ -494,9 +684,12 @@ impl PlanningInputBuilder { policy: Policy { service_ip_pool_ranges: Vec::new(), target_nexus_zone_count: 0, + target_cockroachdb_cluster_version: + CockroachDbClusterVersion::POLICY, }, internal_dns_version: Generation::new(), external_dns_version: Generation::new(), + cockroachdb_settings: CockroachDbSettings::empty(), sleds: BTreeMap::new(), network_resources: OmicronZoneNetworkResources::new(), } @@ -506,11 +699,13 @@ impl PlanningInputBuilder { policy: Policy, internal_dns_version: Generation, external_dns_version: Generation, + cockroachdb_settings: CockroachDbSettings, ) -> Self { Self { policy, internal_dns_version, external_dns_version, + cockroachdb_settings, sleds: BTreeMap::new(), network_resources: OmicronZoneNetworkResources::new(), } @@ -574,13 +769,53 @@ impl PlanningInputBuilder { self.external_dns_version = new_version; } + pub fn set_cockroachdb_settings( + &mut self, + cockroachdb_settings: CockroachDbSettings, + ) { + self.cockroachdb_settings = cockroachdb_settings; + } + pub fn build(self) -> PlanningInput { PlanningInput { policy: self.policy, internal_dns_version: self.internal_dns_version, external_dns_version: self.external_dns_version, + cockroachdb_settings: self.cockroachdb_settings, sleds: self.sleds, network_resources: self.network_resources, } } } + +#[cfg(test)] +mod tests { + use super::CockroachDbClusterVersion; + + #[test] + fn cockroachdb_cluster_versions() { + // This should always be true. + assert!( + CockroachDbClusterVersion::POLICY + <= CockroachDbClusterVersion::NEWLY_INITIALIZED + ); + + let cockroachdb_version = + include_str!("../../../../tools/cockroachdb_version") + .trim_start_matches('v') + .rsplit_once('.') + .unwrap() + .0; + assert_eq!( + CockroachDbClusterVersion::NEWLY_INITIALIZED.to_string(), + cockroachdb_version + ); + + // In the next "tick" release, this version will be stored in a + // different file. + assert_eq!( + CockroachDbClusterVersion::POLICY.to_string(), + cockroachdb_version + ); + } +} diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index ad109a18fa..f9ca60b360 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -1742,6 +1742,18 @@ "$ref": "#/components/schemas/BlueprintZonesConfig" } }, + "cockroachdb_fingerprint": { + "description": "CockroachDB state fingerprint when this blueprint was created", + "type": "string" + }, + "cockroachdb_setting_preserve_downgrade": { + "description": "Whether to set `cluster.preserve_downgrade_option` and what to set it to", + "allOf": [ + { + "$ref": "#/components/schemas/CockroachDbPreserveDowngrade" + } + ] + }, "comment": { "description": "human-readable string describing why this blueprint was created (for debugging)", "type": "string" @@ -1793,6 +1805,8 @@ "required": [ "blueprint_disks", "blueprint_zones", + "cockroachdb_fingerprint", + "cockroachdb_setting_preserve_downgrade", "comment", "creator", "external_dns_version", @@ -1806,6 +1820,19 @@ "description": "Describe high-level metadata about a blueprint", "type": "object", "properties": { + "cockroachdb_fingerprint": { + "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" @@ -1848,6 +1875,7 @@ } }, "required": [ + "cockroachdb_fingerprint", "comment", "creator", "external_dns_version", @@ -2367,6 +2395,67 @@ "key" ] }, + "CockroachDbClusterVersion": { + "description": "CockroachDB cluster versions we are aware of.\n\nCockroachDB can be upgraded from one major version to the next, e.g. v22.1 -> v22.2. Each major version introduces changes in how it stores data on disk to support new features, and each major version has support for reading the previous version's data so that it can perform an upgrade. The version of the data format is called the \"cluster version\", which is distinct from but related to the software version that's being run.\n\nWhile software version v22.2 is using cluster version v22.1, it's possible to downgrade back to v22.1. Once the cluster version is upgraded, there's no going back.\n\nTo give us some time to evaluate new versions of the software while retaining a downgrade path, we currently deploy new versions of CockroachDB across two releases of the Oxide software, in a \"tick-tock\" model:\n\n- In \"tick\" releases, we upgrade the version of the CockroachDB software to a new major version, and update `CockroachDbClusterVersion::NEWLY_INITIALIZED`. On upgraded racks, the new version is running with the previous cluster version; on newly-initialized racks, the new version is running with the new cluser version. - In \"tock\" releases, we change `CockroachDbClusterVersion::POLICY` to the major version we upgraded to in the last \"tick\" release. This results in a new blueprint that upgrades the cluster version, destroying the downgrade path but allowing us to eventually upgrade to the next release.\n\nThese presently describe major versions of CockroachDB. The order of these must be maintained in the correct order (the first variant must be the earliest version).", + "type": "string", + "enum": [ + "V22_1" + ] + }, + "CockroachDbPreserveDowngrade": { + "description": "Whether to set `cluster.preserve_downgrade_option` and what to set it to.", + "oneOf": [ + { + "description": "Do not modify the setting.", + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "do_not_modify" + ] + } + }, + "required": [ + "action" + ] + }, + { + "description": "Ensure the setting is set to an empty string.", + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "allow_upgrade" + ] + } + }, + "required": [ + "action" + ] + }, + { + "description": "Ensure the setting is set to a given cluster version.", + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "set" + ] + }, + "data": { + "$ref": "#/components/schemas/CockroachDbClusterVersion" + } + }, + "required": [ + "action", + "data" + ] + } + ] + }, "CurrentStatus": { "description": "Describes the current status of a background task", "oneOf": [ diff --git a/schema/crdb/blueprint-crdb-preserve-downgrade/up1.sql b/schema/crdb/blueprint-crdb-preserve-downgrade/up1.sql new file mode 100644 index 0000000000..6555cd9cd2 --- /dev/null +++ b/schema/crdb/blueprint-crdb-preserve-downgrade/up1.sql @@ -0,0 +1,3 @@ +ALTER TABLE omicron.public.blueprint + ADD COLUMN IF NOT EXISTS cockroachdb_fingerprint TEXT NOT NULL DEFAULT '', + ADD COLUMN IF NOT EXISTS cockroachdb_setting_preserve_downgrade TEXT; diff --git a/schema/crdb/blueprint-crdb-preserve-downgrade/up2.sql b/schema/crdb/blueprint-crdb-preserve-downgrade/up2.sql new file mode 100644 index 0000000000..0388528071 --- /dev/null +++ b/schema/crdb/blueprint-crdb-preserve-downgrade/up2.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.blueprint + ALTER COLUMN cockroachdb_fingerprint DROP DEFAULT; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 17ea6d5510..cf4ac4b20b 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -3238,7 +3238,20 @@ CREATE TABLE IF NOT EXISTS omicron.public.blueprint ( -- identifies the latest internal DNS version when blueprint planning began internal_dns_version INT8 NOT NULL, -- identifies the latest external DNS version when blueprint planning began - external_dns_version INT8 NOT NULL + external_dns_version INT8 NOT NULL, + -- identifies the CockroachDB state fingerprint when blueprint planning began + cockroachdb_fingerprint TEXT NOT NULL, + + -- CockroachDB settings managed by blueprints. + -- + -- We use NULL in these columns to reflect that blueprint execution should + -- not modify the option; we're able to do this because CockroachDB settings + -- require the value to be the correct type and not NULL. There is no value + -- that represents "please reset this setting to the default value"; that is + -- represented by the presence of the default value in that field. + -- + -- `cluster.preserve_downgrade_option` + cockroachdb_setting_preserve_downgrade TEXT ); -- table describing both the current and historical target blueprints of the @@ -3998,7 +4011,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '65.0.0', NULL) + (TRUE, NOW(), NOW(), '66.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 27435686e7..c39ccffacd 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -93,7 +93,8 @@ use nexus_client::{ }; use nexus_types::deployment::{ Blueprint, BlueprintPhysicalDisksConfig, BlueprintZoneConfig, - BlueprintZoneDisposition, BlueprintZonesConfig, InvalidOmicronZoneType, + BlueprintZoneDisposition, BlueprintZonesConfig, + CockroachDbPreserveDowngrade, InvalidOmicronZoneType, }; use nexus_types::external_api::views::SledState; use omicron_common::address::get_sled_address; @@ -1435,6 +1436,10 @@ pub(crate) fn build_initial_blueprint_from_sled_configs( // generation of 1. Nexus will bump this up when it updates external DNS // (including creating the recovery silo). external_dns_version: Generation::new(), + // Nexus will fill in the CockroachDB values during initialization. + cockroachdb_fingerprint: String::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, time_created: Utc::now(), creator: "RSS".to_string(), comment: "initial blueprint from rack setup".to_string(),