PHD: add Windows Server 2016 adapter & improve WS2016/2019 reliability (#646)

Add a Windows Server 2016 guest adapter to PHD. It's more or less identical to
the Windows Server 2019 adapter (but is still worth keeping separate since the
guests support different command-line tools and Powershell cmdlets out of the
box).

Tighten a few bolts to improve (but not perfect) test performance and
reliability for these guests:

- Rework the way shell commands are issued:
  - `GuestOs`es now specify a serial console `CommandSequence` that the
    framework should execute in order to run a shell command.
  - Waiting for a string to appear in the serial console buffer no longer
    consumes any characters from the buffer. Instead, callers must clear the
    buffer manually.

With this change, it no longer matters that Windows guests don't re-echo
characters that are already on-screen, because the framework no longer
invalidates them until it knows they're no longer wanted. 

Additionally improve the keystroke debounce logic:

- Only apply the debounce delay if the next character to be issued is the same
  as the previous character. This allows the debounce timeout to be lengthened
  considerably without tanking test performance (which is important to minimize
  flakiness).

  It might be worth investigating whether we could avoid this delay by doing
  byte-by-byte "call and response" for characters we expect the guest to echo.
  This may require a more substantial refactoring of the serial console code, so
  I've just resorted to having a longer debounce for now.
- Change `TestVm::send_serial_bytes_async` to wait for the serial task to
  acknowledge that it has sent all the requested bytes to the guest before
  returning so that `TestVm` can time out waiting for characters to be echoed
  without including the time spent waiting for them to be sent.

Tested with WS2016, WS2019, Alpine, and Debian 11 guests.

Author: gjcolombo

phd-tests/framework/src/guest_os/alpine.rs

@@ -11,9 +11,9 @@ pub(super) struct Alpine;
 impl GuestOs for Alpine {
     fn get_login_sequence(&self) -> CommandSequence {
         CommandSequence(vec![
-            CommandSequenceEntry::WaitFor("localhost login: "),
-            CommandSequenceEntry::WriteStr("root"),
-            CommandSequenceEntry::WaitFor(self.get_shell_prompt()),
+            CommandSequenceEntry::wait_for("localhost login: "),
+            CommandSequenceEntry::write_str("root"),
+            CommandSequenceEntry::wait_for(self.get_shell_prompt()),
         ])
     }
 
@@ -24,4 +24,11 @@ impl GuestOs for Alpine {
     fn read_only_fs(&self) -> bool {
         true
     }
+
+    fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> {
+        super::shell_commands::shell_command_sequence(
+            std::borrow::Cow::Borrowed(cmd),
+            crate::serial::BufferKind::Raw,
+        )
+    }
 }

phd-tests/framework/src/guest_os/debian11_nocloud.rs

@@ -11,9 +11,9 @@ pub(super) struct Debian11NoCloud;
 impl GuestOs for Debian11NoCloud {
     fn get_login_sequence(&self) -> CommandSequence {
         CommandSequence(vec![
-            CommandSequenceEntry::WaitFor("debian login: "),
-            CommandSequenceEntry::WriteStr("root"),
-            CommandSequenceEntry::WaitFor(self.get_shell_prompt()),
+            CommandSequenceEntry::wait_for("debian login: "),
+            CommandSequenceEntry::write_str("root"),
+            CommandSequenceEntry::wait_for(self.get_shell_prompt()),
         ])
     }
 

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

@@ -11,30 +11,45 @@ use serde::{Deserialize, Serialize};
 
 mod alpine;
 mod debian11_nocloud;
+mod shell_commands;
 mod ubuntu22_04;
 mod windows;
+mod windows_server_2016;
 mod windows_server_2019;
 mod windows_server_2022;
 
 /// An entry in a sequence of interactions with the guest's command prompt.
 #[derive(Debug)]
-pub(super) enum CommandSequenceEntry {
+pub(super) enum CommandSequenceEntry<'a> {
     /// Wait for the supplied string to appear on the guest serial console.
-    WaitFor(&'static str),
+    WaitFor(Cow<'a, str>),
 
     /// Write the specified string as a command to the guest serial console.
-    WriteStr(&'static str),
+    WriteStr(Cow<'a, str>),
+
+    /// Tell the serial console task to clear its buffer.
+    ClearBuffer,
 
     /// Change the serial console buffering discipline to the supplied
     /// discipline.
     ChangeSerialConsoleBuffer(crate::serial::BufferKind),
 
-    /// Set a delay between writing individual bytes to the guest serial console
+    /// Set a delay between writing identical bytes to the guest serial console
     /// to avoid keyboard debouncing logic in guests.
-    SetSerialByteWriteDelay(std::time::Duration),
+    SetRepeatedCharacterDebounce(std::time::Duration),
 }
 
-pub(super) struct CommandSequence(pub Vec<CommandSequenceEntry>);
+impl<'a> CommandSequenceEntry<'a> {
+    fn write_str(s: impl Into<Cow<'a, str>>) -> Self {
+        Self::WriteStr(s.into())
+    }
+
+    fn wait_for(s: impl Into<Cow<'a, str>>) -> Self {
+        Self::WaitFor(s.into())
+    }
+}
+
+pub(super) struct CommandSequence<'a>(pub Vec<CommandSequenceEntry<'a>>);
 
 pub(super) trait GuestOs: Send + Sync {
     /// Retrieves the command sequence used to wait for the OS to boot and log
@@ -47,13 +62,13 @@ pub(super) trait GuestOs: Send + Sync {
     /// Indicates whether the guest has a read-only filesystem.
     fn read_only_fs(&self) -> bool;
 
-    /// Some guests need to amend incoming shell commands from tests in order to
-    /// get output to display on the serial console in a way those guests can
-    /// accept (e.g. by clearing the screen immediately before running each
-    /// command). This function amends an incoming command according to the
-    /// guest adapter's instructions.
-    fn amend_shell_command<'a>(&self, cmd: &'a str) -> Cow<'a, str> {
-        Cow::Borrowed(cmd)
+    /// Returns the sequence of serial console operations a test VM should issue
+    /// in order to execute `cmd` in the guest's shell.
+    fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> {
+        shell_commands::shell_command_sequence(
+            Cow::Borrowed(cmd),
+            crate::serial::BufferKind::Raw,
+        )
     }
 }
 
@@ -64,6 +79,7 @@ pub enum GuestOsKind {
     Alpine,
     Debian11NoCloud,
     Ubuntu2204,
+    WindowsServer2016,
     WindowsServer2019,
     WindowsServer2022,
 }
@@ -76,6 +92,8 @@ impl FromStr for GuestOsKind {
             "alpine" => Ok(Self::Alpine),
             "debian11nocloud" => Ok(Self::Debian11NoCloud),
             "ubuntu2204" => Ok(Self::Ubuntu2204),
+            "windowsserver2016" => Ok(Self::WindowsServer2016),
+            "windowsserver2019" => Ok(Self::WindowsServer2019),
             "windowsserver2022" => Ok(Self::WindowsServer2022),
             _ => Err(std::io::Error::new(
                 std::io::ErrorKind::InvalidInput,
@@ -92,6 +110,9 @@ pub(super) fn get_guest_os_adapter(kind: GuestOsKind) -> Box<dyn GuestOs> {
             Box::new(debian11_nocloud::Debian11NoCloud)
         }
         GuestOsKind::Ubuntu2204 => Box::new(ubuntu22_04::Ubuntu2204),
+        GuestOsKind::WindowsServer2016 => {
+            Box::new(windows_server_2016::WindowsServer2016)
+        }
         GuestOsKind::WindowsServer2019 => {
             Box::new(windows_server_2019::WindowsServer2019)
         }

phd-tests/framework/src/guest_os/shell_commands.rs

@@ -0,0 +1,61 @@
+// 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/.
+
+//! Common helper functions for issuing shell commands to guests and handling
+//! their outputs.
+
+use std::borrow::Cow;
+
+use super::{CommandSequence, CommandSequenceEntry};
+
+/// Produces the shell command sequence necessary to execute `cmd` in a guest's
+/// shell, given that the guest is using the supplied serial console buffering
+/// discipline.
+///
+/// This routine assumes that multi-line commands will be echoed with `> ` at
+/// the start of each line in the command. This is technically shell-dependent
+/// but is true for all the shell types in PHD's currently-supported guests.
+pub(super) fn shell_command_sequence(
+    cmd: Cow<'_, str>,
+    buffer_kind: crate::serial::BufferKind,
+) -> CommandSequence {
+    let echo = cmd.trim_end().replace('\n', "\n> ");
+    match buffer_kind {
+        crate::serial::BufferKind::Raw => CommandSequence(vec![
+            CommandSequenceEntry::write_str(cmd),
+            CommandSequenceEntry::wait_for(echo),
+            CommandSequenceEntry::ClearBuffer,
+            CommandSequenceEntry::write_str("\n"),
+        ]),
+
+        crate::serial::BufferKind::Vt80x24 => {
+            // In 80x24 mode, it's simplest to issue multi-line operations one
+            // line at a time and wait for each line to be echoed before
+            // starting the next. For very long commands (more than 24 lines),
+            // this avoids having to deal with lines scrolling off the buffer
+            // before they can be waited for.
+            let cmd_lines = cmd.trim_end().lines();
+            let echo_lines = echo.lines();
+            let mut seq = vec![];
+
+            let mut iter = cmd_lines.zip(echo_lines).peekable();
+            while let Some((cmd, echo)) = iter.next() {
+                seq.push(CommandSequenceEntry::write_str(cmd.to_owned()));
+                seq.push(CommandSequenceEntry::wait_for(echo.to_owned()));
+
+                if iter.peek().is_some() {
+                    seq.push(CommandSequenceEntry::write_str("\n"));
+                }
+            }
+
+            // Before issuing the command, clear any stale echoed characters
+            // from the serial console buffer. This ensures that the next prompt
+            // is preceded in the buffer only by the output of the issued
+            // command.
+            seq.push(CommandSequenceEntry::ClearBuffer);
+            seq.push(CommandSequenceEntry::write_str("\n"));
+            CommandSequence(seq)
+        }
+    }
+}

phd-tests/framework/src/guest_os/ubuntu22_04.rs

@@ -12,11 +12,11 @@ pub(super) struct Ubuntu2204;
 impl GuestOs for Ubuntu2204 {
     fn get_login_sequence(&self) -> CommandSequence {
         CommandSequence(vec![
-            CommandSequenceEntry::WaitFor("ubuntu login: "),
-            CommandSequenceEntry::WriteStr("ubuntu"),
-            CommandSequenceEntry::WaitFor("Password: "),
-            CommandSequenceEntry::WriteStr("1!Passw0rd"),
-            CommandSequenceEntry::WaitFor(self.get_shell_prompt()),
+            CommandSequenceEntry::wait_for("ubuntu login: "),
+            CommandSequenceEntry::write_str("ubuntu"),
+            CommandSequenceEntry::wait_for("Password: "),
+            CommandSequenceEntry::write_str("1!Passw0rd"),
+            CommandSequenceEntry::wait_for(self.get_shell_prompt()),
         ])
     }
 

phd-tests/framework/src/guest_os/windows.rs

@@ -14,52 +14,60 @@ use super::{CommandSequence, CommandSequenceEntry, GuestOsKind};
 /// - Cygwin is installed to C:\cygwin and can be launched by invoking
 ///   C:\cygwin\cygwin.bat.
 /// - The local administrator account is enabled with password `0xide#1Fan`.
-pub(super) fn get_login_sequence_for(guest: GuestOsKind) -> CommandSequence {
+pub(super) fn get_login_sequence_for<'a>(
+    guest: GuestOsKind,
+) -> CommandSequence<'a> {
     assert!(matches!(
         guest,
-        GuestOsKind::WindowsServer2019 | GuestOsKind::WindowsServer2022
+        GuestOsKind::WindowsServer2016
+            | GuestOsKind::WindowsServer2019
+            | GuestOsKind::WindowsServer2022
     ));
 
     let mut commands = vec![
-        CommandSequenceEntry::WaitFor(
+        CommandSequenceEntry::wait_for(
             "Computer is booting, SAC started and initialized.",
         ),
-        CommandSequenceEntry::WaitFor(
+        CommandSequenceEntry::wait_for(
             "EVENT: The CMD command is now available.",
         ),
-        CommandSequenceEntry::WaitFor("SAC>"),
-        CommandSequenceEntry::WriteStr("cmd"),
-        CommandSequenceEntry::WaitFor("Channel: Cmd0001"),
-        CommandSequenceEntry::WaitFor("SAC>"),
-        CommandSequenceEntry::WriteStr("ch -sn Cmd0001"),
-        CommandSequenceEntry::WaitFor(
+        CommandSequenceEntry::wait_for("SAC>"),
+        CommandSequenceEntry::write_str("cmd"),
+        CommandSequenceEntry::wait_for("Channel: Cmd0001"),
+        CommandSequenceEntry::wait_for("SAC>"),
+        CommandSequenceEntry::write_str("ch -sn Cmd0001"),
+        CommandSequenceEntry::wait_for(
             "Use any other key to view this channel.",
         ),
-        CommandSequenceEntry::WriteStr(""),
-        CommandSequenceEntry::WaitFor("Username:"),
-        CommandSequenceEntry::WriteStr("Administrator"),
-        CommandSequenceEntry::WaitFor("Domain  :"),
-        CommandSequenceEntry::WriteStr(""),
-        CommandSequenceEntry::WaitFor("Password:"),
-        CommandSequenceEntry::WriteStr("0xide#1Fan"),
+        CommandSequenceEntry::write_str(""),
+        CommandSequenceEntry::wait_for("Username:"),
+        CommandSequenceEntry::write_str("Administrator"),
+        CommandSequenceEntry::wait_for("Domain  :"),
+        CommandSequenceEntry::write_str(""),
+        CommandSequenceEntry::wait_for("Password:"),
+        CommandSequenceEntry::write_str("0xide#1Fan"),
     ];
 
-    // Windows Server 2019's serial console-based command prompts default to
-    // trying to drive a VT100 terminal themselves instead of emitting
-    // characters and letting the recipient display them in whatever style it
-    // likes. This only happens once the command prompt has been activated, so
-    // only switch buffering modes after entering credentials.
-    if let GuestOsKind::WindowsServer2019 = guest {
+    // Earlier Windows Server versions' serial console-based command prompts
+    // default to trying to drive a VT100 terminal themselves instead of
+    // emitting characters and letting the recipient display them in whatever
+    // style it likes. This only happens once the command prompt has been
+    // activated, so only switch buffering modes after entering credentials.
+    if matches!(
+        guest,
+        GuestOsKind::WindowsServer2016 | GuestOsKind::WindowsServer2019
+    ) {
         commands.extend([
             CommandSequenceEntry::ChangeSerialConsoleBuffer(
                 crate::serial::BufferKind::Vt80x24,
             ),
-            // Server 2019 also likes to debounce keystrokes, so set a small
-            // delay between characters to try to avoid this. (This value was
-            // chosen by experimentation; there doesn't seem to be a guest
-            // setting that controls this interval.)
-            CommandSequenceEntry::SetSerialByteWriteDelay(
-                std::time::Duration::from_millis(125),
+            // These versions also like to debounce keystrokes, so set a delay
+            // between repeated characters to try to avoid this. This is a very
+            // conservative delay to try to avoid test flakiness; fortunately,
+            // it only applies when typing the same character multiple times in
+            // a row.
+            CommandSequenceEntry::SetRepeatedCharacterDebounce(
+                std::time::Duration::from_secs(1),
             ),
         ]);
     }
@@ -70,14 +78,14 @@ pub(super) fn get_login_sequence_for(guest: GuestOsKind) -> CommandSequence {
         // eat the command and just process the newline). It also appears to
         // prefer carriage returns to linefeeds. Accommodate this behavior
         // until Cygwin is launched.
-        CommandSequenceEntry::WaitFor("C:\\Windows\\system32>"),
-        CommandSequenceEntry::WriteStr("cls\r"),
-        CommandSequenceEntry::WaitFor("C:\\Windows\\system32>"),
-        CommandSequenceEntry::WriteStr("C:\\cygwin\\cygwin.bat\r"),
-        CommandSequenceEntry::WaitFor("$ "),
+        CommandSequenceEntry::wait_for("C:\\Windows\\system32>"),
+        CommandSequenceEntry::write_str("cls\r"),
+        CommandSequenceEntry::wait_for("C:\\Windows\\system32>"),
+        CommandSequenceEntry::write_str("C:\\cygwin\\cygwin.bat\r"),
+        CommandSequenceEntry::wait_for("$ "),
         // Tweak the command prompt so that it appears on a single line with
         // no leading newlines.
-        CommandSequenceEntry::WriteStr("PS1='\\u@\\h:$ '"),
+        CommandSequenceEntry::write_str("PS1='\\u@\\h:$ '"),
     ]);
 
     CommandSequence(commands)

phd-tests/framework/src/guest_os/windows_server_2016.rs

@@ -0,0 +1,45 @@
+// 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/.
+
+// 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/.
+
+//! Guest OS adaptations for Windows Server 2016 images. See [the general
+//! Windows module](mod@super::windows) documentation for more information.
+
+use std::borrow::Cow;
+
+use super::{CommandSequence, GuestOs, GuestOsKind};
+
+/// The guest adapter for Windows Server 2016 images. See [the general
+/// Windows module](mod@super::windows) documentation for more information about
+/// the configuration this adapter requires.
+pub(super) struct WindowsServer2016;
+
+impl GuestOs for WindowsServer2016 {
+    fn get_login_sequence(&self) -> CommandSequence {
+        super::windows::get_login_sequence_for(GuestOsKind::WindowsServer2016)
+    }
+
+    fn get_shell_prompt(&self) -> &'static str {
+        "Administrator@PHD-WINDOWS:$ "
+    }
+
+    fn read_only_fs(&self) -> bool {
+        false
+    }
+
+    fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> {
+        // `reset` the command prompt before issuing the command to try to force
+        // Windows to redraw the subsequent command prompt. Without this,
+        // Windows may not draw the prompt if the post-command state happens to
+        // place a prompt at a location that already had one pre-command.
+        let cmd = format!("reset && {cmd}");
+        super::shell_commands::shell_command_sequence(
+            Cow::Owned(cmd),
+            crate::serial::BufferKind::Vt80x24,
+        )
+    }
+}