API updates for oxidecomputer/maghemite#88 fix (#4620)

oxidecomputer · Dec 7, 2023 · 7f9d64d · 7f9d64d
1 parent e0882ae
commit 7f9d64d
Show file tree

Hide file tree

Showing 7 changed files with 127 additions and 74 deletions.
diff --git a/nexus/src/app/sagas/switch_port_settings_common.rs b/nexus/src/app/sagas/switch_port_settings_common.rs
@@ -444,7 +444,9 @@ pub(crate) async fn ensure_switch_port_bgp_settings(
             |e| ActionError::action_failed(format!("select mg client: {e}")),
         )?;
 
-    let mut bgp_peer_configs = Vec::new();
+    let mut bgp_peer_configs = HashMap::<String, Vec<BgpPeerConfig>>::new();
+
+    let mut cfg: Option<nexus_db_model::BgpConfig> = None;
 
     for peer in settings.bgp_peers {
         let config = nexus
@@ -454,11 +456,44 @@ pub(crate) async fn ensure_switch_port_bgp_settings(
                 ActionError::action_failed(format!("get bgp config: {e}"))
             })?;
 
+        if let Some(cfg) = &cfg {
+            if config.asn != cfg.asn {
+                return Err(ActionError::action_failed(
+                    "bad request: only one AS allowed per switch".to_string(),
+                ));
+            }
+        } else {
+            cfg = Some(config);
+        }
+
+        let bpc = BgpPeerConfig {
+            name: format!("{}", peer.addr.ip()), //TODO user defined name?
+            host: format!("{}:179", peer.addr.ip()),
+            hold_time: peer.hold_time.0.into(),
+            idle_hold_time: peer.idle_hold_time.0.into(),
+            delay_open: peer.delay_open.0.into(),
+            connect_retry: peer.connect_retry.0.into(),
+            keepalive: peer.keepalive.0.into(),
+            resolution: BGP_SESSION_RESOLUTION,
+            passive: false,
+        };
+
+        match bgp_peer_configs.get_mut(&switch_port_name) {
+            Some(peers) => {
+                peers.push(bpc);
+            }
+            None => {
+                bgp_peer_configs.insert(switch_port_name.clone(), vec![bpc]);
+            }
+        }
+    }
+
+    if let Some(cfg) = &cfg {
         let announcements = nexus
             .bgp_announce_list(
                 &opctx,
                 &params::BgpAnnounceSetSelector {
-                    name_or_id: NameOrId::Id(config.bgp_announce_set_id),
+                    name_or_id: NameOrId::Id(cfg.bgp_announce_set_id),
                 },
             )
             .await
@@ -473,39 +508,25 @@ pub(crate) async fn ensure_switch_port_bgp_settings(
             let value = match a.network.ip() {
                 IpAddr::V4(value) => Ok(value),
                 IpAddr::V6(_) => Err(ActionError::action_failed(
-                    "IPv6 announcement not yet supported".to_string(),
+                    "bad request: IPv6 announcement not yet supported"
+                        .to_string(),
                 )),
             }?;
             prefixes.push(Prefix4 { value, length: a.network.prefix() });
         }
-
-        let bpc = BgpPeerConfig {
-            asn: *config.asn,
-            name: format!("{}", peer.addr.ip()), //TODO user defined name?
-            host: format!("{}:179", peer.addr.ip()),
-            hold_time: peer.hold_time.0.into(),
-            idle_hold_time: peer.idle_hold_time.0.into(),
-            delay_open: peer.delay_open.0.into(),
-            connect_retry: peer.connect_retry.0.into(),
-            keepalive: peer.keepalive.0.into(),
-            resolution: BGP_SESSION_RESOLUTION,
-            originate: prefixes,
-        };
-
-        bgp_peer_configs.push(bpc);
+        mg_client
+            .inner
+            .bgp_apply(&ApplyRequest {
+                asn: cfg.asn.0,
+                peers: bgp_peer_configs,
+                originate: prefixes,
+            })
+            .await
+            .map_err(|e| {
+                ActionError::action_failed(format!("apply bgp settings: {e}"))
+            })?;
     }
 
-    mg_client
-        .inner
-        .bgp_apply(&ApplyRequest {
-            peer_group: switch_port_name,
-            peers: bgp_peer_configs,
-        })
-        .await
-        .map_err(|e| {
-            ActionError::action_failed(format!("apply bgp settings: {e}"))
-        })?;
-
     Ok(())
 }
 

diff --git a/nexus/src/app/switch_port.rs b/nexus/src/app/switch_port.rs
@@ -117,7 +117,6 @@ impl super::Nexus {
             .map_err(|e| {
                     let msg = e.to_string();
                     if msg.contains("bad request") {
-                        //return HttpError::for_client_error(None, StatusCode::BAD_REQUEST, msg.to_string())
                         external::Error::invalid_request(&msg.to_string())
                     } else {
                         e
@@ -255,7 +254,15 @@ impl super::Nexus {
         >(
             saga_params,
         )
-        .await?;
+        .await
+        .map_err(|e| {
+                let msg = e.to_string();
+                if msg.contains("bad request") {
+                    external::Error::invalid_request(&msg.to_string())
+                } else {
+                    e
+                }
+            })?;
 
         Ok(())
     }

diff --git a/package-manifest.toml b/package-manifest.toml
@@ -425,7 +425,7 @@ source.repo = "maghemite"
 # `tools/maghemite_openapi_version`. Failing to do so will cause a failure when
 # building `ddm-admin-client` (which will instruct you to update
 # `tools/maghemite_openapi_version`).
-source.commit = "579592bf474ec4b86805ada60c1b920b3beef5a7"
+source.commit = "2fd39b75df696961e5ea190c7d74dd91f4849cd3"
 # The SHA256 digest is automatically posted to:
 # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/maghemite/image/<commit>/maghemite.sha256.txt
 source.sha256 = "38851c79c85d53e997db748520fb27c82299ce7e58a550e35646a548498f1271"
@@ -441,7 +441,7 @@ source.repo = "maghemite"
 # `tools/maghemite_openapi_version`. Failing to do so will cause a failure when
 # building `ddm-admin-client` (which will instruct you to update
 # `tools/maghemite_openapi_version`).
-source.commit = "579592bf474ec4b86805ada60c1b920b3beef5a7"
+source.commit = "2fd39b75df696961e5ea190c7d74dd91f4849cd3"
 # The SHA256 digest is automatically posted to:
 # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/maghemite/image/<commit>/mg-ddm.sha256.txt
 source.sha256 = "8cd94e9a6f6175081ce78f0281085a08a5306cde453d8e21deb28050945b1d88"
@@ -456,10 +456,10 @@ source.repo = "maghemite"
 # `tools/maghemite_openapi_version`. Failing to do so will cause a failure when
 # building `ddm-admin-client` (which will instruct you to update
 # `tools/maghemite_openapi_version`).
-source.commit = "579592bf474ec4b86805ada60c1b920b3beef5a7"
+source.commit = "2fd39b75df696961e5ea190c7d74dd91f4849cd3"
 # The SHA256 digest is automatically posted to:
 # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/maghemite/image/<commit>/mg-ddm.sha256.txt
-source.sha256 = "82aa1ca1d7701b2221c442d58f912be59798258d574effcb866ffab22753cf38"
+source.sha256 = "802636775fa77dc6eec193e65fde87e403f6a11531745d47ef5e7ff13b242890"
 output.type = "zone"
 output.intermediate_only = true
 

diff --git a/sled-agent/src/bootstrap/early_networking.rs b/sled-agent/src/bootstrap/early_networking.rs
@@ -22,8 +22,8 @@ use mg_admin_client::Client as MgdClient;
 use omicron_common::address::{Ipv6Subnet, MGD_PORT, MGS_PORT};
 use omicron_common::address::{DDMD_PORT, DENDRITE_PORT};
 use omicron_common::api::internal::shared::{
-    PortConfigV1, PortFec, PortSpeed, RackNetworkConfig, RackNetworkConfigV1,
-    SwitchLocation, UplinkConfig,
+    BgpConfig, PortConfigV1, PortFec, PortSpeed, RackNetworkConfig,
+    RackNetworkConfigV1, SwitchLocation, UplinkConfig,
 };
 use omicron_common::backoff::{
     retry_notify, retry_policy_local, BackoffError, ExponentialBackoff,
@@ -472,23 +472,37 @@ impl<'a> EarlyNetworkSetup<'a> {
             ))
         })?;
 
+        let mut config: Option<BgpConfig> = None;
+        let mut bgp_peer_configs = HashMap::<String, Vec<BgpPeerConfig>>::new();
+
         // Iterate through ports and apply BGP config.
         for port in &our_ports {
-            let mut bgp_peer_configs = Vec::new();
             for peer in &port.bgp_peers {
-                let config = rack_network_config
-                    .bgp
-                    .iter()
-                    .find(|x| x.asn == peer.asn)
-                    .ok_or(EarlyNetworkSetupError::BgpConfigurationError(
-                        format!(
-                            "asn {} referenced by peer undefined",
-                            peer.asn
-                        ),
-                    ))?;
+                if let Some(config) = &config {
+                    if peer.asn != config.asn {
+                        return Err(EarlyNetworkSetupError::BadConfig(
+                            "only one ASN per switch is supported".into(),
+                        ));
+                    }
+                } else {
+                    config = Some(
+                        rack_network_config
+                            .bgp
+                            .iter()
+                            .find(|x| x.asn == peer.asn)
+                            .ok_or(
+                                EarlyNetworkSetupError::BgpConfigurationError(
+                                    format!(
+                                        "asn {} referenced by peer undefined",
+                                        peer.asn
+                                    ),
+                                ),
+                            )?
+                            .clone(),
+                    );
+                }
 
                 let bpc = BgpPeerConfig {
-                    asn: peer.asn,
                     name: format!("{}", peer.addr),
                     host: format!("{}:179", peer.addr),
                     hold_time: peer.hold_time.unwrap_or(6),
@@ -497,30 +511,41 @@ impl<'a> EarlyNetworkSetup<'a> {
                     connect_retry: peer.connect_retry.unwrap_or(3),
                     keepalive: peer.keepalive.unwrap_or(2),
                     resolution: BGP_SESSION_RESOLUTION,
-                    originate: config
-                        .originate
-                        .iter()
-                        .map(|x| Prefix4 { length: x.prefix(), value: x.ip() })
-                        .collect(),
+                    passive: false,
                 };
-                bgp_peer_configs.push(bpc);
+                match bgp_peer_configs.get_mut(&port.port) {
+                    Some(peers) => {
+                        peers.push(bpc);
+                    }
+                    None => {
+                        bgp_peer_configs.insert(port.port.clone(), vec![bpc]);
+                    }
+                }
             }
+        }
 
-            if bgp_peer_configs.is_empty() {
-                continue;
+        if !bgp_peer_configs.is_empty() {
+            if let Some(config) = &config {
+                mgd.inner
+                    .bgp_apply(&ApplyRequest {
+                        asn: config.asn,
+                        peers: bgp_peer_configs,
+                        originate: config
+                            .originate
+                            .iter()
+                            .map(|x| Prefix4 {
+                                length: x.prefix(),
+                                value: x.ip(),
+                            })
+                            .collect(),
+                    })
+                    .await
+                    .map_err(|e| {
+                        EarlyNetworkSetupError::BgpConfigurationError(format!(
+                            "BGP peer configuration failed: {e}",
+                        ))
+                    })?;
             }
-
-            mgd.inner
-                .bgp_apply(&ApplyRequest {
-                    peer_group: port.port.clone(),
-                    peers: bgp_peer_configs,
-                })
-                .await
-                .map_err(|e| {
-                    EarlyNetworkSetupError::BgpConfigurationError(format!(
-                        "BGP peer configuration failed: {e}",
-                    ))
-                })?;
         }
 
         Ok(our_ports)

diff --git a/tools/maghemite_ddm_openapi_version b/tools/maghemite_ddm_openapi_version
@@ -1,2 +1,2 @@
-COMMIT="579592bf474ec4b86805ada60c1b920b3beef5a7"
+COMMIT="2fd39b75df696961e5ea190c7d74dd91f4849cd3"
 SHA2="9737906555a60911636532f00f1dc2866dc7cd6553beb106e9e57beabad41cdf"
diff --git a/tools/maghemite_mg_openapi_version b/tools/maghemite_mg_openapi_version
@@ -1,2 +1,2 @@
-COMMIT="579592bf474ec4b86805ada60c1b920b3beef5a7"
-SHA2="6c1fab8d5028b52a161d8bf02aae47844699cdc5f7b28e1ac519fc4ec1ab3971"
+COMMIT="2fd39b75df696961e5ea190c7d74dd91f4849cd3"
+SHA2="931efa310d972b1f8afba2308751fc6a2035afbaebba77b3a40a8358c123ba3c"
diff --git a/tools/maghemite_mgd_checksums b/tools/maghemite_mgd_checksums
@@ -1,2 +1,2 @@
-CIDL_SHA256="82aa1ca1d7701b2221c442d58f912be59798258d574effcb866ffab22753cf38"
-MGD_LINUX_SHA256="81231b30872fa1c581aa22c101f32d11f33f335758ac1fd2653436fbc7aab93f"
+CIDL_SHA256="802636775fa77dc6eec193e65fde87e403f6a11531745d47ef5e7ff13b242890"
+MGD_LINUX_SHA256="1bcadfd700902e3640843e0bb53d3defdbcd8d86c3279efa0953ae8d6437e2b0"