Skip to content

Commit

Permalink
[sled-agent] turn API into a trait (#6159)
Browse files Browse the repository at this point in the history
Building on the changes in #6138, this PR turns the sled-agent API into a trait. This is
high-impact because it currently is part of a circular dependency with the
Nexus internal API -- this PR gets us to the point where the OpenAPI documents
can be generated independently of each other.

There is some code movement but there are no functional changes in this PR -- I
didn't use `replace` here to keep the PR small. I'll try out `replace` for some
of the types with `From` impls in a subsequent PR.

(I did change the internal `VpcFirewallRule` to `ResolvedVpcFirewallRule` to
match the other types in the destination file.)

I've also added some documentation about when `nexus-sled-agent-shared` should
be used.

Depends on #6283.
  • Loading branch information
sunshowers authored Aug 13, 2024
1 parent 2c58852 commit 544fa4c
Show file tree
Hide file tree
Showing 72 changed files with 2,791 additions and 2,421 deletions.
32 changes: 29 additions & 3 deletions Cargo.lock

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

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ members = [
"passwords",
"rpaths",
"sled-agent",
"sled-agent/api",
"sled-agent/bootstrap-agent-api",
"sled-agent/types",
"sled-hardware",
Expand Down Expand Up @@ -196,6 +197,7 @@ default-members = [
"passwords",
"rpaths",
"sled-agent",
"sled-agent/api",
"sled-agent/bootstrap-agent-api",
"sled-agent/types",
"sled-hardware",
Expand Down Expand Up @@ -517,6 +519,7 @@ similar-asserts = "1.5.0"
# are still doing mupdate a change to the on-disk format will break existing DNS
# server zones.
sled = "=0.34.7"
sled-agent-api = { path = "sled-agent/api" }
sled-agent-client = { path = "clients/sled-agent-client" }
sled-agent-types = { path = "sled-agent/types" }
sled-hardware = { path = "sled-hardware" }
Expand Down
1 change: 1 addition & 0 deletions clients/installinator-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ workspace = true

[dependencies]
installinator-common.workspace = true
omicron-common.workspace = true
progenitor.workspace = true
regress.workspace = true
reqwest = { workspace = true, features = ["rustls-tls", "stream"] }
Expand Down
2 changes: 1 addition & 1 deletion clients/installinator-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ progenitor::generate_api!(
replace = {
Duration = std::time::Duration,
EventReportForInstallinatorSpec = installinator_common::EventReport,
M2Slot = installinator_common::M2Slot,
M2Slot = omicron_common::disk::M2Slot,
ProgressEventForGenericSpec = installinator_common::ProgressEvent<update_engine::NestedSpec>,
ProgressEventForInstallinatorSpec = installinator_common::ProgressEvent,
StepEventForGenericSpec = installinator_common::StepEvent<update_engine::NestedSpec>,
Expand Down
49 changes: 49 additions & 0 deletions common/src/api/internal/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ use std::{
};
use uuid::Uuid;

use super::nexus::HostIdentifier;

/// The type of network interface
#[derive(
Clone,
Expand Down Expand Up @@ -635,6 +637,53 @@ pub struct ResolvedVpcRoute {
pub target: RouterTarget,
}

/// VPC firewall rule after object name resolution has been performed by Nexus
#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)]
pub struct ResolvedVpcFirewallRule {
pub status: external::VpcFirewallRuleStatus,
pub direction: external::VpcFirewallRuleDirection,
pub targets: Vec<NetworkInterface>,
pub filter_hosts: Option<Vec<HostIdentifier>>,
pub filter_ports: Option<Vec<external::L4PortRange>>,
pub filter_protocols: Option<Vec<external::VpcFirewallRuleProtocol>>,
pub action: external::VpcFirewallRuleAction,
pub priority: external::VpcFirewallRulePriority,
}

/// A mapping from a virtual NIC to a physical host
#[derive(
Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq, Hash,
)]
pub struct VirtualNetworkInterfaceHost {
pub virtual_ip: IpAddr,
pub virtual_mac: external::MacAddr,
pub physical_host_ip: Ipv6Addr,
pub vni: external::Vni,
}

/// DHCP configuration for a port
///
/// Not present here: Hostname (DHCPv4 option 12; used in DHCPv6 option 39); we
/// use `InstanceRuntimeState::hostname` for this value.
#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)]
pub struct DhcpConfig {
/// DNS servers to send to the instance
///
/// (DHCPv4 option 6; DHCPv6 option 23)
pub dns_servers: Vec<IpAddr>,

/// DNS zone this instance's hostname belongs to (e.g. the `project.example`
/// part of `instance1.project.example`)
///
/// (DHCPv4 option 15; used in DHCPv6 option 39)
pub host_domain: Option<String>,

/// DNS search domains
///
/// (DHCPv4 option 119; DHCPv6 option 24)
pub search_domains: Vec<String>,
}

/// The target for a given router entry.
#[derive(
Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq, Hash,
Expand Down
115 changes: 115 additions & 0 deletions common/src/disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

//! Disk related types shared among crates
use std::fmt;

use anyhow::bail;
use omicron_uuid_kinds::ZpoolUuid;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -114,3 +117,115 @@ 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(
Debug,
Clone,
Copy,
PartialEq,
Eq,
PartialOrd,
Ord,
Deserialize,
Serialize,
JsonSchema,
)]
pub enum M2Slot {
A,
B,
}

impl fmt::Display for M2Slot {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::A => f.write_str("A"),
Self::B => f.write_str("B"),
}
}
}

impl TryFrom<i64> for M2Slot {
type Error = anyhow::Error;

fn try_from(value: i64) -> Result<Self, Self::Error> {
match value {
// Gimlet should have 2 M.2 drives: drive A is assigned slot 17, and
// drive B is assigned slot 18.
17 => Ok(Self::A),
18 => Ok(Self::B),
_ => bail!("unexpected M.2 slot {value}"),
}
}
}
1 change: 1 addition & 0 deletions dev-tools/openapi-manager/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ openapi-lint.workspace = true
owo-colors.workspace = true
oximeter-api.workspace = true
serde_json.workspace = true
sled-agent-api.workspace = true
similar.workspace = true
supports-color.workspace = true
wicketd-api.workspace = true
10 changes: 10 additions & 0 deletions dev-tools/openapi-manager/src/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ pub fn all_apis() -> Vec<ApiSpec> {
filename: "oximeter.json",
extra_validation: None,
},
ApiSpec {
title: "Oxide Sled Agent API",
version: "0.0.1",
description: "API for interacting with individual sleds",
boundary: ApiBoundary::Internal,
api_description:
sled_agent_api::sled_agent_api_mod::stub_api_description,
filename: "sled-agent.json",
extra_validation: None,
},
ApiSpec {
title: "Oxide Technician Port Control Service",
version: "0.0.1",
Expand Down
6 changes: 3 additions & 3 deletions illumos-utils/src/opte/firewall_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
//! Convert Omicron VPC firewall rules to OPTE firewall rules.
use super::net_to_cidr;
use crate::opte::params::VpcFirewallRule;
use crate::opte::Vni;
use macaddr::MacAddr6;
use omicron_common::api::external::VpcFirewallRuleAction;
use omicron_common::api::external::VpcFirewallRuleDirection;
use omicron_common::api::external::VpcFirewallRuleProtocol;
use omicron_common::api::external::VpcFirewallRuleStatus;
use omicron_common::api::internal::nexus::HostIdentifier;
use omicron_common::api::internal::shared::ResolvedVpcFirewallRule;
use oxide_vpc::api::Address;
use oxide_vpc::api::Direction;
use oxide_vpc::api::Filters;
Expand All @@ -34,7 +34,7 @@ trait FromVpcFirewallRule {
fn protos(&self) -> Vec<ProtoFilter>;
}

impl FromVpcFirewallRule for VpcFirewallRule {
impl FromVpcFirewallRule for ResolvedVpcFirewallRule {
fn action(&self) -> FirewallAction {
match self.action {
VpcFirewallRuleAction::Allow => FirewallAction::Allow,
Expand Down Expand Up @@ -118,7 +118,7 @@ impl FromVpcFirewallRule for VpcFirewallRule {
/// a single host address and protocol, so we must unroll rules with multiple
/// hosts/protocols.
pub fn opte_firewall_rules(
rules: &[VpcFirewallRule],
rules: &[ResolvedVpcFirewallRule],
vni: &Vni,
mac: &MacAddr6,
) -> Vec<FirewallRule> {
Expand Down
Loading

0 comments on commit 544fa4c

Please sign in to comment.