Skip to content

Conversation

rcgoodfellow
Copy link
Contributor

@rcgoodfellow rcgoodfellow commented May 6, 2024

From the Nexus API endpoint networking_switch_port_settings_view there were several things missing

  • a bunch of link parameters like autoneg, fec and speed
  • a pile of bgp configuration

This means that these properties are unobservable to operators.

There was also some missing pluming for bgp communities and import/export allow lists during port settings creation where these elements were not making it into the database.

This is all addressed in this PR.

A good chunk of the diff in this PR is moving data structures around to be shared by PUT/POST and GET API calls, as noted inline in the code below.

/// The speed of a link.
#[derive(Copy, Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
#[serde(rename_all = "snake_case")]
pub enum LinkSpeed {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not net-new, it's moved from nexus/external_api/params.rs to be used as a common data structure for PUT/POST and GET requests.

/// parameter is a reference to global BGP parameters. The `interface_name`
/// indicates what interface the peer should be contacted on.
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
pub struct BgpPeer {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not net-new, it's moved from nexus/external_api/params.rs to be used as a common data structure for PUT/POST and GET requests.

use uuid::Uuid;

#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct BgpPeerConfig {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This data structure is mostly the same as the underlying db-model data structure, but includes list data members (allowed_import, allowed_export, communities) which are not in the db-model structure as they've been normalized out into separate tables and thus have their own data structures. This makes it so we can provide a complete view of the underlying BGP peer config to calling clients through this type.

/// The forward error correction mode of a link.
#[derive(Copy, Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
#[serde(rename_all = "snake_case")]
pub enum LinkFec {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not net-new, it's moved from nexus/external_api/params.rs to be used as a common data structure for PUT/POST and GET requests.

@rcgoodfellow rcgoodfellow changed the title various db fixes Various networking API & DB fixes May 6, 2024
@rcgoodfellow
Copy link
Contributor Author

I've been testing this with some new oxide CLI custom commands that show the missing data is now making it out of the API.

ry@ryair:~/src/oxide.rs$ ./target/debug/oxide net port config
switch1/qsfp0
=============
Autoneg  Fec   Speed
false    None  Speed100G

Address          Lot
169.254.20.2/30  initial-infra
169.254.40.2/30  initial-infra

BGP Peer      Export       Import       Communities  Connect Retry  Delay Open  Enforce First AS  Hold Time  Idle Hold Time  Keepalive  Local Pref  Md5 Auth  Min TTL  MED   Remote ASN  VLAN
169.254.20.1  NoFiltering  NoFiltering  []           3              3           false             6          0               2          None        None      None     None  None        None
169.254.40.1  NoFiltering  NoFiltering  []           3              3           false             6          0               2          None        None      None     None  None        None

switch0/qsfp0
=============
Autoneg  Fec   Speed
false    None  Speed100G

Address          Lot
169.254.10.2/30  initial-infra
169.254.30.2/30  initial-infra

BGP Peer      Export       Import       Communities  Connect Retry  Delay Open  Enforce First AS  Hold Time  Idle Hold Time  Keepalive  Local Pref  Md5 Auth  Min TTL  MED   Remote ASN  VLAN
169.254.10.1  NoFiltering  NoFiltering  []           3              3           false             6          0               2          None        None      None     None  None        None
169.254.30.1  NoFiltering  NoFiltering  []           3              3           false             6          0               2          None        None      None     None  None        None

If interested in looking that the source, the command lives here:

Copy link
Contributor

@karencfv karencfv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just a minor typo fix, but I'll leave it as approved so it can be merged as soon as possible

/// The address of the host to peer with.
pub addr: IpAddr,

/// How long to hold peer connections between keppalives (seconds).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// How long to hold peer connections between keppalives (seconds).
/// How long to hold peer connections between keepalives (seconds).

@rcgoodfellow rcgoodfellow enabled auto-merge (squash) May 6, 2024 05:30
@rcgoodfellow rcgoodfellow merged commit ecb6214 into main May 6, 2024
@rcgoodfellow rcgoodfellow deleted the various-api-db-fixes branch May 6, 2024 06:35
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.

2 participants