diff --git a/illumos-utils/src/running_zone.rs b/illumos-utils/src/running_zone.rs index c529a1b6d4..e3105f089c 100644 --- a/illumos-utils/src/running_zone.rs +++ b/illumos-utils/src/running_zone.rs @@ -10,6 +10,7 @@ use crate::link::{Link, VnicAllocator}; use crate::opte::{Port, PortTicket}; use crate::svc::wait_for_service; use crate::zone::{AddressRequest, IPADM, ZONE_PREFIX}; +use crate::zpool::PathInPool; use camino::{Utf8Path, Utf8PathBuf}; use camino_tempfile::Utf8TempDir; use ipnetwork::IpNetwork; @@ -806,85 +807,85 @@ impl RunningZone { /// returned. /// - Other errors may be returned attempting to look up and accessing an /// address on the zone. - pub async fn get( - log: &Logger, - vnic_allocator: &VnicAllocator, - zone_prefix: &str, - addrtype: AddressRequest, - ) -> Result { - let zone_info = Zones::get() - .await - .map_err(|err| GetZoneError::GetZones { - prefix: zone_prefix.to_string(), - err, - })? - .into_iter() - .find(|zone_info| zone_info.name().starts_with(&zone_prefix)) - .ok_or_else(|| GetZoneError::NotFound { - prefix: zone_prefix.to_string(), - })?; - - if zone_info.state() != zone::State::Running { - return Err(GetZoneError::NotRunning { - name: zone_info.name().to_string(), - state: zone_info.state(), - }); - } - - let zone_name = zone_info.name(); - let vnic_name = - Zones::get_control_interface(zone_name).map_err(|err| { - GetZoneError::ControlInterface { - name: zone_name.to_string(), - err, - } - })?; - let addrobj = AddrObject::new_control(&vnic_name).map_err(|err| { - GetZoneError::AddrObject { name: zone_name.to_string(), err } - })?; - Zones::ensure_address(Some(zone_name), &addrobj, addrtype).map_err( - |err| GetZoneError::EnsureAddress { - name: zone_name.to_string(), - err, - }, - )?; - - let control_vnic = vnic_allocator - .wrap_existing(vnic_name) - .expect("Failed to wrap valid control VNIC"); - - // The bootstrap address for a running zone never changes, - // so there's no need to call `Zones::ensure_address`. - // Currently, only the switch zone has a bootstrap interface. - let bootstrap_vnic = Zones::get_bootstrap_interface(zone_name) - .map_err(|err| GetZoneError::BootstrapInterface { - name: zone_name.to_string(), - err, - })? - .map(|name| { - vnic_allocator - .wrap_existing(name) - .expect("Failed to wrap valid bootstrap VNIC") - }); - - Ok(Self { - id: zone_info.id().map(|x| { - x.try_into().expect("zoneid_t is expected to be an i32") - }), - inner: InstalledZone { - log: log.new(o!("zone" => zone_name.to_string())), - zonepath: zone_info.path().to_path_buf().try_into()?, - name: zone_name.to_string(), - control_vnic, - // TODO(https://github.com/oxidecomputer/omicron/issues/725) - // - // Re-initialize guest_vnic state by inspecting the zone. - opte_ports: vec![], - links: vec![], - bootstrap_vnic, - }, - }) - } + // pub async fn get( + // log: &Logger, + // vnic_allocator: &VnicAllocator, + // zone_prefix: &str, + // addrtype: AddressRequest, + // ) -> Result { + // let zone_info = Zones::get() + // .await + // .map_err(|err| GetZoneError::GetZones { + // prefix: zone_prefix.to_string(), + // err, + // })? + // .into_iter() + // .find(|zone_info| zone_info.name().starts_with(&zone_prefix)) + // .ok_or_else(|| GetZoneError::NotFound { + // prefix: zone_prefix.to_string(), + // })?; + // + // if zone_info.state() != zone::State::Running { + // return Err(GetZoneError::NotRunning { + // name: zone_info.name().to_string(), + // state: zone_info.state(), + // }); + // } + // + // let zone_name = zone_info.name(); + // let vnic_name = + // Zones::get_control_interface(zone_name).map_err(|err| { + // GetZoneError::ControlInterface { + // name: zone_name.to_string(), + // err, + // } + // })?; + // let addrobj = AddrObject::new_control(&vnic_name).map_err(|err| { + // GetZoneError::AddrObject { name: zone_name.to_string(), err } + // })?; + // Zones::ensure_address(Some(zone_name), &addrobj, addrtype).map_err( + // |err| GetZoneError::EnsureAddress { + // name: zone_name.to_string(), + // err, + // }, + // )?; + // + // let control_vnic = vnic_allocator + // .wrap_existing(vnic_name) + // .expect("Failed to wrap valid control VNIC"); + // + // // The bootstrap address for a running zone never changes, + // // so there's no need to call `Zones::ensure_address`. + // // Currently, only the switch zone has a bootstrap interface. + // let bootstrap_vnic = Zones::get_bootstrap_interface(zone_name) + // .map_err(|err| GetZoneError::BootstrapInterface { + // name: zone_name.to_string(), + // err, + // })? + // .map(|name| { + // vnic_allocator + // .wrap_existing(name) + // .expect("Failed to wrap valid bootstrap VNIC") + // }); + // + // Ok(Self { + // id: zone_info.id().map(|x| { + // x.try_into().expect("zoneid_t is expected to be an i32") + // }), + // inner: InstalledZone { + // log: log.new(o!("zone" => zone_name.to_string())), + // zonepath: zone_info.path().to_path_buf().try_into()?, + // name: zone_name.to_string(), + // control_vnic, + // // TODO(https://github.com/oxidecomputer/omicron/issues/725) + // // + // // Re-initialize guest_vnic state by inspecting the zone. + // opte_ports: vec![], + // links: vec![], + // bootstrap_vnic, + // }, + // }) + // } /// Return references to the OPTE ports for this zone. pub fn opte_ports(&self) -> impl Iterator { @@ -1081,7 +1082,7 @@ pub struct InstalledZone { log: Logger, // Filesystem path of the zone - zonepath: Utf8PathBuf, + zonepath: PathInPool, // Name of the Zone. name: String, @@ -1131,7 +1132,7 @@ impl InstalledZone { /// Returns the filesystem path to the zonepath pub fn zonepath(&self) -> &Utf8Path { - &self.zonepath + &self.zonepath.path } pub fn site_profile_xml_path(&self) -> Utf8PathBuf { @@ -1147,7 +1148,7 @@ impl InstalledZone { /// Returns the filesystem path to the zone's root in the GZ. pub fn root(&self) -> Utf8PathBuf { - self.zonepath.join(Self::ROOT_FS_PATH) + self.zonepath.path.join(Self::ROOT_FS_PATH) } } @@ -1198,7 +1199,7 @@ pub struct ZoneBuilder<'a> { /// Allocates the NIC used for control plane communication. underlay_vnic_allocator: Option<&'a VnicAllocator>, /// Filesystem path at which the installed zone will reside. - zone_root_path: Option<&'a Utf8Path>, + zone_root_path: Option, /// The directories that will be searched for the image tarball for the /// provided zone type ([`Self::with_zone_type`]). zone_image_paths: Option<&'a [Utf8PathBuf]>, @@ -1251,7 +1252,7 @@ impl<'a> ZoneBuilder<'a> { } /// Filesystem path at which the installed zone will reside. - pub fn with_zone_root_path(mut self, root_path: &'a Utf8Path) -> Self { + pub fn with_zone_root_path(mut self, root_path: PathInPool) -> Self { self.zone_root_path = Some(root_path); self } @@ -1345,8 +1346,11 @@ impl<'a> ZoneBuilder<'a> { self.zone_type?, self.unique_name, ); - let zonepath = temp_dir - .join(self.zone_root_path?.strip_prefix("/").unwrap()) + let mut zonepath = self.zone_root_path?; + zonepath.path = temp_dir + .join( + zonepath.path.strip_prefix("/").unwrap() + ) .join(&full_zone_name); let iz = InstalledZone { log: self.log?, @@ -1376,7 +1380,7 @@ impl<'a> ZoneBuilder<'a> { let Self { log: Some(log), underlay_vnic_allocator: Some(underlay_vnic_allocator), - zone_root_path: Some(zone_root_path), + zone_root_path: Some(mut zone_root_path), zone_image_paths: Some(zone_image_paths), zone_type: Some(zone_type), unique_name, @@ -1440,6 +1444,7 @@ impl<'a> ZoneBuilder<'a> { net_device_names.sort(); net_device_names.dedup(); + zone_root_path.path = zone_root_path.path.join(&full_zone_name); Zones::install_omicron_zone( &log, &zone_root_path, @@ -1460,7 +1465,7 @@ impl<'a> ZoneBuilder<'a> { Ok(InstalledZone { log: log.new(o!("zone" => full_zone_name.clone())), - zonepath: zone_root_path.join(&full_zone_name), + zonepath: zone_root_path, name: full_zone_name, control_vnic, bootstrap_vnic, diff --git a/illumos-utils/src/zone.rs b/illumos-utils/src/zone.rs index 3f749fc352..7ba40af043 100644 --- a/illumos-utils/src/zone.rs +++ b/illumos-utils/src/zone.rs @@ -14,6 +14,7 @@ use std::net::{IpAddr, Ipv6Addr}; use crate::addrobj::AddrObject; use crate::dladm::{EtherstubVnic, VNIC_PREFIX_BOOTSTRAP, VNIC_PREFIX_CONTROL}; +use crate::zpool::PathInPool; use crate::{execute, PFEXEC}; use omicron_common::address::SLED_PREFIX; @@ -282,7 +283,7 @@ impl Zones { #[allow(clippy::too_many_arguments)] pub async fn install_omicron_zone( log: &Logger, - zone_root_path: &Utf8Path, + zone_root_path: &PathInPool, zone_name: &str, zone_image: &Utf8Path, datasets: &[zone::Dataset], @@ -319,10 +320,9 @@ impl Zones { true, zone::CreationOptions::Blank, ); - let path = zone_root_path.join(zone_name); cfg.get_global() .set_brand("omicron1") - .set_path(&path) + .set_path(&zone_root_path.path) .set_autoboot(false) .set_ip_type(zone::IpType::Exclusive); if !limit_priv.is_empty() { diff --git a/illumos-utils/src/zpool.rs b/illumos-utils/src/zpool.rs index fa93760f99..5dabbdecc7 100644 --- a/illumos-utils/src/zpool.rs +++ b/illumos-utils/src/zpool.rs @@ -5,7 +5,7 @@ //! Utilities for managing Zpools. use crate::{execute, ExecutionError, PFEXEC}; -use camino::Utf8Path; +use camino::{Utf8Path, Utf8PathBuf}; use std::str::FromStr; pub use omicron_common::zpool_name::ZpoolName; @@ -181,6 +181,19 @@ impl FromStr for ZpoolInfo { /// Wraps commands for interacting with ZFS pools. pub struct Zpool {} +/// A path which exists within a pool. +/// +/// By storing these types together, it's possible to answer +/// whether or not a path exists on a particular device. +// Technically we could re-derive the pool name from the path, +// but that involves some string parsing, and honestly I'd just +// Rather Not. +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct PathInPool { + pub pool: Option, + pub path: Utf8PathBuf, +} + #[cfg_attr(any(test, feature = "testing"), mockall::automock, allow(dead_code))] impl Zpool { pub fn create( diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs index ec4d503e7b..021b326dde 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -41,7 +41,7 @@ use omicron_common::api::internal::shared::{ use omicron_common::backoff; use omicron_uuid_kinds::{GenericUuid, InstanceUuid, PropolisUuid}; use propolis_client::Client as PropolisClient; -use rand::prelude::SliceRandom; +use rand::prelude::IteratorRandom; use rand::SeedableRng; use sled_storage::dataset::ZONE_DATASET; use sled_storage::manager::StorageHandle; @@ -1343,20 +1343,22 @@ impl InstanceRunner { // configured VNICs. let zname = propolis_zone_name(self.propolis_id()); let mut rng = rand::rngs::StdRng::from_entropy(); - let root = self + let latest_disks = self .storage .get_latest_disks() .await - .all_u2_mountpoints(ZONE_DATASET) + .all_u2_mountpoints(ZONE_DATASET); + + let root = latest_disks + .into_iter() .choose(&mut rng) - .ok_or_else(|| Error::U2NotFound)? - .clone(); + .ok_or_else(|| Error::U2NotFound)?; let installed_zone = self .zone_builder_factory .builder() .with_log(self.log.clone()) .with_underlay_vnic_allocator(&self.vnic_allocator) - .with_zone_root_path(&root) + .with_zone_root_path(root) .with_zone_image_paths(&["/opt/oxide".into()]) .with_zone_type("propolis-server") .with_unique_name(self.propolis_id().into_untyped_uuid()) diff --git a/sled-agent/src/probe_manager.rs b/sled-agent/src/probe_manager.rs index 40af604645..b1a8ab37f7 100644 --- a/sled-agent/src/probe_manager.rs +++ b/sled-agent/src/probe_manager.rs @@ -14,7 +14,7 @@ use omicron_common::api::external::{ VpcFirewallRuleStatus, }; use omicron_common::api::internal::shared::NetworkInterface; -use rand::prelude::SliceRandom; +use rand::prelude::IteratorRandom; use rand::SeedableRng; use sled_storage::dataset::ZONE_DATASET; use sled_storage::manager::StorageHandle; @@ -226,14 +226,15 @@ impl ProbeManagerInner { /// boots the probe zone. async fn add_probe(self: &Arc, probe: &ProbeState) -> Result<()> { let mut rng = rand::rngs::StdRng::from_entropy(); - let root = self + let current_disks = self .storage .get_latest_disks() .await - .all_u2_mountpoints(ZONE_DATASET) + .all_u2_mountpoints(ZONE_DATASET); + let zone_root_path = current_disks + .into_iter() .choose(&mut rng) - .ok_or_else(|| anyhow!("u2 not found"))? - .clone(); + .ok_or_else(|| anyhow!("u2 not found"))?; let nic = probe .interface @@ -268,7 +269,7 @@ impl ProbeManagerInner { .builder() .with_log(self.log.clone()) .with_underlay_vnic_allocator(&self.vnic_allocator) - .with_zone_root_path(&root) + .with_zone_root_path(zone_root_path) .with_zone_image_paths(&["/opt/oxide".into()]) .with_zone_type("probe") .with_unique_name(probe.id) diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 8058d3acae..6bf8a4fbe5 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -57,7 +57,7 @@ use illumos_utils::running_zone::{ }; use illumos_utils::zfs::ZONE_ZFS_RAMDISK_DATASET_MOUNTPOINT; use illumos_utils::zone::AddressRequest; -use illumos_utils::zpool::ZpoolName; +use illumos_utils::zpool::{PathInPool, ZpoolName}; use illumos_utils::{execute, PFEXEC}; use internal_dns::resolver::Resolver; use itertools::Itertools; @@ -445,6 +445,11 @@ impl OmicronZonesConfigLocal { /// Combines the Nexus-provided `OmicronZoneConfig` (which describes what Nexus /// wants for this zone) with any locally-determined configuration (like the /// path to the root filesystem) +// +// NOTE: Although the path to the root filesystem is not exactly equal to the +// ZpoolName, it is derivable from it, and the ZpoolName for the root filesystem +// is now being supplied as a part of OmicronZoneConfig. Therefore, this struct +// is less necessary than it has been historically. #[derive( Clone, Debug, @@ -551,10 +556,15 @@ impl<'a> ZoneArgs<'a> { } /// Return the root filesystem path for this zone - pub fn root(&self) -> &Utf8Path { + pub fn root(&self) -> PathInPool { match self { - ZoneArgs::Omicron(zone_config) => &zone_config.root, - ZoneArgs::Switch(zone_request) => &zone_request.root, + ZoneArgs::Omicron(zone_config) => PathInPool { + pool: zone_config.zone.filesystem_pool.clone(), + path: zone_config.root.clone(), + }, + ZoneArgs::Switch(zone_request) => { + PathInPool { pool: None, path: zone_request.root.clone() } + } } } } @@ -1462,7 +1472,7 @@ impl ServiceManager { let installed_zone = zone_builder .with_log(self.inner.log.clone()) .with_underlay_vnic_allocator(&self.inner.underlay_vnic_allocator) - .with_zone_root_path(&request.root()) + .with_zone_root_path(request.root()) .with_zone_image_paths(zone_image_paths.as_slice()) .with_zone_type(&zone_type_str) .with_datasets(datasets.as_slice()) @@ -2904,7 +2914,8 @@ impl ServiceManager { ) .await?; - let config = OmicronZoneConfigLocal { zone: zone.clone(), root }; + let config = + OmicronZoneConfigLocal { zone: zone.clone(), root: root.path }; let runtime = self .initialize_zone( @@ -3289,7 +3300,7 @@ impl ServiceManager { mount_config: &MountConfig, zone: &OmicronZoneConfig, all_u2_pools: &Vec, - ) -> Result { + ) -> Result { let name = zone.zone_name(); // If the caller has requested a specific durable dataset, @@ -3368,7 +3379,9 @@ impl ServiceManager { device: format!("zpool: {filesystem_pool}"), }); } - Ok(filesystem_pool.dataset_mountpoint(&mount_config.root, ZONE_DATASET)) + let path = filesystem_pool + .dataset_mountpoint(&mount_config.root, ZONE_DATASET); + Ok(PathInPool { pool: Some(filesystem_pool), path }) } pub async fn cockroachdb_initialize(&self) -> Result<(), Error> { diff --git a/sled-agent/src/zone_bundle.rs b/sled-agent/src/zone_bundle.rs index cd9f36fe3a..088e7b356f 100644 --- a/sled-agent/src/zone_bundle.rs +++ b/sled-agent/src/zone_bundle.rs @@ -439,6 +439,7 @@ impl ZoneBundler { let extra_log_dirs = resources .all_u2_mountpoints(U2_DEBUG_DATASET) .into_iter() + .map(|pool_path| pool_path.path) .collect(); let context = ZoneBundleContext { cause, storage_dirs, extra_log_dirs }; info!( diff --git a/sled-storage/src/resources.rs b/sled-storage/src/resources.rs index 87d9056082..f6b3f91486 100644 --- a/sled-storage/src/resources.rs +++ b/sled-storage/src/resources.rs @@ -10,7 +10,7 @@ use crate::disk::{Disk, DiskError, OmicronPhysicalDiskConfig, RawDisk}; use crate::error::Error; use camino::Utf8PathBuf; use cfg_if::cfg_if; -use illumos_utils::zpool::ZpoolName; +use illumos_utils::zpool::{PathInPool, ZpoolName}; use key_manager::StorageKeyRequester; use omicron_common::api::external::Generation; use omicron_common::disk::DiskIdentity; @@ -203,11 +203,13 @@ impl AllDisks { } /// Returns all mountpoints within all U.2s for a particular dataset. - pub fn all_u2_mountpoints(&self, dataset: &str) -> Vec { + pub fn all_u2_mountpoints(&self, dataset: &str) -> Vec { self.all_u2_zpools() - .iter() - .map(|zpool| { - zpool.dataset_mountpoint(&self.mount_config.root, dataset) + .into_iter() + .map(|pool| { + let path = + pool.dataset_mountpoint(&self.mount_config.root, dataset); + PathInPool { pool: Some(pool), path } }) .collect() }