test: Have tests where Firecracker exists mark VMs as killed

This is so that we do not try to kill them again in `Microvm.kill`,
which would result in a test failure.

Also remove various variants of "wait for Firecracker process to go
away", as `mark_killed` has a `wait_for_termination` inside of it.

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

tests/integration_tests/functional/test_api.py

@@ -1386,3 +1386,7 @@ def test_negative_snapshot_load_api(microvm_factory):
         )
 
     assert exc_info.value.args[2].headers["deprecation"]
+
+    # The snapshot/memory files above don't exist, but the request is otherwise syntactically valid.
+    # In this case, Firecracker exits.
+    vm.mark_killed()

tests/integration_tests/functional/test_api_server.py

@@ -26,3 +26,5 @@ def test_api_socket_in_use(uvm_plain):
     microvm.spawn()
     msg = "Failed to open the API socket at: /run/firecracker.socket. Check that it is not already used."
     microvm.check_log_message(msg)
+
+    microvm.mark_killed()

tests/integration_tests/functional/test_cmd_line_start.py

@@ -166,7 +166,8 @@ def test_config_start_no_api_exit(uvm_plain, vm_config_file):
     test_microvm.spawn()  # Start Firecracker and MicroVM
     time.sleep(3)  # Wait for startup
     test_microvm.ssh.run("reboot")  # Exit
-    time.sleep(3)  # Wait for shutdown
+
+    test_microvm.mark_killed()  # waits for process to terminate
 
     # Check error log and exit code
     test_microvm.check_log_message("Firecracker exiting successfully")
@@ -190,6 +191,8 @@ def test_config_bad_machine_config(uvm_plain, vm_config_file):
     test_microvm.spawn()
     test_microvm.check_log_message("Configuration for VMM from one single json failed")
 
+    test_microvm.mark_killed()
+
 
 @pytest.mark.parametrize(
     "test_config",
@@ -228,6 +231,8 @@ def test_config_machine_config_params(uvm_plain, test_config):
                 "Could not Start MicroVM from one single json",
             ]
         )
+
+        test_microvm.mark_killed()
     else:
         test_microvm.check_log_message(
             "Successfully started microvm that was configured from one single json"
@@ -334,6 +339,8 @@ def test_start_with_metadata_limit(uvm_plain):
         "Populating MMDS from file failed: The MMDS patch request doesn't fit."
     )
 
+    test_microvm.mark_killed()
+
 
 def test_start_with_metadata_default_limit(uvm_plain):
     """
@@ -352,6 +359,8 @@ def test_start_with_metadata_default_limit(uvm_plain):
         "Populating MMDS from file failed: The MMDS patch request doesn't fit."
     )
 
+    test_microvm.mark_killed()
+
 
 def test_start_with_missing_metadata(uvm_plain):
     """
@@ -373,6 +382,8 @@ def test_start_with_missing_metadata(uvm_plain):
         )
         test_microvm.check_log_message("No such file or directory")
 
+        test_microvm.mark_killed()
+
 
 def test_start_with_invalid_metadata(uvm_plain):
     """
@@ -392,6 +403,8 @@ def test_start_with_invalid_metadata(uvm_plain):
         test_microvm.check_log_message("MMDS error: metadata provided not valid json")
         test_microvm.check_log_message("EOF while parsing an object")
 
+        test_microvm.mark_killed()
+
 
 @pytest.mark.parametrize(
     "vm_config_file",

tests/integration_tests/functional/test_error_code.py

@@ -6,8 +6,6 @@
 
 import pytest
 
-from framework.utils import wait_process_termination
-
 
 @pytest.mark.skipif(
     platform.machine() != "aarch64",
@@ -32,7 +30,7 @@ def test_enosys_error_code(uvm_plain):
     vm.start()
 
     # Check if FC process is closed
-    wait_process_termination(vm.firecracker_pid)
+    vm.mark_killed()
 
     vm.check_log_message(
         "Received ENOSYS error because KVM failed to emulate an instruction."

tests/integration_tests/functional/test_shut_down.py

@@ -2,9 +2,7 @@
 # SPDX-License-Identifier: Apache-2.0
 """Tests scenarios for shutting down Firecracker/VM."""
 
-import os
 import platform
-import time
 
 from packaging import version
 
@@ -45,13 +43,7 @@ def test_reboot(uvm_plain_any):
     # the instance.
     vm.ssh.run("reboot")
 
-    while True:
-        # Pytest's timeout will kill the test even if the loop doesn't exit.
-        try:
-            os.kill(firecracker_pid, 0)
-            time.sleep(0.01)
-        except OSError:
-            break
+    vm.mark_killed()
 
     # Consume existing metrics
     datapoints = vm.get_all_metrics()

tests/integration_tests/functional/test_signals.py

@@ -10,8 +10,6 @@
 
 import pytest
 
-from framework import utils
-
 signum_str = {
     SIGBUS: "sigbus",
     SIGSEGV: "sigsegv",
@@ -54,11 +52,10 @@ def test_generic_signal_handler(uvm_plain, signum):
         # This is going to fail if process has exited.
         microvm.api.actions.put(action_type="FlushMetrics")
     else:
-        microvm.expect_kill_by_signal = True
-        # Ensure that the process was terminated.
-        utils.wait_process_termination(microvm.firecracker_pid)
         msg = "Shutting down VM after intercepting signal {}".format(signum)
 
+        microvm.mark_killed()
+
     microvm.check_log_message(msg)
 
     if signum != SIGSYS:
@@ -95,14 +92,8 @@ def test_sigxfsz_handler(uvm_plain_rw):
     # in order to make sure the SIGXFSZ metric is logged
     res.prlimit(firecracker_pid, res.RLIMIT_FSIZE, (size * 3, res.RLIM_INFINITY))
 
-    while True:
-        try:
-            utils.check_output("ps -p {}".format(firecracker_pid))
-            sleep(1)
-        except ChildProcessError:
-            break
+    microvm.mark_killed()
 
-    microvm.expect_kill_by_signal = True
     msg = "Shutting down VM after intercepting signal 25, code 0"
     microvm.check_log_message(msg)
     metric_line = json.loads(metrics_fd.readlines()[0])

tests/integration_tests/functional/test_snapshot_basic.py

@@ -15,7 +15,7 @@
 import host_tools.drive as drive_tools
 from framework.microvm import SnapshotType
 from framework.properties import global_props
-from framework.utils import check_filesystem, check_output, wait_process_termination
+from framework.utils import check_filesystem, check_output
 from framework.utils_vsock import (
     ECHO_SERVER_PORT,
     VSOCK_UDS_PATH,
@@ -277,8 +277,7 @@ def test_load_snapshot_failure_handling(uvm_plain):
     with pytest.raises(RuntimeError, match=expected_msg):
         vm.api.snapshot_load.put(mem_file_path=jailed_mem, snapshot_path=jailed_vmstate)
 
-    # Check if FC process is closed
-    wait_process_termination(vm.firecracker_pid)
+    vm.mark_killed()
 
 
 def test_cmp_full_and_first_diff_mem(microvm_factory, guest_kernel, rootfs):
@@ -380,6 +379,8 @@ def test_negative_snapshot_permissions(uvm_plain_rw, microvm_factory):
     with pytest.raises(RuntimeError, match=expected_err):
         microvm.restore_from_snapshot(snapshot, resume=True)
 
+    microvm.mark_killed()
+
     # Remove permissions for state file.
     os.chmod(snapshot.vmstate, 0o000)
 
@@ -393,6 +394,8 @@ def test_negative_snapshot_permissions(uvm_plain_rw, microvm_factory):
     with pytest.raises(RuntimeError, match=expected_err):
         microvm.restore_from_snapshot(snapshot, resume=True)
 
+    microvm.mark_killed()
+
     # Restore permissions for state file.
     os.chmod(snapshot.vmstate, 0o744)
     os.chmod(snapshot.mem, 0o744)
@@ -407,6 +410,8 @@ def test_negative_snapshot_permissions(uvm_plain_rw, microvm_factory):
     with pytest.raises(RuntimeError, match=re.escape(expected_err)):
         microvm.restore_from_snapshot(snapshot, resume=True)
 
+    microvm.mark_killed()
+
 
 def test_negative_snapshot_create(uvm_nano):
     """

tests/integration_tests/functional/test_uffd.py

@@ -72,6 +72,8 @@ def test_bad_socket_path(uvm_plain, snapshot):
             snapshot_path=jailed_vmstate,
         )
 
+    vm.mark_killed()
+
 
 def test_unbinded_socket(uvm_plain, snapshot):
     """
@@ -96,6 +98,8 @@ def test_unbinded_socket(uvm_plain, snapshot):
             snapshot_path=jailed_vmstate,
         )
 
+    vm.mark_killed()
+
 
 def test_valid_handler(uvm_plain, snapshot, uffd_handler_paths):
     """

tests/integration_tests/security/test_custom_seccomp.py

@@ -7,7 +7,6 @@
 import tempfile
 import time
 
-import psutil
 import pytest
 import requests
 
@@ -188,7 +187,6 @@ def test_failing_filter(uvm_plain):
     # Give time for the process to get killed
     time.sleep(1)
 
-    test_microvm.expect_kill_by_signal = True
     # Check the logger output
     ioctl_num = 16 if platform.machine() == "x86_64" else 29
     test_microvm.check_log_message(
@@ -207,8 +205,7 @@ def test_failing_filter(uvm_plain):
 
     assert num_faults == 1
 
-    # assert that the process was killed
-    assert not psutil.pid_exists(test_microvm.firecracker_pid)
+    test_microvm.mark_killed()
 
 
 @pytest.mark.parametrize("vm_config_file", ["framework/vm_config.json"])
@@ -235,5 +232,4 @@ def test_invalid_bpf(uvm_plain, vm_config_file):
     # give time for the process to get killed
     time.sleep(1)
 
-    # assert that the process was killed
-    assert not psutil.pid_exists(test_microvm.firecracker_pid)
+    test_microvm.mark_killed()

tests/integration_tests/security/test_jail.py

@@ -10,7 +10,6 @@
 import time
 from pathlib import Path
 
-import psutil
 import pytest
 import requests
 import urllib3
@@ -511,14 +510,13 @@ def test_negative_file_size_limit(uvm_plain):
         urllib3.exceptions.ProtocolError,
         requests.exceptions.ConnectionError,
     ) as _error:
-        test_microvm.expect_kill_by_signal = True
         # Check the microVM received signal `SIGXFSZ` (25),
         # which corresponds to exceeding file size limit.
         msg = "Shutting down VM after intercepting signal 25, code 0"
         test_microvm.check_log_message(msg)
         time.sleep(1)
-        # Check that the process was terminated.
-        assert not psutil.pid_exists(test_microvm.firecracker_pid)
+
+        test_microvm.mark_killed()
     else:
         assert False, "Negative test failed"
 
@@ -535,6 +533,8 @@ def test_negative_no_file_limit(uvm_plain):
         test_microvm.spawn()
     except ChildProcessError as error:
         assert "No file descriptors available (os error 24)" in str(error)
+
+        test_microvm.mark_killed()
     else:
         assert False, "Negative test failed"