Skip to content

Commit

Permalink
refactor(tests): always check ssh output
Browse files Browse the repository at this point in the history
Ssh connection failure should be considered an error by default. In order
to unify handling of this case, move check for non zero return code
into the function itself. Because of this `check_output` method is
not needed anymore and was removed.

Signed-off-by: Egor Lazarchuk <[email protected]>

sq

Signed-off-by: Egor Lazarchuk <[email protected]>
  • Loading branch information
ShadowCurse committed Dec 4, 2024
1 parent d27e697 commit 42051ba
Show file tree
Hide file tree
Showing 25 changed files with 77 additions and 104 deletions.
3 changes: 1 addition & 2 deletions tests/framework/microvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1003,14 +1003,13 @@ def thread_backtraces(self):
)
return "\n".join(backtraces)

def _dump_debug_information(self, exc: Exception):
def _dump_debug_information(self):
"""
Dumps debug information about this microvm
Used for example when running a command inside the guest via `SSHConnection.check_output` fails.
"""
print(
f"Failure executing command via SSH in microVM: {exc}\n\n"
f"Firecracker logs:\n{self.log_data}\n"
f"Thread backtraces:\n{self.thread_backtraces}"
)
Expand Down
6 changes: 2 additions & 4 deletions tests/framework/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,7 @@ def get_free_mem_ssh(ssh_connection):
:param ssh_connection: connection to the guest
:return: available mem column output of 'free'
"""
_, stdout, stderr = ssh_connection.run("cat /proc/meminfo | grep MemAvailable")
assert stderr == ""
_, stdout, _ = ssh_connection.run("cat /proc/meminfo | grep MemAvailable")

# Split "MemAvailable: 123456 kB" and validate it
meminfo_data = stdout.split()
Expand Down Expand Up @@ -625,8 +624,7 @@ def guest_run_fio_iteration(ssh_connection, iteration):
--output /tmp/fio{} > /dev/null &""".format(
iteration
)
exit_code, _, stderr = ssh_connection.run(fio)
assert exit_code == 0, stderr
ssh_connection.run(fio)


def check_filesystem(ssh_connection, disk_fmt, disk):
Expand Down
2 changes: 1 addition & 1 deletion tests/framework/utils_iperf.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def spawn_iperf3_client(self, client_idx, client_mode_flag):
.build()
)

return self._microvm.ssh.check_output(cmd).stdout
return self._microvm.ssh.run(cmd).stdout

def host_command(self, port_offset):
"""Builds the command used for spawning an iperf3 server on the host"""
Expand Down
5 changes: 2 additions & 3 deletions tests/framework/utils_vsock.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def start_guest_echo_server(vm):
Returns a UDS path to connect to the server.
"""
cmd = f"nohup socat VSOCK-LISTEN:{ECHO_SERVER_PORT},backlog=128,reuseaddr,fork EXEC:'/bin/cat' > /dev/null 2>&1 &"
vm.ssh.check_output(cmd)
vm.ssh.run(cmd)

# Give the server time to initialise
time.sleep(1)
Expand Down Expand Up @@ -214,8 +214,7 @@ def _copy_vsock_data_to_guest(ssh_connection, blob_path, vm_blob_path, vsock_hel
# Copy the data file and a vsock helper to the guest.

cmd = "mkdir -p /tmp/vsock"
ecode, _, _ = ssh_connection.run(cmd)
assert ecode == 0, "Failed to set up tmpfs drive on the guest."
ssh_connection.run(cmd)

ssh_connection.scp_put(vsock_helper, "/tmp/vsock_helper")
ssh_connection.scp_put(blob_path, vm_blob_path)
Expand Down
32 changes: 20 additions & 12 deletions tests/host_tools/network.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def remote_path(self, path):

def _scp(self, path1, path2, options):
"""Copy files to/from the VM using scp."""
self._exec(["scp", *options, path1, path2], check=True)
self._exec(["scp", *options, path1, path2])

def scp_put(self, local_path, remote_path, recursive=False):
"""Copy files to the VM using scp."""
Expand Down Expand Up @@ -111,9 +111,10 @@ def _init_connection(self):
"""
self.check_output("true", timeout=100, debug=True)

def run(self, cmd_string, timeout=None, *, check=False, debug=False):
def run(self, cmd_string, timeout=None, *, check=True, debug=False):
"""
Execute the command passed as a string in the ssh context.
By default raises an exception on non-zero return code of remote command.
If `debug` is set, pass `-vvv` to `ssh`. Note that this will clobber stderr.
"""
Expand All @@ -124,22 +125,29 @@ def run(self, cmd_string, timeout=None, *, check=False, debug=False):

return self._exec(command, timeout, check=check)

def check_output(self, cmd_string, timeout=None, *, debug=False):
"""Same as `run`, but raises an exception on non-zero return code of remote command"""
return self.run(cmd_string, timeout, check=True, debug=debug)

def _exec(self, cmd, timeout=None, check=False):
def _exec(self, cmd, timeout=None, check=True):
"""Private function that handles the ssh client invocation."""
if self.netns is not None:
cmd = ["ip", "netns", "exec", self.netns] + cmd

try:
return utils.run_cmd(cmd, check=check, timeout=timeout)
except Exception as exc:
rc, stdout, stderr = utils.run_cmd(cmd, timeout=timeout)

if not check:
return rc, stdout, stderr

if rc != 0:
print(
f"SSH command {cmd} exited with non zero error code: {rc}\n"
f"stdout: {stdout}\n"
f"stderr: {stderr}\n"
)

if self._on_error:
self._on_error(exc)
self._on_error()

raise
assert False

return rc, stdout, stderr

# pylint:disable=invalid-name
def Popen(
Expand Down
25 changes: 4 additions & 21 deletions tests/integration_tests/functional/test_balloon.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,23 +41,11 @@ def get_rss_from_pmap():

def lower_ssh_oom_chance(ssh_connection):
"""Lure OOM away from ssh process"""
logger = logging.getLogger("lower_ssh_oom_chance")

cmd = "cat /run/sshd.pid"
exit_code, stdout, stderr = ssh_connection.run(cmd)
# add something to the logs for troubleshooting
if exit_code != 0:
logger.error("while running: %s", cmd)
logger.error("stdout: %s", stdout)
logger.error("stderr: %s", stderr)

_, stdout, _ = ssh_connection.run(cmd)
for pid in stdout.split(" "):
cmd = f"choom -n -1000 -p {pid}"
exit_code, stdout, stderr = ssh_connection.run(cmd)
if exit_code != 0:
logger.error("while running: %s", cmd)
logger.error("stdout: %s", stdout)
logger.error("stderr: %s", stderr)
ssh_connection.run(cmd)


def make_guest_dirty_memory(ssh_connection, amount_mib=32):
Expand All @@ -68,12 +56,7 @@ def make_guest_dirty_memory(ssh_connection, amount_mib=32):

cmd = f"/usr/local/bin/fillmem {amount_mib}"
try:
exit_code, stdout, stderr = ssh_connection.run(cmd, timeout=1.0)
# add something to the logs for troubleshooting
if exit_code != 0:
logger.error("while running: %s", cmd)
logger.error("stdout: %s", stdout)
logger.error("stderr: %s", stderr)
ssh_connection.run(cmd, timeout=1.0)
except TimeoutExpired:
# It's ok if this expires. Sometimes the SSH connection
# gets killed by the OOM killer *after* the fillmem program
Expand Down Expand Up @@ -558,4 +541,4 @@ def test_memory_scrub(microvm_factory, guest_kernel, rootfs):
# Wait for the deflate to complete.
_ = get_stable_rss_mem_by_pid(firecracker_pid)

microvm.ssh.check_output("/usr/local/bin/readmem {} {}".format(60, 1))
microvm.ssh.run("/usr/local/bin/readmem {} {}".format(60, 1))
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def _check_cpu_features_arm(test_microvm, guest_kv, template_name=None):
case CpuModel.ARM_NEOVERSE_V1, _, None:
expected_cpu_features = DEFAULT_G3_FEATURES_5_10

_, stdout, _ = test_microvm.ssh.check_output(CPU_FEATURES_CMD)
_, stdout, _ = test_microvm.ssh.run(CPU_FEATURES_CMD)
flags = set(stdout.strip().split(" "))
assert flags == expected_cpu_features

Expand All @@ -77,7 +77,7 @@ def test_host_vs_guest_cpu_features_aarch64(uvm_nano):
vm.add_net_iface()
vm.start()
host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.strip().split(" "))
guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.strip().split(" "))
guest_feats = set(vm.ssh.run(CPU_FEATURES_CMD).stdout.strip().split(" "))

cpu_model = cpuid_utils.get_cpu_model_name()
match cpu_model:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ def test_host_vs_guest_cpu_features_x86_64(uvm_nano):
vm.add_net_iface()
vm.start()
host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.strip().split(" "))
guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.strip().split(" "))
guest_feats = set(vm.ssh.run(CPU_FEATURES_CMD).stdout.strip().split(" "))

cpu_model = cpuid_utils.get_cpu_codename()
match cpu_model:
Expand Down
7 changes: 3 additions & 4 deletions tests/integration_tests/functional/test_drive_vhost_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ def _check_block_size(ssh_connection, dev_path, size):
"""
Checks the size of the block device.
"""
_, stdout, stderr = ssh_connection.run("blockdev --getsize64 {}".format(dev_path))
assert stderr == ""
_, stdout, _ = ssh_connection.run("blockdev --getsize64 {}".format(dev_path))
assert stdout.strip() == str(size)


Expand Down Expand Up @@ -297,7 +296,7 @@ def test_config_change(microvm_factory, guest_kernel, rootfs):
_check_block_size(vm.ssh, "/dev/vdb", orig_size * 1024 * 1024)

# Check that we can create a filesystem and mount it
vm.ssh.check_output(mkfs_mount_cmd)
vm.ssh.run(mkfs_mount_cmd)

for new_size in new_sizes:
# Instruct the backend to resize the device.
Expand All @@ -312,4 +311,4 @@ def test_config_change(microvm_factory, guest_kernel, rootfs):
_check_block_size(vm.ssh, "/dev/vdb", new_size * 1024 * 1024)

# Check that we can create a filesystem and mount it
vm.ssh.check_output(mkfs_mount_cmd)
vm.ssh.run(mkfs_mount_cmd)
14 changes: 5 additions & 9 deletions tests/integration_tests/functional/test_drive_virtio.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ def test_patch_drive(uvm_plain_any, io_engine):
# of the device, in bytes.
blksize_cmd = "LSBLK_DEBUG=all lsblk -b /dev/vdb --output SIZE"
size_bytes_str = "536870912" # = 512 MiB
_, stdout, _ = test_microvm.ssh.check_output(blksize_cmd)
_, stdout, _ = test_microvm.ssh.run(blksize_cmd)
lines = stdout.split("\n")
# skip "SIZE"
assert lines[1].strip() == size_bytes_str
Expand Down Expand Up @@ -354,14 +354,12 @@ def test_flush(uvm_plain_rw, io_engine):


def _check_block_size(ssh_connection, dev_path, size):
_, stdout, stderr = ssh_connection.run("blockdev --getsize64 {}".format(dev_path))
assert stderr == ""
_, stdout, _ = ssh_connection.run("blockdev --getsize64 {}".format(dev_path))
assert stdout.strip() == str(size)


def _check_file_size(ssh_connection, dev_path, size):
_, stdout, stderr = ssh_connection.run("stat --format=%s {}".format(dev_path))
assert stderr == ""
_, stdout, _ = ssh_connection.run("stat --format=%s {}".format(dev_path))
assert stdout.strip() == str(size)


Expand All @@ -379,7 +377,5 @@ def _check_drives(test_microvm, assert_dict, keys_array):


def _check_mount(ssh_connection, dev_path):
_, _, stderr = ssh_connection.run(f"mount {dev_path} /tmp", timeout=30.0)
assert stderr == ""
_, _, stderr = ssh_connection.run("umount /tmp", timeout=30.0)
assert stderr == ""
ssh_connection.run(f"mount {dev_path} /tmp", timeout=30.0)
ssh_connection.run("umount /tmp", timeout=30.0)
4 changes: 2 additions & 2 deletions tests/integration_tests/functional/test_kvm_ptp.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def test_kvm_ptp(uvm_plain_any):
vm.add_net_iface()
vm.start()

vm.ssh.check_output("[ -c /dev/ptp0 ]")
vm.ssh.run("[ -c /dev/ptp0 ]")

# phc_ctl[14515.127]: clock time is 1697545854.728335694 or Tue Oct 17 12:30:54 2023
vm.ssh.check_output("phc_ctl /dev/ptp0 -- get")
vm.ssh.run("phc_ctl /dev/ptp0 -- get")
8 changes: 4 additions & 4 deletions tests/integration_tests/functional/test_net.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_high_ingress_traffic(uvm_plain_any):
test_microvm.start()

# Start iperf3 server on the guest.
test_microvm.ssh.check_output("{} -sD\n".format(IPERF_BINARY_GUEST))
test_microvm.ssh.run("{} -sD\n".format(IPERF_BINARY_GUEST))
time.sleep(1)

# Start iperf3 client on the host. Send 1Gbps UDP traffic.
Expand All @@ -53,7 +53,7 @@ def test_high_ingress_traffic(uvm_plain_any):
# Check if the high ingress traffic broke the net interface.
# If the net interface still works we should be able to execute
# ssh commands.
test_microvm.ssh.check_output("echo success\n")
test_microvm.ssh.run("echo success\n")


def test_multi_queue_unsupported(uvm_plain):
Expand Down Expand Up @@ -97,8 +97,8 @@ def run_udp_offload_test(vm):
message = "x"

# Start a UDP server in the guest
# vm.ssh.check_output(f"nohup socat UDP-LISTEN:{port} - > {out_filename} &")
vm.ssh.check_output(
# vm.ssh.run(f"nohup socat UDP-LISTEN:{port} - > {out_filename} &")
vm.ssh.run(
f"nohup socat UDP4-LISTEN:{port} OPEN:{out_filename},creat > /dev/null 2>&1 &"
)

Expand Down
9 changes: 3 additions & 6 deletions tests/integration_tests/functional/test_net_config_space.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,7 @@ def _find_iomem_range(ssh_connection, dev_name):
# its contents and grep for the VirtIO device name, which
# with ACPI is "LNRO0005:XY".
cmd = f"cat /proc/iomem | grep -m 1 {dev_name}"
rc, stdout, stderr = ssh_connection.run(cmd)
assert rc == 0, stderr
_, stdout, _ = ssh_connection.run(cmd)

# Take range in the form 'start-end' from line. The line looks like this:
# d00002000-d0002fff : LNRO0005:02
Expand Down Expand Up @@ -259,8 +258,7 @@ def _get_net_mem_addr_base_x86_cmdline(ssh_connection, if_name):
"""Check for net device memory start address via command line arguments"""
sys_virtio_mmio_cmdline = "/sys/devices/virtio-mmio-cmdline/"
cmd = "ls {} | grep virtio-mmio. | sed 's/virtio-mmio.//'"
exit_code, stdout, stderr = ssh_connection.run(cmd.format(sys_virtio_mmio_cmdline))
assert exit_code == 0, stderr
_, stdout, _ = ssh_connection.run(cmd.format(sys_virtio_mmio_cmdline))
virtio_devs_idx = stdout.strip().split()

cmd = "cat /proc/cmdline"
Expand Down Expand Up @@ -299,8 +297,7 @@ def _get_net_mem_addr_base(ssh_connection, if_name):
if platform.machine() == "aarch64":
sys_virtio_mmio_cmdline = "/sys/devices/platform"
cmd = "ls {} | grep .virtio_mmio".format(sys_virtio_mmio_cmdline)
rc, stdout, _ = ssh_connection.run(cmd)
assert rc == 0
_, stdout, _ = ssh_connection.run(cmd)

virtio_devs = stdout.split()
devs_addr = list(map(lambda dev: dev.split(".")[0], virtio_devs))
Expand Down
8 changes: 4 additions & 4 deletions tests/integration_tests/functional/test_pause_resume.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ def test_pause_resume(uvm_nano):

# Verify guest is no longer active.
with pytest.raises(ChildProcessError):
microvm.ssh.check_output("true")
microvm.ssh.run("true")

# Verify emulation was indeed paused and no events from either
# guest or host side were handled.
verify_net_emulation_paused(microvm.flush_metrics())

# Verify guest is no longer active.
with pytest.raises(ChildProcessError):
microvm.ssh.check_output("true")
microvm.ssh.run("true")

# Pausing the microVM when it is already `Paused` is allowed
# (microVM remains in `Paused` state).
Expand Down Expand Up @@ -152,7 +152,7 @@ def test_kvmclock_ctrl(uvm_plain_any):
# console. This detail is important as it writing in the console seems to increase the probability
# that we will pause the execution inside the kernel and cause a lock up. Setting KVM_CLOCK_CTRL
# bit that informs the guest we're pausing the vCPUs, should avoid that lock up.
microvm.ssh.check_output(
microvm.ssh.run(
"timeout 60 sh -c 'while true; do ls -R /; done' > /dev/ttyS0 2>&1 < /dev/null &"
)

Expand All @@ -161,7 +161,7 @@ def test_kvmclock_ctrl(uvm_plain_any):
time.sleep(5)
microvm.api.vm.patch(state="Resumed")

dmesg = microvm.ssh.check_output("dmesg").stdout
dmesg = microvm.ssh.run("dmesg").stdout
assert "rcu_sched self-detected stall on CPU" not in dmesg
assert "rcu_preempt detected stalls on CPUs/tasks" not in dmesg
assert "BUG: soft lockup -" not in dmesg
2 changes: 1 addition & 1 deletion tests/integration_tests/functional/test_rng.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def _get_throughput(ssh, random_bytes):
# Issue a `dd` command to request 100 times `random_bytes` from the device.
# 100 here is used to get enough confidence on the achieved throughput.
cmd = "dd if=/dev/hwrng of=/dev/null bs={} count=100".format(random_bytes)
_, _, stderr = ssh.check_output(cmd)
_, _, stderr = ssh.run(cmd)

# dd gives its output on stderr
return _process_dd_output(stderr)
Expand Down
8 changes: 3 additions & 5 deletions tests/integration_tests/functional/test_serial_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,15 +188,13 @@ def test_serial_block(uvm_plain_any):
os.kill(test_microvm.screen_pid, signal.SIGSTOP)

# Generate a random text file.
test_microvm.ssh.check_output(
"base64 /dev/urandom | head -c 100000 > /tmp/file.txt"
)
test_microvm.ssh.run("base64 /dev/urandom | head -c 100000 > /tmp/file.txt")

# Dump output to terminal
test_microvm.ssh.check_output("cat /tmp/file.txt > /dev/ttyS0")
test_microvm.ssh.run("cat /tmp/file.txt > /dev/ttyS0")

# Check that the vCPU isn't blocked.
test_microvm.ssh.check_output("cd /")
test_microvm.ssh.run("cd /")

# Check the metrics to see if the serial missed bytes.
fc_metrics = test_microvm.flush_metrics()
Expand Down
Loading

0 comments on commit 42051ba

Please sign in to comment.