diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index a08fa9519c..ba2c8aea09 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -250,8 +250,6 @@ enum DbCommands { Inventory(InventoryArgs), /// Save the current Reconfigurator inputs to a file ReconfiguratorSave(ReconfiguratorSaveArgs), - /// Print information about control plane services - Services(ServicesArgs), /// Print information about sleds Sleds, /// Print information about customer instances @@ -398,20 +396,6 @@ struct ReconfiguratorSaveArgs { output_file: Utf8PathBuf, } -#[derive(Debug, Args)] -struct ServicesArgs { - #[command(subcommand)] - command: ServicesCommands, -} - -#[derive(Debug, Subcommand)] -enum ServicesCommands { - /// List service instances - ListInstances, - /// List service instances, grouped by sled - ListBySled, -} - #[derive(Debug, Args)] struct NetworkArgs { #[command(subcommand)] @@ -520,26 +504,6 @@ impl DbArgs { ) .await } - DbCommands::Services(ServicesArgs { - command: ServicesCommands::ListInstances, - }) => { - cmd_db_services_list_instances( - &opctx, - &datastore, - &self.fetch_opts, - ) - .await - } - DbCommands::Services(ServicesArgs { - command: ServicesCommands::ListBySled, - }) => { - cmd_db_services_list_by_sled( - &opctx, - &datastore, - &self.fetch_opts, - ) - .await - } DbCommands::Sleds => { cmd_db_sleds(&opctx, &datastore, &self.fetch_opts).await } @@ -697,41 +661,11 @@ struct ServiceInfo { /// Helper function to look up the service with the given ID. /// -/// Requires the caller to first have fetched the current target blueprint, so -/// we can find services that have been added by Reconfigurator. +/// Requires the caller to first have fetched the current target blueprint. async fn lookup_service_info( - datastore: &DataStore, service_id: Uuid, - current_target_blueprint: Option<&Blueprint>, + blueprint: &Blueprint, ) -> anyhow::Result> { - let conn = datastore.pool_connection_for_tests().await?; - - // We need to check the `service` table (populated during rack setup)... - { - use db::schema::service::dsl; - if let Some(kind) = dsl::service - .filter(dsl::id.eq(service_id)) - .limit(1) - .select(dsl::kind) - .get_result_async(&*conn) - .await - .optional() - .with_context(|| format!("loading service {service_id}"))? - { - // XXX: the services table is going to go away soon! - return Ok(Some(ServiceInfo { - service_kind: kind, - disposition: BlueprintZoneDisposition::InService, - })); - } - } - - // ...and if we don't find the service, check the latest blueprint, because - // the service might have been added by Reconfigurator after RSS ran. - let Some(blueprint) = current_target_blueprint else { - return Ok(None); - }; - let Some(zone_config) = blueprint .all_blueprint_zones(BlueprintZoneFilter::All) .find_map(|(_sled_id, zone_config)| { @@ -1466,61 +1400,6 @@ async fn cmd_db_snapshot_info( Ok(()) } -/// Run `omdb db services list-instances`. -async fn cmd_db_services_list_instances( - opctx: &OpContext, - datastore: &DataStore, - fetch_opts: &DbFetchOptions, -) -> Result<(), anyhow::Error> { - let limit = fetch_opts.fetch_limit; - let sled_list = datastore - .sled_list(&opctx, &first_page(limit)) - .await - .context("listing sleds")?; - check_limit(&sled_list, limit, || String::from("listing sleds")); - - let sleds: BTreeMap = - sled_list.into_iter().map(|s| (s.id(), s)).collect(); - - let mut rows = vec![]; - - for service_kind in ServiceKind::iter() { - let context = - || format!("listing instances of kind {:?}", service_kind); - let instances = datastore - .services_list_kind(&opctx, service_kind, &first_page(limit)) - .await - .with_context(&context)?; - check_limit(&instances, limit, &context); - - rows.extend(instances.into_iter().map(|instance| { - let addr = - std::net::SocketAddrV6::new(*instance.ip, *instance.port, 0, 0) - .to_string(); - - ServiceInstanceRow { - kind: format!("{:?}", service_kind), - instance_id: instance.id(), - addr, - sled_serial: sleds - .get(&instance.sled_id) - .map(|s| s.serial_number()) - .unwrap_or("unknown") - .to_string(), - } - })); - } - - let table = tabled::Table::new(rows) - .with(tabled::settings::Style::empty()) - .with(tabled::settings::Padding::new(0, 1, 0, 0)) - .to_string(); - - println!("{}", table); - - Ok(()) -} - // SLEDS #[derive(Tabled)] @@ -1532,63 +1411,6 @@ struct ServiceInstanceSledRow { addr: String, } -/// Run `omdb db services list-by-sled`. -async fn cmd_db_services_list_by_sled( - opctx: &OpContext, - datastore: &DataStore, - fetch_opts: &DbFetchOptions, -) -> Result<(), anyhow::Error> { - let limit = fetch_opts.fetch_limit; - let sled_list = datastore - .sled_list(&opctx, &first_page(limit)) - .await - .context("listing sleds")?; - check_limit(&sled_list, limit, || String::from("listing sleds")); - - let sleds: BTreeMap = - sled_list.into_iter().map(|s| (s.id(), s)).collect(); - let mut services_by_sled: BTreeMap> = - BTreeMap::new(); - - for service_kind in ServiceKind::iter() { - let context = - || format!("listing instances of kind {:?}", service_kind); - let instances = datastore - .services_list_kind(&opctx, service_kind, &first_page(limit)) - .await - .with_context(&context)?; - check_limit(&instances, limit, &context); - - for i in instances { - let addr = - std::net::SocketAddrV6::new(*i.ip, *i.port, 0, 0).to_string(); - let sled_instances = - services_by_sled.entry(i.sled_id).or_insert_with(Vec::new); - sled_instances.push(ServiceInstanceSledRow { - kind: format!("{:?}", service_kind), - instance_id: i.id(), - addr, - }) - } - } - - for (sled_id, instances) in services_by_sled { - println!( - "sled: {} (id {})\n", - sleds.get(&sled_id).map(|s| s.serial_number()).unwrap_or("unknown"), - sled_id, - ); - let table = tabled::Table::new(instances) - .with(tabled::settings::Style::empty()) - .with(tabled::settings::Padding::new(0, 1, 0, 0)) - .to_string(); - println!("{}", textwrap::indent(&table.to_string(), " ")); - println!(""); - } - - Ok(()) -} - #[derive(Tabled)] #[tabled(rename_all = "SCREAMING_SNAKE_CASE")] struct SledRow { @@ -2052,19 +1874,17 @@ async fn cmd_db_eips( let mut rows = Vec::new(); - let current_target_blueprint = datastore + let (_, current_target_blueprint) = datastore .blueprint_target_get_current_full(opctx) .await - .context("loading current target blueprint")? - .map(|(_, blueprint)| blueprint); + .context("loading current target blueprint")?; for ip in &ips { let owner = if let Some(owner_id) = ip.parent_id { if ip.is_service { let (kind, disposition) = match lookup_service_info( - datastore, owner_id, - current_target_blueprint.as_ref(), + ¤t_target_blueprint, ) .await? { diff --git a/dev-tools/omdb/tests/successes.out b/dev-tools/omdb/tests/successes.out index 481df1c6e6..942a5338fb 100644 --- a/dev-tools/omdb/tests/successes.out +++ b/dev-tools/omdb/tests/successes.out @@ -69,45 +69,6 @@ note: database schema version matches expected () assembling reconfigurator state ... done wrote ============================================= -EXECUTING COMMAND: omdb ["db", "services", "list-instances"] -termination: Exited(0) ---------------------------------------------- -stdout: -SERVICE INSTANCE_ID ADDR SLED_SERIAL -CruciblePantry ..................... [::1]:REDACTED_PORT sim-b6d65341 -ExternalDns ..................... [::1]:REDACTED_PORT sim-b6d65341 -InternalDns ..................... [::1]:REDACTED_PORT sim-b6d65341 -Nexus ..................... [::ffff:127.0.0.1]:REDACTED_PORT sim-b6d65341 -Mgd ..................... [::1]:REDACTED_PORT sim-039be560 -Mgd ..................... [::1]:REDACTED_PORT sim-b6d65341 ---------------------------------------------- -stderr: -note: using database URL postgresql://root@[::1]:REDACTED_PORT/omicron?sslmode=disable -note: database schema version matches expected () -============================================= -EXECUTING COMMAND: omdb ["db", "services", "list-by-sled"] -termination: Exited(0) ---------------------------------------------- -stdout: -sled: sim-039be560 (id .....................) - - SERVICE INSTANCE_ID ADDR - Mgd ..................... [::1]:REDACTED_PORT - -sled: sim-b6d65341 (id .....................) - - SERVICE INSTANCE_ID ADDR - CruciblePantry ..................... [::1]:REDACTED_PORT - ExternalDns ..................... [::1]:REDACTED_PORT - InternalDns ..................... [::1]:REDACTED_PORT - Nexus ..................... [::ffff:127.0.0.1]:REDACTED_PORT - Mgd ..................... [::1]:REDACTED_PORT - ---------------------------------------------- -stderr: -note: using database URL postgresql://root@[::1]:REDACTED_PORT/omicron?sslmode=disable -note: database schema version matches expected () -============================================= EXECUTING COMMAND: omdb ["db", "sleds"] termination: Exited(0) --------------------------------------------- @@ -385,7 +346,7 @@ task: "blueprint_loader" currently executing: no last completed activation: iter 2, triggered by an explicit signal started at (s ago) and ran for ms -warning: unknown background task: "blueprint_loader" (don't know how to interpret details: Object {"status": String("no target blueprint")}) + last completion reported error: failed to read target blueprint: Internal Error: no target blueprint set task: "blueprint_executor" configured period: every 10m diff --git a/dev-tools/omdb/tests/test_all_output.rs b/dev-tools/omdb/tests/test_all_output.rs index ca24637040..5f64f9c567 100644 --- a/dev-tools/omdb/tests/test_all_output.rs +++ b/dev-tools/omdb/tests/test_all_output.rs @@ -46,7 +46,6 @@ async fn test_omdb_usage_errors() { &["db", "dns"], &["db", "dns", "diff"], &["db", "dns", "names"], - &["db", "services"], &["db", "snapshots"], &["db", "network"], &["mgs"], @@ -90,8 +89,6 @@ async fn test_omdb_success_cases(cptestctx: &ControlPlaneTestContext) { &["db", "dns", "names", "external", "2"], &["db", "instances"], &["db", "reconfigurator-save", tmppath.as_str()], - &["db", "services", "list-instances"], - &["db", "services", "list-by-sled"], &["db", "sleds"], &["mgs", "inventory"], &["nexus", "background-tasks", "doc"], diff --git a/dev-tools/omdb/tests/usage_errors.out b/dev-tools/omdb/tests/usage_errors.out index b704982266..4fbda8f4f4 100644 --- a/dev-tools/omdb/tests/usage_errors.out +++ b/dev-tools/omdb/tests/usage_errors.out @@ -99,7 +99,6 @@ Commands: dns Print information about internal and external DNS inventory Print information about collected hardware/software inventory reconfigurator-save Save the current Reconfigurator inputs to a file - services Print information about control plane services sleds Print information about sleds instances Print information about customer instances network Print information about the network @@ -129,7 +128,6 @@ Commands: dns Print information about internal and external DNS inventory Print information about collected hardware/software inventory reconfigurator-save Save the current Reconfigurator inputs to a file - services Print information about control plane services sleds Print information about sleds instances Print information about customer instances network Print information about the network @@ -213,24 +211,6 @@ Usage: omdb db dns names For more information, try '--help'. ============================================= -EXECUTING COMMAND: omdb ["db", "services"] -termination: Exited(2) ---------------------------------------------- -stdout: ---------------------------------------------- -stderr: -Print information about control plane services - -Usage: omdb db services - -Commands: - list-instances List service instances - list-by-sled List service instances, grouped by sled - help Print this message or the help of the given subcommand(s) - -Options: - -h, --help Print help -============================================= EXECUTING COMMAND: omdb ["db", "snapshots"] termination: Exited(2) --------------------------------------------- diff --git a/nexus/db-model/src/lib.rs b/nexus/db-model/src/lib.rs index a2e9565d46..6495a0c960 100644 --- a/nexus/db-model/src/lib.rs +++ b/nexus/db-model/src/lib.rs @@ -70,7 +70,6 @@ mod role_builtin; pub mod saga_types; pub mod schema; mod schema_versions; -mod service; mod service_kind; mod silo; mod silo_group; @@ -165,7 +164,6 @@ pub use role_assignment::*; pub use role_builtin::*; pub use schema_versions::*; pub use semver_version::*; -pub use service::*; pub use service_kind::*; pub use silo::*; pub use silo_group::*; diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index b02a8677d4..07b8ce851e 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -875,20 +875,6 @@ table! { } } -table! { - service (id) { - id -> Uuid, - time_created -> Timestamptz, - time_modified -> Timestamptz, - - sled_id -> Uuid, - zone_id -> Nullable, - ip -> Inet, - port -> Int4, - kind -> crate::ServiceKindEnum, - } -} - table! { physical_disk (id) { id -> Uuid, @@ -1691,7 +1677,6 @@ allow_tables_to_appear_in_same_query!( silo, identity_provider, console_session, - service, sled, sled_resource, router_route, @@ -1707,7 +1692,6 @@ allow_tables_to_appear_in_same_query!( ); allow_tables_to_appear_in_same_query!(dns_zone, dns_version, dns_name); -allow_tables_to_appear_in_same_query!(external_ip, service); // used for query to check whether an IP pool association has any allocated IPs before deleting allow_tables_to_appear_in_same_query!(external_ip, instance); diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index ad43cf77c5..86c7d618aa 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(52, 0, 0); +pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(53, 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(53, "drop-service-table"), KnownVersion::new(52, "blueprint-physical-disk"), KnownVersion::new(51, "blueprint-disposition-column"), KnownVersion::new(50, "add-lookup-disk-by-volume-id-index"), diff --git a/nexus/db-model/src/service.rs b/nexus/db-model/src/service.rs deleted file mode 100644 index 45d3ca5a16..0000000000 --- a/nexus/db-model/src/service.rs +++ /dev/null @@ -1,44 +0,0 @@ -// 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/. - -use super::ServiceKind; -use crate::ipv6; -use crate::schema::service; -use crate::SqlU16; -use db_macros::Asset; -use std::net::SocketAddrV6; -use uuid::Uuid; - -/// Representation of services which may run on Sleds. -#[derive(Queryable, Insertable, Debug, Clone, Selectable, Asset)] -#[diesel(table_name = service)] -pub struct Service { - #[diesel(embed)] - identity: ServiceIdentity, - - pub sled_id: Uuid, - pub zone_id: Option, - pub ip: ipv6::Ipv6Addr, - pub port: SqlU16, - pub kind: ServiceKind, -} - -impl Service { - pub fn new( - id: Uuid, - sled_id: Uuid, - zone_id: Option, - addr: SocketAddrV6, - kind: ServiceKind, - ) -> Self { - Self { - identity: ServiceIdentity::new(id), - sled_id, - zone_id, - ip: addr.ip().into(), - port: addr.port().into(), - kind, - } - } -} diff --git a/nexus/db-model/src/sled.rs b/nexus/db-model/src/sled.rs index 1fa436c992..e94da5fbbe 100644 --- a/nexus/db-model/src/sled.rs +++ b/nexus/db-model/src/sled.rs @@ -5,7 +5,7 @@ use super::{ByteCount, Generation, SledState, SqlU16, SqlU32}; use crate::collection::DatastoreCollectionConfig; use crate::ipv6; -use crate::schema::{physical_disk, service, sled, zpool}; +use crate::schema::{physical_disk, sled, zpool}; use crate::sled::shared::Baseboard; use crate::sled_policy::DbSledPolicy; use chrono::{DateTime, Utc}; @@ -177,13 +177,6 @@ impl DatastoreCollectionConfig for Sled { type CollectionIdColumn = zpool::dsl::sled_id; } -impl DatastoreCollectionConfig for Sled { - type CollectionId = Uuid; - type GenerationNumberColumn = sled::dsl::rcgen; - type CollectionTimeDeletedColumn = sled::dsl::time_deleted; - type CollectionIdColumn = service::dsl::sled_id; -} - /// Form of `Sled` used for updates from sled-agent. This is missing some /// columns that are present in `Sled` because sled-agent doesn't control them. #[derive(Debug, Clone)] diff --git a/nexus/db-model/src/sled_resource_kind.rs b/nexus/db-model/src/sled_resource_kind.rs index c17eb2e106..b9a59bdc30 100644 --- a/nexus/db-model/src/sled_resource_kind.rs +++ b/nexus/db-model/src/sled_resource_kind.rs @@ -15,8 +15,5 @@ impl_enum_type!( pub enum SledResourceKind; // Enum values - Dataset => b"dataset" - Service => b"service" Instance => b"instance" - Reserved => b"reserved" ); diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index fa6673842a..d31428e319 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -598,15 +598,13 @@ impl DataStore { // current target. let current_target = self.blueprint_current_target_only(&conn).await?; - if let Some(current_target) = current_target { - if current_target.target_id == blueprint_id { - return Err(TransactionError::CustomError( - Error::conflict(format!( - "blueprint {blueprint_id} is the \ - current target and cannot be deleted", - )), - )); - } + if current_target.target_id == blueprint_id { + return Err(TransactionError::CustomError( + Error::conflict(format!( + "blueprint {blueprint_id} is the \ + current target and cannot be deleted", + )), + )); } // Remove the record describing the blueprint itself. @@ -848,14 +846,11 @@ impl DataStore { pub async fn blueprint_target_get_current_full( &self, opctx: &OpContext, - ) -> Result, Error> { + ) -> Result<(BlueprintTarget, Blueprint), Error> { opctx.authorize(authz::Action::Read, &authz::BLUEPRINT_CONFIG).await?; let conn = self.pool_connection_authorized(opctx).await?; - let Some(target) = self.blueprint_current_target_only(&conn).await? - else { - return Ok(None); - }; + let target = self.blueprint_current_target_only(&conn).await?; // The blueprint for the current target cannot be deleted while it is // the current target, but it's possible someone else (a) made a new @@ -866,14 +861,14 @@ impl DataStore { let authz_blueprint = authz_blueprint_from_id(target.target_id); let blueprint = self.blueprint_read(opctx, &authz_blueprint).await?; - Ok(Some((target, blueprint))) + Ok((target, blueprint)) } /// Get the current target blueprint, if one exists pub async fn blueprint_target_get_current( &self, opctx: &OpContext, - ) -> Result, Error> { + ) -> Result { opctx.authorize(authz::Action::Read, &authz::BLUEPRINT_CONFIG).await?; let conn = self.pool_connection_authorized(opctx).await?; self.blueprint_current_target_only(&conn).await @@ -886,7 +881,7 @@ impl DataStore { async fn blueprint_current_target_only( &self, conn: &async_bb8_diesel::Connection, - ) -> Result, Error> { + ) -> Result { use db::schema::bp_target::dsl; let current_target = dsl::bp_target @@ -896,7 +891,16 @@ impl DataStore { .optional() .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; - Ok(current_target.map(BlueprintTarget::from)) + // We expect a target blueprint to be set on all systems. RSS sets an + // initial blueprint, but we shipped systems before it did so. We added + // target blueprints to those systems via support operations, but let's + // be careful here and return a specific error for this case. + let current_target = + current_target.ok_or_else(|| Error::InternalError { + internal_message: "no target blueprint set".to_string(), + })?; + + Ok(current_target.into()) } } @@ -1483,10 +1487,12 @@ mod tests { datastore.blueprint_insert(&opctx, &blueprint1).await.unwrap_err(); assert!(err.to_string().contains("duplicate key")); - // Delete the blueprint and ensure it's really gone. - datastore.blueprint_delete(&opctx, &authz_blueprint).await.unwrap(); - ensure_blueprint_fully_deleted(&datastore, blueprint1.id).await; - assert_eq!(blueprint_list_all_ids(&opctx, &datastore).await, []); + // We could try to test deleting this blueprint, but deletion checks + // that the blueprint being deleted isn't the current target, and we + // haven't set a current target at all as part of this test. Instead of + // going through the motions of creating another blueprint and making it + // the target just to test deletion, we'll end this test here, and rely + // on other tests to check blueprint deletion. // Clean up. db.cleanup().await.unwrap(); @@ -1549,7 +1555,7 @@ mod tests { .unwrap(); assert_eq!( datastore.blueprint_target_get_current_full(&opctx).await.unwrap(), - Some((bp1_target, blueprint1.clone())) + (bp1_target, blueprint1.clone()) ); let err = datastore .blueprint_delete(&opctx, &authz_blueprint1) @@ -1683,7 +1689,7 @@ mod tests { .unwrap(); assert_eq!( datastore.blueprint_target_get_current_full(&opctx).await.unwrap(), - Some((bp2_target, blueprint2.clone())) + (bp2_target, blueprint2.clone()) ); let err = datastore .blueprint_delete(&opctx, &authz_blueprint2) @@ -1739,11 +1745,14 @@ mod tests { )) ); - // There should be no current target still. - assert_eq!( - datastore.blueprint_target_get_current_full(&opctx).await.unwrap(), - None - ); + // There should be no current target; this is never expected in a real + // system, since RSS sets an initial target blueprint, so we should get + // an error. + let err = datastore + .blueprint_target_get_current_full(&opctx) + .await + .unwrap_err(); + assert!(err.to_string().contains("no target blueprint set")); // Create three blueprints: // * `blueprint1` has no parent @@ -1812,11 +1821,14 @@ mod tests { Error::from(InsertTargetError::ParentNotTarget(blueprint2.id)) ); - // There should be no current target still. - assert_eq!( - datastore.blueprint_target_get_current_full(&opctx).await.unwrap(), - None - ); + // There should be no current target; this is never expected in a real + // system, since RSS sets an initial target blueprint, so we should get + // an error. + let err = datastore + .blueprint_target_get_current_full(&opctx) + .await + .unwrap_err(); + assert!(err.to_string().contains("no target blueprint set")); // We should be able to insert blueprint1, which has no parent (matching // the currently-empty `bp_target` table's lack of a target). @@ -1826,7 +1838,7 @@ mod tests { .unwrap(); assert_eq!( datastore.blueprint_target_get_current_full(&opctx).await.unwrap(), - Some((bp1_target, blueprint1.clone())) + (bp1_target, blueprint1.clone()) ); // Now that blueprint1 is the current target, we should be able to @@ -1837,7 +1849,7 @@ mod tests { .unwrap(); assert_eq!( datastore.blueprint_target_get_current_full(&opctx).await.unwrap(), - Some((bp3_target, blueprint3.clone())) + (bp3_target, blueprint3.clone()) ); // Now that blueprint3 is the target, trying to insert blueprint1 or @@ -1883,7 +1895,7 @@ mod tests { .unwrap(); assert_eq!( datastore.blueprint_target_get_current_full(&opctx).await.unwrap(), - Some((bp4_target, blueprint4)) + (bp4_target, blueprint4) ); // Clean up. @@ -1942,7 +1954,7 @@ mod tests { .unwrap(); assert_eq!( datastore.blueprint_target_get_current(&opctx).await.unwrap(), - Some(bp1_target), + bp1_target, ); // We should be able to toggle its enabled status an arbitrary number of @@ -1955,7 +1967,7 @@ mod tests { .unwrap(); assert_eq!( datastore.blueprint_target_get_current(&opctx).await.unwrap(), - Some(bp1_target), + bp1_target, ); } @@ -1976,7 +1988,7 @@ mod tests { .unwrap(); assert_eq!( datastore.blueprint_target_get_current(&opctx).await.unwrap(), - Some(bp2_target), + bp2_target, ); // We can no longer toggle the enabled bit of bp1_target. @@ -1997,7 +2009,7 @@ mod tests { .unwrap(); assert_eq!( datastore.blueprint_target_get_current(&opctx).await.unwrap(), - Some(bp2_target), + bp2_target, ); } diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index 6e8eecb8ed..4d6b16483d 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -79,7 +79,6 @@ mod region; mod region_snapshot; mod role; mod saga; -mod service; mod silo; mod silo_group; mod silo_user; @@ -385,8 +384,8 @@ mod test { use crate::db::model::{ BlockSize, ConsoleSession, Dataset, DatasetKind, ExternalIp, PhysicalDisk, PhysicalDiskKind, PhysicalDiskPolicy, PhysicalDiskState, - Project, Rack, Region, Service, ServiceKind, SiloUser, SledBaseboard, - SledSystemHardware, SledUpdate, SshKey, VpcSubnet, Zpool, + Project, Rack, Region, SiloUser, SledBaseboard, SledSystemHardware, + SledUpdate, SshKey, VpcSubnet, Zpool, }; use crate::db::queries::vpc_subnet::FilterConflictingVpcSubnetRangesQuery; use chrono::{Duration, Utc}; @@ -397,7 +396,6 @@ mod test { use nexus_db_model::{to_db_typed_uuid, Generation}; use nexus_test_utils::db::test_setup_database; use nexus_types::external_api::params; - use omicron_common::api::external::DataPageParams; use omicron_common::api::external::{ ByteCount, Error, IdentityMetadataCreateParams, LookupType, Name, }; @@ -408,7 +406,6 @@ mod test { use std::collections::HashMap; use std::collections::HashSet; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddrV6}; - use std::num::NonZeroU32; use std::sync::Arc; use strum::EnumCount; use uuid::Uuid; @@ -1746,130 +1743,6 @@ mod test { logctx.cleanup_successful(); } - #[tokio::test] - async fn test_service_upsert_and_list() { - let logctx = dev::test_setup_log("test_service_upsert_and_list"); - let mut db = test_setup_database(&logctx.log).await; - let (opctx, datastore) = datastore_test(&logctx, &db).await; - - // Create a sled on which the service should exist. - let sled_id = create_test_sled(&datastore).await.into_untyped_uuid(); - - // Create a few new service to exist on this sled. - let service1_id = - "ab7bd7fd-7c37-48ab-a84a-9c09a90c4c7f".parse().unwrap(); - let addr = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0); - let kind = ServiceKind::Nexus; - - let service1 = - Service::new(service1_id, sled_id, Some(service1_id), addr, kind); - let result = - datastore.service_upsert(&opctx, service1.clone()).await.unwrap(); - assert_eq!(service1.id(), result.id()); - assert_eq!(service1.ip, result.ip); - assert_eq!(service1.kind, result.kind); - - let service2_id = - "fe5b6e3d-dfee-47b4-8719-c54f78912c0b".parse().unwrap(); - let service2 = Service::new(service2_id, sled_id, None, addr, kind); - let result = - datastore.service_upsert(&opctx, service2.clone()).await.unwrap(); - assert_eq!(service2.id(), result.id()); - assert_eq!(service2.ip, result.ip); - assert_eq!(service2.kind, result.kind); - - let service3_id = Uuid::new_v4(); - let kind = ServiceKind::Oximeter; - let service3 = Service::new( - service3_id, - sled_id, - Some(Uuid::new_v4()), - addr, - kind, - ); - let result = - datastore.service_upsert(&opctx, service3.clone()).await.unwrap(); - assert_eq!(service3.id(), result.id()); - assert_eq!(service3.ip, result.ip); - assert_eq!(service3.kind, result.kind); - - // Try listing services of one kind. - let services = datastore - .services_list_kind( - &opctx, - ServiceKind::Nexus, - &DataPageParams { - marker: None, - direction: dropshot::PaginationOrder::Ascending, - limit: NonZeroU32::new(3).unwrap(), - }, - ) - .await - .unwrap(); - assert_eq!(services[0].id(), service1.id()); - assert_eq!(services[0].sled_id, service1.sled_id); - assert_eq!(services[0].zone_id, service1.zone_id); - assert_eq!(services[0].kind, service1.kind); - assert_eq!(services[1].id(), service2.id()); - assert_eq!(services[1].sled_id, service2.sled_id); - assert_eq!(services[1].zone_id, service2.zone_id); - assert_eq!(services[1].kind, service2.kind); - assert_eq!(services.len(), 2); - - // Try listing services of a different kind. - let services = datastore - .services_list_kind( - &opctx, - ServiceKind::Oximeter, - &DataPageParams { - marker: None, - direction: dropshot::PaginationOrder::Ascending, - limit: NonZeroU32::new(3).unwrap(), - }, - ) - .await - .unwrap(); - assert_eq!(services[0].id(), service3.id()); - assert_eq!(services[0].sled_id, service3.sled_id); - assert_eq!(services[0].zone_id, service3.zone_id); - assert_eq!(services[0].kind, service3.kind); - assert_eq!(services.len(), 1); - - // Try listing services of a kind for which there are no services. - let services = datastore - .services_list_kind( - &opctx, - ServiceKind::Dendrite, - &DataPageParams { - marker: None, - direction: dropshot::PaginationOrder::Ascending, - limit: NonZeroU32::new(3).unwrap(), - }, - ) - .await - .unwrap(); - assert!(services.is_empty()); - - // As a quick check, try supplying a marker. - let services = datastore - .services_list_kind( - &opctx, - ServiceKind::Nexus, - &DataPageParams { - marker: Some(&service1_id), - direction: dropshot::PaginationOrder::Ascending, - limit: NonZeroU32::new(3).unwrap(), - }, - ) - .await - .unwrap(); - assert_eq!(services.len(), 1); - assert_eq!(services[0].id(), service2.id()); - - db.cleanup().await.unwrap(); - logctx.cleanup_successful(); - } - #[tokio::test] async fn test_rack_initialize_is_idempotent() { let logctx = dev::test_setup_log("test_rack_initialize_is_idempotent"); diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index ece9112745..3dff04cc11 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -35,7 +35,6 @@ use diesel::prelude::*; use diesel::result::Error as DieselError; use diesel::upsert::excluded; use ipnetwork::IpNetwork; -use nexus_db_model::ExternalIp; use nexus_db_model::IncompleteNetworkInterface; use nexus_db_model::InitialDnsGroup; use nexus_db_model::PasswordHashString; @@ -44,13 +43,15 @@ use nexus_db_model::SiloUserPasswordHash; use nexus_db_model::SledUnderlaySubnetAllocation; use nexus_types::deployment::Blueprint; use nexus_types::deployment::BlueprintTarget; +use nexus_types::deployment::BlueprintZoneFilter; +use nexus_types::deployment::OmicronZoneConfig; +use nexus_types::deployment::OmicronZoneType; use nexus_types::external_api::params as external_params; use nexus_types::external_api::shared; use nexus_types::external_api::shared::IdentityType; use nexus_types::external_api::shared::IpRange; use nexus_types::external_api::shared::SiloRole; use nexus_types::identity::Resource; -use nexus_types::internal_api::params as internal_params; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::Error; use omicron_common::api::external::IdentityMetadataCreateParams; @@ -60,7 +61,6 @@ use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; use omicron_common::bail_unless; use slog_error_chain::InlineErrorChain; -use std::net::IpAddr; use std::sync::{Arc, OnceLock}; use uuid::Uuid; @@ -70,7 +70,6 @@ pub struct RackInit { pub rack_id: Uuid, pub rack_subnet: IpNetwork, pub blueprint: Blueprint, - pub services: Vec, pub physical_disks: Vec, pub zpools: Vec, pub datasets: Vec, @@ -91,7 +90,6 @@ enum RackInitError { AddingNic(Error), BlueprintInsert(Error), BlueprintTargetSet(Error), - ServiceInsert(Error), DatasetInsert { err: AsyncInsertError, zpool_id: Uuid }, PhysicalDiskInsert(Error), ZpoolInsert(Error), @@ -133,9 +131,6 @@ impl From for Error { }, RackInitError::PhysicalDiskInsert(err) => err, RackInitError::ZpoolInsert(err) => err, - RackInitError::ServiceInsert(err) => Error::internal_error( - &format!("failed to insert Service record: {:#}", err), - ), RackInitError::BlueprintInsert(err) => Error::internal_error( &format!("failed to insert Blueprint: {:#}", err), ), @@ -464,54 +459,64 @@ impl DataStore { Ok(()) } - async fn rack_populate_service_records( + async fn rack_populate_service_networking_records( &self, conn: &async_bb8_diesel::Connection, log: &slog::Logger, service_pool: &db::model::IpPool, - service: internal_params::ServicePutRequest, + zone_config: &OmicronZoneConfig, ) -> Result<(), RackInitError> { - use internal_params::ServiceKind; - - let service_db = db::model::Service::new( - service.service_id, - service.sled_id, - service.zone_id, - service.address, - service.kind.clone().into(), - ); - self.service_upsert_conn(conn, service_db).await.map_err( - |e| match e.retryable() { - Retryable(e) => RackInitError::Retryable(e), - NotRetryable(e) => RackInitError::ServiceInsert(e.into()), - }, - )?; - // For services with external connectivity, we record their // explicit IP allocation and create a service NIC as well. - let service_ip_nic = match service.kind { - ServiceKind::ExternalDns { external_address, ref nic } - | ServiceKind::Nexus { external_address, ref nic } => { + let zone_type = &zone_config.zone_type; + let service_ip_nic = match zone_type { + OmicronZoneType::ExternalDns { nic, .. } + | OmicronZoneType::Nexus { nic, .. } => { + let service_kind = format!("{}", zone_type.kind()); + let external_ip = match zone_type.external_ip() { + Ok(Some(ip)) => ip, + Ok(None) => { + let message = format!( + "missing external IP in blueprint for {} zone {}", + service_kind, zone_config.id + ); + return Err(RackInitError::AddingNic( + Error::internal_error(&message), + )); + } + Err(err) => { + let message = format!( + "error parsing external IP in blueprint for \ + {} zone {}: {err:#}", + service_kind, zone_config.id + ); + return Err(RackInitError::AddingNic( + Error::internal_error(&message), + )); + } + }; let db_ip = IncompleteExternalIp::for_service_explicit( Uuid::new_v4(), &db::model::Name(nic.name.clone()), - &format!("{}", service.kind), - service.service_id, + &service_kind, + zone_config.id, service_pool.id(), - external_address, + external_ip, ); - let vpc_subnet = match service.kind { - ServiceKind::ExternalDns { .. } => DNS_VPC_SUBNET.clone(), - ServiceKind::Nexus { .. } => NEXUS_VPC_SUBNET.clone(), + let vpc_subnet = match zone_type { + OmicronZoneType::ExternalDns { .. } => { + DNS_VPC_SUBNET.clone() + } + OmicronZoneType::Nexus { .. } => NEXUS_VPC_SUBNET.clone(), _ => unreachable!(), }; let db_nic = IncompleteNetworkInterface::new_service( nic.id, - service.service_id, + zone_config.id, vpc_subnet, IdentityMetadataCreateParams { name: nic.name.clone(), - description: format!("{} service vNIC", service.kind), + description: format!("{service_kind} service vNIC"), }, nic.ip, nic.mac, @@ -520,21 +525,24 @@ impl DataStore { .map_err(|e| RackInitError::AddingNic(e))?; Some((db_ip, db_nic)) } - ServiceKind::BoundaryNtp { snat, ref nic } => { + OmicronZoneType::BoundaryNtp { snat_cfg, ref nic, .. } => { let db_ip = IncompleteExternalIp::for_service_explicit_snat( Uuid::new_v4(), - service.service_id, + zone_config.id, service_pool.id(), - snat.ip, - (snat.first_port, snat.last_port), + snat_cfg.ip, + (snat_cfg.first_port, snat_cfg.last_port), ); let db_nic = IncompleteNetworkInterface::new_service( nic.id, - service.service_id, + zone_config.id, NTP_VPC_SUBNET.clone(), IdentityMetadataCreateParams { name: nic.name.clone(), - description: format!("{} service vNIC", service.kind), + description: format!( + "{} service vNIC", + zone_type.kind() + ), }, nic.ip, nic.mac, @@ -543,44 +551,61 @@ impl DataStore { .map_err(|e| RackInitError::AddingNic(e))?; Some((db_ip, db_nic)) } - _ => None, + OmicronZoneType::InternalNtp { .. } + | OmicronZoneType::Clickhouse { .. } + | OmicronZoneType::ClickhouseKeeper { .. } + | OmicronZoneType::CockroachDb { .. } + | OmicronZoneType::Crucible { .. } + | OmicronZoneType::CruciblePantry { .. } + | OmicronZoneType::InternalDns { .. } + | OmicronZoneType::Oximeter { .. } => None, }; - if let Some((db_ip, db_nic)) = service_ip_nic { - Self::allocate_external_ip_on_connection(conn, db_ip) - .await - .map_err(|err| { - error!( - log, - "Initializing Rack: Failed to allocate \ - IP address for {}", - service.kind; - "err" => %err, - ); - match err.retryable() { - Retryable(e) => RackInitError::Retryable(e), - NotRetryable(e) => RackInitError::AddingIp(e.into()), - } - })?; + let Some((db_ip, db_nic)) = service_ip_nic else { + info!( + log, + "No networking records needed for {} service", + zone_type.kind(), + ); + return Ok(()); + }; + Self::allocate_external_ip_on_connection(conn, db_ip).await.map_err( + |err| { + error!( + log, + "Initializing Rack: Failed to allocate \ + IP address for {}", + zone_type.kind(); + "err" => %err, + ); + match err.retryable() { + Retryable(e) => RackInitError::Retryable(e), + NotRetryable(e) => RackInitError::AddingIp(e.into()), + } + }, + )?; - self.create_network_interface_raw_conn(conn, db_nic) - .await - .map(|_| ()) - .or_else(|e| { - use db::queries::network_interface::InsertError; - match e { - InsertError::InterfaceAlreadyExists( - _, - db::model::NetworkInterfaceKind::Service, - ) => Ok(()), - InsertError::Retryable(err) => { - Err(RackInitError::Retryable(err)) - } - _ => Err(RackInitError::AddingNic(e.into_external())), + self.create_network_interface_raw_conn(conn, db_nic) + .await + .map(|_| ()) + .or_else(|e| { + use db::queries::network_interface::InsertError; + match e { + InsertError::InterfaceAlreadyExists( + _, + db::model::NetworkInterfaceKind::Service, + ) => Ok(()), + InsertError::Retryable(err) => { + Err(RackInitError::Retryable(err)) } - })?; - } + _ => Err(RackInitError::AddingNic(e.into_external())), + } + })?; + info!( + log, + "Inserted networking records for {} service", + zone_type.kind(), + ); - info!(log, "Inserted records for {} service", service.kind); Ok(()) } @@ -616,7 +641,6 @@ impl DataStore { async move { let rack_id = rack_init.rack_id; let blueprint = rack_init.blueprint; - let services = rack_init.services; let physical_disks = rack_init.physical_disks; let zpools = rack_init.zpools; let datasets = rack_init.datasets; @@ -719,13 +743,13 @@ impl DataStore { DieselError::RollbackTransaction })?; - // Allocate records for all services. - for service in services { - self.rack_populate_service_records( + // Allocate networking records for all services. + for (_, zone_config) in blueprint.all_omicron_zones(BlueprintZoneFilter::ShouldBeRunning) { + self.rack_populate_service_networking_records( &conn, &log, &service_pool, - service, + zone_config, ) .await .map_err(|e| { @@ -734,7 +758,7 @@ impl DataStore { DieselError::RollbackTransaction })?; } - info!(log, "Inserted services"); + info!(log, "Inserted service networking records"); for physical_disk in physical_disks { if let Err(e) = Self::physical_disk_insert_on_connection(&conn, &opctx, physical_disk) @@ -909,38 +933,6 @@ impl DataStore { Ok(()) } - - // TODO once we eliminate the service table, we can eliminate this function - // and the branch in the sole caller - pub async fn nexus_external_addresses_from_service_table( - &self, - opctx: &OpContext, - ) -> Result, Error> { - opctx.authorize(authz::Action::Read, &authz::DNS_CONFIG).await?; - - use crate::db::schema::external_ip::dsl as extip_dsl; - use crate::db::schema::service::dsl as service_dsl; - - let conn = self.pool_connection_authorized(opctx).await?; - - Ok(extip_dsl::external_ip - .inner_join( - service_dsl::service - .on(service_dsl::id - .eq(extip_dsl::parent_id.assume_not_null())), - ) - .filter(extip_dsl::parent_id.is_not_null()) - .filter(extip_dsl::time_deleted.is_null()) - .filter(extip_dsl::is_service) - .filter(service_dsl::kind.eq(db::model::ServiceKind::Nexus)) - .select(ExternalIp::as_select()) - .get_results_async(&*conn) - .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))? - .into_iter() - .map(|external_ip| external_ip.ip.ip()) - .collect()) - } } #[cfg(test)] @@ -955,28 +947,37 @@ mod test { use crate::db::model::ExternalIp; use crate::db::model::IpKind; use crate::db::model::IpPoolRange; - use crate::db::model::Service; - use crate::db::model::ServiceKind; use crate::db::model::Sled; use async_bb8_diesel::AsyncSimpleConnection; - use internal_params::DnsRecord; use nexus_config::NUM_INITIAL_RESERVED_IP_ADDRESSES; use nexus_db_model::{DnsGroup, Generation, InitialDnsGroup, SledUpdate}; + use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder; + use nexus_reconfigurator_planning::system::{ + SledBuilder, SystemDescription, + }; use nexus_test_utils::db::test_setup_database; + use nexus_types::deployment::OmicronZoneConfig; + use nexus_types::deployment::OmicronZonesConfig; + use nexus_types::deployment::SledFilter; use nexus_types::external_api::shared::SiloIdentityMode; use nexus_types::identity::Asset; - use nexus_types::internal_api::params::ServiceNic; + use nexus_types::internal_api::params::DnsRecord; + use nexus_types::inventory::NetworkInterface; + use nexus_types::inventory::NetworkInterfaceKind; use omicron_common::address::{ DNS_OPTE_IPV4_SUBNET, NEXUS_OPTE_IPV4_SUBNET, NTP_OPTE_IPV4_SUBNET, }; use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_common::api::external::{ - IdentityMetadataCreateParams, MacAddr, + IdentityMetadataCreateParams, MacAddr, Vni, }; use omicron_common::api::internal::shared::SourceNatConfig; use omicron_test_utils::dev; + use omicron_uuid_kinds::TypedUuid; + use omicron_uuid_kinds::{GenericUuid, SledUuid, ZpoolUuid}; + use sled_agent_client::types::OmicronZoneDataset; use std::collections::{BTreeMap, HashMap}; - use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddrV6}; + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV6}; use std::num::NonZeroU32; // Default impl is for tests only, and really just so that tests can more @@ -997,7 +998,6 @@ mod test { creator: "test suite".to_string(), comment: "test suite".to_string(), }, - services: vec![], physical_disks: vec![], zpools: vec![], datasets: vec![], @@ -1212,14 +1212,25 @@ mod test { }; } - fn_to_get_all!(service, Service); fn_to_get_all!(external_ip, ExternalIp); fn_to_get_all!(ip_pool_range, IpPoolRange); fn_to_get_all!(dataset, Dataset); + fn random_dataset() -> OmicronZoneDataset { + OmicronZoneDataset { + pool_name: illumos_utils::zpool::ZpoolName::new_external( + ZpoolUuid::new_v4(), + ) + .to_string() + .parse() + .unwrap(), + } + } + #[tokio::test] async fn rack_set_initialized_with_services() { - let logctx = dev::test_setup_log("rack_set_initialized_with_services"); + let test_name = "rack_set_initialized_with_services"; + let logctx = dev::test_setup_log(test_name); let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; @@ -1233,6 +1244,29 @@ mod test { )) .unwrap()]; + let mut system = SystemDescription::new(); + system + .service_ip_pool_ranges(service_ip_pool_ranges.clone()) + .sled( + SledBuilder::new().id(TypedUuid::from_untyped_uuid(sled1.id())), + ) + .expect("failed to add sled1") + .sled( + SledBuilder::new().id(TypedUuid::from_untyped_uuid(sled2.id())), + ) + .expect("failed to add sled2") + .sled( + SledBuilder::new().id(TypedUuid::from_untyped_uuid(sled3.id())), + ) + .expect("failed to add sled3"); + let planning_input = system + .to_planning_input_builder() + .expect("failed to make planning input") + .build(); + let mut inventory_builder = system + .to_collection_builder() + .expect("failed to make collection builder"); + let external_dns_ip = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4)); let external_dns_pip = DNS_OPTE_IPV4_SUBNET .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 1) @@ -1255,93 +1289,182 @@ mod test { let ntp2_id = Uuid::new_v4(); let ntp3_id = Uuid::new_v4(); let mut macs = MacAddr::iter_system(); - let services = vec![ - internal_params::ServicePutRequest { - service_id: external_dns_id, - sled_id: sled1.id(), - zone_id: Some(external_dns_id), - address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0), - kind: internal_params::ServiceKind::ExternalDns { - external_address: external_dns_ip, - nic: ServiceNic { - id: Uuid::new_v4(), - name: "external-dns".parse().unwrap(), - ip: external_dns_pip.into(), - mac: macs.next().unwrap(), - slot: 0, - }, - }, - }, - internal_params::ServicePutRequest { - service_id: ntp1_id, - sled_id: sled1.id(), - zone_id: Some(ntp1_id), - address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 9090, 0, 0), - kind: internal_params::ServiceKind::BoundaryNtp { - snat: SourceNatConfig { - ip: ntp1_ip, - first_port: 16384, - last_port: 32767, - }, - nic: ServiceNic { - id: Uuid::new_v4(), - name: "ntp1".parse().unwrap(), - ip: ntp1_pip.into(), - mac: macs.next().unwrap(), - slot: 0, - }, + + // Add services for our sleds to the inventory (which will cause them to + // be present in the blueprint we'll generate from it). + inventory_builder + .found_sled_omicron_zones( + "sled1", + SledUuid::from_untyped_uuid(sled1.id()), + OmicronZonesConfig { + generation: Generation::new().next(), + zones: vec![ + OmicronZoneConfig { + id: external_dns_id, + underlay_address: Ipv6Addr::LOCALHOST, + zone_type: OmicronZoneType::ExternalDns { + dataset: random_dataset(), + http_address: "[::1]:80".to_string(), + dns_address: SocketAddr::new( + external_dns_ip, + 53, + ) + .to_string(), + nic: NetworkInterface { + id: Uuid::new_v4(), + kind: NetworkInterfaceKind::Service { + id: external_dns_id, + }, + name: "external-dns".parse().unwrap(), + ip: external_dns_pip.into(), + mac: macs.next().unwrap(), + subnet: IpNetwork::from( + **DNS_OPTE_IPV4_SUBNET, + ) + .into(), + vni: Vni::SERVICES_VNI, + primary: true, + slot: 0, + }, + }, + }, + OmicronZoneConfig { + id: ntp1_id, + underlay_address: Ipv6Addr::LOCALHOST, + zone_type: OmicronZoneType::BoundaryNtp { + address: "[::1]:80".to_string(), + ntp_servers: vec![], + dns_servers: vec![], + domain: None, + nic: NetworkInterface { + id: Uuid::new_v4(), + kind: NetworkInterfaceKind::Service { + id: ntp1_id, + }, + name: "ntp1".parse().unwrap(), + ip: ntp1_pip.into(), + mac: macs.next().unwrap(), + subnet: IpNetwork::from( + **NTP_OPTE_IPV4_SUBNET, + ) + .into(), + vni: Vni::SERVICES_VNI, + primary: true, + slot: 0, + }, + snat_cfg: SourceNatConfig { + ip: ntp1_ip, + first_port: 16384, + last_port: 32767, + }, + }, + }, + ], }, - }, - internal_params::ServicePutRequest { - service_id: nexus_id, - sled_id: sled2.id(), - zone_id: Some(nexus_id), - address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 456, 0, 0), - kind: internal_params::ServiceKind::Nexus { - external_address: nexus_ip, - nic: ServiceNic { - id: Uuid::new_v4(), - name: "nexus".parse().unwrap(), - ip: nexus_pip.into(), - mac: macs.next().unwrap(), - slot: 0, - }, + ) + .expect("recording Omicron zones"); + inventory_builder + .found_sled_omicron_zones( + "sled2", + SledUuid::from_untyped_uuid(sled2.id()), + OmicronZonesConfig { + generation: Generation::new().next(), + zones: vec![ + OmicronZoneConfig { + id: nexus_id, + underlay_address: Ipv6Addr::LOCALHOST, + zone_type: OmicronZoneType::Nexus { + internal_address: "[::1]:80".to_string(), + external_ip: nexus_ip, + external_tls: false, + external_dns_servers: vec![], + nic: NetworkInterface { + id: Uuid::new_v4(), + kind: NetworkInterfaceKind::Service { + id: nexus_id, + }, + name: "nexus".parse().unwrap(), + ip: nexus_pip.into(), + mac: macs.next().unwrap(), + subnet: IpNetwork::from( + **NEXUS_OPTE_IPV4_SUBNET, + ) + .into(), + vni: Vni::SERVICES_VNI, + primary: true, + slot: 0, + }, + }, + }, + OmicronZoneConfig { + id: ntp2_id, + underlay_address: Ipv6Addr::LOCALHOST, + zone_type: OmicronZoneType::BoundaryNtp { + address: "[::1]:80".to_string(), + ntp_servers: vec![], + dns_servers: vec![], + domain: None, + nic: NetworkInterface { + id: Uuid::new_v4(), + kind: NetworkInterfaceKind::Service { + id: ntp2_id, + }, + name: "ntp2".parse().unwrap(), + ip: ntp2_pip.into(), + mac: macs.next().unwrap(), + subnet: IpNetwork::from( + **NTP_OPTE_IPV4_SUBNET, + ) + .into(), + vni: Vni::SERVICES_VNI, + primary: true, + slot: 0, + }, + snat_cfg: SourceNatConfig { + ip: ntp2_ip, + first_port: 0, + last_port: 16383, + }, + }, + }, + ], }, - }, - internal_params::ServicePutRequest { - service_id: ntp2_id, - sled_id: sled2.id(), - zone_id: Some(ntp2_id), - address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 9090, 0, 0), - kind: internal_params::ServiceKind::BoundaryNtp { - snat: SourceNatConfig { - ip: ntp2_ip, - first_port: 0, - last_port: 16383, - }, - nic: ServiceNic { - id: Uuid::new_v4(), - name: "ntp2".parse().unwrap(), - ip: ntp2_pip.into(), - mac: macs.next().unwrap(), - slot: 0, - }, + ) + .expect("recording Omicron zones"); + inventory_builder + .found_sled_omicron_zones( + "sled3", + SledUuid::from_untyped_uuid(sled3.id()), + OmicronZonesConfig { + generation: Generation::new().next(), + zones: vec![OmicronZoneConfig { + id: ntp3_id, + underlay_address: Ipv6Addr::LOCALHOST, + zone_type: OmicronZoneType::InternalNtp { + address: "[::1]:80".to_string(), + ntp_servers: vec![], + dns_servers: vec![], + domain: None, + }, + }], }, - }, - internal_params::ServicePutRequest { - service_id: ntp3_id, - sled_id: sled3.id(), - zone_id: Some(ntp3_id), - address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 9090, 0, 0), - kind: internal_params::ServiceKind::InternalNtp, - }, - ]; + ) + .expect("recording Omicron zones"); + let blueprint = BlueprintBuilder::build_initial_from_collection_seeded( + &inventory_builder.build(), + *Generation::new(), + *Generation::new(), + planning_input.all_sled_ids(SledFilter::All), + "test suite", + (test_name, "initial blueprint"), + ) + .expect("failed to build blueprint"); let rack = datastore .rack_set_initialized( &opctx, RackInit { - services: services.clone(), + blueprint: blueprint.clone(), service_ip_pool_ranges, ..Default::default() }, @@ -1352,48 +1475,12 @@ mod test { assert_eq!(rack.id(), rack_id()); assert!(rack.initialized); - let observed_services = get_all_services(&datastore).await; - let observed_datasets = get_all_datasets(&datastore).await; - - // We should see all the services we initialized - assert_eq!(observed_services.len(), 5); - let dns_service = observed_services - .iter() - .find(|s| s.id() == external_dns_id) - .unwrap(); - let nexus_service = - observed_services.iter().find(|s| s.id() == nexus_id).unwrap(); - let ntp1_service = - observed_services.iter().find(|s| s.id() == ntp1_id).unwrap(); - let ntp2_service = - observed_services.iter().find(|s| s.id() == ntp2_id).unwrap(); - let ntp3_service = - observed_services.iter().find(|s| s.id() == ntp3_id).unwrap(); - - assert_eq!(dns_service.sled_id, sled1.id()); - assert_eq!(dns_service.kind, ServiceKind::ExternalDns); - assert_eq!(*dns_service.ip, Ipv6Addr::LOCALHOST); - assert_eq!(*dns_service.port, 123); - - assert_eq!(nexus_service.sled_id, sled2.id()); - assert_eq!(nexus_service.kind, ServiceKind::Nexus); - assert_eq!(*nexus_service.ip, Ipv6Addr::LOCALHOST); - assert_eq!(*nexus_service.port, 456); - - assert_eq!(ntp1_service.sled_id, sled1.id()); - assert_eq!(ntp1_service.kind, ServiceKind::Ntp); - assert_eq!(*ntp1_service.ip, Ipv6Addr::LOCALHOST); - assert_eq!(*ntp1_service.port, 9090); - - assert_eq!(ntp2_service.sled_id, sled2.id()); - assert_eq!(ntp2_service.kind, ServiceKind::Ntp); - assert_eq!(*ntp2_service.ip, Ipv6Addr::LOCALHOST); - assert_eq!(*ntp2_service.port, 9090); - - assert_eq!(ntp3_service.sled_id, sled3.id()); - assert_eq!(ntp3_service.kind, ServiceKind::Ntp); - assert_eq!(*ntp3_service.ip, Ipv6Addr::LOCALHOST); - assert_eq!(*ntp3_service.port, 9090); + // We should see the blueprint we passed in. + let (_blueprint_target, observed_blueprint) = datastore + .blueprint_target_get_current_full(&opctx) + .await + .expect("failed to read blueprint"); + assert_eq!(observed_blueprint, blueprint); // We should also see the single external IP allocated for each service // save for the non-boundary NTP service. @@ -1419,21 +1506,17 @@ mod test { .iter() .any(|e| e.parent_id == Some(ntp3_id))); - assert_eq!(dns_external_ip.parent_id, Some(dns_service.id())); assert!(dns_external_ip.is_service); assert_eq!(dns_external_ip.kind, IpKind::Floating); - assert_eq!(nexus_external_ip.parent_id, Some(nexus_service.id())); assert!(nexus_external_ip.is_service); assert_eq!(nexus_external_ip.kind, IpKind::Floating); - assert_eq!(ntp1_external_ip.parent_id, Some(ntp1_service.id())); assert!(ntp1_external_ip.is_service); assert_eq!(ntp1_external_ip.kind, IpKind::SNat); assert_eq!(ntp1_external_ip.first_port.0, 16384); assert_eq!(ntp1_external_ip.last_port.0, 32767); - assert_eq!(ntp2_external_ip.parent_id, Some(ntp2_service.id())); assert!(ntp2_external_ip.is_service); assert_eq!(ntp2_external_ip.kind, IpKind::SNat); assert_eq!(ntp2_external_ip.first_port.0, 0); @@ -1478,6 +1561,7 @@ mod test { ); assert_eq!(ntp2_external_ip.ip.ip(), ntp2_ip); + let observed_datasets = get_all_datasets(&datastore).await; assert!(observed_datasets.is_empty()); db.cleanup().await.unwrap(); @@ -1486,9 +1570,8 @@ mod test { #[tokio::test] async fn rack_set_initialized_with_many_nexus_services() { - let logctx = dev::test_setup_log( - "rack_set_initialized_with_many_nexus_services", - ); + let test_name = "rack_set_initialized_with_many_nexus_services"; + let logctx = dev::test_setup_log(test_name); let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; @@ -1497,6 +1580,25 @@ mod test { // Ask for two Nexus services, with different external IPs. let nexus_ip_start = Ipv4Addr::new(1, 2, 3, 4); let nexus_ip_end = Ipv4Addr::new(1, 2, 3, 5); + let service_ip_pool_ranges = + vec![IpRange::try_from((nexus_ip_start, nexus_ip_end)) + .expect("Cannot create IP Range")]; + + let mut system = SystemDescription::new(); + system + .service_ip_pool_ranges(service_ip_pool_ranges.clone()) + .sled( + SledBuilder::new().id(TypedUuid::from_untyped_uuid(sled.id())), + ) + .expect("failed to add sled"); + let planning_input = system + .to_planning_input_builder() + .expect("failed to make planning input") + .build(); + let mut inventory_builder = system + .to_collection_builder() + .expect("failed to make collection builder"); + let nexus_id1 = Uuid::new_v4(); let nexus_id2 = Uuid::new_v4(); let nexus_pip1 = NEXUS_OPTE_IPV4_SUBNET @@ -1506,47 +1608,72 @@ mod test { .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 2) .unwrap(); let mut macs = MacAddr::iter_system(); - let mut services = vec![ - internal_params::ServicePutRequest { - service_id: nexus_id1, - sled_id: sled.id(), - zone_id: Some(nexus_id1), - address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0), - kind: internal_params::ServiceKind::Nexus { - external_address: IpAddr::V4(nexus_ip_start), - nic: ServiceNic { - id: Uuid::new_v4(), - name: "nexus1".parse().unwrap(), - ip: nexus_pip1.into(), - mac: macs.next().unwrap(), - slot: 0, - }, - }, - }, - internal_params::ServicePutRequest { - service_id: nexus_id2, - sled_id: sled.id(), - zone_id: Some(nexus_id2), - address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 456, 0, 0), - kind: internal_params::ServiceKind::Nexus { - external_address: IpAddr::V4(nexus_ip_end), - nic: ServiceNic { - id: Uuid::new_v4(), - name: "nexus2".parse().unwrap(), - ip: nexus_pip2.into(), - mac: macs.next().unwrap(), - slot: 0, - }, + + inventory_builder + .found_sled_omicron_zones( + "sled", + SledUuid::from_untyped_uuid(sled.id()), + OmicronZonesConfig { + generation: Generation::new().next(), + zones: vec![ + OmicronZoneConfig { + id: nexus_id1, + underlay_address: Ipv6Addr::LOCALHOST, + zone_type: OmicronZoneType::Nexus { + internal_address: "[::1]:80".to_string(), + external_ip: nexus_ip_start.into(), + external_tls: false, + external_dns_servers: vec![], + nic: NetworkInterface { + id: Uuid::new_v4(), + kind: NetworkInterfaceKind::Service { + id: nexus_id1, + }, + name: "nexus1".parse().unwrap(), + ip: nexus_pip1.into(), + mac: macs.next().unwrap(), + subnet: IpNetwork::from( + **NEXUS_OPTE_IPV4_SUBNET, + ) + .into(), + vni: Vni::SERVICES_VNI, + primary: true, + slot: 0, + }, + }, + }, + OmicronZoneConfig { + id: nexus_id2, + underlay_address: Ipv6Addr::LOCALHOST, + zone_type: OmicronZoneType::Nexus { + internal_address: "[::1]:80".to_string(), + external_ip: nexus_ip_end.into(), + external_tls: false, + external_dns_servers: vec![], + nic: NetworkInterface { + id: Uuid::new_v4(), + kind: NetworkInterfaceKind::Service { + id: nexus_id2, + }, + name: "nexus2".parse().unwrap(), + ip: nexus_pip2.into(), + mac: macs.next().unwrap(), + subnet: IpNetwork::from( + **NEXUS_OPTE_IPV4_SUBNET, + ) + .into(), + vni: Vni::SERVICES_VNI, + primary: true, + slot: 0, + }, + }, + }, + ], }, - }, - ]; - services - .sort_by(|a, b| a.service_id.partial_cmp(&b.service_id).unwrap()); + ) + .expect("recording Omicron zones"); let datasets = vec![]; - let service_ip_pool_ranges = - vec![IpRange::try_from((nexus_ip_start, nexus_ip_end)) - .expect("Cannot create IP Range")]; let internal_records = vec![ DnsRecord::Aaaa("fe80::1:2:3:4".parse().unwrap()), @@ -1570,11 +1697,21 @@ mod test { HashMap::from([("api.sys".to_string(), external_records.clone())]), ); + let blueprint = BlueprintBuilder::build_initial_from_collection_seeded( + &inventory_builder.build(), + *Generation::new(), + *Generation::new(), + planning_input.all_sled_ids(SledFilter::All), + "test suite", + (test_name, "initial blueprint"), + ) + .expect("failed to build blueprint"); + let rack = datastore .rack_set_initialized( &opctx, RackInit { - services: services.clone(), + blueprint: blueprint.clone(), datasets: datasets.clone(), service_ip_pool_ranges, internal_dns, @@ -1588,21 +1725,20 @@ mod test { assert_eq!(rack.id(), rack_id()); assert!(rack.initialized); - let mut observed_services = get_all_services(&datastore).await; - let observed_datasets = get_all_datasets(&datastore).await; + // We should see the blueprint we passed in. + let (_blueprint_target, observed_blueprint) = datastore + .blueprint_target_get_current_full(&opctx) + .await + .expect("failed to read blueprint"); + assert_eq!(observed_blueprint, blueprint); // We should see both of the Nexus services we provisioned. - assert_eq!(observed_services.len(), 2); - observed_services.sort_by(|a, b| a.id().partial_cmp(&b.id()).unwrap()); - - assert_eq!(observed_services[0].sled_id, sled.id()); - assert_eq!(observed_services[1].sled_id, sled.id()); - assert_eq!(observed_services[0].kind, ServiceKind::Nexus); - assert_eq!(observed_services[1].kind, ServiceKind::Nexus); - assert_eq!(*observed_services[0].ip, Ipv6Addr::LOCALHOST); - assert_eq!(*observed_services[1].ip, Ipv6Addr::LOCALHOST); - assert_eq!(*observed_services[0].port, services[0].address.port()); - assert_eq!(*observed_services[1].port, services[1].address.port()); + let mut observed_zones: Vec<_> = observed_blueprint + .all_omicron_zones(BlueprintZoneFilter::All) + .map(|(_, z)| z) + .collect(); + observed_zones.sort_by_key(|z| z.id); + assert_eq!(observed_zones.len(), 2); // We should see both IPs allocated for these services. let observed_external_ips = get_all_external_ips(&datastore).await; @@ -1619,25 +1755,29 @@ mod test { // The address allocated for the service should match the input. assert_eq!( - observed_external_ips[&observed_services[0].id()].ip.ip(), - if let internal_params::ServiceKind::Nexus { - external_address, - .. - } = services[0].kind + observed_external_ips[&observed_zones[0].id].ip.ip(), + if let OmicronZoneType::Nexus { external_ip, .. } = &blueprint + .all_omicron_zones(BlueprintZoneFilter::All) + .next() + .unwrap() + .1 + .zone_type { - external_address + *external_ip } else { - panic!("Unexpected service kind") + panic!("Unexpected zone type") } ); assert_eq!( - observed_external_ips[&observed_services[1].id()].ip.ip(), - if let internal_params::ServiceKind::Nexus { - external_address, - .. - } = services[1].kind + observed_external_ips[&observed_zones[1].id].ip.ip(), + if let OmicronZoneType::Nexus { external_ip, .. } = &blueprint + .all_omicron_zones(BlueprintZoneFilter::All) + .nth(1) + .unwrap() + .1 + .zone_type { - external_address + *external_ip } else { panic!("Unexpected service kind") } @@ -1653,6 +1793,7 @@ mod test { assert_eq!(observed_ip_pool_ranges.len(), 1); assert_eq!(observed_ip_pool_ranges[0].ip_pool_id, svc_pool.id()); + let observed_datasets = get_all_datasets(&datastore).await; assert!(observed_datasets.is_empty()); // Verify the internal and external DNS configurations. @@ -1692,41 +1833,84 @@ mod test { #[tokio::test] async fn rack_set_initialized_missing_service_pool_ip_throws_error() { - let logctx = dev::test_setup_log( - "rack_set_initialized_missing_service_pool_ip_throws_error", - ); + let test_name = + "rack_set_initialized_missing_service_pool_ip_throws_error"; + let logctx = dev::test_setup_log(test_name); let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; let sled = create_test_sled(&datastore).await; + let mut system = SystemDescription::new(); + system + .sled( + SledBuilder::new().id(TypedUuid::from_untyped_uuid(sled.id())), + ) + .expect("failed to add sled"); + let planning_input = system + .to_planning_input_builder() + .expect("failed to make planning input") + .build(); + let mut inventory_builder = system + .to_collection_builder() + .expect("failed to make collection builder"); + let nexus_ip = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4)); let nexus_pip = NEXUS_OPTE_IPV4_SUBNET .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 1) .unwrap(); let nexus_id = Uuid::new_v4(); let mut macs = MacAddr::iter_system(); - let services = vec![internal_params::ServicePutRequest { - service_id: nexus_id, - sled_id: sled.id(), - zone_id: Some(nexus_id), - address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0), - kind: internal_params::ServiceKind::Nexus { - external_address: nexus_ip, - nic: ServiceNic { - id: Uuid::new_v4(), - name: "nexus".parse().unwrap(), - ip: nexus_pip.into(), - mac: macs.next().unwrap(), - slot: 0, + inventory_builder + .found_sled_omicron_zones( + "sled", + SledUuid::from_untyped_uuid(sled.id()), + OmicronZonesConfig { + generation: Generation::new().next(), + zones: vec![OmicronZoneConfig { + id: nexus_id, + underlay_address: Ipv6Addr::LOCALHOST, + zone_type: OmicronZoneType::Nexus { + internal_address: "[::1]:80".to_string(), + external_ip: nexus_ip, + external_tls: false, + external_dns_servers: vec![], + nic: NetworkInterface { + id: Uuid::new_v4(), + kind: NetworkInterfaceKind::Service { + id: nexus_id, + }, + name: "nexus".parse().unwrap(), + ip: nexus_pip.into(), + mac: macs.next().unwrap(), + subnet: IpNetwork::from( + **NEXUS_OPTE_IPV4_SUBNET, + ) + .into(), + vni: Vni::SERVICES_VNI, + primary: true, + slot: 0, + }, + }, + }], }, - }, - }]; + ) + .expect("recording Omicron zones"); + + let blueprint = BlueprintBuilder::build_initial_from_collection_seeded( + &inventory_builder.build(), + *Generation::new(), + *Generation::new(), + planning_input.all_sled_ids(SledFilter::All), + "test suite", + (test_name, "initial blueprint"), + ) + .expect("failed to build blueprint"); let result = datastore .rack_set_initialized( &opctx, - RackInit { services: services.clone(), ..Default::default() }, + RackInit { blueprint: blueprint.clone(), ..Default::default() }, ) .await; assert!(result.is_err()); @@ -1735,7 +1919,6 @@ mod test { "Invalid Request: Requested external IP address not available" ); - assert!(get_all_services(&datastore).await.is_empty()); assert!(get_all_datasets(&datastore).await.is_empty()); assert!(get_all_external_ips(&datastore).await.is_empty()); @@ -1745,16 +1928,32 @@ mod test { #[tokio::test] async fn rack_set_initialized_overlapping_ips_throws_error() { - let logctx = dev::test_setup_log( - "rack_set_initialized_overlapping_ips_throws_error", - ); + let test_name = "rack_set_initialized_overlapping_ips_throws_error"; + let logctx = dev::test_setup_log(test_name); let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; let sled = create_test_sled(&datastore).await; - // Request two services which happen to be using the same IP address. let ip = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4)); + let service_ip_pool_ranges = vec![IpRange::from(ip)]; + + let mut system = SystemDescription::new(); + system + .service_ip_pool_ranges(service_ip_pool_ranges.clone()) + .sled( + SledBuilder::new().id(TypedUuid::from_untyped_uuid(sled.id())), + ) + .expect("failed to add sled"); + let planning_input = system + .to_planning_input_builder() + .expect("failed to make planning input") + .build(); + let mut inventory_builder = system + .to_collection_builder() + .expect("failed to make collection builder"); + + // Request two services which happen to be using the same IP address. let external_dns_id = Uuid::new_v4(); let external_dns_pip = DNS_OPTE_IPV4_SUBNET .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 1) @@ -1765,48 +1964,86 @@ mod test { .unwrap(); let mut macs = MacAddr::iter_system(); - let services = vec![ - internal_params::ServicePutRequest { - service_id: external_dns_id, - sled_id: sled.id(), - zone_id: Some(external_dns_id), - address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0), - kind: internal_params::ServiceKind::ExternalDns { - external_address: ip, - nic: ServiceNic { - id: Uuid::new_v4(), - name: "external-dns".parse().unwrap(), - ip: external_dns_pip.into(), - mac: macs.next().unwrap(), - slot: 0, - }, - }, - }, - internal_params::ServicePutRequest { - service_id: nexus_id, - sled_id: sled.id(), - zone_id: Some(nexus_id), - address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0), - kind: internal_params::ServiceKind::Nexus { - external_address: ip, - nic: ServiceNic { - id: Uuid::new_v4(), - name: "nexus".parse().unwrap(), - ip: nexus_pip.into(), - mac: macs.next().unwrap(), - slot: 0, - }, + inventory_builder + .found_sled_omicron_zones( + "sled", + SledUuid::from_untyped_uuid(sled.id()), + OmicronZonesConfig { + generation: Generation::new().next(), + zones: vec![ + OmicronZoneConfig { + id: external_dns_id, + underlay_address: Ipv6Addr::LOCALHOST, + zone_type: OmicronZoneType::ExternalDns { + dataset: random_dataset(), + http_address: "[::1]:80".to_string(), + dns_address: SocketAddr::new(ip, 53) + .to_string(), + nic: NetworkInterface { + id: Uuid::new_v4(), + kind: NetworkInterfaceKind::Service { + id: external_dns_id, + }, + name: "external-dns".parse().unwrap(), + ip: external_dns_pip.into(), + mac: macs.next().unwrap(), + subnet: IpNetwork::from( + **DNS_OPTE_IPV4_SUBNET, + ) + .into(), + vni: Vni::SERVICES_VNI, + primary: true, + slot: 0, + }, + }, + }, + OmicronZoneConfig { + id: nexus_id, + underlay_address: Ipv6Addr::LOCALHOST, + zone_type: OmicronZoneType::Nexus { + internal_address: "[::1]:80".to_string(), + external_ip: ip, + external_tls: false, + external_dns_servers: vec![], + nic: NetworkInterface { + id: Uuid::new_v4(), + kind: NetworkInterfaceKind::Service { + id: nexus_id, + }, + name: "nexus".parse().unwrap(), + ip: nexus_pip.into(), + mac: macs.next().unwrap(), + subnet: IpNetwork::from( + **NEXUS_OPTE_IPV4_SUBNET, + ) + .into(), + vni: Vni::SERVICES_VNI, + primary: true, + slot: 0, + }, + }, + }, + ], }, - }, - ]; - let service_ip_pool_ranges = vec![IpRange::from(ip)]; + ) + .expect("recording Omicron zones"); + + let blueprint = BlueprintBuilder::build_initial_from_collection_seeded( + &inventory_builder.build(), + *Generation::new(), + *Generation::new(), + planning_input.all_sled_ids(SledFilter::All), + "test suite", + (test_name, "initial blueprint"), + ) + .expect("failed to build blueprint"); let result = datastore .rack_set_initialized( &opctx, RackInit { rack_id: rack_id(), - services: services.clone(), + blueprint: blueprint.clone(), service_ip_pool_ranges, ..Default::default() }, @@ -1818,7 +2055,6 @@ mod test { "Invalid Request: Requested external IP address not available", ); - assert!(get_all_services(&datastore).await.is_empty()); assert!(get_all_datasets(&datastore).await.is_empty()); assert!(get_all_external_ips(&datastore).await.is_empty()); diff --git a/nexus/db-queries/src/db/datastore/service.rs b/nexus/db-queries/src/db/datastore/service.rs deleted file mode 100644 index df7ed27a6d..0000000000 --- a/nexus/db-queries/src/db/datastore/service.rs +++ /dev/null @@ -1,115 +0,0 @@ -// 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 on [`Service`]s. - -use super::DataStore; -use crate::authz; -use crate::context::OpContext; -use crate::db; -use crate::db::collection_insert::AsyncInsertError; -use crate::db::collection_insert::DatastoreCollection; -use crate::db::error::public_error_from_diesel; -use crate::db::error::retryable; -use crate::db::error::ErrorHandler; -use crate::db::error::TransactionError; -use crate::db::identity::Asset; -use crate::db::model::Service; -use crate::db::model::Sled; -use crate::db::pagination::paginated; -use crate::db::pool::DbConnection; -use async_bb8_diesel::AsyncRunQueryDsl; -use chrono::Utc; -use diesel::prelude::*; -use diesel::upsert::excluded; -use nexus_db_model::ServiceKind; -use omicron_common::api::external::CreateResult; -use omicron_common::api::external::DataPageParams; -use omicron_common::api::external::Error; -use omicron_common::api::external::ListResultVec; -use omicron_common::api::external::LookupType; -use omicron_common::api::external::ResourceType; -use uuid::Uuid; - -impl DataStore { - /// Stores a new service in the database. - pub async fn service_upsert( - &self, - opctx: &OpContext, - service: Service, - ) -> CreateResult { - let conn = self.pool_connection_authorized(opctx).await?; - self.service_upsert_conn(&conn, service).await.map_err(|e| match e { - TransactionError::CustomError(err) => err, - TransactionError::Database(err) => { - public_error_from_diesel(err, ErrorHandler::Server) - } - }) - } - - /// Stores a new service in the database (using an existing db connection). - pub(crate) async fn service_upsert_conn( - &self, - conn: &async_bb8_diesel::Connection, - service: Service, - ) -> Result> { - use db::schema::service::dsl; - - let service_id = service.id(); - let sled_id = service.sled_id; - Sled::insert_resource( - sled_id, - diesel::insert_into(dsl::service) - .values(service) - .on_conflict(dsl::id) - .do_update() - .set(( - dsl::time_modified.eq(Utc::now()), - dsl::sled_id.eq(excluded(dsl::sled_id)), - dsl::ip.eq(excluded(dsl::ip)), - dsl::port.eq(excluded(dsl::port)), - dsl::kind.eq(excluded(dsl::kind)), - )), - ) - .insert_and_get_result_async(conn) - .await - .map_err(|e| match e { - AsyncInsertError::CollectionNotFound => { - TransactionError::CustomError(Error::ObjectNotFound { - type_name: ResourceType::Sled, - lookup_type: LookupType::ById(sled_id), - }) - } - AsyncInsertError::DatabaseError(e) => { - if retryable(&e) { - return TransactionError::Database(e); - } - TransactionError::CustomError(public_error_from_diesel( - e, - ErrorHandler::Conflict( - ResourceType::Service, - &service_id.to_string(), - ), - )) - } - }) - } - - /// List services of a given kind - pub async fn services_list_kind( - &self, - opctx: &OpContext, - kind: ServiceKind, - pagparams: &DataPageParams<'_, Uuid>, - ) -> ListResultVec { - opctx.authorize(authz::Action::Read, &authz::FLEET).await?; - use db::schema::service::dsl; - paginated(dsl::service, dsl::id, pagparams) - .filter(dsl::kind.eq(kind)) - .select(Service::as_select()) - .load_async(&*self.pool_connection_authorized(opctx).await?) - .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) - } -} diff --git a/nexus/db-queries/src/db/datastore/vpc.rs b/nexus/db-queries/src/db/datastore/vpc.rs index f08854734d..079f52ba8c 100644 --- a/nexus/db-queries/src/db/datastore/vpc.rs +++ b/nexus/db-queries/src/db/datastore/vpc.rs @@ -655,7 +655,7 @@ impl DataStore { // Sleds to notify when firewall rules change. use db::schema::{ bp_omicron_zone, bp_target, instance, instance_network_interface, - service, service_network_interface, sled, vmm, + service_network_interface, sled, vmm, }; // Diesel requires us to use aliases in order to refer to the // `bp_target` table twice in the same query. @@ -677,25 +677,7 @@ impl DataStore { .filter(vmm::time_deleted.is_null()) .select(Sled::as_select()); - // When Nexus accepts the rack initialization handoff from RSS, it - // populates the `service` table. We eventually want to retire it - // (https://github.com/oxidecomputer/omicron/issues/4947), and the - // Reconfigurator does not add new entries to it. We still need to query - // it for systems that are not yet under Reconfigurator control... - let rss_service_query = service_network_interface::table - .inner_join( - service::table - .on(service::id.eq(service_network_interface::service_id)), - ) - .inner_join(sled::table.on(sled::id.eq(service::sled_id))) - .filter(service_network_interface::vpc_id.eq(vpc_id)) - .filter(service_network_interface::time_deleted.is_null()) - .select(Sled::as_select()); - - // ... and we also need to query for the current target blueprint to - // support systems that _are_ under Reconfigurator control. - - let reconfig_service_query = service_network_interface::table + let service_query = service_network_interface::table .inner_join(bp_omicron_zone::table.on( bp_omicron_zone::id.eq(service_network_interface::service_id), )) @@ -740,11 +722,7 @@ impl DataStore { let conn = self.pool_connection_unauthorized().await?; sleds - .intersect( - instance_query - .union(rss_service_query) - .union(reconfig_service_query), - ) + .intersect(instance_query.union(service_query)) .get_results_async(&*conn) .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) @@ -1255,8 +1233,6 @@ mod tests { use crate::db::fixed_data::vpc_subnet::NEXUS_VPC_SUBNET; use crate::db::model::Project; use crate::db::queries::vpc::MAX_VNI_SEARCH_RANGE_SIZE; - use async_bb8_diesel::AsyncConnection; - use async_bb8_diesel::AsyncSimpleConnection; use nexus_config::NUM_INITIAL_RESERVED_IP_ADDRESSES; use nexus_db_model::SledUpdate; use nexus_test_utils::db::test_setup_database; @@ -1511,6 +1487,7 @@ mod tests { #[derive(Debug)] struct HarnessNexus { + sled_id: SledUuid, id: Uuid, ip: IpAddr, mac: MacAddr, @@ -1528,8 +1505,11 @@ mod tests { .skip(NUM_INITIAL_RESERVED_IP_ADDRESSES) .map(IpAddr::from); let mut nexus_macs = MacAddr::iter_system(); - let nexuses = (0..num_sleds) - .map(|_| HarnessNexus { + let nexuses = sled_ids + .iter() + .copied() + .map(|sled_id| HarnessNexus { + sled_id, id: Uuid::new_v4(), ip: nexus_ips.next().unwrap(), mac: nexus_macs.next().unwrap(), @@ -1552,21 +1532,13 @@ mod tests { }) } - fn db_services( + fn db_nics( &self, - ) -> impl Iterator< - Item = (db::model::Service, db::model::IncompleteNetworkInterface), - > + '_ { - self.sled_ids.iter().zip(&self.nexuses).map(|(sled_id, nexus)| { - let service = db::model::Service::new( - nexus.id, - sled_id.into_untyped_uuid(), - Some(nexus.id), - "[::1]:0".parse().unwrap(), - db::model::ServiceKind::Nexus, - ); + ) -> impl Iterator + '_ + { + self.nexuses.iter().map(|nexus| { let name = format!("test-nexus-{}", nexus.id); - let nic = db::model::IncompleteNetworkInterface::new_service( + db::model::IncompleteNetworkInterface::new_service( nexus.nic_id, nexus.id, NEXUS_VPC_SUBNET.clone(), @@ -1578,17 +1550,16 @@ mod tests { nexus.mac, 0, ) - .expect("failed to create incomplete Nexus NIC"); - (service, nic) + .expect("failed to create incomplete Nexus NIC") }) } fn blueprint_zone_configs( &self, ) -> impl Iterator + '_ { - self.db_services().map(|(service, nic)| { + self.nexuses.iter().zip(self.db_nics()).map(|(nexus, nic)| { let config = OmicronZoneConfig { - id: service.id(), + id: nexus.id, underlay_address: "::1".parse().unwrap(), zone_type: OmicronZoneType::Nexus { internal_address: "[::1]:0".to_string(), @@ -1596,7 +1567,7 @@ mod tests { nic: NetworkInterface { id: nic.identity.id, kind: NetworkInterfaceKind::Service { - id: service.id(), + id: nexus.id, }, name: format!("test-nic-{}", nic.identity.id) .parse() @@ -1616,11 +1587,26 @@ mod tests { config, disposition: BlueprintZoneDisposition::InService, }; - (service.sled_id, zone_config) + (nexus.sled_id.into_untyped_uuid(), zone_config) }) } } + async fn assert_service_sled_ids( + datastore: &DataStore, + expected_sled_ids: &[SledUuid], + ) { + let mut service_sled_ids = datastore + .vpc_resolve_to_sleds(*SERVICES_VPC_ID, &[]) + .await + .expect("failed to resolve to sleds") + .into_iter() + .map(|sled| SledUuid::from_untyped_uuid(sled.id())) + .collect::>(); + service_sled_ids.sort(); + assert_eq!(expected_sled_ids, service_sled_ids); + } + #[tokio::test] async fn test_vpc_resolve_to_sleds_uses_current_target_blueprint() { // Test setup. @@ -1631,40 +1617,15 @@ mod tests { let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; - // Helper function to fetch and sort the IDs of sleds we've resolved the - // SERVICES_VPC_ID to. - let fetch_service_sled_ids = || async { - let mut service_sled_ids = datastore - .vpc_resolve_to_sleds(*SERVICES_VPC_ID, &[]) - .await - .expect("failed to resolve to sleds") - .into_iter() - .map(|sled| SledUuid::from_untyped_uuid(sled.id())) - .collect::>(); - service_sled_ids.sort(); - service_sled_ids - }; - // Create five sleds. let harness = Harness::new(5); for sled in harness.db_sleds() { datastore.sled_upsert(sled).await.expect("failed to upsert sled"); } - // Insert two Nexus records into `service`, emulating RSS. - for (service, nic) in harness.db_services().take(2) { - datastore - .service_upsert(&opctx, service) - .await - .expect("failed to insert RSS-like service"); - datastore - .service_create_network_interface_raw(&opctx, nic) - .await - .expect("failed to insert Nexus NIC"); - } - - // Ensure we find the two sleds we expect after adding Nexus records. - assert_eq!(&harness.sled_ids[..2], fetch_service_sled_ids().await); + // We don't have a blueprint yet, so we shouldn't find any services on + // sleds. + assert_service_sled_ids(&datastore, &[]).await; // Create a blueprint that has a Nexus on our third sled. (This // blueprint is completely invalid in many ways, but all we care about @@ -1701,9 +1662,9 @@ mod tests { .await .expect("failed to insert blueprint"); - // We haven't set a blueprint target yet, so we should still only see - // the two RSS-inserted service-running sleds. - assert_eq!(&harness.sled_ids[..2], fetch_service_sled_ids().await); + // We haven't set a blueprint target yet, so we should still fail to see + // any services on sleds. + assert_service_sled_ids(&datastore, &[]).await; // Make bp1 the current target. datastore @@ -1719,21 +1680,21 @@ mod tests { .expect("failed to set blueprint target"); // bp1 is the target, but we haven't yet inserted a vNIC record, so - // we'll still only see the original 2 sleds. - assert_eq!(&harness.sled_ids[..2], fetch_service_sled_ids().await); + // we still won't see any services on sleds. + assert_service_sled_ids(&datastore, &[]).await; // Insert the relevant service NIC record (normally performed by the // reconfigurator's executor). datastore .service_create_network_interface_raw( &opctx, - harness.db_services().nth(2).unwrap().1, + harness.db_nics().nth(2).unwrap(), ) .await .expect("failed to insert service VNIC"); - // We should now see _three_ sleds running services. - assert_eq!(&harness.sled_ids[..3], fetch_service_sled_ids().await); + // We should now see our third sled running a service. + assert_service_sled_ids(&datastore, &[harness.sled_ids[2]]).await; // Create another blueprint with no services and make it the target. let bp2_id = Uuid::new_v4(); @@ -1765,20 +1726,19 @@ mod tests { .expect("failed to set blueprint target"); // We haven't removed the service NIC record, but we should no longer - // see the third sled here, because we should be back to just the - // original two services in the `service` table. - assert_eq!(&harness.sled_ids[..2], fetch_service_sled_ids().await); + // see the third sled here. We should be back to no sleds with services. + assert_service_sled_ids(&datastore, &[]).await; // Insert a service NIC record for our fourth sled's Nexus. This // shouldn't change our VPC resolution. datastore .service_create_network_interface_raw( &opctx, - harness.db_services().nth(3).unwrap().1, + harness.db_nics().nth(3).unwrap(), ) .await .expect("failed to insert service VNIC"); - assert_eq!(&harness.sled_ids[..2], fetch_service_sled_ids().await); + assert_service_sled_ids(&datastore, &[]).await; // Create a blueprint that has a Nexus on our fourth sled. This // shouldn't change our VPC resolution. @@ -1813,7 +1773,7 @@ mod tests { .blueprint_insert(&opctx, &bp3) .await .expect("failed to insert blueprint"); - assert_eq!(&harness.sled_ids[..2], fetch_service_sled_ids().await); + assert_service_sled_ids(&datastore, &[]).await; // Make this blueprint the target. We've already created the service // VNIC, so we should immediately see our fourth sled in VPC resolution. @@ -1828,11 +1788,7 @@ mod tests { ) .await .expect("failed to set blueprint target"); - assert_eq!( - &[harness.sled_ids[0], harness.sled_ids[1], harness.sled_ids[3]] - as &[SledUuid], - fetch_service_sled_ids().await - ); + assert_service_sled_ids(&datastore, &[harness.sled_ids[3]]).await; // --- @@ -1842,7 +1798,7 @@ mod tests { datastore .service_create_network_interface_raw( &opctx, - harness.db_services().nth(4).unwrap().1, + harness.db_nics().nth(4).unwrap(), ) .await .expect("failed to insert service VNIC"); @@ -1888,7 +1844,7 @@ mod tests { ) .await .expect("failed to set blueprint target"); - assert_eq!(harness.sled_ids, fetch_service_sled_ids().await); + assert_service_sled_ids(&datastore, &harness.sled_ids[2..]).await; // --- @@ -1904,8 +1860,7 @@ mod tests { .setup(&opctx, &datastore) .await .expect("failed to set up ineligible sleds"); - - assert_eq!(&harness.sled_ids[3..=4], fetch_service_sled_ids().await); + assert_service_sled_ids(&datastore, &harness.sled_ids[3..=4]).await; // --- @@ -1915,35 +1870,6 @@ mod tests { .await .expect("failed to undo ineligible sleds"); - // Clear out the service table entirely so we're only testing - // blueprints. (The services table is going to go away soon so this is - // an easy workaround for now.) - { - use db::schema::service::dsl; - - let conn = datastore - .pool_connection_authorized(&opctx) - .await - .expect("getting a connection succeeded"); - conn.transaction_async(|conn| async move { - // Need to do a full table scan for a full delete. - conn.batch_execute_async( - nexus_test_utils::db::ALLOW_FULL_TABLE_SCAN_SQL, - ) - .await - .expect("allowing full table scan succeeded"); - - diesel::delete(dsl::service) - .execute_async(&conn) - .await - .expect("failed to delete services"); - - Ok::<_, DieselError>(()) - }) - .await - .expect("transaction succeed"); - } - // Make a new blueprint marking one of the zones as quiesced and one as // expunged. Ensure that the sled with *quiesced* zone is returned by // vpc_resolve_to_sleds, but the sled with the *expunged* zone is not. @@ -1963,6 +1889,15 @@ mod tests { }, ); + // We never created a vNIC record for sled 1; do so now. + datastore + .service_create_network_interface_raw( + &opctx, + harness.db_nics().nth(1).unwrap(), + ) + .await + .expect("failed to insert service VNIC"); + // Sled index 2's zone is quiesced (should be included). let (sled_id, mut zone_config) = iter.next().unwrap(); zone_config.disposition = BlueprintZoneDisposition::Quiesced; @@ -2018,7 +1953,7 @@ mod tests { ) .await .expect("failed to set blueprint target"); - assert_eq!(&harness.sled_ids[1..=2], fetch_service_sled_ids().await); + assert_service_sled_ids(&datastore, &harness.sled_ids[1..=2]).await; db.cleanup().await.unwrap(); logctx.cleanup_successful(); diff --git a/nexus/reconfigurator/execution/src/dns.rs b/nexus/reconfigurator/execution/src/dns.rs index fe44d5c25a..5165dcf3ea 100644 --- a/nexus/reconfigurator/execution/src/dns.rs +++ b/nexus/reconfigurator/execution/src/dns.rs @@ -1198,8 +1198,7 @@ mod test { let (_blueprint_target, blueprint) = datastore .blueprint_target_get_current_full(&opctx) .await - .expect("failed to read current target blueprint") - .expect("no target blueprint set"); + .expect("failed to read current target blueprint"); eprintln!("blueprint: {}", blueprint.display()); // Now, execute the initial blueprint. diff --git a/nexus/src/app/background/blueprint_load.rs b/nexus/src/app/background/blueprint_load.rs index d9f6411721..9c13d8e70a 100644 --- a/nexus/src/app/background/blueprint_load.rs +++ b/nexus/src/app/background/blueprint_load.rs @@ -54,12 +54,15 @@ impl BackgroundTask for TargetBlueprintLoader { }; // Retrieve the latest target blueprint - let result = - self.datastore.blueprint_target_get_current_full(opctx).await; - - // Decide what to do with the result - match (&mut self.last, result) { - (_, Err(error)) => { + let (new_bp_target, new_blueprint) = match self + .datastore + .blueprint_target_get_current_full(opctx) + .await + { + Ok((new_bp_target, new_blueprint)) => { + (new_bp_target, new_blueprint) + } + Err(error) => { // We failed to read the blueprint. There's nothing to do // but log an error. We'll retry when we're activated again. let message = format!("{:#}", error); @@ -70,40 +73,84 @@ impl BackgroundTask for TargetBlueprintLoader { ); let e = format!("failed to read target blueprint: {message}"); - json!({"error": e}) - } - (None, Ok(None)) => { - // We haven't found a blueprint yet. Do nothing. - json!({"status": "no target blueprint"}) + return json!({"error": e}); } - (Some(old), Ok(None)) => { - // We have transitioned from having a blueprint to not - // having one. This should not happen. + }; + + // Decide what to do with the new blueprint + let Some((old_bp_target, old_blueprint)) = self.last.as_deref() + else { + // We've found a target blueprint for the first time. + // Save it and notify any watchers. + let target_id = new_blueprint.id; + let time_created = new_blueprint.time_created; + info!( + log, + "found new target blueprint (first find)"; + "target_id" => %target_id, + "time_created" => %time_created + ); + self.last = Some(Arc::new((new_bp_target, new_blueprint))); + self.tx.send_replace(self.last.clone()); + return json!({ + "target_id": target_id, + "time_created": time_created, + "time_found": chrono::Utc::now(), + "status": "first target blueprint", + }); + }; + + let target_id = new_blueprint.id; + let time_created = new_blueprint.time_created; + if old_blueprint.id != new_blueprint.id { + // The current target blueprint has been updated + info!( + log, + "found new target blueprint"; + "target_id" => %target_id, + "time_created" => %time_created + ); + self.last = Some(Arc::new((new_bp_target, new_blueprint))); + self.tx.send_replace(self.last.clone()); + json!({ + "target_id": target_id, + "time_created": time_created, + "time_found": chrono::Utc::now(), + "status": "target blueprint updated" + }) + } else { + // The new target id matches the old target id + // + // Let's see if the blueprints hold the same contents. + // It should not be possible for the contents of a + // blueprint to change, but we check to catch possible + // bugs further up the stack. + if *old_blueprint != new_blueprint { let message = format!( - "target blueprint with id {} was removed. There is no \ - longer any target blueprint", - old.1.id + "blueprint for id {} changed. \ + Blueprints are supposed to be immutable.", + target_id ); - let old_id = old.1.id; - self.last = None; - self.tx.send_replace(self.last.clone()); - error!(&log, "{message:?}"); + error!(&log, "{}", message); json!({ - "removed_target_id": old_id, - "status": "no target blueprint (removed)", + "target_id": target_id, + "status": "target blueprint unchanged (error)", "error": message }) - } - (None, Ok(Some((new_bp_target, new_blueprint)))) => { - // We've found a target blueprint for the first time. - // Save it and notify any watchers. - let target_id = new_blueprint.id; - let time_created = new_blueprint.time_created; + } else if old_bp_target.enabled != new_bp_target.enabled { + // The blueprints have the same contents, but its + // enabled bit has flipped. + let status = if new_bp_target.enabled { + "enabled" + } else { + "disabled" + }; info!( log, - "found new target blueprint (first find)"; + "target blueprint enabled state changed"; "target_id" => %target_id, - "time_created" => %time_created + "time_created" => %time_created, + "state" => status, ); self.last = Some(Arc::new((new_bp_target, new_blueprint))); self.tx.send_replace(self.last.clone()); @@ -111,89 +158,23 @@ impl BackgroundTask for TargetBlueprintLoader { "target_id": target_id, "time_created": time_created, "time_found": chrono::Utc::now(), - "status": "first target blueprint", + "status": format!("target blueprint {status}"), + }) + } else { + // We found a new target blueprint that exactly + // matches the old target blueprint. This is the + // common case when we're activated by a timeout. + debug!( + log, + "found latest target blueprint (unchanged)"; + "target_id" => %target_id, + "time_created" => %time_created.clone() + ); + json!({ + "target_id": target_id, + "time_created": time_created, + "status": "target blueprint unchanged" }) - } - (Some(old), Ok(Some((new_bp_target, new_blueprint)))) => { - let target_id = new_blueprint.id; - let time_created = new_blueprint.time_created; - if old.1.id != new_blueprint.id { - // The current target blueprint has been updated - info!( - log, - "found new target blueprint"; - "target_id" => %target_id, - "time_created" => %time_created - ); - self.last = - Some(Arc::new((new_bp_target, new_blueprint))); - self.tx.send_replace(self.last.clone()); - json!({ - "target_id": target_id, - "time_created": time_created, - "time_found": chrono::Utc::now(), - "status": "target blueprint updated" - }) - } else { - // The new target id matches the old target id - // - // Let's see if the blueprints hold the same contents. - // It should not be possible for the contents of a - // blueprint to change, but we check to catch possible - // bugs further up the stack. - if old.1 != new_blueprint { - let message = format!( - "blueprint for id {} changed. \ - Blueprints are supposed to be immutable.", - target_id - ); - error!(&log, "{}", message); - json!({ - "target_id": target_id, - "status": "target blueprint unchanged (error)", - "error": message - }) - } else if old.0.enabled != new_bp_target.enabled { - // The blueprints have the same contents, but its - // enabled bit has flipped. - let status = if new_bp_target.enabled { - "enabled" - } else { - "disabled" - }; - info!( - log, - "target blueprint enabled state changed"; - "target_id" => %target_id, - "time_created" => %time_created, - "state" => status, - ); - self.last = - Some(Arc::new((new_bp_target, new_blueprint))); - self.tx.send_replace(self.last.clone()); - json!({ - "target_id": target_id, - "time_created": time_created, - "time_found": chrono::Utc::now(), - "status": format!("target blueprint {status}"), - }) - } else { - // We found a new target blueprint that exactly - // matches the old target blueprint. This is the - // common case when we're activated by a timeout. - debug!( - log, - "found latest target blueprint (unchanged)"; - "target_id" => %target_id, - "time_created" => %time_created.clone() - ); - json!({ - "target_id": target_id, - "time_created": time_created, - "status": "target blueprint unchanged" - }) - } - } } } } diff --git a/nexus/src/app/deployment.rs b/nexus/src/app/deployment.rs index 0d8a6834ba..5f2d316efd 100644 --- a/nexus/src/app/deployment.rs +++ b/nexus/src/app/deployment.rs @@ -75,7 +75,7 @@ impl super::Nexus { pub async fn blueprint_target_view( &self, opctx: &OpContext, - ) -> Result, Error> { + ) -> Result { self.db_datastore.blueprint_target_get_current(opctx).await } @@ -238,13 +238,8 @@ impl super::Nexus { &self, opctx: &OpContext, ) -> CreateResult { - let maybe_target = + let (_, parent_blueprint) = self.db_datastore.blueprint_target_get_current_full(opctx).await?; - let Some((_, parent_blueprint)) = maybe_target else { - return Err(Error::conflict( - "cannot regenerate blueprint without existing target", - )); - }; let planning_context = self.blueprint_planning_context(opctx).await?; let inventory = planning_context.inventory.ok_or_else(|| { diff --git a/nexus/src/app/rack.rs b/nexus/src/app/rack.rs index 5b85acb929..29feeb6181 100644 --- a/nexus/src/app/rack.rs +++ b/nexus/src/app/rack.rs @@ -20,6 +20,7 @@ use nexus_db_queries::db::datastore::DnsVersionUpdateBuilder; use nexus_db_queries::db::datastore::RackInit; use nexus_db_queries::db::lookup::LookupPath; use nexus_reconfigurator_execution::silo_dns_name; +use nexus_types::deployment::BlueprintZoneFilter; use nexus_types::external_api::params::Address; use nexus_types::external_api::params::AddressConfig; use nexus_types::external_api::params::AddressLotBlockCreate; @@ -191,15 +192,15 @@ impl super::Nexus { let silo_name = &request.recovery_silo.silo_name; let dns_records = request - .services - .iter() - .filter_map(|s| match &s.kind { - nexus_types::internal_api::params::ServiceKind::Nexus { - external_address, + .blueprint + .all_omicron_zones(BlueprintZoneFilter::ShouldBeExternallyReachable) + .filter_map(|(_, zc)| match zc.zone_type { + nexus_types::deployment::OmicronZoneType::Nexus { + external_ip, .. - } => Some(match external_address { - IpAddr::V4(addr) => DnsRecord::A(*addr), - IpAddr::V6(addr) => DnsRecord::Aaaa(*addr), + } => Some(match external_ip { + IpAddr::V4(addr) => DnsRecord::A(addr), + IpAddr::V6(addr) => DnsRecord::Aaaa(addr), }), _ => None, }) @@ -613,7 +614,6 @@ impl super::Nexus { rack_subnet: rack_network_config.rack_subnet.into(), rack_id, blueprint, - services: request.services, physical_disks, zpools, datasets, diff --git a/nexus/src/app/silo.rs b/nexus/src/app/silo.rs index d07dc7013a..efde55cbd1 100644 --- a/nexus/src/app/silo.rs +++ b/nexus/src/app/silo.rs @@ -101,18 +101,12 @@ impl super::Nexus { .dns_zones_list_all(nexus_opctx, DnsGroup::External) .await .internal_context("listing external DNS zones")?; - let target_blueprint = datastore + let (_, target_blueprint) = datastore .blueprint_target_get_current_full(opctx) .await .internal_context("loading target blueprint")?; - let nexus_external_ips = match target_blueprint { - Some((_, blueprint)) => blueprint_nexus_external_ips(&blueprint), - None => { - datastore - .nexus_external_addresses_from_service_table(nexus_opctx) - .await? - } - }; + let nexus_external_ips = + blueprint_nexus_external_ips(&target_blueprint); let dns_records: Vec = nexus_external_ips .into_iter() .map(|addr| match addr { diff --git a/nexus/src/internal_api/http_entrypoints.rs b/nexus/src/internal_api/http_entrypoints.rs index 401220431a..35ec5167f9 100644 --- a/nexus/src/internal_api/http_entrypoints.rs +++ b/nexus/src/internal_api/http_entrypoints.rs @@ -40,7 +40,6 @@ use omicron_common::api::external::http_pagination::data_page_params_for; use omicron_common::api::external::http_pagination::PaginatedById; use omicron_common::api::external::http_pagination::ScanById; use omicron_common::api::external::http_pagination::ScanParams; -use omicron_common::api::external::Error; use omicron_common::api::internal::nexus::DiskRuntimeState; use omicron_common::api::internal::nexus::DownstairsClientStopRequest; use omicron_common::api::internal::nexus::DownstairsClientStopped; @@ -909,10 +908,7 @@ async fn blueprint_target_view( let handler = async { let opctx = crate::context::op_context_for_internal_api(&rqctx).await; let nexus = &apictx.nexus; - let target = - nexus.blueprint_target_view(&opctx).await?.ok_or_else(|| { - Error::conflict("no target blueprint has been configured") - })?; + let target = nexus.blueprint_target_view(&opctx).await?; Ok(HttpResponseOk(target)) }; apictx.internal_latencies.instrument_dropshot_handler(&rqctx, handler).await diff --git a/nexus/src/lib.rs b/nexus/src/lib.rs index 80c972363f..6c4fe4e91e 100644 --- a/nexus/src/lib.rs +++ b/nexus/src/lib.rs @@ -28,9 +28,11 @@ use external_api::http_entrypoints::external_api; use internal_api::http_entrypoints::internal_api; use nexus_config::NexusConfig; use nexus_types::deployment::Blueprint; +use nexus_types::deployment::BlueprintZoneFilter; +use nexus_types::deployment::OmicronZoneType; use nexus_types::external_api::views::SledProvisionPolicy; use nexus_types::internal_api::params::{ - PhysicalDiskPutRequest, ServiceKind, ZpoolPutRequest, + PhysicalDiskPutRequest, ZpoolPutRequest, }; use nexus_types::inventory::Collection; use omicron_common::address::IpRange; @@ -238,7 +240,6 @@ impl nexus_test_interface::NexusServer for Server { internal_server: InternalServer, config: &NexusConfig, blueprint: Blueprint, - services: Vec, physical_disks: Vec< nexus_types::internal_api::params::PhysicalDiskPutRequest, >, @@ -268,12 +269,19 @@ impl nexus_test_interface::NexusServer for Server { // it's 127.0.0.1, having come straight from the stock testing config // file. Whatever it is, we fake up an IP pool range for use by system // services that includes solely this IP. - let internal_services_ip_pool_ranges = services - .iter() - .filter_map(|s| match s.kind { - ServiceKind::ExternalDns { external_address, .. } - | ServiceKind::Nexus { external_address, .. } => { - Some(IpRange::from(external_address)) + let internal_services_ip_pool_ranges = blueprint + .all_omicron_zones(BlueprintZoneFilter::ShouldBeExternallyReachable) + .filter_map(|(_, zc)| match &zc.zone_type { + OmicronZoneType::ExternalDns { dns_address, .. } => { + // Work around + // https://github.com/oxidecomputer/omicron/issues/4988 + let dns_address: SocketAddr = dns_address + .parse() + .expect("invalid DNS socket address"); + Some(IpRange::from(dns_address.ip())) + } + OmicronZoneType::Nexus { external_ip, .. } => { + Some(IpRange::from(*external_ip)) } _ => None, }) @@ -287,7 +295,6 @@ impl nexus_test_interface::NexusServer for Server { config.deployment.rack_id, internal_api::params::RackInitializationRequest { blueprint, - services, physical_disks, zpools, datasets, diff --git a/nexus/test-interface/src/lib.rs b/nexus/test-interface/src/lib.rs index 54478c0876..2c7f0989ea 100644 --- a/nexus/test-interface/src/lib.rs +++ b/nexus/test-interface/src/lib.rs @@ -57,7 +57,6 @@ pub trait NexusServer: Send + Sync + 'static { internal_server: Self::InternalServer, config: &NexusConfig, blueprint: Blueprint, - services: Vec, physical_disks: Vec, zpools: Vec, datasets: Vec, diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index 6392c729ce..42f1f12546 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -35,9 +35,6 @@ use nexus_types::internal_api::params::DatasetCreateRequest; use nexus_types::internal_api::params::DatasetKind; use nexus_types::internal_api::params::DatasetPutRequest; use nexus_types::internal_api::params::RecoverySiloConfig; -use nexus_types::internal_api::params::ServiceKind; -use nexus_types::internal_api::params::ServiceNic; -use nexus_types::internal_api::params::ServicePutRequest; use nexus_types::inventory::OmicronZoneConfig; use nexus_types::inventory::OmicronZoneDataset; use nexus_types::inventory::OmicronZoneType; @@ -185,7 +182,6 @@ pub async fn test_setup( } struct RackInitRequestBuilder { - services: Vec, datasets: Vec, internal_dns_config: internal_dns::DnsConfigBuilder, mac_addrs: Box + Send>, @@ -194,31 +190,18 @@ struct RackInitRequestBuilder { impl RackInitRequestBuilder { fn new() -> Self { Self { - services: vec![], datasets: vec![], internal_dns_config: internal_dns::DnsConfigBuilder::new(), mac_addrs: Box::new(MacAddr::iter_system()), } } - // Keeps track of: - // - The "ServicePutRequest" (for handoff to Nexus) - // - The internal DNS configuration for this service - fn add_service_with_id( + fn add_service_to_dns( &mut self, zone_id: Uuid, address: SocketAddrV6, - kind: ServiceKind, service_name: internal_dns::ServiceName, - sled_id: Uuid, ) { - self.services.push(ServicePutRequest { - address, - kind, - service_id: zone_id, - sled_id, - zone_id: Some(zone_id), - }); let zone = self .internal_dns_config .host_zone( @@ -232,22 +215,6 @@ impl RackInitRequestBuilder { .expect("Failed to set up DNS for {kind}"); } - fn add_service_without_dns( - &mut self, - zone_id: Uuid, - address: SocketAddrV6, - kind: ServiceKind, - sled_id: Uuid, - ) { - self.services.push(ServicePutRequest { - address, - kind, - service_id: zone_id, - sled_id, - zone_id: Some(zone_id), - }); - } - // Keeps track of: // - The "DatasetPutRequest" (for handoff to Nexus) // - The internal DNS configuration for this service @@ -539,19 +506,6 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { // NOTE: If dendrite is started after Nexus, this is ignored. let config = DpdConfig { address: std::net::SocketAddr::V6(address) }; self.config.pkg.dendrite.insert(switch_location, config); - - let sled_id = Uuid::parse_str(match switch_location { - SwitchLocation::Switch0 => SLED_AGENT_UUID, - SwitchLocation::Switch1 => SLED_AGENT2_UUID, - }) - .unwrap(); - - self.rack_init_builder.add_service_without_dns( - sled_id, - address, - ServiceKind::Dendrite, - sled_id, - ); } pub async fn start_mgd(&mut self, switch_location: SwitchLocation) { @@ -568,19 +522,6 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { let config = MgdConfig { address: std::net::SocketAddr::V6(address) }; self.config.pkg.mgd.insert(switch_location, config); - - let sled_id = Uuid::parse_str(match switch_location { - SwitchLocation::Switch0 => SLED_AGENT_UUID, - SwitchLocation::Switch1 => SLED_AGENT2_UUID, - }) - .unwrap(); - - self.rack_init_builder.add_service_without_dns( - sled_id, - address, - ServiceKind::Mgd, - sled_id, - ); } pub async fn record_switch_dns(&mut self) { @@ -689,7 +630,6 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { 0, ); - let sled_id = Uuid::parse_str(SLED_AGENT_UUID).unwrap(); let mac = self .rack_init_builder .mac_addrs @@ -698,24 +638,10 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { let external_address = self.config.deployment.dropshot_external.dropshot.bind_address.ip(); let nexus_id = self.config.deployment.id; - self.rack_init_builder.add_service_with_id( + self.rack_init_builder.add_service_to_dns( nexus_id, address, - ServiceKind::Nexus { - external_address, - nic: ServiceNic { - id: Uuid::new_v4(), - name: "nexus".parse().unwrap(), - ip: NEXUS_OPTE_IPV4_SUBNET - .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 1) - .unwrap() - .into(), - mac, - slot: 0, - }, - }, internal_dns::ServiceName::Nexus, - sled_id, ); self.omicron_zones.push(OmicronZoneConfig { @@ -732,7 +658,10 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { internal_address: address.to_string(), nic: NetworkInterface { id: Uuid::new_v4(), - ip: external_address, + ip: NEXUS_OPTE_IPV4_SUBNET + .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 1) + .unwrap() + .into(), kind: NetworkInterfaceKind::Service { id: nexus_id }, mac, name: format!("nexus-{}", nexus_id).parse().unwrap(), @@ -864,7 +793,6 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { .expect("Must launch internal nexus first"), self.config, blueprint, - self.rack_init_builder.services.clone(), // NOTE: We should probably hand off // "self.rack_init_builder.datasets" here, but Nexus won't be happy // if we pass it right now: @@ -998,14 +926,11 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { panic!("Expected IPv6 Pantry Address"); }; - let sled_id = Uuid::parse_str(SLED_AGENT_UUID).unwrap(); let zone_id = Uuid::new_v4(); - self.rack_init_builder.add_service_with_id( + self.rack_init_builder.add_service_to_dns( zone_id, address, - ServiceKind::CruciblePantry, internal_dns::ServiceName::CruciblePantry, - sled_id, ); self.omicron_zones.push(OmicronZoneConfig { id: zone_id, @@ -1019,7 +944,6 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { // Set up an external DNS server. pub async fn start_external_dns(&mut self) { let log = self.logctx.log.new(o!("component" => "external_dns_server")); - let sled_id = Uuid::parse_str(SLED_AGENT_UUID).unwrap(); let dns = dns_server::TransientServer::new(&log).await.unwrap(); @@ -1037,24 +961,10 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { .next() .expect("ran out of MAC addresses"); let zone_id = Uuid::new_v4(); - self.rack_init_builder.add_service_with_id( + self.rack_init_builder.add_service_to_dns( zone_id, dropshot_address, - ServiceKind::ExternalDns { - external_address: (*dns_address.ip()).into(), - nic: ServiceNic { - id: Uuid::new_v4(), - name: "external-dns".parse().unwrap(), - ip: DNS_OPTE_IPV4_SUBNET - .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 1) - .unwrap() - .into(), - mac, - slot: 0, - }, - }, internal_dns::ServiceName::ExternalDns, - sled_id, ); let zpool_id = ZpoolUuid::new_v4(); @@ -1071,7 +981,10 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { http_address: dropshot_address.to_string(), nic: NetworkInterface { id: Uuid::new_v4(), - ip: (*dns_address.ip()).into(), + ip: DNS_OPTE_IPV4_SUBNET + .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 1) + .unwrap() + .into(), kind: NetworkInterfaceKind::Service { id: zone_id }, mac, name: format!("external-dns-{}", zone_id).parse().unwrap(), @@ -1089,19 +1002,16 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { // Set up an internal DNS server. pub async fn start_internal_dns(&mut self) { let log = self.logctx.log.new(o!("component" => "internal_dns_server")); - let sled_id = Uuid::parse_str(SLED_AGENT_UUID).unwrap(); let dns = dns_server::TransientServer::new(&log).await.unwrap(); let SocketAddr::V6(address) = dns.dropshot_server.local_addr() else { panic!("Unsupported IPv4 DNS address"); }; let zone_id = Uuid::new_v4(); - self.rack_init_builder.add_service_with_id( + self.rack_init_builder.add_service_to_dns( zone_id, address, - ServiceKind::InternalDns, internal_dns::ServiceName::InternalDns, - sled_id, ); let zpool_id = ZpoolUuid::new_v4(); diff --git a/nexus/types/src/internal_api/params.rs b/nexus/types/src/internal_api/params.rs index a811106c2c..15ab154952 100644 --- a/nexus/types/src/internal_api/params.rs +++ b/nexus/types/src/internal_api/params.rs @@ -193,20 +193,6 @@ impl fmt::Display for ServiceKind { } } -/// Describes a service on a sled -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -pub struct ServicePutRequest { - pub service_id: Uuid, - pub sled_id: Uuid, - pub zone_id: Option, - - /// Address on which a service is responding to requests. - pub address: SocketAddrV6, - - /// Type of service being inserted. - pub kind: ServiceKind, -} - #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] pub struct DatasetCreateRequest { pub zpool_id: Uuid, @@ -233,8 +219,6 @@ impl std::fmt::Debug for Certificate { pub struct RackInitializationRequest { /// Blueprint describing services initialized by RSS. pub blueprint: Blueprint, - /// Services on the rack which have been created by RSS. - pub services: Vec, /// "Managed" physical disks owned by the control plane pub physical_disks: Vec, diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index c4eb89f0f9..ca3eeb0f2d 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -6463,13 +6463,6 @@ } ] }, - "services": { - "description": "Services on the rack which have been created by RSS.", - "type": "array", - "items": { - "$ref": "#/components/schemas/ServicePutRequest" - } - }, "zpools": { "description": "Zpools created within the physical disks created by the control plane.", "type": "array", @@ -6489,7 +6482,6 @@ "physical_disks", "rack_network_config", "recovery_silo", - "services", "zpools" ] }, @@ -6912,326 +6904,6 @@ "timeseries_name" ] }, - "ServiceKind": { - "description": "Describes the purpose of the service.", - "oneOf": [ - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "clickhouse" - ] - } - }, - "required": [ - "type" - ] - }, - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "clickhouse_keeper" - ] - } - }, - "required": [ - "type" - ] - }, - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "cockroach" - ] - } - }, - "required": [ - "type" - ] - }, - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "crucible" - ] - } - }, - "required": [ - "type" - ] - }, - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "crucible_pantry" - ] - } - }, - "required": [ - "type" - ] - }, - { - "type": "object", - "properties": { - "content": { - "type": "object", - "properties": { - "external_address": { - "type": "string", - "format": "ip" - }, - "nic": { - "$ref": "#/components/schemas/ServiceNic" - } - }, - "required": [ - "external_address", - "nic" - ] - }, - "type": { - "type": "string", - "enum": [ - "external_dns" - ] - } - }, - "required": [ - "content", - "type" - ] - }, - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "internal_dns" - ] - } - }, - "required": [ - "type" - ] - }, - { - "type": "object", - "properties": { - "content": { - "type": "object", - "properties": { - "external_address": { - "type": "string", - "format": "ip" - }, - "nic": { - "$ref": "#/components/schemas/ServiceNic" - } - }, - "required": [ - "external_address", - "nic" - ] - }, - "type": { - "type": "string", - "enum": [ - "nexus" - ] - } - }, - "required": [ - "content", - "type" - ] - }, - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "oximeter" - ] - } - }, - "required": [ - "type" - ] - }, - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "dendrite" - ] - } - }, - "required": [ - "type" - ] - }, - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "tfport" - ] - } - }, - "required": [ - "type" - ] - }, - { - "type": "object", - "properties": { - "content": { - "type": "object", - "properties": { - "nic": { - "$ref": "#/components/schemas/ServiceNic" - }, - "snat": { - "$ref": "#/components/schemas/SourceNatConfig" - } - }, - "required": [ - "nic", - "snat" - ] - }, - "type": { - "type": "string", - "enum": [ - "boundary_ntp" - ] - } - }, - "required": [ - "content", - "type" - ] - }, - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "internal_ntp" - ] - } - }, - "required": [ - "type" - ] - }, - { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "mgd" - ] - } - }, - "required": [ - "type" - ] - } - ] - }, - "ServiceNic": { - "description": "Describes the RSS allocated values for a service vnic", - "type": "object", - "properties": { - "id": { - "type": "string", - "format": "uuid" - }, - "ip": { - "type": "string", - "format": "ip" - }, - "mac": { - "$ref": "#/components/schemas/MacAddr" - }, - "name": { - "$ref": "#/components/schemas/Name" - }, - "slot": { - "type": "integer", - "format": "uint8", - "minimum": 0 - } - }, - "required": [ - "id", - "ip", - "mac", - "name", - "slot" - ] - }, - "ServicePutRequest": { - "description": "Describes a service on a sled", - "type": "object", - "properties": { - "address": { - "description": "Address on which a service is responding to requests.", - "type": "string" - }, - "kind": { - "description": "Type of service being inserted.", - "allOf": [ - { - "$ref": "#/components/schemas/ServiceKind" - } - ] - }, - "service_id": { - "type": "string", - "format": "uuid" - }, - "sled_id": { - "type": "string", - "format": "uuid" - }, - "zone_id": { - "nullable": true, - "type": "string", - "format": "uuid" - } - }, - "required": [ - "address", - "kind", - "service_id", - "sled_id" - ] - }, "SledAgentInfo": { "description": "Sent by a sled agent to Nexus to inform about resources", "type": "object", diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index ca9eea49b3..545150d5fa 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -164,30 +164,21 @@ CREATE UNIQUE INDEX IF NOT EXISTS lookup_sled_by_rack ON omicron.public.sled ( ) WHERE time_deleted IS NULL; CREATE TYPE IF NOT EXISTS omicron.public.sled_resource_kind AS ENUM ( - -- omicron.public.dataset - 'dataset', - -- omicron.public.service - 'service', -- omicron.public.instance - 'instance', - -- omicron.public.sled - -- - -- reserved as an approximation of sled internal usage, such as "by the OS - -- and all unaccounted services". - 'reserved' + 'instance' + -- We expect to other resource kinds here in the future; e.g., to track + -- resources used by control plane services. For now, we only track + -- instances. ); -- Accounting for programs using resources on a sled CREATE TABLE IF NOT EXISTS omicron.public.sled_resource ( - -- Should match the UUID of the corresponding service + -- Should match the UUID of the corresponding resource id UUID PRIMARY KEY, -- The sled where resources are being consumed sled_id UUID NOT NULL, - -- Identifies the type of the resource - kind omicron.public.sled_resource_kind NOT NULL, - -- The maximum number of hardware threads usable by this resource hardware_threads INT8 NOT NULL, @@ -195,7 +186,10 @@ CREATE TABLE IF NOT EXISTS omicron.public.sled_resource ( rss_ram INT8 NOT NULL, -- The maximum amount of Reservoir RAM provisioned to this resource - reservoir_ram INT8 NOT NULL + reservoir_ram INT8 NOT NULL, + + -- Identifies the type of the resource + kind omicron.public.sled_resource_kind NOT NULL ); -- Allow looking up all resources which reside on a sled @@ -296,36 +290,6 @@ CREATE TYPE IF NOT EXISTS omicron.public.service_kind AS ENUM ( 'mgd' ); -CREATE TABLE IF NOT EXISTS omicron.public.service ( - /* Identity metadata (asset) */ - id UUID PRIMARY KEY, - time_created TIMESTAMPTZ NOT NULL, - time_modified TIMESTAMPTZ NOT NULL, - - /* FK into the Sled table */ - sled_id UUID NOT NULL, - /* For services in illumos zones, the zone's unique id (for debugging) */ - zone_id UUID, - /* The IP address of the service. */ - ip INET NOT NULL, - /* The UDP or TCP port on which the service listens. */ - port INT4 CHECK (port BETWEEN 0 AND 65535) NOT NULL, - /* Indicates the type of service. */ - kind omicron.public.service_kind NOT NULL -); - -/* Add an index which lets us look up the services on a sled */ -CREATE UNIQUE INDEX IF NOT EXISTS lookup_service_by_sled ON omicron.public.service ( - sled_id, - id -); - -/* Look up (and paginate) services of a given kind. */ -CREATE UNIQUE INDEX IF NOT EXISTS lookup_service_by_kind ON omicron.public.service ( - kind, - id -); - CREATE TYPE IF NOT EXISTS omicron.public.physical_disk_kind AS ENUM ( 'm2', 'u2' @@ -1300,7 +1264,9 @@ CREATE TABLE IF NOT EXISTS omicron.public.oximeter ( CREATE TYPE IF NOT EXISTS omicron.public.producer_kind AS ENUM ( -- A sled agent for an entry in the sled table. 'sled_agent', - -- A service in the omicron.public.service table + -- A service in a blueprint (typically the current target blueprint, but it + -- may reference a prior blueprint if the service is in the process of being + -- removed). 'service', -- A Propolis VMM for an instance in the omicron.public.instance table 'instance' @@ -3790,7 +3756,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - ( TRUE, NOW(), NOW(), '52.0.0', NULL) + ( TRUE, NOW(), NOW(), '53.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/drop-service-table/up1.sql b/schema/crdb/drop-service-table/up1.sql new file mode 100644 index 0000000000..dfb402ba4d --- /dev/null +++ b/schema/crdb/drop-service-table/up1.sql @@ -0,0 +1,17 @@ +-- Ensure there are no `sled_resource` rows with a `kind` other than 'instance' + +-- This is a full table scan, but the sled_resource table does not track +-- historical, deleted resources, so is at most the size of the number of +-- currently-running instances (which should be zero during a schema update). +SET + LOCAL disallow_full_table_scans = OFF; + +WITH count_non_instance_resources AS ( + SELECT COUNT(*) AS num + FROM omicron.public.sled_resource + WHERE kind != 'instance' +) +SELECT CAST( + IF(num = 0, 'true', 'sled_resource contains non-instance rows') + AS bool +) FROM count_non_instance_resources; diff --git a/schema/crdb/drop-service-table/up2.sql b/schema/crdb/drop-service-table/up2.sql new file mode 100644 index 0000000000..3d723a4876 --- /dev/null +++ b/schema/crdb/drop-service-table/up2.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS omicron.public.service; diff --git a/schema/crdb/drop-service-table/up3.sql b/schema/crdb/drop-service-table/up3.sql new file mode 100644 index 0000000000..8b546821a7 --- /dev/null +++ b/schema/crdb/drop-service-table/up3.sql @@ -0,0 +1,3 @@ +-- We are dropping `kind` so that we can drop the `sled_resource` kind; we'll +-- then recreate it (with some variants removed) and add this column back. +ALTER TABLE omicron.public.sled_resource DROP COLUMN IF EXISTS kind; diff --git a/schema/crdb/drop-service-table/up4.sql b/schema/crdb/drop-service-table/up4.sql new file mode 100644 index 0000000000..bbf5a605a4 --- /dev/null +++ b/schema/crdb/drop-service-table/up4.sql @@ -0,0 +1 @@ +DROP TYPE IF EXISTS omicron.public.sled_resource_kind; diff --git a/schema/crdb/drop-service-table/up5.sql b/schema/crdb/drop-service-table/up5.sql new file mode 100644 index 0000000000..9903bb28df --- /dev/null +++ b/schema/crdb/drop-service-table/up5.sql @@ -0,0 +1,3 @@ +CREATE TYPE IF NOT EXISTS omicron.public.sled_resource_kind AS ENUM ( + 'instance' +); diff --git a/schema/crdb/drop-service-table/up6.sql b/schema/crdb/drop-service-table/up6.sql new file mode 100644 index 0000000000..28241f96da --- /dev/null +++ b/schema/crdb/drop-service-table/up6.sql @@ -0,0 +1,5 @@ +ALTER TABLE omicron.public.sled_resource + ADD COLUMN IF NOT EXISTS + kind omicron.public.sled_resource_kind + NOT NULL + DEFAULT 'instance'; diff --git a/schema/crdb/drop-service-table/up7.sql b/schema/crdb/drop-service-table/up7.sql new file mode 100644 index 0000000000..1eeea65813 --- /dev/null +++ b/schema/crdb/drop-service-table/up7.sql @@ -0,0 +1 @@ +ALTER TABLE omicron.public.sled_resource ALTER COLUMN kind DROP DEFAULT; diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index 627fb11aa0..1393934031 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -376,152 +376,6 @@ impl OmicronZoneConfig { Some(self.id), ) } - - /// Returns the structure that describes this zone to Nexus during rack - /// initialization - pub fn to_nexus_service_req( - &self, - sled_id: Uuid, - ) -> nexus_client::types::ServicePutRequest { - use nexus_client::types as NexusTypes; - - let service_id = self.id; - let zone_id = Some(self.id); - match &self.zone_type { - OmicronZoneType::Nexus { - external_ip, - internal_address, - nic, - .. - } => NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: internal_address.to_string(), - kind: NexusTypes::ServiceKind::Nexus { - external_address: *external_ip, - nic: NexusTypes::ServiceNic { - id: nic.id, - name: nic.name.clone(), - ip: nic.ip, - mac: nic.mac, - slot: nic.slot, - }, - }, - }, - OmicronZoneType::ExternalDns { - http_address, - dns_address, - nic, - .. - } => NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: http_address.to_string(), - kind: NexusTypes::ServiceKind::ExternalDns { - external_address: dns_address.ip(), - nic: NexusTypes::ServiceNic { - id: nic.id, - name: nic.name.clone(), - ip: nic.ip, - mac: nic.mac, - slot: nic.slot, - }, - }, - }, - OmicronZoneType::InternalDns { http_address, .. } => { - NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: http_address.to_string(), - kind: NexusTypes::ServiceKind::InternalDns, - } - } - OmicronZoneType::Oximeter { address } => { - NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: address.to_string(), - kind: NexusTypes::ServiceKind::Oximeter, - } - } - OmicronZoneType::CruciblePantry { address } => { - NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: address.to_string(), - kind: NexusTypes::ServiceKind::CruciblePantry, - } - } - OmicronZoneType::BoundaryNtp { address, snat_cfg, nic, .. } => { - NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: address.to_string(), - kind: NexusTypes::ServiceKind::BoundaryNtp { - snat: snat_cfg.into(), - nic: NexusTypes::ServiceNic { - id: nic.id, - name: nic.name.clone(), - ip: nic.ip, - mac: nic.mac, - slot: nic.slot, - }, - }, - } - } - OmicronZoneType::InternalNtp { address, .. } => { - NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: address.to_string(), - kind: NexusTypes::ServiceKind::InternalNtp, - } - } - OmicronZoneType::Clickhouse { address, .. } => { - NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: address.to_string(), - kind: NexusTypes::ServiceKind::Clickhouse, - } - } - OmicronZoneType::ClickhouseKeeper { address, .. } => { - NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: address.to_string(), - kind: NexusTypes::ServiceKind::ClickhouseKeeper, - } - } - OmicronZoneType::Crucible { address, .. } => { - NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: address.to_string(), - kind: NexusTypes::ServiceKind::Crucible, - } - } - OmicronZoneType::CockroachDb { address, .. } => { - NexusTypes::ServicePutRequest { - service_id, - zone_id, - sled_id, - address: address.to_string(), - kind: NexusTypes::ServiceKind::Cockroach, - } - } - } - } } /// Describes a persistent ZFS dataset associated with an Omicron zone diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index ce5cb3fa2d..076ccbd44c 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -678,11 +678,16 @@ impl ServiceInner { ) -> Result<(), SetupServiceError> { info!(self.log, "Handing off control to Nexus"); - // Build a Blueprint describing our service plan. This should never - // fail, unless we've set up an invalid plan. - let blueprint = - build_initial_blueprint_from_plan(sled_plan, service_plan) + // Remap our plan into an easier-to-use type... + let sled_configs_by_id = + build_sled_configs_by_id(sled_plan, service_plan) .map_err(SetupServiceError::ConvertPlanToBlueprint)?; + // ... and use that to derive the initial blueprint from our plan. + let blueprint = build_initial_blueprint_from_plan( + &sled_configs_by_id, + service_plan, + ) + .map_err(SetupServiceError::ConvertPlanToBlueprint)?; info!(self.log, "Nexus address: {}", nexus_address.to_string()); @@ -699,30 +704,10 @@ impl ServiceInner { self.log.new(o!("component" => "NexusClient")), ); - // Ensure we can quickly look up "Sled Agent Address" -> "UUID of sled". - // - // We need the ID when passing info to Nexus. - let mut id_map = HashMap::new(); - for (_, sled_request) in sled_plan.sleds.iter() { - id_map.insert( - get_sled_address(sled_request.body.subnet), - sled_request.body.id, - ); - } - - // Convert all the information we have about services and datasets into - // a format which can be processed by Nexus. - let mut services: Vec = vec![]; + // Convert all the information we have about datasets into a format + // which can be processed by Nexus. let mut datasets: Vec = vec![]; - for (addr, sled_config) in service_plan.services.iter() { - let sled_id = *id_map - .get(addr) - .expect("Sled address in service plan, but not sled plan"); - - for zone in &sled_config.zones { - services.push(zone.to_nexus_service_req(sled_id)); - } - + for sled_config in service_plan.services.values() { for zone in &sled_config.zones { if let Some((dataset_name, dataset_address)) = zone.dataset_name_and_address() @@ -817,11 +802,9 @@ impl ServiceInner { info!(self.log, "rack_network_config: {:#?}", rack_network_config); - let physical_disks: Vec<_> = service_plan - .services + let physical_disks: Vec<_> = sled_configs_by_id .iter() - .flat_map(|(addr, config)| { - let sled_id = id_map.get(addr).expect("Missing sled"); + .flat_map(|(sled_id, config)| { config.disks.disks.iter().map(|config| { NexusTypes::PhysicalDiskPutRequest { id: config.id, @@ -835,11 +818,9 @@ impl ServiceInner { }) .collect(); - let zpools = service_plan - .services + let zpools = sled_configs_by_id .iter() - .flat_map(|(addr, config)| { - let sled_id = id_map.get(addr).expect("Missing sled"); + .flat_map(|(sled_id, config)| { config.disks.disks.iter().map(|config| { NexusTypes::ZpoolPutRequest { id: config.pool_id.into_untyped_uuid(), @@ -852,7 +833,6 @@ impl ServiceInner { let request = NexusTypes::RackInitializationRequest { blueprint, - services, physical_disks, zpools, datasets, @@ -1290,14 +1270,14 @@ impl DeployStepVersion { const V5_EVERYTHING: Generation = Self::V4_COCKROACHDB.next(); } -fn build_initial_blueprint_from_plan( +// Build a map of sled ID to `SledConfig` based on the two plan types we +// generate. This is a bit of a code smell (why doesn't the plan generate this +// on its own if we need it?); we should be able to get rid of it when +// we get to https://github.com/oxidecomputer/omicron/issues/5272. +fn build_sled_configs_by_id( sled_plan: &SledPlan, service_plan: &ServicePlan, -) -> anyhow::Result { - let internal_dns_version = - Generation::try_from(service_plan.dns_config.generation) - .context("invalid internal dns version")?; - +) -> anyhow::Result> { let mut sled_configs = BTreeMap::new(); for sled_request in sled_plan.sleds.values() { let sled_addr = get_sled_address(sled_request.body.subnet); @@ -1320,18 +1300,41 @@ fn build_initial_blueprint_from_plan( entry.insert(sled_config.clone()); } - Ok(build_initial_blueprint_from_sled_configs( - sled_configs, + if sled_configs.len() != service_plan.services.len() { + bail!( + "error mapping service plan to sled IDs; converted {} sled \ + addresses into {} sled configs", + service_plan.services.len(), + sled_configs.len(), + ); + } + + Ok(sled_configs) +} + +// Build an initial blueprint +fn build_initial_blueprint_from_plan( + sled_configs_by_id: &BTreeMap, + service_plan: &ServicePlan, +) -> anyhow::Result { + let internal_dns_version = + Generation::try_from(service_plan.dns_config.generation) + .context("invalid internal dns version")?; + + let blueprint = build_initial_blueprint_from_sled_configs( + &sled_configs_by_id, internal_dns_version, - )) + ); + + Ok(blueprint) } pub(crate) fn build_initial_blueprint_from_sled_configs( - sled_configs: BTreeMap, + sled_configs_by_id: &BTreeMap, internal_dns_version: Generation, ) -> Blueprint { let mut blueprint_zones = BTreeMap::new(); - for (sled_id, sled_config) in &sled_configs { + for (sled_id, sled_config) in sled_configs_by_id { let zones_config = BlueprintZonesConfig { // This is a bit of a hack. We only construct a blueprint after // completing RSS, so we need to know the final generation value @@ -1359,7 +1362,7 @@ pub(crate) fn build_initial_blueprint_from_sled_configs( } let mut blueprint_disks = BTreeMap::new(); - for (sled_id, sled_config) in &sled_configs { + for (sled_id, sled_config) in sled_configs_by_id { blueprint_disks.insert( SledUuid::from_untyped_uuid(*sled_id), BlueprintPhysicalDisksConfig { diff --git a/sled-agent/src/sim/server.rs b/sled-agent/src/sim/server.rs index 089760740a..6f931ea629 100644 --- a/sled-agent/src/sim/server.rs +++ b/sled-agent/src/sim/server.rs @@ -501,17 +501,14 @@ pub async fn run_standalone_server( }; let disks = server.sled_agent.omicron_physical_disks_list().await?; - let services = - zones.iter().map(|z| z.to_nexus_service_req(config.id)).collect(); let mut sled_configs = BTreeMap::new(); sled_configs.insert(config.id, SledConfig { disks, zones }); let rack_init_request = NexusTypes::RackInitializationRequest { blueprint: build_initial_blueprint_from_sled_configs( - sled_configs, + &sled_configs, internal_dns_version, ), - services, physical_disks, zpools, datasets,