-
Notifications
You must be signed in to change notification settings - Fork 45
nexus: update switch_port_settings_route_config
schema
#8587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sudomateo
wants to merge
1
commit into
main
Choose a base branch
from
sudomateo/wuxkwlxksyvk
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sudomateo
commented
Jul 12, 2025
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.
b40fcbd
to
75d9f46
Compare
sudomateo
commented
Jul 12, 2025
Comment on lines
+2
to
+8
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I check for the existence of the local_pref
column before running the update? The failing tests pass as-is.
$ cargo nextest run -p omicron-nexus \
integration_tests::schema::dbinit_equals_sum_of_all_up \
integration_tests::schema::nexus_applies_update_on_boot \
integration_tests::schema::validate_data_migration \
integration_tests::schema::versions_have_idempotent_up
info: experimental features enabled: setup-scripts
Finished `test` profile [unoptimized + debuginfo] target(s) in 0.59s
────────────
Nextest run ID 203a1279-f293-4d27-9798-e7567a3f4e40 with nextest profile: default
Starting 4 tests across 4 binaries (618 tests skipped)
[ 00:00:00] 0/622:
SETUP [ 1/1] crdb-seed: cargo run -p crdb-seed --profile test
Finished `test` profile [unoptimized + debuginfo] target(s) in 0.43s
Running `target/debug/crdb-seed`
Jul 12 18:17:18.961 INFO Using existing CRDB seed tarball: `/tmp/crdb-base-sudomateo/95d6982d0e09b7c2fe1faf36c7b5468930c6e78e2198dccc411c51e5b6d0d362.tar`
SETUP PASS [ 0.510s] crdb-seed: cargo run -p crdb-seed --profile test
PASS [ 31.601s] omicron-nexus::test_all integration_tests::schema::validate_data_migration
PASS [ 34.053s] omicron-nexus::test_all integration_tests::schema::nexus_applies_update_on_boot
PASS [ 35.028s] omicron-nexus::test_all integration_tests::schema::versions_have_idempotent_up
PASS [ 46.814s] omicron-nexus::test_all integration_tests::schema::dbinit_equals_sum_of_all_up
────────────
Summary [ 47.334s] 4 tests run: 4 passed, 618 skipped
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I renamed the
local_pref
column torib_priority
to complete the rename in #6693.I also changed the type of the renamed column from
INT8
toINT2
, clamping existing values to0
or255
. This was missed in #6693 and led to the following error when reading the value from the database.The error occurred because the Rust type was
SqlU8
and the database type wasINT8
. Reads would fail becauseINT8
columns could not be read intoSqlU8
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.