From 67d0cbd0747cac6338caef2c624fa6b8802c15a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karen=20C=C3=A1rcamo?= Date: Mon, 22 Jul 2024 13:31:08 +1200 Subject: [PATCH] [sled-agent] Self assembling switch zone (#5593) ## Overview This PR migrates the switch zone to a self assembling format. There are a few bits of old code I'll be cleaning up, more logs I'll be adding and some documentation about how the switch zone flow works, but I'll do this in follow up PRs to keep this one as compact as possible. ## Caveats I've tested this in a local single node deployment and in the a4x2 testbed. Unfortunately, this is not enough testing to make sure all of the services play nice together on a real rack. We'll have to keep an eye on dogfood when this is deployed. The only services that depend on the [common networking](https://github.com/oxidecomputer/omicron/blob/30eb1ee38987201ac71f2115fdd89da4b08710c7/zone-setup/src/bin/zone-setup.rs#L578-L690) [service](https://github.com/oxidecomputer/omicron/blob/30eb1ee38987201ac71f2115fdd89da4b08710c7/smf/zone-network-setup/manifest.xml) are dendrite and MGS. While this makes sense on the a4x2 testbed, I'd like to verify that these dependencies make sense when running on a real rack. As several people have worked on different parts of this zone, I've tagged a whole bunch of people for review, sorry if this is overkill! Just want to make sure I've got the right eyes on each service of the zone. Related: https://github.com/oxidecomputer/omicron/issues/1898 Closes: https://github.com/oxidecomputer/omicron/issues/2884 TODO: - [x] Update Dendrite hashes after merging https://github.com/oxidecomputer/dendrite/pull/990 --- Cargo.lock | 3 + illumos-utils/src/lib.rs | 4 +- illumos-utils/src/route.rs | 19 + illumos-utils/src/running_zone.rs | 20 +- illumos-utils/src/zone.rs | 2 +- package-manifest.toml | 19 +- sled-agent/src/params.rs | 3 - sled-agent/src/profile.rs | 13 +- sled-agent/src/services.rs | 1044 +++++++++++++---------- sled-agent/src/smf_helper.rs | 89 +- smf/mgs/manifest.xml | 7 +- smf/sp-sim/manifest.xml | 2 +- smf/switch_zone_setup/manifest.xml | 13 +- smf/switch_zone_setup/switch_zone_setup | 49 -- smf/wicketd/manifest.xml | 8 +- smf/zone-network-setup/manifest.xml | 2 +- tools/dendrite_openapi_version | 2 +- tools/dendrite_stub_checksums | 4 +- zone-setup/Cargo.toml | 9 +- zone-setup/src/bin/zone-setup.rs | 352 +++++++- zone-setup/src/lib.rs | 5 + zone-setup/src/switch_zone_user.rs | 304 +++++++ 22 files changed, 1367 insertions(+), 606 deletions(-) delete mode 100755 smf/switch_zone_setup/switch_zone_setup create mode 100644 zone-setup/src/lib.rs create mode 100644 zone-setup/src/switch_zone_user.rs diff --git a/Cargo.lock b/Cargo.lock index 8e796fde7b..56bcc9c2cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11696,7 +11696,10 @@ dependencies = [ "dropshot", "illumos-utils", "omicron-common", + "omicron-sled-agent", "omicron-workspace-hack", + "serde_json", + "sled-hardware-types", "slog", "tokio", "uzers", diff --git a/illumos-utils/src/lib.rs b/illumos-utils/src/lib.rs index d041c866b0..03b4bfb5a7 100644 --- a/illumos-utils/src/lib.rs +++ b/illumos-utils/src/lib.rs @@ -36,8 +36,8 @@ pub const PFEXEC: &str = "/usr/bin/pfexec"; pub struct CommandFailureInfo { command: String, status: std::process::ExitStatus, - stdout: String, - stderr: String, + pub stdout: String, + pub stderr: String, } impl std::fmt::Display for CommandFailureInfo { diff --git a/illumos-utils/src/route.rs b/illumos-utils/src/route.rs index ceff2b3d9e..12f74bfd78 100644 --- a/illumos-utils/src/route.rs +++ b/illumos-utils/src/route.rs @@ -107,4 +107,23 @@ impl Route { }; Ok(()) } + + pub fn add_bootstrap_route( + bootstrap_prefix: u16, + gz_bootstrap_addr: Ipv6Addr, + zone_vnic_name: &str, + ) -> Result<(), ExecutionError> { + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[ + ROUTE, + "add", + "-inet6", + &format!("{bootstrap_prefix:x}::/16"), + &gz_bootstrap_addr.to_string(), + "-ifp", + zone_vnic_name, + ]); + execute(cmd)?; + Ok(()) + } } diff --git a/illumos-utils/src/running_zone.rs b/illumos-utils/src/running_zone.rs index 64bbb91cbd..ea24a6f502 100644 --- a/illumos-utils/src/running_zone.rs +++ b/illumos-utils/src/running_zone.rs @@ -46,7 +46,7 @@ pub enum ServiceError { pub struct RunCommandError { zone: String, #[source] - err: crate::ExecutionError, + pub err: crate::ExecutionError, } /// Errors returned from [`RunningZone::boot`]. @@ -462,7 +462,7 @@ impl RunningZone { /// Note that the zone must already be configured to be booted. pub async fn boot(zone: InstalledZone) -> Result { // Boot the zone. - info!(zone.log, "Zone booting"); + info!(zone.log, "Booting {} zone", zone.name); Zones::boot(&zone.name).await?; @@ -480,6 +480,9 @@ impl RunningZone { zone: zone.name.to_string(), })?; + // TODO https://github.com/oxidecomputer/omicron/issues/1898: + // Remove all non-self assembling code + // If the zone is self-assembling, then SMF service(s) inside the zone // will be creating the listen address for the zone's service(s), // setting the appropriate ifprop MTU, and so on. The idea behind @@ -575,7 +578,6 @@ impl RunningZone { &self, address: Ipv6Addr, ) -> Result<(), EnsureAddressError> { - info!(self.inner.log, "Adding bootstrap address"); let vnic = self.inner.bootstrap_vnic.as_ref().ok_or_else(|| { EnsureAddressError::MissingBootstrapVnic { address: address.to_string(), @@ -735,7 +737,7 @@ impl RunningZone { gz_bootstrap_addr: Ipv6Addr, zone_vnic_name: &str, ) -> Result<(), RunCommandError> { - self.run_cmd([ + let args = [ "/usr/sbin/route", "add", "-inet6", @@ -743,7 +745,8 @@ impl RunningZone { &gz_bootstrap_addr.to_string(), "-ifp", zone_vnic_name, - ])?; + ]; + self.run_cmd(args)?; Ok(()) } @@ -775,7 +778,7 @@ impl RunningZone { /// Return a reference to the links for this zone. pub fn links(&self) -> &Vec { - &self.inner.links + &self.inner.links() } /// Return a mutable reference to the links for this zone. @@ -1010,6 +1013,11 @@ impl InstalledZone { pub fn root(&self) -> Utf8PathBuf { self.zonepath.path.join(Self::ROOT_FS_PATH) } + + /// Return a reference to the links for this zone. + pub fn links(&self) -> &Vec { + &self.links + } } #[derive(Clone)] diff --git a/illumos-utils/src/zone.rs b/illumos-utils/src/zone.rs index 7ba40af043..47cc84dce6 100644 --- a/illumos-utils/src/zone.rs +++ b/illumos-utils/src/zone.rs @@ -640,7 +640,7 @@ impl Zones { // // Does NOT check if the address already exists. #[allow(clippy::needless_lifetimes)] - fn create_address_internal<'a>( + pub fn create_address_internal<'a>( zone: Option<&'a str>, addrobj: &AddrObject, addrtype: AddressRequest, diff --git a/package-manifest.toml b/package-manifest.toml index 29fb7c5da8..098e15d3b8 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -439,7 +439,6 @@ service_name = "switch_zone_setup" source.type = "local" source.paths = [ { from = "smf/switch_zone_setup/manifest.xml", to = "/var/svc/manifest/site/switch_zone_setup/manifest.xml" }, - { from = "smf/switch_zone_setup/switch_zone_setup", to = "/opt/oxide/bin/switch_zone_setup" }, { from = "smf/switch_zone_setup/support_authorized_keys", to = "/opt/oxide/support/authorized_keys" }, { from = "/opt/ooce/pgsql-13/lib/amd64", to = "/opt/ooce/pgsql-13/lib/amd64" }, ] @@ -645,8 +644,8 @@ only_for_targets.image = "standard" # the other `source.*` keys. source.type = "prebuilt" source.repo = "dendrite" -source.commit = "e83f4f164fd3dbb2100989a399a4fa087232ac36" -source.sha256 = "b28247df4d301540b0a46e4d9fdf410ee6fbdb23d18c80acbd36c016a084e30e" +source.commit = "fb571dc6512b24a777c5a9b2927a50501f6be297" +source.sha256 = "c7971efca6500cee8edf2696ec6b38014af82bacfe88a0e583bb9bb3a591bc8d" output.type = "zone" output.intermediate_only = true @@ -672,8 +671,8 @@ only_for_targets.image = "standard" # the other `source.*` keys. source.type = "prebuilt" source.repo = "dendrite" -source.commit = "e83f4f164fd3dbb2100989a399a4fa087232ac36" -source.sha256 = "caf988e39d800bdccb1b9423568a19ba10a79aa2b07f74bf7eb65589fd81f8b1" +source.commit = "fb571dc6512b24a777c5a9b2927a50501f6be297" +source.sha256 = "0a96670ce203bce7bed6a0e40842d319c2b4b8ee1a2e9210d3713423f8bd00b1" output.type = "zone" output.intermediate_only = true @@ -692,8 +691,8 @@ only_for_targets.image = "standard" # the other `source.*` keys. source.type = "prebuilt" source.repo = "dendrite" -source.commit = "e83f4f164fd3dbb2100989a399a4fa087232ac36" -source.sha256 = "378a2f32c1850a5a62fa9b320813e342a647647d2f014ab5eced7c2d1d4f9c95" +source.commit = "fb571dc6512b24a777c5a9b2927a50501f6be297" +source.sha256 = "a5bda6b899bff23fccd4dd74224fd1bc44703741054b50552921efa7470cb11a" output.type = "zone" output.intermediate_only = true @@ -740,6 +739,8 @@ source.packages = [ "switch_zone_setup.tar.gz", "xcvradm.tar.gz", "omicron-omdb.tar.gz", + "zone-setup.tar.gz", + "zone-network-install.tar.gz" ] output.type = "zone" @@ -764,6 +765,8 @@ source.packages = [ "switch_zone_setup.tar.gz", "sp-sim-stub.tar.gz", "omicron-omdb.tar.gz", + "zone-setup.tar.gz", + "zone-network-install.tar.gz" ] output.type = "zone" @@ -788,6 +791,8 @@ source.packages = [ "switch_zone_setup.tar.gz", "sp-sim-softnpu.tar.gz", "omicron-omdb.tar.gz", + "zone-setup.tar.gz", + "zone-network-install.tar.gz" ] output.type = "zone" diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index 465a4abb56..4bddbeaff8 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -597,9 +597,6 @@ impl crate::smf_helper::Service for OmicronZoneType { fn smf_name(&self) -> String { format!("svc:/oxide/{}", self.service_name()) } - fn should_import(&self) -> bool { - true - } } impl From for sled_agent_client::types::OmicronZoneType { diff --git a/sled-agent/src/profile.rs b/sled-agent/src/profile.rs index 33e30d1d7b..a30c15acfc 100644 --- a/sled-agent/src/profile.rs +++ b/sled-agent/src/profile.rs @@ -163,6 +163,7 @@ impl Display for ServiceInstanceBuilder { } } +#[derive(Clone)] pub struct PropertyGroupBuilder { name: String, /// names of the properties that were added, in the order they were added @@ -233,7 +234,7 @@ impl Display for PropertyGroupBuilder { if values.len() == 1 { write!( f, - r#" + r#" "#, name = property_name, value = &values[0], @@ -302,7 +303,7 @@ mod tests { - + "#, @@ -384,7 +385,7 @@ mod tests { - + @@ -429,11 +430,11 @@ mod tests { - - + + - + diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 11a60e5d0e..943ff44e06 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -93,7 +93,6 @@ use rand::prelude::SliceRandom; use sled_hardware::is_gimlet; use sled_hardware::underlay; use sled_hardware::SledMode; -use sled_hardware_types::underlay::BOOTSTRAP_PREFIX; use sled_hardware_types::Baseboard; use sled_storage::config::MountConfig; use sled_storage::dataset::{ @@ -119,6 +118,7 @@ use illumos_utils::zone::MockZones as Zones; use illumos_utils::zone::Zones; const IPV6_UNSPECIFIED: IpAddr = IpAddr::V6(Ipv6Addr::UNSPECIFIED); +pub const SWITCH_ZONE_BASEBOARD_FILE: &str = "/opt/oxide/baseboard.json"; #[derive(thiserror::Error, Debug, slog_error_chain::SlogInlineError)] pub enum Error { @@ -514,9 +514,6 @@ impl crate::smf_helper::Service for SwitchService { fn smf_name(&self) -> String { format!("svc:/oxide/{}", self.service_name()) } - fn should_import(&self) -> bool { - true - } } /// Combines the generic `SwitchZoneConfig` with other locally-determined @@ -1341,19 +1338,33 @@ impl ServiceManager { } fn zone_network_setup_install( - gw_addr: &Ipv6Addr, + gw_addr: Option<&Ipv6Addr>, zone: &InstalledZone, - static_addr: &Ipv6Addr, + static_addrs: &[Ipv6Addr], ) -> Result { let datalink = zone.get_control_vnic_name(); - let gateway = &gw_addr.to_string(); - let static_addr = &static_addr.to_string(); let mut config_builder = PropertyGroupBuilder::new("config"); - config_builder = config_builder - .add_property("datalink", "astring", datalink) - .add_property("gateway", "astring", gateway) - .add_property("static_addr", "astring", static_addr); + config_builder = + config_builder.add_property("datalink", "astring", datalink); + + // The switch zone is the only zone that will sometimes have an + // unknown underlay address at zone boot on the first scrimlet. + if let Some(gateway) = gw_addr { + config_builder = config_builder.add_property( + "gateway", + "astring", + gateway.to_string(), + ); + } + + for s in static_addrs { + config_builder = config_builder.add_property( + "static_addr", + "astring", + &s.to_string(), + ); + } Ok(ServiceBuilder::new("oxide/zone-network-setup") .add_property_group(config_builder) @@ -1514,13 +1525,13 @@ impl ServiceManager { return Err(Error::SledAgentNotReady); }; - let listen_addr = underlay_address; + let listen_addr = *underlay_address; let listen_port = &CLICKHOUSE_PORT.to_string(); let nw_setup_service = Self::zone_network_setup_install( - &info.underlay_address, + Some(&info.underlay_address), &installed_zone, - listen_addr, + &[listen_addr], )?; let dns_service = Self::dns_install(info, None, &None).await?; @@ -1567,13 +1578,13 @@ impl ServiceManager { return Err(Error::SledAgentNotReady); }; - let listen_addr = underlay_address; + let listen_addr = *underlay_address; let listen_port = &CLICKHOUSE_KEEPER_PORT.to_string(); let nw_setup_service = Self::zone_network_setup_install( - &info.underlay_address, + Some(&info.underlay_address), &installed_zone, - listen_addr, + &[listen_addr], )?; let dns_service = Self::dns_install(info, None, &None).await?; @@ -1635,9 +1646,9 @@ impl ServiceManager { .to_string(); let nw_setup_service = Self::zone_network_setup_install( - &info.underlay_address, + Some(&info.underlay_address), &installed_zone, - &crdb_listen_ip, + &[crdb_listen_ip], )?; let dns_service = Self::dns_install(info, None, &None).await?; @@ -1696,13 +1707,13 @@ impl ServiceManager { let Some(info) = self.inner.sled_info.get() else { return Err(Error::SledAgentNotReady); }; - let listen_addr = &underlay_address; + let listen_addr = *underlay_address; let listen_port = &CRUCIBLE_PORT.to_string(); let nw_setup_service = Self::zone_network_setup_install( - &info.underlay_address, + Some(&info.underlay_address), &installed_zone, - listen_addr, + &[listen_addr], )?; let dataset_name = DatasetName::new( @@ -1755,13 +1766,13 @@ impl ServiceManager { return Err(Error::SledAgentNotReady); }; - let listen_addr = &underlay_address; + let listen_addr = *underlay_address; let listen_port = &CRUCIBLE_PANTRY_PORT.to_string(); let nw_setup_service = Self::zone_network_setup_install( - &info.underlay_address, + Some(&info.underlay_address), &installed_zone, - listen_addr, + &[listen_addr], )?; let config = PropertyGroupBuilder::new("config") @@ -1811,9 +1822,9 @@ impl ServiceManager { ); let nw_setup_service = Self::zone_network_setup_install( - &info.underlay_address, + Some(&info.underlay_address), &installed_zone, - underlay_address, + &[*underlay_address], )?; let oximeter_config = PropertyGroupBuilder::new("config") @@ -1852,11 +1863,10 @@ impl ServiceManager { }; let nw_setup_service = Self::zone_network_setup_install( - &info.underlay_address, + Some(&info.underlay_address), &installed_zone, - underlay_address, + &[*underlay_address], )?; - // Like Nexus, we need to be reachable externally via // `dns_address` but we don't listen on that address // directly but instead on a VPC private IP. OPTE will @@ -1940,9 +1950,9 @@ impl ServiceManager { }; let nw_setup_service = Self::zone_network_setup_install( - &info.underlay_address, + Some(&info.underlay_address), &installed_zone, - underlay_address, + &[*underlay_address], )?; let is_boundary = matches!( @@ -2033,9 +2043,9 @@ impl ServiceManager { .. }) => { let nw_setup_service = Self::zone_network_setup_install( - gz_address, + Some(gz_address), &installed_zone, - underlay_address, + &[*underlay_address], )?; // Internal DNS zones require a special route through @@ -2117,9 +2127,9 @@ impl ServiceManager { }; let nw_setup_service = Self::zone_network_setup_install( - &info.underlay_address, + Some(&info.underlay_address), &installed_zone, - underlay_address, + &[*underlay_address], )?; // While Nexus will be reachable via `external_ip`, it @@ -2244,191 +2254,134 @@ impl ServiceManager { })?; return Ok(RunningZone::boot(installed_zone).await?); } - _ => {} - } - - let running_zone = RunningZone::boot(installed_zone).await?; - - for (link, needs_link_local) in - running_zone.links().iter().zip(links_need_link_local) - { - if needs_link_local { - info!( - self.inner.log, - "Ensuring {}/{} exists in zone", - link.name(), - IPV6_LINK_LOCAL_NAME - ); - Zones::ensure_has_link_local_v6_address( - Some(running_zone.name()), - &AddrObject::new(link.name(), IPV6_LINK_LOCAL_NAME) - .unwrap(), - )?; - } - } - - if let Some((bootstrap_name, bootstrap_address)) = - bootstrap_name_and_address.as_ref() - { - info!( - self.inner.log, - "Ensuring bootstrap address {} exists in {} zone", - bootstrap_address.to_string(), - &zone_type_str, - ); - running_zone.ensure_bootstrap_address(*bootstrap_address).await?; - info!( - self.inner.log, - "Forwarding bootstrap traffic via {} to {}", - bootstrap_name, - self.inner.global_zone_bootstrap_link_local_address, - ); - running_zone - .add_bootstrap_route( - BOOTSTRAP_PREFIX, - self.inner.global_zone_bootstrap_link_local_address, - bootstrap_name, - ) - .map_err(|err| Error::ZoneCommand { - intent: "add bootstrap network route".to_string(), - err, - })?; - } - - let addresses = match &request { - ZoneArgs::Omicron(OmicronZoneConfigLocal { - zone: OmicronZoneConfig { underlay_address, .. }, + ZoneArgs::Switch(SwitchZoneConfigLocal { + zone: SwitchZoneConfig { id, services, addresses }, .. - }) => std::slice::from_ref(underlay_address), - ZoneArgs::Switch(req) => &req.zone.addresses, - }; - for addr in addresses { - if *addr == Ipv6Addr::LOCALHOST { - continue; - } - info!( - self.inner.log, - "Ensuring address {} exists", - addr.to_string() - ); - let addr_request = - AddressRequest::new_static(IpAddr::V6(*addr), None); - running_zone.ensure_address(addr_request).await?; - info!( - self.inner.log, - "Ensuring address {} exists - OK", - addr.to_string() - ); - } + }) => { + let info = self.inner.sled_info.get(); - let maybe_gateway = if let Some(info) = self.inner.sled_info.get() { - // Only consider a route to the sled's underlay address if the - // underlay is up. - let sled_underlay_subnet = - Ipv6Subnet::::new(info.underlay_address); + let gw_addr = match info { + Some(i) => Some(&i.underlay_address), + None => None, + }; - if addresses - .iter() - .any(|ip| sled_underlay_subnet.net().contains(*ip)) - { - // If the underlay is up, provide a route to it through an - // existing address in the Zone on the same subnet. - info!(self.inner.log, "Zone using sled underlay as gateway"); - Some(info.underlay_address) - } else { - // If no such address exists in the sled's subnet, don't route - // to anything. - info!( - self.inner.log, - "Zone not using gateway (even though underlay is up)" - ); - None - } - } else { - // If the underlay doesn't exist, no routing occurs. - info!( - self.inner.log, - "Zone not using gateway (underlay is not up)" - ); - None - }; + let nw_setup_service = Self::zone_network_setup_install( + gw_addr, + &installed_zone, + addresses, + )?; - let sidecar_revision = match &self.inner.sidecar_revision { - SidecarRevision::Physical(rev) => rev.to_string(), - SidecarRevision::SoftZone(rev) - | SidecarRevision::SoftPropolis(rev) => format!( - "softnpu_front_{}_rear_{}", - rev.front_port_count, rev.rear_port_count - ), - }; + let sidecar_revision = match &self.inner.sidecar_revision { + SidecarRevision::Physical(rev) => rev.to_string(), + SidecarRevision::SoftZone(rev) + | SidecarRevision::SoftPropolis(rev) => format!( + "softnpu_front_{}_rear_{}", + rev.front_port_count, rev.rear_port_count + ), + }; - if let Some(gateway) = maybe_gateway { - running_zone.add_default_route(gateway).map_err(|err| { - Error::ZoneCommand { intent: "Adding Route".to_string(), err } - })?; - } + // Define all services in the switch zone + let mut mgs_service = ServiceBuilder::new("oxide/mgs"); + let mut wicketd_service = ServiceBuilder::new("oxide/wicketd"); + let mut switch_zone_setup_service = + ServiceBuilder::new("oxide/switch_zone_setup"); + let mut dendrite_service = + ServiceBuilder::new("oxide/dendrite"); + let mut tfport_service = ServiceBuilder::new("oxide/tfport"); + let mut lldpd_service = ServiceBuilder::new("oxide/lldpd"); + let mut pumpkind_service = + ServiceBuilder::new("oxide/pumpkind"); + let mut mgd_service = ServiceBuilder::new("oxide/mgd"); + let mut mg_ddm_service = ServiceBuilder::new("oxide/mg-ddm"); + let mut uplink_service = ServiceBuilder::new("oxide/uplink"); + + let mut switch_zone_setup_config = + PropertyGroupBuilder::new("config").add_property( + "gz_local_link_addr", + "astring", + &format!( + "{}", + self.inner.global_zone_bootstrap_link_local_address + ), + ); - match &request { - ZoneArgs::Omicron(zone_config) => { - match &zone_config.zone.zone_type { - OmicronZoneType::BoundaryNtp { .. } - | OmicronZoneType::Clickhouse { .. } - | OmicronZoneType::ClickhouseKeeper { .. } - | OmicronZoneType::CockroachDb { .. } - | OmicronZoneType::Crucible { .. } - | OmicronZoneType::CruciblePantry { .. } - | OmicronZoneType::ExternalDns { .. } - | OmicronZoneType::InternalDns { .. } - | OmicronZoneType::InternalNtp { .. } - | OmicronZoneType::Nexus { .. } - | OmicronZoneType::Oximeter { .. } => { - panic!( - "{} is a service which exists as part of a \ - self-assembling zone", - &zone_config.zone.zone_type.zone_type_str(), - ) + for (link, needs_link_local) in + installed_zone.links().iter().zip(links_need_link_local) + { + if needs_link_local { + switch_zone_setup_config = switch_zone_setup_config + .add_property( + "link_local_links", + "astring", + link.name(), + ); } - }; - } - ZoneArgs::Switch(request) => { - for service in &request.zone.services { - // TODO: Related to - // https://github.com/oxidecomputer/omicron/pull/1124 , should we - // avoid importing this manifest? - debug!(self.inner.log, "importing manifest"); + } - let smfh = SmfHelper::new(&running_zone, service); - smfh.import_manifest()?; + if let Some((bootstrap_name, bootstrap_address)) = + bootstrap_name_and_address.as_ref() + { + switch_zone_setup_config = switch_zone_setup_config + .add_property( + "link_local_links", + "astring", + bootstrap_name, + ) + .add_property( + "bootstrap_addr", + "astring", + &format!("{bootstrap_address}"), + ) + .add_property( + "bootstrap_vnic", + "astring", + bootstrap_name, + ); + } + // Set properties for each service + for service in services { match service { SwitchService::ManagementGatewayService => { info!(self.inner.log, "Setting up MGS service"); - smfh.setprop("config/id", request.zone.id)?; - - // Always tell MGS to listen on localhost so wicketd - // can contact it even before we have an underlay - // network. - smfh.addpropvalue( - "config/address", - &format!("[::1]:{MGS_PORT}"), - )?; + let mut mgs_config = + PropertyGroupBuilder::new("config") + // Always tell MGS to listen on localhost so wicketd + // can contact it even before we have an underlay + // network. + .add_property( + "address", + "astring", + &format!("[::1]:{MGS_PORT}"), + ) + .add_property( + "id", + "astring", + &id.to_string(), + ); + + if let Some(i) = info { + mgs_config = mgs_config.add_property( + "rack_id", + "astring", + &i.rack_id.to_string(), + ); + } - if let Some(address) = request.zone.addresses.get(0) - { + if let Some(address) = addresses.get(0) { // Don't use localhost twice if *address != Ipv6Addr::LOCALHOST { - smfh.addpropvalue( - "config/address", + mgs_config = mgs_config.add_property( + "address", + "astring", &format!("[{address}]:{MGS_PORT}"), - )?; + ); } } - - if let Some(info) = self.inner.sled_info.get() { - smfh.setprop("config/rack_id", info.rack_id)?; - } - - smfh.refresh()?; + mgs_service = mgs_service.add_instance( + ServiceInstanceBuilder::new("default") + .add_property_group(mgs_config), + ); } SwitchService::SpSim => { info!( @@ -2438,12 +2391,6 @@ impl ServiceManager { } SwitchService::Wicketd { baseboard } => { info!(self.inner.log, "Setting up wicketd service"); - - smfh.setprop( - "config/address", - &format!("[::1]:{WICKETD_PORT}"), - )?; - // If we're launching the switch zone, we'll have a // bootstrap_address based on our call to // `self.bootstrap_address_needed` (which always @@ -2464,103 +2411,111 @@ impl ServiceManager { .to_string(), }); }; - smfh.setprop( - "config/artifact-address", - &format!( - "[{bootstrap_address}]:{BOOTSTRAP_ARTIFACT_PORT}" - ), - )?; - smfh.setprop( - "config/mgs-address", - &format!("[::1]:{MGS_PORT}"), - )?; - - // We intentionally bind `nexus-proxy-address` to - // `::` so wicketd will serve this on all - // interfaces, particularly the tech port - // interfaces, allowing external clients to connect - // to this Nexus proxy. - smfh.setprop( - "config/nexus-proxy-address", - &format!("[::]:{WICKETD_NEXUS_PROXY_PORT}"), - )?; - if let Some(underlay_address) = self - .inner - .sled_info - .get() - .map(|info| info.underlay_address) - { + let mut wicketd_config = + PropertyGroupBuilder::new("config") + .add_property( + "address", + "astring", + &format!("[::1]:{WICKETD_PORT}"), + ) + .add_property( + "artifact-address", + "astring", + &format!("[{bootstrap_address}]:{BOOTSTRAP_ARTIFACT_PORT}"), + ) + .add_property( + "baseboard-file", + "astring", + SWITCH_ZONE_BASEBOARD_FILE, + ) + .add_property( + "mgs-address", + "astring", + &format!("[::1]:{MGS_PORT}"), + ) + // We intentionally bind `nexus-proxy-address` to + // `::` so wicketd will serve this on all + // interfaces, particularly the tech port + // interfaces, allowing external clients to connect + // to this Nexus proxy. + .add_property( + "nexus-proxy-address", + "astring", + &format!("[::]:{WICKETD_NEXUS_PROXY_PORT}"), + ); + + if let Some(i) = info { let rack_subnet = Ipv6Subnet::::new( - underlay_address, + i.underlay_address, ); - smfh.setprop( - "config/rack-subnet", + + wicketd_config = wicketd_config.add_property( + "rack-subnet", + "astring", &rack_subnet.net().addr().to_string(), - )?; + ); } - let serialized_baseboard = - serde_json::to_string_pretty(&baseboard)?; - let serialized_baseboard_path = - Utf8PathBuf::from(format!( - "{}/opt/oxide/baseboard.json", - running_zone.root() - )); - tokio::fs::write( - &serialized_baseboard_path, - &serialized_baseboard, - ) - .await - .map_err(|err| { - Error::io_path(&serialized_baseboard_path, err) - })?; - smfh.setprop( - "config/baseboard-file", - String::from("/opt/oxide/baseboard.json"), - )?; + wicketd_service = wicketd_service.add_instance( + ServiceInstanceBuilder::new("default") + .add_property_group(wicketd_config), + ); - smfh.refresh()?; + let baseboard_info = + serde_json::to_string(&baseboard)?; + + switch_zone_setup_config = + switch_zone_setup_config.clone().add_property( + "baseboard_info", + "astring", + &baseboard_info, + ); } SwitchService::Dendrite { asic } => { info!( self.inner.log, "Setting up dendrite service" ); - - if let Some(info) = self.inner.sled_info.get() { - smfh.setprop("config/rack_id", info.rack_id)?; - smfh.setprop( - "config/sled_id", - info.config.sled_id, - )?; - } else { - info!( - self.inner.log, - "no rack_id/sled_id available yet" - ); + let mut dendrite_config = + PropertyGroupBuilder::new("config"); + + if let Some(i) = info { + dendrite_config = dendrite_config + .add_property( + "sled_id", + "astring", + &i.config.sled_id.to_string(), + ) + .add_property( + "rack_id", + "astring", + &i.rack_id.to_string(), + ); } - smfh.delpropvalue("config/address", "*")?; - smfh.delpropvalue("config/dns_server", "*")?; - for address in &request.zone.addresses { - smfh.addpropvalue( - "config/address", + for address in addresses { + dendrite_config = dendrite_config.add_property( + "address", + "astring", &format!("[{}]:{}", address, DENDRITE_PORT), - )?; + ); if *address != Ipv6Addr::LOCALHOST { let az_prefix = Ipv6Subnet::::new(*address); for addr in Resolver::servers_from_subnet(az_prefix) { - smfh.addpropvalue( - "config/dns_server", - &format!("{addr}"), - )?; + dendrite_config = dendrite_config + .add_property( + "dns_server", + "astring", + &format!("{addr}"), + ); } } } + match asic { DendriteAsic::TofinoAsic => { // There should be exactly one device_name @@ -2568,43 +2523,43 @@ impl ServiceManager { // for the tofino ASIC. let dev_cnt = device_names.len(); if dev_cnt == 1 { - smfh.setprop( - "config/dev_path", - device_names[0].clone(), - )?; + dendrite_config = dendrite_config + .add_property( + "dev_path", + "astring", + &device_names[0].clone(), + ); } else { return Err(Error::SledLocalZone( anyhow::anyhow!( "{dev_cnt} devices needed \ - for tofino asic" + for tofino asic" ), )); } - smfh.setprop( - "config/port_config", - "/opt/oxide/dendrite/misc/sidecar_config.toml", - )?; - smfh.setprop("config/board_rev", &sidecar_revision)?; + dendrite_config = dendrite_config + .add_property( + "port_config", + "astring", + "/opt/oxide/dendrite/misc/sidecar_config.toml", + ) + .add_property("board_rev", "astring", &sidecar_revision); + } + DendriteAsic::TofinoStub => { + dendrite_config = dendrite_config + .add_property( + "port_config", + "astring", + "/opt/oxide/dendrite/misc/model_config.toml", + ); } - DendriteAsic::TofinoStub => smfh.setprop( - "config/port_config", - "/opt/oxide/dendrite/misc/model_config.toml", - )?, asic @ (DendriteAsic::SoftNpuZone | DendriteAsic::SoftNpuPropolisDevice) => { - if asic == &DendriteAsic::SoftNpuZone { - smfh.setprop("config/mgmt", "uds")?; - smfh.setprop( - "config/uds_path", - "/opt/softnpu/stuff", - )?; - } - if asic == &DendriteAsic::SoftNpuPropolisDevice { - smfh.setprop("config/mgmt", "uart")?; - } let s = match self.inner.sidecar_revision { SidecarRevision::SoftZone(ref s) => s, - SidecarRevision::SoftPropolis(ref s) => s, + SidecarRevision::SoftPropolis( + ref s, + ) => s, _ => { return Err(Error::SidecarRevision( anyhow::anyhow!( @@ -2614,24 +2569,66 @@ impl ServiceManager { )) } }; - smfh.setprop( - "config/front_ports", - &s.front_port_count.to_string(), - )?; - smfh.setprop( - "config/rear_ports", - &s.rear_port_count.to_string(), - )?; - smfh.setprop( - "config/port_config", - "/opt/oxide/dendrite/misc/softnpu_single_sled_config.toml", - )? + + dendrite_config = dendrite_config + .add_property( + "front_ports", + "astring", + &s.front_port_count.to_string(), + ) + .add_property( + "rear_ports", + "astring", + &s.rear_port_count.to_string(), + ) + .add_property( + "port_config", + "astring", + "/opt/oxide/dendrite/misc/softnpu_single_sled_config.toml", + ); + + if asic == &DendriteAsic::SoftNpuZone { + dendrite_config = dendrite_config + .add_property( + "mgmt", "astring", "uds", + ) + .add_property( + "uds_path", + "astring", + "/opt/softnpu/stuff", + ); + } + + if asic + == &DendriteAsic::SoftNpuPropolisDevice + { + dendrite_config = dendrite_config + .add_property( + "mgmt", "astring", "uart", + ); + } } - }; - smfh.refresh()?; + } + + dendrite_service = dendrite_service.add_instance( + ServiceInstanceBuilder::new("default") + .add_property_group(dendrite_config), + ); } SwitchService::Tfport { pkt_source, asic } => { info!(self.inner.log, "Setting up tfport service"); + let mut tfport_config = + PropertyGroupBuilder::new("config") + .add_property( + "host", + "astring", + &format!("[{}]", Ipv6Addr::LOCALHOST), + ) + .add_property( + "port", + "astring", + &format!("{}", DENDRITE_PORT), + ); let is_gimlet = is_gimlet().map_err(|e| { Error::Underlay( @@ -2662,62 +2659,80 @@ impl ServiceManager { // Each `prefix` is an `Ipv6Subnet` // including a netmask. Stringify just the // network address, without the mask. - smfh.setprop( - format!("config/techport{i}_prefix"), - prefix.net().addr(), - )?; + tfport_config = tfport_config.add_property( + &format!("techport{i}_prefix"), + "astring", + prefix.net().addr().to_string(), + ) } - smfh.setprop("config/pkt_source", pkt_source)?; - } + }; + + if is_gimlet + || asic == &DendriteAsic::SoftNpuPropolisDevice + { + tfport_config = tfport_config.add_property( + "pkt_source", + "astring", + pkt_source, + ); + }; + if asic == &DendriteAsic::SoftNpuZone { - smfh.setprop("config/flags", "--sync-only")?; - } - if asic == &DendriteAsic::SoftNpuPropolisDevice { - smfh.setprop("config/pkt_source", pkt_source)?; + tfport_config = tfport_config.add_property( + "flags", + "astring", + "--sync-only", + ); } - smfh.setprop( - "config/host", - &format!("[{}]", Ipv6Addr::LOCALHOST), - )?; - smfh.setprop( - "config/port", - &format!("{}", DENDRITE_PORT), - )?; - smfh.refresh()?; + tfport_service = tfport_service.add_instance( + ServiceInstanceBuilder::new("default") + .add_property_group(tfport_config), + ); } SwitchService::Lldpd { baseboard } => { info!(self.inner.log, "Setting up lldpd service"); + let mut lldpd_config = + PropertyGroupBuilder::new("config") + .add_property( + "board_rev", + "astring", + &sidecar_revision, + ); + match baseboard { Baseboard::Gimlet { identifier, model, .. } | Baseboard::Pc { identifier, model, .. } => { - smfh.setprop( - "config/scrimlet_id", - identifier, - )?; - smfh.setprop( - "config/scrimlet_model", - model, - )?; + lldpd_config = lldpd_config + .add_property( + "scrimlet_id", + "astring", + identifier, + ) + .add_property( + "scrimlet_model", + "astring", + model, + ); } Baseboard::Unknown => {} } - smfh.setprop( - "config/board_rev", - &sidecar_revision, - )?; - smfh.delpropvalue("config/address", "*")?; - for address in &request.zone.addresses { - smfh.addpropvalue( - "config/address", + for address in addresses { + lldpd_config = lldpd_config.add_property( + "address", + "astring", &format!("[{}]:{}", address, LLDP_PORT), - )?; + ); } - smfh.refresh()?; + + lldpd_service = lldpd_service.add_instance( + ServiceInstanceBuilder::new("default") + .add_property_group(lldpd_config), + ); } SwitchService::Pumpkind { asic } => { // The pumpkin daemon is only needed when running on @@ -2727,64 +2742,114 @@ impl ServiceManager { self.inner.log, "Setting up pumpkind service" ); - smfh.setprop("config/mode", "switch")?; - smfh.refresh()?; + let pumpkind_config = + PropertyGroupBuilder::new("config") + .add_property( + "mode", "astring", "switch", + ); + + pumpkind_service = pumpkind_service + .add_instance( + ServiceInstanceBuilder::new("default") + .add_property_group( + pumpkind_config, + ), + ); + } else { + pumpkind_service = pumpkind_service + .add_instance( + ServiceInstanceBuilder::new("default") + .disable(), + ); } } SwitchService::Uplink => { // Nothing to do here - this service is special and // configured in // `ensure_switch_zone_uplinks_configured` + uplink_service = uplink_service.add_instance( + ServiceInstanceBuilder::new("default"), + ); } SwitchService::Mgd => { info!(self.inner.log, "Setting up mgd service"); - smfh.delpropvalue("config/dns_servers", "*")?; - if let Some(info) = self.inner.sled_info.get() { - smfh.setprop("config/rack_uuid", info.rack_id)?; - smfh.setprop( - "config/sled_uuid", - info.config.sled_id, - )?; + + let mut mgd_config = + PropertyGroupBuilder::new("config"); + + if let Some(i) = info { + mgd_config = mgd_config + .add_property( + "sled_uuid", + "astring", + &i.config.sled_id.to_string(), + ) + .add_property( + "rack_uuid", + "astring", + &i.rack_id.to_string(), + ); } - for address in &request.zone.addresses { + + for address in addresses { if *address != Ipv6Addr::LOCALHOST { let az_prefix = Ipv6Subnet::::new(*address); for addr in Resolver::servers_from_subnet(az_prefix) { - smfh.addpropvalue( - "config/dns_servers", + mgd_config = mgd_config.add_property( + "dns_servers", + "astring", &format!("{addr}"), - )?; + ); } break; } } - smfh.refresh()?; + + mgd_service = mgd_service.add_instance( + ServiceInstanceBuilder::new("default") + .add_property_group(mgd_config), + ); } SwitchService::MgDdm { mode } => { info!(self.inner.log, "Setting up mg-ddm service"); - smfh.setprop("config/mode", &mode)?; - if let Some(info) = self.inner.sled_info.get() { - smfh.setprop("config/rack_uuid", info.rack_id)?; - smfh.setprop( - "config/sled_uuid", - info.config.sled_id, - )?; + + let mut mg_ddm_config = + PropertyGroupBuilder::new("config") + .add_property("mode", "astring", mode) + .add_property( + "dendrite", "astring", "true", + ); + + if let Some(i) = info { + mg_ddm_config = mg_ddm_config + .add_property( + "sled_uuid", + "astring", + &i.config.sled_id.to_string(), + ) + .add_property( + "rack_uuid", + "astring", + &i.rack_id.to_string(), + ); } - smfh.delpropvalue("config/dns_servers", "*")?; - for address in &request.zone.addresses { + + for address in addresses { if *address != Ipv6Addr::LOCALHOST { let az_prefix = Ipv6Subnet::::new(*address); for addr in Resolver::servers_from_subnet(az_prefix) { - smfh.addpropvalue( - "config/dns_servers", - &format!("{addr}"), - )?; + mg_ddm_config = mg_ddm_config + .add_property( + "dns_servers", + "astring", + &format!("{addr}"), + ); } break; } @@ -2833,45 +2898,62 @@ impl ServiceManager { .collect() }; - smfh.setprop( - "config/interfaces", - // `svccfg setprop` requires a list of values to - // be enclosed in `()`, and each string value to - // be enclosed in `""`. Note that we do _not_ - // need to escape the parentheses, since this is - // not passed through a shell, but directly to - // `exec(2)` in the zone. - format!( - "({})", - maghemite_interfaces - .iter() - .map(|interface| format!( - r#""{}""#, - interface - )) - .join(" "), - ), - )?; + for i in maghemite_interfaces { + mg_ddm_config = mg_ddm_config.add_property( + "interfaces", + "astring", + &i.to_string(), + ); + } if is_gimlet { - // Ddm for a scrimlet needs to be configured to - // talk to dendrite - smfh.setprop("config/dpd_host", "[::1]")?; - smfh.setprop("config/dpd_port", DENDRITE_PORT)?; + mg_ddm_config = mg_ddm_config + .add_property( + "dpd_host", "astring", "[::1]", + ) + .add_property( + "dpd_port", + "astring", + &DENDRITE_PORT.to_string(), + ) } - smfh.setprop("config/dendrite", "true")?; - smfh.refresh()?; + mg_ddm_service = mg_ddm_service.add_instance( + ServiceInstanceBuilder::new("default") + .add_property_group(mg_ddm_config), + ); } } - - debug!(self.inner.log, "enabling service"); - smfh.enable()?; } - } - }; - Ok(running_zone) + switch_zone_setup_service = switch_zone_setup_service + .add_instance( + ServiceInstanceBuilder::new("default") + .add_property_group(switch_zone_setup_config), + ); + + let profile = ProfileBuilder::new("omicron") + .add_service(nw_setup_service) + .add_service(disabled_dns_client_service) + .add_service(mgs_service) + .add_service(wicketd_service) + .add_service(switch_zone_setup_service) + .add_service(dendrite_service) + .add_service(tfport_service) + .add_service(lldpd_service) + .add_service(pumpkind_service) + .add_service(mgd_service) + .add_service(mg_ddm_service) + .add_service(uplink_service); + profile + .add_to_zone(&self.inner.log, &installed_zone) + .await + .map_err(|err| { + Error::io("Failed to setup Switch zone profile", err) + })?; + return Ok(RunningZone::boot(installed_zone).await?); + } + } } // Ensures that a single Omicron zone is running. @@ -3739,6 +3821,7 @@ impl ServiceManager { } }; + info!(self.inner.log, "Setting up uplinkd service"); let smfh = SmfHelper::new(&zone, &SwitchService::Uplink); // We want to delete all the properties in the `uplinks` group, but we @@ -3749,6 +3832,7 @@ impl ServiceManager { for port_config in &our_ports { for addr in &port_config.addrs { + info!(self.inner.log, "configuring port: {port_config:?}"); smfh.addpropvalue_type( &format!("uplinks/{}_0", port_config.port,), &addr.to_string(), @@ -3867,13 +3951,33 @@ impl ServiceManager { ); } + // When the request addresses have changed this means the underlay is + // available now as well. if let Some(info) = self.inner.sled_info.get() { - zone.add_default_route(info.underlay_address).map_err( + info!( + self.inner.log, + "Ensuring there is a default route"; + "gateway" => ?info.underlay_address, + ); + match zone.add_default_route(info.underlay_address).map_err( |err| Error::ZoneCommand { intent: "Adding Route".to_string(), err, }, - )?; + ) { + Ok(_) => (), + Err(e) => { + if e.to_string().contains("entry exists") { + info!( + self.inner.log, + "Default route already exists"; + "gateway" => ?info.underlay_address, + ) + } else { + return Err(e); + } + } + }; } for service in &request.services { @@ -3881,28 +3985,37 @@ impl ServiceManager { match service { SwitchService::ManagementGatewayService => { + info!(self.inner.log, "configuring MGS service"); // Remove any existing `config/address` values // without deleting the property itself. - smfh.delpropvalue("config/address", "*")?; + smfh.delpropvalue_default_instance( + "config/address", + "*", + )?; // Restore the localhost address that we always add // when setting up MGS. - smfh.addpropvalue( + smfh.addpropvalue_type_default_instance( "config/address", &format!("[::1]:{MGS_PORT}"), + "astring", )?; // Add the underlay address. - smfh.addpropvalue( + smfh.addpropvalue_type_default_instance( "config/address", &format!("[{address}]:{MGS_PORT}"), + "astring", )?; // It should be impossible for the `sled_info` not // to be set here, as the underlay is set at the // same time. if let Some(info) = self.inner.sled_info.get() { - smfh.setprop("config/rack_id", info.rack_id)?; + smfh.setprop_default_instance( + "config/rack_id", + info.rack_id, + )?; } else { error!( self.inner.log, @@ -3914,6 +4027,10 @@ impl ServiceManager { } smfh.refresh()?; + info!( + self.inner.log, + "refreshed MGS service with new configuration" + ) } SwitchService::Dendrite { .. } => { info!( @@ -3921,8 +4038,11 @@ impl ServiceManager { "configuring dendrite service" ); if let Some(info) = self.inner.sled_info.get() { - smfh.setprop("config/rack_id", info.rack_id)?; - smfh.setprop( + smfh.setprop_default_instance( + "config/rack_id", + info.rack_id, + )?; + smfh.setprop_default_instance( "config/sled_id", info.config.sled_id, )?; @@ -3932,12 +4052,19 @@ impl ServiceManager { "no rack_id/sled_id available yet" ); } - smfh.delpropvalue("config/address", "*")?; - smfh.delpropvalue("config/dns_server", "*")?; + smfh.delpropvalue_default_instance( + "config/address", + "*", + )?; + smfh.delpropvalue_default_instance( + "config/dns_server", + "*", + )?; for address in &request.addresses { - smfh.addpropvalue( + smfh.addpropvalue_type_default_instance( "config/address", &format!("[{}]:{}", address, DENDRITE_PORT), + "astring", )?; if *address != Ipv6Addr::LOCALHOST { let az_prefix = @@ -3945,14 +4072,16 @@ impl ServiceManager { for addr in Resolver::servers_from_subnet(az_prefix) { - smfh.addpropvalue( + smfh.addpropvalue_type_default_instance( "config/dns_server", &format!("{addr}"), + "astring", )?; } } } smfh.refresh()?; + info!(self.inner.log, "refreshed dendrite service with new configuration") } SwitchService::Wicketd { .. } => { if let Some(&address) = first_address { @@ -3964,12 +4093,13 @@ impl ServiceManager { "rack_subnet" => %rack_subnet.net().addr(), ); - smfh.setprop( + smfh.setprop_default_instance( "config/rack-subnet", &rack_subnet.net().addr().to_string(), )?; smfh.refresh()?; + info!(self.inner.log, "refreshed wicketd service with new configuration") } else { error!( self.inner.log, @@ -3979,14 +4109,19 @@ impl ServiceManager { } SwitchService::Lldpd { .. } => { info!(self.inner.log, "configuring lldp service"); - smfh.delpropvalue("config/address", "*")?; + smfh.delpropvalue_default_instance( + "config/address", + "*", + )?; for address in &request.addresses { - smfh.addpropvalue( + smfh.addpropvalue_type_default_instance( "config/address", &format!("[{}]:{}", address, LLDP_PORT), + "astring", )?; } smfh.refresh()?; + info!(self.inner.log, "refreshed lldpd service with new configuration") } SwitchService::Tfport { .. } => { // Since tfport and dpd communicate using localhost, @@ -4007,10 +4142,16 @@ impl ServiceManager { } SwitchService::Mgd => { info!(self.inner.log, "configuring mgd service"); - smfh.delpropvalue("config/dns_servers", "*")?; + smfh.delpropvalue_default_instance( + "config/dns_servers", + "*", + )?; if let Some(info) = self.inner.sled_info.get() { - smfh.setprop("config/rack_uuid", info.rack_id)?; - smfh.setprop( + smfh.setprop_default_instance( + "config/rack_uuid", + info.rack_id, + )?; + smfh.setprop_default_instance( "config/sled_uuid", info.config.sled_id, )?; @@ -4022,28 +4163,46 @@ impl ServiceManager { for addr in Resolver::servers_from_subnet(az_prefix) { - smfh.addpropvalue( + smfh.addpropvalue_type_default_instance( "config/dns_servers", &format!("{addr}"), + "astring", )?; } break; } } smfh.refresh()?; + info!( + self.inner.log, + "refreshed mgd service with new configuration" + ) } SwitchService::MgDdm { mode } => { info!(self.inner.log, "configuring mg-ddm service"); - smfh.delpropvalue("config/mode", "*")?; - smfh.addpropvalue("config/mode", &mode)?; + smfh.delpropvalue_default_instance( + "config/mode", + "*", + )?; + smfh.addpropvalue_type_default_instance( + "config/mode", + &mode, + "astring", + )?; if let Some(info) = self.inner.sled_info.get() { - smfh.setprop("config/rack_uuid", info.rack_id)?; - smfh.setprop( + smfh.setprop_default_instance( + "config/rack_uuid", + info.rack_id, + )?; + smfh.setprop_default_instance( "config/sled_uuid", info.config.sled_id, )?; } - smfh.delpropvalue("config/dns_servers", "*")?; + smfh.delpropvalue_default_instance( + "config/dns_servers", + "*", + )?; for address in &request.addresses { if *address != Ipv6Addr::LOCALHOST { let az_prefix = @@ -4051,15 +4210,17 @@ impl ServiceManager { for addr in Resolver::servers_from_subnet(az_prefix) { - smfh.addpropvalue( + smfh.addpropvalue_type_default_instance( "config/dns_servers", &format!("{addr}"), + "astring", )?; } break; } } smfh.refresh()?; + info!(self.inner.log, "refreshed mg-ddm service with new configuration") } } } @@ -4108,6 +4269,7 @@ impl ServiceManager { let zone_request = SwitchZoneConfigLocal { root, zone: request.clone() }; let zone_args = ZoneArgs::Switch(&zone_request); + info!(self.inner.log, "Starting switch zone",); let zone = self .initialize_zone(zone_args, filesystems, data_links, None) .await?; diff --git a/sled-agent/src/smf_helper.rs b/sled-agent/src/smf_helper.rs index 837aa59157..230f146323 100644 --- a/sled-agent/src/smf_helper.rs +++ b/sled-agent/src/smf_helper.rs @@ -17,53 +17,27 @@ pub enum Error { pub trait Service { fn service_name(&self) -> String; fn smf_name(&self) -> String; - fn should_import(&self) -> bool; } pub struct SmfHelper<'t> { running_zone: &'t RunningZone, - service_name: String, smf_name: String, default_smf_name: String, - import: bool, } impl<'t> SmfHelper<'t> { pub fn new(running_zone: &'t RunningZone, service: &impl Service) -> Self { - let service_name = service.service_name(); let smf_name = service.smf_name(); - let import = service.should_import(); let default_smf_name = format!("{}:default", smf_name); - SmfHelper { - running_zone, - service_name, - smf_name, - default_smf_name, - import, - } + SmfHelper { running_zone, smf_name, default_smf_name } } - pub fn import_manifest(&self) -> Result<(), Error> { - if self.import { - self.running_zone - .run_cmd(&[ - illumos_utils::zone::SVCCFG, - "import", - &format!( - "/var/svc/manifest/site/{}/manifest.xml", - self.service_name - ), - ]) - .map_err(|err| Error::ZoneCommand { - intent: "importing manifest".to_string(), - err, - })?; - } - Ok(()) - } - - pub fn setprop(&self, prop: P, val: V) -> Result<(), Error> + pub fn setprop_default_instance( + &self, + prop: P, + val: V, + ) -> Result<(), Error> where P: ToString, V: ToString, @@ -72,7 +46,7 @@ impl<'t> SmfHelper<'t> { .run_cmd(&[ illumos_utils::zone::SVCCFG, "-s", - &self.smf_name, + &self.default_smf_name, "setprop", &format!("{}={}", prop.to_string(), val.to_string()), ]) @@ -111,18 +85,25 @@ impl<'t> SmfHelper<'t> { Ok(()) } - pub fn addpropvalue(&self, prop: P, val: V) -> Result<(), Error> + pub fn addpropvalue_type_default_instance( + &self, + prop: P, + val: V, + valtype: T, + ) -> Result<(), Error> where P: ToString, V: ToString, + T: ToString, { self.running_zone .run_cmd(&[ illumos_utils::zone::SVCCFG, "-s", - &self.smf_name, + &self.default_smf_name, "addpropvalue", &prop.to_string(), + &format!("{}:", valtype.to_string()), &val.to_string(), ]) .map_err(|err| Error::ZoneCommand { @@ -183,16 +164,21 @@ impl<'t> SmfHelper<'t> { Ok(()) } - pub fn delpropvalue(&self, prop: P, val: V) -> Result<(), Error> + pub fn delpropvalue_default_instance( + &self, + prop: P, + val: V, + ) -> Result<(), Error> where P: ToString, V: ToString, { - self.running_zone + match self + .running_zone .run_cmd(&[ illumos_utils::zone::SVCCFG, "-s", - &self.smf_name, + &self.default_smf_name, "delpropvalue", &prop.to_string(), &val.to_string(), @@ -200,7 +186,17 @@ impl<'t> SmfHelper<'t> { .map_err(|err| Error::ZoneCommand { intent: format!("del {} smf property value", prop.to_string()), err, - })?; + }) { + Ok(_) => (), + Err(e) => { + // If a property already doesn't exist we don't need to + // return an error + if !e.to_string().contains("No such property") { + return Err(e); + } + } + }; + Ok(()) } @@ -221,19 +217,4 @@ impl<'t> SmfHelper<'t> { })?; Ok(()) } - - pub fn enable(&self) -> Result<(), Error> { - self.running_zone - .run_cmd(&[ - illumos_utils::zone::SVCADM, - "enable", - "-t", - &self.default_smf_name, - ]) - .map_err(|err| Error::ZoneCommand { - intent: format!("Enable {} service", self.default_smf_name), - err, - })?; - Ok(()) - } } diff --git a/smf/mgs/manifest.xml b/smf/mgs/manifest.xml index 125c32ce2b..e129ccf35a 100644 --- a/smf/mgs/manifest.xml +++ b/smf/mgs/manifest.xml @@ -4,13 +4,18 @@ - + + + + +