Skip to content

Commit dd422b1

Browse files
authored
nexus: update switch_port_settings_route_config schema (#8587)
I renamed the `local_pref` column to `rib_priority` to complete the rename in #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 #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 oxidecomputer/terraform-provider-oxide#426 when implementing a Terraform provider resource for switch port settings. It's likely that this has been broken since #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.
1 parent 31b49ec commit dd422b1

File tree

11 files changed

+105
-10
lines changed

11 files changed

+105
-10
lines changed

common/src/api/external/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3016,7 +3016,8 @@ pub struct SwitchPortRouteConfig {
30163016
/// over an 802.1Q tagged L2 segment.
30173017
pub vlan_id: Option<u16>,
30183018

3019-
/// RIB Priority indicating priority within and across protocols.
3019+
/// Route RIB priority. Higher priority indicates precedence within and across
3020+
/// protocols.
30203021
pub rib_priority: Option<u8>,
30213022
}
30223023

nexus/db-model/src/schema_versions.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
1616
///
1717
/// This must be updated when you change the database schema. Refer to
1818
/// schema/crdb/README.adoc in the root of this repository for details.
19-
pub const SCHEMA_VERSION: Version = Version::new(164, 0, 0);
19+
pub const SCHEMA_VERSION: Version = Version::new(165, 0, 0);
2020

2121
/// List of all past database schema versions, in *reverse* order
2222
///
@@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
2828
// | leaving the first copy as an example for the next person.
2929
// v
3030
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
31+
KnownVersion::new(165, "route-config-rib-priority"),
3132
KnownVersion::new(164, "fix-leaked-bp-oximeter-read-policy-rows"),
3233
KnownVersion::new(163, "bp-desired-host-phase-2"),
3334
KnownVersion::new(162, "bundle-by-creation"),

nexus/db-model/src/switch_port.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,6 @@ pub struct SwitchPortRouteConfig {
609609
pub dst: IpNetwork,
610610
pub gw: IpNetwork,
611611
pub vid: Option<SqlU16>,
612-
#[diesel(column_name = local_pref)]
613612
pub rib_priority: Option<SqlU8>,
614613
}
615614

nexus/db-schema/src/schema.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ table! {
202202
dst -> Inet,
203203
gw -> Inet,
204204
vid -> Nullable<Int4>,
205-
local_pref -> Nullable<Int2>,
205+
rib_priority -> Nullable<Int2>,
206206
}
207207
}
208208

nexus/tests/integration_tests/schema.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ use similar_asserts;
2828
use slog::Logger;
2929
use slog_error_chain::InlineErrorChain;
3030
use std::collections::BTreeMap;
31+
use std::collections::HashMap;
3132
use std::net::IpAddr;
33+
use std::str::FromStr;
3234
use std::sync::Mutex;
3335
use tokio::time::Duration;
3436
use tokio::time::timeout;
@@ -2711,6 +2713,84 @@ fn after_164_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> {
27112713
})
27122714
}
27132715

2716+
const PORT_SETTINGS_ID_165_0: &str = "1e700b64-79e0-4515-9771-bcc2391b6d4d";
2717+
const PORT_SETTINGS_ID_165_1: &str = "c6b015ff-1c98-474f-b9e9-dfc30546094f";
2718+
const PORT_SETTINGS_ID_165_2: &str = "8b777d9b-62a3-4c4d-b0b7-314315c2a7fc";
2719+
const PORT_SETTINGS_ID_165_3: &str = "7c675e89-74b1-45da-9577-cf75f028107a";
2720+
const PORT_SETTINGS_ID_165_4: &str = "e2413d63-9307-4918-b9c4-bce959c63042";
2721+
const PORT_SETTINGS_ID_165_5: &str = "05df929f-1596-42f4-b78f-aebb5d7028c4";
2722+
2723+
// Insert records using the `local_pref` column before it's renamed and its
2724+
// database type is changed from INT8 to INT2. The receiving Rust type is u8
2725+
// so 2 records are outside the u8 range, 2 records are at the edge of the u8
2726+
// range, and 1 record is within the u8 range.
2727+
fn before_165_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> {
2728+
Box::pin(async move {
2729+
ctx.client
2730+
.batch_execute(&format!("
2731+
INSERT INTO omicron.public.switch_port_settings_route_config
2732+
(port_settings_id, interface_name, dst, gw, vid, local_pref)
2733+
VALUES
2734+
(
2735+
'{PORT_SETTINGS_ID_165_0}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, -1
2736+
),
2737+
(
2738+
'{PORT_SETTINGS_ID_165_1}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, 0
2739+
),
2740+
(
2741+
'{PORT_SETTINGS_ID_165_2}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, 128
2742+
),
2743+
(
2744+
'{PORT_SETTINGS_ID_165_3}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, 255
2745+
),
2746+
(
2747+
'{PORT_SETTINGS_ID_165_4}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, 256
2748+
),
2749+
(
2750+
'{PORT_SETTINGS_ID_165_5}', 'phy0', '0.0.0.0/0', '0.0.0.0', NULL, NULL
2751+
);
2752+
"),
2753+
)
2754+
.await
2755+
.expect("failed to insert pre-migration rows for 165");
2756+
})
2757+
}
2758+
2759+
// Query the records using the new `rib_priority` column and assert that the
2760+
// values were correctly clamped within the u8 range.
2761+
fn after_165_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> {
2762+
Box::pin(async move {
2763+
let rows = ctx
2764+
.client
2765+
.query(
2766+
"SELECT * FROM omicron.public.switch_port_settings_route_config;",
2767+
&[],
2768+
)
2769+
.await
2770+
.expect("failed to query post-migration switch_port_settings_route_config table");
2771+
assert_eq!(rows.len(), 6);
2772+
2773+
let records: HashMap<Uuid, Option<i16>> = HashMap::from([
2774+
(Uuid::from_str(PORT_SETTINGS_ID_165_0).unwrap(), Some(0)),
2775+
(Uuid::from_str(PORT_SETTINGS_ID_165_1).unwrap(), Some(0)),
2776+
(Uuid::from_str(PORT_SETTINGS_ID_165_2).unwrap(), Some(128)),
2777+
(Uuid::from_str(PORT_SETTINGS_ID_165_3).unwrap(), Some(255)),
2778+
(Uuid::from_str(PORT_SETTINGS_ID_165_4).unwrap(), Some(255)),
2779+
(Uuid::from_str(PORT_SETTINGS_ID_165_5).unwrap(), None),
2780+
]);
2781+
2782+
for row in rows {
2783+
let port_settings_id = row.get::<&str, Uuid>("port_settings_id");
2784+
let rib_priority_got = row.get::<&str, Option<i16>>("rib_priority");
2785+
2786+
let rib_priority_want = records
2787+
.get(&port_settings_id)
2788+
.expect("unexpected port_settings_id value when querying switch_port_settings_route_config");
2789+
assert_eq!(rib_priority_got, *rib_priority_want);
2790+
}
2791+
})
2792+
}
2793+
27142794
// Lazily initializes all migration checks. The combination of Rust function
27152795
// pointers and async makes defining a static table fairly painful, so we're
27162796
// using lazy initialization instead.
@@ -2801,6 +2881,10 @@ fn get_migration_checks() -> BTreeMap<Version, DataMigrationFns> {
28012881
Version::new(164, 0, 0),
28022882
DataMigrationFns::new().before(before_164_0_0).after(after_164_0_0),
28032883
);
2884+
map.insert(
2885+
Version::new(165, 0, 0),
2886+
DataMigrationFns::new().before(before_165_0_0).after(after_165_0_0),
2887+
);
28042888

28052889
map
28062890
}

nexus/types/src/external_api/params.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2004,8 +2004,8 @@ pub struct Route {
20042004
/// VLAN id the gateway is reachable over.
20052005
pub vid: Option<u16>,
20062006

2007-
/// Local preference for route. Higher preference indictes precedence
2008-
/// within and across protocols.
2007+
/// Route RIB priority. Higher priority indicates precedence within and across
2008+
/// protocols.
20092009
pub rib_priority: Option<u8>,
20102010
}
20112011

openapi/nexus.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22549,7 +22549,7 @@
2254922549
},
2255022550
"rib_priority": {
2255122551
"nullable": true,
22552-
"description": "Local preference for route. Higher preference indictes precedence within and across protocols.",
22552+
"description": "Route RIB priority. Higher priority indicates precedence within and across protocols.",
2255322553
"type": "integer",
2255422554
"format": "uint8",
2255522555
"minimum": 0
@@ -24825,7 +24825,7 @@
2482524825
},
2482624826
"rib_priority": {
2482724827
"nullable": true,
24828-
"description": "RIB Priority indicating priority within and across protocols.",
24828+
"description": "Route RIB priority. Higher priority indicates precedence within and across protocols.",
2482924829
"type": "integer",
2483024830
"format": "uint8",
2483124831
"minimum": 0

schema/crdb/dbinit.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3148,7 +3148,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.switch_port_settings_route_config (
31483148
dst INET,
31493149
gw INET,
31503150
vid INT4,
3151-
local_pref INT8,
3151+
rib_priority INT2,
31523152

31533153
/* TODO https://github.com/oxidecomputer/omicron/issues/3013 */
31543154
PRIMARY KEY (port_settings_id, interface_name, dst, gw)
@@ -6209,7 +6209,7 @@ INSERT INTO omicron.public.db_metadata (
62096209
version,
62106210
target_version
62116211
) VALUES
6212-
(TRUE, NOW(), NOW(), '164.0.0', NULL)
6212+
(TRUE, NOW(), NOW(), '165.0.0', NULL)
62136213
ON CONFLICT DO NOTHING;
62146214

62156215
COMMIT;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE omicron.public.switch_port_settings_route_config ADD COLUMN IF NOT EXISTS rib_priority INT2;
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
SET LOCAL disallow_full_table_scans = off;
2+
UPDATE omicron.public.switch_port_settings_route_config
3+
SET rib_priority =
4+
CASE
5+
WHEN local_pref > 255 THEN 255
6+
WHEN local_pref < 0 THEN 0
7+
ELSE local_pref::INT2
8+
END;

0 commit comments

Comments
 (0)