Skip to content

Commit 5ead40c

Browse files
[review, ddmd] Address review ~ HostSwitchZonePorts + real ddmd test fixture
We address @jgallagher's review by: - Replacing the four positional `u16` arguments in `DnsConfigBuilder::host_zone_switch` with a `HostSwitchZonePorts` named-fields structure. - Replacing the dropshot-based stubbed `DdmInstance` in test-utils with a fixture that spawns and supervises a real `ddmd` subprocess running with `--no-state-machine`, analogous to `MgdInstance` and `mgd --no-bgp-dispatcher`. Only the switch-zone `ddmd` is registered in internal DNS, while sled-global-zone instances are accessed locally by their own host and don't need DNS registration. This **does** require maghemite changes, already PR'ed to oxidecomputer/maghemite#729. To make this all work, we wire `ddmd` into the developer xtask toolchain. `cargo xtask download maghemite-ddmd` reuses the existing `mg-ddm.tar.gz` illumos zone artifact (extracting `ddmd`/`ddmadm`). On Linux it overlays a raw `ddmd` binary, and on macOS it builds from source. Also, we had to bump `oxnet` from 0.1.4 to 0.1.5 to satisfy the new maghemite pin.
1 parent 042f398 commit 5ead40c

18 files changed

Lines changed: 276 additions & 105 deletions

File tree

.envrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ PATH_add out/cockroachdb/bin
55
PATH_add out/clickhouse
66
PATH_add out/dendrite-stub/bin
77
PATH_add out/mgd/root/opt/oxide/mgd/bin
8+
PATH_add out/mg-ddm/root/opt/oxide/mg-ddm/bin
89

910
if [ "$OMICRON_USE_FLAKE" = 1 ] && nix flake show &> /dev/null
1011
then

Cargo.lock

Lines changed: 8 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -604,8 +604,8 @@ ntp-admin-api = { path = "ntp-admin/api" }
604604
ntp-admin-client = { path = "clients/ntp-admin-client" }
605605
ntp-admin-types = { path = "ntp-admin/types" }
606606
ntp-admin-types-versions = { path = "ntp-admin/types/versions" }
607-
mg-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "7696ee48d5ee29a917dea459e281fe2e8ff20513" }
608-
ddm-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "7696ee48d5ee29a917dea459e281fe2e8ff20513" }
607+
mg-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "3b54e1630e6f75cdc09a2580cec2438bfecade22" }
608+
ddm-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "3b54e1630e6f75cdc09a2580cec2438bfecade22" }
609609
multimap = "0.10.1"
610610
nexus-auth = { path = "nexus/auth" }
611611
nexus-background-task-interface = { path = "nexus/background-task-interface" }
@@ -669,7 +669,7 @@ oxide-client = { path = "clients/oxide-client" }
669669
oxide-tokio-rt = "0.1.4"
670670
oxide-vpc = { git = "https://github.com/oxidecomputer/opte", rev = "bae0440c199b3908c12903a9532854936353433b", features = [ "api", "std" ] }
671671
oxlog = { path = "dev-tools/oxlog" }
672-
oxnet = "0.1.4"
672+
oxnet = "0.1.5"
673673
once_cell = "1.21.3"
674674
openapi-lint = { git = "https://github.com/oxidecomputer/openapi-lint", branch = "main" }
675675
openapiv3 = "2.2.0"
@@ -742,7 +742,7 @@ rats-corim = { git = "https://github.com/oxidecomputer/rats-corim.git", rev = "f
742742
raw-cpuid = { git = "https://github.com/oxidecomputer/rust-cpuid.git", rev = "a4cf01df76f35430ff5d39dc2fe470bcb953503b" }
743743
rayon = "1.10"
744744
rcgen = "0.12.1"
745-
rdb-types = { git = "https://github.com/oxidecomputer/maghemite", rev = "7696ee48d5ee29a917dea459e281fe2e8ff20513" }
745+
rdb-types = { git = "https://github.com/oxidecomputer/maghemite", rev = "3b54e1630e6f75cdc09a2580cec2438bfecade22" }
746746
reconfigurator-cli = { path = "dev-tools/reconfigurator-cli" }
747747
reedline = "0.40.0"
748748
ref-cast = "1.0"

dev-tools/downloader/src/lib.rs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ enum Target {
6969
/// Maghemite mgd binary
7070
MaghemiteMgd,
7171

72+
/// Maghemite ddmd binary
73+
MaghemiteDdmd,
74+
7275
/// SoftNPU, an admin program (scadm) and a pre-compiled P4 program.
7376
Softnpu,
7477

@@ -137,6 +140,7 @@ pub async fn run_cmd(args: DownloadArgs) -> Result<()> {
137140
Target::Console => downloader.download_console().await,
138141
Target::DendriteStub => downloader.download_dendrite_stub().await,
139142
Target::MaghemiteMgd => downloader.download_maghemite_mgd().await,
143+
Target::MaghemiteDdmd => downloader.download_maghemite_ddmd().await,
140144
Target::Softnpu => downloader.download_softnpu().await,
141145
Target::TransceiverControl => {
142146
downloader.download_transceiver_control().await
@@ -946,6 +950,84 @@ impl Downloader<'_> {
946950
Ok(())
947951
}
948952

953+
async fn download_maghemite_ddmd(&self) -> Result<()> {
954+
let download_dir = self.output_dir.join("downloads");
955+
tokio::fs::create_dir_all(&download_dir).await?;
956+
957+
let checksums_path = self.versions_dir.join("maghemite_mgd_checksums");
958+
let [mg_ddm_sha2, ddmd_linux_sha2] = get_values_from_file(
959+
["MG_DDM_SHA256", "DDMD_LINUX_SHA256"],
960+
&checksums_path,
961+
)
962+
.await?;
963+
let commit_path =
964+
self.versions_dir.join("maghemite_ddm_openapi_version");
965+
let [commit] = get_values_from_file(["COMMIT"], &commit_path).await?;
966+
967+
let repo = "oxidecomputer/maghemite";
968+
let base_url = format!("{BUILDOMAT_URL}/{repo}/image/{commit}");
969+
970+
let filename = "mg-ddm.tar.gz";
971+
let tarball_path = download_dir.join(filename);
972+
download_file_and_verify(
973+
&self.log,
974+
&tarball_path,
975+
&format!("{base_url}/{filename}"),
976+
ChecksumAlgorithm::Sha2,
977+
&mg_ddm_sha2,
978+
)
979+
.await?;
980+
unpack_tarball(&self.log, &tarball_path, &download_dir).await?;
981+
982+
let destination_dir = self.output_dir.join("mg-ddm");
983+
let _ = tokio::fs::remove_dir_all(&destination_dir).await;
984+
tokio::fs::create_dir_all(&destination_dir).await?;
985+
copy_dir_all(
986+
&download_dir.join("root"),
987+
&destination_dir.join("root"),
988+
)?;
989+
990+
let binary_dir = destination_dir.join("root/opt/oxide/mg-ddm/bin");
991+
992+
match os_name()? {
993+
Os::Linux => {
994+
let filename = "ddmd";
995+
let path = download_dir.join(filename);
996+
download_file_and_verify(
997+
&self.log,
998+
&path,
999+
&format!(
1000+
"{BUILDOMAT_URL}/{repo}/linux/{commit}/{filename}"
1001+
),
1002+
ChecksumAlgorithm::Sha2,
1003+
&ddmd_linux_sha2,
1004+
)
1005+
.await?;
1006+
set_permissions(&path, 0o755).await?;
1007+
tokio::fs::copy(path, binary_dir.join(filename)).await?;
1008+
}
1009+
Os::Mac => {
1010+
info!(
1011+
self.log,
1012+
"Building maghemite ddmd from source for macOS"
1013+
);
1014+
1015+
let binaries = [("ddmd", &["--no-default-features"][..])];
1016+
1017+
let built_binaries = self
1018+
.build_from_git("maghemite", &commit, &binaries)
1019+
.await?;
1020+
1021+
let dest = binary_dir.join("ddmd");
1022+
tokio::fs::copy(&built_binaries[0], &dest).await?;
1023+
set_permissions(&dest, 0o755).await?;
1024+
}
1025+
Os::Illumos => (),
1026+
}
1027+
1028+
Ok(())
1029+
}
1030+
9491031
async fn download_softnpu(&self) -> Result<()> {
9501032
let destination_dir = self.output_dir.join("npuzone");
9511033
tokio::fs::create_dir_all(&destination_dir).await?;

env.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export PATH="$OMICRON_WS/out/cockroachdb/bin:$PATH"
1212
export PATH="$OMICRON_WS/out/clickhouse:$PATH"
1313
export PATH="$OMICRON_WS/out/dendrite-stub/bin:$PATH"
1414
export PATH="$OMICRON_WS/out/mgd/root/opt/oxide/mgd/bin:$PATH"
15+
export PATH="$OMICRON_WS/out/mg-ddm/root/opt/oxide/mg-ddm/bin:$PATH"
1516

1617
# if xtrace was set previously, do not unset it
1718
case $OLD_SHELL_OPTS in

internal-dns/types/src/config.rs

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,20 @@ pub struct DnsConfigBuilder {
163163
service_instances_sleds: BTreeMap<ServiceName, BTreeMap<Sled, u16>>,
164164
}
165165

166+
/// Ports for the per-switch services published in internal DNS by
167+
/// [`DnsConfigBuilder::host_zone_switch`].
168+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
169+
pub struct HostSwitchZonePorts {
170+
/// Dendrite (`dpd`) admin API port.
171+
pub dendrite: u16,
172+
/// Management Gateway Service (`mgs`) port.
173+
pub mgs: u16,
174+
/// Maghemite `mgd` admin API port.
175+
pub mgd: u16,
176+
/// Maghemite `ddmd` admin API port.
177+
pub ddm: u16,
178+
}
179+
166180
/// Describes a host of type "sled" in the control plane DNS zone
167181
#[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
168182
pub struct Sled(SledUuid);
@@ -396,11 +410,14 @@ impl DnsConfigBuilder {
396410
&mut self,
397411
sled_id: SledUuid,
398412
switch_zone_ip: Ipv6Addr,
399-
dendrite_port: u16,
400-
mgs_port: u16,
401-
mgd_port: u16,
402-
ddm_port: u16,
413+
ports: HostSwitchZonePorts,
403414
) -> anyhow::Result<()> {
415+
let HostSwitchZonePorts {
416+
dendrite: dendrite_port,
417+
mgs: mgs_port,
418+
mgd: mgd_port,
419+
ddm: ddm_port,
420+
} = ports;
404421
let zone = self.host_dendrite(sled_id, switch_zone_ip)?;
405422
self.service_backend_zone(ServiceName::Dendrite, &zone, dendrite_port)?;
406423
self.service_backend_zone(
@@ -733,7 +750,9 @@ impl DnsConfigBuilder {
733750

734751
#[cfg(test)]
735752
mod test {
736-
use super::{DnsConfigBuilder, DnsRecord, Host, ServiceName};
753+
use super::{
754+
DnsConfigBuilder, DnsRecord, Host, HostSwitchZonePorts, ServiceName,
755+
};
737756
use crate::{config::Zone, names::DNS_ZONE};
738757
use omicron_common::api::external::Generation;
739758
use omicron_uuid_kinds::{OmicronZoneUuid, SledUuid};
@@ -818,10 +837,12 @@ mod test {
818837
.host_zone_switch(
819838
sled_uuid,
820839
switch_zone_ip,
821-
dendrite_port,
822-
mgs_port,
823-
mgd_port,
824-
ddm_port,
840+
HostSwitchZonePorts {
841+
dendrite: dendrite_port,
842+
mgs: mgs_port,
843+
mgd: mgd_port,
844+
ddm: ddm_port,
845+
},
825846
)
826847
.unwrap();
827848

nexus/test-utils/src/nexus_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ impl<N: NexusServer> ControlPlaneTestContext<N> {
322322
mgd.cleanup().await.unwrap();
323323
}
324324
for (_, mut ddm) in self.ddm {
325-
ddm.cleanup().await;
325+
ddm.cleanup().await.unwrap();
326326
}
327327
self.logctx.cleanup_successful();
328328
}

nexus/test-utils/src/starter.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use futures::future::BoxFuture;
2323
use gateway_test_utils::setup::GatewayTestContext;
2424
use iddqd::IdOrdMap;
2525
use internal_dns_types::config::DnsConfigBuilder;
26+
use internal_dns_types::config::HostSwitchZonePorts;
2627
use internal_dns_types::names::DNS_ZONE_EXTERNAL_TESTING;
2728
use internal_dns_types::names::ServiceName;
2829
use nexus_config::Database;
@@ -492,10 +493,18 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> {
492493
.host_zone_switch(
493494
sled_id,
494495
Ipv6Addr::LOCALHOST,
495-
self.dendrite.read().unwrap().get(&switch_slot).unwrap().port,
496-
self.gateway.get(&switch_slot).unwrap().port,
497-
self.mgd.get(&switch_slot).unwrap().port,
498-
self.ddm.get(&switch_slot).unwrap().port,
496+
HostSwitchZonePorts {
497+
dendrite: self
498+
.dendrite
499+
.read()
500+
.unwrap()
501+
.get(&switch_slot)
502+
.unwrap()
503+
.port,
504+
mgs: self.gateway.get(&switch_slot).unwrap().port,
505+
mgd: self.mgd.get(&switch_slot).unwrap().port,
506+
ddm: self.ddm.get(&switch_slot).unwrap().port,
507+
},
499508
)
500509
.unwrap()
501510
}
@@ -1307,7 +1316,7 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> {
13071316
mgd.cleanup().await.unwrap();
13081317
}
13091318
for (_, mut ddm) in self.ddm {
1310-
ddm.cleanup().await;
1319+
ddm.cleanup().await.unwrap();
13111320
}
13121321
self.logctx.cleanup_successful();
13131322
}

nexus/types/src/deployment/execution/dns.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,7 @@ pub fn blueprint_internal_dns_config(
155155
dns_builder.host_zone_switch(
156156
scrimlet.id(),
157157
switch_zone_ip,
158-
overrides.dendrite_port(scrimlet.id()),
159-
overrides.mgs_port(scrimlet.id()),
160-
overrides.mgd_port(scrimlet.id()),
161-
overrides.ddm_port(scrimlet.id()),
158+
overrides.host_switch_zone_ports(scrimlet.id()),
162159
)?;
163160
}
164161

nexus/types/src/deployment/execution/overridables.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// License, v. 2.0. If a copy of the MPL was not distributed with this
33
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
44

5+
use internal_dns_types::config::HostSwitchZonePorts;
56
use omicron_common::address::DDMD_PORT;
67
use omicron_common::address::DENDRITE_PORT;
78
use omicron_common::address::Ipv6Subnet;
@@ -80,6 +81,22 @@ impl Overridables {
8081
self.ddm_ports.get(&sled_id).copied().unwrap_or(DDMD_PORT)
8182
}
8283

84+
/// Returns the per-switch-zone service ports for this sled.
85+
///
86+
/// Bundles the four switch-zone admin ports into a single
87+
/// [`HostSwitchZonePorts`] so callers cannot swap fields by accident.
88+
pub fn host_switch_zone_ports(
89+
&self,
90+
sled_id: SledUuid,
91+
) -> HostSwitchZonePorts {
92+
HostSwitchZonePorts {
93+
dendrite: self.dendrite_port(sled_id),
94+
mgs: self.mgs_port(sled_id),
95+
mgd: self.mgd_port(sled_id),
96+
ddm: self.ddm_port(sled_id),
97+
}
98+
}
99+
83100
/// Specify the IP address of this switch zone
84101
pub fn override_switch_zone_ip(
85102
&mut self,

0 commit comments

Comments
 (0)