[Sled Agent] Expunged disks are not in use after omicron_physical_disks_ensure

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, ZpoolName};
 use camino::{Utf8Path, Utf8PathBuf};
 use camino_tempfile::Utf8TempDir;
 use ipnetwork::IpNetwork;
@@ -101,60 +102,6 @@ pub enum EnsureAddressError {
     OpteGatewayConfig(#[from] RunCommandError),
 }
 
-/// Errors returned from [`RunningZone::get`].
-#[derive(thiserror::Error, Debug)]
-pub enum GetZoneError {
-    #[error("While looking up zones with prefix '{prefix}', could not get zones: {err}")]
-    GetZones {
-        prefix: String,
-        #[source]
-        err: crate::zone::AdmError,
-    },
-
-    #[error("Invalid Utf8 path: {0}")]
-    FromPathBuf(#[from] camino::FromPathBufError),
-
-    #[error("Zone with prefix '{prefix}' not found")]
-    NotFound { prefix: String },
-
-    #[error("Cannot get zone '{name}': it is in the {state:?} state instead of running")]
-    NotRunning { name: String, state: zone::State },
-
-    #[error(
-        "Cannot get zone '{name}': Failed to acquire control interface {err}"
-    )]
-    ControlInterface {
-        name: String,
-        #[source]
-        err: crate::zone::GetControlInterfaceError,
-    },
-
-    #[error("Cannot get zone '{name}': Failed to create addrobj: {err}")]
-    AddrObject {
-        name: String,
-        #[source]
-        err: crate::addrobj::ParseError,
-    },
-
-    #[error(
-        "Cannot get zone '{name}': Failed to ensure address exists: {err}"
-    )]
-    EnsureAddress {
-        name: String,
-        #[source]
-        err: crate::zone::EnsureAddressError,
-    },
-
-    #[error(
-        "Cannot get zone '{name}': Incorrect bootstrap interface access {err}"
-    )]
-    BootstrapInterface {
-        name: String,
-        #[source]
-        err: crate::zone::GetBootstrapInterfaceError,
-    },
-}
-
 #[cfg(target_os = "illumos")]
 static REAPER_THREAD: OnceLock<thread::JoinHandle<()>> = OnceLock::new();
 
@@ -407,6 +354,11 @@ impl RunningZone {
         self.inner.root()
     }
 
+    /// Returns the zpool on which the filesystem path has been placed.
+    pub fn root_zpool(&self) -> Option<&ZpoolName> {
+        self.inner.zonepath.pool.as_ref()
+    }
+
     pub fn control_interface(&self) -> AddrObject {
         AddrObject::new(self.inner.get_control_vnic_name(), "omicron6").unwrap()
     }
@@ -797,95 +749,6 @@ impl RunningZone {
         Ok(())
     }
 
-    /// Looks up a running zone based on the `zone_prefix`, if one already exists.
-    ///
-    /// - If the zone was found, is running, and has a network interface, it is
-    /// returned.
-    /// - If the zone was not found `Error::NotFound` is returned.
-    /// - If the zone was found, but not running, `Error::NotRunning` is
-    /// 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<Etherstub>,
-        zone_prefix: &str,
-        addrtype: AddressRequest,
-    ) -> Result<Self, GetZoneError> {
-        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<Item = &Port> {
         self.inner.opte_ports()
@@ -1081,7 +944,7 @@ pub struct InstalledZone {
     log: Logger,
 
     // Filesystem path of the zone
-    zonepath: Utf8PathBuf,
+    zonepath: PathInPool,
 
     // Name of the Zone.
     name: String,
@@ -1131,7 +994,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 +1010,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 +1061,7 @@ pub struct ZoneBuilder<'a> {
     /// Allocates the NIC used for control plane communication.
     underlay_vnic_allocator: Option<&'a VnicAllocator<Etherstub>>,
     /// Filesystem path at which the installed zone will reside.
-    zone_root_path: Option<&'a Utf8Path>,
+    zone_root_path: Option<PathInPool>,
     /// 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 +1114,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 +1208,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 +1242,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 +1306,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 +1327,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,

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() {

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<ZpoolName>,
+    pub path: Utf8PathBuf,
+}
+
 #[cfg_attr(any(test, feature = "testing"), mockall::automock, allow(dead_code))]
 impl Zpool {
     pub fn create(

sled-agent/src/common/instance.rs

@@ -486,9 +486,15 @@ impl InstanceStates {
     /// instance's state in Nexus may become inconsistent. This routine should
     /// therefore only be invoked by callers who know that an instance is not
     /// migrating.
-    pub(crate) fn terminate_rudely(&mut self) {
+    pub(crate) fn terminate_rudely(&mut self, mark_failed: bool) {
+        let vmm_state = if mark_failed {
+            PropolisInstanceState(PropolisApiState::Failed)
+        } else {
+            PropolisInstanceState(PropolisApiState::Destroyed)
+        };
+
         let fake_observed = ObservedPropolisState {
-            vmm_state: PropolisInstanceState(PropolisApiState::Destroyed),
+            vmm_state,
             migration_status: if self.instance.migration_id.is_some() {
                 ObservedMigrationStatus::Failed
             } else {
@@ -893,7 +899,8 @@ mod test {
         assert_eq!(state.propolis_role(), PropolisRole::MigrationTarget);
 
         let prev = state.clone();
-        state.terminate_rudely();
+        let mark_failed = false;
+        state.terminate_rudely(mark_failed);
 
         assert_state_change_has_gen_change(&prev, &state);
         assert_eq!(state.instance.gen, prev.instance.gen);