Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions clients/sled-agent-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ anyhow.workspace = true
async-trait.workspace = true
byte-wrapper.workspace = true
chrono.workspace = true
iddqd.workspace = true
omicron-generation-kinds.workspace = true
sled-agent-types-versions.workspace = true
omicron-common.workspace = true
Expand Down
22 changes: 21 additions & 1 deletion clients/sled-agent-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,16 @@ progenitor::generate_api!(
},
crates = {
"byte-wrapper" = "0.2.0",
"iddqd" = "*",
"omicron-uuid-kinds" = "*",
"oxnet" = "0.1.0",
},
replace = {
ArtifactConfig = sled_agent_types::artifact::ArtifactConfig,
ArtifactConfigGeneration = omicron_generation_kinds::ArtifactConfigGeneration,
AttachedSubnet = sled_agent_types_versions::latest::attached_subnet::AttachedSubnet,
AttachedSubnetKind = sled_agent_types_versions::latest::attached_subnet::AttachedSubnetKind,
AttachedSubnets = sled_agent_types_versions::latest::attached_subnet::AttachedSubnets,
Attestation = sled_agent_types_versions::latest::rot::Attestation,
Baseboard = sled_agent_types_versions::latest::inventory::Baseboard,
BaseboardId = sled_hardware_types::BaseboardId,
Expand All @@ -41,13 +45,16 @@ progenitor::generate_api!(
CertificateChain = sled_agent_types_versions::latest::rot::CertificateChain,
CommitRequest = trust_quorum_types::messages::CommitRequest,
CommitStatus = trust_quorum_types::status::CommitStatus,
ConfigReconcilerInventory = sled_agent_types_versions::latest::inventory::ConfigReconcilerInventory,
CoordinatorStatus = trust_quorum_types::status::CoordinatorStatus,
DatasetConfig = sled_agent_types_versions::latest::disk::DatasetConfig,
DatasetKind = omicron_common::api::internal::shared::DatasetKind,
DiskIdentity = sled_agent_types::disk::DiskIdentity,
DiskVariant = sled_agent_types::disk::DiskVariant,
Epoch = trust_quorum_types::types::Epoch,
ExternalIpGatewayMap = omicron_common::api::internal::shared::ExternalIpGatewayMap,
ExternalIp = sled_agent_types_versions::latest::probes::ExternalIp,
ExternalIpConfig = sled_agent_types_versions::latest::instance::ExternalIpConfig,
ExternalIpGatewayMap = omicron_common::api::internal::shared::ExternalIpGatewayMap,
ExternalIpv4Config = sled_agent_types_versions::latest::instance::ExternalIpv4Config,
ExternalIpv6Config = sled_agent_types_versions::latest::instance::ExternalIpv6Config,
FmdHostCase = sled_agent_types_versions::latest::inventory::FmdHostCase,
Expand All @@ -61,18 +68,24 @@ progenitor::generate_api!(
Inventory = sled_agent_types_versions::latest::inventory::Inventory,
InventoryDisk = sled_agent_types_versions::latest::inventory::InventoryDisk,
InventoryZpool = sled_agent_types_versions::latest::inventory::InventoryZpool,
IpKind = sled_agent_types_versions::latest::probes::IpKind,
LinkFec = sled_agent_types_versions::latest::early_networking::LinkFec,
LinkSpeed = sled_agent_types_versions::latest::early_networking::LinkSpeed,
LldpAdminStatus = sled_agent_types_versions::latest::early_networking::LldpAdminStatus,
LldpPortConfig = sled_agent_types_versions::latest::early_networking::LldpPortConfig,
LrtqUpgradeMsg = trust_quorum_types::messages::LrtqUpgradeMsg,
MacAddr = omicron_common::api::external::MacAddr,
ManifestBootInventory = sled_agent_types_versions::latest::inventory::ManifestBootInventory,
ManifestInventory = sled_agent_types_versions::latest::inventory::ManifestInventory,
ManifestNonBootInventory = sled_agent_types_versions::latest::inventory::ManifestNonBootInventory,
MaxPathConfig = sled_agent_types_versions::latest::early_networking::MaxPathConfig,
Measurement = sled_agent_types_versions::latest::rot::Measurement,
MeasurementLog = sled_agent_types_versions::latest::rot::MeasurementLog,
MigrationRuntimeState = sled_agent_types_versions::latest::instance::MigrationRuntimeState,
MigrationState = sled_agent_types_versions::latest::instance::MigrationState,
MupdateOverrideBootInventory = sled_agent_types_versions::latest::inventory::MupdateOverrideBootInventory,
MupdateOverrideInventory = sled_agent_types_versions::latest::inventory::MupdateOverrideInventory,
MupdateOverrideNonBootInventory = sled_agent_types_versions::latest::inventory::MupdateOverrideNonBootInventory,
Name = omicron_common::api::external::Name,
NetworkInterface = sled_agent_types_versions::latest::inventory::NetworkInterface,
NetworkInterfaceKind = sled_agent_types_versions::latest::inventory::NetworkInterfaceKind,
Expand All @@ -84,8 +97,11 @@ progenitor::generate_api!(
OmicronZoneImageSource = sled_agent_types_versions::latest::inventory::OmicronZoneImageSource,
OmicronZoneType = sled_agent_types_versions::latest::inventory::OmicronZoneType,
OmicronZonesConfig = sled_agent_types_versions::latest::inventory::OmicronZonesConfig,
OrphanedDataset = sled_agent_types_versions::latest::inventory::OrphanedDataset,
PortConfig = sled_agent_types_versions::latest::early_networking::PortConfig,
PrepareAndCommitRequest = trust_quorum_types::messages::PrepareAndCommitRequest,
ProbeCreate = sled_agent_types_versions::latest::probes::ProbeCreate,
ProbeSet = sled_agent_types_versions::latest::probes::ProbeSet,
RackNetworkConfig = sled_agent_types_versions::latest::early_networking::RackNetworkConfig,
ReconfigureMsg = trust_quorum_types::messages::ReconfigureMsg,
ResolvedVpcFirewallRule = sled_agent_types_versions::latest::instance::ResolvedVpcFirewallRule,
Expand All @@ -97,7 +113,10 @@ progenitor::generate_api!(
RouterLifetimeConfig = sled_agent_types_versions::latest::early_networking::RouterLifetimeConfig,
RouterTarget = omicron_common::api::internal::shared::RouterTarget,
RouterVersion = omicron_common::api::internal::shared::RouterVersion,
ServiceZoneNatEntries = sled_agent_types_versions::latest::system_networking::ServiceZoneNatEntries,
ServiceZoneNatEntry = sled_agent_types_versions::latest::system_networking::ServiceZoneNatEntry,
Sha3_256Digest = sled_agent_types_versions::latest::rot::Sha3_256Digest,
SingleMeasurementInventory = sled_agent_types_versions::latest::inventory::SingleMeasurementInventory,
SledRole = sled_agent_types_versions::latest::inventory::SledRole,
SledVmmState = sled_agent_types_versions::latest::instance::SledVmmState,
SourceNatConfigGeneric = sled_agent_types_versions::latest::inventory::SourceNatConfigGeneric,
Expand All @@ -111,6 +130,7 @@ progenitor::generate_api!(
Vni = omicron_common::api::external::Vni,
VpcFirewallIcmpFilter = omicron_common::api::external::VpcFirewallIcmpFilter,
WriteNetworkConfigRequest = sled_agent_types_versions::latest::system_networking::WriteNetworkConfigRequest,
ZoneArtifactInventory = sled_agent_types_versions::latest::inventory::ZoneArtifactInventory,
ZpoolKind = omicron_common::zpool_name::ZpoolKind,
ZpoolName = omicron_common::zpool_name::ZpoolName,
}
Expand Down
38 changes: 38 additions & 0 deletions nexus/db-queries/src/db/datastore/external_subnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,44 @@ impl DataStore {

// TODO-completeness: We need to fetch the attached VPC Subnets here
// too. See https://github.com/oxidecomputer/omicron/issues/9580.
//
// TODO: Parts of our code base assume that attached subnets are unique
// per subnet (IpNet), but the return value of this function doesn't
// indicate that. As of 2026-10-21, external subnets are unique in the
// `external_subnet` table, so it would seem that data returned from
// the database should uphold this invariant. But:
//
// * `list_attached_external_subnets` below has an inner join with
// `network_interface` to determine the primary NIC. The database
// does not prevent an instance from having two primary NICs, so
// it is possible that we get multiple `AttachedSubnet` entries for
// the same subnet and instance, each with a different MAC. (Since
// pagination is keyed on the external subnet ID, such duplicates
// that straddle a page boundary are silently dropped.) At the
// application layer, Nexus _does_ prevent an instance from having
// two primary NICs, so the risk is mostly theoretical, but ideally
// the database would enforce it.
// * Issue #9580 (mentioned immediately above) would have us fetch
// attached VPC subnets, and there's no current database constraint
// against a VPC subnet being the same as an external subnet. Also,
// VPC subnets are only unique within a VPC, so it is legitimately
// possible for two instances in different VPCs to attach the same
// subnet.
//
// We should:
//
// * Make the primary-NIC join 1:1 with a database constraint (e.g., a

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you file an issue for this? We should have such a constraint.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#11568. thanks!

// partial unique index on `network_interface (parent_id) WHERE
// is_primary AND time_deleted IS NULL`).
// * As part of addressing #9580, figure out the correct uniqueness
// constraint for `shared::AttachedSubnet` (just the subnet?
// instance or VMM + subnet?), and express that in the IdOrdItem
// key.
// * Return an `IdOrdMap` or similar of `shared::AttachedSubnet`s,
// thus expressing the uniqueness in the function signature.
// * Within the scope of an instance, reject equal or overlapping
// subnets of any kind (external or VPC) at attach time with a clear
// error.
Ok(results)
}

Expand Down
41 changes: 26 additions & 15 deletions nexus/db-queries/src/db/datastore/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use chrono::Utc;
use diesel::JoinOnDsl as _;
use diesel::NullableExpressionMethods as _;
use diesel::{ExpressionMethods, QueryDsl, SelectableHelper};
use iddqd::IdHashMap;
use nexus_db_errors::ErrorHandler;
use nexus_db_errors::public_error_from_diesel;
use nexus_db_lookup::LookupPath;
Expand All @@ -28,6 +29,7 @@ use nexus_types::identity::Resource;
use omicron_common::api::external::CreateResult;
use omicron_common::api::external::DataPageParams;
use omicron_common::api::external::DeleteResult;
use omicron_common::api::external::Error;
use omicron_common::api::external::IdentityMetadataCreateParams;
use omicron_common::api::external::ListResultVec;
use omicron_common::api::external::LookupResult;
Expand All @@ -39,8 +41,9 @@ use omicron_uuid_kinds::GenericUuid as _;
use omicron_uuid_kinds::ProbeUuid;
use omicron_uuid_kinds::SledUuid;
use ref_cast::RefCast;
use sled_agent_client::types::ProbeCreate;
use sled_agent_types::inventory::NetworkInterface;
use sled_agent_types::probes::ProbeCreate;
use slog_error_chain::InlineErrorChain;
use uuid::Uuid;

impl super::DataStore {
Expand Down Expand Up @@ -269,11 +272,9 @@ impl super::DataStore {
.probe_create_network_interface(opctx, incomplete)
.await
.map_err(|e| {
omicron_common::api::external::Error::InternalError {
internal_message: format!(
"create network interface: {e:?}"
),
}
Error::internal_error(format!(
"create network interface: {e:?}"
))
})?;
diesel::insert_into(dsl::probe)
.values(probe.clone())
Expand All @@ -294,16 +295,16 @@ impl super::DataStore {
&self,
opctx: &OpContext,
sled_id: SledUuid,
) -> ListResultVec<ProbeCreate> {
) -> Result<IdHashMap<ProbeCreate>, Error> {
opctx.check_complex_operations_allowed()?;
opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?;
let mut paginator = Paginator::new(
SQL_BATCH_SIZE,
dropshot::PaginationOrder::Ascending,
);
let mut all_probes_for_sled = Vec::new();
let mut all_probes_for_sled = IdHashMap::new();
while let Some(p) = paginator.next() {
let mut batch = self
let batch = self
.list_probe_create_params_for_sled(
opctx,
sled_id,
Expand All @@ -312,7 +313,17 @@ impl super::DataStore {
.await?;
paginator =
p.found_batch(&batch, &|probe| probe.id.into_untyped_uuid());
all_probes_for_sled.append(&mut batch);
for probe in batch {
all_probes_for_sled.insert_unique(probe).map_err(|err| {
Error::internal_error(format!(
"encountered multiple probe create parameters for \
sled {sled_id} with the same probe ID {} (does a \
probe have more than one external IP or NIC?): {}",
err.new_item().id,
InlineErrorChain::new(&err),
))
})?;
}
}
Ok(all_probes_for_sled)
}
Expand Down Expand Up @@ -402,7 +413,7 @@ impl super::DataStore {
)| {
let kind = db_ip_kind_to_sled(kind);
let external_ips =
vec![sled_agent_client::types::ExternalIp {
vec![sled_agent_types::probes::ExternalIp {
first_port: first_port.into(),
ip: ip.ip(),
kind,
Expand Down Expand Up @@ -471,14 +482,14 @@ impl super::DataStore {

const fn db_ip_kind_to_sled(
kind: nexus_db_model::IpKind,
) -> sled_agent_client::types::IpKind {
) -> sled_agent_types::probes::IpKind {
match kind {
nexus_db_model::IpKind::SNat => sled_agent_client::types::IpKind::Snat,
nexus_db_model::IpKind::SNat => sled_agent_types::probes::IpKind::Snat,
nexus_db_model::IpKind::Ephemeral => {
sled_agent_client::types::IpKind::Ephemeral
sled_agent_types::probes::IpKind::Ephemeral
}
nexus_db_model::IpKind::Floating => {
sled_agent_client::types::IpKind::Floating
sled_agent_types::probes::IpKind::Floating
}
}
}
Loading
Loading