pytests: reserve kresd ports using files in tmpdir

diff --git a/tests/pytests/README.rst b/tests/pytests/README.rst
index cf682375abc225ba98f5ba37bf7a076a05457122..36f847e29b526986aba920badeef7ccf3451af71 100644 (file)
--- a/tests/pytests/README.rst
+++ b/tests/pytests/README.rst
@@ -28,9 +28,7 @@ wait for kresd timeout. This can be done with `python-xdist`:
    $ pytest-3 -n 24  # parallel with 24 jobs
 
 Each test spawns an independent kresd instance, so test failures shouldn't affect
-each other. However, when using lots of parallel jobs, it is possible an already taken
-port will be assigned to kresd. These cases will be detected and result in skipped
-tests.
+each other.
 
 Some tests are omitted from automatic test collection by default, due to their
 resource contraints. These typicially have to be executed separately by providing

diff --git a/tests/pytests/conftest.py b/tests/pytests/conftest.py
index c1e309707a4f5b2a0fc426b0ecf699ea6758a44a..a9d36b75de12e316ccf7bd1d1833553a42e29066 100644 (file)
--- a/tests/pytests/conftest.py
+++ b/tests/pytests/conftest.py
@@ -2,7 +2,7 @@ import socket
 
 import pytest
 
-from kresd import make_kresd
+from kresd import init_portdir, make_kresd
 
 
 @pytest.fixture
@@ -78,3 +78,7 @@ def pytest_metadata(metadata):  # filter potentially sensitive data from GitLab
             keys_to_delete.append(key)
     for key in keys_to_delete:
         del metadata[key]
+
+
+def pytest_sessionstart(session):  # pylint: disable=unused-argument
+    init_portdir()

diff --git a/tests/pytests/conn_flood.py b/tests/pytests/conn_flood.py
index 1e4da0b5b002bf1bc21f0746b79cd2940ff972bb..5b7a76d2dc574bcff3dd90f701ff671e1ece400e 100644 (file)
--- a/tests/pytests/conn_flood.py
+++ b/tests/pytests/conn_flood.py
@@ -13,7 +13,7 @@ import time
 
 import pytest
 
-from kresd import Kresd, make_port
+from kresd import Kresd
 import utils
 
 
@@ -54,9 +54,7 @@ def test_conn_flood(tmpdir, sock_func_name):
     # create kresd instance with verbose=False
     ip = '127.0.0.1'
     ip6 = '::1'
-    port = make_port(ip, ip6)
-    tls_port = make_port(ip, ip6)
-    with Kresd(tmpdir, port, tls_port, ip, ip6, verbose=False) as kresd:
+    with Kresd(tmpdir, ip=ip, ip6=ip6, verbose=False) as kresd:
         print("\nEstablishing {} connections".format(nsockets))
         make_sock = getattr(kresd, sock_func_name)  # function for creating sockets
         sockets = create_sockets(make_sock, nsockets)

diff --git a/tests/pytests/kresd.py b/tests/pytests/kresd.py
index a184cb0f51e92714388106aefb7e374e4d286344..2e770ba55547f16fa9190a7139727bfab0a8f6ce 100644 (file)
--- a/tests/pytests/kresd.py
+++ b/tests/pytests/kresd.py
@@ -1,14 +1,15 @@
 from collections import namedtuple
 from contextlib import ContextDecorator, contextmanager
 import os
+from pathlib import Path
 import random
 import re
+import shutil
 import socket
 import subprocess
 import time
 
 import jinja2
-import pytest
 
 import utils
 
@@ -18,6 +19,17 @@ CERTS_DIR = os.path.join(PYTESTS_DIR, 'certs')
 TEMPLATES_DIR = os.path.join(PYTESTS_DIR, 'templates')
 KRESD_CONF_TEMPLATE = 'kresd.conf.j2'
 KRESD_STARTUP_MSGID = 10005  # special unique ID at the start of the "test" log
+KRESD_PORTDIR = '/tmp/pytest-kresd-portdir'
+KRESD_TESTPORT_MIN = 10000
+KRESD_TESTPORT_MAX = 49000
+
+
+def init_portdir():
+    try:
+        shutil.rmtree(KRESD_PORTDIR)
+    except FileNotFoundError:
+        pass
+    os.makedirs(KRESD_PORTDIR)
 
 
 def create_file_from_template(template_path, dest, data):
@@ -35,7 +47,7 @@ Forward = namedtuple('Forward', ['proto', 'ip', 'port', 'hostname', 'ca_file'])
 
 class Kresd(ContextDecorator):
     def __init__(
-            self, workdir, port, tls_port, ip=None, ip6=None, certname=None,
+            self, workdir, port=None, tls_port=None, ip=None, ip6=None, certname=None,
             verbose=True, hints=None, forward=None):
         if ip is None and ip6 is None:
             raise ValueError("IPv4 or IPv6 must be specified!")
@@ -67,6 +79,15 @@ class Kresd(ContextDecorator):
         return str(os.path.join(self.workdir, 'kresd.log'))
 
     def __enter__(self):
+        if self.port is not None:
+            take_port(self.port, self.ip, self.ip6)
+        else:
+            self.port = make_port(self.ip, self.ip6)
+        if self.tls_port is not None:
+            take_port(self.tls_port, self.ip, self.ip6)
+        else:
+            self.tls_port = make_port(self.ip, self.ip6)
+
         create_file_from_template(KRESD_CONF_TEMPLATE, self.config_path, {'kresd': self})
         self.logfile = open(self.logfile_path, 'w')
         self.process = subprocess.Popen(
@@ -89,14 +110,6 @@ class Kresd(ContextDecorator):
                     self.process.returncode))
         except (RuntimeError, ConnectionError):  # pylint: disable=try-except-raise
             raise
-        finally:
-            # handle cases where we accidentally attempt to bind to same port
-            # as another test that runs in parallel
-            self.logfile.flush()
-            with open(self.logfile_path) as f:
-                for line in f:
-                    if re.search('Address already in use', line) is not None:
-                        pytest.skip(line)  # mark as skipped instead of failed/error
 
         return self
 
@@ -109,6 +122,7 @@ class Kresd(ContextDecorator):
                 sock.close()
             self.process.terminate()
             self.logfile.close()
+            Path(KRESD_PORTDIR, str(self.port)).unlink()
 
     def all_ports_alive(self, msgid=10001):
         alive = True
@@ -228,11 +242,33 @@ def is_port_free(port, ip=None, ip6=None):
     return True
 
 
+def take_port(port, ip=None, ip6=None):
+    port_path = Path(KRESD_PORTDIR, str(port))
+    try:
+        port_path.touch(exist_ok=False)
+    except FileExistsError:
+        raise ValueError(
+            "Port {} already reserved by system or another kresd instance!".format(port))
+
+    if not is_port_free(port, ip, ip6):
+        # NOTE: The port_path isn't removed, so other instances don't have to attempt to
+        # take the same port again. This has the side effect of leaving many of these
+        # files behind, because when another kresd shuts down and removes its file, the
+        # port still can't be reserved for a while. This shouldn't become an issue unless
+        # we have thousands of tests (and run out of the port range).
+        raise ValueError(
+            "Port {} is reserved by system!".format(port))
+    return port
+
+
 def make_port(ip=None, ip6=None):
     for _ in range(10):  # max attempts
-        port = random.randint(1024, 65535)
-        if is_port_free(port, ip, ip6):
-            return port
+        port = random.randint(KRESD_TESTPORT_MIN, KRESD_TESTPORT_MAX)
+        try:
+            take_port(port, ip, ip6)
+        except ValueError:
+            continue  # port reserved by system / another kresd instance
+        return port
     raise RuntimeError("No available port found!")
 
 
@@ -244,8 +280,6 @@ KRESD_LOG_IO_CLOSE = re.compile(r'^\[io\].*closed by peer.*')
 def make_kresd(
         workdir, certname=None, ip='127.0.0.1', ip6='::1', forward=None, hints=None,
         port=None, tls_port=None):
-    port = make_port(ip, ip6) if port is None else port
-    tls_port = make_port(ip, ip6) if tls_port is None else tls_port
     with Kresd(workdir, port, tls_port, ip, ip6, certname, forward=forward, hints=hints) as kresd:
         yield kresd
         print(kresd.partial_log())