From 75d9f46a44ea2c0d94ca666016ca44d1f89fb18a Mon Sep 17 00:00:00 2001 From: Matthew Sanabria Date: Fri, 11 Jul 2025 09:54:03 -0400 Subject: [PATCH] nexus: update `switch_port_settings_route_config` schema I renamed the `local_pref` column to `rib_priority` to complete the rename in https://github.com/oxidecomputer/omicron/pull/6693. I also changed the type of the renamed column from `INT8` to `INT2`, clamping existing values to `0` or `255`. This was missed in https://github.com/oxidecomputer/omicron/pull/6693 and led to the following error when reading the value from the database. ``` {"msg":"request completed","v":0,"name":"omicron-dev","level":30,"time":"2025-07-11T13:57:48.670343242Z","hostname":"ms-ox01","pid":3113,"uri":"/v1/system/networking/switch-port-settings","method":"POST","req_id":"12b35f5a-2255-4244-a5aa-f9c84032fb81","remote_addr":"127.0.0.1:41766","local_addr":"127.0.0.1:12220","component":"dropshot_external","name":"e6bff1ff-24fb-49dc-a54e-c6a350cd4d6c","error_message_external":"Internal Server Error","error_message_internal":"Unknown diesel error creating SwitchPortSettings called \"example\": Error deserializing field 'local_pref': Received more than 2 bytes while decoding an i16. Was an Integer expression accidentally marked as SmallInt?","latency_us":133766,"response_code":500} ``` The error occurred because the Rust type was `SqlU8` and the database type was `INT8`. Reads would fail because `INT8` columns could not be read into `SqlU8` types without loss of precision. This was caught in https://github.com/oxidecomputer/terraform-provider-oxide/pull/426 when implementing a Terraform provider resource for switch port settings. It's likely that this has been broken since https://github.com/oxidecomputer/omicron/pull/6693 and any user that attempted to set `rib_priority` in their Rack Setup Service (RSS) would have encountered this error. However, `rib_priority` is an uncommon configuration option and none of our customer's RSS configurations show this as being set. Given this information, it seems safe to assume that no customer has the `rib_priority` column set so the clamping logic implemented here should work well for customer installations. --- common/src/api/external/mod.rs | 3 ++- nexus/db-model/src/schema_versions.rs | 3 ++- nexus/db-model/src/switch_port.rs | 1 - nexus/db-schema/src/schema.rs | 2 +- nexus/types/src/external_api/params.rs | 4 ++-- openapi/nexus.json | 4 ++-- schema/crdb/dbinit.sql | 4 ++-- schema/crdb/route-config-rib-priority/up01.sql | 1 + schema/crdb/route-config-rib-priority/up02.sql | 9 +++++++++ schema/crdb/route-config-rib-priority/up03.sql | 1 + 10 files changed, 22 insertions(+), 10 deletions(-) create mode 100644 schema/crdb/route-config-rib-priority/up01.sql create mode 100644 schema/crdb/route-config-rib-priority/up02.sql create mode 100644 schema/crdb/route-config-rib-priority/up03.sql diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index b02b577b5ba..72ffbbc4b27 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -2998,7 +2998,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 7a67315c41e..3fe7a7afb02 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(161, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(162, 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(162, "route-config-rib-priority"), KnownVersion::new(161, "inv_cockroachdb_status"), KnownVersion::new(160, "tuf-trust-root"), KnownVersion::new(159, "sled-config-desired-host-phase-2"), diff --git a/nexus/db-model/src/switch_port.rs b/nexus/db-model/src/switch_port.rs index b768df6ed7e..67b77df9376 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 99285a291f3..13eccdc6d06 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/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 05e1adc30aa..5640227982f 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 a2aa944bfc9..b28fb4627e5 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 39ea3574f13..88914d39267 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -3144,7 +3144,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) @@ -6198,7 +6198,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '161.0.0', NULL) + (TRUE, NOW(), NOW(), '162.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 00000000000..80c6078dcec --- /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 00000000000..bc79a842298 --- /dev/null +++ b/schema/crdb/route-config-rib-priority/up02.sql @@ -0,0 +1,9 @@ +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; +SET LOCAL disallow_full_table_scans = on; 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 00000000000..e719e0b79b9 --- /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;