diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 07e4fd0b83..58cace3032 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -2371,7 +2371,7 @@ pub struct SwitchPortLinkConfig { /// The link-layer discovery protocol service configuration id for this /// link. - pub lldp_link_config_id: Uuid, + pub lldp_link_config_id: Option, /// The name of this link. pub link_name: String, diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index f630bbbeac..f01f33c39d 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -144,7 +144,7 @@ table! { fec -> crate::SwitchLinkFecEnum, speed -> crate::SwitchLinkSpeedEnum, autoneg -> Bool, - lldp_link_config_id -> Uuid, + lldp_link_config_id -> Nullable, } } diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index aef95e6d53..eaed2990c5 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -17,7 +17,7 @@ use std::collections::BTreeMap; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(91, 0, 0); +pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(92, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy> = Lazy::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(92, "lldp-link-config-nullable"), KnownVersion::new(91, "add-management-gateway-producer-kind"), KnownVersion::new(90, "lookup-bgp-config-by-asn"), KnownVersion::new(89, "collapse_lldp_settings"), diff --git a/nexus/db-model/src/switch_port.rs b/nexus/db-model/src/switch_port.rs index 09f1327be2..bbcbb0748a 100644 --- a/nexus/db-model/src/switch_port.rs +++ b/nexus/db-model/src/switch_port.rs @@ -381,7 +381,7 @@ impl Into for SwitchPortConfig { #[diesel(table_name = switch_port_settings_link_config)] pub struct SwitchPortLinkConfig { pub port_settings_id: Uuid, - pub lldp_link_config_id: Uuid, + pub lldp_link_config_id: Option, pub link_name: String, pub mtu: SqlU16, pub fec: SwitchLinkFec, @@ -401,7 +401,7 @@ impl SwitchPortLinkConfig { ) -> Self { Self { port_settings_id, - lldp_link_config_id, + lldp_link_config_id: Some(lldp_link_config_id), link_name, fec, speed, diff --git a/nexus/db-queries/src/db/datastore/switch_port.rs b/nexus/db-queries/src/db/datastore/switch_port.rs index 2e09c1ac13..59748aa4db 100644 --- a/nexus/db-queries/src/db/datastore/switch_port.rs +++ b/nexus/db-queries/src/db/datastore/switch_port.rs @@ -455,7 +455,7 @@ impl DataStore { let lldp_link_ids: Vec = result .links .iter() - .map(|link| link.lldp_link_config_id) + .filter_map(|link| link.lldp_link_config_id) .collect(); use db::schema::lldp_link_config; @@ -1511,7 +1511,7 @@ async fn do_switch_port_settings_delete( // delete lldp configs use db::schema::lldp_link_config; let lldp_link_ids: Vec = - links.iter().map(|link| link.lldp_link_config_id).collect(); + links.iter().filter_map(|link| link.lldp_link_config_id).collect(); diesel::delete(lldp_link_config::dsl::lldp_link_config) .filter(lldp_link_config::id.eq_any(lldp_link_ids)) .execute_async(conn) diff --git a/openapi/nexus.json b/openapi/nexus.json index f6d140ed05..47f1f0822b 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -19335,6 +19335,7 @@ "type": "string" }, "lldp_link_config_id": { + "nullable": true, "description": "The link-layer discovery protocol service configuration id for this link.", "type": "string", "format": "uuid" @@ -19363,7 +19364,6 @@ "autoneg", "fec", "link_name", - "lldp_link_config_id", "mtu", "port_settings_id", "speed" diff --git a/schema/crdb/collapse_lldp_settings/up2.sql b/schema/crdb/collapse_lldp_settings/up2.sql index b2d884d068..8ead8a29b4 100644 --- a/schema/crdb/collapse_lldp_settings/up2.sql +++ b/schema/crdb/collapse_lldp_settings/up2.sql @@ -1,4 +1,4 @@ /* * Add a pointer to this link's LLDP config settings. */ -ALTER TABLE omicron.public.switch_port_settings_link_config ADD COLUMN IF NOT EXISTS lldp_link_config_id UUID NOT NULL; +ALTER TABLE omicron.public.switch_port_settings_link_config ADD COLUMN IF NOT EXISTS lldp_link_config_id UUID; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 1457532c49..d531672832 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -2657,7 +2657,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.switch_port_settings_link_config ( fec omicron.public.switch_link_fec, speed omicron.public.switch_link_speed, autoneg BOOL NOT NULL DEFAULT false, - lldp_link_config_id UUID NOT NULL, + lldp_link_config_id UUID, PRIMARY KEY (port_settings_id, link_name) ); @@ -4214,7 +4214,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '91.0.0', NULL) + (TRUE, NOW(), NOW(), '92.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/lldp-link-config-nullable/up1.sql b/schema/crdb/lldp-link-config-nullable/up1.sql new file mode 100644 index 0000000000..c8e1122f68 --- /dev/null +++ b/schema/crdb/lldp-link-config-nullable/up1.sql @@ -0,0 +1,20 @@ +-- Refer to https://github.com/oxidecomputer/omicron/issues/6433 for the justificaiton +-- behind this schema change. +-- +-- In short: the "collapse_lldp_settings" schema change was edited after +-- merging. That change included a schema change which added a non-null column +-- to an existing table. Such a data-modifying statement is only valid for +-- tables with no rows - however, in our test systems, we observed rows, which +-- prevented this schema change from progressing. +-- +-- To resolve: +-- 1. Within the old "collapse_lldp_settings" change, we retroactively dropped the +-- non-null constraint. For systems with populated +-- "switch_port_settings_link_config" tables, this allows the schema update to +-- complete without an error. +-- 2. Within this new "lldp-link-config-nullable" change, we ALSO dropped the +-- non-null constraint. For systems without populated +-- "switch_port_settings_link_config" tables -- which may have been able to +-- apply the "collapse_lldp_settings" change successfully -- this converges the state +-- of the database to the same outcome, where the columns is nullable. +ALTER TABLE omicron.public.switch_port_settings_link_config ALTER COLUMN lldp_link_config_id DROP NOT NULL;