From 94d0ebfa99255a1aef1524fe89d267329e4e1b06 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 11 Oct 2023 09:47:05 -0500 Subject: [PATCH] rip out silo_id and is_default on ip_pools in db --- nexus/db-model/src/schema.rs | 2 -- nexus/db-queries/src/db/datastore/ip_pool.rs | 17 ++++++++------- nexus/db-queries/src/db/datastore/project.rs | 3 ++- nexus/src/app/ip_pool.rs | 4 +++- nexus/test-utils/src/resource_helpers.rs | 3 +-- nexus/tests/integration_tests/ip_pools.rs | 4 ++-- nexus/types/src/external_api/params.rs | 11 ---------- schema/crdb/7.0.0/up6.sql | 1 + schema/crdb/7.0.0/up7.sql | 3 +++ schema/crdb/dbinit.sql | 22 +------------------- 10 files changed, 23 insertions(+), 47 deletions(-) create mode 100644 schema/crdb/7.0.0/up6.sql create mode 100644 schema/crdb/7.0.0/up7.sql diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index 4ffbd21443..c1ba3f4299 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -453,8 +453,6 @@ table! { time_modified -> Timestamptz, time_deleted -> Nullable, rcgen -> Int8, - silo_id -> Nullable, - is_default -> Bool, } } diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index 90ed5f76c0..9feaef0c14 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -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?) @@ -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)) @@ -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) diff --git a/nexus/db-queries/src/db/datastore/project.rs b/nexus/db-queries/src/db/datastore/project.rs index 0285679cd5..97abbbe102 100644 --- a/nexus/db-queries/src/db/datastore/project.rs +++ b/nexus/db-queries/src/db/datastore/project.rs @@ -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?) diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index 9e06fda435..22b99a3b07 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -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 { diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 2368c3f568..35a4f6242b 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -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) } diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs index 27f4b04290..639ccd788a 100644 --- a/nexus/tests/integration_tests/ip_pools.rs +++ b/nexus/tests/integration_tests/ip_pools.rs @@ -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, ¶ms).await; assert_eq!(created_pool.identity.name, "p0"); diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 1a7bb36cb7..7fa984b4c0 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -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, - - /// 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 diff --git a/schema/crdb/7.0.0/up6.sql b/schema/crdb/7.0.0/up6.sql new file mode 100644 index 0000000000..3e5347e78c --- /dev/null +++ b/schema/crdb/7.0.0/up6.sql @@ -0,0 +1 @@ +DROP INDEX IF EXISTS one_default_pool_per_scope; \ No newline at end of file diff --git a/schema/crdb/7.0.0/up7.sql b/schema/crdb/7.0.0/up7.sql new file mode 100644 index 0000000000..a62bda1adc --- /dev/null +++ b/schema/crdb/7.0.0/up7.sql @@ -0,0 +1,3 @@ +ALTER TABLE omicron.public.ip_pool + DROP COLUMN IF EXISTS silo_id, + DROP COLUMN IF EXISTS is_default; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 2f632bd4a6..1b60bbb68c 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -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. */