From 2b0ff09ff18826b20ac469d4102d1999bfe02fa3 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Tue, 2 Jun 2026 12:08:48 -0700 Subject: [PATCH 1/5] Rename IP pool reservation type - Rename the IP Pool "reservation type" enum to "assignment" and rename the variants. These types will soon appear in public documentation, and these names are clearer and more obvious to customers. - Database schema / data migration for the rename - Clean up a bunch of tests, comments, error messages, etc that referred to "reserve" or the "reservation type" --- nexus/db-model/src/ip_pool.rs | 34 +- nexus/db-model/src/schema_versions.rs | 3 +- .../src/db/datastore/external_ip.rs | 10 +- .../src/db/datastore/external_subnet.rs | 4 +- nexus/db-queries/src/db/datastore/ip_pool.rs | 534 ++++++++---------- .../src/db/datastore/multicast/groups.rs | 30 +- nexus/db-queries/src/db/datastore/rack.rs | 2 +- .../src/db/pub_test_utils/multicast.rs | 4 +- .../db-queries/src/db/queries/external_ip.rs | 4 +- ...p_pool.sql => assign_ip_pool_to_silos.sql} | 4 +- ... => assign_ip_pool_to_system_services.sql} | 7 +- .../output/ip_pool_external_silo_link.sql | 2 +- nexus/db-schema/src/enums.rs | 2 +- nexus/db-schema/src/schema.rs | 2 +- nexus/src/app/ip_pool.rs | 54 +- nexus/tests/integration_tests/external_ips.rs | 2 +- nexus/tests/integration_tests/ip_pools.rs | 2 +- schema/crdb/dbinit.sql | 14 +- .../rename-ip-pool-reservation-type/up01.sql | 4 + .../rename-ip-pool-reservation-type/up02.sql | 2 + .../rename-ip-pool-reservation-type/up03.sql | 5 + .../rename-ip-pool-reservation-type/up04.sql | 2 + .../rename-ip-pool-reservation-type/up05.sql | 2 + .../rename-ip-pool-reservation-type/up06.sql | 1 + 24 files changed, 358 insertions(+), 372 deletions(-) rename nexus/db-queries/tests/output/{reserve_external_ip_pool.sql => assign_ip_pool_to_silos.sql} (75%) rename nexus/db-queries/tests/output/{reserve_internal_ip_pool.sql => assign_ip_pool_to_system_services.sql} (82%) create mode 100644 schema/crdb/rename-ip-pool-reservation-type/up01.sql create mode 100644 schema/crdb/rename-ip-pool-reservation-type/up02.sql create mode 100644 schema/crdb/rename-ip-pool-reservation-type/up03.sql create mode 100644 schema/crdb/rename-ip-pool-reservation-type/up04.sql create mode 100644 schema/crdb/rename-ip-pool-reservation-type/up05.sql create mode 100644 schema/crdb/rename-ip-pool-reservation-type/up06.sql diff --git a/nexus/db-model/src/ip_pool.rs b/nexus/db-model/src/ip_pool.rs index 70c4b6a852a..81993800562 100644 --- a/nexus/db-model/src/ip_pool.rs +++ b/nexus/db-model/src/ip_pool.rs @@ -48,7 +48,7 @@ impl std::fmt::Display for IpRangeConversionError { impl std::error::Error for IpRangeConversionError {} impl_enum_type!( - IpPoolReservationTypeEnum: + IpPoolAssignmentEnum: #[derive( AsExpression, @@ -62,17 +62,17 @@ impl_enum_type!( serde::Deserialize, serde::Serialize, )] - pub enum IpPoolReservationType; + pub enum IpPoolAssignment; - ExternalSilos => b"external_silos" - OxideInternal => b"oxide_internal" + Silos => b"silos" + SystemServices => b"system_services" ); -impl ::std::fmt::Display for IpPoolReservationType { +impl ::std::fmt::Display for IpPoolAssignment { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let s = match self { - IpPoolReservationType::ExternalSilos => "external_silos", - IpPoolReservationType::OxideInternal => "oxide_internal", + IpPoolAssignment::Silos => "silos", + IpPoolAssignment::SystemServices => "system_services", }; f.write_str(s) } @@ -163,8 +163,8 @@ pub struct IpPool { /// the contained ranges. pub rcgen: i64, - /// Indicates what the pool is reserved for. - pub reservation_type: IpPoolReservationType, + /// Indicates what the pool is assigned for. + pub assignment: IpPoolAssignment, /// Pool type for unicast (default) vs multicast pools. pub pool_type: IpPoolType, } @@ -176,7 +176,7 @@ impl IpPool { pub fn new( pool_identity: &external::IdentityMetadataCreateParams, ip_version: IpVersion, - reservation_type: IpPoolReservationType, + assignment: IpPoolAssignment, ) -> Self { Self { identity: IpPoolIdentity::new( @@ -186,7 +186,7 @@ impl IpPool { ip_version, pool_type: IpPoolType::Unicast, rcgen: 0, - reservation_type, + assignment, } } @@ -194,7 +194,7 @@ impl IpPool { pub fn new_multicast( pool_identity: &external::IdentityMetadataCreateParams, ip_version: IpVersion, - reservation_type: IpPoolReservationType, + assignment: IpPoolAssignment, ) -> Self { Self { identity: IpPoolIdentity::new( @@ -204,24 +204,24 @@ impl IpPool { ip_version, pool_type: IpPoolType::Multicast, rcgen: 0, - reservation_type, + assignment, } } /// Create a new IPv4 IP Pool. pub fn new_v4( pool_identity: &external::IdentityMetadataCreateParams, - reservation_type: IpPoolReservationType, + assignment: IpPoolAssignment, ) -> Self { - Self::new(pool_identity, IpVersion::V4, reservation_type) + Self::new(pool_identity, IpVersion::V4, assignment) } /// Create a new IPv6 IP Pool. pub fn new_v6( pool_identity: &external::IdentityMetadataCreateParams, - reservation_type: IpPoolReservationType, + assignment: IpPoolAssignment, ) -> Self { - Self::new(pool_identity, IpVersion::V6, reservation_type) + Self::new(pool_identity, IpVersion::V6, assignment) } } diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 5caf71e9e28..197a64d94b7 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: Version = Version::new(261, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(262, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ pub static KNOWN_VERSIONS: LazyLock> = LazyLock::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(262, "rename-ip-pool-reservation-type"), KnownVersion::new(261, "remove-add-zones-with-mupdate-override"), KnownVersion::new(260, "ereport-trim-serial-trailing-nulls"), KnownVersion::new(259, "vmm-failure-reason"), diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index 9b8ad3bb453..65b71a9e5ec 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -1340,7 +1340,7 @@ mod tests { }; use nexus_db_model::IpPoolResourceType; use nexus_db_model::IpVersion; - use nexus_db_model::{Generation, IpPoolReservationType, Ipv4Net, Ipv6Net}; + use nexus_db_model::{Generation, IpPoolAssignment, Ipv4Net, Ipv6Net}; use nexus_db_model::{ IncompleteIpPoolResource, IncompleteNetworkInterface, IncompleteVpc, VpcSubnet, @@ -1588,7 +1588,7 @@ mod tests { description: "Multicast default pool".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -1660,7 +1660,7 @@ mod tests { description: "Unicast default pool".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -1842,7 +1842,7 @@ mod tests { description: "Multicast default pool".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -1914,7 +1914,7 @@ mod tests { description: "Unicast default pool".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await diff --git a/nexus/db-queries/src/db/datastore/external_subnet.rs b/nexus/db-queries/src/db/datastore/external_subnet.rs index 0cdad2a6ef2..0a7f47d1f37 100644 --- a/nexus/db-queries/src/db/datastore/external_subnet.rs +++ b/nexus/db-queries/src/db/datastore/external_subnet.rs @@ -1591,7 +1591,7 @@ mod tests { use nexus_auth::authz; use nexus_db_model::IpAttachState; use nexus_db_model::IpPool; - use nexus_db_model::IpPoolReservationType; + use nexus_db_model::IpPoolAssignment; use nexus_db_model::Name; use nexus_db_model::Project; use nexus_db_model::SubnetPool; @@ -4152,7 +4152,7 @@ mod tests { IpPool::new( &identity, IpVersion::V6.into(), - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index e4101e60100..93ca5c1eb07 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -17,8 +17,8 @@ use crate::db::identity::Resource; use crate::db::model::IncompleteIpPoolResource; use crate::db::model::IpKind; use crate::db::model::IpPool; +use crate::db::model::IpPoolAssignment; use crate::db::model::IpPoolRange; -use crate::db::model::IpPoolReservationType; use crate::db::model::IpPoolResource; use crate::db::model::IpPoolResourceType; use crate::db::model::IpPoolType; @@ -50,7 +50,7 @@ use nexus_db_model::IpVersion; use nexus_db_model::Project; use nexus_db_model::Vpc; use nexus_db_schema::enums::IpKindEnum; -use nexus_db_schema::enums::IpPoolReservationTypeEnum; +use nexus_db_schema::enums::IpPoolAssignmentEnum; use nexus_types::silo::INTERNAL_SILO_ID; use omicron_common::address::IpRange; use omicron_common::address::{IPV4_SSM_SUBNET, IPV6_SSM_SUBNET}; @@ -108,10 +108,10 @@ pub struct DefaultIpPools { pub v6: Option<(authz::IpPool, IpPool)>, } -// Error message emitted when a user attempts to link an IP Pool and internal -// Silo, but the pool is already reserved for internal use, or vice versa. -const BAD_SILO_LINK_ERROR: &str = "IP Pools cannot be both linked to external \ - Silos and reserved for internal Oxide usage."; +// Error message emitted when a user attempts to link an IP Pool and a Silo, +// but the pool is assigned for system services use, or vice versa. +const BAD_SILO_LINK_ERROR: &str = "IP Pools cannot be both linked to Silos \ + and assigned for system services use."; // Error message emitted when a user attempts to unlink an IP Pool from a Silo // while the pool has external IP addresses or multicast groups allocated @@ -119,11 +119,11 @@ const BAD_SILO_LINK_ERROR: &str = "IP Pools cannot be both linked to external \ const POOL_HAS_IPS_ERROR: &str = "IP addresses from this pool are in use in the linked silo"; -// Error message emitted when a user attempts to unlink an IP Pool from the -// Oxide internal Silo, without at least one other IP Pool linked to it. -const LAST_POOL_ERROR: &str = "Cannot delete the last IP Pool reserved for \ - Oxide internal usage. Create and reserve at least one more IP Pool \ - before deleting this one."; +// Error message emitted when a user attempts to reassign the last IP Pool +// assigned for system services use. +const LAST_POOL_ERROR: &str = "Cannot reassign the last IP Pool assigned for \ + system services use. Create and assign at least one more IP Pool \ + before reassigning this one."; /// Check if pool selection has an IP version conflict. /// @@ -147,11 +147,11 @@ fn check_ip_version_conflict( } impl DataStore { - /// List IP Pools by their reservation type, IP version, and pool type. + /// List IP Pools by their assignment, IP version, and pool type. pub async fn ip_pools_list_paginated( &self, opctx: &OpContext, - reservation_type: IpPoolReservationType, + assignment: IpPoolAssignment, version: Option, pool_type: Option, pagparams: &PaginatedBy<'_>, @@ -180,7 +180,7 @@ impl DataStore { }; query .filter(ip_pool::time_deleted.is_null()) - .filter(ip_pool::reservation_type.eq(reservation_type)) + .filter(ip_pool::assignment.eq(assignment)) .select(IpPool::as_select()) .get_results_async(&*self.pool_connection_authorized(opctx).await?) .await @@ -197,7 +197,7 @@ impl DataStore { ) -> ListResultVec { self.ip_pools_list_paginated( opctx, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, None, None, pagparams, @@ -213,7 +213,7 @@ impl DataStore { ) -> ListResultVec { self.ip_pools_list_paginated( opctx, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, None, Some(IpPoolType::Multicast), pagparams, @@ -229,7 +229,7 @@ impl DataStore { ) -> ListResultVec { self.ip_pools_list_paginated( opctx, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, None, Some(IpPoolType::Unicast), pagparams, @@ -441,7 +441,7 @@ impl DataStore { } } - /// Look up internal service IP Pools for both IP versions. + /// Look up system services IP Pools for both IP versions. /// /// This is useful when you need to handle resources like external IPs where /// the actual address might be from either IP version. @@ -672,7 +672,7 @@ impl DataStore { Ok(authz_pool) } - /// Look up IP pool intended for internal services by its well-known name. + /// Look up system services IP pool by its well-known name. /// /// This method may require an index by Availability Zone in the future. // @@ -722,7 +722,7 @@ impl DataStore { /// Delete an IP Pool, and any links between it an any Silos. /// /// This fails if there are still IP Ranges in the pool, or if we're - /// deleting the last pool reserved for Oxide use. + /// deleting the last pool assigned for system services use. pub async fn ip_pool_delete( &self, opctx: &OpContext, @@ -753,18 +753,16 @@ impl DataStore { } // Add a small subquery, if needed, to ensure that we don't delete this - // IP Pool if it's the last reserved pool. There has to always be at + // IP Pool if it's the last system-services pool. There has to always be at // least one of these. - let enough_reserved_pools = if matches!( - db_pool.reservation_type, - IpPoolReservationType::ExternalSilos - ) { - diesel::dsl::sql::("TRUE") - } else { - diesel::dsl::sql::(&count_reserved_pools_subquery( - db_pool.reservation_type, - )) - }; + let enough_system_services_pools = + if matches!(db_pool.assignment, IpPoolAssignment::Silos) { + diesel::dsl::sql::("TRUE") + } else { + diesel::dsl::sql::( + &count_system_services_pools_subquery(db_pool.assignment), + ) + }; // Delete the pool, conditional on the rcgen not having changed. This // protects the delete from occuring if clients created a new IP range @@ -774,7 +772,7 @@ impl DataStore { .filter(dsl::time_deleted.is_null()) .filter(dsl::id.eq(authz_pool.id())) .filter(dsl::rcgen.eq(db_pool.rcgen)) - .filter(enough_reserved_pools) + .filter(enough_system_services_pools) .set(dsl::time_deleted.eq(now)) .execute_async(&*conn) .await @@ -811,15 +809,14 @@ impl DataStore { Ok(()) } - /// Check whether the pool is internal by checking that it exists and is - /// associated with the internal silo + /// Check whether the pool is assigned for system services. // - // TODO-remove: This should go away when we let operators reserve any IP - // Pools for internal Oxide usage. The pool belongs to them even in that - // case, and so we should show it to them. + // TODO-remove: This should go away when we let operators reassign any IP + // Pool. The pool belongs to them even in that case, and so we should show + // it to them. // // See https://github.com/oxidecomputer/omicron/issues/8947. - pub async fn ip_pool_is_internal( + pub async fn ip_pool_is_assigned_to_system_services( &self, opctx: &OpContext, authz_pool: &authz::IpPool, @@ -828,10 +825,7 @@ impl DataStore { ip_pool::table .find(authz_pool.id()) .filter(ip_pool::time_deleted.is_null()) - .select( - ip_pool::reservation_type - .eq(IpPoolReservationType::OxideInternal), - ) + .select(ip_pool::assignment.eq(IpPoolAssignment::SystemServices)) .first_async::( &*self.pool_connection_authorized(opctx).await?, ) @@ -866,20 +860,20 @@ impl DataStore { } /// Reserve an IP Pool for a specific use. - pub async fn ip_pool_reserve( + pub async fn ip_pool_assign( &self, opctx: &OpContext, authz_pool: &authz::IpPool, db_pool: &IpPool, - reservation_type: IpPoolReservationType, + assignment: IpPoolAssignment, ) -> UpdateResult<()> { - if db_pool.reservation_type == reservation_type { + if db_pool.assignment == assignment { return Err(Error::invalid_request(format!( - "IP Pool already has reservation type '{}'", - reservation_type, + "IP Pool already has assignment '{}'", + assignment, ))); } - let n_rows = reserve_ip_pool_query(db_pool, reservation_type) + let n_rows = assign_ip_pool_query(db_pool, assignment) .execute_async(&*self.pool_connection_authorized(opctx).await?) .await .map_err(|e| match e { @@ -1110,7 +1104,7 @@ impl DataStore { if ip_pool_resource.resource_id == INTERNAL_SILO_ID { return Err(Error::invalid_request( "IP Pools should not be linked to the internal Oxide silo. \ - Reserve the Pool for `oxide_internal` use instead.", + Assign the Pool for system services use instead.", )); } opctx @@ -1489,8 +1483,8 @@ impl DataStore { if authz_silo.id() == INTERNAL_SILO_ID { return Err(Error::internal_error( - "Cannot unlink an internally-reserved IP Pool. \ - Use the `reservation_type` column instead.", + "Cannot unlink an IP Pool assigned for system services. \ + Use the `assignment` column instead.", )); } @@ -2098,9 +2092,9 @@ impl DataStore { } } -// Sentinel we try to cast as a UUID in the database, when linking an IP Pool to -// a Silo of the wrong "type" -- i.e., linking to an external Silo if the Pool -// is already linked to our internal Silo, or vice versa. +// Sentinel we try to cast as a UUID in the database, when linking a +// system-services pool to a silo, or trying to assign a silo-linked pool for +// system services use. const BAD_SILO_LINK_SENTINEL: &str = "bad-link-type"; // Sentinel we try to cast as a UUID in the database when the IP Pool is @@ -2111,8 +2105,8 @@ const IP_POOL_DELETED_SENTINEL: &str = "ip-pool-deleted"; // between selecting it and trying to insert the link. const SILO_DELETED_SENTINEL: &str = "silo-deleted"; -// Sentinel we try to cast as an integer when removing the reservation on the -// last internal pool of a given type. +// Sentinel we try to cast as an integer when trying to reassign the last +// system-services pool of a given type. const LAST_POOL_SENTINEL: &str = "last-pool"; /// Extract the sentinel string from a UUID cast error message. @@ -2128,9 +2122,9 @@ fn extract_uuid_cast_sentinel(msg: &str) -> Option<&str> { // Query to conditionally link an IP Pool to an external customer Silo. // // This method returns a SQL query to conditionally insert a link between an IP -// Pool and a Silo. It maintains the invariant that a pool can be reserved for -// Oxide internal usage XOR linked to customer silos. It also checks that the -// pool and silo still exist when the query is run. +// Pool and a Silo. It maintains the invariant that a pool is assigned for +// system services XOR linked to customer silos. It also checks that the pool +// and silo still exist when the query is run. // // See `tests/output/ip_pool_external_silo_link.sql` for the full generated SQL. fn link_ip_pool_to_external_silo_query( @@ -2139,15 +2133,13 @@ fn link_ip_pool_to_external_silo_query( let mut builder = QueryBuilder::new(); // `ip_pool` CTE: select the pool by ID, ensuring it still exists. // Fail with a 'bad-link-type' sentinel (via CAST-to-UUID trick) if - // the pool is reserved for Oxide rather than external silos. + // the pool is assigned for system services rather than silos. // Also selects pool_type and ip_version for denormalization into // ip_pool_resource (needed for a partial index constraint on defaults). builder - .sql("WITH ip_pool AS (SELECT CAST(IF(reservation_type != ") + .sql("WITH ip_pool AS (SELECT CAST(IF(assignment != ") .param() - .bind::( - IpPoolReservationType::ExternalSilos, - ) + .bind::(IpPoolAssignment::Silos) .sql(", '") .sql(BAD_SILO_LINK_SENTINEL) .sql("', ") @@ -2362,64 +2354,64 @@ fn unlink_ip_pool_from_external_silo_query( } // Generate a small helper subquery which fails with a bool-cast error if there -// are fewer than 2 IP Pools reserved for the provided use. It must be internal. -fn count_reserved_pools_subquery( - reservation_type: IpPoolReservationType, +// are fewer than 2 IP Pools with the given assignment. It must be for system services. +fn count_system_services_pools_subquery( + assignment: IpPoolAssignment, ) -> String { - assert!(!matches!(reservation_type, IpPoolReservationType::ExternalSilos)); + assert!(!matches!(assignment, IpPoolAssignment::Silos)); format!( "CAST(IF(\ (SELECT COUNT(1) \ FROM ip_pool \ - WHERE time_deleted IS NULL AND reservation_type = '{}' LIMIT 2\ + WHERE time_deleted IS NULL AND assignment = '{}' LIMIT 2\ ) >= 2, \ 'true', \ '{}') \ AS BOOL)", - reservation_type, LAST_POOL_SENTINEL, + assignment, LAST_POOL_SENTINEL, ) } -// Conditionally reserve an IP Pool for a specific use. +// Conditionally reassign an IP Pool. // // # Panics // -// Panics if the current and new reservation type are the same. -fn reserve_ip_pool_query( +// Panics if the current and new assignment are the same. +fn assign_ip_pool_query( pool: &IpPool, - reservation_type: IpPoolReservationType, + assignment: IpPoolAssignment, ) -> TypedSqlQuery<()> { - assert_ne!(pool.reservation_type, reservation_type); - match pool.reservation_type { - IpPoolReservationType::ExternalSilos => { - reserve_external_ip_pool_query(pool, reservation_type) + assert_ne!(pool.assignment, assignment); + match pool.assignment { + IpPoolAssignment::Silos => { + assign_ip_pool_to_silos_query(pool, assignment) } - IpPoolReservationType::OxideInternal => { - reserve_internal_ip_pool_query(pool, reservation_type) + IpPoolAssignment::SystemServices => { + assign_ip_pool_to_system_services_query(pool, assignment) } } } -// Query to conditionally reserve an IP Pool that is currently reserved for -// external silo use. +// Query to conditionally reassign an IP Pool that is currently assigned for +// silo use. // // Checks that the pool is not currently linked to any silos first. Note that // this means there cannot be any silo-specific resources using the pool. -fn reserve_external_ip_pool_query( +fn assign_ip_pool_to_silos_query( ip_pool: &IpPool, - new_reservation_type: IpPoolReservationType, + new_assignment: IpPoolAssignment, ) -> TypedSqlQuery<()> { let mut builder = QueryBuilder::new(); builder - .sql("UPDATE ip_pool SET reservation_type = ") + .sql("UPDATE ip_pool SET assignment = ") .param() - .bind::(new_reservation_type) + .bind::(new_assignment) .sql(", time_modified = NOW() WHERE id = ") .param() .bind::(ip_pool.id()) - .sql(" AND time_deleted IS NULL AND reservation_type = ") + .sql(" AND time_deleted IS NULL AND assignment = ") .param() - .bind::(ip_pool.reservation_type) + .bind::(ip_pool.assignment) .sql( " AND CAST(IF(EXISTS(\ SELECT 1 \ @@ -2434,28 +2426,28 @@ fn reserve_external_ip_pool_query( builder.query() } -// Query to conditionally reserve an IP Pool that is currently reserved for Oxide -// internal use. +// Query to conditionally reassign an IP Pool that is currently assigned for +// system services use. // // Checks that: // // - There are no external IPs in use by Oxide resources. -// - There is at least one other internal pool of the same reservation type. -fn reserve_internal_ip_pool_query( +// - There is at least one other system-services pool of the same assignment. +fn assign_ip_pool_to_system_services_query( ip_pool: &IpPool, - new_reservation_type: IpPoolReservationType, + new_assignment: IpPoolAssignment, ) -> TypedSqlQuery<()> { let mut builder = QueryBuilder::new(); builder - .sql("UPDATE ip_pool SET reservation_type = ") + .sql("UPDATE ip_pool SET assignment = ") .param() - .bind::(new_reservation_type) + .bind::(new_assignment) .sql(", time_modified = NOW() WHERE id = ") .param() .bind::(ip_pool.id()) - .sql(" AND time_deleted IS NULL AND reservation_type = ") + .sql(" AND time_deleted IS NULL AND assignment = ") .param() - .bind::(ip_pool.reservation_type) + .bind::(ip_pool.assignment) // Generate div-by-zero error if there are IPs .sql( " AND (\ @@ -2473,17 +2465,16 @@ fn reserve_internal_ip_pool_query( LIMIT 1\ ), 1/0, 1) AS BOOL))", ) - // Generate int-cast error if this is the last pool of this reservation - // type. + // Generate int-cast error if this is the last pool of this assignment. .sql( " AND CAST(IF(\ (SELECT COUNT(1) \ FROM ip_pool \ WHERE time_deleted IS NULL \ - AND reservation_type = ", + AND assignment = ", ) .param() - .bind::(ip_pool.reservation_type) + .bind::(ip_pool.assignment) .sql( " \ LIMIT 2\ @@ -2505,7 +2496,7 @@ mod test { use crate::db::datastore::external_ip::FloatingIpAllocation; use crate::db::datastore::ip_pool::{ BAD_SILO_LINK_ERROR, LAST_POOL_ERROR, POOL_HAS_IPS_ERROR, - link_ip_pool_to_external_silo_query, reserve_ip_pool_query, + assign_ip_pool_query, link_ip_pool_to_external_silo_query, unlink_ip_pool_from_external_silo_query, }; use crate::db::explain::ExplainableAsync as _; @@ -2523,8 +2514,8 @@ mod test { }; use nexus_db_lookup::LookupPath; use nexus_db_model::{ - InternetGatewayIpPool, IpPoolIdentity, IpPoolReservationType, - IpPoolType, IpVersion, + InternetGatewayIpPool, IpPoolAssignment, IpPoolIdentity, IpPoolType, + IpVersion, }; use nexus_types::deployment::{ OmicronZoneExternalFloatingIp, OmicronZoneExternalIp, @@ -2591,11 +2582,7 @@ mod test { let pool1_for_silo = datastore .ip_pool_create( &opctx, - IpPool::new( - &identity, - IpVersion::V4, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V4, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); @@ -2689,11 +2676,7 @@ mod test { let second_silo_default = datastore .ip_pool_create( &opctx, - IpPool::new( - &identity, - IpVersion::V4, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V4, IpPoolAssignment::Silos), ) .await .expect("Failed to create pool"); @@ -2771,24 +2754,26 @@ mod test { } #[tokio::test] - async fn test_internal_ip_pools() { - let logctx = dev::test_setup_log("test_internal_ip_pools"); + async fn test_ip_pools_assigned_to_system_services() { + let logctx = + dev::test_setup_log("test_ip_pools_assigned_to_system_services"); let db = TestDatabase::new_with_datastore(&logctx.log).await; let (opctx, datastore) = (db.opctx(), db.datastore()); for version in [IpVersion::V4, IpVersion::V6] { - // confirm internal pools appear as internal + // confirm system services pools are identified correctly let (authz_pool, pool) = datastore .ip_pools_service_lookup(&opctx, version) .await .unwrap(); assert_eq!(pool.ip_version, version); - let is_internal = - datastore.ip_pool_is_internal(&opctx, &authz_pool).await; + let is_internal = datastore + .ip_pool_is_assigned_to_system_services(&opctx, &authz_pool) + .await; assert_eq!(is_internal, Ok(true)); - // another random pool should not be considered internal + // a silo pool should not be considered assigned for system services let identity = IdentityMetadataCreateParams { name: format!("other-{version}-pool").parse().unwrap(), description: "".to_string(), @@ -2796,11 +2781,7 @@ mod test { let other_pool = datastore .ip_pool_create( &opctx, - IpPool::new( - &identity, - version, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, version, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); @@ -2811,11 +2792,15 @@ mod test { other_pool.id(), LookupType::ById(other_pool.id()), ); - let is_internal = - datastore.ip_pool_is_internal(&opctx, &authz_other_pool).await; + let is_internal = datastore + .ip_pool_is_assigned_to_system_services( + &opctx, + &authz_other_pool, + ) + .await; assert_eq!(is_internal, Ok(false)); - // now link it to the current silo, and it is still not internal. + // now link it to the current silo; it is still not assigned for system services let silo_id = opctx.authn.silo_required().unwrap().id(); let is_default = matches!(version, IpVersion::V4); let link = IncompleteIpPoolResource { @@ -2829,8 +2814,12 @@ mod test { .await .expect("Failed to link IP pool to silo"); - let is_internal = - datastore.ip_pool_is_internal(&opctx, &authz_other_pool).await; + let is_internal = datastore + .ip_pool_is_assigned_to_system_services( + &opctx, + &authz_other_pool, + ) + .await; assert_eq!(is_internal, Ok(false)); } @@ -2869,11 +2858,7 @@ mod test { let pool = datastore .ip_pool_create( &opctx, - IpPool::new( - &identity, - IpVersion::V4, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V4, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); @@ -2986,11 +2971,7 @@ mod test { let pool = datastore .ip_pool_create( &opctx, - IpPool::new( - &identity, - IpVersion::V6, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V6, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); @@ -3133,11 +3114,7 @@ mod test { let pool = datastore .ip_pool_create( &opctx, - IpPool::new( - &identity, - version, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, version, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); @@ -3181,7 +3158,7 @@ mod test { IpPool::new_multicast( &identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3255,7 +3232,7 @@ mod test { IpPool::new_multicast( &identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3318,7 +3295,7 @@ mod test { IpPool::new_multicast( &identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3361,7 +3338,7 @@ mod test { IpPool::new_multicast( &default_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3465,7 +3442,7 @@ mod test { description: "IPv4 multicast pool".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3493,7 +3470,7 @@ mod test { description: "IPv6 multicast pool".to_string(), }, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3567,7 +3544,7 @@ mod test { IpPool::new_multicast( &ipv4_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3608,7 +3585,7 @@ mod test { IpPool::new_multicast( &ipv6_identity, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3686,7 +3663,7 @@ mod test { IpPool::new_multicast( &ssm_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3741,7 +3718,7 @@ mod test { IpPool::new_multicast( &asm_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3802,7 +3779,7 @@ mod test { IpPool::new_multicast( &asm_v6_identity, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3902,7 +3879,7 @@ mod test { IpPool::new_multicast( &asm_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -3958,7 +3935,7 @@ mod test { IpPool::new_multicast( &ssm_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -4019,7 +3996,7 @@ mod test { IpPool::new_multicast( &ssm_v6_identity, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -4096,8 +4073,8 @@ mod test { let db = TestDatabase::new_with_datastore(&logctx.log).await; let (opctx, datastore) = (db.opctx(), db.datastore()); - // Insert a bunch of pools, not linked to any silo, and so reserved for - // customer use. + // Insert a bunch of pools, not linked to any silo, and so assigned for + // silo use. const N_POOLS: usize = 20; let mut customer_pools = Vec::with_capacity(N_POOLS); for i in 0..N_POOLS { @@ -4112,7 +4089,7 @@ mod test { IpPool::new( &identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -4121,7 +4098,7 @@ mod test { } customer_pools.sort_by_key(|pool| pool.id()); - // Create a bunch which _are_ reserved for Oxide's usage. + // Create a bunch which _are_ assigned for system services use. let mut oxide_pools = Vec::with_capacity(N_POOLS); for i in 0..N_POOLS { // Create the pool @@ -4135,16 +4112,16 @@ mod test { IpPool::new( &identity, IpVersion::V4, - IpPoolReservationType::OxideInternal, + IpPoolAssignment::SystemServices, ), ) .await - .expect("Failed to create reserved IP pool"); + .expect("Failed to create IP pool"); oxide_pools.push(pool); } assert_eq!(oxide_pools.len(), N_POOLS); - let fetch_paginated = |reservation_type| async move { + let fetch_paginated = |assignment| async move { let mut found = Vec::with_capacity(N_POOLS); let mut paginator = Paginator::new( NonZeroU32::new(5).unwrap(), @@ -4154,7 +4131,7 @@ mod test { let batch = datastore .ip_pools_list_paginated( opctx, - reservation_type, + assignment, None, None, &PaginatedBy::Id(page.current_pagparams()), @@ -4167,20 +4144,20 @@ mod test { found }; - // Paginate all the customer-reserved. + // Paginate all the customer/silo pools. let customer_pools_found = - fetch_paginated(IpPoolReservationType::ExternalSilos).await; + fetch_paginated(IpPoolAssignment::Silos).await; assert_eq!(customer_pools.len(), customer_pools_found.len()); assert_eq!(customer_pools, customer_pools_found); - // Paginate all those reserved for Oxide. + // Paginate all those _assigned for system services use. // // Note that we have 2 extra pools today, which are the builtin service // pools. These will go away in the future, so we'll unfortunately need // to update this test at that time. Until then, fetch those service // pools explicitly and add them. let oxide_reserved_found = - fetch_paginated(IpPoolReservationType::OxideInternal).await; + fetch_paginated(IpPoolAssignment::SystemServices).await; let pools = datastore .ip_pools_service_lookup_both_versions(opctx) .await @@ -4239,10 +4216,9 @@ mod test { } #[tokio::test] - async fn cannot_link_oxide_internal_pool_to_external_silo() { - let logctx = dev::test_setup_log( - "cannot_link_oxide_internal_pool_to_external_silo", - ); + async fn cannot_link_system_services_pool_to_silo() { + let logctx = + dev::test_setup_log("cannot_link_system_services_pool_to_silo"); let db = TestDatabase::new_with_datastore(&logctx.log).await; let (opctx, datastore) = (db.opctx(), db.datastore()); @@ -4257,7 +4233,7 @@ mod test { IpPool::new( &identity, IpVersion::V4, - IpPoolReservationType::OxideInternal, + IpPoolAssignment::SystemServices, ), ) .await @@ -4273,7 +4249,7 @@ mod test { let res = datastore.ip_pool_link_silo(&opctx, link).await; let Err(Error::InvalidRequest { message }) = &res else { panic!( - "Expected to fail linking an internally-reserved \ + "Expected to fail linking an assigned for system services \ IP Pool to an external Silo, found: {res:#?}", ); }; @@ -4284,14 +4260,14 @@ mod test { } #[tokio::test] - async fn cannot_reserve_externally_linked_pool_for_internal_use() { + async fn cannot_assign_externally_linked_pool_to_system_services() { let logctx = dev::test_setup_log( - "cannot_reserve_externally_linked_pool_for_internal_use", + "cannot_assign_externally_linked_pool_to_system_services", ); let db = TestDatabase::new_with_datastore(&logctx.log).await; let (opctx, datastore) = (db.opctx(), db.datastore()); - // Create the pool, reserved for external silos. + // Create the pool, assigned for silo use. let identity = IdentityMetadataCreateParams { name: "external-ip-pool".parse().unwrap(), description: "".to_string(), @@ -4299,11 +4275,7 @@ mod test { let ip_pool = datastore .ip_pool_create( opctx, - IpPool::new( - &identity, - IpVersion::V4, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V4, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); @@ -4320,18 +4292,18 @@ mod test { .await .expect("Should be able to link unlinked pool to default silo"); - // We should fail to reserve it for Oxide-internal use now. + // We should fail to assign it for system services use now. let (authz_pool, db_pool) = LookupPath::new(opctx, datastore) .ip_pool_id(ip_pool.id()) .fetch_for(authz::Action::Modify) .await .unwrap(); let res = datastore - .ip_pool_reserve( + .ip_pool_assign( opctx, &authz_pool, &db_pool, - IpPoolReservationType::OxideInternal, + IpPoolAssignment::SystemServices, ) .await; let Err(Error::InvalidRequest { message }) = &res else { @@ -4361,11 +4333,7 @@ mod test { let ip_pool = datastore .ip_pool_create( opctx, - IpPool::new( - &identity, - IpVersion::V4, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V4, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); @@ -4417,11 +4385,7 @@ mod test { let ip_pool = datastore .ip_pool_create( opctx, - IpPool::new( - &identity, - IpVersion::V4, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V4, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); @@ -4473,11 +4437,7 @@ mod test { let ip_pool = datastore .ip_pool_create( opctx, - IpPool::new( - &identity, - IpVersion::V4, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V4, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); @@ -4551,11 +4511,7 @@ mod test { let ip_pool = datastore .ip_pool_create( opctx, - IpPool::new( - &identity, - IpVersion::V4, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V4, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); @@ -4638,7 +4594,7 @@ mod test { description: "IPv4 unicast pool".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -4670,7 +4626,7 @@ mod test { description: "IPv6 unicast pool".to_string(), }, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -4718,7 +4674,7 @@ mod test { description: "Second IPv4 unicast pool".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -4815,7 +4771,7 @@ mod test { description: "Unicast IPv4".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -4830,7 +4786,7 @@ mod test { description: "Unicast IPv6".to_string(), }, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -4845,7 +4801,7 @@ mod test { description: "Multicast IPv4".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -4860,7 +4816,7 @@ mod test { description: "Multicast IPv6".to_string(), }, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -4985,7 +4941,7 @@ mod test { IpPool::new_multicast( &mcast_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5037,7 +4993,7 @@ mod test { IpPool::new( &unicast_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5082,7 +5038,7 @@ mod test { description: "Multicast default pool".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5118,7 +5074,7 @@ mod test { description: "Unicast default pool".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5173,7 +5129,7 @@ mod test { IpPool::new( &identity_ipv6, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5202,7 +5158,7 @@ mod test { IpPool::new( &identity_ipv4, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5301,7 +5257,7 @@ mod test { IpPool::new( &identity_ipv6, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5330,7 +5286,7 @@ mod test { IpPool::new( &identity_ipv4, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5425,7 +5381,7 @@ mod test { description: "IPv6 only pool".to_string(), }, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5481,7 +5437,7 @@ mod test { description: "Test unicast V4".to_string(), }, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5496,7 +5452,7 @@ mod test { description: "Test unicast V6".to_string(), }, IpVersion::V6, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -5586,10 +5542,9 @@ mod test { } #[tokio::test] - async fn cannot_delete_last_internally_reserved_ip_pool() { - let logctx = dev::test_setup_log( - "cannot_delete_last_internally_reserved_ip_pool", - ); + async fn cannot_delete_last_system_services_ip_pool() { + let logctx = + dev::test_setup_log("cannot_delete_last_system_services_ip_pool"); let db = TestDatabase::new_with_datastore(&logctx.log).await; let (opctx, datastore) = (db.opctx(), db.datastore()); @@ -5604,7 +5559,7 @@ mod test { .ip_pool_delete(opctx, &pools.ipv4.authz_pool, &pools.ipv4.db_pool) .await .expect( - "Should be able to delete internally-reserved \ + "Should be able to delete assigned for system services \ IP Pool when at least one remains", ); @@ -5617,7 +5572,7 @@ mod test { let l = datastore .ip_pools_list_paginated( opctx, - IpPoolReservationType::OxideInternal, + IpPoolAssignment::SystemServices, None, None, &pagparams, @@ -5634,7 +5589,7 @@ mod test { let Err(Error::InvalidRequest { message }) = &res else { panic!( - "Should not be able to delete internally-reserved \ + "Should not be able to delete assigned for system services \ IP Pool when only one remains, found {res:#?}" ); }; @@ -5643,7 +5598,7 @@ mod test { let l = datastore .ip_pools_list_paginated( opctx, - IpPoolReservationType::OxideInternal, + IpPoolAssignment::SystemServices, None, None, &pagparams, @@ -5657,9 +5612,9 @@ mod test { } #[tokio::test] - async fn cannot_externally_reserve_last_internally_reserved_ip_pool() { + async fn cannot_assign_last_system_services_ip_pool_to_silos() { let logctx = dev::test_setup_log( - "cannot_externally_reserve_last_internally_reserved_ip_pool", + "cannot_assign_last_system_services_ip_pool_to_silos", ); let db = TestDatabase::new_with_datastore(&logctx.log).await; let (opctx, datastore) = (db.opctx(), db.datastore()); @@ -5670,18 +5625,18 @@ mod test { .await .unwrap(); - // We should be able to reserve one of these for external use. + // We should be able to assign one of these for silo use. let _ = datastore - .ip_pool_reserve( + .ip_pool_assign( opctx, &pools.ipv4.authz_pool, &pools.ipv4.db_pool, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ) .await .expect( - "Should be able to externally reserve IP Pool \ - when at least one internally-reserved pool remains", + "Should be able to assign to silos IP Pool \ + when at least one assigned for system services pool remains", ); // Check there's only one left. @@ -5693,7 +5648,7 @@ mod test { let l = datastore .ip_pools_list_paginated( opctx, - IpPoolReservationType::OxideInternal, + IpPoolAssignment::SystemServices, None, None, &pagparams, @@ -5702,20 +5657,20 @@ mod test { .unwrap(); assert_eq!(l.len(), 1); - // We should _not_ be able to reserve the other for external use now, - // because there's only one left for internal use. + // We should _not_ be able to assign the other for silo use now, + // because there's only one left for system services use. let res = datastore - .ip_pool_reserve( + .ip_pool_assign( opctx, &pools.ipv6.authz_pool, &pools.ipv6.db_pool, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ) .await; let Err(Error::InvalidRequest { message }) = &res else { panic!( - "Should not be able to externally-reserve an \ - internally-reserved IP Pool when only one remains, \ + "Should not be able to assign to silos an \ + assigned for system services IP Pool when only one remains, \ found {res:#?}" ); }; @@ -5724,7 +5679,7 @@ mod test { let l = datastore .ip_pools_list_paginated( opctx, - IpPoolReservationType::OxideInternal, + IpPoolAssignment::SystemServices, None, None, &pagparams, @@ -5738,9 +5693,9 @@ mod test { } #[tokio::test] - async fn cannot_externally_reserve_ip_pool_with_outstanding_external_ips() { + async fn cannot_assign_ip_pool_to_silos_with_outstanding_external_ips() { let logctx = dev::test_setup_log( - "cannot_externally_reserve_ip_pool_with_outstanding_external_ips", + "cannot_assign_ip_pool_to_silos_with_outstanding_external_ips", ); let db = TestDatabase::new_with_datastore(&logctx.log).await; let (opctx, datastore) = (db.opctx(), db.datastore()); @@ -5784,34 +5739,34 @@ mod test { .await .expect("Should be able to create zone external IP"); - // Should not be able to externally-reserve the IPv4 pool now, since + // Should not be able to assign to silos the IPv4 pool now, since // we've got an address in use. let res = datastore - .ip_pool_reserve( + .ip_pool_assign( opctx, &pools.ipv4.authz_pool, &pools.ipv4.db_pool, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ) .await; let Err(Error::InvalidRequest { message }) = &res else { panic!( - "Should not be able to externally reserve internal \ + "Should not be able to assign to silos internal \ IP Pool when an address is in use, found {res:#?}" ); }; assert_eq!(message.external_message(), POOL_HAS_IPS_ERROR); - // Delete the address, and now we can reserve the pool for external use. + // Delete the address, and now we can assign the pool for silo use. let _ = datastore .deallocate_external_ip(opctx, eip.id) .await .expect("Should be able to delete external IP"); - let _ = datastore.ip_pool_reserve( + let _ = datastore.ip_pool_assign( opctx, &pools.ipv4.authz_pool, &pools.ipv4.db_pool, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ).await .expect( "Should be able to delete internal IP Pool when more than one remains, \ @@ -5823,9 +5778,9 @@ mod test { } #[tokio::test] - async fn can_explain_reserve_external_ip_pool_query() { + async fn can_explain_assign_ip_pool_to_silos_query() { let logctx = - dev::test_setup_log("can_explain_reserve_external_ip_pool_query"); + dev::test_setup_log("can_explain_assign_ip_pool_to_silos_query"); let db = TestDatabase::new_with_pool(&logctx.log).await; let pool = db.pool(); let conn = pool.claim().await.unwrap(); @@ -5841,12 +5796,10 @@ mod test { ip_version: IpVersion::V4, pool_type: IpPoolType::Unicast, rcgen: 0, - reservation_type: IpPoolReservationType::ExternalSilos, + assignment: IpPoolAssignment::Silos, }; - let query = reserve_ip_pool_query( - &ip_pool, - IpPoolReservationType::OxideInternal, - ); + let query = + assign_ip_pool_query(&ip_pool, IpPoolAssignment::SystemServices); let _ = query .explain_async(&conn) .await @@ -5857,7 +5810,7 @@ mod test { } #[tokio::test] - async fn expectorate_reserve_external_ip_pool_query() { + async fn expectorate_assign_ip_pool_to_silos_query() { let ip_pool = IpPool { identity: IpPoolIdentity::new( uuid::uuid!("93fea64d-5d0a-4cc6-8f94-7c527ee640a9"), @@ -5869,23 +5822,22 @@ mod test { ip_version: IpVersion::V4, pool_type: IpPoolType::Unicast, rcgen: 0, - reservation_type: IpPoolReservationType::ExternalSilos, + assignment: IpPoolAssignment::Silos, }; - let query = reserve_ip_pool_query( - &ip_pool, - IpPoolReservationType::OxideInternal, - ); + let query = + assign_ip_pool_query(&ip_pool, IpPoolAssignment::SystemServices); expectorate_query_contents( &query, - "tests/output/reserve_external_ip_pool.sql", + "tests/output/assign_ip_pool_to_silos.sql", ) .await; } #[tokio::test] - async fn can_explain_reserve_internal_ip_pool_query() { - let logctx = - dev::test_setup_log("can_explain_reserve_internal_ip_pool_query"); + async fn can_explain_assign_ip_pool_to_system_services_query() { + let logctx = dev::test_setup_log( + "can_explain_assign_ip_pool_to_system_services_query", + ); let db = TestDatabase::new_with_pool(&logctx.log).await; let pool = db.pool(); let conn = pool.claim().await.unwrap(); @@ -5901,12 +5853,9 @@ mod test { ip_version: IpVersion::V4, pool_type: IpPoolType::Unicast, rcgen: 0, - reservation_type: IpPoolReservationType::OxideInternal, + assignment: IpPoolAssignment::SystemServices, }; - let query = reserve_ip_pool_query( - &ip_pool, - IpPoolReservationType::ExternalSilos, - ); + let query = assign_ip_pool_query(&ip_pool, IpPoolAssignment::Silos); let _ = query .explain_async(&conn) .await @@ -5917,7 +5866,7 @@ mod test { } #[tokio::test] - async fn expectorate_reserve_internal_ip_pool_query() { + async fn expectorate_assign_ip_pool_to_system_services_query() { let ip_pool = IpPool { identity: IpPoolIdentity::new( uuid::uuid!("93fea64d-5d0a-4cc6-8f94-7c527ee640a9"), @@ -5929,15 +5878,12 @@ mod test { ip_version: IpVersion::V4, pool_type: IpPoolType::Unicast, rcgen: 0, - reservation_type: IpPoolReservationType::OxideInternal, + assignment: IpPoolAssignment::SystemServices, }; - let query = reserve_ip_pool_query( - &ip_pool, - IpPoolReservationType::ExternalSilos, - ); + let query = assign_ip_pool_query(&ip_pool, IpPoolAssignment::Silos); expectorate_query_contents( &query, - "tests/output/reserve_internal_ip_pool.sql", + "tests/output/assign_ip_pool_to_system_services.sql", ) .await; } @@ -5959,11 +5905,7 @@ mod test { let pool = datastore .ip_pool_create( &opctx, - IpPool::new( - &identity, - IpVersion::V4, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V4, IpPoolAssignment::Silos), ) .await .expect("Should create unicast IP pool"); @@ -6072,7 +6014,7 @@ mod test { IpPool::new_multicast( &mcast_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -6190,11 +6132,7 @@ mod test { let pool = datastore .ip_pool_create( &opctx, - IpPool::new( - &identity, - IpVersion::V6, - IpPoolReservationType::ExternalSilos, - ), + IpPool::new(&identity, IpVersion::V6, IpPoolAssignment::Silos), ) .await .expect("Failed to create IP pool"); diff --git a/nexus/db-queries/src/db/datastore/multicast/groups.rs b/nexus/db-queries/src/db/datastore/multicast/groups.rs index 0fb1b6e1e2b..41f19abcb3e 100644 --- a/nexus/db-queries/src/db/datastore/multicast/groups.rs +++ b/nexus/db-queries/src/db/datastore/multicast/groups.rs @@ -931,8 +931,8 @@ mod tests { use crate::db::datastore::Error; use crate::db::datastore::LookupType; use crate::db::model::{ - IncompleteIpPoolResource, IpPool, IpPoolReservationType, - IpPoolResourceType, IpVersion, + IncompleteIpPoolResource, IpPool, IpPoolAssignment, IpPoolResourceType, + IpVersion, }; use crate::db::pub_test_utils::helpers::create_instance_with_vmm; use crate::db::pub_test_utils::{TestDatabase, multicast}; @@ -956,7 +956,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -1101,7 +1101,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -1252,7 +1252,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -1351,7 +1351,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -1459,7 +1459,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -1582,7 +1582,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -1688,7 +1688,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -1817,7 +1817,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -2150,7 +2150,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -2239,7 +2239,7 @@ mod tests { IpPool::new_multicast( &asm_pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -2311,7 +2311,7 @@ mod tests { IpPool::new_multicast( &ssm_pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -2417,7 +2417,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await @@ -2555,7 +2555,7 @@ mod tests { IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index 99867d79aab..dcfe124d897 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -1049,7 +1049,7 @@ impl DataStore { ), }, version, - nexus_db_model::IpPoolReservationType::OxideInternal, + nexus_db_model::IpPoolAssignment::SystemServices, ); match self.ip_pool_create(opctx, internal_pool).await { Ok(_) | Err(Error::ObjectAlreadyExists { .. }) => {} diff --git a/nexus/db-queries/src/db/pub_test_utils/multicast.rs b/nexus/db-queries/src/db/pub_test_utils/multicast.rs index ba576bb23cd..2ed27fe1c32 100644 --- a/nexus/db-queries/src/db/pub_test_utils/multicast.rs +++ b/nexus/db-queries/src/db/pub_test_utils/multicast.rs @@ -15,7 +15,7 @@ pub const NO_SOURCE_IPS: &[IpAddr] = &[]; use uuid::Uuid; use nexus_db_model::{ - IncompleteIpPoolResource, IncompleteVpc, IpPool, IpPoolReservationType, + IncompleteIpPoolResource, IncompleteVpc, IpPool, IpPoolAssignment, IpPoolResourceType, IpVersion, }; use nexus_types::external_api::vpc; @@ -108,7 +108,7 @@ pub async fn create_test_setup_with_range( IpPool::new_multicast( &pool_identity, IpVersion::V4, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ) .await diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index 34e9f06baac..eab3491b649 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -899,7 +899,7 @@ mod tests { use nexus_db_model::IncompleteIpPoolResource; use nexus_db_model::Instance; use nexus_db_model::InstanceCpuCount; - use nexus_db_model::IpPoolReservationType; + use nexus_db_model::IpPoolAssignment; use nexus_db_model::IpPoolResourceType; use nexus_db_model::Name; use nexus_db_model::NetworkInterfaceKind; @@ -952,7 +952,7 @@ mod tests { description: format!("ip pool {}", name), }, ip_version.into(), - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ); let db_pool = self diff --git a/nexus/db-queries/tests/output/reserve_external_ip_pool.sql b/nexus/db-queries/tests/output/assign_ip_pool_to_silos.sql similarity index 75% rename from nexus/db-queries/tests/output/reserve_external_ip_pool.sql rename to nexus/db-queries/tests/output/assign_ip_pool_to_silos.sql index 8dacb652b63..8b6e2bdbecd 100644 --- a/nexus/db-queries/tests/output/reserve_external_ip_pool.sql +++ b/nexus/db-queries/tests/output/assign_ip_pool_to_silos.sql @@ -1,11 +1,11 @@ UPDATE ip_pool SET - reservation_type = $1, time_modified = now() + assignment = $1, time_modified = now() WHERE id = $2 AND time_deleted IS NULL - AND reservation_type = $3 + AND assignment = $3 AND CAST( IF( EXISTS(SELECT 1 FROM ip_pool_resource WHERE ip_pool_id = $4 LIMIT 1), diff --git a/nexus/db-queries/tests/output/reserve_internal_ip_pool.sql b/nexus/db-queries/tests/output/assign_ip_pool_to_system_services.sql similarity index 82% rename from nexus/db-queries/tests/output/reserve_internal_ip_pool.sql rename to nexus/db-queries/tests/output/assign_ip_pool_to_system_services.sql index 4421c5612b2..5e6081b25b0 100644 --- a/nexus/db-queries/tests/output/reserve_internal_ip_pool.sql +++ b/nexus/db-queries/tests/output/assign_ip_pool_to_system_services.sql @@ -1,11 +1,11 @@ UPDATE ip_pool SET - reservation_type = $1, time_modified = now() + assignment = $1, time_modified = now() WHERE id = $2 AND time_deleted IS NULL - AND reservation_type = $3 + AND assignment = $3 AND ( SELECT CAST( @@ -28,8 +28,7 @@ WHERE ) AND CAST( IF( - (SELECT count(1) FROM ip_pool WHERE time_deleted IS NULL AND reservation_type = $5 LIMIT 2) - >= 2, + (SELECT count(1) FROM ip_pool WHERE time_deleted IS NULL AND assignment = $5 LIMIT 2) >= 2, '1', $6 ) diff --git a/nexus/db-queries/tests/output/ip_pool_external_silo_link.sql b/nexus/db-queries/tests/output/ip_pool_external_silo_link.sql index 8e4b7024b9b..d7395a0dcf9 100644 --- a/nexus/db-queries/tests/output/ip_pool_external_silo_link.sql +++ b/nexus/db-queries/tests/output/ip_pool_external_silo_link.sql @@ -2,7 +2,7 @@ WITH ip_pool AS ( SELECT - CAST(IF(reservation_type != $1, 'bad-link-type', $2) AS UUID) AS id, pool_type, ip_version + CAST(IF(assignment != $1, 'bad-link-type', $2) AS UUID) AS id, pool_type, ip_version FROM ip_pool WHERE diff --git a/nexus/db-schema/src/enums.rs b/nexus/db-schema/src/enums.rs index cddaf8f4d36..03e12f2aa0e 100644 --- a/nexus/db-schema/src/enums.rs +++ b/nexus/db-schema/src/enums.rs @@ -74,7 +74,7 @@ define_enums! { InvZpoolHealthEnum => "inv_zpool_health", IpAttachStateEnum => "ip_attach_state", IpKindEnum => "ip_kind", - IpPoolReservationTypeEnum => "ip_pool_reservation_type", + IpPoolAssignmentEnum => "ip_pool_assignment", IpPoolResourceTypeEnum => "ip_pool_resource_type", IpPoolTypeEnum => "ip_pool_type", IpVersionEnum => "ip_version", diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 05d6c96b1db..f5196fa9d18 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -672,7 +672,7 @@ table! { time_deleted -> Nullable, rcgen -> Int8, ip_version -> crate::enums::IpVersionEnum, - reservation_type -> crate::enums::IpPoolReservationTypeEnum, + assignment -> crate::enums::IpPoolAssignmentEnum, pool_type -> crate::enums::IpPoolTypeEnum, } } diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index 75a4c52259b..fce312a4f9a 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -8,7 +8,7 @@ use ipnetwork::IpNetwork; use nexus_db_lookup::LookupPath; use nexus_db_lookup::lookup; use nexus_db_model::IpPool; -use nexus_db_model::IpPoolReservationType; +use nexus_db_model::IpPoolAssignment; use nexus_db_model::IpPoolType; use nexus_db_model::IpPoolUpdate; use nexus_db_model::IpVersion; @@ -209,12 +209,12 @@ impl super::Nexus { ip_pool::IpPoolType::Unicast => IpPool::new( &pool_params.identity, ip_version, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), ip_pool::IpPoolType::Multicast => IpPool::new_multicast( &pool_params.identity, ip_version, - IpPoolReservationType::ExternalSilos, + IpPoolAssignment::Silos, ), }; @@ -314,7 +314,11 @@ impl super::Nexus { let (authz_pool,) = pool_lookup.lookup_for(authz::Action::Modify).await?; - if self.db_datastore.ip_pool_is_internal(opctx, &authz_pool).await? { + if self + .db_datastore + .ip_pool_is_assigned_to_system_services(opctx, &authz_pool) + .await? + { return Err(not_found_from_lookup(pool_lookup)); } @@ -344,7 +348,11 @@ impl super::Nexus { let (.., authz_pool) = pool_lookup.lookup_for(authz::Action::Modify).await?; - if self.db_datastore.ip_pool_is_internal(opctx, &authz_pool).await? { + if self + .db_datastore + .ip_pool_is_assigned_to_system_services(opctx, &authz_pool) + .await? + { return Err(not_found_from_lookup(pool_lookup)); } @@ -366,7 +374,11 @@ impl super::Nexus { let (.., authz_pool) = pool_lookup.lookup_for(authz::Action::Modify).await?; - if self.db_datastore.ip_pool_is_internal(opctx, &authz_pool).await? { + if self + .db_datastore + .ip_pool_is_assigned_to_system_services(opctx, &authz_pool) + .await? + { return Err(not_found_from_lookup(pool_lookup)); } @@ -399,7 +411,11 @@ impl super::Nexus { let (.., authz_pool, db_pool) = pool_lookup.fetch_for(authz::Action::Delete).await?; - if self.db_datastore.ip_pool_is_internal(opctx, &authz_pool).await? { + if self + .db_datastore + .ip_pool_is_assigned_to_system_services(opctx, &authz_pool) + .await? + { return Err(not_found_from_lookup(pool_lookup)); } @@ -415,7 +431,11 @@ impl super::Nexus { let (.., authz_pool) = pool_lookup.lookup_for(authz::Action::Modify).await?; - if self.db_datastore.ip_pool_is_internal(opctx, &authz_pool).await? { + if self + .db_datastore + .ip_pool_is_assigned_to_system_services(opctx, &authz_pool) + .await? + { return Err(not_found_from_lookup(pool_lookup)); } @@ -433,7 +453,11 @@ impl super::Nexus { let (.., authz_pool) = pool_lookup.lookup_for(authz::Action::ListChildren).await?; - if self.db_datastore.ip_pool_is_internal(opctx, &authz_pool).await? { + if self + .db_datastore + .ip_pool_is_assigned_to_system_services(opctx, &authz_pool) + .await? + { return Err(not_found_from_lookup(pool_lookup)); } @@ -451,7 +475,11 @@ impl super::Nexus { let (.., authz_pool, db_pool) = pool_lookup.fetch_for(authz::Action::Modify).await?; - if self.db_datastore.ip_pool_is_internal(opctx, &authz_pool).await? { + if self + .db_datastore + .ip_pool_is_assigned_to_system_services(opctx, &authz_pool) + .await? + { return Err(not_found_from_lookup(pool_lookup)); } @@ -538,7 +566,11 @@ impl super::Nexus { let (.., authz_pool, _db_pool) = pool_lookup.fetch_for(authz::Action::Modify).await?; - if self.db_datastore.ip_pool_is_internal(opctx, &authz_pool).await? { + if self + .db_datastore + .ip_pool_is_assigned_to_system_services(opctx, &authz_pool) + .await? + { return Err(not_found_from_lookup(pool_lookup)); } diff --git a/nexus/tests/integration_tests/external_ips.rs b/nexus/tests/integration_tests/external_ips.rs index 8d4a37198f9..d09d60321aa 100644 --- a/nexus/tests/integration_tests/external_ips.rs +++ b/nexus/tests/integration_tests/external_ips.rs @@ -1463,7 +1463,7 @@ async fn test_floating_ip_ip_version_conflict( nexus_db_model::IpPool::new( &v6_identity, nexus_db_model::IpVersion::V6, - nexus_db_model::IpPoolReservationType::ExternalSilos, + nexus_db_model::IpPoolAssignment::Silos, ), ) .await diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs index f4f3a6d266d..c5b5b14e331 100644 --- a/nexus/tests/integration_tests/ip_pools.rs +++ b/nexus/tests/integration_tests/ip_pools.rs @@ -1000,7 +1000,7 @@ async fn test_ipv6_ip_pool_utilization_total( nexus_db_model::IpPool::new( &identity, nexus_db_model::IpVersion::V6, - nexus_db_model::IpPoolReservationType::ExternalSilos, + nexus_db_model::IpPoolAssignment::Silos, ), ) .await diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 308379c85c5..2eca6d47cf9 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -2254,10 +2254,10 @@ CREATE TYPE IF NOT EXISTS omicron.public.ip_version AS ENUM ( ); -/* Indicates what an IP Pool is reserved for. */ -CREATE TYPE IF NOT EXISTS omicron.public.ip_pool_reservation_type AS ENUM ( - 'external_silos', - 'oxide_internal' +/* Indicates what an IP Pool is assigned to. */ +CREATE TYPE IF NOT EXISTS omicron.public.ip_pool_assignment AS ENUM ( + 'silos', + 'system_services' ); /* @@ -2286,8 +2286,8 @@ CREATE TABLE IF NOT EXISTS omicron.public.ip_pool ( /* The IP version of the ranges contained in this pool. */ ip_version omicron.public.ip_version NOT NULL, - /* Indicates what the IP Pool is reserved for. */ - reservation_type omicron.public.ip_pool_reservation_type NOT NULL, + /* Indicates what the IP Pool is assigned to. */ + assignment omicron.public.ip_pool_assignment NOT NULL, /* Pool type for unicast (default) vs multicast pools. */ pool_type omicron.public.ip_pool_type NOT NULL DEFAULT 'unicast' @@ -8623,7 +8623,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '261.0.0', NULL) + (TRUE, NOW(), NOW(), '262.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/rename-ip-pool-reservation-type/up01.sql b/schema/crdb/rename-ip-pool-reservation-type/up01.sql new file mode 100644 index 00000000000..916ca48cf82 --- /dev/null +++ b/schema/crdb/rename-ip-pool-reservation-type/up01.sql @@ -0,0 +1,4 @@ +CREATE TYPE IF NOT EXISTS omicron.public.ip_pool_assignment AS ENUM ( + 'silos', + 'system_services' +); diff --git a/schema/crdb/rename-ip-pool-reservation-type/up02.sql b/schema/crdb/rename-ip-pool-reservation-type/up02.sql new file mode 100644 index 00000000000..7aa34087ed5 --- /dev/null +++ b/schema/crdb/rename-ip-pool-reservation-type/up02.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.ip_pool + ADD COLUMN IF NOT EXISTS assignment omicron.public.ip_pool_assignment; diff --git a/schema/crdb/rename-ip-pool-reservation-type/up03.sql b/schema/crdb/rename-ip-pool-reservation-type/up03.sql new file mode 100644 index 00000000000..c7a9b518eb8 --- /dev/null +++ b/schema/crdb/rename-ip-pool-reservation-type/up03.sql @@ -0,0 +1,5 @@ +UPDATE omicron.public.ip_pool + SET assignment = CASE reservation_type + WHEN 'external_silos' THEN 'silos'::omicron.public.ip_pool_assignment + ELSE 'system_services'::omicron.public.ip_pool_assignment + END; diff --git a/schema/crdb/rename-ip-pool-reservation-type/up04.sql b/schema/crdb/rename-ip-pool-reservation-type/up04.sql new file mode 100644 index 00000000000..148ce7f0f98 --- /dev/null +++ b/schema/crdb/rename-ip-pool-reservation-type/up04.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.ip_pool + ALTER COLUMN assignment SET NOT NULL; diff --git a/schema/crdb/rename-ip-pool-reservation-type/up05.sql b/schema/crdb/rename-ip-pool-reservation-type/up05.sql new file mode 100644 index 00000000000..bd4ad7be75e --- /dev/null +++ b/schema/crdb/rename-ip-pool-reservation-type/up05.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.ip_pool + DROP COLUMN IF EXISTS reservation_type; diff --git a/schema/crdb/rename-ip-pool-reservation-type/up06.sql b/schema/crdb/rename-ip-pool-reservation-type/up06.sql new file mode 100644 index 00000000000..000f4254ea1 --- /dev/null +++ b/schema/crdb/rename-ip-pool-reservation-type/up06.sql @@ -0,0 +1 @@ +DROP TYPE IF EXISTS omicron.public.ip_pool_reservation_type; From 2939cece58f912ebad0b656e1a4f5cc52c538f68 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Tue, 2 Jun 2026 13:15:38 -0700 Subject: [PATCH 2/5] Add new column at the end, allow full table scans in migration --- schema/crdb/dbinit.sql | 8 ++++---- schema/crdb/rename-ip-pool-reservation-type/up03.sql | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 2eca6d47cf9..09c538588f8 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -2286,11 +2286,11 @@ CREATE TABLE IF NOT EXISTS omicron.public.ip_pool ( /* The IP version of the ranges contained in this pool. */ ip_version omicron.public.ip_version NOT NULL, - /* Indicates what the IP Pool is assigned to. */ - assignment omicron.public.ip_pool_assignment NOT NULL, - /* Pool type for unicast (default) vs multicast pools. */ - pool_type omicron.public.ip_pool_type NOT NULL DEFAULT 'unicast' + pool_type omicron.public.ip_pool_type NOT NULL DEFAULT 'unicast', + + /* Indicates what the IP Pool is assigned to. */ + assignment omicron.public.ip_pool_assignment NOT NULL ); /* diff --git a/schema/crdb/rename-ip-pool-reservation-type/up03.sql b/schema/crdb/rename-ip-pool-reservation-type/up03.sql index c7a9b518eb8..fa157e9a060 100644 --- a/schema/crdb/rename-ip-pool-reservation-type/up03.sql +++ b/schema/crdb/rename-ip-pool-reservation-type/up03.sql @@ -1,3 +1,4 @@ +SET disallow_full_table_scans = 'off'; UPDATE omicron.public.ip_pool SET assignment = CASE reservation_type WHEN 'external_silos' THEN 'silos'::omicron.public.ip_pool_assignment From 3234f22f595c71b9f174e07ba4584a80288d6bc9 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Tue, 2 Jun 2026 15:40:01 -0700 Subject: [PATCH 3/5] Move column ordering in schema.rs too --- nexus/db-schema/src/schema.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index f5196fa9d18..8b34aac356f 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -672,8 +672,8 @@ table! { time_deleted -> Nullable, rcgen -> Int8, ip_version -> crate::enums::IpVersionEnum, - assignment -> crate::enums::IpPoolAssignmentEnum, pool_type -> crate::enums::IpPoolTypeEnum, + assignment -> crate::enums::IpPoolAssignmentEnum, } } From 5b81dff057137d3a5b35ba05f01f0c524ec950c4 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Wed, 3 Jun 2026 11:55:24 -0700 Subject: [PATCH 4/5] Review feedback --- nexus/db-queries/src/db/datastore/ip_pool.rs | 24 ++++++++++++------- .../rename-ip-pool-reservation-type/up03.sql | 2 +- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index 93ca5c1eb07..157e318b666 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -121,9 +121,15 @@ const POOL_HAS_IPS_ERROR: &str = // Error message emitted when a user attempts to reassign the last IP Pool // assigned for system services use. -const LAST_POOL_ERROR: &str = "Cannot reassign the last IP Pool assigned for \ - system services use. Create and assign at least one more IP Pool \ - before reassigning this one."; +const REASSIGN_LAST_POOL_ERROR: &str = "Cannot reassign the last IP Pool \ + assigned for system services use. Create and assign at least one more \ + IP Pool before reassigning this one."; + +// Error message emitted when a user attempts to delete the last IP Pool +// assigned for system services use. +const DELETE_LAST_POOL_ERROR: &str = "Cannot delete the last IP Pool \ + assigned for system services use. Create and assign at least one more \ + IP Pool before deleting this one."; /// Check if pool selection has an IP version conflict. /// @@ -781,7 +787,7 @@ impl DataStore { DatabaseErrorKind::Unknown, ref info, ) if info.message().ends_with("invalid bool value") => { - Error::invalid_request(LAST_POOL_ERROR) + Error::invalid_request(DELETE_LAST_POOL_ERROR) } _ => public_error_from_diesel( e, @@ -889,7 +895,7 @@ impl DataStore { } else if message.starts_with("could not parse") && message.contains("as type int") { - Error::invalid_request(LAST_POOL_ERROR) + Error::invalid_request(REASSIGN_LAST_POOL_ERROR) } else { public_error_from_diesel(e, ErrorHandler::Server) } @@ -2495,7 +2501,7 @@ mod test { use crate::authz; use crate::db::datastore::external_ip::FloatingIpAllocation; use crate::db::datastore::ip_pool::{ - BAD_SILO_LINK_ERROR, LAST_POOL_ERROR, POOL_HAS_IPS_ERROR, + BAD_SILO_LINK_ERROR, POOL_HAS_IPS_ERROR, REASSIGN_LAST_POOL_ERROR, assign_ip_pool_query, link_ip_pool_to_external_silo_query, unlink_ip_pool_from_external_silo_query, }; @@ -4150,7 +4156,7 @@ mod test { assert_eq!(customer_pools.len(), customer_pools_found.len()); assert_eq!(customer_pools, customer_pools_found); - // Paginate all those _assigned for system services use. + // Paginate all those assigned for system services use. // // Note that we have 2 extra pools today, which are the builtin service // pools. These will go away in the future, so we'll unfortunately need @@ -5593,7 +5599,7 @@ mod test { IP Pool when only one remains, found {res:#?}" ); }; - assert_eq!(message.external_message(), LAST_POOL_ERROR); + assert_eq!(message.external_message(), REASSIGN_LAST_POOL_ERROR); let l = datastore .ip_pools_list_paginated( @@ -5674,7 +5680,7 @@ mod test { found {res:#?}" ); }; - assert_eq!(message.external_message(), LAST_POOL_ERROR); + assert_eq!(message.external_message(), REASSIGN_LAST_POOL_ERROR); let l = datastore .ip_pools_list_paginated( diff --git a/schema/crdb/rename-ip-pool-reservation-type/up03.sql b/schema/crdb/rename-ip-pool-reservation-type/up03.sql index fa157e9a060..2fbb1457772 100644 --- a/schema/crdb/rename-ip-pool-reservation-type/up03.sql +++ b/schema/crdb/rename-ip-pool-reservation-type/up03.sql @@ -1,4 +1,4 @@ -SET disallow_full_table_scans = 'off'; +SET LOCAL disallow_full_table_scans = 'off'; UPDATE omicron.public.ip_pool SET assignment = CASE reservation_type WHEN 'external_silos' THEN 'silos'::omicron.public.ip_pool_assignment From d794b6e8890f6a36ca9eb5a92f20dab7a9805fc5 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Wed, 3 Jun 2026 12:44:28 -0700 Subject: [PATCH 5/5] Fixup error message assertion --- nexus/db-queries/src/db/datastore/ip_pool.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index 157e318b666..9d4f9b58139 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -2501,8 +2501,9 @@ mod test { use crate::authz; use crate::db::datastore::external_ip::FloatingIpAllocation; use crate::db::datastore::ip_pool::{ - BAD_SILO_LINK_ERROR, POOL_HAS_IPS_ERROR, REASSIGN_LAST_POOL_ERROR, - assign_ip_pool_query, link_ip_pool_to_external_silo_query, + BAD_SILO_LINK_ERROR, DELETE_LAST_POOL_ERROR, POOL_HAS_IPS_ERROR, + REASSIGN_LAST_POOL_ERROR, assign_ip_pool_query, + link_ip_pool_to_external_silo_query, unlink_ip_pool_from_external_silo_query, }; use crate::db::explain::ExplainableAsync as _; @@ -5599,7 +5600,7 @@ mod test { IP Pool when only one remains, found {res:#?}" ); }; - assert_eq!(message.external_message(), REASSIGN_LAST_POOL_ERROR); + assert_eq!(message.external_message(), DELETE_LAST_POOL_ERROR); let l = datastore .ip_pools_list_paginated(