From 3237201b615c72109c8d9b365324d0453f0545b7 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Tue, 16 Jul 2024 19:57:54 +0000 Subject: [PATCH] Switch Gimlet revision from i64 to u32 Fixes #6095 --- dev-tools/omdb/src/bin/omdb/nexus.rs | 2 +- nexus/db-model/src/sled.rs | 12 ++++++------ nexus/db-model/src/switch.rs | 16 ++++++++++------ nexus/reconfigurator/planning/src/system.rs | 4 ++-- nexus/src/app/rack.rs | 2 +- nexus/test-utils/src/resource_helpers.rs | 2 +- nexus/types/src/external_api/shared.rs | 2 +- openapi/bootstrap-agent.json | 3 ++- openapi/nexus-internal.json | 3 ++- openapi/nexus.json | 3 ++- openapi/sled-agent.json | 3 ++- openapi/wicketd.json | 3 ++- schema/rss-sled-plan.json | 3 ++- sled-agent/src/http_entrypoints.rs | 7 +------ sled-agent/src/sled_agent.rs | 13 ++++--------- sled-hardware/src/illumos/mod.rs | 17 ++++++++++++++++- sled-hardware/types/src/lib.rs | 6 +++--- sled-storage/src/dataset.rs | 1 + wicketd/src/rss_config.rs | 2 +- 19 files changed, 60 insertions(+), 44 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 593e6c1645..8649d15aa6 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -1575,7 +1575,7 @@ async fn cmd_nexus_sleds_list_uninitialized( cubby: u16, serial: String, part: String, - revision: i64, + revision: u32, } let rows = sleds.into_iter().map(|sled| UninitializedSledRow { rack_id: sled.rack_id, diff --git a/nexus/db-model/src/sled.rs b/nexus/db-model/src/sled.rs index c177650991..b02a082f07 100644 --- a/nexus/db-model/src/sled.rs +++ b/nexus/db-model/src/sled.rs @@ -26,7 +26,7 @@ use uuid::Uuid; pub struct SledBaseboard { pub serial_number: String, pub part_number: String, - pub revision: i64, + pub revision: u32, } /// Hardware information about the sled. @@ -53,7 +53,7 @@ pub struct Sled { is_scrimlet: bool, serial_number: String, part_number: String, - revision: i64, + revision: SqlU32, pub usable_hardware_threads: SqlU32, pub usable_physical_ram: ByteCount, @@ -128,7 +128,7 @@ impl From for views::Sled { baseboard: shared::Baseboard { serial: sled.serial_number, part: sled.part_number, - revision: sled.revision, + revision: *sled.revision, }, policy: sled.policy.into(), state: sled.state.into(), @@ -155,7 +155,7 @@ impl From for params::SledAgentInfo { baseboard: Baseboard { serial: sled.serial_number.clone(), part: sled.part_number.clone(), - revision: sled.revision, + revision: *sled.revision, }, usable_hardware_threads: sled.usable_hardware_threads.into(), usable_physical_ram: sled.usable_physical_ram.into(), @@ -192,7 +192,7 @@ pub struct SledUpdate { is_scrimlet: bool, serial_number: String, part_number: String, - revision: i64, + revision: SqlU32, pub usable_hardware_threads: SqlU32, pub usable_physical_ram: ByteCount, @@ -221,7 +221,7 @@ impl SledUpdate { is_scrimlet: hardware.is_scrimlet, serial_number: baseboard.serial_number, part_number: baseboard.part_number, - revision: baseboard.revision, + revision: SqlU32(baseboard.revision), usable_hardware_threads: SqlU32::new( hardware.usable_hardware_threads, ), diff --git a/nexus/db-model/src/switch.rs b/nexus/db-model/src/switch.rs index 159888d91e..d5c523e9c4 100644 --- a/nexus/db-model/src/switch.rs +++ b/nexus/db-model/src/switch.rs @@ -1,5 +1,9 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + use super::Generation; -use crate::schema::switch; +use crate::{schema::switch, SqlU32}; use chrono::{DateTime, Utc}; use db_macros::Asset; use nexus_types::{external_api::shared, external_api::views, identity::Asset}; @@ -12,7 +16,7 @@ use uuid::Uuid; pub struct SwitchBaseboard { pub serial_number: String, pub part_number: String, - pub revision: i64, + pub revision: u32, } /// Database representation of a Switch. @@ -28,7 +32,7 @@ pub struct Switch { serial_number: String, part_number: String, - revision: i64, + revision: SqlU32, } impl Switch { @@ -37,7 +41,7 @@ impl Switch { id: Uuid, serial_number: String, part_number: String, - revision: i64, + revision: u32, rack_id: Uuid, ) -> Self { Self { @@ -47,7 +51,7 @@ impl Switch { rack_id, serial_number, part_number, - revision, + revision: SqlU32(revision), } } } @@ -60,7 +64,7 @@ impl From for views::Switch { baseboard: shared::Baseboard { serial: switch.serial_number, part: switch.part_number, - revision: switch.revision, + revision: *switch.revision, }, } } diff --git a/nexus/reconfigurator/planning/src/system.rs b/nexus/reconfigurator/planning/src/system.rs index 0499e0ef5b..5f00ea8172 100644 --- a/nexus/reconfigurator/planning/src/system.rs +++ b/nexus/reconfigurator/planning/src/system.rs @@ -521,7 +521,7 @@ impl Sled { sled_agent_client::types::Baseboard::Gimlet { identifier: serial.clone(), model: model.clone(), - revision: i64::from(revision), + revision, } } SledHardware::Pc => sled_agent_client::types::Baseboard::Pc { @@ -591,7 +591,7 @@ impl Sled { .map(|sledhw| sled_agent_client::types::Baseboard::Gimlet { identifier: sledhw.baseboard_id.serial_number.clone(), model: sledhw.baseboard_id.part_number.clone(), - revision: i64::from(sledhw.sp.baseboard_revision), + revision: sledhw.sp.baseboard_revision, }) .unwrap_or(sled_agent_client::types::Baseboard::Unknown); diff --git a/nexus/src/app/rack.rs b/nexus/src/app/rack.rs index ee3818f40c..422540c0b8 100644 --- a/nexus/src/app/rack.rs +++ b/nexus/src/app/rack.rs @@ -778,7 +778,7 @@ impl super::Nexus { baseboard: Baseboard { serial: k.serial_number.clone(), part: k.part_number.clone(), - revision: v.baseboard_revision.into(), + revision: v.baseboard_revision, }, rack_id: self.rack_id, cubby: v.sp_slot, diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 013ce7b630..1a92a6ef8e 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -332,7 +332,7 @@ pub async fn create_switch( client: &ClientTestContext, serial: &str, part: &str, - revision: i64, + revision: u32, rack_id: Uuid, ) -> views::Switch { object_put( diff --git a/nexus/types/src/external_api/shared.rs b/nexus/types/src/external_api/shared.rs index 32d8765a54..9bfa9c8358 100644 --- a/nexus/types/src/external_api/shared.rs +++ b/nexus/types/src/external_api/shared.rs @@ -267,7 +267,7 @@ pub enum UpdateableComponentType { pub struct Baseboard { pub serial: String, pub part: String, - pub revision: i64, + pub revision: u32, } /// A sled that has not been added to an initialized rack yet diff --git a/openapi/bootstrap-agent.json b/openapi/bootstrap-agent.json index 370f0fb404..879e8cdc3f 100644 --- a/openapi/bootstrap-agent.json +++ b/openapi/bootstrap-agent.json @@ -217,7 +217,8 @@ }, "revision": { "type": "integer", - "format": "int64" + "format": "uint32", + "minimum": 0 }, "type": { "type": "string", diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index c5fc2c3b56..257a505401 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -1526,7 +1526,8 @@ }, "revision": { "type": "integer", - "format": "int64" + "format": "uint32", + "minimum": 0 }, "serial": { "type": "string" diff --git a/openapi/nexus.json b/openapi/nexus.json index 8763441402..9e46a92039 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10013,7 +10013,8 @@ }, "revision": { "type": "integer", - "format": "int64" + "format": "uint32", + "minimum": 0 }, "serial": { "type": "string" diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index 13036f115b..27cfe576b7 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -1414,7 +1414,8 @@ }, "revision": { "type": "integer", - "format": "int64" + "format": "uint32", + "minimum": 0 }, "type": { "type": "string", diff --git a/openapi/wicketd.json b/openapi/wicketd.json index 2098f0bca4..34e7eadb54 100644 --- a/openapi/wicketd.json +++ b/openapi/wicketd.json @@ -880,7 +880,8 @@ }, "revision": { "type": "integer", - "format": "int64" + "format": "uint32", + "minimum": 0 }, "type": { "type": "string", diff --git a/schema/rss-sled-plan.json b/schema/rss-sled-plan.json index cb3c5c8eeb..1abba084ac 100644 --- a/schema/rss-sled-plan.json +++ b/schema/rss-sled-plan.json @@ -85,7 +85,8 @@ }, "revision": { "type": "integer", - "format": "int64" + "format": "uint32", + "minimum": 0.0 }, "type": { "type": "string", diff --git a/sled-agent/src/http_entrypoints.rs b/sled-agent/src/http_entrypoints.rs index ff8c13105a..2612e504f5 100644 --- a/sled-agent/src/http_entrypoints.rs +++ b/sled-agent/src/http_entrypoints.rs @@ -1021,12 +1021,7 @@ async fn inventory( async fn sled_identifiers( request_context: RequestContext, ) -> Result, HttpError> { - request_context - .context() - .sled_identifiers() - .await - .map(HttpResponseOk) - .map_err(HttpError::from) + Ok(HttpResponseOk(request_context.context().sled_identifiers().await)) } /// Get the internal state of the local bootstore node diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index 23a13487ef..dc946c1bfa 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -1197,20 +1197,15 @@ impl SledAgent { /// interested in the switch identifiers, MGS is the current best way to do /// that, by asking for the local switch's slot, and then that switch's SP /// state. - pub(crate) async fn sled_identifiers( - &self, - ) -> Result { + pub(crate) async fn sled_identifiers(&self) -> SledIdentifiers { let baseboard = self.inner.hardware.baseboard(); - Ok(SledIdentifiers { + SledIdentifiers { rack_id: self.inner.start_request.body.rack_id, sled_id: self.inner.id, model: baseboard.model().to_string(), - revision: baseboard - .revision() - .try_into() - .map_err(|_| Error::UnexpectedRevision(baseboard.revision()))?, + revision: baseboard.revision(), serial: baseboard.identifier().to_string(), - }) + } } /// Return basic information about ourselves: identity and status diff --git a/sled-hardware/src/illumos/mod.rs b/sled-hardware/src/illumos/mod.rs index 18e360c919..65f439eaeb 100644 --- a/sled-hardware/src/illumos/mod.rs +++ b/sled-hardware/src/illumos/mod.rs @@ -162,7 +162,7 @@ impl HardwareSnapshot { let baseboard = Baseboard::new_gimlet( string_from_property(&properties[0])?, string_from_property(&properties[1])?, - i64_from_property(&properties[2])?, + u32_from_property(&properties[2])?, ); let boot_storage_unit = BootStorageUnit::try_from(i64_from_property(&properties[3])?)?; @@ -412,6 +412,21 @@ fn get_parent_node<'a>( Ok(parent) } +/// Convert a property to a `u32` if possible, passing through an `i64`. +/// +/// Returns an error if either: +/// +/// - The actual devinfo property isn't an integer type. +/// - The value does not fit in a `u32`. +fn u32_from_property(prop: &Property<'_>) -> Result { + i64_from_property(prop).and_then(|val| { + u32::try_from(val).map_err(|_| Error::UnexpectedPropertyType { + name: prop.name(), + ty: "u32".to_string(), + }) + }) +} + fn i64_from_property(prop: &Property<'_>) -> Result { prop.as_i64().ok_or_else(|| Error::UnexpectedPropertyType { name: prop.name(), diff --git a/sled-hardware/types/src/lib.rs b/sled-hardware/types/src/lib.rs index e589498ff8..b34b5b1f42 100644 --- a/sled-hardware/types/src/lib.rs +++ b/sled-hardware/types/src/lib.rs @@ -22,7 +22,7 @@ pub mod underlay; )] #[serde(tag = "type", rename_all = "snake_case")] pub enum Baseboard { - Gimlet { identifier: String, model: String, revision: i64 }, + Gimlet { identifier: String, model: String, revision: u32 }, Unknown, @@ -34,7 +34,7 @@ impl Baseboard { pub fn new_gimlet( identifier: String, model: String, - revision: i64, + revision: u32, ) -> Self { Self::Gimlet { identifier, model, revision } } @@ -73,7 +73,7 @@ impl Baseboard { } } - pub fn revision(&self) -> i64 { + pub fn revision(&self) -> u32 { match self { Self::Gimlet { revision, .. } => *revision, Self::Pc { .. } => 0, diff --git a/sled-storage/src/dataset.rs b/sled-storage/src/dataset.rs index 7846826ee8..e3d43fa39a 100644 --- a/sled-storage/src/dataset.rs +++ b/sled-storage/src/dataset.rs @@ -311,6 +311,7 @@ pub(crate) async fn ensure_zpool_has_datasets( if let Ok(epoch) = epoch_str.parse::() { epoch } else { + println!(">>>> {epoch_str}"); return Err(DatasetError::CannotParseEpochProperty( dataset.to_string(), )); diff --git a/wicketd/src/rss_config.rs b/wicketd/src/rss_config.rs index 3894697dd2..b7545af19d 100644 --- a/wicketd/src/rss_config.rs +++ b/wicketd/src/rss_config.rs @@ -130,7 +130,7 @@ impl CurrentRssConfig { let baseboard = Baseboard::new_gimlet( state.serial_number.clone(), state.model.clone(), - state.revision.into(), + state.revision, ); let bootstrap_ip = bootstrap_sleds.get(&baseboard).copied(); Some(BootstrapSledDescription {