[sled-agent] Conform datasets to RFD 118 format, but still use ramdisks

illumos-utils/src/running_zone.rs

@@ -9,15 +9,14 @@ use crate::dladm::Etherstub;
 use crate::link::{Link, VnicAllocator};
 use crate::opte::{Port, PortTicket};
 use crate::svc::wait_for_service;
-use crate::zfs::ZONE_ZFS_DATASET_MOUNTPOINT;
 use crate::zone::{AddressRequest, ZONE_PREFIX};
 use ipnetwork::IpNetwork;
 use slog::info;
 use slog::o;
 use slog::warn;
 use slog::Logger;
 use std::net::{Ipv4Addr, Ipv6Addr};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 #[cfg(any(test, feature = "testing"))]
 use crate::zone::MockZones as Zones;
@@ -126,7 +125,7 @@ impl RunningZone {
 
     /// Returns the filesystem path to the zone's root
     pub fn root(&self) -> String {
-        format!("{}/{}/root", ZONE_ZFS_DATASET_MOUNTPOINT, self.name())
+        format!("{}/root", self.inner.zonepath.display())
     }
 
     /// Runs a command within the Zone, return the output.
@@ -355,6 +354,7 @@ impl RunningZone {
             running: true,
             inner: InstalledZone {
                 log: log.new(o!("zone" => zone_name.to_string())),
+                zonepath: zone_info.path().into(),
                 name: zone_name.to_string(),
                 control_vnic,
                 // TODO(https://github.com/oxidecomputer/omicron/issues/725)
@@ -440,6 +440,9 @@ pub enum InstallZoneError {
 pub struct InstalledZone {
     log: Logger,
 
+    // Filesystem path of the zone
+    zonepath: PathBuf,
+
     // Name of the Zone.
     name: String,
 
@@ -483,10 +486,16 @@ impl InstalledZone {
         &self.name
     }
 
+    /// Returns the filesystem path to the zonepath
+    pub fn zonepath(&self) -> &Path {
+        &self.zonepath
+    }
+
     #[allow(clippy::too_many_arguments)]
     pub async fn install(
         log: &Logger,
         underlay_vnic_allocator: &VnicAllocator<Etherstub>,
+        zone_root_path: &Path,
         zone_name: &str,
         unique_name: Option<&str>,
         datasets: &[zone::Dataset],
@@ -519,6 +528,7 @@ impl InstalledZone {
 
         Zones::install_omicron_zone(
             log,
+            &zone_root_path,
             &full_zone_name,
             &zone_image_path,
             &datasets,
@@ -536,6 +546,7 @@ impl InstalledZone {
 
         Ok(InstalledZone {
             log: log.new(o!("zone" => full_zone_name.clone())),
+            zonepath: zone_root_path.join(&full_zone_name),
             name: full_zone_name,
             control_vnic,
             bootstrap_vnic,

illumos-utils/src/zfs.rs

@@ -8,8 +8,8 @@ use crate::{execute, PFEXEC};
 use std::fmt;
 use std::path::PathBuf;
 
-pub const ZONE_ZFS_DATASET_MOUNTPOINT: &str = "/zone";
-pub const ZONE_ZFS_DATASET: &str = "rpool/zone";
+pub const ZONE_ZFS_RAMDISK_DATASET_MOUNTPOINT: &str = "/zone";
+pub const ZONE_ZFS_RAMDISK_DATASET: &str = "rpool/zone";
 const ZFS: &str = "/usr/sbin/zfs";
 
 /// Error returned by [`Zfs::list_datasets`].
@@ -42,7 +42,7 @@ enum EnsureFilesystemErrorRaw {
     Output(String),
 }
 
-/// Error returned by [`Zfs::ensure_zoned_filesystem`].
+/// Error returned by [`Zfs::ensure_filesystem`].
 #[derive(thiserror::Error, Debug)]
 #[error(
     "Failed to ensure filesystem '{name}' exists at '{mountpoint:?}': {err}"
@@ -137,15 +137,15 @@ impl Zfs {
     }
 
     /// Creates a new ZFS filesystem named `name`, unless one already exists.
-    pub fn ensure_zoned_filesystem(
+    pub fn ensure_filesystem(
         name: &str,
         mountpoint: Mountpoint,
+        zoned: bool,
         do_format: bool,
     ) -> Result<(), EnsureFilesystemError> {
         // If the dataset exists, we're done.
         let mut command = std::process::Command::new(ZFS);
         let cmd = command.args(&["list", "-Hpo", "name,type,mountpoint", name]);
-
         // If the list command returns any valid output, validate it.
         if let Ok(output) = execute(cmd) {
             let stdout = String::from_utf8_lossy(&output.stdout);
@@ -170,16 +170,11 @@ impl Zfs {
 
         // If it doesn't exist, make it.
         let mut command = std::process::Command::new(PFEXEC);
-        let cmd = command.args(&[
-            ZFS,
-            "create",
-            // The filesystem is managed from the Global Zone.
-            "-o",
-            "zoned=on",
-            "-o",
-            &format!("mountpoint={}", mountpoint),
-            name,
-        ]);
+        let cmd = command.args(&[ZFS, "create"]);
+        if zoned {
+            cmd.args(&["-o", "zoned=on"]);
+        }
+        cmd.args(&["-o", &format!("mountpoint={}", mountpoint), name]);
         execute(cmd).map_err(|err| EnsureFilesystemError {
             name: name.to_string(),
             mountpoint,
@@ -254,19 +249,21 @@ pub fn get_all_omicron_datasets_for_delete() -> anyhow::Result<Vec<String>> {
     // This includes cockroachdb, clickhouse, and crucible datasets.
     let zpools = crate::zpool::Zpool::list()?;
     for pool in &zpools {
-        // For now, avoid erasing any datasets which exist on internal zpools.
-        if pool.kind() == crate::zpool::ZpoolKind::Internal {
-            continue;
-        }
+        let internal = pool.kind() == crate::zpool::ZpoolKind::Internal;
         let pool = pool.to_string();
         for dataset in &Zfs::list_datasets(&pool)? {
+            // Avoid erasing crashdump datasets on internal pools
+            if dataset == "crash" && internal {
+                continue;
+            }
+
             datasets.push(format!("{pool}/{dataset}"));
         }
     }
 
-    // Collect all datasets for Oxide zones.
-    for dataset in &Zfs::list_datasets(&ZONE_ZFS_DATASET)? {
-        datasets.push(format!("{}/{dataset}", ZONE_ZFS_DATASET));
+    // Collect all datasets for ramdisk-based Oxide zones.
+    for dataset in &Zfs::list_datasets(&ZONE_ZFS_RAMDISK_DATASET)? {
+        datasets.push(format!("{}/{dataset}", ZONE_ZFS_RAMDISK_DATASET));
     }
 
     Ok(datasets)

illumos-utils/src/zone.rs

@@ -9,10 +9,10 @@ use ipnetwork::IpNetwork;
 use slog::info;
 use slog::Logger;
 use std::net::{IpAddr, Ipv6Addr};
+use std::path::Path;
 
 use crate::addrobj::AddrObject;
 use crate::dladm::{EtherstubVnic, VNIC_PREFIX_BOOTSTRAP, VNIC_PREFIX_CONTROL};
-use crate::zfs::ZONE_ZFS_DATASET_MOUNTPOINT;
 use crate::{execute, PFEXEC};
 use omicron_common::address::SLED_PREFIX;
 
@@ -233,6 +233,7 @@ impl Zones {
     #[allow(clippy::too_many_arguments)]
     pub async fn install_omicron_zone(
         log: &Logger,
+        zone_root_path: &Path,
         zone_name: &str,
         zone_image: &std::path::Path,
         datasets: &[zone::Dataset],
@@ -269,7 +270,7 @@ impl Zones {
             true,
             zone::CreationOptions::Blank,
         );
-        let path = format!("{}/{}", ZONE_ZFS_DATASET_MOUNTPOINT, zone_name);
+        let path = zone_root_path.join(zone_name);
         cfg.get_global()
             .set_brand("omicron1")
             .set_path(&path)

illumos-utils/src/zpool.rs

@@ -5,9 +5,10 @@
 //! Utilities for managing Zpools.
 
 use crate::{execute, ExecutionError, PFEXEC};
-use serde::{Deserialize, Deserializer};
+use schemars::JsonSchema;
+use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use std::fmt;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use uuid::Uuid;
 
@@ -26,6 +27,9 @@ pub enum Error {
 
     #[error(transparent)]
     Parse(#[from] ParseError),
+
+    #[error("No Zpools found")]
+    NoZpools,
 }
 
 #[derive(thiserror::Error, Debug)]
@@ -244,7 +248,7 @@ impl Zpool {
     }
 }
 
-#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
+#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, JsonSchema)]
 pub enum ZpoolKind {
     // This zpool is used for external storage (u.2)
     External,
@@ -256,7 +260,7 @@ pub enum ZpoolKind {
 ///
 /// This expects that the format will be: `ox{i,p}_<UUID>` - we parse the prefix
 /// when reading the structure, and validate that the UUID can be utilized.
-#[derive(Clone, Debug, Hash, PartialEq, Eq)]
+#[derive(Clone, Debug, Hash, PartialEq, Eq, JsonSchema)]
 pub struct ZpoolName {
     id: Uuid,
     kind: ZpoolKind,
@@ -278,6 +282,21 @@ impl ZpoolName {
     pub fn kind(&self) -> ZpoolKind {
         self.kind
     }
+
+    /// Returns a path to a dataset within the zpool.
+    ///
+    /// For example: oxp_(UUID) -> /pool/ext/(UUID)/(dataset)
+    pub fn dataset_path(&self, dataset: &str) -> PathBuf {
+        let mut path = PathBuf::new();
+        path.push("/pool");
+        match self.kind {
+            ZpoolKind::External => path.push("ext"),
+            ZpoolKind::Internal => path.push("int"),
+        };
+        path.push(self.id().to_string());
+        path.push(dataset);
+        path
+    }
 }
 
 impl<'de> Deserialize<'de> for ZpoolName {
@@ -290,6 +309,15 @@ impl<'de> Deserialize<'de> for ZpoolName {
     }
 }
 
+impl Serialize for ZpoolName {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        serializer.serialize_str(&self.to_string())
+    }
+}
+
 impl FromStr for ZpoolName {
     type Err = String;
 

sled-agent/src/bootstrap/agent.rs

@@ -23,13 +23,15 @@ use crate::config::Config as SledConfig;
 use crate::server::Server as SledServer;
 use crate::services::ServiceManager;
 use crate::sp::SpHandle;
+use crate::storage_manager::StorageManager;
 use crate::updates::UpdateManager;
 use futures::stream::{self, StreamExt, TryStreamExt};
 use illumos_utils::dladm::{
     self, Dladm, Etherstub, EtherstubVnic, GetMacError, PhysicalLink,
 };
 use illumos_utils::zfs::{
-    self, Mountpoint, Zfs, ZONE_ZFS_DATASET, ZONE_ZFS_DATASET_MOUNTPOINT,
+    self, Mountpoint, Zfs, ZONE_ZFS_RAMDISK_DATASET,
+    ZONE_ZFS_RAMDISK_DATASET_MOUNTPOINT,
 };
 use illumos_utils::zone::Zones;
 use omicron_common::address::Ipv6Subnet;
@@ -345,13 +347,15 @@ impl Agent {
         // TODO(https://github.com/oxidecomputer/omicron/issues/1934):
         // We should carefully consider which dataset this is using; it's
         // currently part of the ramdisk.
-        Zfs::ensure_zoned_filesystem(
-            ZONE_ZFS_DATASET,
+        let zoned = true;
+        let do_format = true;
+        Zfs::ensure_filesystem(
+            ZONE_ZFS_RAMDISK_DATASET,
             Mountpoint::Path(std::path::PathBuf::from(
-                ZONE_ZFS_DATASET_MOUNTPOINT,
+                ZONE_ZFS_RAMDISK_DATASET_MOUNTPOINT,
             )),
-            // do_format=
-            true,
+            zoned,
+            do_format,
         )?;
 
         // Before we start monitoring for hardware, ensure we're running from a
@@ -463,17 +467,18 @@ impl Agent {
                 // we should restart the hardware monitor, so we can react to
                 // changes in the switch regardless of the success or failure of
                 // this sled agent.
-                let (hardware, services) = match hardware_monitor.take() {
-                    // This is the normal case; transfer hardware monitoring responsibilities from
-                    // the bootstrap agent to the sled agent.
-                    Some(hardware_monitor) => hardware_monitor,
-                    // This is a less likely case, but if we previously failed to start (or
-                    // restart) the hardware monitor, for any reason, recreate it.
-                    None => self.start_hardware_monitor().await?,
-                }
-                .stop()
-                .await
-                .expect("Failed to stop hardware monitor");
+                let (hardware, services, storage) =
+                    match hardware_monitor.take() {
+                        // This is the normal case; transfer hardware monitoring responsibilities from
+                        // the bootstrap agent to the sled agent.
+                        Some(hardware_monitor) => hardware_monitor,
+                        // This is a less likely case, but if we previously failed to start (or
+                        // restart) the hardware monitor, for any reason, recreate it.
+                        None => self.start_hardware_monitor().await?,
+                    }
+                    .stop()
+                    .await
+                    .expect("Failed to stop hardware monitor");
 
                 // This acts like a "run-on-drop" closure, to restart the
                 // hardware monitor in the bootstrap agent if we fail to
@@ -486,6 +491,7 @@ impl Agent {
                     log: Logger,
                     hardware: Option<HardwareManager>,
                     services: Option<ServiceManager>,
+                    storage: Option<StorageManager>,
                     monitor: &'a mut Option<HardwareMonitor>,
                 }
                 impl<'a> RestartMonitor<'a> {
@@ -500,6 +506,7 @@ impl Agent {
                                 &self.log,
                                 self.hardware.take().unwrap(),
                                 self.services.take().unwrap(),
+                                self.storage.take().unwrap(),
                             ));
                         }
                     }
@@ -509,6 +516,7 @@ impl Agent {
                     log: self.log.clone(),
                     hardware: Some(hardware),
                     services: Some(services.clone()),
+                    storage: Some(storage.clone()),
                     monitor: hardware_monitor,
                 };
 
@@ -518,6 +526,7 @@ impl Agent {
                     self.parent_log.clone(),
                     request.clone(),
                     services.clone(),
+                    storage.clone(),
                 )
                 .await
                 .map_err(|e| {