diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 7d415f2ba0..3ed8e98026 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -3016,7 +3016,8 @@ pub struct SwitchPortRouteConfig { /// over an 802.1Q tagged L2 segment. pub vlan_id: Option, - /// RIB Priority indicating priority within and across protocols. + /// Route RIB priority. Higher priority indicates precedence within and across + /// protocols. pub rib_priority: Option, } diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 31ee927bd2..2d2d48dfbb 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// 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: Version = Version::new(164, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(165, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock> = LazyLock::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(165, "route-config-rib-priority"), KnownVersion::new(164, "fix-leaked-bp-oximeter-read-policy-rows"), KnownVersion::new(163, "bp-desired-host-phase-2"), KnownVersion::new(162, "bundle-by-creation"), diff --git a/nexus/db-model/src/switch_port.rs b/nexus/db-model/src/switch_port.rs index b768df6ed7..67b77df937 100644 --- a/nexus/db-model/src/switch_port.rs +++ b/nexus/db-model/src/switch_port.rs @@ -609,7 +609,6 @@ pub struct SwitchPortRouteConfig { pub dst: IpNetwork, pub gw: IpNetwork, pub vid: Option, - #[diesel(column_name = local_pref)] pub rib_priority: Option, } diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 16c116c8e4..c5a6caae9e 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -202,7 +202,7 @@ table! { dst -> Inet, gw -> Inet, vid -> Nullable, - local_pref -> Nullable, + rib_priority -> Nullable, } } diff --git a/nexus/tests/integration_tests/schema.rs b/nexus/tests/integration_tests/schema.rs index 1d289007df..b2f4e03aaf 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -28,7 +28,9 @@ use similar_asserts; use slog::Logger; use slog_error_chain::InlineErrorChain; use std::collections::BTreeMap; +use std::collections::HashMap; use std::net::IpAddr; +use std::str::FromStr; use std::sync::Mutex; use tokio::time::Duration; use tokio::time::timeout; @@ -2711,6 +2713,84 @@ fn after_164_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { }) } +const PORT_SETTINGS_ID_165_0: &str = "1e700b64-79e0-4515-9771-bcc2391b6d4d"; +const PORT_SETTINGS_ID_165_1: &str = "c6b015ff-1c98-474f-b9e9-dfc30546094f"; +const PORT_SETTINGS_ID_165_2: &str = "8b777d9b-62a3-4c4d-b0b7-314315c2a7fc"; +const PORT_SETTINGS_ID_165_3: &str = "7c675e89-74b1-45da-9577-cf75f028107a"; +const PORT_SETTINGS_ID_165_4: &str = "e2413d63-9307-4918-b9c4-bce959c63042"; +const PORT_SETTINGS_ID_165_5: &str = "05df929f-1596-42f4-b78f-aebb5d7028c4"; + +// Insert records using the `local_pref` column before it's renamed and its +// database type is changed from INT8 to INT2. The receiving Rust type is u8 +// so 2 records are outside the u8 range, 2 records are at the edge of the u8 +// range, and 1 record is within the u8 range. +fn before_165_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { + Box::pin(async move { + ctx.client + .batch_execute(&format!(" + INSERT INTO omicron.public.switch_port_settings_route_config + (port_settings_id, interface_name, dst, gw, vid, local_pref) + VALUES + ( + '{PORT_SETTINGS_ID_165_0}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, -1 + ), + ( + '{PORT_SETTINGS_ID_165_1}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, 0 + ), + ( + '{PORT_SETTINGS_ID_165_2}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, 128 + ), + ( + '{PORT_SETTINGS_ID_165_3}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, 255 + ), + ( + '{PORT_SETTINGS_ID_165_4}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, 256 + ), + ( + '{PORT_SETTINGS_ID_165_5}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, NULL + ); + "), + ) + .await + .expect("failed to insert pre-migration rows for 165"); + }) +} + +// Query the records using the new `rib_priority` column and assert that the +// values were correctly clamped within the u8 range. +fn after_165_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { + Box::pin(async move { + let rows = ctx + .client + .query( + "SELECT * FROM omicron.public.switch_port_settings_route_config;", + &[], + ) + .await + .expect("failed to query post-migration switch_port_settings_route_config table"); + assert_eq!(rows.len(), 6); + + let records: HashMap> = HashMap::from([ + (Uuid::from_str(PORT_SETTINGS_ID_165_0).unwrap(), Some(0)), + (Uuid::from_str(PORT_SETTINGS_ID_165_1).unwrap(), Some(0)), + (Uuid::from_str(PORT_SETTINGS_ID_165_2).unwrap(), Some(128)), + (Uuid::from_str(PORT_SETTINGS_ID_165_3).unwrap(), Some(255)), + (Uuid::from_str(PORT_SETTINGS_ID_165_4).unwrap(), Some(255)), + (Uuid::from_str(PORT_SETTINGS_ID_165_5).unwrap(), None), + ]); + + for row in rows { + let port_settings_id = row.get::<&str, Uuid>("port_settings_id"); + let rib_priority_got = row.get::<&str, Option>("rib_priority"); + + let rib_priority_want = records + .get(&port_settings_id) + .expect("unexpected port_settings_id value when querying switch_port_settings_route_config"); + assert_eq!(rib_priority_got, *rib_priority_want); + } + }) +} + // Lazily initializes all migration checks. The combination of Rust function // pointers and async makes defining a static table fairly painful, so we're // using lazy initialization instead. @@ -2801,6 +2881,10 @@ fn get_migration_checks() -> BTreeMap { Version::new(164, 0, 0), DataMigrationFns::new().before(before_164_0_0).after(after_164_0_0), ); + map.insert( + Version::new(165, 0, 0), + DataMigrationFns::new().before(before_165_0_0).after(after_165_0_0), + ); map } diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 05e1adc30a..5640227982 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -2003,8 +2003,8 @@ pub struct Route { /// VLAN id the gateway is reachable over. pub vid: Option, - /// Local preference for route. Higher preference indictes precedence - /// within and across protocols. + /// Route RIB priority. Higher priority indicates precedence within and across + /// protocols. pub rib_priority: Option, } diff --git a/openapi/nexus.json b/openapi/nexus.json index 239807b84a..0d0834cf2d 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -22294,7 +22294,7 @@ }, "rib_priority": { "nullable": true, - "description": "Local preference for route. Higher preference indictes precedence within and across protocols.", + "description": "Route RIB priority. Higher priority indicates precedence within and across protocols.", "type": "integer", "format": "uint8", "minimum": 0 @@ -24570,7 +24570,7 @@ }, "rib_priority": { "nullable": true, - "description": "RIB Priority indicating priority within and across protocols.", + "description": "Route RIB priority. Higher priority indicates precedence within and across protocols.", "type": "integer", "format": "uint8", "minimum": 0 diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 9b90c2b8e8..1a76f8b342 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -3148,7 +3148,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.switch_port_settings_route_config ( dst INET, gw INET, vid INT4, - local_pref INT8, + rib_priority INT2, /* TODO https://github.com/oxidecomputer/omicron/issues/3013 */ PRIMARY KEY (port_settings_id, interface_name, dst, gw) @@ -6209,7 +6209,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '164.0.0', NULL) + (TRUE, NOW(), NOW(), '165.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/route-config-rib-priority/up01.sql b/schema/crdb/route-config-rib-priority/up01.sql new file mode 100644 index 0000000000..80c6078dce --- /dev/null +++ b/schema/crdb/route-config-rib-priority/up01.sql @@ -0,0 +1 @@ +ALTER TABLE omicron.public.switch_port_settings_route_config ADD COLUMN IF NOT EXISTS rib_priority INT2; diff --git a/schema/crdb/route-config-rib-priority/up02.sql b/schema/crdb/route-config-rib-priority/up02.sql new file mode 100644 index 0000000000..2179ee13d8 --- /dev/null +++ b/schema/crdb/route-config-rib-priority/up02.sql @@ -0,0 +1,8 @@ +SET LOCAL disallow_full_table_scans = off; +UPDATE omicron.public.switch_port_settings_route_config +SET rib_priority = + CASE + WHEN local_pref > 255 THEN 255 + WHEN local_pref < 0 THEN 0 + ELSE local_pref::INT2 + END; diff --git a/schema/crdb/route-config-rib-priority/up03.sql b/schema/crdb/route-config-rib-priority/up03.sql new file mode 100644 index 0000000000..e719e0b79b --- /dev/null +++ b/schema/crdb/route-config-rib-priority/up03.sql @@ -0,0 +1 @@ +ALTER TABLE omicron.public.switch_port_settings_route_config DROP COLUMN IF EXISTS local_pref;