Skip to content

Commit

Permalink
PHD: support guest-initiated reboot (#785)
Browse files Browse the repository at this point in the history
While in many cases a procedure like "`reboot` and wait for login" is
sufficient to reboot a guest, it is not sufficient for *all* cases. For
Windows, the command is spelled `shutdown /r` instead. And on Debian (at
least 11), `reboot` will immediately take the guest down instead of
printing a new shell prompt first.

So, have guest OS adapters provide commands that impel a guest to
reboot, and wait for that reboot to occur (rather than the naive initial
approach of reboot, wait for shell prompt, wait for login prompt,
proceed).

Fixes #783.
  • Loading branch information
iximeow authored Oct 10, 2024
1 parent 5fe523a commit f5f7bf8
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 23 deletions.
8 changes: 8 additions & 0 deletions phd-tests/framework/src/guest_os/alpine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,12 @@ impl GuestOs for Alpine {
crate::serial::BufferKind::Raw,
)
}

fn graceful_reboot(&self) -> CommandSequence {
// For Alpine guests we've looked at, `reboot` kicks off OpenRC behavior
// to reboot the system. We *could* wait for a new shell prompt at this
// point, but it's more reliable to wait for a guest to have fully
// rebooted and log back in.
self.shell_command_sequence("reboot")
}
}
12 changes: 12 additions & 0 deletions phd-tests/framework/src/guest_os/debian11_nocloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,16 @@ impl GuestOs for Debian11NoCloud {
fn read_only_fs(&self) -> bool {
false
}

fn graceful_reboot(&self) -> CommandSequence {
// On Debian 11, `reboot` does not seem to be the same wrapper for
// `systemctl reboot` as it is on more recent Ubuntu. Whatever it *is*,
// it does its job before a new prompt line is printed, so we can only
// wait to see a new login sequence.
//
// While `systemctl reboot` does exist here, and is mechanically more
// like Ubuntu's `reboot`, just using `reboot` on Debian gets the job
// done and keeps our instructions consistent across Linuxes.
self.shell_command_sequence("reboot")
}
}
6 changes: 6 additions & 0 deletions phd-tests/framework/src/guest_os/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ pub(super) trait GuestOs: Send + Sync {
crate::serial::BufferKind::Raw,
)
}

/// Returns the sequence of serial console operations a test VM must perform
/// in order to perform a graceful (e.g. guest-initiated and expected)
/// reboot. PHD's expectation following these commands will be to wait for
/// the guest's login sequence.
fn graceful_reboot(&self) -> CommandSequence;
}

#[allow(dead_code)]
Expand Down
9 changes: 9 additions & 0 deletions phd-tests/framework/src/guest_os/ubuntu22_04.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,13 @@ impl GuestOs for Ubuntu2204 {
fn read_only_fs(&self) -> bool {
false
}

fn graceful_reboot(&self) -> CommandSequence {
// Ubuntu `reboot` seems to be mechanically similar to Alpine `reboot`,
// except mediated by SystemD rather than OpenRC. We'll get a new shell
// prompt, and then the system reboots shortly after. Just issuing
// `reboot` and waiting for a login prompt is the lowest common
// denominator across Linuxes.
self.shell_command_sequence("reboot")
}
}
4 changes: 4 additions & 0 deletions phd-tests/framework/src/guest_os/windows_server_2016.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,8 @@ impl GuestOs for WindowsServer2016 {
crate::serial::BufferKind::Vt80x24,
)
}

fn graceful_reboot(&self) -> CommandSequence {
self.shell_command_sequence("shutdown /r /t 0 /d p:0:0")
}
}
4 changes: 4 additions & 0 deletions phd-tests/framework/src/guest_os/windows_server_2019.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,8 @@ impl GuestOs for WindowsServer2019 {
crate::serial::BufferKind::Vt80x24,
)
}

fn graceful_reboot(&self) -> CommandSequence {
self.shell_command_sequence("shutdown /r /t 0 /d p:0:0")
}
}
4 changes: 4 additions & 0 deletions phd-tests/framework/src/guest_os/windows_server_2022.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,8 @@ impl GuestOs for WindowsServer2022 {
fn read_only_fs(&self) -> bool {
false
}

fn graceful_reboot(&self) -> CommandSequence {
self.shell_command_sequence("shutdown /r /t 0 /d p:0:0")
}
}
53 changes: 36 additions & 17 deletions phd-tests/framework/src/test_vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
use std::{fmt::Debug, io::Write, sync::Arc, time::Duration};

use crate::{
guest_os::{self, CommandSequenceEntry, GuestOs, GuestOsKind},
guest_os::{
self, CommandSequence, CommandSequenceEntry, GuestOs, GuestOsKind,
},
serial::{BufferKind, SerialConsole},
test_vm::{
environment::Environment, server::ServerProcessParameters, spec::VmSpec,
Expand Down Expand Up @@ -872,6 +874,38 @@ impl TestVm {
// type (which affects e.g. affects how it displays multi-line commands)
// and serial console buffering discipline.
let command_sequence = self.guest_os.shell_command_sequence(cmd);
self.run_command_sequence(command_sequence).await?;

// `shell_command_sequence` promises that the generated command sequence
// clears buffer of everything up to and including the input command
// before actually issuing the final '\n' that issues the command.
// This ensures that the buffer contents returned by this call contain
// only the command's output.
let out = self
.wait_for_serial_output(
self.guest_os.get_shell_prompt(),
Duration::from_secs(300),
)
.await?;

// Trim any leading newlines inserted when the command was issued and
// any trailing whitespace that isn't actually part of the command
// output. Any other embedded whitespace is the caller's problem.
Ok(out.trim().to_string())
}

pub async fn graceful_reboot(&self) -> Result<()> {
self.run_command_sequence(self.guest_os.graceful_reboot()).await?;
self.wait_to_boot().await
}

/// Run a [`CommandSequence`] in the context of a booted and logged-in
/// guest. The guest is expected to be at a shell prompt when this sequence
/// is begun.
async fn run_command_sequence(
&self,
command_sequence: CommandSequence<'_>,
) -> Result<()> {
for step in command_sequence.0 {
match step {
CommandSequenceEntry::WaitFor(s) => {
Expand All @@ -896,22 +930,7 @@ impl TestVm {
}
}

// `shell_command_sequence` promises that the generated command sequence
// clears buffer of everything up to and including the input command
// before actually issuing the final '\n' that issues the command.
// This ensures that the buffer contents returned by this call contain
// only the command's output.
let out = self
.wait_for_serial_output(
self.guest_os.get_shell_prompt(),
Duration::from_secs(300),
)
.await?;

// Trim any leading newlines inserted when the command was issued and
// any trailing whitespace that isn't actually part of the command
// output. Any other embedded whitespace is the caller's problem.
Ok(out.trim().to_string())
Ok(())
}

/// Sends `string` to the guest's serial console worker, then waits for the
Expand Down
9 changes: 3 additions & 6 deletions phd-tests/tests/src/boot_order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,7 @@ async fn guest_can_adjust_boot_order(ctx: &Framework) {
assert_eq!(new_boot_order, written_boot_order);

// Now, reboot and check that the settings stuck.
vm.run_shell_command("reboot").await?;
vm.wait_to_boot().await?;
vm.graceful_reboot().await?;

let boot_order_after_reboot = read_efivar(&vm, BOOT_ORDER_VAR).await?;
assert_eq!(new_boot_order, boot_order_after_reboot);
Expand Down Expand Up @@ -469,8 +468,7 @@ async fn boot_order_source_priority(ctx: &Framework) {
.await?
.expect("unbootable was in the boot order");

vm_no_bootorder.run_shell_command("reboot").await?;
vm_no_bootorder.wait_to_boot().await?;
vm_no_bootorder.graceful_reboot().await?;

let reloaded_order = read_efivar(&vm_no_bootorder, BOOT_ORDER_VAR).await?;

Expand Down Expand Up @@ -515,8 +513,7 @@ async fn boot_order_source_priority(ctx: &Framework) {
.await?
.expect("unbootable was in the boot order");

vm.run_shell_command("reboot").await?;
vm.wait_to_boot().await?;
vm.graceful_reboot().await?;

let reloaded_order = read_efivar(&vm, BOOT_ORDER_VAR).await?;

Expand Down

0 comments on commit f5f7bf8

Please sign in to comment.