From 23a3335313f94141bdb44555215e3b8f753425c4 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 27 Aug 2025 10:27:01 -0700 Subject: [PATCH 1/2] (1/N) db_metadata_nexus schema changes, db queries. Populate the tables --- nexus/db-model/src/db_metadata.rs | 50 ++ nexus/db-model/src/schema_versions.rs | 6 +- .../src/db/datastore/db_metadata.rs | 635 +++++++++++++++++- nexus/db-queries/src/db/datastore/rack.rs | 17 + nexus/db-schema/src/enums.rs | 1 + nexus/db-schema/src/schema.rs | 8 + .../reconfigurator/execution/src/database.rs | 24 + nexus/reconfigurator/execution/src/lib.rs | 31 + .../execution/src/omicron_zones.rs | 9 +- nexus/tests/integration_tests/schema.rs | 156 ++++- nexus/types/src/deployment/execution/spec.rs | 1 + schema/crdb/dbinit.sql | 82 ++- .../crdb/populate-db-metadata-nexus/up01.sql | 6 + .../crdb/populate-db-metadata-nexus/up02.sql | 11 + .../crdb/populate-db-metadata-nexus/up03.sql | 4 + .../crdb/populate-db-metadata-nexus/up04.sql | 16 + 16 files changed, 1023 insertions(+), 34 deletions(-) create mode 100644 nexus/reconfigurator/execution/src/database.rs create mode 100644 schema/crdb/populate-db-metadata-nexus/up01.sql create mode 100644 schema/crdb/populate-db-metadata-nexus/up02.sql create mode 100644 schema/crdb/populate-db-metadata-nexus/up03.sql create mode 100644 schema/crdb/populate-db-metadata-nexus/up04.sql diff --git a/nexus/db-model/src/db_metadata.rs b/nexus/db-model/src/db_metadata.rs index de7e2862eb7..080da4d423c 100644 --- a/nexus/db-model/src/db_metadata.rs +++ b/nexus/db-model/src/db_metadata.rs @@ -3,8 +3,14 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use crate::SemverVersion; +use crate::impl_enum_type; +use crate::typed_uuid::DbTypedUuid; use chrono::{DateTime, Utc}; use nexus_db_schema::schema::db_metadata; +use nexus_db_schema::schema::db_metadata_nexus; +use omicron_uuid_kinds::{ + BlueprintKind, BlueprintUuid, OmicronZoneKind, OmicronZoneUuid, +}; use serde::{Deserialize, Serialize}; /// Internal database metadata @@ -33,3 +39,47 @@ impl DbMetadata { &self.version } } + +impl_enum_type!( + DbMetadataNexusStateEnum: + + #[derive(Clone, Copy, Debug, AsExpression, FromSqlRow, PartialEq, Serialize, Deserialize)] + pub enum DbMetadataNexusState; + + // Enum values + Active => b"active" + NotYet => b"not_yet" + Quiesced => b"quiesced" +); + +#[derive( + Queryable, Insertable, Debug, Clone, Selectable, Serialize, Deserialize, +)] +#[diesel(table_name = db_metadata_nexus)] +pub struct DbMetadataNexus { + nexus_id: DbTypedUuid, + last_drained_blueprint_id: Option>, + state: DbMetadataNexusState, +} + +impl DbMetadataNexus { + pub fn new(nexus_id: OmicronZoneUuid, state: DbMetadataNexusState) -> Self { + Self { + nexus_id: nexus_id.into(), + last_drained_blueprint_id: None, + state, + } + } + + pub fn state(&self) -> DbMetadataNexusState { + self.state + } + + pub fn nexus_id(&self) -> OmicronZoneUuid { + self.nexus_id.into() + } + + pub fn last_drained_blueprint_id(&self) -> Option { + self.last_drained_blueprint_id.map(|id| id.into()) + } +} diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 80df59b44f4..c471d07e50d 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// 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: Version = Version::new(184, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(185, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock> = LazyLock::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(185, "populate-db-metadata-nexus"), KnownVersion::new(184, "store-silo-admin-group-name"), KnownVersion::new(183, "add-ip-version-to-pools"), KnownVersion::new(182, "add-tuf-artifact-board"), @@ -228,6 +229,9 @@ static KNOWN_VERSIONS: LazyLock> = LazyLock::new(|| { /// The earliest supported schema version. pub const EARLIEST_SUPPORTED_VERSION: Version = Version::new(1, 0, 0); +/// The version where "db_metadata_nexus" was added. +pub const DB_METADATA_NEXUS_SCHEMA_VERSION: Version = Version::new(185, 0, 0); + /// Describes one version of the database schema #[derive(Debug, Clone)] struct KnownVersion { diff --git a/nexus/db-queries/src/db/datastore/db_metadata.rs b/nexus/db-queries/src/db/datastore/db_metadata.rs index dbc1de58571..43a60817848 100644 --- a/nexus/db-queries/src/db/datastore/db_metadata.rs +++ b/nexus/db-queries/src/db/datastore/db_metadata.rs @@ -4,18 +4,27 @@ //! [`DataStore`] methods on Database Metadata. -use super::DataStore; +use super::{DataStore, DbConnection}; +use crate::authz; +use crate::context::OpContext; + use anyhow::{Context, bail, ensure}; use async_bb8_diesel::{AsyncRunQueryDsl, AsyncSimpleConnection}; use chrono::Utc; use diesel::prelude::*; +use futures::FutureExt; use nexus_db_errors::ErrorHandler; use nexus_db_errors::public_error_from_diesel; use nexus_db_model::AllSchemaVersions; +use nexus_db_model::DbMetadataNexus; +use nexus_db_model::DbMetadataNexusState; use nexus_db_model::EARLIEST_SUPPORTED_VERSION; use nexus_db_model::SchemaUpgradeStep; use nexus_db_model::SchemaVersion; +use nexus_types::deployment::BlueprintZoneDisposition; use omicron_common::api::external::Error; +use omicron_uuid_kinds::GenericUuid; +use omicron_uuid_kinds::OmicronZoneUuid; use semver::Version; use slog::{Logger, error, info, o}; use std::ops::Bound; @@ -340,6 +349,183 @@ impl DataStore { Ok(()) } + // Returns the access this Nexus has to the database + #[cfg(test)] + async fn database_nexus_access( + &self, + nexus_id: OmicronZoneUuid, + ) -> Result, Error> { + use nexus_db_schema::schema::db_metadata_nexus::dsl; + + let nexus_access: Option = dsl::db_metadata_nexus + .filter( + dsl::nexus_id.eq(nexus_db_model::to_db_typed_uuid(nexus_id)), + ) + .first_async(&*self.pool_connection_unauthorized().await?) + .await + .optional() + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + Ok(nexus_access) + } + + // Checks if any db_metadata_nexus records exist in the database using an + // existing connection + async fn database_nexus_access_any_exist_on_connection( + conn: &async_bb8_diesel::Connection, + ) -> Result { + use nexus_db_schema::schema::db_metadata_nexus::dsl; + + let exists: bool = diesel::select(diesel::dsl::exists( + dsl::db_metadata_nexus.select(dsl::nexus_id), + )) + .get_result_async(conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + Ok(exists) + } + + /// Deletes the "db_metadata_nexus" record for a Nexus ID, if it exists. + pub async fn database_nexus_access_delete( + &self, + opctx: &OpContext, + nexus_id: OmicronZoneUuid, + ) -> Result<(), Error> { + use nexus_db_schema::schema::db_metadata_nexus::dsl; + + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; + let conn = &*self.pool_connection_authorized(&opctx).await?; + + diesel::delete( + dsl::db_metadata_nexus + .filter(dsl::nexus_id.eq(nexus_id.into_untyped_uuid())), + ) + .execute_async(conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + Ok(()) + } + + /// Propagate the nexus records to the database if and only if + /// the blueprint is the current target. + /// + /// If any of these records already exist, they are unmodified. + pub async fn database_nexus_access_create( + &self, + opctx: &OpContext, + blueprint: &nexus_types::deployment::Blueprint, + ) -> Result<(), Error> { + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; + + // TODO: Without https://github.com/oxidecomputer/omicron/pull/8863, we + // treat all Nexuses as active. Some will become "not_yet", depending on + // the Nexus Generation, once it exists. + let active_nexus_zones = blueprint + .all_omicron_zones(BlueprintZoneDisposition::is_in_service) + .filter_map(|(_sled, zone_cfg)| { + if zone_cfg.zone_type.is_nexus() { + Some(zone_cfg) + } else { + None + } + }); + let new_nexuses = active_nexus_zones + .map(|z| DbMetadataNexus::new(z.id, DbMetadataNexusState::Active)) + .collect::>(); + + let conn = &*self.pool_connection_authorized(&opctx).await?; + self.transaction_if_current_blueprint_is( + &conn, + "database_nexus_access_create", + opctx, + blueprint.id, + |conn| { + let new_nexuses = new_nexuses.clone(); + async move { + use nexus_db_schema::schema::db_metadata_nexus::dsl; + + diesel::insert_into(dsl::db_metadata_nexus) + .values(new_nexuses) + .on_conflict(dsl::nexus_id) + .do_nothing() + .execute_async(conn) + .await?; + Ok(()) + } + .boxed() + }, + ) + .await + } + + // Registers a Nexus instance as having active access to the database + #[cfg(test)] + async fn database_nexus_access_insert( + &self, + nexus_id: OmicronZoneUuid, + state: DbMetadataNexusState, + ) -> Result<(), Error> { + use nexus_db_schema::schema::db_metadata_nexus::dsl; + + let new_nexus = DbMetadataNexus::new(nexus_id, state); + + diesel::insert_into(dsl::db_metadata_nexus) + .values(new_nexus) + .on_conflict(dsl::nexus_id) + .do_update() + .set(dsl::state.eq(diesel::upsert::excluded(dsl::state))) + .execute_async(&*self.pool_connection_unauthorized().await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + Ok(()) + } + + /// Initializes Nexus database access records from a blueprint using an + /// existing connection + /// + /// This function finds all Nexus zones in the given blueprint and creates + /// active database access records for them. Used during RSS rack setup. + /// + /// Returns an error if: + /// - Any db_metadata_nexus records already exist (should only be called + /// during initial setup) + pub async fn initialize_nexus_access_from_blueprint_on_connection( + &self, + conn: &async_bb8_diesel::Connection, + nexus_zone_ids: Vec, + ) -> Result<(), Error> { + use nexus_db_schema::schema::db_metadata_nexus::dsl; + + // Ensure no db_metadata_nexus records already exist + let any_records_exist = + Self::database_nexus_access_any_exist_on_connection(conn).await?; + if any_records_exist { + return Err(Error::conflict( + "Cannot initialize Nexus access from blueprint: \ + db_metadata_nexus records already exist. This function should \ + only be called during initial rack setup.", + )); + } + + // Create db_metadata_nexus records for all Nexus zones + let new_nexuses: Vec = nexus_zone_ids + .iter() + .map(|&nexus_id| { + DbMetadataNexus::new(nexus_id, DbMetadataNexusState::Active) + }) + .collect(); + + diesel::insert_into(dsl::db_metadata_nexus) + .values(new_nexuses) + .execute_async(conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + Ok(()) + } pub async fn database_schema_version( &self, ) -> Result<(Version, Option), Error> { @@ -497,8 +683,35 @@ mod test { use crate::db::pub_test_utils::TestDatabase; use camino::Utf8Path; use camino_tempfile::Utf8TempDir; + use id_map::IdMap; use nexus_db_model::SCHEMA_VERSION; + use nexus_inventory::now_db_precision; + use nexus_types::deployment::Blueprint; + use nexus_types::deployment::BlueprintHostPhase2DesiredSlots; + use nexus_types::deployment::BlueprintSledConfig; + use nexus_types::deployment::BlueprintTarget; + use nexus_types::deployment::BlueprintZoneConfig; + use nexus_types::deployment::BlueprintZoneDisposition; + use nexus_types::deployment::BlueprintZoneImageSource; + use nexus_types::deployment::BlueprintZoneType; + use nexus_types::deployment::CockroachDbPreserveDowngrade; + use nexus_types::deployment::OximeterReadMode; + use nexus_types::deployment::PendingMgsUpdates; + use nexus_types::deployment::PlanningReport; + use nexus_types::deployment::blueprint_zone_type; + use nexus_types::external_api::views::SledState; + use nexus_types::inventory::NetworkInterface; + use nexus_types::inventory::NetworkInterfaceKind; + use omicron_common::api::external::Generation; + use omicron_common::api::external::MacAddr; + use omicron_common::api::external::Vni; + use omicron_common::zpool_name::ZpoolName; use omicron_test_utils::dev; + use omicron_uuid_kinds::BlueprintUuid; + use omicron_uuid_kinds::ExternalIpUuid; + use omicron_uuid_kinds::SledUuid; + use omicron_uuid_kinds::ZpoolUuid; + use std::collections::BTreeMap; // Confirms that calling the internal "ensure_schema" function can succeed // when the database is already at that version. @@ -768,6 +981,426 @@ mod test { .expect("Failed to get data"); assert_eq!(data, "abcd"); + db.terminate().await; + logctx.cleanup_successful(); + } + fn create_test_blueprint( + nexus_zones: Vec<(OmicronZoneUuid, BlueprintZoneDisposition)>, + ) -> Blueprint { + let blueprint_id = BlueprintUuid::new_v4(); + let sled_id = SledUuid::new_v4(); + + let zones: IdMap = nexus_zones + .into_iter() + .map(|(zone_id, disposition)| BlueprintZoneConfig { + disposition, + id: zone_id, + filesystem_pool: ZpoolName::new_external(ZpoolUuid::new_v4()), + zone_type: BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { + internal_address: "[::1]:0".parse().unwrap(), + external_dns_servers: Vec::new(), + external_ip: nexus_types::deployment::OmicronZoneExternalFloatingIp { + id: ExternalIpUuid::new_v4(), + ip: std::net::IpAddr::V6(std::net::Ipv6Addr::LOCALHOST), + }, + external_tls: true, + nic: NetworkInterface { + id: uuid::Uuid::new_v4(), + kind: NetworkInterfaceKind::Service { + id: zone_id.into_untyped_uuid(), + }, + name: "test-nic".parse().unwrap(), + ip: "192.168.1.1".parse().unwrap(), + mac: MacAddr::random_system(), + subnet: ipnetwork::IpNetwork::V4( + "192.168.1.0/24".parse().unwrap() + ).into(), + vni: Vni::try_from(100).unwrap(), + primary: true, + slot: 0, + transit_ips: Vec::new(), + }, + }), + image_source: BlueprintZoneImageSource::InstallDataset, + }) + .collect(); + + let mut sleds = BTreeMap::new(); + sleds.insert( + sled_id, + BlueprintSledConfig { + state: SledState::Active, + sled_agent_generation: Generation::new(), + zones, + disks: IdMap::new(), + datasets: IdMap::new(), + remove_mupdate_override: None, + host_phase_2: BlueprintHostPhase2DesiredSlots::current_contents( + ), + }, + ); + + Blueprint { + id: blueprint_id, + sleds, + pending_mgs_updates: PendingMgsUpdates::new(), + parent_blueprint_id: None, + internal_dns_version: Generation::new(), + external_dns_version: Generation::new(), + target_release_minimum_generation: Generation::new(), + cockroachdb_fingerprint: String::new(), + cockroachdb_setting_preserve_downgrade: + CockroachDbPreserveDowngrade::DoNotModify, + clickhouse_cluster_config: None, + oximeter_read_mode: OximeterReadMode::SingleNode, + oximeter_read_version: Generation::new(), + time_created: now_db_precision(), + creator: "test suite".to_string(), + comment: "test blueprint".to_string(), + report: PlanningReport::new(blueprint_id), + } + } + + #[tokio::test] + async fn test_database_nexus_access_create() { + let logctx = dev::test_setup_log("test_database_nexus_access_create"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let datastore = db.datastore(); + let opctx = db.opctx(); + + // Create a blueprint with two in-service Nexus zones, + // and one expunged Nexus. + let nexus1_id = OmicronZoneUuid::new_v4(); + let nexus2_id = OmicronZoneUuid::new_v4(); + let expunged_nexus = OmicronZoneUuid::new_v4(); + let blueprint = create_test_blueprint(vec![ + (nexus1_id, BlueprintZoneDisposition::InService), + (nexus2_id, BlueprintZoneDisposition::InService), + ( + expunged_nexus, + BlueprintZoneDisposition::Expunged { + as_of_generation: Generation::new(), + ready_for_cleanup: true, + }, + ), + ]); + + // Insert the blueprint and make it the target + datastore + .blueprint_insert(&opctx, &blueprint) + .await + .expect("Failed to insert blueprint"); + datastore + .blueprint_target_set_current( + &opctx, + BlueprintTarget { + target_id: blueprint.id, + enabled: false, + time_made_target: chrono::Utc::now(), + }, + ) + .await + .expect("Failed to set blueprint target"); + + // Create nexus access records + datastore + .database_nexus_access_create(&opctx, &blueprint) + .await + .expect("Failed to create nexus access"); + + // Verify records were created with Active state + let nexus1_access = datastore + .database_nexus_access(nexus1_id) + .await + .expect("Failed to get nexus1 access"); + let nexus2_access = datastore + .database_nexus_access(nexus2_id) + .await + .expect("Failed to get nexus2 access"); + let expunged_access = datastore + .database_nexus_access(expunged_nexus) + .await + .expect("Failed to get expunged access"); + + assert!(nexus1_access.is_some(), "nexus1 should have access record"); + assert!(nexus2_access.is_some(), "nexus2 should have access record"); + assert!( + expunged_access.is_none(), + "expunged nexus should not have access record" + ); + + let nexus1_record = nexus1_access.unwrap(); + let nexus2_record = nexus2_access.unwrap(); + assert_eq!(nexus1_record.state(), DbMetadataNexusState::Active); + assert_eq!(nexus2_record.state(), DbMetadataNexusState::Active); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn test_database_nexus_access_create_idempotent() { + let logctx = + dev::test_setup_log("test_database_nexus_access_create_idempotent"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let datastore = db.datastore(); + let opctx = db.opctx(); + + // Create a blueprint with one Nexus zone + let nexus_id = OmicronZoneUuid::new_v4(); + let blueprint = create_test_blueprint(vec![( + nexus_id, + BlueprintZoneDisposition::InService, + )]); + + // Insert the blueprint and make it the target + datastore + .blueprint_insert(&opctx, &blueprint) + .await + .expect("Failed to insert blueprint"); + datastore + .blueprint_target_set_current( + &opctx, + BlueprintTarget { + target_id: blueprint.id, + enabled: false, + time_made_target: chrono::Utc::now(), + }, + ) + .await + .expect("Failed to set blueprint target"); + + // Create nexus access records (first time) + datastore + .database_nexus_access_create(&opctx, &blueprint) + .await + .expect("Failed to create nexus access (first time)"); + + // Verify record was created + async fn confirm_state( + datastore: &DataStore, + nexus_id: OmicronZoneUuid, + expected_state: DbMetadataNexusState, + ) { + let state = datastore + .database_nexus_access(nexus_id) + .await + .expect("Failed to get nexus access after first create") + .expect("Entry for Nexus should have been inserted"); + assert_eq!(state.state(), expected_state); + } + + confirm_state(datastore, nexus_id, DbMetadataNexusState::Active).await; + + // Creating the record again: not an error. + datastore + .database_nexus_access_create(&opctx, &blueprint) + .await + .expect("Failed to create nexus access (first time)"); + confirm_state(datastore, nexus_id, DbMetadataNexusState::Active).await; + + // Manually make the record "Quiesced". + use nexus_db_schema::schema::db_metadata_nexus::dsl; + diesel::update(dsl::db_metadata_nexus) + .filter(dsl::nexus_id.eq(nexus_id.into_untyped_uuid())) + .set(dsl::state.eq(DbMetadataNexusState::Quiesced)) + .execute_async( + &*datastore.pool_connection_unauthorized().await.unwrap(), + ) + .await + .expect("Failed to update record"); + confirm_state(datastore, nexus_id, DbMetadataNexusState::Quiesced) + .await; + + // Create nexus access records another time - should be idempotent, + // but should be "on-conflict, ignore". + datastore + .database_nexus_access_create(&opctx, &blueprint) + .await + .expect("Failed to create nexus access (second time)"); + confirm_state(datastore, nexus_id, DbMetadataNexusState::Quiesced) + .await; + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn test_database_nexus_access_create_fails_wrong_target_blueprint() { + let logctx = dev::test_setup_log( + "test_database_nexus_access_create_fails_wrong_target_blueprint", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let datastore = db.datastore(); + let opctx = db.opctx(); + + // Create two different blueprints + let nexus_id = OmicronZoneUuid::new_v4(); + let target_blueprint = create_test_blueprint(vec![( + nexus_id, + BlueprintZoneDisposition::InService, + )]); + let non_target_blueprint = create_test_blueprint(vec![( + nexus_id, + BlueprintZoneDisposition::InService, + )]); + + // Insert both blueprints + datastore + .blueprint_insert(&opctx, &target_blueprint) + .await + .expect("Failed to insert target blueprint"); + datastore + .blueprint_insert(&opctx, &non_target_blueprint) + .await + .expect("Failed to insert non-target blueprint"); + + // Set the first blueprint as the target + datastore + .blueprint_target_set_current( + &opctx, + BlueprintTarget { + target_id: target_blueprint.id, + enabled: false, + time_made_target: chrono::Utc::now(), + }, + ) + .await + .expect("Failed to set target blueprint"); + + // Try to create nexus access records using the non-target blueprint. + // This should fail because the transaction should check if the + // blueprint is the current target + let result = datastore + .database_nexus_access_create(&opctx, &non_target_blueprint) + .await; + assert!( + result.is_err(), + "Creating nexus access with wrong target blueprint should fail" + ); + + // Verify no records were created for the nexus + let access = datastore + .database_nexus_access(nexus_id) + .await + .expect("Failed to get nexus access"); + assert!( + access.is_none(), + "No access record should exist when wrong blueprint is used" + ); + + // Verify that using the correct target blueprint works + datastore + .database_nexus_access_create(&opctx, &target_blueprint) + .await + .expect( + "Creating nexus access with correct blueprint should succeed", + ); + + let access_after_correct = datastore + .database_nexus_access(nexus_id) + .await + .expect("Failed to get nexus access after correct blueprint"); + assert!( + access_after_correct.is_some(), + "Access record should exist after using correct target blueprint" + ); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn test_database_nexus_access_delete() { + let logctx = dev::test_setup_log("test_database_nexus_access_delete"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let datastore = db.datastore(); + let opctx = db.opctx(); + + // Create test nexus IDs + let nexus1_id = OmicronZoneUuid::new_v4(); + let nexus2_id = OmicronZoneUuid::new_v4(); + + // Insert records directly using the test method + datastore + .database_nexus_access_insert( + nexus1_id, + DbMetadataNexusState::Active, + ) + .await + .expect("Failed to insert nexus1 access"); + datastore + .database_nexus_access_insert( + nexus2_id, + DbMetadataNexusState::NotYet, + ) + .await + .expect("Failed to insert nexus2 access"); + + // Verify records were created + let nexus1_before = datastore + .database_nexus_access(nexus1_id) + .await + .expect("Failed to get nexus1 access"); + let nexus2_before = datastore + .database_nexus_access(nexus2_id) + .await + .expect("Failed to get nexus2 access"); + assert!(nexus1_before.is_some(), "nexus1 should have access record"); + assert!(nexus2_before.is_some(), "nexus2 should have access record"); + + // Delete nexus1 record + datastore + .database_nexus_access_delete(&opctx, nexus1_id) + .await + .expect("Failed to delete nexus1 access"); + + // Verify nexus1 record was deleted, nexus2 record remains + let nexus1_after = datastore + .database_nexus_access(nexus1_id) + .await + .expect("Failed to get nexus1 access after delete"); + let nexus2_after = datastore + .database_nexus_access(nexus2_id) + .await + .expect("Failed to get nexus2 access after delete"); + assert!( + nexus1_after.is_none(), + "nexus1 should not have access record after delete" + ); + assert!( + nexus2_after.is_some(), + "nexus2 should still have access record" + ); + + // Delete nexus2 record + datastore + .database_nexus_access_delete(&opctx, nexus2_id) + .await + .expect("Failed to delete nexus2 access"); + + // Verify nexus2 record was also deleted + let nexus2_final = datastore + .database_nexus_access(nexus2_id) + .await + .expect("Failed to get nexus2 access after final delete"); + assert!( + nexus2_final.is_none(), + "nexus2 should not have access record after delete" + ); + + // Confirm deletion is idempotent + datastore + .database_nexus_access_delete(&opctx, nexus1_id) + .await + .expect("Failed to delete nexus1 access idempotently"); + + // This also means deleting non-existent records should be fine + datastore + .database_nexus_access_delete(&opctx, OmicronZoneUuid::new_v4()) + .await + .expect("Failed to delete non-existent record"); + db.terminate().await; logctx.cleanup_successful(); } diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index a2fcb2c6c82..31e0ec43284 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -727,6 +727,7 @@ impl DataStore { // - Zpools // - Datasets // - A blueprint + // - Nexus database access records // // Which RSS has already allocated during bootstrapping. @@ -793,6 +794,22 @@ impl DataStore { DieselError::RollbackTransaction })?; + // Insert Nexus database access records + self.initialize_nexus_access_from_blueprint_on_connection( + &conn, + blueprint.all_omicron_zones(BlueprintZoneDisposition::is_in_service) + .filter_map(|(_sled, zone_cfg)| { + if zone_cfg.zone_type.is_nexus() { + Some(zone_cfg.id) + } else { + None + } + }).collect(), + ).await.map_err(|e| { + err.set(RackInitError::BlueprintTargetSet(e)).unwrap(); + DieselError::RollbackTransaction + })?; + // Allocate networking records for all services. for (_, zone_config) in blueprint.all_omicron_zones(BlueprintZoneDisposition::is_in_service) { self.rack_populate_service_networking_records( diff --git a/nexus/db-schema/src/enums.rs b/nexus/db-schema/src/enums.rs index 1766054c9ad..bac6d5ae3b4 100644 --- a/nexus/db-schema/src/enums.rs +++ b/nexus/db-schema/src/enums.rs @@ -37,6 +37,7 @@ define_enums! { CabooseWhichEnum => "caboose_which", ClickhouseModeEnum => "clickhouse_mode", DatasetKindEnum => "dataset_kind", + DbMetadataNexusStateEnum => "db_metadata_nexus_state", DnsGroupEnum => "dns_group", DownstairsClientStopRequestReasonEnum => "downstairs_client_stop_request_reason_type", DownstairsClientStoppedReasonEnum => "downstairs_client_stopped_reason_type", diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 28a168d6f76..4f8bd54a7a2 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -2373,6 +2373,14 @@ table! { } } +table! { + db_metadata_nexus (nexus_id) { + nexus_id -> Uuid, + last_drained_blueprint_id -> Nullable, + state -> crate::enums::DbMetadataNexusStateEnum, + } +} + table! { migration (id) { id -> Uuid, diff --git a/nexus/reconfigurator/execution/src/database.rs b/nexus/reconfigurator/execution/src/database.rs new file mode 100644 index 00000000000..9652e53ec1a --- /dev/null +++ b/nexus/reconfigurator/execution/src/database.rs @@ -0,0 +1,24 @@ +// 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/. + +//! Manages deployment of records into the database. + +use anyhow::anyhow; +use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::DataStore; +use nexus_types::deployment::Blueprint; + +/// Idempotently ensure that the Nexus records for the zones are populated +/// in the database. +pub(crate) async fn deploy_db_metadata_nexus_records( + opctx: &OpContext, + datastore: &DataStore, + blueprint: &Blueprint, +) -> Result<(), anyhow::Error> { + datastore + .database_nexus_access_create(opctx, blueprint) + .await + .map_err(|err| anyhow!(err))?; + Ok(()) +} diff --git a/nexus/reconfigurator/execution/src/lib.rs b/nexus/reconfigurator/execution/src/lib.rs index 43c09485557..5060a037e22 100644 --- a/nexus/reconfigurator/execution/src/lib.rs +++ b/nexus/reconfigurator/execution/src/lib.rs @@ -32,8 +32,10 @@ use tokio::sync::watch; use update_engine::StepSuccess; use update_engine::StepWarning; use update_engine::merge_anyhow_list; + mod clickhouse; mod cockroachdb; +mod database; mod dns; mod omicron_physical_disks; mod omicron_sled_config; @@ -196,6 +198,13 @@ pub async fn realize_blueprint( ) .into_shared(); + register_deploy_db_metadata_nexus_records_step( + &engine.for_component(ExecutionComponent::DeployNexusRecords), + &opctx, + datastore, + blueprint, + ); + register_deploy_sled_configs_step( &engine.for_component(ExecutionComponent::SledAgent), &opctx, @@ -390,6 +399,28 @@ fn register_sled_list_step<'a>( .register() } +fn register_deploy_db_metadata_nexus_records_step<'a>( + registrar: &ComponentRegistrar<'_, 'a>, + opctx: &'a OpContext, + datastore: &'a DataStore, + blueprint: &'a Blueprint, +) { + registrar + .new_step( + ExecutionStepId::Ensure, + "Ensure db_metadata_nexus_state records exist", + async move |_cx| { + database::deploy_db_metadata_nexus_records( + opctx, &datastore, &blueprint, + ) + .await + .context("ensuring db_metadata_nexus_state")?; + StepSuccess::new(()).into() + }, + ) + .register(); +} + fn register_deploy_sled_configs_step<'a>( registrar: &ComponentRegistrar<'_, 'a>, opctx: &'a OpContext, diff --git a/nexus/reconfigurator/execution/src/omicron_zones.rs b/nexus/reconfigurator/execution/src/omicron_zones.rs index 28c981fd90e..74e4625358d 100644 --- a/nexus/reconfigurator/execution/src/omicron_zones.rs +++ b/nexus/reconfigurator/execution/src/omicron_zones.rs @@ -72,10 +72,13 @@ async fn clean_up_expunged_zones_impl( )); let result = match &config.zone_type { - // Zones which need no cleanup work after expungement. - BlueprintZoneType::Nexus(_) => None, - // Zones which need cleanup after expungement. + BlueprintZoneType::Nexus(_) => Some( + datastore + .database_nexus_access_delete(&opctx, config.id) + .await + .map_err(|err| anyhow::anyhow!(err)), + ), BlueprintZoneType::CockroachDb(_) => { if decommission_cockroach { Some( diff --git a/nexus/tests/integration_tests/schema.rs b/nexus/tests/integration_tests/schema.rs index 9111e6a0949..db6c0cb6eaf 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -2889,6 +2889,157 @@ fn after_171_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { }) } +const NEXUS_ID_185_0: &str = "387433f9-1473-4ca2-b156-9670452985e0"; +const EXPUNGED_NEXUS_ID_185_0: &str = "287433f9-1473-4ca2-b156-9670452985e0"; +const OLD_NEXUS_ID_185_0: &str = "187433f9-1473-4ca2-b156-9670452985e0"; + +const BP_ID_185_0: &str = "5a5ff941-3b5a-403b-9fda-db2049f4c736"; +const OLD_BP_ID_185_0: &str = "4a5ff941-3b5a-403b-9fda-db2049f4c736"; + +fn before_185_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { + Box::pin(async move { + // Create a blueprint which contains a Nexus - we'll use this for the migration. + // + // It also contains an exupnged Nexus, which should be ignored. + ctx.client + .execute( + &format!( + "INSERT INTO omicron.public.bp_target + (version, blueprint_id, enabled, time_made_target) + VALUES + (1, '{BP_ID_185_0}', true, now());", + ), + &[], + ) + .await + .expect("inserted bp_target rows for 182"); + ctx.client + .execute( + &format!( + "INSERT INTO omicron.public.bp_omicron_zone ( + blueprint_id, sled_id, id, zone_type, + primary_service_ip, primary_service_port, + second_service_ip, second_service_port, + dataset_zpool_name, bp_nic_id, + dns_gz_address, dns_gz_address_index, + ntp_ntp_servers, ntp_dns_servers, ntp_domain, + nexus_external_tls, nexus_external_dns_servers, + snat_ip, snat_first_port, snat_last_port, + external_ip_id, filesystem_pool, disposition, + disposition_expunged_as_of_generation, + disposition_expunged_ready_for_cleanup, + image_source, image_artifact_sha256 + ) + VALUES ( + '{BP_ID_185_0}', gen_random_uuid(), '{NEXUS_ID_185_0}', + 'nexus', '192.168.1.10', 8080, NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, NULL, false, ARRAY[]::INET[], + NULL, NULL, NULL, NULL, gen_random_uuid(), + 'in_service', NULL, false, 'install_dataset', NULL + ), + ( + '{BP_ID_185_0}', gen_random_uuid(), + '{EXPUNGED_NEXUS_ID_185_0}', 'nexus', '192.168.1.11', + 8080, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, + NULL, false, ARRAY[]::INET[], NULL, NULL, NULL, NULL, + gen_random_uuid(), 'expunged', 1, false, + 'install_dataset', NULL + );" + ), + &[], + ) + .await + .expect("inserted bp_omicron_zone rows for 182"); + + // ALSO create an old blueprint, which isn't the latest target. + // + // We should ignore this one! No rows should be inserted for old data. + ctx.client + .execute( + &format!( + "INSERT INTO omicron.public.bp_target + (version, blueprint_id, enabled, time_made_target) + VALUES + (0, '{OLD_BP_ID_185_0}', true, now());", + ), + &[], + ) + .await + .expect("inserted bp_target rows for 182"); + ctx.client + .execute( + &format!( + "INSERT INTO omicron.public.bp_omicron_zone ( + blueprint_id, sled_id, id, zone_type, + primary_service_ip, primary_service_port, + second_service_ip, second_service_port, + dataset_zpool_name, bp_nic_id, + dns_gz_address, dns_gz_address_index, + ntp_ntp_servers, ntp_dns_servers, ntp_domain, + nexus_external_tls, nexus_external_dns_servers, + snat_ip, snat_first_port, snat_last_port, + external_ip_id, filesystem_pool, disposition, + disposition_expunged_as_of_generation, + disposition_expunged_ready_for_cleanup, + image_source, image_artifact_sha256 + ) + VALUES ( + '{OLD_BP_ID_185_0}', gen_random_uuid(), + '{OLD_NEXUS_ID_185_0}', 'nexus', '192.168.1.10', 8080, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, NULL, + false, ARRAY[]::INET[], NULL, NULL, NULL, + NULL, gen_random_uuid(), 'in_service', + NULL, false, 'install_dataset', NULL + );" + ), + &[], + ) + .await + .expect("inserted bp_omicron_zone rows for 182"); + }) +} + +fn after_185_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { + Box::pin(async move { + // After the migration, the new row should be created - only for Nexuses + // in the latest blueprint. + // + // Note that "OLD_NEXUS_ID_185_0" doesn't get a row - it's in an old + // blueprint. + let rows = ctx + .client + .query( + "SELECT + nexus_id, + last_drained_blueprint_id, + state + FROM omicron.public.db_metadata_nexus;", + &[], + ) + .await + .expect("queried post-migration inv_sled_config_reconciler"); + + let rows = process_rows(&rows); + assert_eq!(rows.len(), 1); + let row = &rows[0]; + + // Create a new row for the Nexuses in the target blueprint + assert_eq!( + row.values[0].expect("nexus_id").unwrap(), + &AnySqlType::Uuid(NEXUS_ID_185_0.parse().unwrap()) + ); + assert_eq!(row.values[1].expect("last_drained_blueprint_id"), None); + assert_eq!( + row.values[2].expect("state").unwrap(), + &AnySqlType::Enum(SqlEnum::from(( + "db_metadata_nexus_state", + "active" + ))) + ); + }) +} + // Lazily initializes all migration checks. The combination of Rust function // pointers and async makes defining a static table fairly painful, so we're // using lazy initialization instead. @@ -2987,7 +3138,10 @@ fn get_migration_checks() -> BTreeMap { Version::new(171, 0, 0), DataMigrationFns::new().before(before_171_0_0).after(after_171_0_0), ); - + map.insert( + Version::new(185, 0, 0), + DataMigrationFns::new().before(before_185_0_0).after(after_185_0_0), + ); map } diff --git a/nexus/types/src/deployment/execution/spec.rs b/nexus/types/src/deployment/execution/spec.rs index 482355dfee5..27d85b431db 100644 --- a/nexus/types/src/deployment/execution/spec.rs +++ b/nexus/types/src/deployment/execution/spec.rs @@ -31,6 +31,7 @@ pub enum ExecutionComponent { SupportBundles, SledList, SledAgent, + DeployNexusRecords, PhysicalDisks, OmicronZones, FirewallRules, diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index e6e2949c08b..bad381cb5c8 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -5630,29 +5630,6 @@ CREATE INDEX IF NOT EXISTS lookup_region_snapshot_replacement_step_by_state CREATE INDEX IF NOT EXISTS lookup_region_snapshot_replacement_step_by_old_volume_id on omicron.public.region_snapshot_replacement_step (old_snapshot_volume_id); -/* - * Metadata for the schema itself. This version number isn't great, as there's - * nothing to ensure it gets bumped when it should be, but it's a start. - */ -CREATE TABLE IF NOT EXISTS omicron.public.db_metadata ( - -- There should only be one row of this table for the whole DB. - -- It's a little goofy, but filter on "singleton = true" before querying - -- or applying updates, and you'll access the singleton row. - -- - -- We also add a constraint on this table to ensure it's not possible to - -- access the version of this table with "singleton = false". - singleton BOOL NOT NULL PRIMARY KEY, - time_created TIMESTAMPTZ NOT NULL, - time_modified TIMESTAMPTZ NOT NULL, - -- Semver representation of the DB version - version STRING(64) NOT NULL, - - -- (Optional) Semver representation of the DB version to which we're upgrading - target_version STRING(64), - - CHECK (singleton = true) -); - -- An allowlist of IP addresses that can make requests to user-facing services. CREATE TABLE IF NOT EXISTS omicron.public.allow_list ( id UUID PRIMARY KEY, @@ -6553,10 +6530,59 @@ ON omicron.public.host_ereport ( ) WHERE time_deleted IS NULL; -/* - * Keep this at the end of file so that the database does not contain a version - * until it is fully populated. - */ +-- Metadata for the schema itself. +-- +-- This table may be read by Nexuses with different notions of "what the schema should be". +-- Unlike other tables in the database, caution should be taken when upgrading this schema. +CREATE TABLE IF NOT EXISTS omicron.public.db_metadata ( + -- There should only be one row of this table for the whole DB. + -- It's a little goofy, but filter on "singleton = true" before querying + -- or applying updates, and you'll access the singleton row. + -- + -- We also add a constraint on this table to ensure it's not possible to + -- access the version of this table with "singleton = false". + singleton BOOL NOT NULL PRIMARY KEY, + time_created TIMESTAMPTZ NOT NULL, + time_modified TIMESTAMPTZ NOT NULL, + -- Semver representation of the DB version + version STRING(64) NOT NULL, + + -- (Optional) Semver representation of the DB version to which we're upgrading + target_version STRING(64), + + CHECK (singleton = true) +); + +CREATE TYPE IF NOT EXISTS omicron.public.db_metadata_nexus_state AS ENUM ( + -- This Nexus is allowed to access this database + 'active', + + -- This Nexus is not yet allowed to access the database + 'not_yet', + + -- This Nexus has committed to no longer accessing this database + 'quiesced' +); + +-- Nexuses which may be attempting to access the database, and a state +-- which identifies if they should be allowed to do so. +-- +-- This table is used during upgrade implement handoff between old and new +-- Nexus zones. It is read by all Nexuses during initialization to identify +-- if they should have access to the database. +CREATE TABLE IF NOT EXISTS omicron.public.db_metadata_nexus ( + nexus_id UUID NOT NULL PRIMARY KEY, + last_drained_blueprint_id UUID, + state omicron.public.db_metadata_nexus_state NOT NULL +); + +CREATE UNIQUE INDEX IF NOT EXISTS lookup_db_metadata_nexus_by_state on omicron.public.db_metadata_nexus ( + state, + nexus_id +); + +-- Keep this at the end of file so that the database does not contain a version +-- until it is fully populated. INSERT INTO omicron.public.db_metadata ( singleton, time_created, @@ -6564,7 +6590,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '184.0.0', NULL) + (TRUE, NOW(), NOW(), '185.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/populate-db-metadata-nexus/up01.sql b/schema/crdb/populate-db-metadata-nexus/up01.sql new file mode 100644 index 00000000000..25c42761e04 --- /dev/null +++ b/schema/crdb/populate-db-metadata-nexus/up01.sql @@ -0,0 +1,6 @@ +CREATE TYPE IF NOT EXISTS omicron.public.db_metadata_nexus_state AS ENUM ( + 'active', + 'not_yet', + 'quiesced' +); + diff --git a/schema/crdb/populate-db-metadata-nexus/up02.sql b/schema/crdb/populate-db-metadata-nexus/up02.sql new file mode 100644 index 00000000000..9fac217eec4 --- /dev/null +++ b/schema/crdb/populate-db-metadata-nexus/up02.sql @@ -0,0 +1,11 @@ +-- Nexuses which may be attempting to access the database, and a state +-- which identifies if they should be allowed to do so. +-- +-- This table is used during upgrade implement handoff between old and new +-- Nexus zones. +CREATE TABLE IF NOT EXISTS omicron.public.db_metadata_nexus ( + nexus_id UUID NOT NULL PRIMARY KEY, + last_drained_blueprint_id UUID, + state omicron.public.db_metadata_nexus_state NOT NULL +); + diff --git a/schema/crdb/populate-db-metadata-nexus/up03.sql b/schema/crdb/populate-db-metadata-nexus/up03.sql new file mode 100644 index 00000000000..42fbf004137 --- /dev/null +++ b/schema/crdb/populate-db-metadata-nexus/up03.sql @@ -0,0 +1,4 @@ +CREATE UNIQUE INDEX IF NOT EXISTS lookup_db_metadata_nexus_by_state on omicron.public.db_metadata_nexus ( + state, + nexus_id +); diff --git a/schema/crdb/populate-db-metadata-nexus/up04.sql b/schema/crdb/populate-db-metadata-nexus/up04.sql new file mode 100644 index 00000000000..36b876b9cdd --- /dev/null +++ b/schema/crdb/populate-db-metadata-nexus/up04.sql @@ -0,0 +1,16 @@ +-- Populate db_metadata_nexus records for all Nexus zones in the current target blueprint +-- +-- This migration handles backfill for existing deployments that are upgrading +-- to include db_metadata_nexus. It finds all Nexus zones in the current +-- target blueprint and marks them as 'active' in the db_metadata_nexus table. + +SET LOCAL disallow_full_table_scans = off; + +INSERT INTO omicron.public.db_metadata_nexus (nexus_id, last_drained_blueprint_id, state) +SELECT bz.id, NULL, 'active' +FROM omicron.public.bp_target bt +JOIN omicron.public.bp_omicron_zone bz ON bt.blueprint_id = bz.blueprint_id +WHERE bz.zone_type = 'nexus' + AND bz.disposition != 'expunged' + AND bt.version = (SELECT MAX(version) FROM omicron.public.bp_target) +ON CONFLICT (nexus_id) DO NOTHING; From faa0b5dd4876562ad3e689afb749854959d29129 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 29 Aug 2025 13:47:34 -0700 Subject: [PATCH 2/2] non-fatal step, executioncomponent order --- nexus/reconfigurator/execution/src/lib.rs | 16 ++++++++++------ nexus/types/src/deployment/execution/spec.rs | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/nexus/reconfigurator/execution/src/lib.rs b/nexus/reconfigurator/execution/src/lib.rs index 5060a037e22..adfda2e5958 100644 --- a/nexus/reconfigurator/execution/src/lib.rs +++ b/nexus/reconfigurator/execution/src/lib.rs @@ -409,13 +409,17 @@ fn register_deploy_db_metadata_nexus_records_step<'a>( .new_step( ExecutionStepId::Ensure, "Ensure db_metadata_nexus_state records exist", - async move |_cx| { - database::deploy_db_metadata_nexus_records( - opctx, &datastore, &blueprint, + async move |_cx| match database::deploy_db_metadata_nexus_records( + opctx, &datastore, &blueprint, + ) + .await + { + Ok(()) => StepSuccess::new(()).into(), + Err(err) => StepWarning::new( + (), + err.context("ensuring db_metadata_nexus_state").to_string(), ) - .await - .context("ensuring db_metadata_nexus_state")?; - StepSuccess::new(()).into() + .into(), }, ) .register(); diff --git a/nexus/types/src/deployment/execution/spec.rs b/nexus/types/src/deployment/execution/spec.rs index 27d85b431db..df02d3c58a3 100644 --- a/nexus/types/src/deployment/execution/spec.rs +++ b/nexus/types/src/deployment/execution/spec.rs @@ -30,8 +30,8 @@ pub enum ExecutionComponent { ExternalNetworking, SupportBundles, SledList, - SledAgent, DeployNexusRecords, + SledAgent, PhysicalDisks, OmicronZones, FirewallRules,