Skip to content
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

configure bgp during early networking #4480

Merged
merged 2 commits into from
Nov 10, 2023
Merged

configure bgp during early networking #4480

merged 2 commits into from
Nov 10, 2023

Conversation

rcgoodfellow
Copy link
Contributor

@rcgoodfellow rcgoodfellow commented Nov 9, 2023

Also brings in the following which fixes GitHub rate limit errors in the deploy CI job.

To test this I supplied the following config-rss.toml file to sled-agent in a single-node Omicron development environment and the system came up with an established BGP session.

rack_subnet = "fd00:1122:3344:0100::"
bootstrap_discovery.type = "only_ours"
ntp_servers = [ "0.pool.ntp.org" ]
dns_servers = [ "1.1.1.1", "9.9.9.9" ]
external_dns_zone_name = "oxide.test"
external_dns_ips = [ "192.168.20.20", "192.168.20.21" ]
external_certificates = []

[[internal_services_ip_pool_ranges]]
first = "192.168.20.20"
last = "192.168.20.30"

[rack_network_config]
rack_subnet = "fd00:1122:3344:01::/56"
infra_ip_first = "192.168.20.3"
infra_ip_last = "192.168.20.3"

[[rack_network_config.bgp]]
asn = 47
originate = [ "192.168.20.0/24" ]

[[rack_network_config.ports]]
routes = [{nexthop = "192.168.20.1", destination = "0.0.0.0/0"}]
addresses = ["192.168.20.3/32", "192.168.22.3/29"]
port = "qsfp0"
uplink_port_speed = "100G"
uplink_port_fec="none"
switch = "switch0"
bgp_peers = [{asn = 47, addr = "192.168.22.1", port = "qsfp0"}]

[recovery_silo]
silo_name = "recovery"
user_name = "recovery"
user_password_hash = "$argon2id$v=19$m=98304,t=13,p=1$RUlWc0ZxaHo0WFdrN0N6ZQ$S8p52j85GPvMhR/ek3GL0el/oProgTwWpHJZ8lsQQoY"

Updated docs for this RSS config are in

  • oxidecomputer/docs#220

@rcgoodfellow rcgoodfellow marked this pull request as ready for review November 10, 2023 05:33
Copy link
Contributor

@andrewjstone andrewjstone left a comment

Choose a reason for hiding this comment

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

@rcgoodfellow This looks very good. I suggest testing the upgrade before merge though.

@@ -103,6 +103,17 @@ pub struct BgpPeerConfig {
pub port: String,
/// Address of the peer.
pub addr: Ipv4Addr,
/// How long to keep a session alive without a keepalive in seconds.
/// Defaults to 6.
pub hold_time: Option<u64>,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use a Duration typically for these, but if you want/need to keep them as u64 then I'd suggest adding a suffix like s as in hold_time_s

Copy link
Contributor

Choose a reason for hiding this comment

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

This is currently part of the data stored in the bootstore, but I think because these are all optional fields deserialization should still work. I would suggest testing the upgrade path though, where you initialize a rack with the old version and then install the new to see what happens.

@@ -132,6 +132,51 @@
"format": "uint32",
"minimum": 0.0
},
"connect_retry": {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe we rewrite the sled-plan after RSS time, so this should be fine. I also just confirmed that adding a sled doesn't create a plan, and so we don't have to worry about reading an old plan with a new schema.

Copy link
Contributor

@andrewjstone andrewjstone left a comment

Choose a reason for hiding this comment

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

Upgrade tested on madrid. Ship it!

@rcgoodfellow rcgoodfellow merged commit 85ac741 into main Nov 10, 2023
22 checks passed
@rcgoodfellow rcgoodfellow deleted the bgp-rss branch November 10, 2023 22: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.

BGP configuration is not applied to switches after upgrade
2 participants