From d4d7550108e4e3fe6f9303e2a57379fba10da77d Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Mon, 23 Oct 2023 02:11:11 +0200 Subject: [PATCH 1/4] nixos/test-driver: provide a global timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the debut of the test-driver, we didn't obtain a race timer with the test execution to ensure that tests doesn't run beyond a certain amount of time. This is particularly important when you are running into hanging tests which cannot be detected by current facilities (requires more pvpanic wiring up, QMP API stuff, etc.). Two easy examples: - Some QEMU tests may get stuck in some situation and run for more than 24 hours → we default to 1 hour max. - Some QEMU tests may panic in the wrong place, e.g. UEFI firmware or worse → end users can set a "reasonable" amount of time And then, we should let the retry logic retest them until they succeed and adjust their global timeouts. Of course, this does not help with the fact that the timeout may need to be a function of the actual busyness of the machine running the tests. This is only one step towards increased reliability. --- nixos/lib/test-driver/test_driver/__init__.py | 9 +++++++ nixos/lib/test-driver/test_driver/driver.py | 25 +++++++++++++++++++ nixos/lib/testing-python.nix | 1 + nixos/lib/testing/driver.nix | 13 ++++++++++ 4 files changed, 48 insertions(+) diff --git a/nixos/lib/test-driver/test_driver/__init__.py b/nixos/lib/test-driver/test_driver/__init__.py index 371719d7a988..9daae1e941a6 100755 --- a/nixos/lib/test-driver/test_driver/__init__.py +++ b/nixos/lib/test-driver/test_driver/__init__.py @@ -76,6 +76,14 @@ def main() -> None: nargs="*", help="vlans to span by the driver", ) + arg_parser.add_argument( + "--global-timeout", + type=int, + metavar="GLOBAL_TIMEOUT", + action=EnvDefault, + envvar="globalTimeout", + help="Timeout in seconds for the whole test", + ) arg_parser.add_argument( "-o", "--output_directory", @@ -103,6 +111,7 @@ def main() -> None: args.testscript.read_text(), args.output_directory.resolve(), args.keep_vm_state, + args.global_timeout, ) as driver: if args.interactive: history_dir = os.getcwd() diff --git a/nixos/lib/test-driver/test_driver/driver.py b/nixos/lib/test-driver/test_driver/driver.py index 723c80717860..786821b0cc0d 100644 --- a/nixos/lib/test-driver/test_driver/driver.py +++ b/nixos/lib/test-driver/test_driver/driver.py @@ -1,6 +1,8 @@ import os import re +import signal import tempfile +import threading from contextlib import contextmanager from pathlib import Path from typing import Any, Callable, ContextManager, Dict, Iterator, List, Optional, Union @@ -41,6 +43,8 @@ class Driver: vlans: List[VLan] machines: List[Machine] polling_conditions: List[PollingCondition] + global_timeout: int + race_timer: threading.Timer def __init__( self, @@ -49,9 +53,12 @@ class Driver: tests: str, out_dir: Path, keep_vm_state: bool = False, + global_timeout: int = 24 * 60 * 60 * 7, ): self.tests = tests self.out_dir = out_dir + self.global_timeout = global_timeout + self.race_timer = threading.Timer(global_timeout, self.terminate_test) tmp_dir = get_tmp_dir() @@ -82,6 +89,7 @@ class Driver: def __exit__(self, *_: Any) -> None: with rootlog.nested("cleanup"): + self.race_timer.cancel() for machine in self.machines: machine.release() @@ -144,6 +152,10 @@ class Driver: def run_tests(self) -> None: """Run the test script (for non-interactive test runs)""" + rootlog.info( + f"Test will time out and terminate in {self.global_timeout} seconds" + ) + self.race_timer.start() self.test_script() # TODO: Collect coverage data for machine in self.machines: @@ -161,6 +173,19 @@ class Driver: with rootlog.nested("wait for all VMs to finish"): for machine in self.machines: machine.wait_for_shutdown() + self.race_timer.cancel() + + def terminate_test(self) -> None: + # This will be usually running in another thread than + # the thread actually executing the test script. + with rootlog.nested("timeout reached; test terminating..."): + for machine in self.machines: + machine.release() + # As we cannot `sys.exit` from another thread + # We can at least force the main thread to get SIGTERM'ed. + # This will prevent any user who caught all the exceptions + # to swallow them and prevent itself from terminating. + os.kill(os.getpid(), signal.SIGTERM) def create_machine(self, args: Dict[str, Any]) -> Machine: tmp_dir = get_tmp_dir() diff --git a/nixos/lib/testing-python.nix b/nixos/lib/testing-python.nix index 4904ad6e3591..f5222351518b 100644 --- a/nixos/lib/testing-python.nix +++ b/nixos/lib/testing-python.nix @@ -42,6 +42,7 @@ rec { , nodes ? {} , testScript , enableOCR ? false + , globalTimeout ? (60 * 60) , name ? "unnamed" , skipTypeCheck ? false # Skip linting (mainly intended for faster dev cycles) diff --git a/nixos/lib/testing/driver.nix b/nixos/lib/testing/driver.nix index cc97ca72083f..b6f01c38191d 100644 --- a/nixos/lib/testing/driver.nix +++ b/nixos/lib/testing/driver.nix @@ -94,6 +94,7 @@ let wrapProgram $out/bin/nixos-test-driver \ --set startScripts "''${vmStartScripts[*]}" \ --set testScript "$out/test-script" \ + --set globalTimeout "${toString config.globalTimeout}" \ --set vlans '${toString vlans}' \ ${lib.escapeShellArgs (lib.concatMap (arg: ["--add-flags" arg]) config.extraDriverArgs)} ''; @@ -123,6 +124,18 @@ in defaultText = "hostPkgs.qemu_test"; }; + globalTimeout = mkOption { + description = mdDoc '' + A global timeout for the complete test, expressed in seconds. + Beyond that timeout, every resource will be killed and released and the test will fail. + + By default, we use a 1 hour timeout. + ''; + type = types.int; + default = 60 * 60; + example = 10 * 60; + }; + enableOCR = mkOption { description = mdDoc '' Whether to enable Optical Character Recognition functionality for From a0dc17bd572018e9a4c6f77e30583f5fc292c5d5 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Wed, 25 Oct 2023 13:44:37 +0200 Subject: [PATCH 2/4] nixos/lib/testing/run: expose `rawTestDerivation` For `testBuildFailure` and similar functions, we need a full blown derivation and not a lazy one. This is an internal option for test framework developers. --- nixos/lib/testing/run.nix | 46 ++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/nixos/lib/testing/run.nix b/nixos/lib/testing/run.nix index 0cd07d8afd21..9440c1acdfd8 100644 --- a/nixos/lib/testing/run.nix +++ b/nixos/lib/testing/run.nix @@ -16,6 +16,15 @@ in ''; }; + rawTestDerivation = mkOption { + type = types.package; + description = mdDoc '' + Unfiltered version of `test`, for troubleshooting the test framework and `testBuildFailure` in the test framework's test suite. + This is not intended for general use. Use `test` instead. + ''; + internal = true; + }; + test = mkOption { type = types.package; # TODO: can the interactive driver be configured to access the network? @@ -29,25 +38,26 @@ in }; config = { + rawTestDerivation = hostPkgs.stdenv.mkDerivation { + name = "vm-test-run-${config.name}"; + + requiredSystemFeatures = [ "kvm" "nixos-test" ]; + + buildCommand = '' + mkdir -p $out + + # effectively mute the XMLLogger + export LOGFILE=/dev/null + + ${config.driver}/bin/nixos-test-driver -o $out + ''; + + passthru = config.passthru; + + meta = config.meta; + }; test = lib.lazyDerivation { # lazyDerivation improves performance when only passthru items and/or meta are used. - derivation = hostPkgs.stdenv.mkDerivation { - name = "vm-test-run-${config.name}"; - - requiredSystemFeatures = [ "kvm" "nixos-test" ]; - - buildCommand = '' - mkdir -p $out - - # effectively mute the XMLLogger - export LOGFILE=/dev/null - - ${config.driver}/bin/nixos-test-driver -o $out - ''; - - passthru = config.passthru; - - meta = config.meta; - }; + derivation = config.rawTestDerivation; inherit (config) passthru meta; }; From c90219633c40eaeb9ca69c7a8cea75123a13d596 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Fri, 27 Oct 2023 13:32:04 +0200 Subject: [PATCH 3/4] nixos/tests/nixos-test-driver: test timeout failures We test that the test framework timeouts are working as expected. --- nixos/tests/all-tests.nix | 8 ++++++++ nixos/tests/nixos-test-driver/timeout.nix | 15 +++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 nixos/tests/nixos-test-driver/timeout.nix diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix index 1c8ee32428ea..2b50c09872ea 100644 --- a/nixos/tests/all-tests.nix +++ b/nixos/tests/all-tests.nix @@ -90,6 +90,14 @@ in { lib-extend = handleTestOn [ "x86_64-linux" "aarch64-linux" ] ./nixos-test-driver/lib-extend.nix {}; node-name = runTest ./nixos-test-driver/node-name.nix; busybox = runTest ./nixos-test-driver/busybox.nix; + driver-timeout = pkgs.runCommand "ensure-timeout-induced-failure" { + failed = pkgs.testers.testBuildFailure ((runTest ./nixos-test-driver/timeout.nix).config.rawTestDerivation); + } '' + grep -F "timeout reached; test terminating" $failed/testBuildFailure.log + # The program will always be terminated by SIGTERM (143) if it waits for the deadline thread. + [[ 143 = $(cat $failed/testBuildFailure.exit) ]] + touch $out + ''; }; # NixOS vm tests and non-vm unit tests diff --git a/nixos/tests/nixos-test-driver/timeout.nix b/nixos/tests/nixos-test-driver/timeout.nix new file mode 100644 index 000000000000..29bd85d2498e --- /dev/null +++ b/nixos/tests/nixos-test-driver/timeout.nix @@ -0,0 +1,15 @@ +{ + name = "Test that sleep of 6 seconds fails a timeout of 5 seconds"; + globalTimeout = 5; + + nodes = { + machine = ({ pkgs, ... }: { + }); + }; + + testScript = '' + start_all() + machine.wait_for_unit("multi-user.target") + machine.succeed("sleep 6") + ''; +} From 05dd78cf4b22b4c15d2ddcf5958c8e508616b86e Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Fri, 27 Oct 2023 13:34:03 +0200 Subject: [PATCH 4/4] nixos/lib/test-driver: add driver-timeout as a passthru test From now on, we will aim to ensure that the test driver gets tested by OfBorg using all our available tests. This commit adds the driver timeout test to the driver. --- nixos/lib/test-driver/default.nix | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/nixos/lib/test-driver/default.nix b/nixos/lib/test-driver/default.nix index 6e01e00b4355..09d80deb8546 100644 --- a/nixos/lib/test-driver/default.nix +++ b/nixos/lib/test-driver/default.nix @@ -11,6 +11,7 @@ , tesseract4 , vde2 , extraPythonPackages ? (_ : []) +, nixosTests }: python3Packages.buildPythonApplication { @@ -31,6 +32,10 @@ python3Packages.buildPythonApplication { ++ (lib.optionals enableOCR [ imagemagick_light tesseract4 ]) ++ extraPythonPackages python3Packages; + passthru.tests = { + inherit (nixosTests.nixos-test-driver) driver-timeout; + }; + doCheck = true; nativeCheckInputs = with python3Packages; [ mypy ruff black ]; checkPhase = ''