PHD: refactor & add support Propolis server "environments"

.github/buildomat/jobs/phd-build.sh

@@ -7,6 +7,16 @@
 #: output_rules = [
 #:   "/out/*",
 #: ]
+#:
+#: [[publish]]
+#: series = "phd_build"
+#: name = "propolis-server-debug.tar.gz"
+#: from_output = "/out/propolis-server-debug.tar.gz"
+#:
+#: [[publish]]
+#: series = "phd_build"
+#: name = "propolis-server-debug.sha256.txt"
+#: from_output = "/out/propolis-server-debug.sha256.txt"
 
 set -o errexit
 set -o pipefail

phd-tests/framework/src/artifacts/mod.rs

@@ -11,6 +11,8 @@ mod store;
 
 pub use store::Store as ArtifactStore;
 
+pub const DEFAULT_PROPOLIS_ARTIFACT: &str = "__DEFAULT_PROPOLIS";
+
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
 enum ArtifactKind {

phd-tests/framework/src/artifacts/store.rs

@@ -3,7 +3,9 @@
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 use crate::{
-    artifacts::{manifest::Manifest, ArtifactSource},
+    artifacts::{
+        manifest::Manifest, ArtifactSource, DEFAULT_PROPOLIS_ARTIFACT,
+    },
     guest_os::GuestOsKind,
 };
 
@@ -24,6 +26,10 @@ struct StoredArtifact {
 }
 
 impl StoredArtifact {
+    fn new(description: super::Artifact) -> Self {
+        Self { description, cached_path: None }
+    }
+
     fn ensure(
         &mut self,
         local_dir: &Utf8Path,
@@ -194,22 +200,56 @@ impl Store {
         let Manifest { artifacts, remote_server_uris } = manifest;
         let artifacts = artifacts
             .into_iter()
-            .map(|(k, v)| {
-                (
-                    k,
-                    Mutex::new(StoredArtifact {
-                        description: v,
-                        cached_path: None,
-                    }),
-                )
-            })
+            .map(|(k, v)| (k, Mutex::new(StoredArtifact::new(v))))
             .collect();
 
         let store = Self { local_dir, artifacts, remote_server_uris };
         info!(?store, "Created new artifact store from manifest");
         store
     }
 
+    pub fn add_propolis_from_local_cmd(
+        &mut self,
+        propolis_server_cmd: &Utf8Path,
+    ) -> anyhow::Result<()> {
+        if self.artifacts.contains_key(DEFAULT_PROPOLIS_ARTIFACT) {
+            anyhow::bail!(
+                "artifact store already contains key {}",
+                DEFAULT_PROPOLIS_ARTIFACT
+            );
+        }
+
+        let full_path = propolis_server_cmd.canonicalize_utf8()?;
+        let filename = full_path.file_name().ok_or_else(|| {
+            anyhow::anyhow!(
+                "Propolis server command '{}' contains no file component",
+                propolis_server_cmd
+            )
+        })?;
+        let dir = full_path.parent().ok_or_else(|| {
+            anyhow::anyhow!(
+                "canonicalized Propolis path '{}' has no directory component",
+                full_path
+            )
+        })?;
+
+        let artifact = super::Artifact {
+            filename: filename.to_string(),
+            kind: super::ArtifactKind::PropolisServer,
+            source: super::ArtifactSource::LocalPath {
+                path: dir.to_path_buf(),
+                sha256: None,
+            },
+        };
+
+        let _old = self.artifacts.insert(
+            DEFAULT_PROPOLIS_ARTIFACT.to_string(),
+            Mutex::new(StoredArtifact::new(artifact)),
+        );
+        assert!(_old.is_none());
+        Ok(())
+    }
+
     pub fn get_guest_os_image(
         &self,
         artifact_name: &str,
@@ -255,6 +295,28 @@ impl Store {
             )),
         }
     }
+
+    pub fn get_propolis_server(
+        &self,
+        artifact_name: &str,
+    ) -> anyhow::Result<Utf8PathBuf> {
+        let entry = if let Some(e) = self.artifacts.get(artifact_name) {
+            e
+        } else {
+            anyhow::bail!("artifact {} not found in store", artifact_name);
+        };
+
+        let mut guard = entry.lock().unwrap();
+        match guard.description.kind {
+            super::ArtifactKind::PropolisServer => {
+                guard.ensure(&self.local_dir, &self.remote_server_uris)
+            }
+            _ => Err(anyhow::anyhow!(
+                "artifact {} is not a Propolis server",
+                artifact_name
+            )),
+        }
+    }
 }
 
 fn sha256_digest(file: &mut File) -> anyhow::Result<Digest> {

phd-tests/framework/src/disk/crucible.rs

@@ -57,6 +57,9 @@ impl Drop for Downstairs {
 /// An RAII wrapper around a Crucible disk.
 #[derive(Debug)]
 pub struct CrucibleDisk {
+    /// The name of the backend to use in instance specs that include this disk.
+    backend_name: String,
+
     /// The UUID to insert into this disk's `VolumeConstructionRequest`s.
     id: Uuid,
 
@@ -91,6 +94,7 @@ impl CrucibleDisk {
     /// `data_dir`.
     #[allow(clippy::too_many_arguments)]
     pub(crate) fn new(
+        backend_name: String,
         disk_size_gib: u64,
         block_size: BlockSize,
         downstairs_binary_path: &impl AsRef<std::ffi::OsStr>,
@@ -205,6 +209,7 @@ impl CrucibleDisk {
         }
 
         Ok(Self {
+            backend_name,
             id: disk_uuid,
             block_size,
             blocks_per_extent,
@@ -233,7 +238,7 @@ impl CrucibleDisk {
 }
 
 impl super::DiskConfig for CrucibleDisk {
-    fn backend_spec(&self) -> instance_spec::v0::StorageBackendV0 {
+    fn backend_spec(&self) -> (String, instance_spec::v0::StorageBackendV0) {
         let gen = self.generation.load(Ordering::Relaxed);
         let downstairs_addrs =
             self.downstairs_instances.iter().map(|ds| ds.address).collect();
@@ -268,18 +273,25 @@ impl super::DiskConfig for CrucibleDisk {
             }),
         };
 
-        instance_spec::v0::StorageBackendV0::Crucible(
-            instance_spec::components::backends::CrucibleStorageBackend {
-                request_json: serde_json::to_string(&vcr)
-                    .expect("VolumeConstructionRequest should serialize"),
-                readonly: false,
-            },
+        (
+            self.backend_name.clone(),
+            instance_spec::v0::StorageBackendV0::Crucible(
+                instance_spec::components::backends::CrucibleStorageBackend {
+                    request_json: serde_json::to_string(&vcr)
+                        .expect("VolumeConstructionRequest should serialize"),
+                    readonly: false,
+                },
+            ),
         )
     }
 
     fn guest_os(&self) -> Option<GuestOsKind> {
         self.guest_os
     }
+
+    fn as_crucible(&self) -> Option<&CrucibleDisk> {
+        Some(self)
+    }
 }
 
 fn run_crucible_downstairs(

phd-tests/framework/src/disk/file.rs

@@ -17,6 +17,9 @@ use crate::guest_os::GuestOsKind;
 /// An RAII wrapper for a disk wrapped by a file.
 #[derive(Debug)]
 pub struct FileBackedDisk {
+    /// The name to use for instance spec backends that refer to this disk.
+    backend_name: String,
+
     /// The path at which the disk is stored.
     disk_path: PathBuf,
 
@@ -29,6 +32,7 @@ impl FileBackedDisk {
     /// Creates a new file-backed disk whose initial contents are copied from
     /// the specified artifact on the host file system.
     pub(crate) fn new_from_artifact(
+        backend_name: String,
         artifact_path: &impl AsRef<Path>,
         data_dir: &impl AsRef<Path>,
         guest_os: Option<GuestOsKind>,
@@ -55,16 +59,19 @@ impl FileBackedDisk {
         permissions.set_readonly(false);
         disk_file.set_permissions(permissions)?;
 
-        Ok(Self { disk_path, guest_os })
+        Ok(Self { backend_name, disk_path, guest_os })
     }
 }
 
 impl super::DiskConfig for FileBackedDisk {
-    fn backend_spec(&self) -> StorageBackendV0 {
-        StorageBackendV0::File(FileStorageBackend {
-            path: self.disk_path.to_string_lossy().to_string(),
-            readonly: false,
-        })
+    fn backend_spec(&self) -> (String, StorageBackendV0) {
+        (
+            self.backend_name.clone(),
+            StorageBackendV0::File(FileStorageBackend {
+                path: self.disk_path.to_string_lossy().to_string(),
+                readonly: false,
+            }),
+        )
     }
 
     fn guest_os(&self) -> Option<GuestOsKind> {

phd-tests/framework/src/disk/mod.rs

@@ -10,6 +10,7 @@
 
 use std::{
     path::{Path, PathBuf},
+    rc::Rc,
     sync::Arc,
 };
 
@@ -63,11 +64,17 @@ impl BlockSize {
 /// A trait for functions exposed by all disk backends (files, Crucible, etc.).
 pub trait DiskConfig: std::fmt::Debug {
     /// Yields the backend spec for this disk's storage backend.
-    fn backend_spec(&self) -> StorageBackendV0;
+    fn backend_spec(&self) -> (String, StorageBackendV0);
 
     /// Yields the guest OS kind of the guest image the disk was created from,
     /// or `None` if the disk was not created from a guest image.
     fn guest_os(&self) -> Option<GuestOsKind>;
+
+    /// If this disk is a Crucible disk, yields `Some` reference to that disk as
+    /// a Crucible disk.
+    fn as_crucible(&self) -> Option<&CrucibleDisk> {
+        None
+    }
 }
 
 /// The possible sources for a disk's initial data.
@@ -100,35 +107,35 @@ pub enum DiskSource<'a> {
 ///      multiple VMs do use a single set of backend resources, the resulting
 ///      behavior will depend on the chosen backend's semantics and the way the
 ///      Propolis backend implementations interact with the disk.
-pub struct DiskFactory<'a> {
+pub(crate) struct DiskFactory {
     /// The directory in which disk files should be stored.
     storage_dir: PathBuf,
 
     /// A reference to the artifact store to use to look up guest OS artifacts
     /// when those are used as a disk source.
-    artifact_store: &'a ArtifactStore,
+    artifact_store: Rc<ArtifactStore>,
 
     /// The path to the Crucible downstairs binary to launch to serve Crucible
     /// disks.
     crucible_downstairs_binary: Option<PathBuf>,
 
     /// The port allocator to use to allocate ports to Crucible server
     /// processes.
-    port_allocator: &'a PortAllocator,
+    port_allocator: Rc<PortAllocator>,
 
     /// The logging discipline to use for Crucible server processes.
     log_mode: ServerLogMode,
 }
 
-impl<'a> DiskFactory<'a> {
+impl DiskFactory {
     /// Creates a new disk factory. The disks this factory generates will store
     /// their data in `storage_dir` and will look up guest OS images in the
     /// supplied `artifact_store`.
     pub fn new(
         storage_dir: &impl AsRef<Path>,
-        artifact_store: &'a ArtifactStore,
+        artifact_store: Rc<ArtifactStore>,
         crucible_downstairs_binary: Option<&impl AsRef<Path>>,
-        port_allocator: &'a PortAllocator,
+        port_allocator: Rc<PortAllocator>,
         log_mode: ServerLogMode,
     ) -> Self {
         Self {
@@ -140,9 +147,13 @@ impl<'a> DiskFactory<'a> {
             log_mode,
         }
     }
+
+    pub fn crucible_enabled(&self) -> bool {
+        self.crucible_downstairs_binary.is_some()
+    }
 }
 
-impl DiskFactory<'_> {
+impl DiskFactory {
     fn get_guest_artifact_info(
         &self,
         artifact_name: &str,
@@ -158,15 +169,17 @@ impl DiskFactory<'_> {
 
     /// Creates a new disk backed by a file whose initial contents are specified
     /// by `source`.
-    pub fn create_file_backed_disk(
+    pub(crate) fn create_file_backed_disk(
         &self,
+        name: String,
         source: DiskSource,
     ) -> Result<Arc<FileBackedDisk>, DiskError> {
         let DiskSource::Artifact(artifact_name) = source;
         let (artifact_path, guest_os) =
             self.get_guest_artifact_info(artifact_name)?;
 
         FileBackedDisk::new_from_artifact(
+            name,
             &artifact_path,
             &self.storage_dir,
             Some(guest_os),
@@ -186,8 +199,9 @@ impl DiskFactory<'_> {
     ///   file should be used as a read-only parent volume.
     /// - disk_size_gib: The disk's expected size in GiB.
     /// - block_size: The disk's block size.
-    pub fn create_crucible_disk(
+    pub(crate) fn create_crucible_disk(
         &self,
+        name: String,
         source: DiskSource,
         disk_size_gib: u64,
         block_size: BlockSize,
@@ -207,6 +221,7 @@ impl DiskFactory<'_> {
         }
 
         CrucibleDisk::new(
+            name,
             disk_size_gib,
             block_size,
             binary_path,