Skip to content

Commit

Permalink
add vmm -> propolis ID constraint
Browse files Browse the repository at this point in the history
  • Loading branch information
gjcolombo committed Jun 5, 2024
1 parent b4dbc49 commit a64cba5
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 5 deletions.
8 changes: 6 additions & 2 deletions schema/crdb/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,6 @@ CREATE TABLE IF NOT EXISTS omicron.public.instance (
state_generation INT NOT NULL,

/* FK into `vmm` for the Propolis server that's backing this instance. */
/* TODO(gjc): add constraint that state = 'active' iff this is not NULL */
active_propolis_id UUID,

/* FK into `vmm` for the migration target Propolis server, if one exists. */
Expand Down Expand Up @@ -1041,7 +1040,12 @@ CREATE TABLE IF NOT EXISTS omicron.public.instance (
* deleted and recreated by the schema upgrade process; see the
* `separate-instance-and-vmm-states` schema change for details.
*/
state omicron.public.instance_state_v2 NOT NULL
state omicron.public.instance_state_v2 NOT NULL,

CONSTRAINT vmm_iff_active_propolis CHECK (
((state = 'vmm') AND (active_propolis_id IS NOT NULL)) OR
((state != 'vmm') AND (active_propolis_id IS NULL))
)
);

-- Names for instances within a project should be unique
Expand Down
9 changes: 6 additions & 3 deletions schema/crdb/separate-instance-and-vmm-states/README.adoc
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
This schema change splits the "instance state" enum that instances and VMMs
share into two enums, one for instance states and one for VMM states. Variants
used by only one of these objects only appear in the corresponding enum.
used by only one of these objects only appear in the corresponding enum. This
upgrade also adds a database-level constraint that requires that an instance's
state reports that it has an active VMM if and only if it has an active Propolis
ID.

This change is mechanically tricky for two reasons. First, the states instances
and VMMs have after an upgrade depends on the state that they have before the
Expand Down Expand Up @@ -43,8 +46,8 @@ Once all this work is done the original `instance_state` enum can be dropped.

There are two other small details to keep in mind:

- The upgrade process needs to delete and recreate the `sled_instance` view
because it depends on the instance and VMM state columns.
- Any views that depend on the affected state columns need to be dropped before
removing the old columns and recreated after the migration is done.
- Deleting and recreating a state column (instead of modifying it in place)
changes its column index in its table, so these columns need to be moved
to the (current) ends of the table definitions in dbinit.sql.
5 changes: 5 additions & 0 deletions schema/crdb/separate-instance-and-vmm-states/up22.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
ALTER TABLE omicron.public.instance
ADD CONSTRAINT IF NOT EXISTS vmm_iff_active_propolis CHECK (
((state = 'vmm') AND (active_propolis_id IS NOT NULL)) OR
((state != 'vmm') AND (active_propolis_id IS NULL))
)

0 comments on commit a64cba5

Please sign in to comment.