Skip to content

Commit

Permalink
avoid dependency from openapi-manager to libnvme
Browse files Browse the repository at this point in the history
Created using spr 1.3.6-beta.1
  • Loading branch information
sunshowers committed Aug 10, 2024
1 parent 623e8f6 commit 1ed5565
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 92 deletions.
2 changes: 0 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

70 changes: 70 additions & 0 deletions common/src/disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,76 @@ impl From<ZpoolKind> for DiskVariant {
}
}

/// Identifies how a single disk management operation may have succeeded or
/// failed.
#[derive(Debug, JsonSchema, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub struct DiskManagementStatus {
pub identity: DiskIdentity,
pub err: Option<DiskManagementError>,
}

/// The result from attempting to manage underlying disks.
///
/// This is more complex than a simple "Error" type because it's possible
/// for some disks to be initialized correctly, while others can fail.
///
/// This structure provides a mechanism for callers to learn about partial
/// failures, and handle them appropriately on a per-disk basis.
#[derive(Default, Debug, JsonSchema, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
#[must_use = "this `DiskManagementResult` may contain errors, which should be handled"]
pub struct DisksManagementResult {
pub status: Vec<DiskManagementStatus>,
}

impl DisksManagementResult {
pub fn has_error(&self) -> bool {
for status in &self.status {
if status.err.is_some() {
return true;
}
}
false
}

pub fn has_retryable_error(&self) -> bool {
for status in &self.status {
if let Some(err) = &status.err {
if err.retryable() {
return true;
}
}
}
false
}
}

#[derive(Debug, thiserror::Error, JsonSchema, Serialize, Deserialize)]
#[serde(rename_all = "snake_case", tag = "type", content = "value")]
pub enum DiskManagementError {
#[error("Disk requested by control plane, but not found on device")]
NotFound,

#[error("Expected zpool UUID of {expected}, but saw {observed}")]
ZpoolUuidMismatch { expected: ZpoolUuid, observed: ZpoolUuid },

#[error("Failed to access keys necessary to unlock storage. This error may be transient.")]
KeyManager(String),

#[error("Other error starting disk management: {0}")]
Other(String),
}

impl DiskManagementError {
fn retryable(&self) -> bool {
match self {
DiskManagementError::KeyManager(_) => true,
_ => false,
}
}
}

/// Describes an M.2 slot, often in the context of writing a system image to
/// it.
#[derive(
Expand Down
1 change: 0 additions & 1 deletion sled-agent/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,4 @@ schemars.workspace = true
serde.workspace = true
sled-agent-types.workspace = true
sled-hardware-types.workspace = true
sled-storage.workspace = true
uuid.workspace = true
3 changes: 1 addition & 2 deletions sled-agent/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use omicron_common::{
SwitchPorts, VirtualNetworkInterfaceHost,
},
},
disk::{DiskVariant, OmicronPhysicalDisksConfig},
disk::{DiskVariant, DisksManagementResult, OmicronPhysicalDisksConfig},
};
use omicron_uuid_kinds::{InstanceUuid, ZpoolUuid};
use schemars::JsonSchema;
Expand All @@ -46,7 +46,6 @@ use sled_agent_types::{
ZoneBundleId, ZoneBundleMetadata,
},
};
use sled_storage::resources::DisksManagementResult;
use uuid::Uuid;

#[dropshot::api_description]
Expand Down
5 changes: 3 additions & 2 deletions sled-agent/src/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ use omicron_common::api::internal::shared::{
ResolvedVpcRouteSet, ResolvedVpcRouteState, SledIdentifiers, SwitchPorts,
VirtualNetworkInterfaceHost,
};
use omicron_common::disk::{DiskVariant, M2Slot, OmicronPhysicalDisksConfig};
use omicron_common::disk::{
DiskVariant, DisksManagementResult, M2Slot, OmicronPhysicalDisksConfig,
};
use omicron_uuid_kinds::{GenericUuid, InstanceUuid};
use sled_agent_api::*;
use sled_agent_types::boot_disk::{
Expand All @@ -48,7 +50,6 @@ use sled_agent_types::zone_bundle::{
BundleUtilization, CleanupContext, CleanupCount, CleanupPeriod,
StorageLimit, ZoneBundleId, ZoneBundleMetadata,
};
use sled_storage::resources::DisksManagementResult;
use std::collections::BTreeMap;

type SledApiDescription = ApiDescription<SledAgent>;
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/sim/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use omicron_common::api::internal::shared::VirtualNetworkInterfaceHost;
use omicron_common::api::internal::shared::{
ResolvedVpcRouteSet, ResolvedVpcRouteState, SwitchPorts,
};
use omicron_common::disk::DisksManagementResult;
use omicron_common::disk::OmicronPhysicalDisksConfig;
use omicron_uuid_kinds::{GenericUuid, InstanceUuid};
use schemars::JsonSchema;
Expand All @@ -34,7 +35,6 @@ use sled_agent_types::instance::InstancePutStateBody;
use sled_agent_types::instance::InstancePutStateResponse;
use sled_agent_types::instance::InstanceUnregisterResponse;
use sled_agent_types::sled::AddSledRequest;
use sled_storage::resources::DisksManagementResult;
use std::sync::Arc;
use uuid::Uuid;

Expand Down
4 changes: 2 additions & 2 deletions sled-agent/src/sim/sled_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ use omicron_common::api::internal::shared::{
VirtualNetworkInterfaceHost,
};
use omicron_common::disk::{
DiskIdentity, DiskVariant, OmicronPhysicalDisksConfig,
DiskIdentity, DiskVariant, DisksManagementResult,
OmicronPhysicalDisksConfig,
};
use omicron_uuid_kinds::{GenericUuid, InstanceUuid, PropolisUuid, ZpoolUuid};
use oxnet::Ipv6Net;
Expand All @@ -52,7 +53,6 @@ use sled_agent_types::instance::{
InstancePutStateResponse, InstanceStateRequested,
InstanceUnregisterResponse,
};
use sled_storage::resources::DisksManagementResult;
use slog::Logger;
use std::collections::{HashMap, HashSet, VecDeque};
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
Expand Down
4 changes: 2 additions & 2 deletions sled-agent/src/sim/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ use dropshot::HandlerTaskMode;
use dropshot::HttpError;
use futures::lock::Mutex;
use omicron_common::disk::DiskIdentity;
use omicron_common::disk::DiskManagementStatus;
use omicron_common::disk::DiskVariant;
use omicron_common::disk::DisksManagementResult;
use omicron_common::disk::OmicronPhysicalDisksConfig;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::InstanceUuid;
use omicron_uuid_kinds::OmicronZoneUuid;
use omicron_uuid_kinds::ZpoolUuid;
use propolis_client::types::VolumeConstructionRequest;
use sled_storage::resources::DiskManagementStatus;
use sled_storage::resources::DisksManagementResult;
use slog::Logger;
use std::collections::HashMap;
use std::collections::HashSet;
Expand Down
3 changes: 1 addition & 2 deletions sled-agent/src/sled_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ use omicron_common::api::{
use omicron_common::backoff::{
retry_notify, retry_policy_internal_service_aggressive, BackoffError,
};
use omicron_common::disk::OmicronPhysicalDisksConfig;
use omicron_common::disk::{DisksManagementResult, OmicronPhysicalDisksConfig};
use omicron_ddm_admin_client::Client as DdmAdminClient;
use omicron_uuid_kinds::{InstanceUuid, PropolisUuid};
use sled_agent_api::Zpool;
Expand All @@ -74,7 +74,6 @@ use sled_hardware::{underlay, HardwareManager};
use sled_hardware_types::underlay::BootstrapInterface;
use sled_hardware_types::Baseboard;
use sled_storage::manager::StorageHandle;
use sled_storage::resources::DisksManagementResult;
use slog::Logger;
use std::collections::BTreeMap;
use std::net::{Ipv6Addr, SocketAddr, SocketAddrV6};
Expand Down
1 change: 0 additions & 1 deletion sled-agent/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ serde_human_bytes.workspace = true
serde_json.workspace = true
sha3.workspace = true
sled-hardware-types.workspace = true
sled-storage.workspace = true
slog.workspace = true
thiserror.workspace = true
toml.workspace = true
Expand Down
7 changes: 4 additions & 3 deletions sled-storage/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@ use crate::config::MountConfig;
use crate::dataset::{DatasetName, CONFIG_DATASET};
use crate::disk::RawDisk;
use crate::error::Error;
use crate::resources::{AllDisks, DisksManagementResult, StorageResources};
use crate::resources::{AllDisks, StorageResources};
use camino::Utf8PathBuf;
use debug_ignore::DebugIgnore;
use futures::future::FutureExt;
use illumos_utils::zfs::{Mountpoint, Zfs};
use illumos_utils::zpool::ZpoolName;
use key_manager::StorageKeyRequester;
use omicron_common::disk::{
DiskIdentity, DiskVariant, OmicronPhysicalDisksConfig,
DiskIdentity, DiskVariant, DisksManagementResult,
OmicronPhysicalDisksConfig,
};
use omicron_common::ledger::Ledger;
use slog::{info, o, warn, Logger};
Expand Down Expand Up @@ -826,10 +827,10 @@ mod tests {
use crate::dataset::DatasetType;
use crate::disk::RawSyntheticDisk;
use crate::manager_test_harness::StorageManagerTestHarness;
use crate::resources::DiskManagementError;

use super::*;
use camino_tempfile::tempdir_in;
use omicron_common::disk::DiskManagementError;
use omicron_common::ledger;
use omicron_test_utils::dev::test_setup_log;
use sled_hardware::DiskFirmware;
Expand Down
76 changes: 2 additions & 74 deletions sled-storage/src/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ use illumos_utils::zpool::{PathInPool, ZpoolName};
use key_manager::StorageKeyRequester;
use omicron_common::api::external::Generation;
use omicron_common::disk::{
DiskIdentity, DiskVariant, OmicronPhysicalDiskConfig,
DiskIdentity, DiskManagementError, DiskManagementStatus, DiskVariant,
DisksManagementResult, OmicronPhysicalDiskConfig,
OmicronPhysicalDisksConfig,
};
use omicron_uuid_kinds::ZpoolUuid;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use sled_hardware::DiskFirmware;
use slog::{info, o, warn, Logger};
use std::collections::BTreeMap;
Expand All @@ -32,76 +30,6 @@ const BUNDLE_DIRECTORY: &str = "bundle";
// The directory for zone bundles.
const ZONE_BUNDLE_DIRECTORY: &str = "zone";

#[derive(Debug, thiserror::Error, JsonSchema, Serialize, Deserialize)]
#[serde(rename_all = "snake_case", tag = "type", content = "value")]
pub enum DiskManagementError {
#[error("Disk requested by control plane, but not found on device")]
NotFound,

#[error("Expected zpool UUID of {expected}, but saw {observed}")]
ZpoolUuidMismatch { expected: ZpoolUuid, observed: ZpoolUuid },

#[error("Failed to access keys necessary to unlock storage. This error may be transient.")]
KeyManager(String),

#[error("Other error starting disk management: {0}")]
Other(String),
}

impl DiskManagementError {
fn retryable(&self) -> bool {
match self {
DiskManagementError::KeyManager(_) => true,
_ => false,
}
}
}

/// Identifies how a single disk management operation may have succeeded or
/// failed.
#[derive(Debug, JsonSchema, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub struct DiskManagementStatus {
pub identity: DiskIdentity,
pub err: Option<DiskManagementError>,
}

/// The result from attempting to manage underlying disks.
///
/// This is more complex than a simple "Error" type because it's possible
/// for some disks to be initialized correctly, while others can fail.
///
/// This structure provides a mechanism for callers to learn about partial
/// failures, and handle them appropriately on a per-disk basis.
#[derive(Default, Debug, JsonSchema, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
#[must_use = "this `DiskManagementResult` may contain errors, which should be handled"]
pub struct DisksManagementResult {
pub status: Vec<DiskManagementStatus>,
}

impl DisksManagementResult {
pub fn has_error(&self) -> bool {
for status in &self.status {
if status.err.is_some() {
return true;
}
}
false
}

pub fn has_retryable_error(&self) -> bool {
for status in &self.status {
if let Some(err) = &status.err {
if err.retryable() {
return true;
}
}
}
false
}
}

// The Sled Agent is responsible for both observing disks and managing them at
// the request of the broader control plane. This enum encompasses that duality,
// by representing all disks that can exist, managed or not.
Expand Down

0 comments on commit 1ed5565

Please sign in to comment.