diff --git a/tmt/checks/watchdog.py b/tmt/checks/watchdog.py index 36b2968796..d849c91a4c 100644 --- a/tmt/checks/watchdog.py +++ b/tmt/checks/watchdog.py @@ -94,6 +94,12 @@ class WatchdogCheck(Check): ssh_ping: bool = field(default=False) ssh_ping_threshold: int = field(default=10) + def notify(self, invocation: 'TestInvocation') -> None: + """ Notify invocation that hard reboot is required """ + + invocation.hard_reboot_requested = True + invocation.terminate_process() + def do_ping( self, invocation: 'TestInvocation', @@ -183,7 +189,7 @@ def _handle_output(ping_output: str) -> None: if guest_context.ping_failures >= self.ping_threshold: logger.fail(f'exhausted {self.ping_threshold} ping attempts') - invocation.hard_reboot_requested = True + self.notify(invocation) try: assert invocation.guest.guest is not None # narrow type @@ -290,7 +296,7 @@ def _success(ncat_output: str) -> None: if guest_context.ping_failures >= self.ping_threshold: logger.fail(f'exhausted {self.ping_threshold} ping attempts') - invocation.hard_reboot_requested = True + self.notify(invocation) @provides_check('watchdog') diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index b51e79558b..8b0965db5e 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -3,7 +3,9 @@ import datetime import json import os +import signal as _signal import subprocess +import threading from contextlib import suppress from dataclasses import dataclass from typing import TYPE_CHECKING, Any, Optional, TypeVar, cast @@ -157,6 +159,7 @@ class TestInvocation: #: implementation and the test, it may be, for example, a shell process, #: SSH process, or a ``podman`` process. process: Optional[subprocess.Popen[bytes]] = None + process_lock: threading.Lock = field(default_factory=threading.Lock) check_data: dict[str, Any] = field(default_factory=dict) @@ -202,6 +205,7 @@ def reboot_request_path(self) -> Path: @property def soft_reboot_requested(self) -> bool: + """ If set, test requested a reboot """ return self.reboot_request_path.exists() #: If set, an asynchronous observer requested a reboot while the test was @@ -210,7 +214,7 @@ def soft_reboot_requested(self) -> bool: @property def reboot_requested(self) -> bool: - """ Whether a guest reboot has been requested by the test """ + """ Whether a guest reboot has been requested while the test was running """ return self.soft_reboot_requested or self.hard_reboot_requested def handle_reboot(self) -> bool: @@ -285,6 +289,38 @@ def handle_reboot(self) -> bool: return True + def terminate_process( + self, + signal: _signal.Signals = _signal.SIGTERM, + logger: Optional[tmt.log.Logger] = None) -> None: + """ + Terminate the invocation process. + + .. warning:: + + This method should be used carefully. Process managing the + invocation has been started by by some part of tmt code which is + responsible for its well-being. Unless you have a really good reason + to do so, doing things behind the tmt's back may lead to unexpected + results. + + :param signal: signal to send to the invocation process. + :param logger: logger to use for logging. + """ + + logger = logger or self.logger + + with self.process_lock: + if self.process is None: + logger.debug('Test invocation process cannot be terminated because it is unset.', + level=3) + + return + + logger.debug(f'Terminating process with {signal.name}.', level=3) + + self.process.send_signal(signal) + class ExecutePlugin(tmt.steps.Plugin[ExecuteStepDataT]): """ Common parent of execute plugins """ diff --git a/tmt/steps/execute/internal.py b/tmt/steps/execute/internal.py index 236c375a73..a29a60132c 100644 --- a/tmt/steps/execute/internal.py +++ b/tmt/steps/execute/internal.py @@ -325,7 +325,8 @@ def _save_process( command: Command, process: subprocess.Popen[bytes], logger: tmt.log.Logger) -> None: - invocation.process = process + with invocation.process_lock: + invocation.process = process # TODO: do we want timestamps? Yes, we do, leaving that for refactoring later, # to use some reusable decorator. @@ -363,7 +364,9 @@ def _save_process( elif tmt.utils.ProcessExitCodes.is_pidfile(invocation.return_code): logger.warn('Test failed to manage its pidfile.') - invocation.process = None + with invocation.process_lock: + invocation.process = None + invocation.end_time = self.format_timestamp(timer.end_time) invocation.real_duration = self.format_duration(timer.duration)