From 5eb4c2b0fc261061aa88761cd12f75398efa4f1f Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Wed, 20 Nov 2024 11:24:13 +0100
Subject: [PATCH] tests: fix bmp test parallelization

Multiple BMP tests can run in parallel but, when one instance ends,
it kills the BMP server process of all BMP tests.

Save the PID of a BMP server and only kill it at the end.

Link: https://github.com/FRRouting/frr/issues/17465
Fixes: 875511c466 ("topotests: add basic bmp collector")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 .../topotests/lib/bmp_collector/bmpserver.py  | 78 ++++++++++++++++++-
 tests/topotests/lib/topogen.py                |  7 +-
 2 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/tests/topotests/lib/bmp_collector/bmpserver.py b/tests/topotests/lib/bmp_collector/bmpserver.py
index 1f8c12cdd5cf..a7c9743c9604 100755
--- a/tests/topotests/lib/bmp_collector/bmpserver.py
+++ b/tests/topotests/lib/bmp_collector/bmpserver.py
@@ -7,6 +7,7 @@
 import argparse
 
 # XXX: something more reliable should be used "Twisted" a great choice.
+import os
 import signal
 import socket
 import sys
@@ -20,11 +21,11 @@
 # Global variable to track shutdown signal
 shutdown = False
 
-
 parser = argparse.ArgumentParser()
 parser.add_argument("-a", "--address", type=str, default="0.0.0.0")
 parser.add_argument("-p", "--port", type=int, default=1789)
 parser.add_argument("-l", "--logfile", type=str, default="/var/log/bmp.log")
+parser.add_argument("-r", "--pidfile", type=str, default="/var/run/bmp.pid")
 
 
 def handle_signal(signum, frame):
@@ -40,6 +41,74 @@ def timestamp_print(message, file=sys.stderr):
     print(f"[{current_time}] {message}", file=file)
 
 
+def check_pid(pid):
+    if pid < 0:  # user input error
+        return False
+    if pid == 0:  # all processes
+        return False
+    try:
+        os.kill(pid, 0)
+        return True
+    except OSError as err:
+        if err.errno == errno.EPERM:  # a process we were denied access to
+            return True
+        if err.errno == errno.ESRCH:  # No such process
+            return False
+        # should never happen
+        return False
+
+
+def savepid():
+    ownid = os.getpid()
+
+    flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY
+    mode = ((os.R_OK | os.W_OK) << 6) | (os.R_OK << 3) | os.R_OK
+
+    try:
+        fd = os.open(pid_file, flags, mode)
+    except OSError:
+        try:
+            pid = open(pid_file, "r").readline().strip()
+            if check_pid(int(pid)):
+                timestamp_print(
+                    "PID file already exists and program still running %s\n" % pid_file
+                )
+                return False
+            else:
+                # If pid is not running, reopen file without O_EXCL
+                fd = os.open(pid_file, flags ^ os.O_EXCL, mode)
+        except (OSError, IOError, ValueError):
+            timestamp_print(
+                "issue accessing PID file %s (most likely permission or ownership)\n"
+                % pid_file
+            )
+            return False
+
+    try:
+        f = os.fdopen(fd, "w")
+        line = "%d\n" % ownid
+        f.write(line)
+        f.close()
+        saved_pid = True
+    except IOError:
+        timestamp_print("Can not create PID file %s\n" % pid_file)
+        return False
+    timestamp_print("Created PID file %s with value %d\n" % (pid_file, ownid))
+    return True
+
+
+def removepid():
+    try:
+        os.remove(pid_file)
+    except OSError as exc:
+        if exc.errno == errno.ENOENT:
+            pass
+        else:
+            timestamp_print("Can not remove PID file %s\n" % pid_file)
+            return
+    timestamp_print("Removed PID file %s\n" % pid_file)
+
+
 def main():
     global shutdown
 
@@ -51,8 +120,13 @@ def main():
     ADDRESS, PORT = args.address, args.port
     LOG_FILE = args.logfile
 
+    global pid_file
+    pid_file = args.pidfile
+
     timestamp_print(f"Starting bmpserver on {args.address}:{args.port}")
 
+    savepid()
+
     with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
         s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
         try:
@@ -97,6 +171,7 @@ def main():
             timestamp_print(f"{e}")
         finally:
             timestamp_print(f"Server shutting down on {ADDRESS}:{PORT}")
+            removepid()
 
 
 if __name__ == "__main__":
@@ -104,4 +179,5 @@ def main():
         sys.exit(main())
     except KeyboardInterrupt:
         logging.info("BMP server was interrupted and is shutting down.")
+        removepid()
         sys.exit(0)
diff --git a/tests/topotests/lib/topogen.py b/tests/topotests/lib/topogen.py
index dce83d5ef5a3..0a9a84a4bb07 100644
--- a/tests/topotests/lib/topogen.py
+++ b/tests/topotests/lib/topogen.py
@@ -1293,18 +1293,19 @@ def start(self, log_file=None):
         log_err = os.path.join(log_dir, "bmpserver.log")
 
         log_arg = "-l {}".format(log_file) if log_file else ""
+        self.pid_file = os.path.join(log_dir, "bmpserver.pid")
 
         with open(log_err, "w") as err:
             self.run(
-                "{}/bmp_collector/bmpserver.py -a {} -p {} {}&".format(
-                    CWD, self.ip, self.port, log_arg
+                "{}/bmp_collector/bmpserver.py -a {} -p {} -r {} {}&".format(
+                    CWD, self.ip, self.port, self.pid_file, log_arg
                 ),
                 stdout=None,
                 stderr=err,
             )
 
     def stop(self):
-        self.run("pkill -f bmpserver.py")
+        self.run(f"kill $(cat {self.pid_file}")
         return ""