Move port env vars into isctest.vars.ports module

The fixture for port assignment isn't needed, replace it with the common
way of handling environment variables.

bin/tests/system/conftest.py

@@ -52,9 +52,6 @@ else:
 XDIST_WORKER = os.environ.get("PYTEST_XDIST_WORKER", "")
 FILE_DIR = os.path.abspath(Path(__file__).parent)
 ENV_RE = re.compile(b"([^=]+)=(.*)")
-PORT_MIN = 5001
-PORT_MAX = 32767
-PORTS_PER_TEST = 20
 PRIORITY_TESTS = [
     # Tests that are scheduled first. Speeds up parallel execution.
     "rpz/",
@@ -210,8 +207,10 @@ def module_base_ports(modules):
     exactly what happens - every worker thread will call this fixture to
     determine test ports.
     """
-    port_min = PORT_MIN
+    port_min = isctest.vars.ports.PORT_MIN
-    port_max = PORT_MAX - len(modules) * PORTS_PER_TEST
+    port_max = (
+        isctest.vars.ports.PORT_MAX - len(modules) * isctest.vars.ports.PORTS_PER_TEST
+    )
     if port_max < port_min:
         raise RuntimeError("not enough ports to assign unique port set to each module")
 
@@ -223,62 +222,44 @@ def module_base_ports(modules):
     # be misleading.
     base_port = int(time.time() // 3600) % (port_max - port_min) + port_min
 
-    return {mod: base_port + i * PORTS_PER_TEST for i, mod in enumerate(modules)}
+    return {
+        mod: base_port + i * isctest.vars.ports.PORTS_PER_TEST
+        for i, mod in enumerate(modules)
+    }
 
 
-@pytest.fixture(scope="module")
+@pytest.fixture(autouse=True, scope="module")
 def base_port(request, module_base_ports):
     """Start of the port range assigned to a particular test module."""
     port = module_base_ports[request.fspath]
+    isctest.vars.ports.set_base_port(port)
     return port
 
 
 @pytest.fixture(scope="module")
-def ports(base_port):
+def named_port():
-    """Dictionary containing port names and their assigned values."""
+    return int(os.environ["PORT"])
-    return {
-        "PORT": base_port,
-        "TLSPORT": base_port + 1,
-        "HTTPPORT": base_port + 2,
-        "HTTPSPORT": base_port + 3,
-        "EXTRAPORT1": base_port + 4,
-        "EXTRAPORT2": base_port + 5,
-        "EXTRAPORT3": base_port + 6,
-        "EXTRAPORT4": base_port + 7,
-        "EXTRAPORT5": base_port + 8,
-        "EXTRAPORT6": base_port + 9,
-        "EXTRAPORT7": base_port + 10,
-        "EXTRAPORT8": base_port + 11,
-        "CONTROLPORT": base_port + 12,
-    }
 
 
 @pytest.fixture(scope="module")
-def named_port(ports):
+def named_tlsport():
-    return ports["PORT"]
+    return int(os.environ["TLSPORT"])
 
 
 @pytest.fixture(scope="module")
-def named_tlsport(ports):
+def named_httpsport():
-    return ports["TLSPORT"]
+    return int(os.environ["HTTPSPORT"])
 
 
 @pytest.fixture(scope="module")
-def named_httpsport(ports):
+def control_port():
-    return ports["HTTPSPORT"]
+    return int(os.environ["CONTROLPORT"])
 
 
 @pytest.fixture(scope="module")
-def control_port(ports):
+def env():
-    return ports["CONTROLPORT"]
-
-
-@pytest.fixture(scope="module")
-def env(ports):
     """Dictionary containing environment variables for the test."""
     env = dict(isctest.vars.ALL)
-    for portname, portnum in ports.items():
-        env[portname] = str(portnum)
     env["builddir"] = f"{env['TOP_BUILDDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
     env["srcdir"] = f"{env['TOP_SRCDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
     os.environ.update(env)
@@ -576,7 +557,9 @@ def system_test(  # pylint: disable=too-many-arguments,too-many-statements
 
     isctest.log.info(f"test started: {request.node.name}")
     port = int(env["PORT"])
-    isctest.log.info("using port range: <%d, %d>", port, port + PORTS_PER_TEST - 1)
+    isctest.log.info(
+        "using port range: <%d, %d>", port, port + isctest.vars.ports.PORTS_PER_TEST - 1
+    )
 
     if not hasattr(request.node, "stash"):  # compatibility with pytest<7.0.0
         request.node.stash = {}  # use regular dict instead of pytest.Stash
@@ -604,17 +587,13 @@ def system_test(  # pylint: disable=too-many-arguments,too-many-statements
 
 
 @pytest.fixture
-def servers(ports, system_test_dir):
+def servers(system_test_dir):
     instances = {}
     for entry in system_test_dir.rglob("*"):
         if entry.is_dir():
             try:
                 dir_name = entry.name
-                # LATER: Make ports fixture return NamedPorts directly
+                instance = isctest.instance.NamedInstance(dir_name)
-                named_ports = isctest.instance.NamedPorts(
-                    dns=int(ports["PORT"]), rndc=int(ports["CONTROLPORT"])
-                )
-                instance = isctest.instance.NamedInstance(dir_name, named_ports)
                 instances[dir_name] = instance
             except ValueError:
                 continue

bin/tests/system/isctest/instance.py

@@ -25,6 +25,13 @@ class NamedPorts(NamedTuple):
     dns: int = 53
     rndc: int = 953
 
+    @staticmethod
+    def from_env():
+        return NamedPorts(
+            dns=int(os.environ["PORT"]),
+            rndc=int(os.environ["CONTROLPORT"]),
+        )
+
 
 class NamedInstance:
     """
@@ -42,7 +49,7 @@ class NamedInstance:
     def __init__(
         self,
         identifier: str,
-        ports: NamedPorts = NamedPorts(),
+        ports: Optional[NamedPorts] = None,
         rndc_logger: Optional[logging.Logger] = None,
         rndc_executor: Optional[RNDCExecutor] = None,
     ) -> None:
@@ -52,7 +59,8 @@ class NamedInstance:
 
         `ports` is the `NamedPorts` instance listing the UDP/TCP ports on which
         this `named` instance is listening for various types of traffic (both
-        DNS traffic and RNDC commands).
+        DNS traffic and RNDC commands). Defaults to ports set by the test
+        framework.
 
         `rndc_logger` is the `logging.Logger` to use for logging RNDC
         commands sent to this `named` instance.
@@ -61,6 +69,8 @@ class NamedInstance:
         that is used for executing RNDC commands on this `named` instance.
         """
         self.ip = self._identifier_to_ip(identifier)
+        if ports is None:
+            ports = NamedPorts.from_env()
         self.ports = ports
         self.log = LogFile(os.path.join(identifier, "named.run"))
         self._rndc_executor = rndc_executor or RNDCBinaryExecutor()

bin/tests/system/isctest/vars/all.py

@@ -17,6 +17,7 @@ from .autoconf import AC_VARS  # type: ignore
 # pylint: enable=import-error
 from .basic import BASIC_VARS
 from .openssl import OPENSSL_VARS
+from .ports import PORT_VARS
 
 
 class VarLookup(ChainMap):
@@ -50,4 +51,4 @@ class VarLookup(ChainMap):
         return iter(self.keys())
 
 
-ALL = VarLookup(AC_VARS, BASIC_VARS, OPENSSL_VARS)
+ALL = VarLookup(AC_VARS, BASIC_VARS, OPENSSL_VARS, PORT_VARS)

bin/tests/system/isctest/vars/ports.py

@@ -0,0 +1,54 @@
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# SPDX-License-Identifier: MPL-2.0
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0.  If a copy of the MPL was not distributed with this
+# file, you can obtain one at https://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+import os
+
+from .. import log
+
+PORT_MIN = 5001
+PORT_MAX = 32767
+PORTS_PER_TEST = 20
+
+PORT_VARS = {
+    "PORT": "5300",
+    "TLSPORT": "5301",
+    "HTTPPORT": "5302",
+    "HTTPSPORT": "5303",
+    "EXTRAPORT1": "5304",
+    "EXTRAPORT2": "5305",
+    "EXTRAPORT3": "5306",
+    "EXTRAPORT4": "5307",
+    "EXTRAPORT5": "5308",
+    "EXTRAPORT6": "5309",
+    "EXTRAPORT7": "5310",
+    "EXTRAPORT8": "5311",
+    "CONTROLPORT": "5312",
+}
+
+
+def set_base_port(base_port: int):
+    log.debug(f"setting base port {base_port}")
+    assert base_port >= PORT_MIN
+    assert base_port <= PORT_MAX
+    PORT_VARS["PORT"] = str(base_port)
+    PORT_VARS["TLSPORT"] = str(base_port + 1)
+    PORT_VARS["HTTPPORT"] = str(base_port + 2)
+    PORT_VARS["HTTPSPORT"] = str(base_port + 3)
+    PORT_VARS["EXTRAPORT1"] = str(base_port + 4)
+    PORT_VARS["EXTRAPORT2"] = str(base_port + 5)
+    PORT_VARS["EXTRAPORT3"] = str(base_port + 6)
+    PORT_VARS["EXTRAPORT4"] = str(base_port + 7)
+    PORT_VARS["EXTRAPORT5"] = str(base_port + 8)
+    PORT_VARS["EXTRAPORT6"] = str(base_port + 9)
+    PORT_VARS["EXTRAPORT7"] = str(base_port + 10)
+    PORT_VARS["EXTRAPORT8"] = str(base_port + 11)
+    PORT_VARS["CONTROLPORT"] = str(base_port + 12)
+    os.environ.update(PORT_VARS)

bin/tests/system/shutdown/tests_shutdown.py

@@ -170,7 +170,7 @@ def wait_for_proc_termination(proc, max_timeout=10):
     "kill_method",
     ["rndc", "sigterm"],
 )
-def test_named_shutdown(ports, kill_method):
+def test_named_shutdown(kill_method):
     # pylint: disable-msg=too-many-locals
     cfg_dir = os.path.join(os.getcwd(), "resolver")
     assert os.path.isdir(cfg_dir)
@@ -186,9 +186,7 @@ def test_named_shutdown(ports, kill_method):
     # 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(
+    named_ports = isctest.instance.NamedPorts.from_env()
-        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.

bin/tests/system/statschannel/conftest.py

@@ -9,9 +9,11 @@
 # See the COPYRIGHT file distributed with this work for additional
 # information regarding copyright ownership.
 
+import os
+
 import pytest
 
 
 @pytest.fixture(scope="module")
-def statsport(ports):
+def statsport():
-    return ports["EXTRAPORT1"]
+    return int(os.environ["EXTRAPORT1"])