Skip to content

[internal-dns] register and publish ddmd in the switch zone #10381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zeeshanlakhani wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[internal-dns] register and publish ddmd in the switch zone #10381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 1 addition & 3 deletions clients/ddm-admin-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub use ddm_admin_client::types;

use ddm_admin_client::Client as InnerClient;
use either::Either;
use omicron_common::address::DDMD_PORT;
use oxnet::Ipv6Net;
use sled_hardware_types::underlay::BOOTSTRAP_MASK;
use sled_hardware_types::underlay::BOOTSTRAP_PREFIX;
Expand All @@ -26,9 +27,6 @@ use thiserror::Error;

use crate::types::EnableStatsRequest;

// TODO-cleanup Is it okay to hardcode this port number here?
const DDMD_PORT: u16 = 8000;

#[derive(Debug, Error, SlogInlineError)]
pub enum DdmError {
#[error("Failed to construct an HTTP client:")]
Expand Down
71 changes: 69 additions & 2 deletions internal-dns/types/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ impl DnsConfigBuilder {
dendrite_port: u16,
mgs_port: u16,
mgd_port: u16,
ddm_port: u16,
Copy link
Copy Markdown
Contributor

@jgallagher jgallagher May 6, 2026

Choose a reason for hiding this comment

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

I think this function taking 3 u16s in a row was already sketchy - what do you think about changing this to take something like

struct HostSwitchZonePorts {
    dendrite: u16,
    mgs: u16,
    mgd: u16,
    ddm: u16,
}

instead of four separate arguments? That way we get named parameters for the call sites, and don't have to rely on them getting the order correct, and inside this function, we can destructure them:

let HostSwitchZonePorts { dendrite, ...all the rest ... } = ports;

so we also get a compile-time check that we update this function if we change the struct?

) -> anyhow::Result<()> {
let zone = self.host_dendrite(sled_id, switch_zone_ip)?;
self.service_backend_zone(ServiceName::Dendrite, &zone, dendrite_port)?;
Expand All @@ -407,7 +408,8 @@ impl DnsConfigBuilder {
&zone,
mgs_port,
)?;
self.service_backend_zone(ServiceName::Mgd, &zone, mgd_port)
self.service_backend_zone(ServiceName::Mgd, &zone, mgd_port)?;
self.service_backend_zone(ServiceName::Ddm, &zone, ddm_port)
}

/// Higher-level shorthand for adding a Nexus zone with both its internal
Expand Down Expand Up @@ -731,7 +733,7 @@ impl DnsConfigBuilder {

#[cfg(test)]
mod test {
use super::{DnsConfigBuilder, Host, ServiceName};
use super::{DnsConfigBuilder, DnsRecord, Host, ServiceName};
use crate::{config::Zone, names::DNS_ZONE};
use omicron_common::api::external::Generation;
use omicron_uuid_kinds::{OmicronZoneUuid, SledUuid};
Expand Down Expand Up @@ -779,6 +781,8 @@ mod test {
"_oximeter-reader._tcp",
);
assert_eq!(ServiceName::Dendrite.dns_name(), "_dendrite._tcp",);
assert_eq!(ServiceName::Mgd.dns_name(), "_mgd._tcp",);
assert_eq!(ServiceName::Ddm.dns_name(), "_ddm._tcp",);
assert_eq!(
ServiceName::CruciblePantry.dns_name(),
"_crucible-pantry._tcp",
Expand All @@ -796,6 +800,69 @@ mod test {
);
}

#[test]
fn host_zone_switch_publishes_all_services() {
let sled_uuid: SledUuid =
"001de000-51ed-4000-8000-000000000001".parse().unwrap();
let switch_zone_ip = Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1);

// Use distinct port numbers so an arg-order swap in `host_zone_switch`
// surfaces as a port mismatch on the affected service.
let dendrite_port = 11;
let mgs_port = 13;
let mgd_port = 17;
let ddm_port = 19;

let mut builder = DnsConfigBuilder::new();
builder
.host_zone_switch(
sled_uuid,
switch_zone_ip,
dendrite_port,
mgs_port,
mgd_port,
ddm_port,
)
.unwrap();

let config = builder.build_full_config_for_initial_generation();

let mut by_name: BTreeMap<&str, &[DnsRecord]> = BTreeMap::new();
for zone in &config.zones {
for (name, records) in &zone.records {
by_name.insert(name.as_str(), records.as_slice());
}
}

for (expected_name, expected_port) in [
("_dendrite._tcp", dendrite_port),
("_mgs._tcp", mgs_port),
("_mgd._tcp", mgd_port),
("_ddm._tcp", ddm_port),
] {
let records = by_name.get(expected_name).unwrap_or_else(|| {
panic!(
"expected {expected_name} in published switch-zone \
services; got {by_name:?}"
)
});
let srv_port = records
.iter()
.find_map(|r| match r {
DnsRecord::Srv(s) => Some(s.port),
_ => None,
})
.unwrap_or_else(|| {
panic!("no SRV record for {expected_name}: {records:?}")
});

assert_eq!(
srv_port, expected_port,
"wrong SRV port for {expected_name}"
);
}
}

#[test]
fn display_hosts() {
let sled_uuid = SledUuid::nil();
Expand Down
5 changes: 4 additions & 1 deletion internal-dns/types/src/names.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ pub enum ServiceName {
BoundaryNtp,
InternalNtp,
Mgd,
Ddm,
}

impl ServiceName {
Expand Down Expand Up @@ -116,6 +117,7 @@ impl ServiceName {
ServiceName::BoundaryNtp => "boundary-ntp",
ServiceName::InternalNtp => "internal-ntp",
ServiceName::Mgd => "mgd",
ServiceName::Ddm => "ddm",
}
}

Expand Down Expand Up @@ -144,7 +146,8 @@ impl ServiceName {
| ServiceName::CruciblePantry
| ServiceName::BoundaryNtp
| ServiceName::InternalNtp
| ServiceName::Mgd => {
| ServiceName::Mgd
| ServiceName::Ddm => {
format!("_{}._tcp", self.service_kind())
}
ServiceName::SledAgent(id) => {
Expand Down
7 changes: 4 additions & 3 deletions nexus/reconfigurator/execution/src/dns.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -988,9 +988,8 @@ mod test {
// the previous pass (i.e., that corresponds to an Omicron zone).
//
// There are some ServiceNames missing here because they are not part of
// our representative config (e.g., ClickhouseKeeper) or they don't
// currently have DNS record at all (e.g., SledAgent, Maghemite, Mgd,
// Tfport).
// our representative config (e.g., ClickhouseKeeper) or because they
// do not currently have a DNS record at all (e.g., SledAgent).
let mut srv_kinds_expected = BTreeSet::from([
ServiceName::Clickhouse,
ServiceName::ClickhouseNative,
Expand All @@ -1001,6 +1000,8 @@ mod test {
ServiceName::NexusLockstep,
ServiceName::Oximeter,
ServiceName::Dendrite,
ServiceName::Mgd,
ServiceName::Ddm,
ServiceName::CruciblePantry,
ServiceName::BoundaryNtp,
ServiceName::InternalNtp,
Expand Down
2 changes: 2 additions & 0 deletions nexus/reconfigurator/execution/src/test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,12 @@ pub fn overridables_for_test(
let dendrite_port =
cptestctx.dendrite.read().unwrap().get(&switch_slot).unwrap().port;
let mgd_port = cptestctx.mgd.get(&switch_slot).unwrap().port;
let ddm_port = cptestctx.ddm.get(&switch_slot).unwrap().port;
overrides.override_switch_zone_ip(sled_id, ip);
overrides.override_dendrite_port(sled_id, dendrite_port);
overrides.override_mgs_port(sled_id, mgs_port);
overrides.override_mgd_port(sled_id, mgd_port);
overrides.override_ddm_port(sled_id, ddm_port);
}
overrides
}
3 changes: 2 additions & 1 deletion nexus/reconfigurator/planning/src/example.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1854,7 +1854,8 @@ mod tests {
| ServiceName::RepoDepot
| ServiceName::ManagementGatewayService
| ServiceName::Dendrite
| ServiceName::Mgd => {
| ServiceName::Mgd
| ServiceName::Ddm => {
out.insert(service, Ok(()));
}
// InternalNtp is too large to fit in a single DNS packet and
Expand Down
4 changes: 4 additions & 0 deletions nexus/test-utils/src/nexus_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ pub struct ControlPlaneTestContext<N> {
/// Ports of stopped dendrite instances (for use by start_dendrite)
pub stopped_dendrite_ports: RwLock<HashMap<SwitchSlot, u16>>,
pub mgd: HashMap<SwitchSlot, dev::maghemite::MgdInstance>,
pub ddm: HashMap<SwitchSlot, dev::maghemite::DdmInstance>,
pub external_dns_zone_name: String,
pub external_dns: TransientDnsServer,
pub internal_dns: TransientDnsServer,
Expand Down Expand Up @@ -320,6 +321,9 @@ impl<N: NexusServer> ControlPlaneTestContext<N> {
for (_, mut mgd) in self.mgd {
mgd.cleanup().await.unwrap();
}
for (_, mut ddm) in self.ddm {
ddm.cleanup().await;
}
self.logctx.cleanup_successful();
}
}
Expand Down
30 changes: 30 additions & 0 deletions nexus/test-utils/src/starter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ pub struct ControlPlaneStarter<'a, N: NexusServer> {
pub gateway: BTreeMap<SwitchSlot, GatewayTestContext>,
pub dendrite: RwLock<HashMap<SwitchSlot, dev::dendrite::DendriteInstance>>,
pub mgd: HashMap<SwitchSlot, dev::maghemite::MgdInstance>,
pub ddm: HashMap<SwitchSlot, dev::maghemite::DdmInstance>,

// NOTE: Only exists after starting Nexus, until external Nexus is
// initialized.
Expand Down Expand Up @@ -203,6 +204,7 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> {
gateway: BTreeMap::new(),
dendrite: RwLock::new(HashMap::new()),
mgd: HashMap::new(),
ddm: HashMap::new(),
nexus_internal: None,
nexus_internal_addr: None,
external_dns_zone_name: None,
Expand Down Expand Up @@ -461,6 +463,17 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> {
self.config.pkg.mgd.insert(switch_slot, config);
}

pub async fn start_ddm(&mut self, switch_slot: SwitchSlot) {
let log = &self.logctx.log;
debug!(log, "Starting DDM sim"; "switch_slot" => ?switch_slot);

let ddm = dev::maghemite::DdmInstance::start().await.unwrap();
let port = ddm.port;
self.ddm.insert(switch_slot, ddm);

debug!(log, "DDM sim started"; "port" => port);
}

pub async fn record_switch_dns(
&mut self,
sled_id: SledUuid,
Expand All @@ -482,6 +495,7 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> {
self.dendrite.read().unwrap().get(&switch_slot).unwrap().port,
self.gateway.get(&switch_slot).unwrap().port,
self.mgd.get(&switch_slot).unwrap().port,
self.ddm.get(&switch_slot).unwrap().port,
)
.unwrap()
}
Expand Down Expand Up @@ -1250,6 +1264,7 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> {
dendrite: RwLock::new(self.dendrite.into_inner().unwrap()),
stopped_dendrite_ports: RwLock::new(HashMap::new()),
mgd: self.mgd,
ddm: self.ddm,
external_dns_zone_name: self.external_dns_zone_name.unwrap(),
external_dns: self.external_dns.unwrap(),
internal_dns: self.internal_dns.unwrap(),
Expand Down Expand Up @@ -1291,6 +1306,9 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> {
for (_, mut mgd) in self.mgd {
mgd.cleanup().await.unwrap();
}
for (_, mut ddm) in self.ddm {
ddm.cleanup().await;
}
self.logctx.cleanup_successful();
}

Expand Down Expand Up @@ -1631,6 +1649,12 @@ pub(crate) async fn setup_with_config_impl<N: NexusServer>(
builder.start_mgd(SwitchSlot::Switch0).boxed()
}),
),
(
"start_ddm_switch0",
Box::new(|builder| {
builder.start_ddm(SwitchSlot::Switch0).boxed()
}),
),
(
"record_switch_dns",
Box::new(|builder| {
Expand Down Expand Up @@ -1675,6 +1699,12 @@ pub(crate) async fn setup_with_config_impl<N: NexusServer>(
builder.start_mgd(SwitchSlot::Switch1).boxed()
}),
),
(
"start_ddm_switch1",
Box::new(|builder| {
builder.start_ddm(SwitchSlot::Switch1).boxed()
}),
),
(
"record_switch_dns",
Box::new(|builder| {
Expand Down
7 changes: 7 additions & 0 deletions nexus/tests/integration_tests/initialization.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ async fn test_nexus_boots_before_dendrite() {
starter.start_mgd(SwitchSlot::Switch1).await;
info!(log, "Started mgd");

info!(log, "Starting ddm");
starter.start_ddm(SwitchSlot::Switch0).await;
starter.start_ddm(SwitchSlot::Switch1).await;
info!(log, "Started ddm");

info!(log, "Populating internal DNS records");
starter
.record_switch_dns(
Expand Down Expand Up @@ -197,6 +202,8 @@ async fn nexus_schema_test_setup(
starter.start_dendrite(SwitchSlot::Switch1).await;
starter.start_mgd(SwitchSlot::Switch0).await;
starter.start_mgd(SwitchSlot::Switch1).await;
starter.start_ddm(SwitchSlot::Switch0).await;
starter.start_ddm(SwitchSlot::Switch1).await;
starter.populate_internal_dns().await;
}

Expand Down
1 change: 1 addition & 0 deletions nexus/types/src/deployment/execution/dns.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ pub fn blueprint_internal_dns_config(
overrides.dendrite_port(scrimlet.id()),
overrides.mgs_port(scrimlet.id()),
overrides.mgd_port(scrimlet.id()),
overrides.ddm_port(scrimlet.id()),
)?;
}

Expand Down
13 changes: 13 additions & 0 deletions nexus/types/src/deployment/execution/overridables.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use omicron_common::address::DDMD_PORT;
use omicron_common::address::DENDRITE_PORT;
use omicron_common::address::Ipv6Subnet;
use omicron_common::address::MGD_PORT;
Expand Down Expand Up @@ -29,6 +30,8 @@ pub struct Overridables {
pub mgs_ports: BTreeMap<SledUuid, u16>,
/// map: sled id -> TCP port on which that sled's MGD is listening
pub mgd_ports: BTreeMap<SledUuid, u16>,
/// map: sled id -> TCP port on which that sled's DDM is listening
pub ddm_ports: BTreeMap<SledUuid, u16>,
/// map: sled id -> IP address of the sled's switch zone
pub switch_zone_ips: BTreeMap<SledUuid, Ipv6Addr>,
}
Expand Down Expand Up @@ -67,6 +70,16 @@ impl Overridables {
self.mgd_ports.get(&sled_id).copied().unwrap_or(MGD_PORT)
}

/// Specify the TCP port on which this sled's DDM is listening
pub fn override_ddm_port(&mut self, sled_id: SledUuid, port: u16) {
self.ddm_ports.insert(sled_id, port);
}

/// Returns the TCP port on which this sled's DDM is listening
pub fn ddm_port(&self, sled_id: SledUuid) -> u16 {
self.ddm_ports.get(&sled_id).copied().unwrap_or(DDMD_PORT)
}

/// Specify the IP address of this switch zone
pub fn override_switch_zone_ip(
&mut self,
Expand Down
9 changes: 5 additions & 4 deletions sled-agent/rack-setup/src/plan/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ use nexus_types::deployment::{
};
use nexus_types::external_api::sled::SledState;
use omicron_common::address::{
CP_SERVICES_RESERVED_ADDRESSES, DENDRITE_PORT, DNS_HTTP_PORT, DNS_PORT,
Ipv6Subnet, MGD_PORT, MGS_PORT, NEXUS_INTERNAL_PORT, NEXUS_LOCKSTEP_PORT,
NTP_PORT, NUM_SOURCE_NAT_PORTS, REPO_DEPOT_PORT, ReservedRackSubnet,
SLED_PREFIX, SLED_RESERVED_ADDRESSES, get_sled_address,
CP_SERVICES_RESERVED_ADDRESSES, DDMD_PORT, DENDRITE_PORT, DNS_HTTP_PORT,
DNS_PORT, Ipv6Subnet, MGD_PORT, MGS_PORT, NEXUS_INTERNAL_PORT,
NEXUS_LOCKSTEP_PORT, NTP_PORT, NUM_SOURCE_NAT_PORTS, REPO_DEPOT_PORT,
ReservedRackSubnet, SLED_PREFIX, SLED_RESERVED_ADDRESSES, get_sled_address,
get_switch_zone_address,
};
use omicron_common::api::external::{Generation, MacAddr, Vni};
Expand Down Expand Up @@ -341,6 +341,7 @@ impl ServicePlan {
DENDRITE_PORT,
MGS_PORT,
MGD_PORT,
DDMD_PORT,
)
.unwrap();
}
Expand Down
Loading
Loading