From ce8e1891a11ce58e277324b2d0606c68f98dc87d Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 Mar 2026 09:36:02 -0400 Subject: [PATCH 1/7] wip: remove illumos-utils dep from sled-agent-types-versions --- Cargo.lock | 1 - illumos-utils/src/svcs.rs | 2 + illumos-utils/src/zpool.rs | 5 +- sled-agent/types/versions/Cargo.toml | 1 - .../add_attached_subnets/attached_subnet.rs | 8 +- .../inventory.rs | 22 +++++- .../types/versions/src/impls/inventory.rs | 75 ++++++++++++++++++- .../types/versions/src/initial/inventory.rs | 2 + sled-agent/types/versions/src/latest.rs | 3 + .../modify_services_in_inventory/inventory.rs | 38 +++++++++- 10 files changed, 146 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9bf955f36f6..981e52271ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13574,7 +13574,6 @@ dependencies = [ "chrono", "daft", "iddqd", - "illumos-utils", "indent_write", "omicron-common", "omicron-ledger", diff --git a/illumos-utils/src/svcs.rs b/illumos-utils/src/svcs.rs index aef65acf9d1..de528c10ddf 100644 --- a/illumos-utils/src/svcs.rs +++ b/illumos-utils/src/svcs.rs @@ -165,6 +165,7 @@ impl SvcsResult { } } +/* /// Each service instance is always in a well-defined state based on its /// dependencies, the results of the execution of its methods, and its potential /// contracts events. See for more information. @@ -233,6 +234,7 @@ pub struct Svc { zone: String, state: SvcState, } +*/ #[cfg(test)] mod tests { diff --git a/illumos-utils/src/zpool.rs b/illumos-utils/src/zpool.rs index 595ef6cc55a..345093ffa69 100644 --- a/illumos-utils/src/zpool.rs +++ b/illumos-utils/src/zpool.rs @@ -6,10 +6,7 @@ use crate::{ExecutionError, PFEXEC, execute_async}; use camino::{Utf8Path, Utf8PathBuf}; -use schemars::JsonSchema; -use serde::{Deserialize, Serialize}; use slog_error_chain::SlogInlineError; -use std::fmt::Display; use std::str::FromStr; use tokio::process::Command; @@ -64,6 +61,7 @@ pub struct GetInfoError { err: Error, } +/* #[derive( Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema, )] @@ -114,6 +112,7 @@ impl Display for ZpoolHealth { write!(f, "{s}") } } +*/ /// Describes a Zpool. #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/sled-agent/types/versions/Cargo.toml b/sled-agent/types/versions/Cargo.toml index 9cab1ecf651..cdbad58eb89 100644 --- a/sled-agent/types/versions/Cargo.toml +++ b/sled-agent/types/versions/Cargo.toml @@ -16,7 +16,6 @@ camino.workspace = true chrono.workspace = true daft.workspace = true iddqd.workspace = true -illumos-utils.workspace = true indent_write.workspace = true omicron-common.workspace = true omicron-ledger.workspace = true diff --git a/sled-agent/types/versions/src/add_attached_subnets/attached_subnet.rs b/sled-agent/types/versions/src/add_attached_subnets/attached_subnet.rs index 66dbca8f650..dc9124a851a 100644 --- a/sled-agent/types/versions/src/add_attached_subnets/attached_subnet.rs +++ b/sled-agent/types/versions/src/add_attached_subnets/attached_subnet.rs @@ -3,8 +3,8 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use iddqd::IdOrdMap; -use illumos_utils::opte::cidr_to_net; -use illumos_utils::opte::net_to_cidr; +//use illumos_utils::opte::cidr_to_net; +//use illumos_utils::opte::net_to_cidr; use omicron_uuid_kinds::PropolisUuid; use oxnet::IpNet; use schemars::JsonSchema; @@ -36,6 +36,7 @@ impl iddqd::IdOrdItem for AttachedSubnet { iddqd::id_upcast!(); } +/* impl From for illumos_utils::opte::AttachedSubnet { fn from(value: AttachedSubnet) -> Self { Self { cidr: net_to_cidr(value.subnet), kind: value.kind.into() } @@ -47,6 +48,7 @@ impl From for AttachedSubnet { Self { subnet: cidr_to_net(value.cidr), kind: value.kind.into() } } } +*/ /// The kind of attached subnet. #[derive(Clone, Copy, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] @@ -58,6 +60,7 @@ pub enum AttachedSubnetKind { External, } +/* impl From for illumos_utils::opte::AttachedSubnetKind { fn from(value: AttachedSubnetKind) -> Self { match value { @@ -75,6 +78,7 @@ impl From for AttachedSubnetKind { } } } +*/ /// Path parameters for referring to a single subnet attached to an instance. #[derive(Clone, Copy, Debug, Deserialize, JsonSchema, Serialize)] diff --git a/sled-agent/types/versions/src/add_zpool_health_to_inventory/inventory.rs b/sled-agent/types/versions/src/add_zpool_health_to_inventory/inventory.rs index 90682daeb56..d9df9893782 100644 --- a/sled-agent/types/versions/src/add_zpool_health_to_inventory/inventory.rs +++ b/sled-agent/types/versions/src/add_zpool_health_to_inventory/inventory.rs @@ -3,7 +3,6 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use iddqd::IdOrdMap; -use illumos_utils::zpool::ZpoolHealth; use omicron_common::api::external::ByteCount; use omicron_common::snake_case_result; use omicron_common::snake_case_result::SnakeCaseResult; @@ -26,6 +25,27 @@ use crate::v16::inventory::ConfigReconcilerInventory; use crate::v16::inventory::SingleMeasurementInventory; use crate::v22; +#[derive( + Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema, +)] +#[serde(rename_all = "snake_case")] +pub enum ZpoolHealth { + /// The device is online and functioning. + Online, + /// One or more components are degraded or faulted, but sufficient + /// replicas exist to continue functioning. + Degraded, + /// One or more components are degraded or faulted, and insufficient + /// replicas exist to continue functioning. + Faulted, + /// The device was explicitly taken offline by "zpool offline". + Offline, + /// The device was physically removed. + Removed, + /// The device could not be opened. + Unavailable, +} + /// Identity and basic status information about this sled agent #[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] pub struct Inventory { diff --git a/sled-agent/types/versions/src/impls/inventory.rs b/sled-agent/types/versions/src/impls/inventory.rs index 59761b4abff..6c62db26f60 100644 --- a/sled-agent/types/versions/src/impls/inventory.rs +++ b/sled-agent/types/versions/src/impls/inventory.rs @@ -5,6 +5,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::fmt::{self, Write}; use std::net::{IpAddr, Ipv6Addr}; +use std::str::FromStr; use camino::Utf8PathBuf; use chrono::Utc; @@ -26,8 +27,8 @@ use crate::latest::inventory::{ OmicronFileSourceResolverInventory, OmicronSledConfig, OmicronZoneConfig, OmicronZoneImageSource, OmicronZoneType, OmicronZonesConfig, RemoveMupdateOverrideBootSuccessInventory, RemoveMupdateOverrideInventory, - SingleMeasurementInventory, SvcsEnabledNotOnline, SvcsError, - ZoneArtifactInventory, ZoneKind, + SingleMeasurementInventory, SvcsEnabledNotOnline, SvcState, + ZoneArtifactInventory, ZoneKind, ZpoolHealth, }; impl ZoneKind { @@ -360,6 +361,7 @@ impl OmicronZoneConfig { self.zone_type.underlay_ip() } + /* /// Returns the zone name for this zone configuration. pub fn zone_name(&self) -> String { illumos_utils::running_zone::InstalledZone::get_zone_name( @@ -367,6 +369,7 @@ impl OmicronZoneConfig { Some(self.id), ) } + */ /// If this kind of zone has an associated dataset, return the dataset's /// name. Otherwise, return `None`. @@ -873,6 +876,7 @@ impl HostPhase2DesiredSlots { } } +/* impl From for SvcsEnabledNotOnline { fn from(value: illumos_utils::svcs::SvcsResult) -> Self { let illumos_utils::svcs::SvcsResult { @@ -903,6 +907,7 @@ impl From for SvcsError { } } } +*/ impl Default for OmicronSledConfig { fn default() -> Self { @@ -944,3 +949,69 @@ impl fmt::Display for SingleMeasurementInventoryDisplay<'_> { Ok(()) } } + +#[derive(Debug, thiserror::Error)] +#[error("unrecognized zpool health value `{0}`")] +pub struct ZpoolHeathParseError(pub String); + +impl FromStr for ZpoolHealth { + type Err = ZpoolHeathParseError; + + fn from_str(s: &str) -> Result { + match s { + "ONLINE" => Ok(ZpoolHealth::Online), + "DEGRADED" => Ok(ZpoolHealth::Degraded), + "FAULTED" => Ok(ZpoolHealth::Faulted), + "OFFLINE" => Ok(ZpoolHealth::Offline), + "REMOVED" => Ok(ZpoolHealth::Removed), + "UNAVAIL" => Ok(ZpoolHealth::Unavailable), + _ => Err(ZpoolHeathParseError(s.to_owned())), + } + } +} + +impl fmt::Display for ZpoolHealth { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let s = match self { + ZpoolHealth::Online => "online", + ZpoolHealth::Degraded => "degraded", + ZpoolHealth::Faulted => "faulted", + ZpoolHealth::Offline => "offline", + ZpoolHealth::Removed => "removed", + ZpoolHealth::Unavailable => "unavailable", + }; + write!(f, "{s}") + } +} + +impl From<&'_ str> for SvcState { + fn from(value: &str) -> Self { + match value { + "uninitialized" => SvcState::Uninitialized, + "offline" => SvcState::Offline, + "online" => SvcState::Online, + "degraded" => SvcState::Degraded, + "maintenance" => SvcState::Maintenance, + "disabled" => SvcState::Disabled, + "legacy_run" => SvcState::LegacyRun, + _ => SvcState::Unknown, + } + } +} + +impl fmt::Display for SvcState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let state = match self { + SvcState::Uninitialized => "uninitialized", + SvcState::Offline => "offline", + SvcState::Online => "online", + SvcState::Degraded => "degraded", + SvcState::Maintenance => "maintenance", + SvcState::Disabled => "disabled", + SvcState::LegacyRun => "legacy_run", + SvcState::Unknown => "unknown", + }; + + write!(f, "{state}") + } +} diff --git a/sled-agent/types/versions/src/initial/inventory.rs b/sled-agent/types/versions/src/initial/inventory.rs index fadf4b848c9..c71060cfb01 100644 --- a/sled-agent/types/versions/src/initial/inventory.rs +++ b/sled-agent/types/versions/src/initial/inventory.rs @@ -89,6 +89,7 @@ pub struct InventoryDataset { pub compression: String, } +/* impl From for InventoryDataset { fn from(props: illumos_utils::zfs::DatasetProperties) -> Self { Self { @@ -102,6 +103,7 @@ impl From for InventoryDataset { } } } +*/ /// Describes the role of the sled within the rack. /// diff --git a/sled-agent/types/versions/src/latest.rs b/sled-agent/types/versions/src/latest.rs index 13a25c808c4..d8bd6899253 100644 --- a/sled-agent/types/versions/src/latest.rs +++ b/sled-agent/types/versions/src/latest.rs @@ -155,8 +155,11 @@ pub mod inventory { pub use crate::v16::inventory::SingleMeasurementInventory; pub use crate::v24::inventory::InventoryZpool; + pub use crate::v24::inventory::ZpoolHealth; pub use crate::v28::inventory::Inventory; + pub use crate::v28::inventory::Svc; + pub use crate::v28::inventory::SvcState; pub use crate::v28::inventory::SvcsEnabledNotOnline; pub use crate::v28::inventory::SvcsEnabledNotOnlineResult; pub use crate::v28::inventory::SvcsError; diff --git a/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs b/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs index 1dd813eb268..dc6664bb7a4 100644 --- a/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs +++ b/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs @@ -5,7 +5,6 @@ use chrono::DateTime; use chrono::Utc; use iddqd::IdOrdMap; -use illumos_utils::svcs::Svc; use omicron_common::api::external::ByteCount; use omicron_uuid_kinds::SledUuid; use schemars::JsonSchema; @@ -24,6 +23,43 @@ use crate::v16::inventory::SingleMeasurementInventory; use crate::v24; use crate::v24::inventory::InventoryZpool; +/// Each service instance is always in a well-defined state based on its +/// dependencies, the results of the execution of its methods, and its potential +/// contracts events. See for more information. +#[derive( + Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema, +)] +#[serde(rename_all = "snake_case")] +pub enum SvcState { + /// Initial state for all service instances. + Uninitialized, + /// The instance is enabled, but not yet running or available to run. + Offline, + /// The instance is enabled and running or is available to run. + Online, + /// The instance is enabled and running or available to run. It is, however, + /// functioning at a limited capacity in comparison to normal operation. + Degraded, + /// The instance is enabled, but not able to run. + Maintenance, + /// The instance is disabled. + Disabled, + /// Represents a legacy instance that is not managed by the service + /// management facility. + LegacyRun, + /// We were unable to determine the state of the service instance. + Unknown, +} + +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +/// Information about an SMF service that is enabled but not running +pub struct Svc { + pub fmri: String, + pub zone: String, + pub state: SvcState, +} + /// Identity and basic status information about this sled agent #[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] pub struct Inventory { From acb25a169a47570bed12f9f28d0188891b2d4664 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 Mar 2026 09:44:31 -0400 Subject: [PATCH 2/7] wip: illumos-utils compiles again, dep on sled-agent-types --- Cargo.lock | 1 + illumos-utils/Cargo.toml | 1 + illumos-utils/src/svcs.rs | 5 ++-- illumos-utils/src/zpool.rs | 29 +++++++++++++------ .../types/versions/src/impls/inventory.rs | 8 ++--- sled-agent/types/versions/src/latest.rs | 1 + 6 files changed, 30 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 981e52271ac..be80cb79150 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5230,6 +5230,7 @@ dependencies = [ "schemars 0.8.22", "serde", "serde_json", + "sled-agent-types", "slog", "slog-async", "slog-error-chain", diff --git a/illumos-utils/Cargo.toml b/illumos-utils/Cargo.toml index 55c9bb2a281..1077bac6bfa 100644 --- a/illumos-utils/Cargo.toml +++ b/illumos-utils/Cargo.toml @@ -36,6 +36,7 @@ oxlog.workspace = true oxnet.workspace = true schemars.workspace = true serde.workspace = true +sled-agent-types.workspace = true slog.workspace = true slog-async.workspace = true slog-term.workspace = true diff --git a/illumos-utils/src/svcs.rs b/illumos-utils/src/svcs.rs index de528c10ddf..f1efa62d0d3 100644 --- a/illumos-utils/src/svcs.rs +++ b/illumos-utils/src/svcs.rs @@ -19,7 +19,8 @@ use serde::Deserialize; use serde::Serialize; use slog::Logger; use slog::{error, info}; -use std::fmt::Display; +use sled_agent_types::inventory::Svc; +use sled_agent_types::inventory::SvcState; #[cfg(target_os = "illumos")] use tokio::process::Command; @@ -89,7 +90,7 @@ impl SvcsResult { if let Some(svc_state) = svc.next() { // Only parse services that are in a known SMF service state. - let state = SvcState::from(svc_state.to_string()); + let state = SvcState::from(svc_state); match &state { SvcState::Maintenance | SvcState::Degraded diff --git a/illumos-utils/src/zpool.rs b/illumos-utils/src/zpool.rs index 345093ffa69..76cfb352d90 100644 --- a/illumos-utils/src/zpool.rs +++ b/illumos-utils/src/zpool.rs @@ -6,7 +6,10 @@ use crate::{ExecutionError, PFEXEC, execute_async}; use camino::{Utf8Path, Utf8PathBuf}; +use sled_agent_types::inventory::ZpoolHealth; +use sled_agent_types::inventory::ZpoolHealthParseError; use slog_error_chain::SlogInlineError; +use std::num::ParseIntError; use std::str::FromStr; use tokio::process::Command; @@ -17,8 +20,18 @@ const ZPOOL: &str = "/usr/sbin/zpool"; pub const ZPOOL_MOUNTPOINT_ROOT: &str = "/"; #[derive(thiserror::Error, Debug, PartialEq, Eq)] -#[error("Failed to parse output: {0}")] -pub struct ParseError(String); +pub enum ZpoolInfoParseError { + #[error(transparent)] + ZpoolHealth(#[from] ZpoolHealthParseError), + #[error("zpool list output: missing field `{0}`)")] + MissingField(&'static str), + #[error("zpool list output: failed to parse field `{field}`")] + IntegerField { + field: &'static str, + #[source] + err: ParseIntError, + }, +} #[derive(thiserror::Error, Debug, SlogInlineError)] pub enum Error { @@ -26,7 +39,7 @@ pub enum Error { Execution(#[from] crate::ExecutionError), #[error(transparent)] - Parse(#[from] ParseError), + Parse(#[from] ZpoolInfoParseError), #[error("No Zpools found")] NoZpools, @@ -161,15 +174,13 @@ impl ZpoolInfo { } impl FromStr for ZpoolInfo { - type Err = ParseError; + type Err = ZpoolInfoParseError; fn from_str(s: &str) -> Result { // Lambda helpers for error handling. - let expected_field = |name| { - ParseError(format!("Missing '{}' value in zpool list output", name)) - }; - let failed_to_parse = |name, err| { - ParseError(format!("Failed to parse field '{}': {}", name, err)) + let expected_field = |name| ZpoolInfoParseError::MissingField(name); + let failed_to_parse = |field, err| { + ZpoolInfoParseError::IntegerField { field, err } }; let mut values = s.trim().split_whitespace(); diff --git a/sled-agent/types/versions/src/impls/inventory.rs b/sled-agent/types/versions/src/impls/inventory.rs index 6c62db26f60..394246cef87 100644 --- a/sled-agent/types/versions/src/impls/inventory.rs +++ b/sled-agent/types/versions/src/impls/inventory.rs @@ -950,12 +950,12 @@ impl fmt::Display for SingleMeasurementInventoryDisplay<'_> { } } -#[derive(Debug, thiserror::Error)] +#[derive(Debug, thiserror::Error, PartialEq, Eq)] #[error("unrecognized zpool health value `{0}`")] -pub struct ZpoolHeathParseError(pub String); +pub struct ZpoolHealthParseError(pub String); impl FromStr for ZpoolHealth { - type Err = ZpoolHeathParseError; + type Err = ZpoolHealthParseError; fn from_str(s: &str) -> Result { match s { @@ -965,7 +965,7 @@ impl FromStr for ZpoolHealth { "OFFLINE" => Ok(ZpoolHealth::Offline), "REMOVED" => Ok(ZpoolHealth::Removed), "UNAVAIL" => Ok(ZpoolHealth::Unavailable), - _ => Err(ZpoolHeathParseError(s.to_owned())), + _ => Err(ZpoolHealthParseError(s.to_owned())), } } } diff --git a/sled-agent/types/versions/src/latest.rs b/sled-agent/types/versions/src/latest.rs index d8bd6899253..c172473e20f 100644 --- a/sled-agent/types/versions/src/latest.rs +++ b/sled-agent/types/versions/src/latest.rs @@ -172,6 +172,7 @@ pub mod inventory { pub use crate::impls::inventory::MupdateOverrideNonBootInventoryDisplay; pub use crate::impls::inventory::OmicronFileSourceResolverInventoryDisplay; pub use crate::impls::inventory::ZoneArtifactInventoryDisplay; + pub use crate::impls::inventory::ZpoolHealthParseError; } pub mod probes { From 3155412ce5a28a99b90b3d773a16079442843308 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 Mar 2026 10:08:25 -0400 Subject: [PATCH 3/7] move implementations - non-test code now compiles --- illumos-utils/src/opte/mod.rs | 44 ++++++++++ illumos-utils/src/running_zone.rs | 22 +---- illumos-utils/src/svcs.rs | 83 +++---------------- illumos-utils/src/zfs.rs | 15 ++++ illumos-utils/src/zone.rs | 30 +++++++ illumos-utils/src/zpool.rs | 53 ------------ nexus/db-model/src/inventory.rs | 2 +- nexus/inventory/src/examples.rs | 3 +- nexus/reconfigurator/planning/src/system.rs | 2 +- nexus/test-utils/src/resource_helpers.rs | 2 +- nexus/types/src/inventory.rs | 2 +- .../src/debug_collector/handle.rs | 2 +- .../src/reconciler_task/datasets.rs | 1 + .../src/reconciler_task/external_disks.rs | 2 +- .../src/reconciler_task/zones.rs | 1 + .../health-monitor/src/health_checks.rs | 26 +++++- sled-agent/src/bin/sled-agent-sim.rs | 2 +- sled-agent/src/sim/config.rs | 2 +- sled-agent/src/sim/sled_agent.rs | 3 +- sled-agent/src/sim/storage.rs | 2 +- .../add_attached_subnets/attached_subnet.rs | 36 -------- .../types/versions/src/impls/inventory.rs | 43 ---------- .../types/versions/src/initial/inventory.rs | 16 ---- 23 files changed, 139 insertions(+), 255 deletions(-) diff --git a/illumos-utils/src/opte/mod.rs b/illumos-utils/src/opte/mod.rs index 697b720ffb6..e9e2546cb0a 100644 --- a/illumos-utils/src/opte/mod.rs +++ b/illumos-utils/src/opte/mod.rs @@ -171,6 +171,22 @@ pub struct AttachedSubnet { pub kind: AttachedSubnetKind, } +impl From + for AttachedSubnet +{ + fn from(value: sled_agent_types::attached_subnet::AttachedSubnet) -> Self { + Self { cidr: net_to_cidr(value.subnet), kind: value.kind.into() } + } +} + +impl From + for sled_agent_types::attached_subnet::AttachedSubnet +{ + fn from(value: AttachedSubnet) -> Self { + Self { subnet: cidr_to_net(value.cidr), kind: value.kind.into() } + } +} + /// The kind of subnet that is attached. #[derive(Clone, Copy, Debug)] pub enum AttachedSubnetKind { @@ -180,6 +196,34 @@ pub enum AttachedSubnetKind { External, } +impl From + for AttachedSubnetKind +{ + fn from( + value: sled_agent_types::attached_subnet::AttachedSubnetKind, + ) -> Self { + match value { + sled_agent_types::attached_subnet::AttachedSubnetKind::Vpc => { + Self::Vpc + } + sled_agent_types::attached_subnet::AttachedSubnetKind::External => { + Self::External + } + } + } +} + +impl From + for sled_agent_types::attached_subnet::AttachedSubnetKind +{ + fn from(value: AttachedSubnetKind) -> Self { + match value { + AttachedSubnetKind::Vpc => Self::Vpc, + AttachedSubnetKind::External => Self::External, + } + } +} + /// A set of removed / added attached subnets in an OPTE API call. /// /// This is used to ensure we keep our in-memory state in sync with whatever we diff --git a/illumos-utils/src/running_zone.rs b/illumos-utils/src/running_zone.rs index 007e6242943..aad563fd7e1 100644 --- a/illumos-utils/src/running_zone.rs +++ b/illumos-utils/src/running_zone.rs @@ -720,23 +720,6 @@ impl InstalledZone { /// The path to the zone's root filesystem (i.e., `/`), within zonepath. pub const ROOT_FS_PATH: &'static str = "root"; - /// Returns the name of a zone, based on the base zone name plus any unique - /// identifying info. - /// - /// The zone name is based on: - /// - A unique Oxide prefix ("oxz_") - /// - The name of the zone type being hosted (e.g., "nexus") - /// - An optional, zone-unique UUID - /// - /// This results in a zone name which is distinct across different zpools, - /// but stable and predictable across reboots. - pub fn get_zone_name( - zone_type: &str, - unique_name: Option, - ) -> String { - crate::zone::zone_name(zone_type, unique_name) - } - /// Get the name of the bootstrap VNIC in the zone, if any. pub fn get_bootstrap_vnic_name(&self) -> Option<&str> { self.bootstrap_vnic.as_ref().map(|link| link.name()) @@ -992,7 +975,7 @@ impl<'a> ZoneBuilder<'a> { let temp_dir = fake_cfg.temp_dir; (|| { let zone_type = self.zone_type?; - let full_zone_name = InstalledZone::get_zone_name( + let full_zone_name = crate::zone::zone_name( zone_type, self.unique_name, ); @@ -1058,8 +1041,7 @@ impl<'a> ZoneBuilder<'a> { err, })?; - let full_zone_name = - InstalledZone::get_zone_name(zone_type, unique_name); + let full_zone_name = crate::zone::zone_name(zone_type, unique_name); // Look for the image within `file_source.search_paths`, in order. let zone_image_path = file_source diff --git a/illumos-utils/src/svcs.rs b/illumos-utils/src/svcs.rs index f1efa62d0d3..8bfdd48bdaa 100644 --- a/illumos-utils/src/svcs.rs +++ b/illumos-utils/src/svcs.rs @@ -17,10 +17,11 @@ use chrono::Utc; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; -use slog::Logger; -use slog::{error, info}; use sled_agent_types::inventory::Svc; use sled_agent_types::inventory::SvcState; +use sled_agent_types::inventory::SvcsEnabledNotOnline; +use slog::Logger; +use slog::{error, info}; #[cfg(target_os = "illumos")] use tokio::process::Command; @@ -62,6 +63,13 @@ pub struct SvcsResult { pub time_of_status: DateTime, } +impl From for SvcsEnabledNotOnline { + fn from(value: SvcsResult) -> Self { + let SvcsResult { services, errors, time_of_status } = value; + Self { services, errors, time_of_status } + } +} + impl SvcsResult { pub fn new() -> Self { Self { services: vec![], errors: vec![], time_of_status: Utc::now() } @@ -166,77 +174,6 @@ impl SvcsResult { } } -/* -/// Each service instance is always in a well-defined state based on its -/// dependencies, the results of the execution of its methods, and its potential -/// contracts events. See for more information. -#[derive( - Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema, -)] -#[serde(rename_all = "snake_case")] -pub enum SvcState { - /// Initial state for all service instances. - Uninitialized, - /// The instance is enabled, but not yet running or available to run. - Offline, - /// The instance is enabled and running or is available to run. - Online, - /// The instance is enabled and running or available to run. It is, however, - /// functioning at a limited capacity in comparison to normal operation. - Degraded, - /// The instance is enabled, but not able to run. - Maintenance, - /// The instance is disabled. - Disabled, - /// Represents a legacy instance that is not managed by the service - /// management facility. - LegacyRun, - /// We were unable to determine the state of the service instance. - Unknown, -} - -impl From for SvcState { - fn from(value: String) -> Self { - match value.as_str() { - "uninitialized" => SvcState::Uninitialized, - "offline" => SvcState::Offline, - "online" => SvcState::Online, - "degraded" => SvcState::Degraded, - "maintenance" => SvcState::Maintenance, - "disabled" => SvcState::Disabled, - "legacy_run" => SvcState::LegacyRun, - _ => SvcState::Unknown, - } - } -} - -impl Display for SvcState { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let state = match self { - SvcState::Uninitialized => "uninitialized", - SvcState::Offline => "offline", - SvcState::Online => "online", - SvcState::Degraded => "degraded", - SvcState::Maintenance => "maintenance", - SvcState::Disabled => "disabled", - SvcState::LegacyRun => "legacy_run", - SvcState::Unknown => "unknown", - }; - - write!(f, "{state}") - } -} - -#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "snake_case")] -/// Information about an SMF service that is enabled but not running -pub struct Svc { - fmri: String, - zone: String, - state: SvcState, -} -*/ - #[cfg(test)] mod tests { use super::*; diff --git a/illumos-utils/src/zfs.rs b/illumos-utils/src/zfs.rs index 79a66701447..becf8b2d7d3 100644 --- a/illumos-utils/src/zfs.rs +++ b/illumos-utils/src/zfs.rs @@ -20,6 +20,7 @@ use omicron_common::disk::DiskIdentity; use omicron_common::disk::SharedDatasetConfig; use omicron_uuid_kinds::DatasetUuid; use rustix::fd::AsRawFd; +use sled_agent_types::inventory::InventoryDataset; use slog_error_chain::SlogInlineError; use std::collections::BTreeMap; use std::fmt; @@ -592,6 +593,20 @@ impl TryFrom<&DatasetProperties> for SharedDatasetConfig { } } +impl From for InventoryDataset { + fn from(props: DatasetProperties) -> Self { + Self { + id: props.id, + name: props.name, + available: props.avail, + used: props.used, + quota: props.quota, + reservation: props.reservation, + compression: props.compression, + } + } +} + impl DatasetProperties { /// Parses dataset properties, assuming that the caller is providing the /// output of the following command as stdout: diff --git a/illumos-utils/src/zone.rs b/illumos-utils/src/zone.rs index 735750bd202..6720b8bbe17 100644 --- a/illumos-utils/src/zone.rs +++ b/illumos-utils/src/zone.rs @@ -8,6 +8,7 @@ use anyhow::anyhow; use camino::Utf8Path; use ipnetwork::IpNetwork; use ipnetwork::IpNetworkError; +use sled_agent_types::inventory::OmicronZoneConfig; use slog::Logger; use slog::info; use std::net::{IpAddr, Ipv6Addr}; @@ -33,6 +34,16 @@ pub const ROUTE: &str = "/usr/sbin/route"; pub const ZONE_PREFIX: &str = "oxz_"; pub const PROPOLIS_ZONE_PREFIX: &str = "oxz_propolis-server_"; +/// Returns the name of a zone, based on the base zone name plus any unique +/// identifying info. +/// +/// The zone name is based on: +/// - A unique Oxide prefix ("oxz_") +/// - The name of the zone type being hosted (e.g., "nexus") +/// - An optional, zone-unique UUID +/// +/// This results in a zone name which is distinct across different zpools, +/// but stable and predictable across reboots. pub fn zone_name(prefix: &str, id: Option) -> String { if let Some(id) = id { format!("{ZONE_PREFIX}{}_{}", prefix, id) @@ -41,6 +52,25 @@ pub fn zone_name(prefix: &str, id: Option) -> String { } } +pub trait OmicronZoneConfigExt { + /// Returns the name of a zone, based on the zone type plus its unique ID. + /// + /// The zone name is based on: + /// - A unique Oxide prefix ("oxz_") + /// - The zone type being hosted (e.g., "nexus") + /// - The zone's ID. + /// + /// This results in a zone name which is distinct across different zpools, + /// but stable and predictable across reboots. + fn zone_name(&self) -> String; +} + +impl OmicronZoneConfigExt for OmicronZoneConfig { + fn zone_name(&self) -> String { + zone_name(self.zone_type.kind().zone_prefix(), Some(self.id)) + } +} + #[derive(thiserror::Error, Debug)] enum Error { #[error("Zone execution error")] diff --git a/illumos-utils/src/zpool.rs b/illumos-utils/src/zpool.rs index 76cfb352d90..f102d30e779 100644 --- a/illumos-utils/src/zpool.rs +++ b/illumos-utils/src/zpool.rs @@ -74,59 +74,6 @@ pub struct GetInfoError { err: Error, } -/* -#[derive( - Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema, -)] -#[serde(rename_all = "snake_case")] -pub enum ZpoolHealth { - /// The device is online and functioning. - Online, - /// One or more components are degraded or faulted, but sufficient - /// replicas exist to continue functioning. - Degraded, - /// One or more components are degraded or faulted, and insufficient - /// replicas exist to continue functioning. - Faulted, - /// The device was explicitly taken offline by "zpool offline". - Offline, - /// The device was physically removed. - Removed, - /// The device could not be opened. - Unavailable, -} - -impl FromStr for ZpoolHealth { - type Err = ParseError; - - fn from_str(s: &str) -> Result { - match s { - "ONLINE" => Ok(ZpoolHealth::Online), - "DEGRADED" => Ok(ZpoolHealth::Degraded), - "FAULTED" => Ok(ZpoolHealth::Faulted), - "OFFLINE" => Ok(ZpoolHealth::Offline), - "REMOVED" => Ok(ZpoolHealth::Removed), - "UNAVAIL" => Ok(ZpoolHealth::Unavailable), - _ => Err(ParseError(format!("Unrecognized zpool 'health': {}", s))), - } - } -} - -impl Display for ZpoolHealth { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let s = match self { - ZpoolHealth::Online => "online", - ZpoolHealth::Degraded => "degraded", - ZpoolHealth::Faulted => "faulted", - ZpoolHealth::Offline => "offline", - ZpoolHealth::Removed => "removed", - ZpoolHealth::Unavailable => "unavailable", - }; - write!(f, "{s}") - } -} -*/ - /// Describes a Zpool. #[derive(Clone, Debug, PartialEq, Eq)] pub struct ZpoolInfo { diff --git a/nexus/db-model/src/inventory.rs b/nexus/db-model/src/inventory.rs index 7863f000081..0f7e5c74bd6 100644 --- a/nexus/db-model/src/inventory.rs +++ b/nexus/db-model/src/inventory.rs @@ -27,7 +27,6 @@ use diesel::pg::Pg; use diesel::serialize::ToSql; use diesel::{serialize, sql_types}; use iddqd::IdOrdMap; -use illumos_utils::zpool::ZpoolHealth; use ipnetwork::IpNetwork; use nexus_db_schema::schema::inv_zone_manifest_non_boot; use nexus_db_schema::schema::inv_zone_manifest_zone; @@ -94,6 +93,7 @@ use sled_agent_types::inventory::RemoveMupdateOverrideBootSuccessInventory; use sled_agent_types::inventory::RemoveMupdateOverrideInventory; use sled_agent_types::inventory::SingleMeasurementInventory; use sled_agent_types::inventory::ZoneArtifactInventory; +use sled_agent_types::inventory::ZpoolHealth; use sled_agent_types::inventory::{ ConfigReconcilerInventoryResult, OmicronSledConfig, OmicronZoneConfig, OmicronZoneDataset, OmicronZoneImageSource, OmicronZoneType, diff --git a/nexus/inventory/src/examples.rs b/nexus/inventory/src/examples.rs index 1b34b420c02..22196313b72 100644 --- a/nexus/inventory/src/examples.rs +++ b/nexus/inventory/src/examples.rs @@ -64,6 +64,7 @@ use sled_agent_types::inventory::SingleMeasurementInventory; use sled_agent_types::inventory::SledCpuFamily; use sled_agent_types::inventory::SledRole; use sled_agent_types::inventory::SvcsEnabledNotOnlineResult; +use sled_agent_types::inventory::ZpoolHealth; use sled_agent_types::resolvable_files::MeasurementManifestStatus; use sled_agent_types::resolvable_files::MupdateOverrideNonBootInfo; use sled_agent_types::resolvable_files::MupdateOverrideNonBootMismatch; @@ -532,7 +533,7 @@ pub fn representative() -> Representative { zpools.push(InventoryZpool { id: pool_id, total_size: ByteCount::from(4096), - health: illumos_utils::zpool::ZpoolHealth::Online, + health: ZpoolHealth::Online, }); } let dataset_name = DatasetName::new( diff --git a/nexus/reconfigurator/planning/src/system.rs b/nexus/reconfigurator/planning/src/system.rs index c670874aa22..163a87e80e1 100644 --- a/nexus/reconfigurator/planning/src/system.rs +++ b/nexus/reconfigurator/planning/src/system.rs @@ -12,7 +12,6 @@ use clickhouse_admin_types::keeper::ClickhouseKeeperClusterMembership; use gateway_client::types::RotState; use gateway_client::types::SpComponentCaboose; use gateway_client::types::SpState; -use illumos_utils::zpool::ZpoolHealth; use indexmap::IndexMap; use ipnet::Ipv6Net; use ipnet::Ipv6Subnets; @@ -74,6 +73,7 @@ use sled_agent_types::inventory::SledCpuFamily; use sled_agent_types::inventory::SledRole; use sled_agent_types::inventory::SvcsEnabledNotOnlineResult; use sled_agent_types::inventory::ZoneKind; +use sled_agent_types::inventory::ZpoolHealth; use sled_hardware_types::BaseboardId; use sled_hardware_types::GIMLET_SLED_MODEL; use std::collections::BTreeMap; diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 39eeeaf9765..bd1d715b356 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -14,7 +14,6 @@ use dropshot::Method; use dropshot::test_util::ClientTestContext; use http::StatusCode; use http::header; -use illumos_utils::zpool::ZpoolHealth; use nexus_db_queries::db::fixed_data::silo::DEFAULT_SILO; use nexus_test_interface::NexusServer; use nexus_types::deployment::Blueprint; @@ -93,6 +92,7 @@ use omicron_uuid_kinds::ZpoolUuid; use oxnet::IpNet; use oxnet::Ipv4Net; use oxnet::Ipv6Net; +use sled_agent_types::inventory::ZpoolHealth; use slog::debug; use std::collections::BTreeMap; use std::net::IpAddr; diff --git a/nexus/types/src/inventory.rs b/nexus/types/src/inventory.rs index 3a26e18633f..93acd13fc01 100644 --- a/nexus/types/src/inventory.rs +++ b/nexus/types/src/inventory.rs @@ -21,7 +21,6 @@ pub use gateway_types::rot::RotSlot; use iddqd::IdOrdItem; use iddqd::IdOrdMap; use iddqd::id_upcast; -use illumos_utils::zpool::ZpoolHealth; use omicron_common::api::external::ByteCount; pub use omicron_common::api::internal::shared::NetworkInterface; pub use omicron_common::api::internal::shared::NetworkInterfaceKind; @@ -49,6 +48,7 @@ use sled_agent_types_versions::latest::inventory::SingleMeasurementInventory; use sled_agent_types_versions::latest::inventory::SledCpuFamily; use sled_agent_types_versions::latest::inventory::SledRole; use sled_agent_types_versions::latest::inventory::SvcsEnabledNotOnlineResult; +use sled_agent_types_versions::latest::inventory::ZpoolHealth; use sled_hardware_types::BaseboardId; use std::collections::BTreeMap; use std::collections::BTreeSet; diff --git a/sled-agent/config-reconciler/src/debug_collector/handle.rs b/sled-agent/config-reconciler/src/debug_collector/handle.rs index 79721df0fd3..1d1f8296a22 100644 --- a/sled-agent/config-reconciler/src/debug_collector/handle.rs +++ b/sled-agent/config-reconciler/src/debug_collector/handle.rs @@ -11,8 +11,8 @@ use super::worker::DebugCollectorWorker; use super::worker::DebugZpool; use super::worker::DumpSlicePath; use camino::Utf8Path; -use illumos_utils::zpool::ZpoolHealth; use omicron_common::disk::DiskVariant; +use sled_agent_types::inventory::ZpoolHealth; use sled_storage::config::MountConfig; use sled_storage::disk::Disk; use slog::Logger; diff --git a/sled-agent/config-reconciler/src/reconciler_task/datasets.rs b/sled-agent/config-reconciler/src/reconciler_task/datasets.rs index 223948afcb6..a2f2cf02530 100644 --- a/sled-agent/config-reconciler/src/reconciler_task/datasets.rs +++ b/sled-agent/config-reconciler/src/reconciler_task/datasets.rs @@ -21,6 +21,7 @@ use crate::dataset_serialization_task::RekeyResult; use iddqd::IdOrdItem; use iddqd::IdOrdMap; use iddqd::id_upcast; +use illumos_utils::zone::OmicronZoneConfigExt; use illumos_utils::zpool::PathInPool; use illumos_utils::zpool::ZpoolOrRamdisk; use omicron_common::disk::DatasetConfig; diff --git a/sled-agent/config-reconciler/src/reconciler_task/external_disks.rs b/sled-agent/config-reconciler/src/reconciler_task/external_disks.rs index ff0fe14f8f9..bda94789f73 100644 --- a/sled-agent/config-reconciler/src/reconciler_task/external_disks.rs +++ b/sled-agent/config-reconciler/src/reconciler_task/external_disks.rs @@ -15,7 +15,6 @@ use iddqd::id_ord_map::Entry; use iddqd::id_upcast; use illumos_utils::zfs::Zfs; use illumos_utils::zpool::Zpool; -use illumos_utils::zpool::ZpoolHealth; use illumos_utils::zpool::ZpoolName; use key_manager::StorageKeyRequester; use omicron_common::api::external::ByteCount; @@ -26,6 +25,7 @@ use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::ZpoolUuid; use rand::distr::{Alphanumeric, SampleString}; use sled_agent_types::inventory::ConfigReconcilerInventoryResult; +use sled_agent_types::inventory::ZpoolHealth; use sled_storage::config::MountConfig; use sled_storage::dataset::CRYPT_DATASET; use sled_storage::dataset::DatasetError; diff --git a/sled-agent/config-reconciler/src/reconciler_task/zones.rs b/sled-agent/config-reconciler/src/reconciler_task/zones.rs index 7b28ae9ed49..c97387362ec 100644 --- a/sled-agent/config-reconciler/src/reconciler_task/zones.rs +++ b/sled-agent/config-reconciler/src/reconciler_task/zones.rs @@ -24,6 +24,7 @@ use illumos_utils::running_zone::RunningZone; use illumos_utils::zone::AdmError; use illumos_utils::zone::Api as _; use illumos_utils::zone::DeleteAddressError; +use illumos_utils::zone::OmicronZoneConfigExt; use illumos_utils::zone::Zones; use ntp_admin_client::types::TimeSync; use omicron_common::address::Ipv6Subnet; diff --git a/sled-agent/health-monitor/src/health_checks.rs b/sled-agent/health-monitor/src/health_checks.rs index c6f158ce8bc..c62dd1a765b 100644 --- a/sled-agent/health-monitor/src/health_checks.rs +++ b/sled-agent/health-monitor/src/health_checks.rs @@ -6,6 +6,7 @@ use illumos_utils::svcs::Svcs; use sled_agent_types::inventory::SvcsEnabledNotOnlineResult; +use sled_agent_types::inventory::SvcsError; use slog::Logger; use tokio::sync::watch; use tokio::time::Duration; @@ -36,8 +37,9 @@ pub(crate) async fn poll_smf_services_enabled_not_online( // `send_if_modified()`. Err(e) => { smf_services_enabled_not_online_tx.send_modify(|status| { - *status = - SvcsEnabledNotOnlineResult::SvcsCmdError(e.into()); + *status = SvcsEnabledNotOnlineResult::SvcsCmdError( + execution_err_to_svcs_error(e), + ) }) } Ok(svcs) => { @@ -50,3 +52,23 @@ pub(crate) async fn poll_smf_services_enabled_not_online( }; } } + +fn execution_err_to_svcs_error( + err: illumos_utils::ExecutionError, +) -> SvcsError { + match err { + illumos_utils::ExecutionError::ExecutionStart { command, err } => { + SvcsError::ExecutionStart { command, err: err.to_string() } + } + illumos_utils::ExecutionError::CommandFailure(e) => { + SvcsError::CommandFailure(e.to_string()) + } + illumos_utils::ExecutionError::ContractFailure { msg, err } => { + SvcsError::ContractFailure { msg, err: err.to_string() } + } + illumos_utils::ExecutionError::ParseFailure(e) => { + SvcsError::ParseFailure(e) + } + illumos_utils::ExecutionError::NotRunning => SvcsError::NotRunning, + } +} diff --git a/sled-agent/src/bin/sled-agent-sim.rs b/sled-agent/src/bin/sled-agent-sim.rs index ace54c9b18a..e01b4414b16 100644 --- a/sled-agent/src/bin/sled-agent-sim.rs +++ b/sled-agent/src/bin/sled-agent-sim.rs @@ -12,7 +12,6 @@ use clap::Parser; use dropshot::ConfigDropshot; use dropshot::ConfigLogging; use dropshot::ConfigLoggingLevel; -use illumos_utils::zpool::ZpoolHealth; use omicron_common::api::internal::nexus::Certificate; use omicron_common::cmd::CmdError; use omicron_common::cmd::fatal; @@ -22,6 +21,7 @@ use omicron_sled_agent::sim::{ run_standalone_server, }; use omicron_uuid_kinds::SledUuid; +use sled_agent_types::inventory::ZpoolHealth; use sled_hardware_types::{Baseboard, SledCpuFamily}; use std::net::SocketAddr; use std::net::SocketAddrV6; diff --git a/sled-agent/src/sim/config.rs b/sled-agent/src/sim/config.rs index e5864403434..cca28fe7c8f 100644 --- a/sled-agent/src/sim/config.rs +++ b/sled-agent/src/sim/config.rs @@ -7,10 +7,10 @@ use crate::updates::ConfigUpdates; use camino::Utf8Path; use dropshot::ConfigDropshot; -use illumos_utils::zpool::ZpoolHealth; use omicron_uuid_kinds::SledUuid; use serde::Deserialize; use serde::Serialize; +use sled_agent_types::inventory::ZpoolHealth; pub use sled_hardware_types::{Baseboard, SledCpuFamily}; use sp_sim::FAKE_GIMLET_MODEL; use std::net::Ipv6Addr; diff --git a/sled-agent/src/sim/sled_agent.rs b/sled-agent/src/sim/sled_agent.rs index 0b4a66fe2bb..6ad61297ff5 100644 --- a/sled-agent/src/sim/sled_agent.rs +++ b/sled-agent/src/sim/sled_agent.rs @@ -25,7 +25,6 @@ use dropshot::Body; use dropshot::HttpError; use futures::Stream; use iddqd::IdOrdMap; -use illumos_utils::zpool::ZpoolHealth; use omicron_common::api::external::{ ByteCount, Error, Generation, ResourceType, }; @@ -68,7 +67,7 @@ use sled_agent_types::inventory::{ ConfigReconcilerInventoryStatus, HostPhase2DesiredSlots, Inventory, InventoryDataset, InventoryDisk, InventoryZpool, OmicronFileSourceResolverInventory, OmicronSledConfig, OmicronZonesConfig, - SingleMeasurementInventory, SledRole, + SingleMeasurementInventory, SledRole, ZpoolHealth, }; use sled_agent_types::support_bundle::SupportBundleMetadata; diff --git a/sled-agent/src/sim/storage.rs b/sled-agent/src/sim/storage.rs index a287868c2c3..c4398cda195 100644 --- a/sled-agent/src/sim/storage.rs +++ b/sled-agent/src/sim/storage.rs @@ -23,7 +23,6 @@ use crucible_agent_client::types::{ use dropshot::HandlerTaskMode; use dropshot::HttpError; use illumos_utils::zfs::DatasetProperties; -use illumos_utils::zpool::ZpoolHealth; use omicron_common::api::external::ByteCount; use omicron_common::disk::DatasetManagementStatus; use omicron_common::disk::DatasetName; @@ -44,6 +43,7 @@ use omicron_uuid_kinds::ZpoolUuid; use propolis_client::VolumeConstructionRequest; use serde::Serialize; use sled_agent_types::dataset::LocalStorageDatasetEnsureRequest; +use sled_agent_types::inventory::ZpoolHealth; use sled_agent_types::support_bundle::NESTED_DATASET_NOT_FOUND; use sled_storage::nested_dataset::NestedDatasetConfig; use sled_storage::nested_dataset::NestedDatasetListOptions; diff --git a/sled-agent/types/versions/src/add_attached_subnets/attached_subnet.rs b/sled-agent/types/versions/src/add_attached_subnets/attached_subnet.rs index dc9124a851a..6b10edd2694 100644 --- a/sled-agent/types/versions/src/add_attached_subnets/attached_subnet.rs +++ b/sled-agent/types/versions/src/add_attached_subnets/attached_subnet.rs @@ -3,8 +3,6 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use iddqd::IdOrdMap; -//use illumos_utils::opte::cidr_to_net; -//use illumos_utils::opte::net_to_cidr; use omicron_uuid_kinds::PropolisUuid; use oxnet::IpNet; use schemars::JsonSchema; @@ -36,20 +34,6 @@ impl iddqd::IdOrdItem for AttachedSubnet { iddqd::id_upcast!(); } -/* -impl From for illumos_utils::opte::AttachedSubnet { - fn from(value: AttachedSubnet) -> Self { - Self { cidr: net_to_cidr(value.subnet), kind: value.kind.into() } - } -} - -impl From for AttachedSubnet { - fn from(value: illumos_utils::opte::AttachedSubnet) -> Self { - Self { subnet: cidr_to_net(value.cidr), kind: value.kind.into() } - } -} -*/ - /// The kind of attached subnet. #[derive(Clone, Copy, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "snake_case")] @@ -60,26 +44,6 @@ pub enum AttachedSubnetKind { External, } -/* -impl From for illumos_utils::opte::AttachedSubnetKind { - fn from(value: AttachedSubnetKind) -> Self { - match value { - AttachedSubnetKind::Vpc => Self::Vpc, - AttachedSubnetKind::External => Self::External, - } - } -} - -impl From for AttachedSubnetKind { - fn from(value: illumos_utils::opte::AttachedSubnetKind) -> Self { - match value { - illumos_utils::opte::AttachedSubnetKind::Vpc => Self::Vpc, - illumos_utils::opte::AttachedSubnetKind::External => Self::External, - } - } -} -*/ - /// Path parameters for referring to a single subnet attached to an instance. #[derive(Clone, Copy, Debug, Deserialize, JsonSchema, Serialize)] pub struct VmmSubnetPathParam { diff --git a/sled-agent/types/versions/src/impls/inventory.rs b/sled-agent/types/versions/src/impls/inventory.rs index 394246cef87..a3480294b08 100644 --- a/sled-agent/types/versions/src/impls/inventory.rs +++ b/sled-agent/types/versions/src/impls/inventory.rs @@ -361,16 +361,6 @@ impl OmicronZoneConfig { self.zone_type.underlay_ip() } - /* - /// Returns the zone name for this zone configuration. - pub fn zone_name(&self) -> String { - illumos_utils::running_zone::InstalledZone::get_zone_name( - self.zone_type.kind().zone_prefix(), - Some(self.id), - ) - } - */ - /// If this kind of zone has an associated dataset, return the dataset's /// name. Otherwise, return `None`. pub fn dataset_name(&self) -> Option { @@ -876,39 +866,6 @@ impl HostPhase2DesiredSlots { } } -/* -impl From for SvcsEnabledNotOnline { - fn from(value: illumos_utils::svcs::SvcsResult) -> Self { - let illumos_utils::svcs::SvcsResult { - services, - errors, - time_of_status, - } = value; - Self { services, errors, time_of_status } - } -} - -impl From for SvcsError { - fn from(e: illumos_utils::ExecutionError) -> Self { - match e { - illumos_utils::ExecutionError::ExecutionStart { command, err } => { - Self::ExecutionStart { command, err: err.to_string() } - } - illumos_utils::ExecutionError::CommandFailure(e) => { - Self::CommandFailure(e.to_string()) - } - illumos_utils::ExecutionError::ContractFailure { msg, err } => { - Self::ContractFailure { msg, err: err.to_string() } - } - illumos_utils::ExecutionError::ParseFailure(e) => { - Self::ParseFailure(e) - } - illumos_utils::ExecutionError::NotRunning => Self::NotRunning, - } - } -} -*/ - impl Default for OmicronSledConfig { fn default() -> Self { Self { diff --git a/sled-agent/types/versions/src/initial/inventory.rs b/sled-agent/types/versions/src/initial/inventory.rs index c71060cfb01..7d21a160720 100644 --- a/sled-agent/types/versions/src/initial/inventory.rs +++ b/sled-agent/types/versions/src/initial/inventory.rs @@ -89,22 +89,6 @@ pub struct InventoryDataset { pub compression: String, } -/* -impl From for InventoryDataset { - fn from(props: illumos_utils::zfs::DatasetProperties) -> Self { - Self { - id: props.id, - name: props.name, - available: props.avail, - used: props.used, - quota: props.quota, - reservation: props.reservation, - compression: props.compression, - } - } -} -*/ - /// Describes the role of the sled within the rack. /// /// Note that this may change if the sled is physically moved From 8545ce5ef8f8d2f810609332cd2a17e0d195b1f4 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 Mar 2026 10:15:11 -0400 Subject: [PATCH 4/7] fix up tests --- illumos-utils/src/zpool.rs | 6 ++---- nexus/db-queries/src/db/datastore/mod.rs | 2 +- nexus/db-queries/src/db/datastore/sled.rs | 2 +- nexus/db-queries/src/db/queries/region_allocation.rs | 2 +- nexus/src/app/background/tasks/support_bundle_collector.rs | 2 +- nexus/tests/integration_tests/unauthorized.rs | 2 +- sled-agent/src/support_bundle/storage.rs | 2 +- 7 files changed, 8 insertions(+), 10 deletions(-) diff --git a/illumos-utils/src/zpool.rs b/illumos-utils/src/zpool.rs index f102d30e779..49b8eb605d3 100644 --- a/illumos-utils/src/zpool.rs +++ b/illumos-utils/src/zpool.rs @@ -345,11 +345,9 @@ mod test { // Similar to the prior test case, just omit "health". let input = format!("{} {} {} {}", name, size, allocated, free); - let result: Result = input.parse(); + let result = input.parse::(); - let expected_err = ParseError( - "Missing 'health' value in zpool list output".to_owned(), - ); + let expected_err = ZpoolInfoParseError::MissingField("health"); assert_eq!(result.unwrap_err(), expected_err,); } } diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index 8350472e638..f93cafecb2f 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -680,7 +680,6 @@ mod test { use chrono::{Duration, Utc}; use futures::StreamExt; use futures::stream; - use illumos_utils::zpool::ZpoolHealth; use nexus_config::RegionAllocationStrategy; use nexus_db_fixed_data::silo::DEFAULT_SILO; use nexus_db_lookup::LookupPath; @@ -705,6 +704,7 @@ mod test { use omicron_uuid_kinds::SledUuid; use omicron_uuid_kinds::VolumeUuid; use omicron_uuid_kinds::ZpoolUuid; + use sled_agent_types::inventory::ZpoolHealth; use std::collections::HashMap; use std::collections::HashSet; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddrV6}; diff --git a/nexus/db-queries/src/db/datastore/sled.rs b/nexus/db-queries/src/db/datastore/sled.rs index 5655d8d6bd8..e260c87312f 100644 --- a/nexus/db-queries/src/db/datastore/sled.rs +++ b/nexus/db-queries/src/db/datastore/sled.rs @@ -1788,7 +1788,6 @@ pub(in crate::db::datastore) mod test { use anyhow::{Context, Result}; use async_bb8_diesel::AsyncConnection; use async_bb8_diesel::AsyncSimpleConnection; - use illumos_utils::zpool::ZpoolHealth; use itertools::Itertools; use nexus_db_lookup::LookupPath; use nexus_db_model::PhysicalDiskKind; @@ -1810,6 +1809,7 @@ pub(in crate::db::datastore) mod test { use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::SledUuid; use predicates::{BoxPredicate, prelude::*}; + use sled_agent_types::inventory::ZpoolHealth; use std::collections::HashMap; use std::net::SocketAddrV6; diff --git a/nexus/db-queries/src/db/queries/region_allocation.rs b/nexus/db-queries/src/db/queries/region_allocation.rs index 9116dfabf68..4765197d7ec 100644 --- a/nexus/db-queries/src/db/queries/region_allocation.rs +++ b/nexus/db-queries/src/db/queries/region_allocation.rs @@ -615,7 +615,6 @@ mod test { use crate::db::raw_query_builder::expectorate_query_contents; use async_bb8_diesel::AsyncRunQueryDsl; use chrono::Utc; - use illumos_utils::zpool::ZpoolHealth; use omicron_test_utils::dev; use omicron_uuid_kinds::BlueprintUuid; use omicron_uuid_kinds::CollectionUuid; @@ -623,6 +622,7 @@ mod test { use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::SledUuid; use omicron_uuid_kinds::ZpoolUuid; + use sled_agent_types::inventory::ZpoolHealth; use std::net::SocketAddrV6; use uuid::Uuid; diff --git a/nexus/src/app/background/tasks/support_bundle_collector.rs b/nexus/src/app/background/tasks/support_bundle_collector.rs index eef7d524b3e..17ddfe8511e 100644 --- a/nexus/src/app/background/tasks/support_bundle_collector.rs +++ b/nexus/src/app/background/tasks/support_bundle_collector.rs @@ -443,7 +443,6 @@ mod test { use crate::app::background::tasks::support_bundle::perfetto; use crate::app::support_bundles::SupportBundleQueryType; use http_body_util::BodyExt; - use illumos_utils::zpool::ZpoolHealth; use nexus_db_model::PhysicalDisk; use nexus_db_model::PhysicalDiskKind; use nexus_db_model::RendezvousDebugDataset; @@ -468,6 +467,7 @@ mod test { BlueprintUuid, DatasetUuid, EreporterRestartUuid, OmicronZoneUuid, PhysicalDiskUuid, SledUuid, }; + use sled_agent_types::inventory::ZpoolHealth; use std::collections::HashSet; use std::num::NonZeroU64; use uuid::Uuid; diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 6358fb96b59..4dae5811432 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -16,7 +16,6 @@ use headers::authorization::Credentials; use http::StatusCode; use http::method::Method; use httptest::{Expectation, ServerBuilder, matchers::*, responders::*}; -use illumos_utils::zpool::ZpoolHealth; use nexus_db_queries::authn::external::spoof; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; @@ -28,6 +27,7 @@ use nexus_types::external_api::snapshot; use omicron_common::disk::DatasetKind; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::ZpoolUuid; +use sled_agent_types::inventory::ZpoolHealth; use std::sync::LazyLock; type DiskTest<'a> = diff --git a/sled-agent/src/support_bundle/storage.rs b/sled-agent/src/support_bundle/storage.rs index da6244b0be8..cbe90273efb 100644 --- a/sled-agent/src/support_bundle/storage.rs +++ b/sled-agent/src/support_bundle/storage.rs @@ -1110,7 +1110,6 @@ mod tests { use hyper::header::{ ACCEPT_RANGES, CONTENT_LENGTH, CONTENT_RANGE, CONTENT_TYPE, }; - use illumos_utils::zpool::ZpoolHealth; use omicron_common::disk::DatasetConfig; use omicron_common::disk::DatasetKind; use omicron_common::disk::DatasetName; @@ -1119,6 +1118,7 @@ mod tests { use omicron_test_utils::dev::test_setup_log; use omicron_uuid_kinds::PhysicalDiskUuid; use sha2::Sha256; + use sled_agent_types::inventory::ZpoolHealth; use std::collections::BTreeMap; use uuid::Uuid; use zip::ZipWriter; From 48cca8e0e95016976c068bb798afcb6ec97075f1 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 Mar 2026 10:15:25 -0400 Subject: [PATCH 5/7] cargo fmt --- illumos-utils/src/zpool.rs | 5 ++--- sled-agent/types/versions/src/impls/inventory.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/illumos-utils/src/zpool.rs b/illumos-utils/src/zpool.rs index 49b8eb605d3..9614b5cd92a 100644 --- a/illumos-utils/src/zpool.rs +++ b/illumos-utils/src/zpool.rs @@ -126,9 +126,8 @@ impl FromStr for ZpoolInfo { fn from_str(s: &str) -> Result { // Lambda helpers for error handling. let expected_field = |name| ZpoolInfoParseError::MissingField(name); - let failed_to_parse = |field, err| { - ZpoolInfoParseError::IntegerField { field, err } - }; + let failed_to_parse = + |field, err| ZpoolInfoParseError::IntegerField { field, err }; let mut values = s.trim().split_whitespace(); let name = diff --git a/sled-agent/types/versions/src/impls/inventory.rs b/sled-agent/types/versions/src/impls/inventory.rs index a3480294b08..d20aab96487 100644 --- a/sled-agent/types/versions/src/impls/inventory.rs +++ b/sled-agent/types/versions/src/impls/inventory.rs @@ -27,7 +27,7 @@ use crate::latest::inventory::{ OmicronFileSourceResolverInventory, OmicronSledConfig, OmicronZoneConfig, OmicronZoneImageSource, OmicronZoneType, OmicronZonesConfig, RemoveMupdateOverrideBootSuccessInventory, RemoveMupdateOverrideInventory, - SingleMeasurementInventory, SvcsEnabledNotOnline, SvcState, + SingleMeasurementInventory, SvcState, SvcsEnabledNotOnline, ZoneArtifactInventory, ZoneKind, ZpoolHealth, }; From ca6f8fdde8ab211a14b2581bc15c2a6f2fc10578 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 Mar 2026 10:26:50 -0400 Subject: [PATCH 6/7] minor cleanup --- illumos-utils/src/zpool.rs | 2 +- nexus/db-model/src/inventory.rs | 2 +- sled-agent/types/versions/src/impls/inventory.rs | 3 +++ .../versions/src/modify_services_in_inventory/inventory.rs | 2 ++ 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/illumos-utils/src/zpool.rs b/illumos-utils/src/zpool.rs index 9614b5cd92a..2766beb1784 100644 --- a/illumos-utils/src/zpool.rs +++ b/illumos-utils/src/zpool.rs @@ -23,7 +23,7 @@ pub const ZPOOL_MOUNTPOINT_ROOT: &str = "/"; pub enum ZpoolInfoParseError { #[error(transparent)] ZpoolHealth(#[from] ZpoolHealthParseError), - #[error("zpool list output: missing field `{0}`)")] + #[error("zpool list output: missing field `{0}`")] MissingField(&'static str), #[error("zpool list output: failed to parse field `{field}`")] IntegerField { diff --git a/nexus/db-model/src/inventory.rs b/nexus/db-model/src/inventory.rs index 0f7e5c74bd6..ecbf7e6f31c 100644 --- a/nexus/db-model/src/inventory.rs +++ b/nexus/db-model/src/inventory.rs @@ -2259,7 +2259,7 @@ impl From } } -// See [`illumos_utils::zpool::ZpoolHealth`]. +// See [`sled_agent_types::inventory::ZpoolHealth`]. impl_enum_type!( InvZpoolHealthEnum: diff --git a/sled-agent/types/versions/src/impls/inventory.rs b/sled-agent/types/versions/src/impls/inventory.rs index d20aab96487..bfce6e6a5e6 100644 --- a/sled-agent/types/versions/src/impls/inventory.rs +++ b/sled-agent/types/versions/src/impls/inventory.rs @@ -911,6 +911,9 @@ impl fmt::Display for SingleMeasurementInventoryDisplay<'_> { #[error("unrecognized zpool health value `{0}`")] pub struct ZpoolHealthParseError(pub String); +// TODO-correctness `ZpoolHealth` implements both `FromStr` and `Display`, but +// they aren't symmetric - we should replace one of these (probably `FromStr`?) +// with an explicitly-named method. impl FromStr for ZpoolHealth { type Err = ZpoolHealthParseError; diff --git a/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs b/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs index dc6664bb7a4..18f484c75bf 100644 --- a/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs +++ b/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs @@ -54,6 +54,8 @@ pub enum SvcState { #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "snake_case")] /// Information about an SMF service that is enabled but not running +// TODO-correctness `SvcState::Onlien` is one possibility; should we have a +// different enum if we're actually restricted to "enabled but not running"? pub struct Svc { pub fmri: String, pub zone: String, From cc54ca7db0d1c4e5f0fe73808f49a0a2a131d722 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 Mar 2026 10:45:22 -0400 Subject: [PATCH 7/7] typo --- .../versions/src/modify_services_in_inventory/inventory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs b/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs index 18f484c75bf..4e56908a20f 100644 --- a/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs +++ b/sled-agent/types/versions/src/modify_services_in_inventory/inventory.rs @@ -54,7 +54,7 @@ pub enum SvcState { #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "snake_case")] /// Information about an SMF service that is enabled but not running -// TODO-correctness `SvcState::Onlien` is one possibility; should we have a +// TODO-correctness `SvcState::Online` is one possibility; should we have a // different enum if we're actually restricted to "enabled but not running"? pub struct Svc { pub fmri: String,