From 4cba5c1a023f9889289908e18bce56b9a1a26426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 25 Jul 2023 14:37:05 +0200 Subject: [PATCH] Simplify use of RNDC in Python-based tests The "addzone" and "shutdown" system tests currently invoke rndc using test-specific helper code. Rework the relevant bits of those tests so that they use the helper classes from bin/tests/system/isctest.py. (cherry picked from commit 00003e497ca34d541587ce6f8b7e0dac3115eb07) --- .gitlab-ci.yml | 2 + .../system/addzone/tests_rndc_deadlock.py | 35 ++++--------- bin/tests/system/shutdown/tests_shutdown.py | 51 +++++++++++-------- 3 files changed, 42 insertions(+), 46 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 93185f871d..b6a05bd07f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -592,6 +592,8 @@ coccinelle: pylint: <<: *precheck_job needs: [] + variables: + PYTHONPATH: "${CI_PROJECT_DIR}/bin/tests/system" script: - pylint --rcfile $CI_PROJECT_DIR/.pylintrc $(git ls-files '*.py' | grep -vE '(ans\.py|dangerfile\.py|^bin/tests/system/)') # Ignore Pylint wrong-import-position error in system test to enable use of pytest.importorskip diff --git a/bin/tests/system/addzone/tests_rndc_deadlock.py b/bin/tests/system/addzone/tests_rndc_deadlock.py index fefcc2dc8b..78fa11a095 100755 --- a/bin/tests/system/addzone/tests_rndc_deadlock.py +++ b/bin/tests/system/addzone/tests_rndc_deadlock.py @@ -10,25 +10,12 @@ # information regarding copyright ownership. import concurrent.futures -import os -import subprocess import time - -def run_rndc(server, rndc_command): - """ - Send the specified 'rndc_command' to 'server' with a timeout of 10 seconds - """ - rndc = os.getenv("RNDC") - port = os.getenv("CONTROLPORT") - - cmdline = [rndc, "-c", "../_common/rndc.conf", "-p", port, "-s", server] - cmdline.extend(rndc_command) - - subprocess.check_output(cmdline, stderr=subprocess.STDOUT, timeout=10) +import isctest -def rndc_loop(test_state, domain): +def rndc_loop(test_state, domain, ns3): """ Run "rndc addzone", "rndc modzone", and "rndc delzone" in a tight loop until the test is considered finished, ignoring errors @@ -45,35 +32,33 @@ def rndc_loop(test_state, domain): while not test_state["finished"]: for command in rndc_commands: - try: - run_rndc("10.53.0.3", command) - except subprocess.SubprocessError: - pass + ns3.rndc(" ".join(command), ignore_errors=True, log=False) -def check_if_server_is_responsive(): +def check_if_server_is_responsive(ns3): """ Check if server status can be successfully retrieved using "rndc status" """ try: - run_rndc("10.53.0.3", ["status"]) + ns3.rndc("status", log=False) return True - except subprocess.SubprocessError: + except isctest.rndc.RNDCException: return False -def test_rndc_deadlock(): +def test_rndc_deadlock(servers): """ Test whether running "rndc addzone", "rndc modzone", and "rndc delzone" commands concurrently does not trigger a deadlock """ test_state = {"finished": False} + ns3 = servers["ns3"] # Create 4 worker threads running "rndc" commands in a loop. with concurrent.futures.ThreadPoolExecutor() as executor: for i in range(1, 5): domain = "example%d" % i - executor.submit(rndc_loop, test_state, domain) + executor.submit(rndc_loop, test_state, domain, ns3) # Run "rndc status" 10 times, with 1-second pauses between attempts. # Each "rndc status" invocation has a timeout of 10 seconds. If any of @@ -81,7 +66,7 @@ def test_rndc_deadlock(): server_is_responsive = True attempts = 10 while server_is_responsive and attempts > 0: - server_is_responsive = check_if_server_is_responsive() + server_is_responsive = check_if_server_is_responsive(ns3) attempts -= 1 time.sleep(1) diff --git a/bin/tests/system/shutdown/tests_shutdown.py b/bin/tests/system/shutdown/tests_shutdown.py index 19f853bc32..4769c1ce9e 100755 --- a/bin/tests/system/shutdown/tests_shutdown.py +++ b/bin/tests/system/shutdown/tests_shutdown.py @@ -25,8 +25,10 @@ pytest.importorskip("dns", minversion="2.0.0") import dns.exception import dns.resolver +import isctest -def do_work(named_proc, resolver, rndc_cmd, kill_method, n_workers, n_queries): + +def do_work(named_proc, resolver, instance, kill_method, n_workers, n_queries): """Creates a number of A queries to run in parallel in order simulate a slightly more realistic test scenario. @@ -48,8 +50,8 @@ def do_work(named_proc, resolver, rndc_cmd, kill_method, n_workers, n_queries): :param resolver: target resolver :type resolver: dns.resolver.Resolver - :param rndc_cmd: rndc command with default arguments - :type rndc_cmd: list of strings, e.g. ["rndc", "-p", "23750"] + :param instance: the named instance to send RNDC commands to + :type instance: isctest.instance.NamedInstance :kill_method: "rndc" or "sigterm" :type kill_method: str @@ -63,9 +65,13 @@ def do_work(named_proc, resolver, rndc_cmd, kill_method, n_workers, n_queries): # pylint: disable-msg=too-many-arguments # pylint: disable-msg=too-many-locals - # helper function, args must be a list or tuple with arguments to rndc. - def launch_rndc(args): - return subprocess.call(rndc_cmd + args, timeout=10) + # helper function, 'command' is the rndc command to run + def launch_rndc(command): + try: + instance.rndc(command, log=False) + return 0 + except isctest.rndc.RNDCException: + return -1 # We're going to execute queries in parallel by means of a thread pool. # dnspython functions block, so we need to circunvent that. @@ -99,13 +105,13 @@ def do_work(named_proc, resolver, rndc_cmd, kill_method, n_workers, n_queries): elif shutdown: # We attempt to stop named in the middle shutdown = False if kill_method == "rndc": - futures[executor.submit(launch_rndc, ["stop"])] = "stop" + futures[executor.submit(launch_rndc, "stop")] = "stop" else: futures[executor.submit(named_proc.terminate)] = "kill" else: # We attempt to send couple rndc commands while named is # being shutdown - futures[executor.submit(launch_rndc, ["status"])] = "status" + futures[executor.submit(launch_rndc, "-t 5 status")] = "status" ret_code = -1 for future in as_completed(futures): @@ -160,8 +166,11 @@ def wait_for_proc_termination(proc, max_timeout=10): # Method 1: using rndc ctop # Method 2: killing with SIGTERM # In both methods named should exit gracefully. -@pytest.mark.parametrize("kill_method", ["rndc", "sigterm"]) -def test_named_shutdown(named_port, control_port, kill_method): +@pytest.mark.parametrize( + "kill_method", + [pytest.param("rndc", marks=pytest.mark.xfail(reason="GL#4060")), "sigterm"], +) +def test_named_shutdown(ports, kill_method): # pylint: disable-msg=too-many-locals cfg_dir = os.path.join(os.getcwd(), "resolver") assert os.path.isdir(cfg_dir) @@ -172,20 +181,20 @@ def test_named_shutdown(named_port, control_port, kill_method): named = os.getenv("NAMED") assert named is not None - rndc = os.getenv("RNDC") - assert rndc is not None - - # rndc configuration resides in ../_common/rndc.conf - rndc_cfg = os.path.join("..", "_common", "rndc.conf") - assert os.path.isfile(rndc_cfg) - - # rndc command with default arguments. - rndc_cmd = [rndc, "-c", rndc_cfg, "-p", str(control_port), "-s", "10.53.0.3"] + # This test launches and monitors a named instance itself rather than using + # bin/tests/system/start.pl, so manually defining a NamedInstance here is + # necessary for sending RNDC commands to that instance. This "custom" + # instance listens on 10.53.0.3, so use "ns3" as the identifier passed to + # the NamedInstance constructor. + named_ports = isctest.instance.NamedPorts( + dns=ports["PORT"], rndc=ports["CONTROLPORT"] + ) + instance = isctest.instance.NamedInstance("ns3", named_ports) # We create a resolver instance that will be used to send queries. resolver = dns.resolver.Resolver() resolver.nameservers = ["10.53.0.3"] - resolver.port = named_port + resolver.port = named_ports.dns named_cmdline = [named, "-c", cfg_file, "-f"] with subprocess.Popen(named_cmdline, cwd=cfg_dir) as named_proc: @@ -195,7 +204,7 @@ def test_named_shutdown(named_port, control_port, kill_method): do_work( named_proc, resolver, - rndc_cmd, + instance, kill_method, n_workers=12, n_queries=16,