refactor(tests): always check ssh output

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]>

tests/framework/utils.py

@@ -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()
@@ -441,7 +440,7 @@ def assert_seccomp_level(pid, seccomp_level):
 
 def run_guest_cmd(ssh_connection, cmd, expected, use_json=False):
     """Runs a shell command at the remote accessible via SSH"""
-    _, stdout, stderr = ssh_connection.check_output(cmd)
+    _, stdout, stderr = ssh_connection.run(cmd)
     assert stderr == ""
     stdout = stdout if not use_json else json.loads(stdout)
     assert stdout == expected
@@ -625,20 +624,19 @@ 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):
     """Check for filesystem corruption inside a microVM."""
     if disk_fmt == "squashfs":
         return
-    ssh_connection.check_output(f"fsck.{disk_fmt} -n {disk}")
+    ssh_connection.run(f"fsck.{disk_fmt} -n {disk}")
 
 
 def check_entropy(ssh_connection):
     """Check that we can get random numbers from /dev/hwrng"""
-    ssh_connection.check_output("dd if=/dev/hwrng of=/dev/null bs=4096 count=1")
+    ssh_connection.run("dd if=/dev/hwrng of=/dev/null bs=4096 count=1")
 
 
 @retry(wait=wait_fixed(0.5), stop=stop_after_attempt(5), reraise=True)

tests/framework/utils_iperf.py

@@ -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"""

tests/framework/utils_vsock.py

@@ -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)
@@ -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)

tests/host_tools/network.py

@@ -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."""
@@ -109,11 +109,12 @@ def _init_connection(self):
         We'll keep trying to execute a remote command that can't fail
         (`/bin/true`), until we get a successful (0) exit code.
         """
-        self.check_output("true", timeout=100, debug=True)
+        self.run("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.
         """
@@ -124,11 +125,7 @@ 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

tests/integration_tests/functional/test_balloon.py

@@ -41,39 +41,20 @@ 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):
     """Tell the guest, over ssh, to dirty `amount` pages of memory."""
-    logger = logging.getLogger("make_guest_dirty_memory")
-
     lower_ssh_oom_chance(ssh_connection)
 
     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
@@ -558,4 +539,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))

tests/integration_tests/functional/test_cpu_features_aarch64.py

@@ -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
 
@@ -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:

tests/integration_tests/functional/test_cpu_features_x86_64.py

@@ -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:

tests/integration_tests/functional/test_drive_vhost_user.py

@@ -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)
 
 
@@ -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.
@@ -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)

tests/integration_tests/functional/test_drive_virtio.py

@@ -51,7 +51,8 @@ def test_rescan_file(uvm_plain_any, io_engine):
     utils.check_output(f"truncate --size {truncated_size}M {fs.path}")
     block_copy_name = "/tmp/dev_vdb_copy"
     _, _, stderr = test_microvm.ssh.run(
-        f"dd if=/dev/vdb of={block_copy_name} bs=1M count={block_size}"
+        f"dd if=/dev/vdb of={block_copy_name} bs=1M count={block_size}",
+        check=False,
     )
     assert "dd: error reading '/dev/vdb': Input/output error" in stderr
     _check_file_size(test_microvm.ssh, f"{block_copy_name}", truncated_size * MB)
@@ -283,7 +284,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
@@ -354,14 +355,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)
 
 
@@ -379,7 +378,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)

tests/integration_tests/functional/test_kvm_ptp.py

@@ -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")

tests/integration_tests/functional/test_max_devices.py

@@ -33,7 +33,7 @@ def test_attach_maximum_devices(uvm_plain_any):
     # Test that network devices attached are operational.
     for i in range(MAX_DEVICES_ATTACHED - 1):
         # Verify if guest can run commands.
-        test_microvm.ssh_iface(i).check_output("sync")
+        test_microvm.ssh_iface(i).run("sync")
 
 
 @pytest.mark.skipif(

tests/integration_tests/functional/test_net.py

@@ -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.
@@ -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):
@@ -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 &"
     )
 
@@ -110,8 +110,8 @@ def run_udp_offload_test(vm):
     assert ret.returncode == 0, f"{ret.stdout=} {ret.stderr=}"
 
     # Check that the server received the message
-    ret = vm.ssh.run(f"cat {out_filename}")
-    assert ret.stdout == message, f"{ret.stdout=} {ret.stderr=}"
+    _, stdout, stderr = vm.ssh.run(f"cat {out_filename}")
+    assert stdout == message, f"{stdout=} {stderr=}"
 
 
 def test_tap_offload_booted(uvm_plain_any):

tests/integration_tests/functional/test_net_config_space.py

@@ -75,7 +75,7 @@ def test_net_change_mac_address(uvm_plain_any, change_net_config_space_bin):
     cmd = f"chmod u+x {rmt_path} && {rmt_path} {net_addr_base} {mac_hex}"
 
     # This should be executed successfully.
-    _, stdout, _ = ssh_conn.check_output(cmd)
+    _, stdout, _ = ssh_conn.run(cmd)
     assert stdout == mac
 
     # Discard any parasite data exchange which might've been
@@ -154,7 +154,7 @@ def _send_data_g2h(ssh_connection, host_ip, host_port, iterations, data, retries
     )
 
     # Wait server to initialize.
-    _, _, stderr = ssh_connection.check_output(cmd)
+    _, _, stderr = ssh_connection.run(cmd)
     # If this assert fails, a connection refused happened.
     assert stderr == ""
 
@@ -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
@@ -237,7 +236,7 @@ def _get_net_mem_addr_base_x86_acpi(ssh_connection, if_name):
     # are identified as "LNRO0005" and appear under /sys/devices/platform
     sys_virtio_mmio_cmdline = "/sys/devices/platform/"
     cmd = "ls {}"
-    _, stdout, _ = ssh_connection.check_output(cmd.format(sys_virtio_mmio_cmdline))
+    _, stdout, _ = ssh_connection.run(cmd.format(sys_virtio_mmio_cmdline))
     virtio_devs = list(filter(lambda x: "LNRO0005" in x, stdout.strip().split()))
 
     # For virtio-net LNRO0005 devices, we should have a path like:
@@ -247,7 +246,8 @@ def _get_net_mem_addr_base_x86_acpi(ssh_connection, if_name):
     cmd = "ls {}/{}/virtio{}/net"
     for idx, dev in enumerate(virtio_devs):
         _, guest_if_name, _ = ssh_connection.run(
-            cmd.format(sys_virtio_mmio_cmdline, dev, idx)
+            cmd.format(sys_virtio_mmio_cmdline, dev, idx),
+            check=False
         )
         if guest_if_name.strip() == if_name:
             return _find_iomem_range(ssh_connection, dev)[0]
@@ -259,12 +259,11 @@ 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"
-    _, cmd_line, _ = ssh_connection.check_output(cmd)
+    _, cmd_line, _ = ssh_connection.run(cmd)
     pattern_dev = re.compile("(virtio_mmio.device=4K@0x[0-9a-f]+:[0-9]+)+")
     pattern_addr = re.compile("virtio_mmio.device=4K@(0x[0-9a-f]+):[0-9]+")
     devs_addr = []
@@ -279,7 +278,8 @@ def _get_net_mem_addr_base_x86_cmdline(ssh_connection, if_name):
     cmd = "ls {}/virtio-mmio.{}/virtio{}/net"
     for idx in virtio_devs_idx:
         _, guest_if_name, _ = ssh_connection.run(
-            cmd.format(sys_virtio_mmio_cmdline, idx, idx)
+            cmd.format(sys_virtio_mmio_cmdline, idx, idx),
+            check=False
         )
         if guest_if_name.strip() == if_name:
             return devs_addr[int(idx)]
@@ -299,8 +299,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))