qa/tasks/mgr: bounce the mgrs once per test class

The dashboard backend API suite occasionally fails in CI on the job's
7200 second timeout alone, with every test passing. Suite runtime
varies with builder load: identical runs of the same 311 tests took
anywhere from about 5400 to over 7000 seconds, and teardown needs
roughly another 180 seconds on top. On the runs that landed at the
high end, only 60 to 80 seconds of margin were left before the
timeout fired. Cutting setup overhead gives the suite that margin
back.

DashboardTestCase.setUpClass() restarted every mgr twice: once in
MgrTestCase.setup_mgrs(), and again right after in _assign_ports(),
whose only job is to set the per-mgr dashboard port, which has to
happen while the mgrs are down.

Let subclasses declare their module ports in a MODULE_PORTS class
attribute instead. setup_mgrs() assigns them while the daemons are
already stopped, so the single restart it performs picks them up and
the second stop/fail/restart cycle disappears.

The assignment loop moves to _assign_module_ports(). _assign_ports()
keeps its old stop/assign/restart contract for callers that reassign
ports outside class setup (test_prometheus, test_dashboard,
test_module_selftest) and delegates to the shared helper.

With 66 dashboard test classes per API suite run this removes one
full mgr bounce per class, roughly 10 to 15 minutes of wall clock in
the CI runs examined.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>

diff --git a/qa/tasks/mgr/dashboard/helper.py b/qa/tasks/mgr/dashboard/helper.py
index a628ae65ef978b8f853236e0ae6e297cb4268285..eb6cf459d136ed44d03644f77898d6ee5fd31482 100644 (file)
--- a/qa/tasks/mgr/dashboard/helper.py
+++ b/qa/tasks/mgr/dashboard/helper.py
@@ -36,6 +36,9 @@ class DashboardTestCase(MgrTestCase):
     REQUIRE_FILESYSTEM = True
     CLIENTS_REQUIRED = 1
     CEPHFS = False
+    # assigned by setup_mgrs() while the mgrs are down, so setUpClass
+    # bounces the mgrs once instead of twice
+    MODULE_PORTS = [("dashboard", "ssl_server_port", 7789)]
     ORCHESTRATOR = False
     ORCHESTRATOR_TEST_DATA = {
         'inventory': [
@@ -212,8 +215,10 @@ class DashboardTestCase(MgrTestCase):
 
     @classmethod
     def setUpClass(cls):
+        # setup_mgrs() assigns the dashboard port (MODULE_PORTS) while
+        # the mgrs are down, so no second _assign_ports() bounce is
+        # needed here
         super(DashboardTestCase, cls).setUpClass()
-        cls._assign_ports("dashboard", "ssl_server_port")
         cls._load_module("dashboard")
         cls.update_base_uri()
 

diff --git a/qa/tasks/mgr/mgr_test_case.py b/qa/tasks/mgr/mgr_test_case.py
index 4a5506391f2acedc35fd1a6fb7b6263fa33dbe8f..c528e9661ad8d4f56392b209a7547be35deb56ff 100644 (file)
--- a/qa/tasks/mgr/mgr_test_case.py
+++ b/qa/tasks/mgr/mgr_test_case.py
@@ -2,6 +2,7 @@ import json
 import logging
 import socket
 
+from typing import List, Tuple
 from unittest import SkipTest
 
 from teuthology import misc
@@ -95,8 +96,20 @@ MgrCluster = MgrClusterBase
 class MgrTestCase(CephTestCase):
     MGRS_REQUIRED = 1
 
+    # (module_name, config_name, min_port) tuples whose per-mgr port
+    # options are assigned inside setup_mgrs(), while the daemons are
+    # stopped anyway. A subclass that needs a module port should prefer
+    # this over calling _assign_ports() after setup_mgrs(): the latter
+    # stops, fails and restarts every mgr a second time.
+    MODULE_PORTS: List[Tuple[str, str, int]] = []
+
     @classmethod
     def setup_mgrs(cls):
+        # for the port-in-use probes below; the map keeps the address
+        # usable even with no active mgr
+        mgr_map = cls.mgr_cluster.get_mgr_map()
+        ip_addr = mgr_map.get('active_addr', '').split(':')[0]
+
         # Stop all the daemons
         for daemon in cls.mgr_cluster.mgr_daemons.values():
             daemon.stop()
@@ -115,6 +128,12 @@ class MgrTestCase(CephTestCase):
             cls.mgr_cluster.mon_manager.raw_cluster_cmd(
                 "mgr", "module", "disable", m)
 
+        # Assign module ports while the daemons are down, so the single
+        # restart below picks them up
+        for module_name, config_name, min_port in cls.MODULE_PORTS:
+            cls._assign_module_ports(ip_addr, module_name, config_name,
+                                     min_port)
+
         # Start all the daemons
         for daemon in cls.mgr_cluster.mgr_daemons.values():
             daemon.restart()
@@ -219,29 +238,20 @@ class MgrTestCase(CephTestCase):
         return uri
 
     @classmethod
-    def _assign_ports(cls, module_name, config_name, min_port=7789):
+    def _assign_module_ports(cls, ip_addr, module_name, config_name,
+                             min_port=7789):
         """
-        To avoid the need to run lots of hosts in teuthology tests to
-        get different URLs per mgr, we will hand out different ports
-        to each mgr here.
-
-        This is already taken care of for us when running in a vstart
-        environment.
+        Hand out a distinct port per mgr for the given module option.
+        Call with the mgr daemons stopped, so the probes below don't
+        mistake a previous instance of the module for a foreign user
+        of the port.
         """
         # Start handing out ports well above Ceph's range.
         assign_port = min_port
-        ip_addr = cls.mgr_cluster.get_mgr_map()['active_addr'].split(':')[0]
-
-        for mgr_id in cls.mgr_cluster.mgr_ids:
-            cls.mgr_cluster.mgr_stop(mgr_id)
-            cls.mgr_cluster.mgr_fail(mgr_id)
-
 
         for mgr_id in cls.mgr_cluster.mgr_ids:
             # Find a port that isn't in use
-            while True:
-                if not cls.is_port_in_use(ip_addr, assign_port):
-                    break
+            while ip_addr and cls.is_port_in_use(ip_addr, assign_port):
                 log.debug(f"Port {assign_port} in use, trying next")
                 assign_port += 1
 
@@ -252,6 +262,24 @@ class MgrTestCase(CephTestCase):
                                                       force=True)
             assign_port += 1
 
+    @classmethod
+    def _assign_ports(cls, module_name, config_name, min_port=7789):
+        """
+        To avoid the need to run lots of hosts in teuthology tests to
+        get different URLs per mgr, we will hand out different ports
+        to each mgr here.
+
+        This is already taken care of for us when running in a vstart
+        environment.
+        """
+        ip_addr = cls.mgr_cluster.get_mgr_map()['active_addr'].split(':')[0]
+
+        for mgr_id in cls.mgr_cluster.mgr_ids:
+            cls.mgr_cluster.mgr_stop(mgr_id)
+            cls.mgr_cluster.mgr_fail(mgr_id)
+
+        cls._assign_module_ports(ip_addr, module_name, config_name, min_port)
+
         for mgr_id in cls.mgr_cluster.mgr_ids:
             cls.mgr_cluster.mgr_restart(mgr_id)