PHD: allow a single test case to launch multiple VMs

phd-tests/framework/src/serial/vt100.rs

@@ -6,7 +6,7 @@ use tokio::{
     sync::{mpsc, Mutex},
     task::JoinHandle,
 };
-use tracing::{info, info_span, Instrument};
+use tracing::{debug, info, info_span, Instrument};
 
 #[derive(Error, Debug)]
 pub enum Vt100Error {
@@ -29,11 +29,15 @@ pub struct Vt100Processor {
 
 impl Vt100Processor {
     pub fn new(vt_rx: mpsc::Receiver<Vec<u8>>) -> Self {
-        let state = Arc::new(SharedState::default());
-        let state_for_task = state.clone();
-
         let vt_span = info_span!("Serial");
         vt_span.follows_from(tracing::Span::current());
+
+        let state = Arc::new(SharedState {
+            span: vt_span.clone(),
+            inner: Mutex::default(),
+        });
+
+        let state_for_task = state.clone();
         let task = tokio::spawn(
             async move {
                 vt100_handler(state_for_task, vt_rx).await;
@@ -98,8 +102,8 @@ struct Waiter {
 
 /// State shared between the receiver task and the public interface to the VT
 /// processor.
-#[derive(Default)]
 struct SharedState {
+    span: tracing::Span,
     inner: Mutex<Inner>,
 }
 
@@ -140,6 +144,7 @@ impl SharedState {
         wanted: String,
         output_tx: mpsc::Sender<String>,
     ) -> Result<()> {
+        let _span = self.span.enter();
         info!(wanted, "Registering wait for serial console output");
         let mut guard = self.inner.lock().await;
 
@@ -177,7 +182,7 @@ struct Inner {
 
 impl Drop for Inner {
     fn drop(&mut self) {
-        info!(
+        debug!(
             self.next_output_line,
             "Dropped serial console with partial line"
         );

phd-tests/framework/src/test_vm/factory.rs

@@ -2,8 +2,10 @@
 
 use std::{
     net::{Ipv4Addr, SocketAddrV4},
+    ops::Range,
     path::PathBuf,
     str::FromStr,
+    sync::atomic::{AtomicU16, Ordering},
 };
 
 use anyhow::Result;
@@ -93,6 +95,9 @@ pub struct FactoryOptions {
     /// The default amount of memory to set in [`vm_config::VmConfig`] structs
     /// generated by this factory.
     pub default_guest_memory_mib: u64,
+
+    /// The range of ports to assign to servers created by this factory.
+    pub server_port_range: Range<u16>,
 }
 
 /// A VM factory that provides routines to generate new test VMs.
@@ -101,6 +106,14 @@ pub struct VmFactory {
     default_guest_image_path: String,
     default_guest_kind: GuestOsKind,
     default_bootrom_path: String,
+
+    // Mutable state in the VM factory must be unwind-safe because the runner
+    // passes the factory to test cases (via their test contexts), and test
+    // cases run in a `catch_unwind` block to enable the use of `assert!` and
+    // `panic!`. For assigning sequential port numbers, `AtomicU16` fits the
+    // bill without requiring the extra interlocked operations needed to acquire
+    // and release an entire `Mutex`.
+    next_port: AtomicU16,
 }
 
 impl VmFactory {
@@ -120,14 +133,23 @@ impl VmFactory {
                 opts.default_bootrom_artifact.clone(),
             ))?;
 
+        let first_port = opts.server_port_range.start;
         Ok(Self {
             opts,
             default_guest_image_path: guest_path.to_string_lossy().to_string(),
             default_guest_kind: kind,
             default_bootrom_path: bootrom_path.to_string_lossy().to_string(),
+            next_port: AtomicU16::new(first_port),
         })
     }
 
+    /// 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.next_port
+            .store(self.opts.server_port_range.start, Ordering::Relaxed);
+    }
+
     /// Creates a VM configuration that specifies this factory's defaults for
     /// CPUs, memory, bootrom, and guest image.
     ///
@@ -183,10 +205,16 @@ impl VmFactory {
             }
         };
 
+        let server_port = self.next_port.fetch_add(1, Ordering::Relaxed);
+        let vnc_port = self.next_port.fetch_add(1, Ordering::Relaxed);
         let server_params = ServerProcessParameters {
             server_path: &self.opts.propolis_server_path,
             config_toml_path: &config_toml_path.as_os_str().to_string_lossy(),
-            server_addr: SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), 9000),
+            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,
         };

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

@@ -1,12 +1,7 @@
 //! Routines for starting VMs, changing their states, and interacting with their
 //! guest OSes.
 
-use std::{
-    fmt::Debug,
-    net::{Ipv4Addr, SocketAddrV4},
-    process::Stdio,
-    time::Duration,
-};
+use std::{fmt::Debug, net::SocketAddrV4, process::Stdio, time::Duration};
 
 use crate::guest_os::{self, CommandSequenceEntry, GuestOs, GuestOsKind};
 use crate::serial::SerialConsole;
@@ -71,6 +66,9 @@ pub struct ServerProcessParameters<'a, T: Into<Stdio> + Debug> {
     /// The address at which the server should serve.
     pub server_addr: SocketAddrV4,
 
+    /// 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,
@@ -116,6 +114,7 @@ impl TestVm {
             server_path,
             config_toml_path,
             server_addr,
+            vnc_addr,
             server_stdout,
             server_stderr,
         } = process_params;
@@ -134,6 +133,7 @@ impl TestVm {
                     "run",
                     config_toml_path,
                     server_addr.to_string().as_str(),
+                    vnc_addr.to_string().as_str(),
                 ])
                 .stdout(server_stdout)
                 .stderr(server_stderr)
@@ -148,10 +148,7 @@ impl TestVm {
         let client_async_drain =
             slog_async::Async::new(client_drain).build().fuse();
         let client = Client::new(
-            std::net::SocketAddr::V4(SocketAddrV4::new(
-                Ipv4Addr::new(127, 0, 0, 1),
-                9000,
-            )),
+            server_addr.into(),
             slog::Logger::root(client_async_drain, slog::o!()),
         );
 
@@ -250,9 +247,8 @@ impl TestVm {
     /// initial login prompt and the login prompt itself.
     pub fn wait_to_boot(&self) -> Result<()> {
         let timeout_duration = Duration::from_secs(300);
-        let wait_span =
-            info_span!("Waiting {} for guest to boot", ?timeout_duration);
-        wait_span.follows_from(&self.tracing_span);
+        let _span = self.tracing_span.enter();
+        info!("Waiting {:?} for guest to boot", timeout_duration);
 
         let boot_sequence = self.guest_os.get_login_sequence();
         let _ = self.rt.block_on(async {
@@ -275,7 +271,7 @@ impl TestVm {
                     }
                     Ok::<(), anyhow::Error>(())
                 }
-                .instrument(wait_span),
+                .instrument(info_span!("wait_to_boot")),
             )
             .await
             .map_err(|e| anyhow!(e))

phd-tests/runner/src/execute.rs

@@ -53,7 +53,7 @@ thread_local! {
 
 /// Executes a set of tests using the supplied test context.
 pub fn run_tests_with_ctx<'fix>(
-    ctx: TestContext,
+    ctx: &TestContext,
     mut fixtures: TestFixtures,
     run_opts: &RunOptions,
 ) -> ExecutionStats {

phd-tests/runner/src/fixtures.rs

@@ -2,12 +2,15 @@ use anyhow::Result;
 use phd_framework::artifacts::ArtifactStore;
 use tracing::instrument;
 
+use crate::TestContext;
+
 use super::config;
 use super::zfs::ZfsFixture;
 
 /// A wrapper containing the objects needed to run the executor's test fixtures.
 pub struct TestFixtures<'a> {
     artifact_store: &'a ArtifactStore,
+    test_context: &'a TestContext,
     zfs: Option<ZfsFixture>,
 }
 
@@ -17,6 +20,7 @@ impl<'a> TestFixtures<'a> {
     pub fn new(
         run_opts: &config::RunOptions,
         artifact_store: &'a ArtifactStore,
+        test_context: &'a TestContext,
     ) -> Result<Self> {
         let zfs = run_opts
             .zfs_fs_name
@@ -30,7 +34,7 @@ impl<'a> TestFixtures<'a> {
             })
             .transpose()?;
 
-        Ok(Self { artifact_store, zfs })
+        Ok(Self { artifact_store, test_context, zfs })
     }
 
     /// Calls fixture routines that need to run before any tests run.
@@ -73,6 +77,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();
         if let Some(zfs) = &mut self.zfs {
             zfs.rollback_to_artifact_snapshot()
         } else {

phd-tests/runner/src/main.rs

@@ -51,6 +51,7 @@ fn run_tests(run_opts: &RunOptions) {
         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,
+        server_port_range: 9000..10000,
     };
 
     // The VM factory config and artifact store are enough to create a test
@@ -62,10 +63,11 @@ fn run_tests(run_opts: &RunOptions) {
         )
         .unwrap(),
     };
-    let fixtures = TestFixtures::new(&run_opts, &artifact_store).unwrap();
+    let fixtures = TestFixtures::new(&run_opts, &artifact_store, &ctx).unwrap();
 
     // Run the tests and print results.
-    let execution_stats = execute::run_tests_with_ctx(ctx, fixtures, &run_opts);
+    let execution_stats =
+        execute::run_tests_with_ctx(&ctx, fixtures, &run_opts);
     if execution_stats.failed_test_cases.len() != 0 {
         println!("\nfailures:");
         for tc in execution_stats.failed_test_cases {

phd-tests/tests/src/smoke.rs

@@ -1,24 +1,4 @@
-use phd_testcase::{phd_framework::guest_os::GuestOsKind, *};
-
-#[phd_testcase]
-fn uname_test(ctx: &TestContext) {
-    let vm = ctx
-        .vm_factory
-        .new_vm("uname_test", ctx.vm_factory.default_vm_config())?;
-    vm.run()?;
-    vm.wait_to_boot()?;
-
-    vm.run_shell_command("echo $SHELL")?;
-
-    let uname = vm.run_shell_command("uname -r")?;
-    assert_eq!(
-        uname,
-        match vm.guest_os_kind() {
-            GuestOsKind::Alpine => "5.15.41-0-virt",
-            GuestOsKind::Debian11NoCloud => "5.10.0-16-amd64",
-        }
-    );
-}
+use phd_testcase::*;
 
 #[phd_testcase]
 fn nproc_test(ctx: &TestContext) {
@@ -31,3 +11,22 @@ fn nproc_test(ctx: &TestContext) {
     let nproc = vm.run_shell_command("nproc")?;
     assert_eq!(nproc.parse::<u8>().unwrap(), 6);
 }
+
+#[phd_testcase]
+fn multiple_vms_test(ctx: &TestContext) {
+    let vms = (0..5)
+        .into_iter()
+        .map(|i| {
+            let name = format!("multiple_vms_test_vm{}", i);
+            ctx.vm_factory.new_vm(&name, ctx.vm_factory.default_vm_config())
+        })
+        .collect::<Result<Vec<_>, _>>()?;
+
+    for vm in &vms {
+        vm.run()?;
+    }
+
+    for vm in &vms {
+        vm.wait_to_boot()?;
+    }
+}