nexus: update switch_port_settings_route_config schema

common/src/api/external/mod.rs

@@ -3016,7 +3016,8 @@ pub struct SwitchPortRouteConfig {
     /// over an 802.1Q tagged L2 segment.
     pub vlan_id: Option<u16>,
 
-    /// RIB Priority indicating priority within and across protocols.
+    /// Route RIB priority. Higher priority indicates precedence within and across
+    /// protocols.
     pub rib_priority: Option<u8>,
 }
 

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<Vec<KnownVersion>> = 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"),

nexus/db-model/src/switch_port.rs

@@ -609,7 +609,6 @@ pub struct SwitchPortRouteConfig {
     pub dst: IpNetwork,
     pub gw: IpNetwork,
     pub vid: Option<SqlU16>,
-    #[diesel(column_name = local_pref)]
     pub rib_priority: Option<SqlU8>,
 }
 

nexus/db-schema/src/schema.rs

@@ -202,7 +202,7 @@ table! {
         dst -> Inet,
         gw -> Inet,
         vid -> Nullable<Int4>,
-        local_pref -> Nullable<Int2>,
+        rib_priority -> Nullable<Int2>,
     }
 }
 

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<Uuid, Option<i16>> = 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<i16>>("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, DataMigrationFns> {
         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
 }

nexus/types/src/external_api/params.rs

@@ -2003,8 +2003,8 @@ pub struct Route {
     /// VLAN id the gateway is reachable over.
     pub vid: Option<u16>,
 
-    /// 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<u8>,
 }
 

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

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;

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;

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;

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;