Skip to content

Commit

Permalink
rip out silo_id and is_default on ip_pools in db
Browse files Browse the repository at this point in the history
  • Loading branch information
david-crespo committed Oct 11, 2023
1 parent 6404e27 commit 94d0ebf
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 47 deletions.
2 changes: 0 additions & 2 deletions nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,6 @@ table! {
time_modified -> Timestamptz,
time_deleted -> Nullable<Timestamptz>,
rcgen -> Int8,
silo_id -> Nullable<Uuid>,
is_default -> Bool,
}
}

Expand Down
17 changes: 10 additions & 7 deletions nexus/db-queries/src/db/datastore/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ impl DataStore {
),
}
// != excludes nulls so we explicitly include them
.filter(dsl::silo_id.ne(*INTERNAL_SILO_ID).or(dsl::silo_id.is_null()))
// TODO: join to join table to exclude internal
// .filter(dsl::silo_id.ne(*INTERNAL_SILO_ID).or(dsl::silo_id.is_null()))
.filter(dsl::time_deleted.is_null())
.select(db::model::IpPool::as_select())
.get_results_async(&*self.pool_connection_authorized(opctx).await?)
Expand Down Expand Up @@ -233,9 +234,10 @@ impl DataStore {
let now = Utc::now();
let updated_rows = diesel::update(dsl::ip_pool)
// != excludes nulls so we explicitly include them
.filter(
dsl::silo_id.ne(*INTERNAL_SILO_ID).or(dsl::silo_id.is_null()),
)
// TODO:
// .filter(
// dsl::silo_id.ne(*INTERNAL_SILO_ID).or(dsl::silo_id.is_null()),
// )
.filter(dsl::time_deleted.is_null())
.filter(dsl::id.eq(authz_pool.id()))
.filter(dsl::rcgen.eq(db_pool.rcgen))
Expand Down Expand Up @@ -268,9 +270,10 @@ impl DataStore {
opctx.authorize(authz::Action::Modify, authz_pool).await?;
diesel::update(dsl::ip_pool)
// != excludes nulls so we explicitly include them
.filter(
dsl::silo_id.ne(*INTERNAL_SILO_ID).or(dsl::silo_id.is_null()),
)
// TODO: exclude internal pool the right way
// .filter(
// dsl::silo_id.ne(*INTERNAL_SILO_ID).or(dsl::silo_id.is_null()),
// )
.filter(dsl::id.eq(authz_pool.id()))
.filter(dsl::time_deleted.is_null())
.set(updates)
Expand Down
3 changes: 2 additions & 1 deletion nexus/db-queries/src/db/datastore/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,8 @@ impl DataStore {
// TODO(2148, 2056): filter only pools accessible by the given
// project, once specific projects for pools are implemented
// != excludes nulls so we explicitly include them
.filter(dsl::silo_id.ne(*INTERNAL_SILO_ID).or(dsl::silo_id.is_null()))
// TODO: filter out internal the right way
// .filter(dsl::silo_id.ne(*INTERNAL_SILO_ID).or(dsl::silo_id.is_null()))
.filter(dsl::time_deleted.is_null())
.select(db::model::IpPool::as_select())
.get_results_async(&*self.pool_connection_authorized(opctx).await?)
Expand Down
4 changes: 3 additions & 1 deletion nexus/src/app/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ use omicron_common::api::external::UpdateResult;
use ref_cast::RefCast;

fn is_internal(pool: &IpPool) -> bool {
pool.silo_id == Some(*INTERNAL_SILO_ID)
// pool.silo_id == Some(*INTERNAL_SILO_ID)
// TODO: this is no longer a simple function of the pool itself
false
}

impl super::Nexus {
Expand Down
3 changes: 1 addition & 2 deletions nexus/test-utils/src/resource_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,10 @@ pub async fn create_ip_pool(
name: pool_name.parse().unwrap(),
description: String::from("an ip pool"),
},
silo: None,
is_default: false,
},
)
.await;
// TODO: associate with fleet as a non-default like before?
let range = populate_ip_pool(client, pool_name, ip_range).await;
(pool, range)
}
Expand Down
4 changes: 2 additions & 2 deletions nexus/tests/integration_tests/ip_pools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) {
name: String::from("p0").parse().unwrap(),
description: String::from(""),
},
silo: Some(NameOrId::Name(cptestctx.silo_name.clone())),
is_default: false,
// silo: Some(NameOrId::Name(cptestctx.silo_name.clone())),
// is_default: false,
};
let created_pool = create_pool(client, &params).await;
assert_eq!(created_pool.identity.name, "p0");
Expand Down
11 changes: 0 additions & 11 deletions nexus/types/src/external_api/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,17 +731,6 @@ impl std::fmt::Debug for CertificateCreate {
pub struct IpPoolCreate {
#[serde(flatten)]
pub identity: IdentityMetadataCreateParams,

/// If an IP pool is associated with a silo, instance IP allocations in that
/// silo can draw from that pool.
pub silo: Option<NameOrId>,

/// Whether the IP pool is considered a default pool for its scope (fleet
/// or silo). If a pool is marked default and is associated with a silo,
/// instances created in that silo will draw IPs from that pool unless
/// another pool is specified at instance create time.
#[serde(default)]
pub is_default: bool,
}

/// Parameters for updating an IP Pool
Expand Down
1 change: 1 addition & 0 deletions schema/crdb/7.0.0/up6.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP INDEX IF EXISTS one_default_pool_per_scope;
3 changes: 3 additions & 0 deletions schema/crdb/7.0.0/up7.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE omicron.public.ip_pool
DROP COLUMN IF EXISTS silo_id,
DROP COLUMN IF EXISTS is_default;
22 changes: 1 addition & 21 deletions schema/crdb/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1498,29 +1498,9 @@ CREATE TABLE IF NOT EXISTS omicron.public.ip_pool (
time_deleted TIMESTAMPTZ,

/* The collection's child-resource generation number */
rcgen INT8 NOT NULL,

/*
* Association with a silo. silo_id is also used to mark an IP pool as
* "internal" by associating it with the oxide-internal silo. Null silo_id
* means the pool is can be used fleet-wide.
*/
silo_id UUID,

/* Is this the default pool for its scope (fleet or silo) */
is_default BOOLEAN NOT NULL DEFAULT FALSE
rcgen INT8 NOT NULL
);

/*
* Ensure there can only be one default pool for the fleet or a given silo.
* Coalesce is needed because otherwise different nulls are considered to be
* distinct from each other.
*/
CREATE UNIQUE INDEX IF NOT EXISTS one_default_pool_per_scope ON omicron.public.ip_pool (
COALESCE(silo_id, '00000000-0000-0000-0000-000000000000'::uuid)
) WHERE
is_default = true AND time_deleted IS NULL;

/*
* Index ensuring uniqueness of IP Pool names, globally.
*/
Expand Down

0 comments on commit 94d0ebf

Please sign in to comment.