Skip to content

default bgp max_paths to 1 when db value is null#9863

Closed
rcgoodfellow wants to merge 1 commit into
mainfrom
ry/bgp-max-path-default-to-one
Closed

default bgp max_paths to 1 when db value is null#9863
rcgoodfellow wants to merge 1 commit into
mainfrom
ry/bgp-max-path-default-to-one

Conversation

@rcgoodfellow

Copy link
Copy Markdown
Contributor

We hit an issue on dogfood where the bootstore has an empty BGP config, but peers are configured. There was previously a BGP config on this rack, and it appears it was removed from the bootstore during an online upgrade to the release 18 candidate. After looking through the code I noticed this logic

let Some(max_paths) = NonZeroU8::new(*config.max_paths) else {
// This should be impossible - our db constraint
// requires this column to be nonzero.
error!(
log,
"database contains illegal max_paths value 0"
);
return None;
};

in combination with this db update

-- add a column for configuring maximum number of paths for ECMP
ALTER TABLE IF EXISTS omicron.public.bgp_config
ADD COLUMN IF NOT EXISTS max_paths INT2 CHECK (max_paths > 0 AND max_paths <= 32);

I believe this will result in null values for the max_paths column for existing rows in the bgp_config table invalidating the assumptions in the code above.

This PR fixes this situation by defaulting a null max_paths value to one, which as the implicit behavior before this explicit config parameter was introduced.

@askfongjojo askfongjojo mentioned this pull request Feb 15, 2026
17 tasks
@rcgoodfellow

Copy link
Copy Markdown
Contributor Author

The issue is deeper than this. We are failing before we even get here, because we cannot read bgp_config entries with null values for max_paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant