From 4af78378a9d646853e78c5c9ce7aa6141d12d3fa Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 10 Jul 2026 11:08:40 +0800 Subject: [PATCH] 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 --- qa/tasks/mgr/dashboard/helper.py | 7 +++- qa/tasks/mgr/mgr_test_case.py | 60 +++++++++++++++++++++++--------- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/qa/tasks/mgr/dashboard/helper.py b/qa/tasks/mgr/dashboard/helper.py index a628ae65ef9..eb6cf459d13 100644 --- 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 4a5506391f2..c528e9661ad 100644 --- 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)