diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 97ba2538511..01dc6bca472 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -1143,9 +1143,10 @@ CREATE TYPE omicron.public.ip_kind AS ENUM ( ); /* - * External IP addresses used for guest instances + * External IP addresses used for guest instances and externally-facing + * services. */ -CREATE TABLE omicron.public.instance_external_ip ( +CREATE TABLE omicron.public.external_ip ( /* Identity metadata */ id UUID PRIMARY KEY, @@ -1219,7 +1220,7 @@ CREATE TABLE omicron.public.instance_external_ip ( * Index used to support quickly looking up children of the IP Pool range table, * when checking for allocated addresses during deletion. */ -CREATE INDEX ON omicron.public.instance_external_ip ( +CREATE INDEX ON omicron.public.external_ip ( ip_pool_id, ip_pool_range_id ) @@ -1232,13 +1233,13 @@ CREATE INDEX ON omicron.public.instance_external_ip ( * pools, _and_ on the fact that the number of ports assigned to each instance * is fixed at compile time. */ -CREATE UNIQUE INDEX ON omicron.public.instance_external_ip ( +CREATE UNIQUE INDEX ON omicron.public.external_ip ( ip, first_port ) WHERE time_deleted IS NULL; -CREATE UNIQUE INDEX ON omicron.public.instance_external_ip ( +CREATE UNIQUE INDEX ON omicron.public.external_ip ( instance_id, id ) diff --git a/nexus/db-model/src/external_ip.rs b/nexus/db-model/src/external_ip.rs index 2571eb7714f..75c51835403 100644 --- a/nexus/db-model/src/external_ip.rs +++ b/nexus/db-model/src/external_ip.rs @@ -6,7 +6,7 @@ //! services. use crate::impl_enum_type; -use crate::schema::instance_external_ip; +use crate::schema::external_ip; use crate::Name; use crate::SqlU16; use chrono::DateTime; @@ -35,7 +35,8 @@ impl_enum_type!( Service => b"service" ); -/// The main model type for external IP addresses for instances. +/// The main model type for external IP addresses for instances +/// and externally-facing services. /// /// This encompasses the three flavors of external IPs: automatic source NAT /// IPs, Ephemeral IPs, and Floating IPs. The first two are similar in that they @@ -45,8 +46,8 @@ impl_enum_type!( /// API at all, and only provide outbound connectivity to instances, not /// inbound. #[derive(Debug, Clone, Selectable, Queryable, Insertable)] -#[diesel(table_name = instance_external_ip)] -pub struct InstanceExternalIp { +#[diesel(table_name = external_ip)] +pub struct ExternalIp { pub id: Uuid, // Only Some(_) for Floating IPs pub name: Option, @@ -69,8 +70,8 @@ pub struct InstanceExternalIp { pub last_port: SqlU16, } -impl From for sled_agent_client::types::SourceNatConfig { - fn from(eip: InstanceExternalIp) -> Self { +impl From for sled_agent_client::types::SourceNatConfig { + fn from(eip: ExternalIp) -> Self { Self { ip: eip.ip.ip(), first_port: eip.first_port.0, @@ -94,7 +95,7 @@ pub enum IpSource { /// An incomplete external IP, used to store state required for issuing the /// database query that selects an available IP and stores the resulting record. #[derive(Debug, Clone)] -pub struct IncompleteInstanceExternalIp { +pub struct IncompleteExternalIp { id: Uuid, name: Option, description: Option, @@ -104,7 +105,7 @@ pub struct IncompleteInstanceExternalIp { source: IpSource, } -impl IncompleteInstanceExternalIp { +impl IncompleteExternalIp { pub fn for_instance_source_nat( id: Uuid, project_id: Uuid, @@ -212,10 +213,10 @@ impl TryFrom for shared::IpKind { } } -impl TryFrom for views::ExternalIp { +impl TryFrom for views::ExternalIp { type Error = Error; - fn try_from(ip: InstanceExternalIp) -> Result { + fn try_from(ip: ExternalIp) -> Result { let kind = ip.kind.try_into()?; Ok(views::ExternalIp { kind, ip: ip.ip.ip() }) } diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index dd9dfe3150f..73d91b11a51 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -168,7 +168,7 @@ table! { } table! { - instance_external_ip (id) { + external_ip (id) { id -> Uuid, name -> Nullable, description -> Nullable, diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 7d98e772f82..47f1a68b1d5 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -255,10 +255,7 @@ impl super::Nexus { .await?; // Ignore the count of addresses deleted self.db_datastore - .deallocate_instance_external_ip_by_instance_id( - opctx, - authz_instance.id(), - ) + .deallocate_external_ip_by_instance_id(opctx, authz_instance.id()) .await?; Ok(()) } diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs index 8005f134e67..42e85b31e30 100644 --- a/nexus/src/app/sagas/instance_create.rs +++ b/nexus/src/app/sagas/instance_create.rs @@ -605,7 +605,7 @@ async fn sic_allocate_instance_snat_ip_undo( OpContext::for_saga_action(&sagactx, &saga_params.serialized_authn); let ip_id = sagactx.lookup::("snat_ip_id")?; datastore - .deallocate_instance_external_ip(&opctx, ip_id) + .deallocate_external_ip(&opctx, ip_id) .await .map_err(ActionError::action_failed)?; Ok(()) @@ -668,7 +668,7 @@ async fn sic_allocate_instance_external_ip_undo( OpContext::for_saga_action(&sagactx, &saga_params.serialized_authn); let ip_id = repeat_saga_params.new_id; datastore - .deallocate_instance_external_ip(&opctx, ip_id) + .deallocate_external_ip(&opctx, ip_id) .await .map_err(ActionError::action_failed)?; Ok(()) diff --git a/nexus/src/db/datastore/instance_external_ip.rs b/nexus/src/db/datastore/external_ip.rs similarity index 82% rename from nexus/src/db/datastore/instance_external_ip.rs rename to nexus/src/db/datastore/external_ip.rs index ca330e9415b..4c908461924 100644 --- a/nexus/src/db/datastore/instance_external_ip.rs +++ b/nexus/src/db/datastore/external_ip.rs @@ -2,15 +2,15 @@ // 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 [`InstanceExternalIp`]s. +//! [`DataStore`] methods on [`ExternalIp`]s. use super::DataStore; use crate::context::OpContext; use crate::db; use crate::db::error::public_error_from_diesel_pool; use crate::db::error::ErrorHandler; -use crate::db::model::IncompleteInstanceExternalIp; -use crate::db::model::InstanceExternalIp; +use crate::db::model::ExternalIp; +use crate::db::model::IncompleteExternalIp; use crate::db::model::IpKind; use crate::db::model::IpPool; use crate::db::model::Name; @@ -36,14 +36,14 @@ impl DataStore { ip_id: Uuid, project_id: Uuid, instance_id: Uuid, - ) -> CreateResult { - let data = IncompleteInstanceExternalIp::for_instance_source_nat( + ) -> CreateResult { + let data = IncompleteExternalIp::for_instance_source_nat( ip_id, project_id, instance_id, /* pool_id = */ None, ); - self.allocate_instance_external_ip(opctx, data).await + self.allocate_external_ip(opctx, data).await } /// Create an Ephemeral IP address for an instance. @@ -54,7 +54,7 @@ impl DataStore { project_id: Uuid, instance_id: Uuid, pool_name: Option, - ) -> CreateResult { + ) -> CreateResult { let pool_id = if let Some(ref name) = pool_name { // We'd like to add authz checks here, and use the `LookupPath` // methods on the project-scoped view of this resource. It's not @@ -92,13 +92,13 @@ impl DataStore { } else { None }; - let data = IncompleteInstanceExternalIp::for_ephemeral( + let data = IncompleteExternalIp::for_ephemeral( ip_id, project_id, instance_id, pool_id, ); - self.allocate_instance_external_ip(opctx, data).await + self.allocate_external_ip(opctx, data).await } /// Allocates an IP address for internal service usage. @@ -107,19 +107,19 @@ impl DataStore { opctx: &OpContext, ip_id: Uuid, rack_id: Uuid, - ) -> CreateResult { + ) -> CreateResult { let (.., pool) = self.ip_pools_lookup_by_rack_id(opctx, rack_id).await?; - let data = IncompleteInstanceExternalIp::for_service(ip_id, pool.id()); - self.allocate_instance_external_ip(opctx, data).await + let data = IncompleteExternalIp::for_service(ip_id, pool.id()); + self.allocate_external_ip(opctx, data).await } - async fn allocate_instance_external_ip( + async fn allocate_external_ip( &self, opctx: &OpContext, - data: IncompleteInstanceExternalIp, - ) -> CreateResult { + data: IncompleteExternalIp, + ) -> CreateResult { NextExternalIp::new(data) .get_result_async(self.pool_authorized(opctx).await?) .await @@ -129,7 +129,7 @@ impl DataStore { use diesel::result::Error::NotFound; match e { Connection(Query(NotFound)) => Error::invalid_request( - "No external IP addresses available for new instance", + "No external IP addresses available", ), _ => public_error_from_diesel_pool(e, ErrorHandler::Server), } @@ -146,18 +146,18 @@ impl DataStore { /// - `Ok(false)`: The record was already deleted, such as by a previous /// call /// - `Err(_)`: Any other condition, including a non-existent record. - pub async fn deallocate_instance_external_ip( + pub async fn deallocate_external_ip( &self, opctx: &OpContext, ip_id: Uuid, ) -> Result { - use db::schema::instance_external_ip::dsl; + use db::schema::external_ip::dsl; let now = Utc::now(); - diesel::update(dsl::instance_external_ip) + diesel::update(dsl::external_ip) .filter(dsl::time_deleted.is_null()) .filter(dsl::id.eq(ip_id)) .set(dsl::time_deleted.eq(now)) - .check_if_exists::(ip_id) + .check_if_exists::(ip_id) .execute_and_check(self.pool_authorized(opctx).await?) .await .map(|r| match r.status { @@ -175,14 +175,14 @@ impl DataStore { /// if callers have some invariants they'd like to check. // TODO-correctness: This can't be used for Floating IPs, we'll need a // _detatch_ method for that. - pub async fn deallocate_instance_external_ip_by_instance_id( + pub async fn deallocate_external_ip_by_instance_id( &self, opctx: &OpContext, instance_id: Uuid, ) -> Result { - use db::schema::instance_external_ip::dsl; + use db::schema::external_ip::dsl; let now = Utc::now(); - diesel::update(dsl::instance_external_ip) + diesel::update(dsl::external_ip) .filter(dsl::time_deleted.is_null()) .filter(dsl::instance_id.eq(instance_id)) .filter(dsl::kind.ne(IpKind::Floating)) @@ -197,12 +197,12 @@ impl DataStore { &self, opctx: &OpContext, instance_id: Uuid, - ) -> LookupResult> { - use db::schema::instance_external_ip::dsl; - dsl::instance_external_ip + ) -> LookupResult> { + use db::schema::external_ip::dsl; + dsl::external_ip .filter(dsl::instance_id.eq(instance_id)) .filter(dsl::time_deleted.is_null()) - .select(InstanceExternalIp::as_select()) + .select(ExternalIp::as_select()) .get_results_async(self.pool_authorized(opctx).await?) .await .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) diff --git a/nexus/src/db/datastore/ip_pool.rs b/nexus/src/db/datastore/ip_pool.rs index 7d3a40ae390..cb229e6fce4 100644 --- a/nexus/src/db/datastore/ip_pool.rs +++ b/nexus/src/db/datastore/ip_pool.rs @@ -331,7 +331,7 @@ impl DataStore { authz_pool: &authz::IpPool, range: &IpRange, ) -> DeleteResult { - use db::schema::instance_external_ip; + use db::schema::external_ip; use db::schema::ip_pool_range::dsl; opctx.authorize(authz::Action::Modify, authz_pool).await?; @@ -370,12 +370,10 @@ impl DataStore { // Find external IPs allocated out of this pool and range. let range_id = range.id; let has_children = diesel::dsl::select(diesel::dsl::exists( - instance_external_ip::table - .filter(instance_external_ip::dsl::ip_pool_id.eq(pool_id)) - .filter( - instance_external_ip::dsl::ip_pool_range_id.eq(range_id), - ) - .filter(instance_external_ip::dsl::time_deleted.is_null()), + external_ip::table + .filter(external_ip::dsl::ip_pool_id.eq(pool_id)) + .filter(external_ip::dsl::ip_pool_range_id.eq(range_id)) + .filter(external_ip::dsl::time_deleted.is_null()), )) .get_result_async::(self.pool_authorized(opctx).await?) .await diff --git a/nexus/src/db/datastore/mod.rs b/nexus/src/db/datastore/mod.rs index 0f968715a83..4a7a4f055e9 100644 --- a/nexus/src/db/datastore/mod.rs +++ b/nexus/src/db/datastore/mod.rs @@ -44,10 +44,10 @@ mod console_session; mod dataset; mod device_auth; mod disk; +mod external_ip; mod global_image; mod identity_provider; mod instance; -mod instance_external_ip; mod ip_pool; mod network_interface; mod organization; @@ -233,7 +233,7 @@ mod test { use crate::db::identity::Resource; use crate::db::lookup::LookupPath; use crate::db::model::Dataset; - use crate::db::model::InstanceExternalIp; + use crate::db::model::ExternalIp; use crate::db::model::Rack; use crate::db::model::Region; use crate::db::model::Service; @@ -1010,13 +1010,12 @@ mod test { } #[tokio::test] - async fn test_deallocate_instance_external_ip_by_instance_id_is_idempotent() - { + async fn test_deallocate_external_ip_by_instance_id_is_idempotent() { use crate::db::model::IpKind; - use crate::db::schema::instance_external_ip::dsl; + use crate::db::schema::external_ip::dsl; let logctx = dev::test_setup_log( - "test_deallocate_instance_external_ip_by_instance_id_is_idempotent", + "test_deallocate_external_ip_by_instance_id_is_idempotent", ); let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; @@ -1025,7 +1024,7 @@ mod test { let now = Utc::now(); let instance_id = Uuid::new_v4(); let ips = (0..4) - .map(|i| InstanceExternalIp { + .map(|i| ExternalIp { id: Uuid::new_v4(), name: None, description: None, @@ -1044,7 +1043,7 @@ mod test { last_port: crate::db::model::SqlU16(10), }) .collect::>(); - diesel::insert_into(dsl::instance_external_ip) + diesel::insert_into(dsl::external_ip) .values(ips.clone()) .execute_async(datastore.pool()) .await @@ -1052,7 +1051,7 @@ mod test { // Delete everything, make sure we delete all records we made above let count = datastore - .deallocate_instance_external_ip_by_instance_id(&opctx, instance_id) + .deallocate_external_ip_by_instance_id(&opctx, instance_id) .await .expect("Failed to delete instance external IPs"); assert_eq!( @@ -1063,7 +1062,7 @@ mod test { // Do it again, we should get zero records let count = datastore - .deallocate_instance_external_ip_by_instance_id(&opctx, instance_id) + .deallocate_external_ip_by_instance_id(&opctx, instance_id) .await .expect("Failed to delete instance external IPs"); assert_eq!(count, 0, "Expected to delete zero IPs for the instance"); @@ -1073,19 +1072,18 @@ mod test { } #[tokio::test] - async fn test_deallocate_instance_external_ip_is_idempotent() { + async fn test_deallocate_external_ip_is_idempotent() { use crate::db::model::IpKind; - use crate::db::schema::instance_external_ip::dsl; + use crate::db::schema::external_ip::dsl; - let logctx = dev::test_setup_log( - "test_deallocate_instance_external_ip_is_idempotent", - ); + let logctx = + dev::test_setup_log("test_deallocate_external_ip_is_idempotent"); let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; // Create a record. let now = Utc::now(); - let ip = InstanceExternalIp { + let ip = ExternalIp { id: Uuid::new_v4(), name: None, description: None, @@ -1103,26 +1101,22 @@ mod test { first_port: crate::db::model::SqlU16(0), last_port: crate::db::model::SqlU16(10), }; - diesel::insert_into(dsl::instance_external_ip) + diesel::insert_into(dsl::external_ip) .values(ip.clone()) .execute_async(datastore.pool()) .await .unwrap(); // Delete it twice, make sure we get the right sentinel return values. - let deleted = datastore - .deallocate_instance_external_ip(&opctx, ip.id) - .await - .unwrap(); + let deleted = + datastore.deallocate_external_ip(&opctx, ip.id).await.unwrap(); assert!( deleted, "Got unexpected sentinel value back when \ deleting external IP the first time" ); - let deleted = datastore - .deallocate_instance_external_ip(&opctx, ip.id) - .await - .unwrap(); + let deleted = + datastore.deallocate_external_ip(&opctx, ip.id).await.unwrap(); assert!( !deleted, "Got unexpected sentinel value back when \ @@ -1131,7 +1125,7 @@ mod test { // Deleting a non-existing record fails assert!(datastore - .deallocate_instance_external_ip(&opctx, Uuid::nil()) + .deallocate_external_ip(&opctx, Uuid::nil()) .await .is_err()); @@ -1142,7 +1136,7 @@ mod test { #[tokio::test] async fn test_external_ip_check_constraints() { use crate::db::model::IpKind; - use crate::db::schema::instance_external_ip::dsl; + use crate::db::schema::external_ip::dsl; use async_bb8_diesel::ConnectionError::Query; use async_bb8_diesel::PoolError::Connection; use diesel::result::DatabaseErrorKind::CheckViolation; @@ -1160,7 +1154,7 @@ mod test { ) .unwrap(); let mut addresses = subnet.iter(); - let ip = InstanceExternalIp { + let ip = ExternalIp { id: Uuid::new_v4(), name: None, description: None, @@ -1192,7 +1186,7 @@ mod test { for name in names.iter() { for description in descriptions.iter() { for instance_id in instance_ids.iter() { - let new_ip = InstanceExternalIp { + let new_ip = ExternalIp { id: Uuid::new_v4(), name: name.clone(), description: description.clone(), @@ -1200,7 +1194,7 @@ mod test { instance_id: *instance_id, ..ip }; - let res = diesel::insert_into(dsl::instance_external_ip) + let res = diesel::insert_into(dsl::external_ip) .values(new_ip) .execute_async(datastore.pool()) .await; @@ -1238,7 +1232,7 @@ mod test { for name in names.iter() { for description in descriptions.iter() { for instance_id in instance_ids.iter() { - let new_ip = InstanceExternalIp { + let new_ip = ExternalIp { id: Uuid::new_v4(), name: name.clone(), description: description.clone(), @@ -1247,11 +1241,10 @@ mod test { instance_id: *instance_id, ..ip }; - let res = - diesel::insert_into(dsl::instance_external_ip) - .values(new_ip.clone()) - .execute_async(datastore.pool()) - .await; + let res = diesel::insert_into(dsl::external_ip) + .values(new_ip.clone()) + .execute_async(datastore.pool()) + .await; if name.is_none() && description.is_none() && instance_id.is_some() diff --git a/nexus/src/db/queries/external_ip.rs b/nexus/src/db/queries/external_ip.rs index 916bc62a64a..24092aa8f27 100644 --- a/nexus/src/db/queries/external_ip.rs +++ b/nexus/src/db/queries/external_ip.rs @@ -5,8 +5,8 @@ //! Implementation of queries for operating on external IP addresses from IP //! Pools. -use crate::db::model::IncompleteInstanceExternalIp; -use crate::db::model::InstanceExternalIp; +use crate::db::model::ExternalIp; +use crate::db::model::IncompleteExternalIp; use crate::db::model::IpKind; use crate::db::model::IpKindEnum; use crate::db::model::IpSource; @@ -31,10 +31,9 @@ type FromClause = type IpPoolRangeFromClause = FromClause; const IP_POOL_RANGE_FROM_CLAUSE: IpPoolRangeFromClause = IpPoolRangeFromClause::new(); -type InstanceExternalIpFromClause = - FromClause; -const INSTANCE_EXTERNAL_IP_FROM_CLAUSE: InstanceExternalIpFromClause = - InstanceExternalIpFromClause::new(); +type ExternalIpFromClause = FromClause; +const EXTERNAL_IP_FROM_CLAUSE: ExternalIpFromClause = + ExternalIpFromClause::new(); // The number of ports available to an instance when doing source NAT. Note // that for static NAT, this value isn't used, and all ports are available. @@ -65,7 +64,7 @@ const MAX_PORT: i32 = u16::MAX as _; /// In general, the query: /// /// - Selects the next available IP address and port range from _any_ IP Pool -/// - Inserts that record into the `instance_external_ip` table +/// - Inserts that record into the `external_ip` table /// - Updates the rcgen and time modified of the parent `ip_pool_range` table /// /// In detail, the query is: @@ -118,7 +117,7 @@ const MAX_PORT: i32 = u16::MAX as _; /// -- Join with existing IPs, selecting the first row from the /// -- address and port sequence subqueryes that has no match. I.e., /// -- is not yet reserved. -/// instance_external_ip +/// external_ip /// ON /// (ip, first_port, time_deleted IS NULL) = /// (candidate_ip, candidate_first_port, TRUE) @@ -134,7 +133,7 @@ const MAX_PORT: i32 = u16::MAX as _; /// -- everything else as it exists in the record. This should only be /// -- possible on replay of a saga node. /// INSERT INTO -/// instance_external_ip +/// external_ip /// (SELECT * FROM next_external_ip) /// ON CONFLICT (id) /// DO UPDATE SET @@ -213,7 +212,7 @@ const MAX_PORT: i32 = u16::MAX as _; /// violates that expectation. #[derive(Debug, Clone)] pub struct NextExternalIp { - ip: IncompleteInstanceExternalIp, + ip: IncompleteExternalIp, // Number of ports reserved per IP address. Only applicable if the IP kind // is snat. n_ports_per_chunk: i32, @@ -225,7 +224,7 @@ pub struct NextExternalIp { } impl NextExternalIp { - pub fn new(ip: IncompleteInstanceExternalIp) -> Self { + pub fn new(ip: IncompleteExternalIp) -> Self { let now = Utc::now(); let n_ports_per_chunk = i32::try_from(NUM_SOURCE_NAT_PORTS).unwrap(); Self { @@ -240,7 +239,7 @@ impl NextExternalIp { &'a self, mut out: AstPass<'_, 'a, Pg>, ) -> QueryResult<()> { - use schema::instance_external_ip::dsl; + use schema::external_ip::dsl; out.push_sql("SELECT "); // NOTE: These columns must be selected in the order in which they @@ -335,7 +334,7 @@ impl NextExternalIp { out.push_sql(") CROSS JOIN ("); self.push_port_sequence_subquery(out.reborrow())?; out.push_sql(") LEFT OUTER JOIN "); - INSTANCE_EXTERNAL_IP_FROM_CLAUSE.walk_ast(out.reborrow())?; + EXTERNAL_IP_FROM_CLAUSE.walk_ast(out.reborrow())?; // The JOIN conditions depend on the IP type. For automatic SNAT IP // addresses, we need to consider existing records with their port @@ -395,10 +394,10 @@ impl NextExternalIp { // the `time_deleted` is null. That means that if the record has been // soft-deleted, it won't be considered a match, and thus both the IP // and first port (in the join result) will be null. Note that there - // _is_ a record in the `instance_external_ip` table that has that IP + // _is_ a record in the `external_ip` table that has that IP // and possibly first port, but since it's been soft-deleted, it's not a // match. In that case, we can get away with _only_ filtering the join - // results on the IP from the `instance_external_ip` table being NULL. + // results on the IP from the `external_ip` table being NULL. out.push_sql(" WHERE ("); out.push_identifier(dsl::ip::NAME)?; out.push_sql(" IS NULL) OR ("); @@ -555,9 +554,7 @@ impl NextExternalIp { out.push_sql(" + 1 WHERE "); out.push_identifier(dsl::id::NAME)?; out.push_sql(" = (SELECT "); - out.push_identifier( - schema::instance_external_ip::ip_pool_range_id::NAME, - )?; + out.push_identifier(schema::external_ip::ip_pool_range_id::NAME)?; out.push_sql(" FROM next_external_ip) AND "); out.push_identifier(dsl::time_deleted::NAME)?; out.push_sql(" IS NULL RETURNING "); @@ -565,16 +562,16 @@ impl NextExternalIp { Ok(()) } - // Push the subquery that updates the actual `instance_external_ip` table + // Push the subquery that updates the actual `external_ip` table // with the candidate record created in the main `next_external_ip` CTE and // returns it. Note that this may just update the timestamps, if a record // with the same primary key is found. - fn push_update_instance_external_ip_subquery<'a>( + fn push_update_external_ip_subquery<'a>( &'a self, mut out: AstPass<'_, 'a, Pg>, ) -> QueryResult<()> { out.push_sql("INSERT INTO "); - INSTANCE_EXTERNAL_IP_FROM_CLAUSE.walk_ast(out.reborrow())?; + EXTERNAL_IP_FROM_CLAUSE.walk_ast(out.reborrow())?; out.push_sql( " (SELECT * FROM next_external_ip) \ ON CONFLICT (id) \ @@ -609,7 +606,7 @@ impl QueryFragment for NextExternalIp { // Push the subquery that potentially inserts this record, or ignores // primary key conflicts (for idempotency). out.push_sql("), external_ip AS ("); - self.push_update_instance_external_ip_subquery(out.reborrow())?; + self.push_update_external_ip_subquery(out.reborrow())?; // Push the subquery that bumps the `rcgen` of the IP Pool range table out.push_sql("), updated_pool_range AS ("); @@ -623,7 +620,7 @@ impl QueryFragment for NextExternalIp { } impl Query for NextExternalIp { - type SqlType = <>::SelectExpression as diesel::Expression>::SqlType; } @@ -798,9 +795,7 @@ mod tests { assert_eq!( err, Error::InvalidRequest { - message: String::from( - "No external IP addresses available for new instance" - ), + message: String::from("No external IP addresses available"), } ); context.success().await; @@ -857,7 +852,7 @@ mod tests { // Release the first context .db_datastore - .deallocate_instance_external_ip(&context.opctx, ips[0].id) + .deallocate_external_ip(&context.opctx, ips[0].id) .await .expect("Failed to release the first external IP address"); @@ -1074,11 +1069,7 @@ mod tests { assert_eq!( err, Error::InvalidRequest { - message: String::from( - // TODO: The error is a bit misleading; this isn't an IP - // intended for an instance necessarily. - "No external IP addresses available for new instance" - ), + message: String::from("No external IP addresses available"), } );