Skip to content

Rename InstanceExternalIp to ExternalIp #1635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Sep 6, 2022
11 changes: 6 additions & 5 deletions common/src/sql/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down Expand Up @@ -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
)
Expand All @@ -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
)
Expand Down
21 changes: 11 additions & 10 deletions nexus/db-model/src/external_ip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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<Name>,
Expand All @@ -69,8 +70,8 @@ pub struct InstanceExternalIp {
pub last_port: SqlU16,
}

impl From<InstanceExternalIp> for sled_agent_client::types::SourceNatConfig {
fn from(eip: InstanceExternalIp) -> Self {
impl From<ExternalIp> for sled_agent_client::types::SourceNatConfig {
fn from(eip: ExternalIp) -> Self {
Self {
ip: eip.ip.ip(),
first_port: eip.first_port.0,
Expand All @@ -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<Name>,
description: Option<String>,
Expand All @@ -104,7 +105,7 @@ pub struct IncompleteInstanceExternalIp {
source: IpSource,
}

impl IncompleteInstanceExternalIp {
impl IncompleteExternalIp {
pub fn for_instance_source_nat(
id: Uuid,
project_id: Uuid,
Expand Down Expand Up @@ -212,10 +213,10 @@ impl TryFrom<IpKind> for shared::IpKind {
}
}

impl TryFrom<InstanceExternalIp> for views::ExternalIp {
impl TryFrom<ExternalIp> for views::ExternalIp {
type Error = Error;

fn try_from(ip: InstanceExternalIp) -> Result<Self, Self::Error> {
fn try_from(ip: ExternalIp) -> Result<Self, Self::Error> {
let kind = ip.kind.try_into()?;
Ok(views::ExternalIp { kind, ip: ip.ip.ip() })
}
Expand Down
2 changes: 1 addition & 1 deletion nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ table! {
}

table! {
instance_external_ip (id) {
external_ip (id) {
id -> Uuid,
name -> Nullable<Text>,
description -> Nullable<Text>,
Expand Down
5 changes: 1 addition & 4 deletions nexus/src/app/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
4 changes: 2 additions & 2 deletions nexus/src/app/sagas/instance_create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Uuid>("snat_ip_id")?;
datastore
.deallocate_instance_external_ip(&opctx, ip_id)
.deallocate_external_ip(&opctx, ip_id)
.await
.map_err(ActionError::action_failed)?;
Ok(())
Expand Down Expand Up @@ -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(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,14 +36,14 @@ impl DataStore {
ip_id: Uuid,
project_id: Uuid,
instance_id: Uuid,
) -> CreateResult<InstanceExternalIp> {
let data = IncompleteInstanceExternalIp::for_instance_source_nat(
) -> CreateResult<ExternalIp> {
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.
Expand All @@ -54,7 +54,7 @@ impl DataStore {
project_id: Uuid,
instance_id: Uuid,
pool_name: Option<Name>,
) -> CreateResult<InstanceExternalIp> {
) -> CreateResult<ExternalIp> {
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
Expand Down Expand Up @@ -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.
Expand All @@ -107,19 +107,19 @@ impl DataStore {
opctx: &OpContext,
ip_id: Uuid,
rack_id: Uuid,
) -> CreateResult<InstanceExternalIp> {
) -> CreateResult<ExternalIp> {
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<InstanceExternalIp> {
data: IncompleteExternalIp,
) -> CreateResult<ExternalIp> {
NextExternalIp::new(data)
.get_result_async(self.pool_authorized(opctx).await?)
.await
Expand All @@ -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),
}
Expand All @@ -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<bool, Error> {
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::<InstanceExternalIp>(ip_id)
.check_if_exists::<ExternalIp>(ip_id)
.execute_and_check(self.pool_authorized(opctx).await?)
.await
.map(|r| match r.status {
Expand All @@ -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<usize, Error> {
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))
Expand All @@ -197,12 +197,12 @@ impl DataStore {
&self,
opctx: &OpContext,
instance_id: Uuid,
) -> LookupResult<Vec<InstanceExternalIp>> {
use db::schema::instance_external_ip::dsl;
dsl::instance_external_ip
) -> LookupResult<Vec<ExternalIp>> {
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))
Expand Down
12 changes: 5 additions & 7 deletions nexus/src/db/datastore/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;

Expand Down Expand Up @@ -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::<bool>(self.pool_authorized(opctx).await?)
.await
Expand Down
Loading