Skip to content

Commit

Permalink
[nexus] Silo IP pools schema change (#3981)
Browse files Browse the repository at this point in the history
This is quite small, but I'm pretty confident it's a sufficient basis
for the rest of the work in #3926, so we might as well review and merge
already.

A couple of minor pain points I ran into while doing this:

* Needed guidance on version number (3.0.0 it is) — it will be more
obvious when there are more, but I added a line to the readme anyway
* Diff output for
[`dbinit_equals_sum_of_all_up`](https://github.com/oxidecomputer/omicron/blob/fb16870de8c0dba92d0868a984dd715749141b73/nexus/tests/integration_tests/schema.rs#L601)
is terrible — thousands of lines with only a few relevant ones. It turns
out the column order matters, so if you add columns to a table in a
migration, you have to add them to the _end_ of the `create table` in
`dbinit.sql`.
  • Loading branch information
david-crespo authored Aug 29, 2023
1 parent fb16870 commit f5d8e09
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 4 deletions.
16 changes: 16 additions & 0 deletions nexus/db-model/src/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ use std::net::IpAddr;
use uuid::Uuid;

/// An IP Pool is a collection of IP addresses external to the rack.
///
/// IP pools can be external or internal. External IP pools can be associated
/// with a silo or project so that instance IP allocation draws from that pool
/// instead of a system pool.
#[derive(Queryable, Insertable, Selectable, Clone, Debug, Resource)]
#[diesel(table_name = ip_pool)]
pub struct IpPool {
Expand All @@ -37,6 +41,16 @@ pub struct IpPool {
/// Child resource generation number, for optimistic concurrency control of
/// the contained ranges.
pub rcgen: i64,

/// Silo, if IP pool is associated with a particular silo. Must be null
/// if internal is true. Must be non-null if project_id is non-null. When
/// project_id is non-null, silo_id will (naturally) be the ID of the
/// project's silo.
pub silo_id: Option<Uuid>,

/// Project, if IP pool is associated with a particular project. Must be
/// null if internal is true.
pub project_id: Option<Uuid>,
}

impl IpPool {
Expand All @@ -51,6 +65,8 @@ impl IpPool {
),
internal,
rcgen: 0,
silo_id: None,
project_id: None,
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,8 @@ table! {
time_deleted -> Nullable<Timestamptz>,
internal -> Bool,
rcgen -> Int8,
silo_id -> Nullable<Uuid>,
project_id -> Nullable<Uuid>,
}
}

Expand Down Expand Up @@ -1129,7 +1131,7 @@ table! {
///
/// This should be updated whenever the schema is changed. For more details,
/// refer to: schema/crdb/README.adoc
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(2, 0, 0);
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(3, 0, 0);

allow_tables_to_appear_in_same_query!(
system_update,
Expand Down
36 changes: 36 additions & 0 deletions schema/crdb/3.0.0/up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
-- CRDB documentation recommends the following:
-- "Execute schema changes either as single statements (as an implicit transaction),
-- or in an explicit transaction consisting of the single schema change statement."
--
-- For each schema change, we transactionally:
-- 1. Check the current version
-- 2. Apply the idempotent update

BEGIN;

SELECT CAST(
IF(
(
SELECT version = '2.0.0' and target_version = '3.0.0'
FROM omicron.public.db_metadata WHERE singleton = true
),
'true',
'Invalid starting version for schema change'
) AS BOOL
);

ALTER TABLE omicron.public.ip_pool
ADD COLUMN IF NOT EXISTS silo_ID UUID,
ADD COLUMN IF NOT EXISTS project_id UUID,

-- if silo_id is null, then project_id must be null
ADD CONSTRAINT IF NOT EXISTS project_implies_silo CHECK (
NOT ((silo_id IS NULL) AND (project_id IS NOT NULL))
),

-- if internal = true, non-null silo_id and project_id are not allowed
ADD CONSTRAINT IF NOT EXISTS internal_pools_have_null_silo_and_project CHECK (
NOT (INTERNAL AND ((silo_id IS NOT NULL) OR (project_id IS NOT NULL)))
);

COMMIT;
1 change: 1 addition & 0 deletions schema/crdb/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Assumptions:

Process:

* Choose a `NEW_VERSION` number. This should almost certainly be a major version bump over `OLD_VERSION`.
* Add a file to `schema/crdb/NEW_VERSION/up.sql` with your changes to the schema.
** This file should validate the expected current version transactionally.
** This file should only issue a single schema-modifying statement per transaction.
Expand Down
24 changes: 21 additions & 3 deletions schema/crdb/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1480,12 +1480,30 @@ CREATE TABLE IF NOT EXISTS omicron.public.ip_pool (
time_modified TIMESTAMPTZ NOT NULL,
time_deleted TIMESTAMPTZ,


/* Identifies if the IP Pool is dedicated to Control Plane services */
internal BOOL NOT NULL,

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

/*
* Fields representating association with a silo or project. silo_id must be
* non-null if project_id is non-null. When project_id is non-null, silo_id
* will (naturally) be the ID of the project's silo. Both must be null if
* internal is true, i.e., internal IP pools must be fleet-level pools.
*/
silo_id UUID,
project_id UUID,

-- if silo_id is null, then project_id must be null
CONSTRAINT project_implies_silo CHECK (
NOT ((silo_id IS NULL) AND (project_id IS NOT NULL))
),

-- if internal = true, non-null silo_id and project_id are not allowed
CONSTRAINT internal_pools_have_null_silo_and_project CHECK (
NOT (INTERNAL AND ((silo_id IS NOT NULL) OR (project_id IS NOT NULL)))
)
);

/*
Expand Down Expand Up @@ -2544,7 +2562,7 @@ INSERT INTO omicron.public.db_metadata (
version,
target_version
) VALUES
( TRUE, NOW(), NOW(), '2.0.0', NULL)
( TRUE, NOW(), NOW(), '3.0.0', NULL)
ON CONFLICT DO NOTHING;

COMMIT;

0 comments on commit f5d8e09

Please sign in to comment.