-
Notifications
You must be signed in to change notification settings - Fork 66
incorporate maghemite #533 #9449
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
Conversation
| &self, | ||
| opctx: &OpContext, | ||
| sel: ¶ms::BgpRouteSelector, | ||
| _sel: ¶ms::BgpRouteSelector, |
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.
If we're no longer querying imported routes by ASN, we should propagate that up through the omicron API function networking_bgp_imported_routes_ipv4 so users of the API don't have to specify an ASN that's no longer used for anything. Then we can remove the sel parameter.
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.
Agreed. However, the change I'm envisioning would be to deprecate the old endpoint (/v1/system/networking/bgp-routes-ipv4) and introduce a new endpoint that will not be specific to BGP or IP version (e.g. /v1/system/networking/routes). I'm not sure exactly what this will look like yet from a Nexus API standpoint, so I put this in as a placeholder for the time being.
Signed-off-by: Trey Aspelund <trey@oxidecomputer.com>
maghemite #533 adds impl PartialEq<rdb_types::Prefix> for oxnet::IpNet, which caused some preexisting type inferences to no longer be resolved unambiguously (now there are multiple PartialEq<T> for oxnet::IpNet). This updates these spots to be explicit about the types being compared. Signed-off-by: Trey Aspelund <trey@oxidecomputer.com>
This PR connects IPv6 routes from the switch port settings API to maghemite through the sync switch configuration RPW.
767a2fc to
ae3aec5
Compare
| let mut switch_zone_addrs = HashMap::new(); | ||
|
|
||
| for addr in switch_zone_addresses { | ||
| let port = match resolver |
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.
Special scrutiny is warranted here. I think this should be ok as we fallback to the previous constant port when DNS does not work out.
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.
FWIW, this function will probably go away entirely as we continue to chop away at #8999. In the branch I'm building off of this PR for multi-rack omicron-dev, building the mgd clients will no longer rely on this and will instead look like dpd_clients() in this same file. I think LLDP is the only one left after that, which I also plan to address once MGD is working.
| let mut clients: Vec<(SwitchLocation, mg_admin_client::Client)> = vec![]; | ||
| for (location, addr) in &mappings { | ||
| let port = MGD_PORT; | ||
| let port = match resolver.lookup_all_socket_v6(ServiceName::Mgd).await { |
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.
Special scrutiny is warranted here. I think this should be ok as we fallback to the previous constant port when DNS does not work out.
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.
Same as noted above
internet-diglett
left a comment
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.
Some very minor comments, LGTM
- Connects IPv6 routes from the switch port settings API to maghemite through the sync switch configuration RPW - Adds tests to verify that v6 routes make it all the way to mgd - Fixes some hardcoded ports for mgs and mgd to allow e2e maghemite testing - Fixes `tools/update_maghemite.sh` as maghemite no longer ships openapi definitions as buildomat artifacts. --------- Signed-off-by: Trey Aspelund <trey@oxidecomputer.com> Co-authored-by: Ryan Goodfellow <ryan.goodfellow@oxide.computer>
tools/update_maghemite.shas maghemite no longer ships openapi definitions as buildomat artifacts.