Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor test fixes to improve debuggability and readability #4862

Merged
merged 9 commits into from
Oct 22, 2024
27 changes: 16 additions & 11 deletions tests/framework/microvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,7 @@ def ssh_iface(self, iface_idx=0):
ssh_key=self.ssh_key,
user="root",
host=guest_ip,
on_error=self._dump_debug_information,
)

@property
Expand All @@ -1002,19 +1003,23 @@ def thread_backtraces(self):
)
return "\n".join(backtraces)

def _dump_debug_information(self, exc: Exception):
"""
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}"
)

def wait_for_ssh_up(self):
"""Wait for guest running inside the microVM to come up and respond."""
try:
# Ensure that we have an initialized SSH connection to the guest that can
# run commands. The actual connection retry loop happens in SSHConnection._init_connection
self.ssh_iface(0)
except Exception as exc:
print(
f"Failed to establish SSH connection to guest: {exc}\n\n"
f"Firecracker logs:\n{self.log_data}\n"
f"Thread backtraces:\n{self.thread_backtraces}"
)
raise
# Ensure that we have an initialized SSH connection to the guest that can
# run commands. The actual connection retry loop happens in SSHConnection._init_connection
_ = self.ssh_iface(0)


class MicroVMFactory:
Expand Down
2 changes: 0 additions & 2 deletions tests/framework/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,6 @@ def run_cmd(cmd, check=False, shell=True, cwd=None, timeout=None) -> CommandRetu

# If a non-zero return code was thrown, raise an exception
if check and proc.returncode != 0:
CMDLOG.warning("Command failed: %s\n", output_message)

raise ChildProcessError(output_message)

CMDLOG.debug(output_message)
Expand Down
2 changes: 1 addition & 1 deletion tests/framework/utils_vsock.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ def check_guest_connections(vm, server_port_path, blob_path, blob_hash):
cmd += " ({})& ".format(worker_cmd)
cmd += ' workers="$workers $!";'
cmd += "done;"
cmd += "for w in $workers; do wait $w || exit -1; done"
cmd += "for w in $workers; do wait $w || (wait; exit 1); done"
pb8o marked this conversation as resolved.
Show resolved Hide resolved

ecode, _, stderr = vm.ssh.run(cmd)
echo_server.terminate()
Expand Down
27 changes: 23 additions & 4 deletions tests/host_tools/network.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class SSHConnection:
ssh -i ssh_key_path username@hostname
"""

def __init__(self, netns, ssh_key: Path, host, user):
def __init__(self, netns, ssh_key: Path, host, user, *, on_error=None):
"""Instantiate a SSH client and connect to a microVM."""
self.netns = netns
self.ssh_key = ssh_key
Expand All @@ -37,6 +37,8 @@ def __init__(self, netns, ssh_key: Path, host, user):
self.host = host
self.user = user

self._on_error = None

self.options = [
"-o",
"LogLevel=ERROR",
Expand All @@ -52,7 +54,18 @@ def __init__(self, netns, ssh_key: Path, host, user):
str(self.ssh_key),
]

self._init_connection()
# _init_connection loops until it can connect to the guest
# dumping debug state on every iteration is not useful or wanted, so
# only dump it once if _all_ iterations fail.
try:
self._init_connection()
except Exception as exc:
if on_error:
on_error(exc)

raise

self._on_error = on_error

def remote_path(self, path):
"""Convert a path to remote"""
Expand Down Expand Up @@ -90,7 +103,7 @@ 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=10, debug=True)
self.check_output("true", timeout=100, debug=True)

def run(self, cmd_string, timeout=None, *, check=False, debug=False):
"""
Expand Down Expand Up @@ -123,7 +136,13 @@ def _exec(self, cmd, timeout=None, check=False):
if self.netns is not None:
cmd = ["ip", "netns", "exec", self.netns] + cmd

return utils.run_cmd(cmd, check=check, timeout=timeout)
try:
return utils.run_cmd(cmd, check=check, timeout=timeout)
except Exception as exc:
if self._on_error:
self._on_error(exc)

raise


def mac_from_ip(ip_address):
Expand Down
2 changes: 1 addition & 1 deletion tests/host_tools/vsock_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ int run_echo(uint32_t cid, uint32_t port) {
}
}

return 0;
return close(sock);
}


Expand Down
11 changes: 5 additions & 6 deletions tests/integration_tests/functional/test_snapshot_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ def test_5_snapshots(
# Create a snapshot from a microvm.
start_guest_echo_server(vm)
snapshot = vm.make_snapshot(snapshot_type)
base_snapshot = snapshot
vm.kill()

for i in range(seq_len):
Expand All @@ -183,20 +182,20 @@ def test_5_snapshots(

time.sleep(2)
logger.info("Create snapshot %s #%d.", snapshot_type, i + 1)
snapshot = microvm.make_snapshot(snapshot_type)
new_snapshot = microvm.make_snapshot(snapshot_type)

# If we are testing incremental snapshots we must merge the base with
# current layer.
if snapshot.is_diff:
logger.info("Base: %s, Layer: %s", base_snapshot.mem, snapshot.mem)
snapshot = snapshot.rebase_snapshot(
base_snapshot, use_snapshot_editor=use_snapshot_editor
logger.info("Base: %s, Layer: %s", snapshot.mem, new_snapshot.mem)
new_snapshot = new_snapshot.rebase_snapshot(
snapshot, use_snapshot_editor=use_snapshot_editor
)

microvm.kill()
copied_snapshot.delete()
# Update the base for next iteration.
base_snapshot = snapshot
snapshot = new_snapshot


def test_patch_drive_snapshot(uvm_nano, microvm_factory):
Expand Down
Loading