From 159000abc3df207d94f094b0e27302b9778774f9 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Wed, 25 Oct 2023 12:19:49 -0700 Subject: [PATCH] Port settings fixes (#4343) --- nexus/db-model/src/switch_port.rs | 11 ++++++-- .../src/db/datastore/switch_port.rs | 7 ++++- nexus/src/app/rack.rs | 10 +++++-- .../app/sagas/switch_port_settings_apply.rs | 28 +++++++++++-------- nexus/src/app/switch_port.rs | 26 ++++++++++++++--- package-manifest.toml | 24 ++++++++-------- sled-agent/src/bootstrap/early_networking.rs | 4 +-- tools/create_virtual_hardware.sh | 2 +- tools/dendrite_openapi_version | 4 +-- tools/dendrite_stub_checksums | 6 ++-- tools/maghemite_ddm_openapi_version | 2 +- tools/maghemite_mg_openapi_version | 2 +- tools/maghemite_mgd_checksums | 4 +-- wicketd/src/preflight_check/uplink.rs | 9 ++---- 14 files changed, 88 insertions(+), 51 deletions(-) diff --git a/nexus/db-model/src/switch_port.rs b/nexus/db-model/src/switch_port.rs index f6df50ef97..44588899b6 100644 --- a/nexus/db-model/src/switch_port.rs +++ b/nexus/db-model/src/switch_port.rs @@ -246,14 +246,21 @@ pub struct SwitchPortSettings { } impl SwitchPortSettings { - pub fn new(id: &external::IdentityMetadataCreateParams) -> Self { + pub fn new(meta: &external::IdentityMetadataCreateParams) -> Self { Self { identity: SwitchPortSettingsIdentity::new( Uuid::new_v4(), - id.clone(), + meta.clone(), ), } } + + pub fn with_id( + id: Uuid, + meta: &external::IdentityMetadataCreateParams, + ) -> Self { + Self { identity: SwitchPortSettingsIdentity::new(id, meta.clone()) } + } } impl Into for SwitchPortSettings { diff --git a/nexus/db-queries/src/db/datastore/switch_port.rs b/nexus/db-queries/src/db/datastore/switch_port.rs index f2126bd968..f301750ee9 100644 --- a/nexus/db-queries/src/db/datastore/switch_port.rs +++ b/nexus/db-queries/src/db/datastore/switch_port.rs @@ -139,6 +139,7 @@ impl DataStore { &self, opctx: &OpContext, params: ¶ms::SwitchPortSettingsCreate, + id: Option, ) -> CreateResult { use db::schema::{ address_lot::dsl as address_lot_dsl, @@ -171,7 +172,11 @@ impl DataStore { // Audit external networking database transaction usage conn.transaction_async(|conn| async move { // create the top level port settings object - let port_settings = SwitchPortSettings::new(¶ms.identity); + let port_settings = match id { + Some(id) => SwitchPortSettings::with_id(id, ¶ms.identity), + None => SwitchPortSettings::new(¶ms.identity), + }; + //let port_settings = SwitchPortSettings::new(¶ms.identity); let db_port_settings: SwitchPortSettings = diesel::insert_into(port_settings_dsl::switch_port_settings) .values(port_settings) diff --git a/nexus/src/app/rack.rs b/nexus/src/app/rack.rs index 907c3ffa78..7697d34ecd 100644 --- a/nexus/src/app/rack.rs +++ b/nexus/src/app/rack.rs @@ -278,7 +278,9 @@ impl super::Nexus { let qsfp_ports: Vec = all_ports .iter() - .filter(|port| port.starts_with("qsfp")) + .filter(|port| { + matches!(port, dpd_client::types::PortId::Qsfp(_)) + }) .map(|port| port.to_string().parse().unwrap()) .collect(); @@ -493,7 +495,11 @@ impl super::Nexus { match self .db_datastore - .switch_port_settings_create(opctx, &port_settings_params) + .switch_port_settings_create( + opctx, + &port_settings_params, + None, + ) .await { Ok(_) | Err(Error::ObjectAlreadyExists { .. }) => Ok(()), diff --git a/nexus/src/app/sagas/switch_port_settings_apply.rs b/nexus/src/app/sagas/switch_port_settings_apply.rs index 93dc45751a..fb06dc5fc0 100644 --- a/nexus/src/app/sagas/switch_port_settings_apply.rs +++ b/nexus/src/app/sagas/switch_port_settings_apply.rs @@ -234,11 +234,7 @@ pub(crate) fn api_to_dpd_port_settings( dpd_port_settings.v4_routes.insert( Ipv4Cidr { prefix: n.ip(), prefix_len: n.prefix() } .to_string(), - RouteSettingsV4 { - link_id: link_id.0, - nexthop: gw, - vid: r.vid.map(Into::into), - }, + vec![RouteSettingsV4 { link_id: link_id.0, nexthop: gw }], ); } IpNetwork::V6(n) => { @@ -253,11 +249,7 @@ pub(crate) fn api_to_dpd_port_settings( dpd_port_settings.v6_routes.insert( Ipv6Cidr { prefix: n.ip(), prefix_len: n.prefix() } .to_string(), - RouteSettingsV6 { - link_id: link_id.0, - nexthop: gw, - vid: r.vid.map(Into::into), - }, + vec![RouteSettingsV6 { link_id: link_id.0, nexthop: gw }], ); } } @@ -294,8 +286,20 @@ async fn spa_ensure_switch_port_settings( dpd_client.port_settings_apply(&port_id, &dpd_port_settings).await }) .await - .map_err(|e| { - ActionError::action_failed(format!("dpd port settings apply {e}")) + .map_err(|e| match e { + progenitor_client::Error::ErrorResponse(ref er) => { + if er.status().is_client_error() { + ActionError::action_failed(format!( + "bad request: dpd port settings apply {}", + er.message, + )) + } else { + ActionError::action_failed(format!( + "dpd port settings apply {e}" + )) + } + } + _ => ActionError::action_failed(format!("dpd port settings apply {e}")), })?; Ok(()) diff --git a/nexus/src/app/switch_port.rs b/nexus/src/app/switch_port.rs index 03b874727b..acc57459fd 100644 --- a/nexus/src/app/switch_port.rs +++ b/nexus/src/app/switch_port.rs @@ -8,6 +8,8 @@ use crate::app::sagas; use crate::external_api::params; use db::datastore::SwitchPortSettingsCombinedResult; +use dropshot::HttpError; +use http::StatusCode; use ipnetwork::IpNetwork; use nexus_db_model::{SwitchLinkFec, SwitchLinkSpeed}; use nexus_db_queries::authn; @@ -51,7 +53,9 @@ impl super::Nexus { .await { Ok(id) => self.switch_port_settings_update(opctx, id, params).await, - Err(_) => self.switch_port_settings_create(opctx, params).await, + Err(_) => { + self.switch_port_settings_create(opctx, params, None).await + } } } @@ -59,8 +63,9 @@ impl super::Nexus { self: &Arc, opctx: &OpContext, params: params::SwitchPortSettingsCreate, + id: Option, ) -> CreateResult { - self.db_datastore.switch_port_settings_create(opctx, ¶ms).await + self.db_datastore.switch_port_settings_create(opctx, ¶ms, id).await } pub(crate) async fn switch_port_settings_update( @@ -80,7 +85,11 @@ impl super::Nexus { // create new settings let result = self - .switch_port_settings_create(opctx, new_settings.clone()) + .switch_port_settings_create( + opctx, + new_settings.clone(), + Some(switch_port_settings_id), + ) .await?; // run the port settings apply saga for each port referencing the @@ -104,7 +113,16 @@ impl super::Nexus { >( saga_params, ) - .await?; + .await + .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 + } + })?; } Ok(result) diff --git a/package-manifest.toml b/package-manifest.toml index 5fc8114116..b8ffb2756a 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -422,10 +422,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 = "2f25a2005521f643317879b46692141b4127608a" +source.commit = "d7169a61fd8833b3a1e6f46d897ca3295b2a28b6" # The SHA256 digest is automatically posted to: # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/maghemite/image//maghemite.sha256.txt -source.sha256 = "e808388cd080a3325fb5429314a5674809bcde24ad0456b58b57c87cbaa1300d" +source.sha256 = "d871406ed926571efebdab248de08d4f1ca6c31d4f9a691ce47b186474165c57" output.type = "tarball" [package.mg-ddm] @@ -438,10 +438,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 = "2f25a2005521f643317879b46692141b4127608a" +source.commit = "d7169a61fd8833b3a1e6f46d897ca3295b2a28b6" # The SHA256 digest is automatically posted to: # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/maghemite/image//mg-ddm.sha256.txt -source.sha256 = "27e4845fd11b9768559eb9835309387e83c95628a6a292977e734e8bc7f9fa0f" +source.sha256 = "85ec05a8726989b5cb0a567de6b0855f6f84b6f3409ac99ccaf372be5821e45d" output.type = "zone" output.intermediate_only = true @@ -453,10 +453,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 = "2f25a2005521f643317879b46692141b4127608a" +source.commit = "d7169a61fd8833b3a1e6f46d897ca3295b2a28b6" # The SHA256 digest is automatically posted to: # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/maghemite/image//mg-ddm.sha256.txt -source.sha256 = "16878501f5440590674acd82bee6ce5dcf3d1326531c25064dd9c060ab6440a4" +source.sha256 = "452dfb3491e1b6d4df6be1cb689921f59623aed082e47606a78c0f44d918f66a" output.type = "zone" output.intermediate_only = true @@ -473,8 +473,8 @@ only_for_targets.image = "standard" # 2. Copy dendrite.tar.gz from dendrite/out to omicron/out source.type = "prebuilt" source.repo = "dendrite" -source.commit = "c0cbc39b55fac54b95468304c497e00f3d3cf686" -source.sha256 = "3706e0e8230b7f76407ec0acea9020b9efc7d6c78b74c304102fd8e62cac6760" +source.commit = "343e3a572cc02efe3f8b68f9affd008623a33966" +source.sha256 = "0808f331741e02d55e199847579dfd01f3658b21c7122cef8c3f9279f43dbab0" output.type = "zone" output.intermediate_only = true @@ -498,8 +498,8 @@ only_for_targets.image = "standard" # 2. Copy the output zone image from dendrite/out to omicron/out source.type = "prebuilt" source.repo = "dendrite" -source.commit = "c0cbc39b55fac54b95468304c497e00f3d3cf686" -source.sha256 = "f0847927f7d7197d9a5c4267a0bd0af609d18fd8d6d9b80755c370872c5297fa" +source.commit = "343e3a572cc02efe3f8b68f9affd008623a33966" +source.sha256 = "c359de1be5073a484d86d4c58e8656a36002ce1dc38506f97b730e21615ccae1" output.type = "zone" output.intermediate_only = true @@ -516,8 +516,8 @@ only_for_targets.image = "standard" # 2. Copy dendrite.tar.gz from dendrite/out to omicron/out/dendrite-softnpu.tar.gz source.type = "prebuilt" source.repo = "dendrite" -source.commit = "c0cbc39b55fac54b95468304c497e00f3d3cf686" -source.sha256 = "33b5897db1fe7b57d282531724ecd7bf74f5156f9aa23f10c6f0d9b54c38a987" +source.commit = "343e3a572cc02efe3f8b68f9affd008623a33966" +source.sha256 = "110bfbfb2cf3d3471f3e3a64d26129c7a02f6c5857f9623ebb99690728c3b2ff" 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 index 6c19080e9c..9adfa47d9b 100644 --- a/sled-agent/src/bootstrap/early_networking.rs +++ b/sled-agent/src/bootstrap/early_networking.rs @@ -509,7 +509,7 @@ impl<'a> EarlyNetworkSetup<'a> { { dpd_port_settings.v4_routes.insert( dst.to_string(), - RouteSettingsV4 { link_id: link_id.0, nexthop, vid: None }, + vec![RouteSettingsV4 { link_id: link_id.0, nexthop }], ); } if let (IpNetwork::V6(dst), IpAddr::V6(nexthop)) = @@ -517,7 +517,7 @@ impl<'a> EarlyNetworkSetup<'a> { { dpd_port_settings.v6_routes.insert( dst.to_string(), - RouteSettingsV6 { link_id: link_id.0, nexthop, vid: None }, + vec![RouteSettingsV6 { link_id: link_id.0, nexthop }], ); } } diff --git a/tools/create_virtual_hardware.sh b/tools/create_virtual_hardware.sh index 248cbcde73..908cb752e9 100755 --- a/tools/create_virtual_hardware.sh +++ b/tools/create_virtual_hardware.sh @@ -62,7 +62,7 @@ function ensure_softnpu_zone { --omicron-zone \ --ports sc0_0,tfportrear0_0 \ --ports sc0_1,tfportqsfp0_0 \ - --sidecar-lite-branch omicron-tracking + --sidecar-lite-branch main } "$SOURCE_DIR"/scrimlet/softnpu-init.sh success "softnpu zone exists" diff --git a/tools/dendrite_openapi_version b/tools/dendrite_openapi_version index 9a2ea85ac0..c91d1c2e98 100644 --- a/tools/dendrite_openapi_version +++ b/tools/dendrite_openapi_version @@ -1,2 +1,2 @@ -COMMIT="c0cbc39b55fac54b95468304c497e00f3d3cf686" -SHA2="cb3f0cfbe6216d2441d34e0470252e0fb142332e47b33b65c24ef7368a694b6d" +COMMIT="343e3a572cc02efe3f8b68f9affd008623a33966" +SHA2="544ab42ccc7942d8ece9cdc80cd85d002bcf9d5646a291322bf2f79087ab6df0" diff --git a/tools/dendrite_stub_checksums b/tools/dendrite_stub_checksums index fe52c59381..8fa98114fb 100644 --- a/tools/dendrite_stub_checksums +++ b/tools/dendrite_stub_checksums @@ -1,3 +1,3 @@ -CIDL_SHA256_ILLUMOS="3706e0e8230b7f76407ec0acea9020b9efc7d6c78b74c304102fd8e62cac6760" -CIDL_SHA256_LINUX_DPD="b275a1c688eae1024b9ce1cbb766a66e37072e84b4a6cbc18746c903739ccf51" -CIDL_SHA256_LINUX_SWADM="7e604cc4b67c1a711a63ece2a8d0e2e7c8ef2b9ac6bb433b3c2e02f5f66018ba" +CIDL_SHA256_ILLUMOS="0808f331741e02d55e199847579dfd01f3658b21c7122cef8c3f9279f43dbab0" +CIDL_SHA256_LINUX_DPD="3e276dd553dd7cdb75c8ad023c2cd29b91485fafb94f27097a745b2b7ef5ecea" +CIDL_SHA256_LINUX_SWADM="645faf8a93bcae9814b2f116bccd66a54763332b56220e93b66316c853ce13d2" diff --git a/tools/maghemite_ddm_openapi_version b/tools/maghemite_ddm_openapi_version index a315c31d4b..89c3e46164 100644 --- a/tools/maghemite_ddm_openapi_version +++ b/tools/maghemite_ddm_openapi_version @@ -1,2 +1,2 @@ -COMMIT="2f25a2005521f643317879b46692141b4127608a" +COMMIT="d7169a61fd8833b3a1e6f46d897ca3295b2a28b6" SHA2="9737906555a60911636532f00f1dc2866dc7cd6553beb106e9e57beabad41cdf" diff --git a/tools/maghemite_mg_openapi_version b/tools/maghemite_mg_openapi_version index acd5a5f546..a7e18285ae 100644 --- a/tools/maghemite_mg_openapi_version +++ b/tools/maghemite_mg_openapi_version @@ -1,2 +1,2 @@ -COMMIT="2f25a2005521f643317879b46692141b4127608a" +COMMIT="d7169a61fd8833b3a1e6f46d897ca3295b2a28b6" SHA2="d0f7611e5ecd049b0f83bcfa843942401f155a0be36d9a2dfd73b8341d5f816e" diff --git a/tools/maghemite_mgd_checksums b/tools/maghemite_mgd_checksums index 45fbec5274..e65e1fc0a2 100644 --- a/tools/maghemite_mgd_checksums +++ b/tools/maghemite_mgd_checksums @@ -1,2 +1,2 @@ -CIDL_SHA256="16878501f5440590674acd82bee6ce5dcf3d1326531c25064dd9c060ab6440a4" -MGD_LINUX_SHA256="45e5ddc9d81cfcb94917f9c58942c3a7211fb34a3c563fbfc2434b0a97306b3d" +CIDL_SHA256="452dfb3491e1b6d4df6be1cb689921f59623aed082e47606a78c0f44d918f66a" +MGD_LINUX_SHA256="d4c48eb6374c0cc7812b7af2c0ac92acdcbc91b7718a9ce64d069da00ae5ae73" diff --git a/wicketd/src/preflight_check/uplink.rs b/wicketd/src/preflight_check/uplink.rs index ebcba90645..4d199d28b8 100644 --- a/wicketd/src/preflight_check/uplink.rs +++ b/wicketd/src/preflight_check/uplink.rs @@ -190,11 +190,8 @@ fn add_steps_for_single_local_uplink_preflight_check<'a>( { Ok(_response) => { let metadata = vec![format!( - "configured {}/{}: ips {:#?}, routes {:#?}", - *port_id, - link_id.0, - uplink.addresses, - uplink.routes + "configured {:?}/{}: ips {:#?}, routes {:#?}", + port_id, link_id.0, uplink.addresses, uplink.routes )]; StepSuccess::new((port_id, link_id)) .with_metadata(metadata) @@ -790,7 +787,7 @@ fn build_port_settings( { port_settings.v4_routes.insert( dst.to_string(), - RouteSettingsV4 { link_id: link_id.0, nexthop, vid: None }, + vec![RouteSettingsV4 { link_id: link_id.0, nexthop }], ); } }