refactor(tests): move return code check for ssh into ssh.run

Ssh connection 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.

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

Author: ShadowCurse

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()
@@ -531,7 +530,7 @@ def generate_mmds_session_token(ssh_connection, ipv4_address, token_ttl):
     cmd += " -X PUT"
     cmd += ' -H  "X-metadata-token-ttl-seconds: {}"'.format(token_ttl)
     cmd += " http://{}/latest/api/token".format(ipv4_address)
-    _, stdout, _ = ssh_connection.run(cmd)
+    stdout, _ = ssh_connection.run(cmd)
     token = stdout
 
     return token
@@ -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):

tests/framework/utils_vsock.py

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

@@ -122,7 +122,14 @@ def run(self, cmd_string, timeout=None, *, check=False, debug=False):
         if debug:
             command.insert(1, "-vvv")
 
-        return self._exec(command, timeout, check=check)
+        exit_code, stdout, stderr = self._exec(command, timeout, check=check)
+        if exit_code != 0:
+            logger = logging.getLogger("ssh connection")
+            logger.error("while running: %s", cmd)
+            logger.error("stdout: %s", stdout)
+            logger.error("stderr: %s", stderr)
+            assert False, "ssh failed"
+        return stdout, stderr
 
     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"""
@@ -185,7 +192,7 @@ def mac_from_ip(ip_address):
 def get_guest_net_if_name(ssh_connection, guest_ip):
     """Get network interface name based on its IPv4 address."""
     cmd = "ip a s | grep '{}' | tr -s ' ' | cut -d' ' -f6".format(guest_ip)
-    _, guest_if_name, _ = ssh_connection.run(cmd)
+    guest_if_name, _ = ssh_connection.run(cmd)
     if_name = guest_if_name.strip()
     return if_name if if_name != "" else None
 

tests/integration_tests/functional/test_balloon.py

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

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)
 
 

tests/integration_tests/functional/test_drive_virtio.py

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

tests/integration_tests/functional/test_mmds.py

@@ -503,14 +503,14 @@ def test_guest_mmds_hang(uvm_plain, version):
     get_cmd += f" http://{DEFAULT_IPV4}/"
 
     if version == "V1":
-        _, stdout, _ = ssh_connection.run(get_cmd)
+        stdout, _ = ssh_connection.run(get_cmd)
         assert "Invalid request" in stdout
     else:
         # Generate token.
         token = generate_mmds_session_token(ssh_connection, DEFAULT_IPV4, token_ttl=60)
 
         get_cmd += ' -H  "X-metadata-token: {}"'.format(token)
-        _, stdout, _ = ssh_connection.run(get_cmd)
+        stdout, _ = ssh_connection.run(get_cmd)
         assert "Invalid request" in stdout
 
         # Do the same for a PUT request.
@@ -522,7 +522,7 @@ def test_guest_mmds_hang(uvm_plain, version):
         cmd += ' -d "some body"'
         cmd += " http://{}/".format(DEFAULT_IPV4)
 
-        _, stdout, _ = ssh_connection.run(cmd)
+        stdout, _ = ssh_connection.run(cmd)
         assert "Invalid request" in stdout
 
 
@@ -705,7 +705,7 @@ def test_mmds_v2_negative(uvm_plain):
         run_guest_cmd(ssh_connection, put_cmd.format(ttl), expected)
 
     # Valid `PUT` request to generate token.
-    _, stdout, _ = ssh_connection.run(put_cmd.format(1))
+    stdout, _ = ssh_connection.run(put_cmd.format(1))
     token = stdout
     assert len(token) > 0
 

tests/integration_tests/functional/test_net_config_space.py

@@ -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
@@ -246,7 +245,7 @@ def _get_net_mem_addr_base_x86_acpi(ssh_connection, if_name):
     # that corresponds to the virtio-net device.
     cmd = "ls {}/{}/virtio{}/net"
     for idx, dev in enumerate(virtio_devs):
-        _, guest_if_name, _ = ssh_connection.run(
+        guest_if_name, _ = ssh_connection.run(
             cmd.format(sys_virtio_mmio_cmdline, dev, idx)
         )
         if guest_if_name.strip() == if_name:
@@ -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"
@@ -278,7 +276,7 @@ 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(
+        guest_if_name, _ = ssh_connection.run(
             cmd.format(sys_virtio_mmio_cmdline, idx, idx)
         )
         if guest_if_name.strip() == if_name:
@@ -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))
@@ -310,7 +307,7 @@ def _get_net_mem_addr_base(ssh_connection, if_name):
         # accordingly when parsed inside `change_config_space.c`.
         hex_prefix = "0x"
         for idx, dev in enumerate(virtio_devs):
-            _, guest_if_name, _ = ssh_connection.run(
+            guest_if_name, _ = ssh_connection.run(
                 cmd.format(sys_virtio_mmio_cmdline, dev, idx)
             )
             if guest_if_name.strip() == if_name:

tests/integration_tests/functional/test_snapshot_basic.py

@@ -44,8 +44,7 @@ def _get_guest_drive_size(ssh_connection, guest_dev_name="/dev/vdb"):
     # `lsblk` command outputs 2 lines to STDOUT:
     # "SIZE" and the size of the device, in bytes.
     blksize_cmd = "LSBLK_DEBUG=all lsblk -b {} --output SIZE".format(guest_dev_name)
-    rc, stdout, stderr = ssh_connection.run(blksize_cmd)
-    assert rc == 0, stderr
+    stdout, _ = ssh_connection.run(blksize_cmd)
     lines = stdout.split("\n")
     return lines[1].strip()