From ed584e833a18b2b5d80ec918b4ca6f3b8de0a646 Mon Sep 17 00:00:00 2001 From: Greg Colombo Date: Wed, 11 Oct 2023 19:59:31 -0700 Subject: [PATCH] PHD: refactor & add support Propolis server "environments" (#547) Rework the way PHD tests create VMs to introduce the notion of an "environment" in which a VM runs. In this model, starting a test VM requires a test to describe both the virtualized guest's configuration (CPUs, memory, disks, and so forth) and the Propolis server environment that should host it (local vs. remote machine, Propolis server version, etc.). Test VMs store their configurations and yield them on request. This allows tests to create "successors" to a test VM that have the same configuration but might run in a different environment. Set up a basic helper for running instance lifecycle tests using the new framework. A test case supplies a VM, a list of actions (reboot the VM, start and stop it in a new Propolis, or migrate it to a new environment), and a closure that checks invariants after each action. This allows test authors to write low-boilerplate tests that check that an invariant holds after, say, migrating from an older Propolis version to a new one, or from one test machine to another. Add a "starter" test case that runs `lspci` and `lshw` in a guest and verifies that the results are consistent across a start/stop transition. To support the notion of migrating between Propolis versions, environments specify Propolis binaries as artifacts. To maintain compatibility with the test runner's `--propolis-server-cmd` switch, add artifact store code to import a runner-supplied Propolis server as an artifact with a well-known name that test cases can refer to. Future changes will add additional well-known Propolis binaries like "whatever binary Buildomat says is at the head of the main Propolis branch." Change the `phd-build` job to publish the Propolis server binaries it builds. This is necessary to refer to them as Buildomat artifacts in the artifact store. (This needs to exist in at least one build to enable further development of the "implicit Buildomat HEAD artifact" series of changes.) Finally, touch up the way test cases refer to disks. Previously, tests created disk handles and passed them to the VM configuration builder. Now, tests just have to define a VM configuration and can ignore the "disk handle" abstraction if they don't use it. The tradeoff is that tests that want to work with interfaces specific to Crucible disks need to explicitly cast their disk handles to that type, but this seems like a small price to pay to make other tests more concise. Tested with local PHD runs. --- .github/buildomat/jobs/phd-build.sh | 10 + phd-tests/README.md | 7 +- phd-tests/framework/src/artifacts/mod.rs | 2 + phd-tests/framework/src/artifacts/store.rs | 96 +++++-- phd-tests/framework/src/disk/crucible.rs | 26 +- phd-tests/framework/src/disk/file.rs | 19 +- phd-tests/framework/src/disk/mod.rs | 35 ++- phd-tests/framework/src/lib.rs | 211 ++++++++++++++- phd-tests/framework/src/lifecycle.rs | 100 +++++++ phd-tests/framework/src/test_vm/config.rs | 249 ++++++++++++++++++ .../framework/src/test_vm/environment.rs | 94 +++++++ phd-tests/framework/src/test_vm/factory.rs | 187 ------------- phd-tests/framework/src/test_vm/mod.rs | 138 +++++++--- phd-tests/framework/src/test_vm/server.rs | 43 +-- phd-tests/framework/src/test_vm/spec.rs | 59 +++++ phd-tests/framework/src/test_vm/vm_config.rs | 215 --------------- phd-tests/runner/src/execute.rs | 4 +- phd-tests/runner/src/fixtures.rs | 8 +- phd-tests/runner/src/main.rs | 45 +--- phd-tests/testcase/src/lib.rs | 42 +-- phd-tests/tests/src/crucible/migrate.rs | 47 ++-- phd-tests/tests/src/crucible/mod.rs | 63 +++-- phd-tests/tests/src/crucible/smoke.rs | 45 ++-- phd-tests/tests/src/hw.rs | 27 ++ phd-tests/tests/src/lib.rs | 1 + phd-tests/tests/src/migrate.rs | 57 ++-- phd-tests/tests/src/server_state_machine.rs | 28 +- phd-tests/tests/src/smoke.rs | 26 +- 28 files changed, 1145 insertions(+), 739 deletions(-) create mode 100644 phd-tests/framework/src/lifecycle.rs create mode 100644 phd-tests/framework/src/test_vm/config.rs create mode 100644 phd-tests/framework/src/test_vm/environment.rs delete mode 100644 phd-tests/framework/src/test_vm/factory.rs create mode 100644 phd-tests/framework/src/test_vm/spec.rs delete mode 100644 phd-tests/framework/src/test_vm/vm_config.rs create mode 100644 phd-tests/tests/src/hw.rs diff --git a/.github/buildomat/jobs/phd-build.sh b/.github/buildomat/jobs/phd-build.sh index 8f5a8578c..d8be34556 100644 --- a/.github/buildomat/jobs/phd-build.sh +++ b/.github/buildomat/jobs/phd-build.sh @@ -7,6 +7,16 @@ #: output_rules = [ #: "/out/*", #: ] +#: +#: [[publish]] +#: series = "phd_build" +#: name = "propolis-server.tar.gz" +#: from_output = "/out/propolis-server-debug.tar.gz" +#: +#: [[publish]] +#: series = "phd_build" +#: name = "propolis-server.sha256.txt" +#: from_output = "/out/propolis-server-debug.sha256.txt" set -o errexit set -o pipefail diff --git a/phd-tests/README.md b/phd-tests/README.md index 1b02f6cc8..4c372fcaa 100644 --- a/phd-tests/README.md +++ b/phd-tests/README.md @@ -136,7 +136,7 @@ path = "/home/oxide/propolis/target/debug" ## Authoring tests PHD's test cases live in the `tests` crate. To write a new test, add a function -of the form `fn my_test(ctx: &TestContext)` and tag it with the +of the form `fn my_test(ctx: &Framework)` and tag it with the `#[phd_testcase]` attribute macro. The framework will automatically register the test into the crate's test inventory for the runner to discover. @@ -156,8 +156,9 @@ function body is reached. This means that ### Test context -The `TestContext` structure contains a `VmFactory` helper object that tests can -use to construct VMs. See the module documentation for more information. +Every test gets a `phd_testcase::Framework` that contains helper methods for +constructing VMs and their execution environments. See the module documentation +for more information. The tests in `tests/src/smoke.rs` provide some simple examples of using the factory to customize and launch a new VM. diff --git a/phd-tests/framework/src/artifacts/mod.rs b/phd-tests/framework/src/artifacts/mod.rs index 36603b5fd..0659607ec 100644 --- a/phd-tests/framework/src/artifacts/mod.rs +++ b/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 { diff --git a/phd-tests/framework/src/artifacts/store.rs b/phd-tests/framework/src/artifacts/store.rs index adae978ee..f10e58223 100644 --- a/phd-tests/framework/src/artifacts/store.rs +++ b/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,15 +200,7 @@ 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 }; @@ -210,15 +208,55 @@ impl Store { 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, ) -> anyhow::Result<(Utf8PathBuf, GuestOsKind)> { - let entry = if let Some(e) = self.artifacts.get(artifact_name) { - e - } else { - anyhow::bail!("artifact {} not found in store", artifact_name); - }; + let entry = self.artifacts.get(artifact_name).ok_or_else(|| { + anyhow::anyhow!("artifact {} not found in store", artifact_name) + })?; let mut guard = entry.lock().unwrap(); match guard.description.kind { @@ -238,11 +276,9 @@ impl Store { &self, artifact_name: &str, ) -> anyhow::Result { - let entry = if let Some(e) = self.artifacts.get(artifact_name) { - e - } else { - anyhow::bail!("artifact {} not found in store", artifact_name); - }; + let entry = self.artifacts.get(artifact_name).ok_or_else(|| { + anyhow::anyhow!("artifact {} not found in store", artifact_name) + })?; let mut guard = entry.lock().unwrap(); match guard.description.kind { @@ -255,6 +291,26 @@ impl Store { )), } } + + pub fn get_propolis_server( + &self, + artifact_name: &str, + ) -> anyhow::Result { + let entry = self.artifacts.get(artifact_name).ok_or_else(|| { + anyhow::anyhow!("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 { diff --git a/phd-tests/framework/src/disk/crucible.rs b/phd-tests/framework/src/disk/crucible.rs index ec2dbb30a..620ede37b 100644 --- a/phd-tests/framework/src/disk/crucible.rs +++ b/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, @@ -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 { self.guest_os } + + fn as_crucible(&self) -> Option<&CrucibleDisk> { + Some(self) + } } fn run_crucible_downstairs( diff --git a/phd-tests/framework/src/disk/file.rs b/phd-tests/framework/src/disk/file.rs index 9f19d9924..33b084f8b 100644 --- a/phd-tests/framework/src/disk/file.rs +++ b/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, data_dir: &impl AsRef, guest_os: Option, @@ -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 { diff --git a/phd-tests/framework/src/disk/mod.rs b/phd-tests/framework/src/disk/mod.rs index a75163e13..a02eb86fa 100644 --- a/phd-tests/framework/src/disk/mod.rs +++ b/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; + + /// 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,13 +107,13 @@ 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, /// The path to the Crucible downstairs binary to launch to serve Crucible /// disks. @@ -114,21 +121,21 @@ pub struct DiskFactory<'a> { /// The port allocator to use to allocate ports to Crucible server /// processes. - port_allocator: &'a PortAllocator, + port_allocator: Rc, /// 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, - artifact_store: &'a ArtifactStore, + artifact_store: Rc, crucible_downstairs_binary: Option<&impl AsRef>, - port_allocator: &'a PortAllocator, + port_allocator: Rc, 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,8 +169,9 @@ 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, DiskError> { let DiskSource::Artifact(artifact_name) = source; @@ -167,6 +179,7 @@ impl DiskFactory<'_> { 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, diff --git a/phd-tests/framework/src/lib.rs b/phd-tests/framework/src/lib.rs index 980e41ded..017f1bbba 100644 --- a/phd-tests/framework/src/lib.rs +++ b/phd-tests/framework/src/lib.rs @@ -4,14 +4,221 @@ //! The Pheidippides framework: interfaces for creating and interacting with //! VMs. +//! +//! This module defines a `Framework` object that contains a set of default VM +//! parameters (shape, bootrom, boot disk image) and the context needed to +//! launch new guest VMs (paths, logging options, an artifact store, etc.). The +//! PHD runner process instantiates a `Framework` and then passes a reference to +//! it to each PHD test case. Test cases then use the `Framework`'s public +//! interface to create test VMs with various configurations. +//! +//! Tests are expected to access `Framework` functions to create VMs and public +//! `TestVm` functions to work directly with those VMs. Most other functionality +//! in this crate is private to the crate. +//! +//! To launch a VM, the framework needs to know how to configure the VM itself +//! and how to run the Propolis server process that will host it. The framework +//! supplies builders, `vm_builder` and `environment_builder`, that allow tests +//! to configure both of these options. +//! +//! Often, tests will want to spawn a "successor" to an existing VM that +//! maintains the VM's configuration and any related objects but that runs in a +//! separate Propolis server process that may have been spawned in a different +//! environment. The `spawn_successor_vm` function provides a shorthand way to +//! do this. + +use std::{ops::Range, rc::Rc}; + +use anyhow::Context; +use artifacts::DEFAULT_PROPOLIS_ARTIFACT; +use camino::Utf8PathBuf; + +use disk::DiskFactory; +use port_allocator::PortAllocator; +use server_log_mode::ServerLogMode; +pub use test_vm::TestVm; +use test_vm::{ + environment::EnvironmentSpec, spec::VmSpec, VmConfig, VmLocation, +}; pub mod artifacts; pub mod disk; pub mod guest_os; pub mod host_api; -pub mod port_allocator; +pub mod lifecycle; +mod port_allocator; mod serial; pub mod server_log_mode; pub mod test_vm; -pub use test_vm::TestVm; +/// An instance of the PHD test framework. +pub struct Framework { + pub(crate) tmp_directory: Utf8PathBuf, + pub(crate) server_log_mode: ServerLogMode, + + pub(crate) default_guest_cpus: u8, + pub(crate) default_guest_memory_mib: u64, + pub(crate) default_guest_os_artifact: String, + pub(crate) default_bootrom_artifact: String, + + // The disk factory used to be a freestanding struct that took references to + // an artifact store and port allocator that were owned by someone else. + // Putting all these components into a single struct makes the struct + // self-referencing. Since the runner is single-threaded, avoid arguing with + // anyone about lifetimes by wrapping the relevant shared components in an + // `Rc`. + pub(crate) artifact_store: Rc, + pub(crate) disk_factory: DiskFactory, + pub(crate) port_allocator: Rc, +} + +pub struct FrameworkParameters { + pub propolis_server_path: Utf8PathBuf, + pub crucible_downstairs_cmd: Option, + + pub tmp_directory: Utf8PathBuf, + pub artifact_toml: Utf8PathBuf, + pub server_log_mode: ServerLogMode, + + pub default_guest_cpus: u8, + pub default_guest_memory_mib: u64, + pub default_guest_os_artifact: String, + pub default_bootrom_artifact: String, + + pub port_range: Range, +} + +// The framework implementation includes some "runner-only" functions +// (constructing and resetting a framework) that are marked `pub`. This could be +// improved by splitting the "test case" functions into a trait and giving test +// cases trait objects. +impl Framework { + /// Builds a brand new framework. Called from the test runner, which creates + /// one framework and then distributes it to tests. + pub fn new(params: FrameworkParameters) -> anyhow::Result { + let mut artifact_store = artifacts::ArtifactStore::from_toml_path( + params.tmp_directory.clone(), + ¶ms.artifact_toml, + ) + .context("creating PHD framework")?; + + artifact_store + .add_propolis_from_local_cmd(¶ms.propolis_server_path) + .with_context(|| { + format!( + "adding Propolis server '{}' from options", + ¶ms.propolis_server_path + ) + })?; + + let artifact_store = Rc::new(artifact_store); + let port_allocator = Rc::new(PortAllocator::new(params.port_range)); + let disk_factory = DiskFactory::new( + ¶ms.tmp_directory, + artifact_store.clone(), + params.crucible_downstairs_cmd.clone().as_ref(), + port_allocator.clone(), + params.server_log_mode, + ); + + Ok(Self { + tmp_directory: params.tmp_directory, + server_log_mode: params.server_log_mode, + default_guest_cpus: params.default_guest_cpus, + default_guest_memory_mib: params.default_guest_memory_mib, + default_guest_os_artifact: params.default_guest_os_artifact, + default_bootrom_artifact: params.default_bootrom_artifact, + artifact_store, + disk_factory, + port_allocator, + }) + } + + /// Resets the state of any stateful objects in the framework to prepare it + /// to run a new test case. + pub fn reset(&self) { + self.port_allocator.reset(); + } + + /// Creates a new VM configuration builder using the default configuration + /// from this framework instance. + pub fn vm_config_builder(&self, vm_name: &str) -> VmConfig { + VmConfig::new( + vm_name, + self.default_guest_cpus, + self.default_guest_memory_mib, + &self.default_bootrom_artifact, + &self.default_guest_os_artifact, + ) + } + + /// Yields an environment builder with default settings (run the VM on the + /// test runner's machine using the default Propolis from the command line). + pub fn environment_builder(&self) -> EnvironmentSpec { + EnvironmentSpec::new(VmLocation::Local, DEFAULT_PROPOLIS_ARTIFACT) + } + + /// Spawns a test VM using the default configuration returned from + /// `vm_builder` and the default environment returned from + /// `environment_builder`. + pub fn spawn_default_vm(&self, vm_name: &str) -> anyhow::Result { + self.spawn_vm(&self.vm_config_builder(vm_name), None) + } + + /// Spawns a new test VM using the supplied `config`. If `environment` is + /// `Some`, the VM is spawned using the supplied environment; otherwise it + /// is spawned using the default `environment_builder`. + pub fn spawn_vm( + &self, + config: &VmConfig, + environment: Option<&EnvironmentSpec>, + ) -> anyhow::Result { + TestVm::new( + self, + config.vm_spec(self).context("building VM config for test VM")?, + environment.unwrap_or(&self.environment_builder()), + ) + .context("constructing test VM") + } + + /// Spawns a "successor" to the supplied `vm`. The successor has the same + /// configuration and takes additional references to all of its + /// predecessor's backing objects (e.g. disk handles). If `environment` is + /// `None`, the successor is launched using the predecessor's environment + /// spec. + pub fn spawn_successor_vm( + &self, + vm_name: &str, + vm: &TestVm, + environment: Option<&EnvironmentSpec>, + ) -> anyhow::Result { + let mut vm_spec = + VmSpec { vm_name: vm_name.to_owned(), ..vm.vm_spec() }; + + // Reconcile any differences between the generation numbers in the VM + // objects' instance spec and the associated Crucible disk handles. + // This may be needed because a test can call `set_generation` on a disk + // handle to change its active generation number mid-test, and this + // won't automatically be reflected in the VM's instance spec. + vm_spec.refresh_crucible_backends(); + TestVm::new( + self, + vm_spec, + environment.unwrap_or(&vm.environment_spec()), + ) + } + + /// Yields this framework instance's default guest OS artifact name. This + /// can be used to configure boot disks with different parameters than the + /// builder defaults. + pub fn default_guest_os_artifact(&self) -> &str { + &self.default_guest_os_artifact + } + + /// Indicates whether the disk factory in this framework supports the + /// creation of Crucible disks. This can be used to skip tests that require + /// Crucible support. + pub fn crucible_enabled(&self) -> bool { + self.disk_factory.crucible_enabled() + } +} diff --git a/phd-tests/framework/src/lifecycle.rs b/phd-tests/framework/src/lifecycle.rs new file mode 100644 index 000000000..c4635991b --- /dev/null +++ b/phd-tests/framework/src/lifecycle.rs @@ -0,0 +1,100 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use std::time::Duration; + +use tracing::info; +use uuid::Uuid; + +use crate::{Framework, TestVm}; + +/// The set of actions that can be taken on a VM undergoing lifecycle testing. +pub enum Action<'a> { + /// Reset the VM using the Propolis server reset API. This sort of reboot + /// does not involve the guest OS. It can be used to verify that components' + /// reset implementations don't change properties that shouldn't change + /// without fully stopping and restarting a VM. + // + // N.B. This isn't used in any lifecycle tests yet. + Reset, + + /// Stop the VM and restart it in a successor Propolis using the same + /// environment as its predecessor. + StopAndStart, + + /// Migrate the VM to a new Propolis server. The wrapped `&str` names a + /// Propolis server artifact to migrate to. + // + // N.B. This isn't used in any lifecycle tests yet, mostly because there are + // no well-known, stable Propolis artifact names other than the name of the + // default artifact supplied on the command line. This will change in the + // future as new well-known artifacts (like "Buildomat HEAD") are added. + MigrateToPropolis(&'a str), +} + +impl Framework { + /// Runs a lifecycle test on the supplied `vm` by iterating over the + /// `actions`, performing the specified action, and then calling `check_fn` + /// on the resulting VM to verify invariants. + pub fn lifecycle_test( + &self, + vm: TestVm, + actions: &[Action], + check_fn: impl Fn(&TestVm), + ) -> anyhow::Result<()> { + let mut vm = vm; + let original_name = vm.name().to_owned(); + for (idx, action) in actions.iter().enumerate() { + match action { + Action::Reset => { + info!( + vm_name = original_name, + "rebooting VM for lifecycle test" + ); + vm.reset()?; + } + Action::StopAndStart => { + info!( + vm_name = original_name, + "stopping and starting VM for lifecycle test" + ); + let new_vm_name = + format!("{}_lifecycle_{}", original_name, idx); + vm.stop()?; + let mut new_vm = + self.spawn_successor_vm(&new_vm_name, &vm, None)?; + new_vm.launch()?; + new_vm.wait_to_boot()?; + vm = new_vm; + } + Action::MigrateToPropolis(propolis) => { + info!( + vm_name = original_name, + propolis_artifact = propolis, + "migrating to new Propolis artifact for lifecycle test" + ); + + let new_vm_name = + format!("{}_lifecycle_{}", original_name, idx); + + let mut env = self.environment_builder(); + env.propolis(propolis); + let mut new_vm = + self.spawn_successor_vm(&new_vm_name, &vm, Some(&env))?; + + new_vm.migrate_from( + &vm, + Uuid::new_v4(), + Duration::from_secs(120), + )?; + vm = new_vm; + } + } + + check_fn(&vm); + } + + Ok(()) + } +} diff --git a/phd-tests/framework/src/test_vm/config.rs b/phd-tests/framework/src/test_vm/config.rs new file mode 100644 index 000000000..153852a97 --- /dev/null +++ b/phd-tests/framework/src/test_vm/config.rs @@ -0,0 +1,249 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use std::sync::Arc; + +use anyhow::Context; +use propolis_client::instance_spec::{ + components as spec_components, v0::StorageDeviceV0, +}; +use propolis_types::PciPath; + +use crate::{ + disk::{DiskConfig, DiskSource}, + test_vm::spec::VmSpec, + Framework, +}; + +/// The disk interface to use for a given guest disk. +#[derive(Clone, Copy, Debug)] +pub enum DiskInterface { + Virtio, + Nvme, +} + +#[derive(Clone, Copy, Debug)] +pub enum DiskBackend { + File, + Crucible { disk_size_gib: u64, block_size: crate::disk::BlockSize }, +} + +#[derive(Clone, Debug)] +struct DiskRequest { + interface: DiskInterface, + backend: DiskBackend, + source_artifact: String, + pci_device_num: u8, +} + +pub struct VmConfig { + vm_name: String, + cpus: u8, + memory_mib: u64, + bootrom_artifact: String, + boot_disk: DiskRequest, + data_disks: Vec, +} + +impl VmConfig { + pub(crate) fn new( + vm_name: &str, + cpus: u8, + memory_mib: u64, + bootrom: &str, + guest_artifact: &str, + ) -> Self { + let boot_disk = DiskRequest { + interface: DiskInterface::Nvme, + backend: DiskBackend::File, + source_artifact: guest_artifact.to_owned(), + pci_device_num: 4, + }; + + Self { + vm_name: vm_name.to_owned(), + cpus, + memory_mib, + bootrom_artifact: bootrom.to_owned(), + boot_disk, + data_disks: Vec::new(), + } + } + + pub fn cpus(&mut self, cpus: u8) -> &mut Self { + self.cpus = cpus; + self + } + + pub fn memory_mib(&mut self, mem: u64) -> &mut Self { + self.memory_mib = mem; + self + } + + pub fn bootrom(&mut self, artifact: &str) -> &mut Self { + self.bootrom_artifact = artifact.to_owned(); + self + } + + pub fn boot_disk( + &mut self, + artifact: &str, + interface: DiskInterface, + backend: DiskBackend, + pci_device_num: u8, + ) -> &mut Self { + self.boot_disk = DiskRequest { + interface, + backend, + source_artifact: artifact.to_owned(), + pci_device_num, + }; + self + } + + pub fn data_disk( + &mut self, + artifact: &str, + interface: DiskInterface, + backend: DiskBackend, + pci_device_num: u8, + ) -> &mut Self { + self.data_disks.push(DiskRequest { + interface, + backend, + source_artifact: artifact.to_owned(), + pci_device_num, + }); + self + } + + pub(crate) fn vm_spec( + &self, + framework: &Framework, + ) -> anyhow::Result { + // Figure out where the bootrom is and generate the serialized contents + // of a Propolis server config TOML that points to it. + let bootrom = framework + .artifact_store + .get_bootrom(&self.bootrom_artifact) + .context("looking up bootrom artifact")?; + + let config_toml_contents = + toml::ser::to_string(&propolis_server_config::Config { + bootrom: bootrom.clone().into(), + ..Default::default() + }) + .context("serializing Propolis server config")?; + + let (_, guest_os_kind) = framework + .artifact_store + .get_guest_os_image(&self.boot_disk.source_artifact) + .context("getting guest OS kind for boot disk")?; + + // This closure helps to construct disk handles from disk requests. + let make_disk = |name: String, + req: &DiskRequest| + -> anyhow::Result> { + let source = DiskSource::Artifact(&req.source_artifact); + Ok(match req.backend { + DiskBackend::File => framework + .disk_factory + .create_file_backed_disk(name, source) + .with_context(|| { + format!( + "creating new file-backed disk from '{}'", + &req.source_artifact + ) + })?, + DiskBackend::Crucible { disk_size_gib, block_size } => { + framework + .disk_factory + .create_crucible_disk( + name, + source, + disk_size_gib, + block_size, + ) + .with_context(|| { + format!( + "creating new Crucible-backed disk from \ + '{}'", + &req.source_artifact + ) + })? + } + }) + }; + + let mut disk_handles = Vec::new(); + disk_handles.push( + make_disk("boot-disk".to_owned(), &self.boot_disk) + .context("creating boot disk")?, + ); + for (idx, data_disk) in self.data_disks.iter().enumerate() { + disk_handles.push( + make_disk(format!("data-disk-{}", idx), data_disk) + .context("creating data disk")?, + ); + } + + let mut spec_builder = + propolis_client::instance_spec::v0::builder::SpecBuilder::new( + self.cpus, + self.memory_mib, + false, + ); + + // Iterate over the collection of disks and handles and add spec + // elements for all of them. This assumes the disk handles were created + // in the correct order: boot disk first, then in the data disks' + // iteration order. + let all_disks = [&self.boot_disk] + .into_iter() + .chain(self.data_disks.iter()) + .zip(disk_handles.iter()); + for (idx, (req, hdl)) in all_disks.enumerate() { + let device_name = format!("disk-device{}", idx); + let pci_path = PciPath::new(0, req.pci_device_num, 0).unwrap(); + let (backend_name, backend_spec) = hdl.backend_spec(); + let device_spec = match req.interface { + DiskInterface::Virtio => StorageDeviceV0::VirtioDisk( + spec_components::devices::VirtioDisk { + backend_name: backend_name.clone(), + pci_path, + }, + ), + DiskInterface::Nvme => StorageDeviceV0::NvmeDisk( + spec_components::devices::NvmeDisk { + backend_name: backend_name.clone(), + pci_path, + }, + ), + }; + + spec_builder + .add_storage_device( + device_name, + device_spec, + backend_name, + backend_spec, + ) + .context("adding storage device to spec")?; + } + + spec_builder + .add_serial_port(spec_components::devices::SerialPortNumber::Com1) + .context("adding serial port to spec")?; + + let instance_spec = spec_builder.finish(); + + Ok(VmSpec { + vm_name: self.vm_name.clone(), + instance_spec: instance_spec.into(), + disk_handles, + guest_os_kind, + config_toml_contents, + }) + } +} diff --git a/phd-tests/framework/src/test_vm/environment.rs b/phd-tests/framework/src/test_vm/environment.rs new file mode 100644 index 000000000..4b03ff516 --- /dev/null +++ b/phd-tests/framework/src/test_vm/environment.rs @@ -0,0 +1,94 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use std::net::{Ipv4Addr, SocketAddrV4}; + +use anyhow::Context; + +use crate::{test_vm::server::ServerProcessParameters, Framework}; + +/// Specifies where the framework should start a new test VM. +#[derive(Clone, Copy, Debug)] +pub enum VmLocation { + /// Start the VM on the system where the test runner is executing. + Local, + // TODO: Support remote VMs. +} + +#[derive(Clone, Debug)] +pub struct EnvironmentSpec { + pub(crate) location: VmLocation, + pub(crate) propolis_artifact: String, +} + +impl EnvironmentSpec { + pub(crate) fn new(location: VmLocation, propolis_artifact: &str) -> Self { + Self { location, propolis_artifact: propolis_artifact.to_owned() } + } + + pub fn location(&mut self, location: VmLocation) -> &mut Self { + self.location = location; + self + } + + pub fn propolis(&mut self, artifact_name: &str) -> &mut Self { + self.propolis_artifact = artifact_name.to_owned(); + self + } + + pub(crate) fn build<'a>( + &self, + framework: &'a Framework, + ) -> anyhow::Result> { + Environment::from_builder(self, framework) + } +} + +/// Specifies all of the details the framework needs to stand up a VM in a +/// specific environment. +/// +/// When tests want to spawn a new VM, they pass a `VmLocation` to the +/// framework, and the framework augments that with +#[derive(Clone, Debug)] +pub(crate) enum Environment<'a> { + Local(ServerProcessParameters<'a>), +} + +impl<'a> Environment<'a> { + fn from_builder( + builder: &EnvironmentSpec, + framework: &'a Framework, + ) -> anyhow::Result { + match builder.location { + VmLocation::Local => { + let propolis_server = framework + .artifact_store + .get_propolis_server(&builder.propolis_artifact) + .context("setting up VM execution environment")?; + let server_port = framework + .port_allocator + .next() + .context("getting Propolis server port")?; + let vnc_port = framework + .port_allocator + .next() + .context("getting VNC server port")?; + let params = ServerProcessParameters { + server_path: propolis_server, + data_dir: framework.tmp_directory.as_path(), + server_addr: SocketAddrV4::new( + Ipv4Addr::new(127, 0, 0, 1), + server_port, + ), + vnc_addr: SocketAddrV4::new( + Ipv4Addr::new(127, 0, 0, 1), + vnc_port, + ), + log_mode: framework.server_log_mode, + }; + Ok(Self::Local(params)) + } + } + } +} diff --git a/phd-tests/framework/src/test_vm/factory.rs b/phd-tests/framework/src/test_vm/factory.rs deleted file mode 100644 index 0cf608615..000000000 --- a/phd-tests/framework/src/test_vm/factory.rs +++ /dev/null @@ -1,187 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! Helpers for configuring and starting new VMs. - -use std::{ - net::{Ipv4Addr, SocketAddrV4}, - path::PathBuf, -}; - -use anyhow::{Context, Result}; -use camino::Utf8PathBuf; -use thiserror::Error; -use tracing::info; - -use crate::{ - artifacts::ArtifactStore, port_allocator::PortAllocator, - server_log_mode::ServerLogMode, test_vm::server::ServerProcessParameters, -}; - -use super::{ - vm_config::{self, VmConfig}, - TestVm, -}; - -/// Errors that can arise while creating a VM factory. -#[derive(Debug, Error)] -pub enum FactoryConstructionError { - /// Raised if the default guest image key in the [`FactoryOptions`] does not - /// yield a valid image from the artifact store. - #[error("Default guest image {0} not in artifact store")] - DefaultGuestImageMissing(String), - - /// Raised on a failure to convert from a named server logging mode to a - /// [`ServerLogMode`]. - #[error("Invalid server log mode name '{0}'")] - InvalidServerLogModeName(String), -} - -/// Parameters used to construct a new VM factory. -#[derive(Debug)] -pub struct FactoryOptions { - /// The path to the Propolis server binary to use for VMs created by this - /// factory. - pub propolis_server_path: Utf8PathBuf, - - /// The directory to use as a temporary directory for config TOML files, - /// server logs, and the like. - pub tmp_directory: Utf8PathBuf, - - /// The logging discipline to use for this factory's Propolis servers. - pub server_log_mode: ServerLogMode, - - /// An artifact store key specifying the default bootrom artifact to use for - /// this factory's VMs. - pub default_bootrom_artifact: String, - - /// The default number of CPUs to set in [`vm_config::VmConfig`] structs - /// generated by this factory. - pub default_guest_cpus: u8, - - /// The default amount of memory to set in [`vm_config::VmConfig`] structs - /// generated by this factory. - pub default_guest_memory_mib: u64, -} - -/// A VM factory that provides routines to generate new test VMs. -pub struct VmFactory<'a> { - opts: FactoryOptions, - default_bootrom_path: String, - port_allocator: &'a PortAllocator, -} - -impl<'a> VmFactory<'a> { - /// Creates a new VM factory with default bootrom/guest image artifacts - /// drawn from the supplied artifact store. - pub fn new( - opts: FactoryOptions, - store: &ArtifactStore, - port_allocator: &'a PortAllocator, - ) -> Result { - info!(?opts, "Building VM factory"); - let bootrom_path = store - .get_bootrom(&opts.default_bootrom_artifact) - .with_context(|| { - format!( - "failed to get path to bootrom artifact '{}'", - &opts.default_bootrom_artifact - ) - })?; - - Ok(Self { - opts, - default_bootrom_path: bootrom_path.into_string(), - port_allocator, - }) - } -} - -impl VmFactory<'_> { - /// Resets this factory to the state it had when it was created, preparing - /// it for use in a new test case. - pub fn reset(&self) { - self.port_allocator.reset(); - } - - /// Creates a VM configuration that specifies this factory's defaults for - /// CPUs, memory, and bootrom. - pub fn deviceless_vm_config(&self) -> vm_config::ConfigRequest { - let bootrom_path = - PathBuf::try_from(&self.default_bootrom_path).unwrap(); - vm_config::ConfigRequest::new() - .set_bootrom_path(bootrom_path) - .set_cpus(self.opts.default_guest_cpus) - .set_memory_mib(self.opts.default_guest_memory_mib) - } - - /// Launches a new Propolis server process with a VM configuration created - /// by the supplied configuration builder. Returns the [`TestVm`] associated - /// with this server. - pub fn new_vm( - &self, - vm_name: &str, - config: vm_config::ConfigRequest, - ) -> Result { - info!(?vm_name, ?config, "Configuring VM from request"); - let realized = config.finish( - &self.opts.tmp_directory, - &format!("{}.config.toml", vm_name), - )?; - - self.create_vm_from_config(vm_name, realized) - } - - /// Launches a new Propolis server process with a VM configuration cloned - /// from an existing VM. Returns the [`TestVm`] associated with this server. - /// - /// This is useful for live migration tests where the source and target are - /// expected to have identical configuration data. - pub fn new_vm_from_cloned_config( - &self, - vm_name: &str, - vm_to_clone: &TestVm, - ) -> Result { - let config = vm_to_clone.clone_config(); - let server_toml_path = config.server_toml_path(); - let mut new_toml_path = - server_toml_path.parent().unwrap().to_path_buf(); - new_toml_path.push(format!("{}.config.toml", vm_name)); - info!( - ?server_toml_path, - ?new_toml_path, - "Copying existing server config TOML" - ); - std::fs::copy(server_toml_path, &new_toml_path)?; - - self.create_vm_from_config(vm_name, config) - } - - fn create_vm_from_config( - &self, - vm_name: &str, - vm_config: VmConfig, - ) -> Result { - let (server_stdout, server_stderr) = self - .opts - .server_log_mode - .get_handles(&self.opts.tmp_directory, vm_name)?; - - let server_port = self.port_allocator.next()?; - let vnc_port = self.port_allocator.next()?; - let server_params = ServerProcessParameters { - server_path: self.opts.propolis_server_path.as_str(), - config_toml_path: vm_config.server_toml_path().clone(), - server_addr: SocketAddrV4::new( - Ipv4Addr::new(127, 0, 0, 1), - server_port, - ), - vnc_addr: SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), vnc_port), - server_stdout, - server_stderr, - }; - - TestVm::new(vm_name, server_params, vm_config) - } -} diff --git a/phd-tests/framework/src/test_vm/mod.rs b/phd-tests/framework/src/test_vm/mod.rs index 93566e529..f93ac6bfd 100644 --- a/phd-tests/framework/src/test_vm/mod.rs +++ b/phd-tests/framework/src/test_vm/mod.rs @@ -5,19 +5,24 @@ //! Routines for starting VMs, changing their states, and interacting with their //! guest OSes. -use std::{fmt::Debug, process::Stdio, time::Duration}; - -use crate::guest_os::{self, CommandSequenceEntry, GuestOs, GuestOsKind}; -use crate::serial::SerialConsole; +use std::{fmt::Debug, io::Write, sync::Arc, time::Duration}; + +use crate::{ + guest_os::{self, CommandSequenceEntry, GuestOs, GuestOsKind}, + serial::SerialConsole, + test_vm::{ + environment::Environment, server::ServerProcessParameters, spec::VmSpec, + }, + Framework, +}; use anyhow::{anyhow, Context, Result}; use core::result::Result as StdResult; -use propolis_client::instance_spec::VersionedInstanceSpec; use propolis_client::types::{ InstanceGetResponse, InstanceMigrateInitiateRequest, InstanceProperties, InstanceSerialConsoleHistoryResponse, InstanceSpecEnsureRequest, InstanceSpecGetResponse, InstanceState, InstanceStateRequested, - MigrationState, + MigrationState, VersionedInstanceSpec, }; use propolis_client::{Client, ResponseValue}; use thiserror::Error; @@ -29,11 +34,15 @@ type PropolisClientError = propolis_client::Error; type PropolisClientResult = StdResult, PropolisClientError>; -use self::vm_config::VmConfig; +pub(crate) mod config; +pub(crate) mod environment; +mod server; +pub(crate) mod spec; -pub mod factory; -pub mod server; -pub mod vm_config; +pub use config::*; +pub use environment::VmLocation; + +use self::environment::EnvironmentSpec; #[derive(Debug, Error)] pub enum VmStateError { @@ -62,7 +71,9 @@ pub struct TestVm { rt: tokio::runtime::Runtime, client: Client, server: server::PropolisServer, - config: VmConfig, + spec: VmSpec, + environment_spec: EnvironmentSpec, + guest_os: Box, tracing_span: tracing::Span, @@ -89,41 +100,95 @@ impl TestVm { /// this location. /// - guest_os_kind: The kind of guest OS this VM will host. #[instrument(skip_all)] - pub(crate) fn new + Debug>( - vm_name: &str, - process_params: server::ServerProcessParameters, - vm_config: vm_config::VmConfig, + pub(crate) fn new( + framework: &Framework, + spec: VmSpec, + environment: &EnvironmentSpec, ) -> Result { let id = Uuid::new_v4(); - let guest_os_kind = vm_config.guest_os_kind(); - info!(?process_params, ?vm_config, ?guest_os_kind); - let span = info_span!(parent: None, "VM", vm = ?vm_name, %id); + let guest_os_kind = spec.guest_os_kind; + + let vm_name = &spec.vm_name; + info!(%vm_name, ?spec.instance_spec, ?guest_os_kind, ?environment); + + match environment + .build(framework) + .context("building environment for new VM")? + { + Environment::Local(params) => { + Self::start_local_vm(id, spec, environment.clone(), params) + } + } + } + + fn start_local_vm( + vm_id: Uuid, + vm_spec: VmSpec, + environment_spec: EnvironmentSpec, + params: ServerProcessParameters, + ) -> Result { + let config_filename = format!("{}.config.toml", &vm_spec.vm_name); + let mut config_toml_path = params.data_dir.to_path_buf(); + config_toml_path.push(config_filename); + let mut config_file = std::fs::OpenOptions::new() + .write(true) + .truncate(true) + .create(true) + .open(&config_toml_path) + .with_context(|| { + format!("opening config file {} for writing", config_toml_path) + })?; + + config_file + .write_all(vm_spec.config_toml_contents.as_bytes()) + .with_context(|| { + format!( + "writing config toml to config file {}", + config_toml_path + ) + })?; + + let span = + info_span!(parent: None, "VM", vm = %vm_spec.vm_name, %vm_id); let rt = tokio::runtime::Builder::new_multi_thread().enable_all().build()?; - let server_addr = process_params.server_addr; - let server = server::PropolisServer::new(process_params)?; + let server_addr = params.server_addr; + let server = server::PropolisServer::new( + &vm_spec.vm_name, + params, + &config_toml_path, + )?; let client = Client::new(&format!("http://{}", server_addr)); - + let guest_os = guest_os::get_guest_os_adapter(vm_spec.guest_os_kind); Ok(Self { - id, + id: vm_id, rt, client, server, - config: vm_config, - guest_os: guest_os::get_guest_os_adapter(guest_os_kind), + spec: vm_spec, + environment_spec, + guest_os, tracing_span: span, state: VmState::New, }) } - /// Obtains a clone of the configuration parameters that were supplied when - /// this VM was created so that a new VM can be created from them. - /// - /// N.B. This also clones handles to the backend objects this VM is using. - pub(crate) fn clone_config(&self) -> vm_config::VmConfig { - self.config.clone() + pub fn name(&self) -> &str { + &self.spec.vm_name + } + + pub fn cloned_disk_handles(&self) -> Vec> { + self.spec.disk_handles.clone() + } + + pub(crate) fn vm_spec(&self) -> VmSpec { + self.spec.clone() + } + + pub(crate) fn environment_spec(&self) -> EnvironmentSpec { + self.environment_spec.clone() } /// Sends an instance ensure request to this VM's server, allowing it to @@ -135,8 +200,8 @@ impl TestVm { let _span = self.tracing_span.enter(); let (vcpus, memory_mib) = match self.state { VmState::New => ( - self.config.instance_spec().devices.board.cpus, - self.config.instance_spec().devices.board.memory_mb, + self.spec.instance_spec.devices.board.cpus, + self.spec.instance_spec.devices.board.memory_mb, ), VmState::Ensured { .. } => { return Err(VmStateError::InstanceAlreadyEnsured.into()) @@ -154,10 +219,10 @@ impl TestVm { }; let versioned_spec = - VersionedInstanceSpec::V0(self.config.instance_spec().to_owned()); + VersionedInstanceSpec::V0(self.spec.instance_spec.clone()); let ensure_req = InstanceSpecEnsureRequest { properties, - instance_spec: versioned_spec.into(), + instance_spec: versioned_spec, migrate, }; @@ -189,10 +254,9 @@ impl TestVm { anyhow!("failed to get instance properties") })?; - let instance_spec = self.config.instance_spec(); info!( ?instance_description.instance, - ?instance_spec, + ?self.spec.instance_spec, "Started instance" ); @@ -201,7 +265,7 @@ impl TestVm { /// Returns the kind of guest OS running in this VM. pub fn guest_os_kind(&self) -> GuestOsKind { - self.config.guest_os_kind() + self.spec.guest_os_kind } /// Sets the VM to the running state. If the VM has not yet been launched diff --git a/phd-tests/framework/src/test_vm/server.rs b/phd-tests/framework/src/test_vm/server.rs index 87ae26ec1..660c520c6 100644 --- a/phd-tests/framework/src/test_vm/server.rs +++ b/phd-tests/framework/src/test_vm/server.rs @@ -4,22 +4,25 @@ //! Routines and data structures for working with Propolis server processes. -use std::{fmt::Debug, net::SocketAddrV4, path::PathBuf, process::Stdio}; +use std::{fmt::Debug, net::SocketAddrV4}; use anyhow::Result; +use camino::{Utf8Path, Utf8PathBuf}; use tracing::info; +use crate::server_log_mode::ServerLogMode; + /// Parameters used to launch and configure the Propolis server process. These /// are distinct from the parameters used to configure the VM that that process /// will host. -#[derive(Debug)] -pub struct ServerProcessParameters<'a, T: Into> { +#[derive(Clone, Debug)] +pub struct ServerProcessParameters<'a> { /// The path to the server binary to launch. - pub server_path: &'a str, + pub server_path: Utf8PathBuf, - /// The path to the configuration TOML that should be placed on the server's - /// command line. - pub config_toml_path: PathBuf, + /// The directory in which to find or place files that are read or written + /// by this server process. + pub data_dir: &'a Utf8Path, /// The address at which the server should serve. pub server_addr: SocketAddrV4, @@ -27,13 +30,7 @@ pub struct ServerProcessParameters<'a, T: Into> { /// The address at which the server should offer its VNC server. pub vnc_addr: SocketAddrV4, - /// The [`Stdio`] descriptor to which the server's stdout should be - /// directed. - pub server_stdout: T, - - /// The [`Stdio`] descriptor to which the server's stderr should be - /// directed. - pub server_stderr: T, + pub log_mode: ServerLogMode, } pub struct PropolisServer { @@ -42,16 +39,17 @@ pub struct PropolisServer { } impl PropolisServer { - pub(crate) fn new + Debug>( - process_params: ServerProcessParameters, + pub(crate) fn new( + vm_name: &str, + process_params: ServerProcessParameters, + config_toml_path: &Utf8Path, ) -> Result { let ServerProcessParameters { server_path, - config_toml_path, + data_dir, server_addr, vnc_addr, - server_stdout, - server_stderr, + log_mode, } = process_params; info!( @@ -61,12 +59,15 @@ impl PropolisServer { "Launching Propolis server" ); + let (server_stdout, server_stderr) = + log_mode.get_handles(&data_dir, vm_name)?; + let server = PropolisServer { server: std::process::Command::new("pfexec") .args([ - server_path, + server_path.as_str(), "run", - config_toml_path.as_os_str().to_string_lossy().as_ref(), + config_toml_path.as_str(), server_addr.to_string().as_str(), vnc_addr.to_string().as_str(), ]) diff --git a/phd-tests/framework/src/test_vm/spec.rs b/phd-tests/framework/src/test_vm/spec.rs new file mode 100644 index 000000000..c985d5fa0 --- /dev/null +++ b/phd-tests/framework/src/test_vm/spec.rs @@ -0,0 +1,59 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use std::sync::Arc; + +use crate::{ + disk::{self, DiskConfig}, + guest_os::GuestOsKind, +}; +use propolis_client::types::{InstanceSpecV0, StorageBackendV0}; + +/// The set of objects needed to start and run a guest in a `TestVm`. +#[derive(Clone)] +pub(crate) struct VmSpec { + pub vm_name: String, + + /// The instance spec to pass to the VM when starting the guest. + pub instance_spec: InstanceSpecV0, + + /// A set of handles to disk files that the VM's disk backends refer to. + pub disk_handles: Vec>, + + /// The guest OS adapter to use for the VM. + pub guest_os_kind: GuestOsKind, + + /// The contents of the config TOML to write to run this VM. + pub config_toml_contents: String, +} + +impl VmSpec { + /// Update the Crucible backend specs in the instance spec to match the + /// current backend specs given by this specification's disk handles. + pub(crate) fn refresh_crucible_backends(&mut self) { + for disk in &self.disk_handles { + let disk = if let Some(disk) = disk.as_crucible() { + disk + } else { + continue; + }; + + let (backend_name, backend_spec) = disk.backend_spec(); + match self + .instance_spec + .backends + .storage_backends + .get(&backend_name) + { + Some(StorageBackendV0::Crucible(_)) => { + self.instance_spec + .backends + .storage_backends + .insert(backend_name, backend_spec.into()); + } + Some(_) | None => {} + } + } + } +} diff --git a/phd-tests/framework/src/test_vm/vm_config.rs b/phd-tests/framework/src/test_vm/vm_config.rs deleted file mode 100644 index f3139b6f2..000000000 --- a/phd-tests/framework/src/test_vm/vm_config.rs +++ /dev/null @@ -1,215 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! Structures that express how a VM should be configured. - -use std::{ - io::Write, - path::{Path, PathBuf}, - sync::Arc, -}; - -use propolis_client::instance_spec::{ - components as spec_components, - v0::{InstanceSpecV0, StorageDeviceV0}, -}; -use propolis_server_config as config; -use propolis_types::PciPath; -use thiserror::Error; - -use crate::{disk, guest_os::GuestOsKind}; - -/// Errors raised when configuring a VM or realizing a requested configuration. -#[derive(Debug, Error)] -pub enum VmConfigError { - #[error("No boot disk specified")] - NoBootDisk, - - #[error("Boot disk does not have an associated guest OS")] - BootDiskNotGuestImage, - - #[error("Could not find artifact {0} when populating disks")] - ArtifactNotFound(String), -} - -/// The disk interface to use for a given guest disk. -#[derive(Clone, Copy, Debug)] -pub enum DiskInterface { - Virtio, - Nvme, -} - -/// Parameters used to initialize a guest disk. -#[derive(Debug)] -struct DiskRequest { - /// A reference to the resources needed to create this disk's backend. VMs - /// created from this configuration also get a reference to these resources. - disk: Arc, - - /// The PCI device number to assign to this disk. The disk's BDF will be - /// 0/this value/0. - pci_device_num: u8, - - /// The PCI device interface to present to the guest. - interface: DiskInterface, -} - -/// An abstract description of a test VM's configuration and any objects needed -/// to launch the VM with that configuration. -#[derive(Default, Debug)] -pub struct ConfigRequest { - cpus: u8, - memory_mib: u64, - bootrom_path: PathBuf, - boot_disk: Option, - data_disks: Vec, -} - -impl ConfigRequest { - pub(crate) fn new() -> Self { - Self::default() - } - - pub fn set_cpus(mut self, cpus: u8) -> Self { - self.cpus = cpus; - self - } - - pub fn set_memory_mib(mut self, mem: u64) -> Self { - self.memory_mib = mem; - self - } - - pub fn set_bootrom_path(mut self, path: PathBuf) -> Self { - self.bootrom_path = path; - self - } - - pub fn set_boot_disk( - mut self, - disk: Arc, - pci_device_num: u8, - interface: DiskInterface, - ) -> Self { - self.boot_disk = Some(DiskRequest { disk, pci_device_num, interface }); - self - } - - fn all_disks(&self) -> impl Iterator { - self.boot_disk.iter().chain(self.data_disks.iter()) - } - - pub fn finish( - self, - object_dir: &impl AsRef, - toml_filename: &impl AsRef, - ) -> anyhow::Result { - let guest_os_kind = if let Some(disk_request) = &self.boot_disk { - disk_request - .disk - .guest_os() - .ok_or(VmConfigError::BootDiskNotGuestImage)? - } else { - return Err(VmConfigError::NoBootDisk.into()); - }; - - let mut spec_builder = - propolis_client::instance_spec::v0::builder::SpecBuilder::new( - self.cpus, - self.memory_mib, - false, - ); - - let mut disk_handles = Vec::new(); - for (disk_idx, disk_req) in self.all_disks().enumerate() { - let device_name = format!("disk-device{}", disk_idx); - let backend_name = format!("storage-backend{}", disk_idx); - let pci_path = PciPath::new(0, disk_req.pci_device_num, 0).unwrap(); - let device_spec = match disk_req.interface { - DiskInterface::Virtio => StorageDeviceV0::VirtioDisk( - spec_components::devices::VirtioDisk { - backend_name: backend_name.clone(), - pci_path, - }, - ), - DiskInterface::Nvme => StorageDeviceV0::NvmeDisk( - spec_components::devices::NvmeDisk { - backend_name: backend_name.clone(), - pci_path, - }, - ), - }; - - let backend_spec = disk_req.disk.backend_spec(); - spec_builder.add_storage_device( - device_name, - device_spec, - backend_name, - backend_spec, - )?; - - disk_handles.push(disk_req.disk.clone()); - } - - spec_builder.add_serial_port( - spec_components::devices::SerialPortNumber::Com1, - )?; - - let instance_spec = spec_builder.finish(); - - // Propolis selects its bootrom via the config TOML, so write a shell - // TOML that specifies this. All other VM configuration comes from the - // instance spec. - let mut server_toml_path = object_dir.as_ref().to_owned(); - server_toml_path.push(toml_filename); - self.write_config_toml(&server_toml_path)?; - - Ok(VmConfig { - instance_spec, - _disk_handles: disk_handles, - guest_os_kind, - server_toml_path, - }) - } - - fn write_config_toml(&self, toml_path: &Path) -> anyhow::Result<()> { - let config = config::Config { - bootrom: self.bootrom_path.clone(), - ..Default::default() - }; - - let serialized = toml::ser::to_string(&config).unwrap(); - let mut cfg_file = std::fs::OpenOptions::new() - .write(true) - .truncate(true) - .create(true) - .open(toml_path)?; - - cfg_file.write_all(serialized.as_bytes())?; - - Ok(()) - } -} - -#[derive(Clone, Debug)] -pub struct VmConfig { - instance_spec: InstanceSpecV0, - _disk_handles: Vec>, - guest_os_kind: GuestOsKind, - server_toml_path: PathBuf, -} - -impl VmConfig { - pub fn instance_spec(&self) -> &InstanceSpecV0 { - &self.instance_spec - } - - pub fn guest_os_kind(&self) -> GuestOsKind { - self.guest_os_kind - } - - pub fn server_toml_path(&self) -> &PathBuf { - &self.server_toml_path - } -} diff --git a/phd-tests/runner/src/execute.rs b/phd-tests/runner/src/execute.rs index 69f575f89..7156ba130 100644 --- a/phd-tests/runner/src/execute.rs +++ b/phd-tests/runner/src/execute.rs @@ -5,7 +5,7 @@ use std::cell::RefCell; use std::time::{Duration, Instant}; -use phd_tests::phd_testcase::{TestCase, TestContext, TestOutcome}; +use phd_tests::phd_testcase::{Framework, TestCase, TestOutcome}; use tracing::{error, info}; use crate::config::RunOptions; @@ -57,7 +57,7 @@ thread_local! { /// Executes a set of tests using the supplied test context. pub fn run_tests_with_ctx( - ctx: &TestContext, + ctx: &Framework, mut fixtures: TestFixtures, run_opts: &RunOptions, ) -> ExecutionStats { diff --git a/phd-tests/runner/src/fixtures.rs b/phd-tests/runner/src/fixtures.rs index 9b02b1854..d96ff89a6 100644 --- a/phd-tests/runner/src/fixtures.rs +++ b/phd-tests/runner/src/fixtures.rs @@ -5,17 +5,17 @@ use anyhow::Result; use tracing::instrument; -use crate::TestContext; +use crate::Framework; /// A wrapper containing the objects needed to run the executor's test fixtures. pub struct TestFixtures<'a> { - test_context: &'a TestContext<'a>, + test_context: &'a Framework, } impl<'a> TestFixtures<'a> { /// Creates a new set of test fixtures using the supplied command-line /// parameters and artifact store. - pub fn new(test_context: &'a TestContext) -> Result { + pub fn new(test_context: &'a Framework) -> Result { Ok(Self { test_context }) } @@ -48,7 +48,7 @@ impl<'a> TestFixtures<'a> { /// corresponding setup fixture has run. #[instrument(skip_all)] pub fn test_cleanup(&mut self) -> Result<()> { - self.test_context.vm_factory.reset(); + self.test_context.reset(); Ok(()) } } diff --git a/phd-tests/runner/src/main.rs b/phd-tests/runner/src/main.rs index 46f491e39..b49ebd21d 100644 --- a/phd-tests/runner/src/main.rs +++ b/phd-tests/runner/src/main.rs @@ -8,9 +8,7 @@ mod fixtures; use clap::Parser; use config::{ListOptions, ProcessArgs, RunOptions}; -use phd_framework::artifacts::ArtifactStore; -use phd_framework::port_allocator::PortAllocator; -use phd_tests::phd_testcase::TestContext; +use phd_tests::phd_testcase::{Framework, FrameworkParameters}; use tracing::{debug, info, warn}; use tracing_bunyan_formatter::{BunyanFormattingLayer, JsonStorageLayer}; use tracing_subscriber::layer::SubscriberExt; @@ -44,45 +42,22 @@ fn main() { } fn run_tests(run_opts: &RunOptions) -> ExecutionStats { - let artifact_store = ArtifactStore::from_toml_path( - run_opts.artifact_directory.clone(), - &run_opts.artifact_toml_path, - ) - .unwrap(); - - let port_allocator = PortAllocator::new(9000..10000); - - // Convert the command-line config and artifact store into a VM factory - // definition. - let mut config_toml_path = run_opts.tmp_directory.clone(); - config_toml_path.push("vm_config.toml"); - let factory_config = phd_framework::test_vm::factory::FactoryOptions { + let ctx_params = FrameworkParameters { propolis_server_path: run_opts.propolis_server_cmd.clone(), + crucible_downstairs_cmd: run_opts.crucible_downstairs_cmd.clone(), tmp_directory: run_opts.tmp_directory.clone(), + artifact_toml: run_opts.artifact_toml_path.clone(), server_log_mode: run_opts.server_logging_mode, - default_bootrom_artifact: run_opts.default_bootrom_artifact.clone(), default_guest_cpus: run_opts.default_guest_cpus, default_guest_memory_mib: run_opts.default_guest_memory_mib, + default_guest_os_artifact: run_opts.default_guest_artifact.clone(), + default_bootrom_artifact: run_opts.default_bootrom_artifact.clone(), + port_range: 9000..10000, }; - // The VM factory config and artifact store are enough to create a test - // context to pass to test cases and a set of fixtures. - let ctx = TestContext { - default_guest_image_artifact: run_opts.default_guest_artifact.clone(), - vm_factory: phd_framework::test_vm::factory::VmFactory::new( - factory_config, - &artifact_store, - &port_allocator, - ) - .unwrap(), - disk_factory: phd_framework::disk::DiskFactory::new( - &run_opts.tmp_directory, - &artifact_store, - run_opts.crucible_downstairs_cmd.clone().as_ref(), - &port_allocator, - run_opts.server_logging_mode, - ), - }; + let ctx = Framework::new(ctx_params) + .expect("should be able to set up a test context"); + let fixtures = TestFixtures::new(&ctx).unwrap(); // Run the tests and print results. diff --git a/phd-tests/testcase/src/lib.rs b/phd-tests/testcase/src/lib.rs index 2fe0a340f..790b4ad88 100644 --- a/phd-tests/testcase/src/lib.rs +++ b/phd-tests/testcase/src/lib.rs @@ -8,10 +8,8 @@ pub use phd_framework; pub use phd_testcase_macros::*; use thiserror::Error; -use phd_framework::{ - disk::{DiskFactory, DiskSource}, - test_vm::{factory::VmFactory, vm_config::ConfigRequest}, -}; +pub use phd_framework::Framework; +pub use phd_framework::FrameworkParameters; #[derive(Debug, Error)] pub enum TestSkippedError { @@ -34,42 +32,10 @@ pub enum TestOutcome { Skipped(Option), } -/// The test context structure passed to every PHD test case. -pub struct TestContext<'a> { - pub default_guest_image_artifact: String, - pub vm_factory: VmFactory<'a>, - pub disk_factory: DiskFactory<'a>, -} - -impl TestContext<'_> { - /// Yields a VM configuration builder that uses the default CPU count and - /// memory size and has a file-backed boot disk attached that supplies the - /// default guest OS image. - pub fn default_vm_config(&self) -> ConfigRequest { - let boot_disk = self - .disk_factory - .create_file_backed_disk(DiskSource::Artifact( - &self.default_guest_image_artifact, - )) - .unwrap(); - self.vm_factory.deviceless_vm_config().set_boot_disk( - boot_disk, - 4, - phd_framework::test_vm::vm_config::DiskInterface::Nvme, - ) - } - - /// Yields a VM configuration builder with the default CPU count and memory - /// size, but with no devices attached. - pub fn deviceless_vm_config(&self) -> ConfigRequest { - self.vm_factory.deviceless_vm_config() - } -} - /// A wrapper for test functions. This is needed to allow [`TestCase`] to have a /// `const` constructor for the inventory crate. pub struct TestFunction { - pub f: fn(&TestContext) -> TestOutcome, + pub f: fn(&Framework) -> TestOutcome, } /// A description of a single test case. @@ -109,7 +75,7 @@ impl TestCase { /// Runs the test case's body with the supplied test context and returns its /// outcome. - pub fn run(&self, ctx: &TestContext) -> TestOutcome { + pub fn run(&self, ctx: &Framework) -> TestOutcome { (self.function.f)(ctx) } } diff --git a/phd-tests/tests/src/crucible/migrate.rs b/phd-tests/tests/src/crucible/migrate.rs index 0932916d4..aab52fb53 100644 --- a/phd-tests/tests/src/crucible/migrate.rs +++ b/phd-tests/tests/src/crucible/migrate.rs @@ -4,23 +4,19 @@ use std::time::Duration; -use phd_framework::test_vm::vm_config::DiskInterface; use phd_testcase::*; use tracing::info; use uuid::Uuid; #[phd_testcase] -fn smoke_test(ctx: &TestContext) { - let disk = super::create_default_boot_disk(ctx, 10)?; +fn smoke_test(ctx: &Framework) { + let mut config = ctx.vm_config_builder("crucible_migrate_smoke_source"); + super::add_default_boot_disk(ctx, &mut config)?; + let mut source = ctx.spawn_vm(&config, None)?; + let disk_handles = source.cloned_disk_handles(); + let disk = disk_handles[0].as_crucible().unwrap(); disk.set_generation(1); - let config = ctx.deviceless_vm_config().set_boot_disk( - disk.clone(), - 4, - DiskInterface::Nvme, - ); - let mut source = - ctx.vm_factory.new_vm("crucible_migrate_smoke_source", config)?; source.launch()?; source.wait_to_boot()?; @@ -30,37 +26,30 @@ fn smoke_test(ctx: &TestContext) { source.run_shell_command("sync ./foo.bar")?; disk.set_generation(2); - let config = - ctx.deviceless_vm_config().set_boot_disk(disk, 4, DiskInterface::Nvme); - let mut target = - ctx.vm_factory.new_vm("crucible_migrate_smoke_target", config)?; + ctx.spawn_successor_vm("crucible_migrate_smoke_target", &source, None)?; + target.migrate_from(&source, Uuid::new_v4(), Duration::from_secs(60))?; let lsout = target.run_shell_command("ls foo.bar")?; assert_eq!(lsout, "foo.bar"); } #[phd_testcase] -fn load_test(ctx: &TestContext) { - let disk = super::create_default_boot_disk(ctx, 12)?; +fn load_test(ctx: &Framework) { + let mut config = ctx.vm_config_builder("crucible_load_test_source"); + super::add_default_boot_disk(ctx, &mut config)?; + let mut source = ctx.spawn_vm(&config, None)?; + let disk_handles = source.cloned_disk_handles(); + let disk = disk_handles[0].as_crucible().unwrap(); disk.set_generation(1); - let config = ctx.deviceless_vm_config().set_boot_disk( - disk.clone(), - 4, - DiskInterface::Nvme, - ); - let mut source = - ctx.vm_factory.new_vm("crucible_migrate_load_source", config)?; - - disk.set_generation(2); - let config = - ctx.deviceless_vm_config().set_boot_disk(disk, 4, DiskInterface::Nvme); - let mut target = - ctx.vm_factory.new_vm("crucible_migrate_load_target", config)?; source.launch()?; source.wait_to_boot()?; + disk.set_generation(2); + let mut target = + ctx.spawn_successor_vm("crucible_load_test_target", &source, None)?; + // Create some random data. let block_count = 10; let ddout = source.run_shell_command( diff --git a/phd-tests/tests/src/crucible/mod.rs b/phd-tests/tests/src/crucible/mod.rs index e69f1eb61..5bfcbfb53 100644 --- a/phd-tests/tests/src/crucible/mod.rs +++ b/phd-tests/tests/src/crucible/mod.rs @@ -2,52 +2,51 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use std::sync::Arc; - use phd_testcase::{ - phd_framework::disk::{ - crucible::CrucibleDisk, BlockSize, DiskError, DiskSource, + phd_framework::{ + disk::BlockSize, + test_vm::{DiskBackend, DiskInterface, VmConfig}, }, - phd_skip, TestContext, + *, }; mod migrate; mod smoke; -/// Attempts to create a Crucible disk with the specified parameters. If the -/// runner did not specify a Crucible downstairs path, produces the special -/// "test skipped" error status for the caller to return. -fn create_disk_or_skip( - ctx: &TestContext, - source: DiskSource, +fn add_crucible_boot_disk_or_skip( + ctx: &Framework, + config: &mut VmConfig, + artifact: &str, + interface: DiskInterface, + pci_slot: u8, disk_size_gib: u64, block_size: BlockSize, -) -> phd_testcase::Result> { - let disk = ctx.disk_factory.create_crucible_disk( - source, - disk_size_gib, - block_size, - ); - - if let Err(DiskError::NoCrucibleDownstairsPath) = disk { - phd_skip!("Crucible binary not specified, can't create crucible disk"); +) -> phd_testcase::Result<()> { + if !ctx.crucible_enabled() { + phd_skip!("Crucible backends not enabled (no downstairs path)"); } - Ok(disk?) + config.boot_disk( + artifact, + interface, + DiskBackend::Crucible { disk_size_gib, block_size }, + pci_slot, + ); + + Ok(()) } -/// Creates a boot disk of the supplied size using the default guest image -/// artifact and a 512-byte block size. Returns the special "test skipped" error -/// status if Crucible is not enabled for this test run (see -/// [`create_disk_or_skip`]). -fn create_default_boot_disk( - ctx: &TestContext, - disk_size_gib: u64, -) -> phd_testcase::Result> { - create_disk_or_skip( +fn add_default_boot_disk( + ctx: &Framework, + config: &mut VmConfig, +) -> phd_testcase::Result<()> { + add_crucible_boot_disk_or_skip( ctx, - DiskSource::Artifact(&ctx.default_guest_image_artifact), - disk_size_gib, + config, + ctx.default_guest_os_artifact(), + DiskInterface::Nvme, + 4, + 10, BlockSize::Bytes512, ) } diff --git a/phd-tests/tests/src/crucible/smoke.rs b/phd-tests/tests/src/crucible/smoke.rs index d91fdfd92..41ad89433 100644 --- a/phd-tests/tests/src/crucible/smoke.rs +++ b/phd-tests/tests/src/crucible/smoke.rs @@ -4,32 +4,24 @@ use std::time::Duration; -use phd_testcase::{phd_framework::test_vm::vm_config::DiskInterface, *}; +use phd_testcase::*; use propolis_client::types::InstanceState; #[phd_testcase] -fn boot_test(ctx: &TestContext) { - let disk = super::create_default_boot_disk(ctx, 10)?; - let config = - ctx.deviceless_vm_config().set_boot_disk(disk, 4, DiskInterface::Nvme); - - let mut vm = ctx.vm_factory.new_vm("crucible_boot_test", config)?; +fn boot_test(ctx: &Framework) { + let mut config = ctx.vm_config_builder("crucible_boot_test"); + super::add_default_boot_disk(ctx, &mut config)?; + let mut vm = ctx.spawn_vm(&config, None)?; vm.launch()?; vm.wait_to_boot()?; } #[phd_testcase] -fn shutdown_persistence_test(ctx: &TestContext) { - let disk = super::create_default_boot_disk(ctx, 10)?; - disk.set_generation(1); - let config = ctx.deviceless_vm_config().set_boot_disk( - disk.clone(), - 4, - DiskInterface::Nvme, - ); - - let mut vm = - ctx.vm_factory.new_vm("crucible_shutdown_persistence_test", config)?; +fn shutdown_persistence_test(ctx: &Framework) { + let mut config = + ctx.vm_config_builder("crucible_shutdown_persistence_test"); + super::add_default_boot_disk(ctx, &mut config)?; + let mut vm = ctx.spawn_vm(&config, None)?; if vm.guest_os_has_read_only_fs() { phd_skip!( "Can't run data persistence test on a guest with a read-only file @@ -37,6 +29,9 @@ fn shutdown_persistence_test(ctx: &TestContext) { ); } + let disk_handles = vm.cloned_disk_handles(); + let disk = disk_handles[0].as_crucible().unwrap(); + disk.set_generation(1); vm.launch()?; vm.wait_to_boot()?; @@ -51,16 +46,16 @@ fn shutdown_persistence_test(ctx: &TestContext) { // Increment the disk's generation before attaching it to a new VM. disk.set_generation(2); - let config = - ctx.deviceless_vm_config().set_boot_disk(disk, 4, DiskInterface::Nvme); - - // The touched file from the previous VM should be present in the new one. - let mut vm = ctx - .vm_factory - .new_vm("crucible_shutdown_persistence_test_2", config)?; + let mut vm = ctx.spawn_successor_vm( + "crucible_shutdown_persistence_test_2", + &vm, + None, + )?; vm.launch()?; vm.wait_to_boot()?; + + // The touched file from the previous VM should be present in the new one. let lsout = vm.run_shell_command("ls foo.bar 2> /dev/null")?; assert_eq!(lsout, "foo.bar"); } diff --git a/phd-tests/tests/src/hw.rs b/phd-tests/tests/src/hw.rs new file mode 100644 index 000000000..385258f9c --- /dev/null +++ b/phd-tests/tests/src/hw.rs @@ -0,0 +1,27 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use phd_framework::lifecycle::Action; +use phd_testcase::*; + +#[phd_testcase] +fn lspci_lifecycle_test(ctx: &Framework) { + const LSPCI: &str = "sudo lspci -vvx"; + const LSHW: &str = "sudo lshw -notime"; + + let mut vm = + ctx.spawn_vm(&ctx.vm_config_builder("lspci_lifecycle_test"), None)?; + + vm.launch()?; + vm.wait_to_boot()?; + + let lspci = vm.run_shell_command(LSPCI)?; + let lshw = vm.run_shell_command(LSHW)?; + ctx.lifecycle_test(vm, &[Action::StopAndStart], |vm| { + let new_lspci = vm.run_shell_command(LSPCI).unwrap(); + assert_eq!(new_lspci, lspci); + let new_lshw = vm.run_shell_command(LSHW).unwrap(); + assert_eq!(new_lshw, lshw); + })?; +} diff --git a/phd-tests/tests/src/lib.rs b/phd-tests/tests/src/lib.rs index eff412d9c..ba6d7d2c6 100644 --- a/phd-tests/tests/src/lib.rs +++ b/phd-tests/tests/src/lib.rs @@ -5,6 +5,7 @@ pub use phd_testcase; mod crucible; +mod hw; mod migrate; mod server_state_machine; mod smoke; diff --git a/phd-tests/tests/src/migrate.rs b/phd-tests/tests/src/migrate.rs index f80fcd015..dcfc5e1f0 100644 --- a/phd-tests/tests/src/migrate.rs +++ b/phd-tests/tests/src/migrate.rs @@ -9,10 +9,8 @@ use propolis_client::types::MigrationState; use uuid::Uuid; #[phd_testcase] -fn smoke_test(ctx: &TestContext) { - let mut source = ctx - .vm_factory - .new_vm("migration_smoke_source", ctx.default_vm_config())?; +fn smoke_test(ctx: &Framework) { + let mut source = ctx.spawn_default_vm("migration_smoke_source")?; source.launch()?; source.wait_to_boot()?; @@ -21,9 +19,8 @@ fn smoke_test(ctx: &TestContext) { source.run_shell_command("touch ./foo.bar")?; source.run_shell_command("sync ./foo.bar")?; - let mut target = ctx - .vm_factory - .new_vm_from_cloned_config("migration_smoke_target", &source)?; + let mut target = + ctx.spawn_successor_vm("migration_smoke_target", &source, None)?; let serial_hist_pre = source.get_serial_console_history(0)?; assert!(!serial_hist_pre.data.is_empty()); @@ -52,24 +49,28 @@ fn smoke_test(ctx: &TestContext) { } #[phd_testcase] -fn incompatible_vms(ctx: &TestContext) { - let configs = vec![ - ctx.default_vm_config().set_cpus(8), - ctx.default_vm_config().set_memory_mib(1024), +fn incompatible_vms(ctx: &Framework) { + let mut builders = vec![ + ctx.vm_config_builder("migration_incompatible_target_1"), + ctx.vm_config_builder("migration_incompatible_target_2"), ]; - for (i, cfg) in configs.into_iter().enumerate() { - let mut source = ctx.vm_factory.new_vm( - format!("migration_incompatible_source_{}", i).as_str(), - ctx.default_vm_config().set_cpus(4).set_memory_mib(512), + builders[0].cpus(8); + builders[1].memory_mib(1024); + + for (i, cfg) in builders.into_iter().enumerate() { + let mut source = ctx.spawn_vm( + ctx.vm_config_builder(&format!( + "migration_incompatible_source_{}", + i + )) + .cpus(4) + .memory_mib(512), + None, )?; source.launch()?; - - let mut target = ctx.vm_factory.new_vm( - format!("migration_incompatible_target_{}", i).as_str(), - cfg, - )?; + let mut target = ctx.spawn_vm(&cfg, None)?; let migration_id = Uuid::new_v4(); assert!(target @@ -87,16 +88,12 @@ fn incompatible_vms(ctx: &TestContext) { } #[phd_testcase] -fn multiple_migrations(ctx: &TestContext) { - let mut vm0 = ctx - .vm_factory - .new_vm("multiple_migrations_0", ctx.default_vm_config())?; - let mut vm1 = ctx - .vm_factory - .new_vm_from_cloned_config("multiple_migrations_1", &vm0)?; - let mut vm2 = ctx - .vm_factory - .new_vm_from_cloned_config("multiple_migrations_2", &vm1)?; +fn multiple_migrations(ctx: &Framework) { + let mut vm0 = ctx.spawn_default_vm("multiple_migrations_0")?; + let mut vm1 = + ctx.spawn_successor_vm("multiple_migrations_1", &vm0, None)?; + let mut vm2 = + ctx.spawn_successor_vm("multiple_migrations_2", &vm1, None)?; vm0.launch()?; vm0.wait_to_boot()?; diff --git a/phd-tests/tests/src/server_state_machine.rs b/phd-tests/tests/src/server_state_machine.rs index edb3fa66e..f94c5f2fc 100644 --- a/phd-tests/tests/src/server_state_machine.rs +++ b/phd-tests/tests/src/server_state_machine.rs @@ -10,10 +10,8 @@ use phd_testcase::*; use propolis_client::types::InstanceState; #[phd_testcase] -fn instance_start_stop_test(ctx: &TestContext) { - let mut vm = ctx - .vm_factory - .new_vm("instance_ensure_running_test", ctx.default_vm_config())?; +fn instance_start_stop_test(ctx: &Framework) { + let mut vm = ctx.spawn_default_vm("instance_ensure_running_test")?; vm.instance_ensure()?; let instance = vm.get()?.instance; @@ -27,10 +25,8 @@ fn instance_start_stop_test(ctx: &TestContext) { } #[phd_testcase] -fn instance_stop_causes_destroy_test(ctx: &TestContext) { - let mut vm = ctx - .vm_factory - .new_vm("instance_stop_causes_destroy_test", ctx.default_vm_config())?; +fn instance_stop_causes_destroy_test(ctx: &Framework) { + let mut vm = ctx.spawn_default_vm("instance_stop_causes_destroy_test")?; vm.launch()?; vm.stop()?; @@ -51,11 +47,9 @@ fn instance_stop_causes_destroy_test(ctx: &TestContext) { } #[phd_testcase] -fn instance_reset_test(ctx: &TestContext) { - let mut vm = ctx.vm_factory.new_vm( - "instance_reset_returns_to_running_test", - ctx.default_vm_config(), - )?; +fn instance_reset_test(ctx: &Framework) { + let mut vm = + ctx.spawn_default_vm("instance_reset_returns_to_running_test")?; assert!(vm.reset().is_err()); vm.launch()?; @@ -83,11 +77,9 @@ fn instance_reset_test(ctx: &TestContext) { } #[phd_testcase] -fn instance_reset_requires_running_test(ctx: &TestContext) { - let mut vm = ctx.vm_factory.new_vm( - "instance_reset_requires_running_test", - ctx.default_vm_config(), - )?; +fn instance_reset_requires_running_test(ctx: &Framework) { + let mut vm = + ctx.spawn_default_vm("instance_reset_requires_running_test")?; assert!(vm.reset().is_err()); vm.launch()?; diff --git a/phd-tests/tests/src/smoke.rs b/phd-tests/tests/src/smoke.rs index 143a8bff2..4ec566b96 100644 --- a/phd-tests/tests/src/smoke.rs +++ b/phd-tests/tests/src/smoke.rs @@ -2,16 +2,12 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use phd_testcase::{ - phd_framework::{disk::DiskSource, test_vm::vm_config::DiskInterface}, - *, -}; +use phd_testcase::*; #[phd_testcase] -fn nproc_test(ctx: &TestContext) { - let mut vm = ctx - .vm_factory - .new_vm("nproc_test", ctx.default_vm_config().set_cpus(6))?; +fn nproc_test(ctx: &Framework) { + let mut vm = + ctx.spawn_vm(ctx.vm_config_builder("nproc_test").cpus(6), None)?; vm.launch()?; vm.wait_to_boot()?; @@ -20,17 +16,11 @@ fn nproc_test(ctx: &TestContext) { } #[phd_testcase] -fn instance_spec_get_test(ctx: &TestContext) { - let disk = ctx.disk_factory.create_file_backed_disk( - DiskSource::Artifact(&ctx.default_guest_image_artifact), +fn instance_spec_get_test(ctx: &Framework) { + let mut vm = ctx.spawn_vm( + ctx.vm_config_builder("instance_spec_test").cpus(4).memory_mib(3072), + None, )?; - - let config = ctx - .deviceless_vm_config() - .set_cpus(4) - .set_memory_mib(3072) - .set_boot_disk(disk, 4, DiskInterface::Nvme); - let mut vm = ctx.vm_factory.new_vm("instance_spec_test", config)?; vm.launch()?; let spec_get_response = vm.get_spec()?;