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 00003e497c)

.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

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)
 

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,