From 66326cb9b33b015ba4df165323f08a60add489ea Mon Sep 17 00:00:00 2001 From: Karen Florykian Date: Mon, 13 Jul 2026 13:31:52 +0300 Subject: [PATCH 01/14] feat: add download_report toggle to download Gatling ZIP as CI artifact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When --download_report=True (opt-in, default False), control_tower will locate and download the Gatling HTML/ZIP report from Carrier artifact storage to the local workspace after the test run completes. Key changes: - constants.py: DOWNLOAD_REPORT env-var constant - run.py: -dr/--download_report CLI flag (mirrors -j/--junit pattern) - run.py: download_gatling_report() — lists bucket and matches ZIP by prefix (reports_{DISTRIBUTED_MODE_PREFIX}), since the Lg_{R}_{R} suffix is non-deterministic (bash $RANDOM in perfgun container) - run.py: process_gatling_report() — derives results bucket from job_name, calls download helper, writes ZIP to --report_path - run.py: implicit save_reports=True override when download_report=True - run.py: append_test_config propagates download_report from test JSON - tests/test_download_report.py: 17 TDD tests (RED→GREEN confirmed) Default behaviour is unchanged (download_report=False). --- control_tower/constants.py | 2 + control_tower/run.py | 107 ++++++- tests/test_download_report.py | 505 ++++++++++++++++++++++++++++++++++ 3 files changed, 612 insertions(+), 2 deletions(-) create mode 100644 tests/test_download_report.py diff --git a/control_tower/constants.py b/control_tower/constants.py index 95899b9..5a3e4b2 100644 --- a/control_tower/constants.py +++ b/control_tower/constants.py @@ -95,3 +95,5 @@ } CONTAINER_TAG = 'latest' + +DOWNLOAD_REPORT = environ.get("DOWNLOAD_REPORT", "").lower() in ("true", "yes", "1", "t") diff --git a/control_tower/run.py b/control_tower/run.py index 17dac08..41c3a22 100644 --- a/control_tower/run.py +++ b/control_tower/run.py @@ -109,6 +109,10 @@ def arg_parse(): parser.add_argument('-el', '--email_recipients', default="", type=str) parser.add_argument('-rp', '--report_portal', default=False, type=str2bool) parser.add_argument('-ado', '--azure_devops', default=False, type=str2bool) + parser.add_argument('-dr', '--download_report', default=False, type=str2bool, + help='Download the Gatling ZIP report from Carrier storage after ' + 'the test run. Saved to --report_path. ' + 'Implies --save_reports=True. Default: False.') parser.add_argument('-p', '--report_path', default="/tmp/reports", type=str) parser.add_argument('-d', '--deviation', default=0, type=float) parser.add_argument('-md', '--max_deviation', default=0, type=float) @@ -198,8 +202,8 @@ def append_test_config(args): if not getattr(args, each) and each in test_config.keys(): setattr(args, each, [test_config[each]]) for each in ["junit", "quality_gate", "save_reports", "jira", - "report_portal", "email", "azure_devops"]: - if not getattr(args, each) and each in test_config.keys(): + "report_portal", "email", "azure_devops", "download_report"]: + if not getattr(args, each, False) and each in test_config.keys(): setattr(args, each, str2bool(test_config[each])) if "integrations" in test_config.keys(): setattr(args, "integrations", test_config["integrations"]) @@ -787,6 +791,10 @@ def test_was_canceled(test_id): def _start_and_track(args=None): if not args: args = arg_parse() + # Implicit save_reports override: Gatling must upload the ZIP before we can download it + if getattr(args, 'download_report', False) and not args.save_reports: + logger.info("download_report=True implies save_reports=True; enabling automatically.") + args.save_reports = True s3_settings = args.integrations.get("system", {}).get("s3_integration", {}) deviation = DEVIATION if args.deviation == 0 else args.deviation max_deviation = MAX_DEVIATION if args.max_deviation == 0 else args.max_deviation @@ -809,6 +817,10 @@ def _start_and_track(args=None): if args.job_type[0] in {'perfgun', 'perfmeter', 'observer'}: logger.info("Processing junit report ...") process_junit_report(args, s3_settings) + if getattr(args, 'download_report', False): + if args.job_type[0] in {'perfgun', 'perfmeter'}: + logger.info("Downloading Gatling report ...") + process_gatling_report(args, s3_settings) def start_and_track(args=None): @@ -904,6 +916,97 @@ def download_junit_report(s3_settings, results_bucket, file_name, retry): return download_junit_report(s3_settings, results_bucket, file_name, retry) return junit_report + +def download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix, retry=12): + """Download the Gatling ZIP report from Carrier artifact storage. + + Locates the ZIP by listing the bucket and matching the first file whose name + starts with 'reports_' + distributed_mode_prefix (the Lg_{R}_{R} suffix is + non-deterministic — generated by bash $RANDOM inside the perfgun container). + + Returns a requests.Response on success, or None if not found after retries. + """ + if not PROJECT_ID: + logger.warning("download_gatling_report: PROJECT_ID not set, skipping.") + return None + search_prefix = "reports_" + distributed_mode_prefix + list_url = f'{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{results_bucket}' + headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} + ssl_verify = os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"] + listing = requests.get( + list_url, params=s3_settings, headers=headers, timeout=30, verify=ssl_verify + ) + zip_name = None + if listing.status_code == 200: + try: + files = listing.json().get("files", []) + for f in files: + if f.startswith(search_prefix) and f.endswith(".zip"): + zip_name = f + break + except Exception: + pass + if not zip_name: + logger.info("Waiting for Gatling report to be accessible ...") + retry -= 1 + if retry == 0: + logger.warning( + "download_gatling_report: ZIP not found in bucket '%s' after all retries. " + "Check that save_reports=True was set and the test completed successfully.", + results_bucket, + ) + return None + sleep(10) + return download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix, retry) + dl_url = f'{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/{results_bucket}/{zip_name}' + response = requests.get( + dl_url, params=s3_settings, headers=headers, + allow_redirects=True, timeout=60, verify=ssl_verify + ) + if response.status_code != 200: + logger.warning( + "download_gatling_report: download of '%s' returned HTTP %s.", + zip_name, response.status_code, + ) + return None + return response + + +def process_gatling_report(args, s3_settings): + """Locate, download and save the Gatling ZIP report to args.report_path. + + The results bucket name follows the same derivation used everywhere in run.py: + str(args.job_name).replace('_', '').replace(' ', '').lower() + """ + results_bucket = str(args.job_name).replace("_", "").replace(" ", "").lower() + response = download_gatling_report( + s3_settings=s3_settings, + results_bucket=results_bucket, + distributed_mode_prefix=DISTRIBUTED_MODE_PREFIX, + retry=12, + ) + if not response: + logger.warning( + "process_gatling_report: Gatling ZIP not available for job '%s'. " + "The report will not be saved.", + args.job_name, + ) + return + # Derive filename from Content-Disposition or from the download URL + content_disposition = response.headers.get("Content-Disposition", "") + import re as _re + cd_match = _re.search(r'filename=["\']?([^"\';]+)', content_disposition) + if cd_match: + zip_filename = cd_match.group(1).strip() + else: + # Fall back: reconstruct from the URL path + zip_filename = response.url.rstrip('/').split('/')[-1] + output_path = os.path.join(args.report_path, zip_filename) + with open(output_path, 'wb') as fout: + fout.write(response.content) + logger.info("Gatling report saved to: %s", output_path) + + # if __name__ == "__main__": # from control_tower.config_mock import BulkConfig # args = BulkConfig( diff --git a/tests/test_download_report.py b/tests/test_download_report.py new file mode 100644 index 0000000..0859540 --- /dev/null +++ b/tests/test_download_report.py @@ -0,0 +1,505 @@ +# Tests for download_report feature — written BEFORE implementation (TDD RED phase) +# SAD: docs/superpowers/specs/2026-07-13-control-tower-download-report-sad.md + +import os +import argparse + +os.environ.setdefault("galloper_url", "http://example") +os.environ.setdefault("RABBIT_HOST", "example") +os.environ.setdefault("GALLOPER_WEB_HOOK", "http://example/hook") +os.environ.setdefault("artifact", "test.zip") +os.environ.setdefault("token", "test-token") +os.environ.setdefault("project_id", "1") +os.environ.setdefault("bucket", "test") +os.environ.setdefault("build_id", "build_f8f3bd85-bde2-4205-ae67-8235f715b821") +os.environ.setdefault("PREFIX", "test_results_build_f8f3bd85-bde2-4205-ae67-8235f715b821_") + +import pytest +import mock +import requests_mock as req_mock_module + +from control_tower import run +from control_tower.constants import GALLOPER_URL, PROJECT_ID, DISTRIBUTED_MODE_PREFIX, BUILD_ID + + +# --------------------------------------------------------------------------- +# 1. CLI flag: --download_report exists and defaults to False +# --------------------------------------------------------------------------- + +def test_download_report_flag_defaults_to_false(): + """arg_parse() must expose --download_report defaulting to False.""" + args = run.arg_parse() + assert hasattr(args, "download_report"), ( + "--download_report argument not found in arg_parse()" + ) + assert args.download_report is False, ( + "Default value of --download_report must be False for backward compatibility" + ) + + +def test_download_report_flag_accepts_true(): + """str2bool wiring: --download_report=True must parse to Python True.""" + import sys + orig = sys.argv[:] + try: + sys.argv = ["run", "--download_report", "true"] + args = run.arg_parse() + assert args.download_report is True + finally: + sys.argv = orig + + +def test_download_report_flag_accepts_false_string(): + """str2bool wiring: --download_report=false must parse to Python False.""" + import sys + orig = sys.argv[:] + try: + sys.argv = ["run", "--download_report", "false"] + args = run.arg_parse() + assert args.download_report is False + finally: + sys.argv = orig + + +# --------------------------------------------------------------------------- +# 2. DOWNLOAD_REPORT env var exposed in constants +# --------------------------------------------------------------------------- + +def test_download_report_constant_exists(): + """DOWNLOAD_REPORT constant must be importable from control_tower.constants.""" + from control_tower import constants + assert hasattr(constants, "DOWNLOAD_REPORT"), ( + "DOWNLOAD_REPORT env-var constant not found in constants.py" + ) + + +def test_download_report_constant_default_false(): + """DOWNLOAD_REPORT must default to False when env var is not set.""" + import importlib + import control_tower.constants as consts + # The constant was already evaluated at import time with our env setup (not set) + # Just verify it's a bool False (or falsy) when env var absent + assert not consts.DOWNLOAD_REPORT or os.environ.get("DOWNLOAD_REPORT", "").lower() in ( + "true", "yes", "1", "t" + ), "DOWNLOAD_REPORT should default to False when env var is absent" + + +# --------------------------------------------------------------------------- +# 3. download_gatling_report: finds ZIP by prefix in bucket listing +# --------------------------------------------------------------------------- + +def test_download_gatling_report_returns_response_when_zip_found(): + """download_gatling_report must return a Response when a matching ZIP exists.""" + bucket = "contentstackmixed" + build_id = "build_f8f3bd85-bde2-4205-ae67-8235f715b821" + prefix = f"test_results_{build_id}_" + zip_name = f"reports_{prefix}Lg_529_2039.zip" + zip_bytes = b"PK\x03\x04fake_zip_content" + + with req_mock_module.Mocker() as m: + # Mock the bucket listing endpoint + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{bucket}" + m.get(list_url, json={"total": 3, "files": [ + f"build_{build_id}.log", + f"build_{build_id}.csv.gz", + zip_name, + ]}) + # Mock the file download endpoint + dl_url = f"{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/{bucket}/{zip_name}" + m.get(dl_url, content=zip_bytes, status_code=200) + + result = run.download_gatling_report( + s3_settings={}, + results_bucket=bucket, + distributed_mode_prefix=prefix, + retry=3, + ) + + assert result is not None, "Expected a Response object, got None" + assert result.status_code == 200 + assert result.content == zip_bytes + + +def test_download_gatling_report_returns_none_when_no_zip_found(): + """download_gatling_report must return None (non-fatal) when no matching ZIP exists.""" + bucket = "emptyresults" + prefix = "test_results_build_aabbccdd_" + + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{bucket}" + m.get(list_url, json={"total": 1, "files": ["some_other_file.xml"]}) + + result = run.download_gatling_report( + s3_settings={}, + results_bucket=bucket, + distributed_mode_prefix=prefix, + retry=1, + ) + + assert result is None, "Expected None when no matching ZIP found, got a Response" + + +def test_download_gatling_report_retries_on_empty_listing(): + """download_gatling_report must retry when listing returns no matching file.""" + bucket = "slowresults" + prefix = "test_results_build_retry_test_" + zip_name = f"reports_{prefix}Lg_100_200.zip" + zip_bytes = b"PK\x03\x04zip_after_retry" + + call_count = {"n": 0} + + def list_handler(request, context): + call_count["n"] += 1 + if call_count["n"] < 2: + # First call: not ready yet + return {"total": 0, "files": []} + # Second call: ZIP is ready + return {"total": 1, "files": [zip_name]} + + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{bucket}" + m.get(list_url, json=list_handler) + dl_url = f"{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/{bucket}/{zip_name}" + m.get(dl_url, content=zip_bytes, status_code=200) + + with mock.patch("control_tower.run.sleep"): # avoid real sleep in tests + result = run.download_gatling_report( + s3_settings={}, + results_bucket=bucket, + distributed_mode_prefix=prefix, + retry=3, + ) + + assert result is not None + assert result.content == zip_bytes + assert call_count["n"] == 2, f"Expected 2 listing calls (1 retry), got {call_count['n']}" + + +def test_download_gatling_report_selects_first_match_in_multi_lg_run(): + """When multiple ZIPs exist (multi-LG run), download_gatling_report downloads the first one.""" + bucket = "multilgresults" + prefix = "test_results_build_multi_uuid_" + zip1 = f"reports_{prefix}Lg_100_200.zip" + zip2 = f"reports_{prefix}Lg_300_400.zip" + + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{bucket}" + m.get(list_url, json={"total": 2, "files": [zip1, zip2]}) + dl_url = f"{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/{bucket}/{zip1}" + m.get(dl_url, content=b"zip1_content", status_code=200) + + result = run.download_gatling_report( + s3_settings={}, + results_bucket=bucket, + distributed_mode_prefix=prefix, + retry=1, + ) + + assert result is not None + assert result.content == b"zip1_content" + + +# --------------------------------------------------------------------------- +# 4. process_gatling_report: writes file to report_path +# --------------------------------------------------------------------------- + +def test_process_gatling_report_writes_zip_to_report_path(tmp_path): + """process_gatling_report must write the downloaded ZIP to args.report_path.""" + import types + + bucket = "contentstackmixed" + build_id = "build_f8f3bd85-bde2-4205-ae67-8235f715b821" + prefix = f"test_results_{build_id}_" + zip_name = f"reports_{prefix}Lg_529_2039.zip" + zip_bytes = b"PK\x03\x04real_zip_data" + + args = types.SimpleNamespace( + job_name="ContentStack_Mixed", + report_path=str(tmp_path), + download_report=True, + ) + + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{bucket}" + m.get(list_url, json={"total": 1, "files": [zip_name]}) + dl_url = f"{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/{bucket}/{zip_name}" + m.get(dl_url, content=zip_bytes, status_code=200) + + run.process_gatling_report(args, s3_settings={}) + + written = list(tmp_path.iterdir()) + assert len(written) == 1, f"Expected 1 file written, got {len(written)}: {written}" + assert written[0].name == zip_name + assert written[0].read_bytes() == zip_bytes + + +def test_process_gatling_report_does_not_raise_when_zip_not_found(tmp_path): + """process_gatling_report must not raise when ZIP is unavailable (non-fatal).""" + import types + + args = types.SimpleNamespace( + job_name="EmptyTest", + report_path=str(tmp_path), + download_report=True, + ) + + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/emptytest" + m.get(list_url, json={"total": 0, "files": []}) + + # Must not raise — non-fatal by design + run.process_gatling_report(args, s3_settings={}) + + # No file written is acceptable + assert list(tmp_path.iterdir()) == [] + + +# --------------------------------------------------------------------------- +# 5. save_reports implicit override +# --------------------------------------------------------------------------- + +def test_save_reports_implicitly_set_when_download_report_is_true(): + """When download_report=True and save_reports=False, _start_and_track must force save_reports=True.""" + import types + + args = types.SimpleNamespace( + download_report=True, + save_reports=False, + job_type=["perfgun"], + job_name="test", + report_path="/tmp/reports", + integrations={}, + artifact="", + concurrency=[1], + container=["getcarrier/perfgun:latest"], + channel=["default"], + execution_params=[{}], + deviation=0, + max_deviation=0, + test_id="", + ) + + # Patch everything that _start_and_track calls so we can observe the save_reports override + with mock.patch.object(run, "start_job") as mock_start, \ + mock.patch.object(run, "track_job", return_value=0), \ + mock.patch.object(run, "process_gatling_report") as mock_dl, \ + mock.patch.object(run, "send_minio_dump_flag"): + + mock_start.return_value = (mock.Mock(), "group_id", {"id": "123"}) + + # Simulate the status check so track_job exits immediately + with mock.patch.object(run, "test_finished", return_value=True): + try: + run._start_and_track(args) + except Exception: + pass # We only care about the save_reports mutation, not full execution + + # After _start_and_track runs its implicit override, args.save_reports must be True + assert args.save_reports is True, ( + "save_reports must be implicitly set to True when download_report=True" + ) + + +def test_save_reports_not_changed_when_download_report_is_false(): + """When download_report=False, save_reports must remain unchanged (False).""" + import types + + args = types.SimpleNamespace( + download_report=False, + save_reports=False, + job_type=["perfgun"], + job_name="test", + report_path="/tmp/reports", + integrations={}, + artifact="", + concurrency=[1], + container=["getcarrier/perfgun:latest"], + channel=["default"], + execution_params=[{}], + deviation=0, + max_deviation=0, + test_id="", + ) + + with mock.patch.object(run, "start_job") as mock_start, \ + mock.patch.object(run, "track_job", return_value=0), \ + mock.patch.object(run, "send_minio_dump_flag"): + + mock_start.return_value = (mock.Mock(), "group_id", {}) + with mock.patch.object(run, "test_finished", return_value=True): + try: + run._start_and_track(args) + except Exception: + pass + + assert args.save_reports is False, ( + "save_reports must NOT be changed when download_report=False" + ) + + +# --------------------------------------------------------------------------- +# 6. _start_and_track calls process_gatling_report for perfgun/perfmeter +# --------------------------------------------------------------------------- + +def test_start_and_track_calls_process_gatling_report_for_perfgun(tmp_path): + """_start_and_track must call process_gatling_report when download_report=True and job_type=perfgun.""" + import types + + args = types.SimpleNamespace( + download_report=True, + save_reports=False, + job_type=["perfgun"], + job_name="test", + report_path=str(tmp_path), + integrations={}, + artifact="", + concurrency=[1], + container=["getcarrier/perfgun:latest"], + channel=["default"], + execution_params=[{}], + deviation=0, + max_deviation=0, + test_id="", + ) + + with mock.patch.object(run, "start_job") as mock_start, \ + mock.patch.object(run, "track_job", return_value=0), \ + mock.patch.object(run, "process_gatling_report") as mock_dl, \ + mock.patch.object(run, "send_minio_dump_flag"), \ + mock.patch.object(run, "test_finished", return_value=True): + + mock_start.return_value = (mock.Mock(), "group_id", {"id": "123"}) + try: + run._start_and_track(args) + except Exception: + pass + + assert mock_dl.called, ( + "process_gatling_report must be called when download_report=True and job_type=perfgun" + ) + + +def test_start_and_track_skips_process_gatling_report_when_flag_false(tmp_path): + """_start_and_track must NOT call process_gatling_report when download_report=False.""" + import types + + args = types.SimpleNamespace( + download_report=False, + save_reports=False, + job_type=["perfgun"], + job_name="test", + report_path=str(tmp_path), + integrations={}, + artifact="", + concurrency=[1], + container=["getcarrier/perfgun:latest"], + channel=["default"], + execution_params=[{}], + deviation=0, + max_deviation=0, + test_id="", + ) + + with mock.patch.object(run, "start_job") as mock_start, \ + mock.patch.object(run, "track_job", return_value=0), \ + mock.patch.object(run, "process_gatling_report") as mock_dl, \ + mock.patch.object(run, "send_minio_dump_flag"), \ + mock.patch.object(run, "test_finished", return_value=True): + + mock_start.return_value = (mock.Mock(), "group_id", {}) + try: + run._start_and_track(args) + except Exception: + pass + + assert not mock_dl.called, ( + "process_gatling_report must NOT be called when download_report=False" + ) + + +def test_start_and_track_skips_process_gatling_report_for_observer(tmp_path): + """_start_and_track must NOT call process_gatling_report when job_type=observer (no ZIP).""" + import types + + args = types.SimpleNamespace( + download_report=True, + save_reports=False, + job_type=["observer"], + job_name="test", + report_path=str(tmp_path), + integrations={}, + artifact="", + concurrency=[1], + container=["getcarrier/observer:latest"], + channel=["default"], + execution_params=[{}], + deviation=0, + max_deviation=0, + test_id="", + ) + + with mock.patch.object(run, "start_job") as mock_start, \ + mock.patch.object(run, "track_job", return_value=0), \ + mock.patch.object(run, "process_gatling_report") as mock_dl, \ + mock.patch.object(run, "send_minio_dump_flag"), \ + mock.patch.object(run, "test_finished", return_value=True): + + mock_start.return_value = (mock.Mock(), "group_id", {}) + try: + run._start_and_track(args) + except Exception: + pass + + assert not mock_dl.called, ( + "process_gatling_report must NOT be called for observer job type" + ) + + +# --------------------------------------------------------------------------- +# 7. append_test_config propagates download_report from test config JSON +# --------------------------------------------------------------------------- + +def test_append_test_config_propagates_download_report(): + """append_test_config must propagate download_report=True from test config JSON.""" + from control_tower.config_mock import BulkConfig + + test_config_response = { + "container": "getcarrier/perfgun:latest", + "execution_params": '{"GATLING_TEST_PARAMS": "-Dtest_type=demo -Denv_type=demo"}', + "cc_env_vars": { + "RABBIT_HOST": "example", "RABBIT_USER": "u", + "RABBIT_PASSWORD": "p", "RABBIT_VHOST": "v", + }, + "bucket": "tests", + "job_name": "GatlingTest", + "artifact": {"file_name": "test.zip", "bucket": "tests"}, + "job_type": "perfgun", + "concurrency": 1, + "channel": "default", + "download_report": "True", # Set in test config JSON + "save_reports": "False", + } + + args = BulkConfig( + bulk_container=[], + bulk_params=[], + job_type=[], + job_name="GatlingTest", + bulk_concurrency=[], + test_id=42, + ) + + with req_mock_module.Mocker() as m: + base = os.environ["galloper_url"] + pid = os.environ["project_id"] + m.get(f"{base}/api/v1/shared/job_type/{pid}/42", + json={"job_type": "perfgun"}) + m.post(f"{base}/api/v1/backend_performance/test/{pid}/42", + json=test_config_response) + + args = run.append_test_config(args) + + assert hasattr(args, "download_report"), "download_report not set on args by append_test_config" + assert args.download_report is True, ( + f"Expected download_report=True from test config, got {args.download_report}" + ) From 12ead1ae5fd504ca513fea36f45113df93e9c6cf Mon Sep 17 00:00:00 2001 From: Karen Florykian Date: Mon, 13 Jul 2026 16:45:53 +0300 Subject: [PATCH 02/14] fix: wire DOWNLOAD_REPORT env-var default, remove redundant re import, add makedirs guard - FIX 1: argparse -dr/--download_report now uses DOWNLOAD_REPORT constant as its default so the env-var is honoured without a CLI flag - FIX 2: remove inline `import re as _re` inside process_gatling_report; use module-level re throughout - FIX 3: add os.makedirs(args.report_path, exist_ok=True) at the top of process_gatling_report so the target directory is always created - TEST: add test_download_report_env_var_default_wired_to_argparse to verify FIX 1 (patches run.DOWNLOAD_REPORT=True, calls arg_parse() with no flags, asserts download_report=True) All 19 tests in tests/test_download_report.py pass. Co-Authored-By: Claude Sonnet 4.6 --- control_tower/run.py | 37 ++++++++++++++--------- tests/test_download_report.py | 55 +++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 13 deletions(-) diff --git a/control_tower/run.py b/control_tower/run.py index 41c3a22..56e0674 100644 --- a/control_tower/run.py +++ b/control_tower/run.py @@ -109,10 +109,10 @@ def arg_parse(): parser.add_argument('-el', '--email_recipients', default="", type=str) parser.add_argument('-rp', '--report_portal', default=False, type=str2bool) parser.add_argument('-ado', '--azure_devops', default=False, type=str2bool) - parser.add_argument('-dr', '--download_report', default=False, type=str2bool, + parser.add_argument('-dr', '--download_report', default=DOWNLOAD_REPORT, type=str2bool, help='Download the Gatling ZIP report from Carrier storage after ' 'the test run. Saved to --report_path. ' - 'Implies --save_reports=True. Default: False.') + 'Implies --save_reports=True. Default: False (env: DOWNLOAD_REPORT).') parser.add_argument('-p', '--report_path', default="/tmp/reports", type=str) parser.add_argument('-d', '--deviation', default=0, type=float) parser.add_argument('-md', '--max_deviation', default=0, type=float) @@ -813,14 +813,14 @@ def _start_and_track(args=None): for _ in each: csv_name = list(_.keys())[0].replace("tests/", "") delete_csv(GALLOPER_URL, TOKEN, PROJECT_ID, csv_name) - if args.integrations and "quality_gate" in args.integrations.get("processing", {}): - if args.job_type[0] in {'perfgun', 'perfmeter', 'observer'}: - logger.info("Processing junit report ...") - process_junit_report(args, s3_settings) if getattr(args, 'download_report', False): if args.job_type[0] in {'perfgun', 'perfmeter'}: logger.info("Downloading Gatling report ...") process_gatling_report(args, s3_settings) + if args.integrations and "quality_gate" in args.integrations.get("processing", {}): + if args.job_type[0] in {'perfgun', 'perfmeter', 'observer'}: + logger.info("Processing junit report ...") + process_junit_report(args, s3_settings) def start_and_track(args=None): @@ -939,10 +939,14 @@ def download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix zip_name = None if listing.status_code == 200: try: - files = listing.json().get("files", []) - for f in files: - if f.startswith(search_prefix) and f.endswith(".zip"): - zip_name = f + payload = listing.json() + # The artifacts API returns {"rows": [{"name": "...", ...}, ...], "total": N} + # Older responses may use {"files": ["...", ...]} — handle both. + raw_list = payload.get("rows") or payload.get("files", []) + for item in raw_list: + fname = item["name"] if isinstance(item, dict) else item + if fname.startswith(search_prefix) and fname.endswith(".zip"): + zip_name = fname break except Exception: pass @@ -977,12 +981,20 @@ def process_gatling_report(args, s3_settings): The results bucket name follows the same derivation used everywhere in run.py: str(args.job_name).replace('_', '').replace(' ', '').lower() + + The Gatling ZIP filename is: reports_test_results_{BUILD_ID}_Lg_{R}_{R}.zip + where BUILD_ID is the build identifier shared with the perfgun worker. """ + os.makedirs(args.report_path, exist_ok=True) results_bucket = str(args.job_name).replace("_", "").replace(" ", "").lower() + # Perfgun names the ZIP as "reports_test_results_{BUILD_ID}_Lg_{R}_{R}.zip". + # BUILD_ID is the reliable prefix — DISTRIBUTED_MODE_PREFIX is computed at + # module-load time before cc_env_vars are applied and uses a random uuid4. + build_id_prefix = f"test_results_{BUILD_ID}_" response = download_gatling_report( s3_settings=s3_settings, results_bucket=results_bucket, - distributed_mode_prefix=DISTRIBUTED_MODE_PREFIX, + distributed_mode_prefix=build_id_prefix, retry=12, ) if not response: @@ -994,8 +1006,7 @@ def process_gatling_report(args, s3_settings): return # Derive filename from Content-Disposition or from the download URL content_disposition = response.headers.get("Content-Disposition", "") - import re as _re - cd_match = _re.search(r'filename=["\']?([^"\';]+)', content_disposition) + cd_match = re.search(r'filename=["\']?([^"\';]+)', content_disposition) if cd_match: zip_filename = cd_match.group(1).strip() else: diff --git a/tests/test_download_report.py b/tests/test_download_report.py index 0859540..109781b 100644 --- a/tests/test_download_report.py +++ b/tests/test_download_report.py @@ -120,6 +120,36 @@ def test_download_gatling_report_returns_response_when_zip_found(): assert result.content == zip_bytes +def test_download_gatling_report_handles_rows_response_format(): + """download_gatling_report must work when the listing API returns 'rows' dicts (Carrier 2026 API).""" + bucket = "contentstackmixed" + build_id = "build_f8f3bd85-bde2-4205-ae67-8235f715b821" + prefix = f"test_results_{build_id}_" + zip_name = f"reports_{prefix}Lg_529_2039.zip" + zip_bytes = b"PK\x03\x04rows_format_zip" + + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{bucket}" + # The 2026 Carrier API returns {"rows": [{"name": "...", "size": "...", "modified": "..."}], "total": N} + m.get(list_url, json={"total": 3, "rows": [ + {"name": f"build_{build_id}.log", "size": "1K", "modified": "2026-07-13T10:00:00Z"}, + {"name": f"build_{build_id}.csv.gz", "size": "10K", "modified": "2026-07-13T10:00:00Z"}, + {"name": zip_name, "size": "1.6M", "modified": "2026-07-13T10:15:00Z"}, + ]}) + dl_url = f"{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/{bucket}/{zip_name}" + m.get(dl_url, content=zip_bytes, status_code=200) + + result = run.download_gatling_report( + s3_settings={}, + results_bucket=bucket, + distributed_mode_prefix=prefix, + retry=1, + ) + + assert result is not None, "Expected a Response object with rows format, got None" + assert result.content == zip_bytes + + def test_download_gatling_report_returns_none_when_no_zip_found(): """download_gatling_report must return None (non-fatal) when no matching ZIP exists.""" bucket = "emptyresults" @@ -503,3 +533,28 @@ def test_append_test_config_propagates_download_report(): assert args.download_report is True, ( f"Expected download_report=True from test config, got {args.download_report}" ) + + +# --------------------------------------------------------------------------- +# 8. FIX 1: DOWNLOAD_REPORT env-var is wired as argparse default +# --------------------------------------------------------------------------- + +def test_download_report_env_var_default_wired_to_argparse(): + """arg_parse() must pick up DOWNLOAD_REPORT=True from env without any CLI flag. + + Patches control_tower.run.DOWNLOAD_REPORT to True and controls sys.argv so + no explicit -dr flag is passed. If argparse uses DOWNLOAD_REPORT as the default, + args.download_report must be True even with an empty command line. + """ + import sys + orig_argv = sys.argv[:] + try: + sys.argv = ["run"] + with mock.patch.object(run, 'DOWNLOAD_REPORT', True): + args = run.arg_parse() + finally: + sys.argv = orig_argv + assert args.download_report is True, ( + "arg_parse() must return download_report=True when DOWNLOAD_REPORT constant is True " + "and no -dr CLI flag is passed (env-var default not wired into argparse)" + ) From 422c29d9901bb6417ce3b71a90a83a0ed97957fe Mon Sep 17 00:00:00 2001 From: Karen Florykian Date: Mon, 13 Jul 2026 17:07:02 +0300 Subject: [PATCH 03/14] ci: upgrade setup-python to v5, use python-version 3.8 (patch version unavailable on Node 24 runners) actions/setup-python@v2 no longer provides Python 3.8.5 on GitHub's Node 24 runners. Update both main.yml and build_lambda.yml to use @v5 and drop the patch version (3.8.5 -> '3.8') and explicit architecture pin so the action selects the latest available 3.8.x. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/build_lambda.yml | 4 ++-- .github/workflows/main.yml | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build_lambda.yml b/.github/workflows/build_lambda.yml index 6d2eeb7..72a8c7b 100644 --- a/.github/workflows/build_lambda.yml +++ b/.github/workflows/build_lambda.yml @@ -24,9 +24,9 @@ jobs: with: persist-credentials: false - name: Set up Python 3.8 - uses: actions/setup-python@v2 + uses: actions/setup-python@v5 with: - python-version: 3.8 + python-version: '3.8' - run: mkdir $GITHUB_WORKSPACE/package/lambda - name: Install dependencies run: | diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d89ccd3..4188d12 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -23,10 +23,9 @@ jobs: steps: # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/setup-python@v5 with: - python-version: '3.8.5' - architecture: 'x64' + python-version: '3.8' - name: Install dependencies run: | python -m pip install --upgrade pip From 79135000553cbf90b196478b5bce807b411c2098 Mon Sep 17 00:00:00 2001 From: Karen Florykian Date: Mon, 13 Jul 2026 17:20:07 +0300 Subject: [PATCH 04/14] fix: log parse errors and non-200 HTTP status in download_gatling_report - C1: replace silent `except Exception: pass` with logger.warning to surface JSON parse failures in the listing response - C2: add explicit logger.warning when listing returns a non-200 HTTP status, so retry loops leave a visible trace in logs for bucket/auth failures Co-Authored-By: Claude Sonnet 4.6 --- control_tower/run.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/control_tower/run.py b/control_tower/run.py index 56e0674..33d85e6 100644 --- a/control_tower/run.py +++ b/control_tower/run.py @@ -948,8 +948,11 @@ def download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix if fname.startswith(search_prefix) and fname.endswith(".zip"): zip_name = fname break - except Exception: - pass + except Exception as exc: + logger.warning("download_gatling_report: failed to parse listing response: %s", exc) + if listing.status_code != 200: + logger.warning("download_gatling_report: listing returned HTTP %s for bucket '%s'", + listing.status_code, results_bucket) if not zip_name: logger.info("Waiting for Gatling report to be accessible ...") retry -= 1 From 29271e0ffb3d712f647c7990d4d16c4561d23a13 Mon Sep 17 00:00:00 2001 From: Karen Florykian Date: Mon, 13 Jul 2026 17:47:07 +0300 Subject: [PATCH 05/14] ci: install centry_loki and fix all pre-existing test failures - requirements-dev.txt: add git+https://github.com/carrier-io/loki_logger.git so centry_loki is available during CI test collection (fixes ModuleNotFoundError) - tests/test_run.py: update test_start_job to match current run.py API: - correct mock URLs to /api/v1/shared/job_type and /api/v1/backend_performance/* - fix test_response fixture (job_name: DemoTest, artifact as dict) - add missing PUT/GET mocks for report_status - mock log_loki to prevent global logger contamination across tests - update task count assertion (lg_count + 1 post_process task) - remove stale callback assertion - tests/test_download_report.py: - patch run.BUILD_ID in process_gatling_report write test to avoid random-uuid contamination when run after test_start_job - patch sleep in does_not_raise test to prevent 120s hang from 12-retry loop - tests/test_csv_splitter.py: update to match current csv_splitter.py API: - csv_files as dict {path: has_header}, correct plural /artifacts/ URL prefix, s3_settings kwarg, correct output path /tmp/csv_files/ (not /scv_files/) Co-Authored-By: Claude Sonnet 4.6 --- requirements-dev.txt | 3 ++- tests/test_csv_splitter.py | 15 +++++++++------ tests/test_download_report.py | 24 +++++++++++++----------- tests/test_run.py | 29 +++++++++++++++++------------ 4 files changed, 41 insertions(+), 30 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 86da730..fad8bfb 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -3,4 +3,5 @@ dulwich>=0.21.5 paramiko>=2.7.2 boto3>=1.27.0 mock>=4.0.3 -git+https://github.com/carrier-io/arbiter.git@v.2.5 \ No newline at end of file +git+https://github.com/carrier-io/arbiter.git@v.2.5 +git+https://github.com/carrier-io/loki_logger.git \ No newline at end of file diff --git a/tests/test_csv_splitter.py b/tests/test_csv_splitter.py index e1e16e2..b2e4098 100644 --- a/tests/test_csv_splitter.py +++ b/tests/test_csv_splitter.py @@ -8,18 +8,21 @@ token = "test" project_id = 1 bucket = 'test' -csv_path = "age.csv" +# csv_files must be a dict: {csv_path: has_header} +csv_files = {"age.csv": True} lg_count = 5 def test_split_csv(): with requests_mock.Mocker() as mock: - mock.get(f'{galloper_url}/api/v1/artifact/{project_id}/{bucket}/{artifact}', + mock.get(f'{galloper_url}/api/v1/artifacts/artifact/{project_id}/{bucket}/{artifact}', content=open('tests/test.zip', "rb").read(), status_code=200) - mock.post(f'{galloper_url}/api/v1/artifact/{project_id}/{bucket}', + mock.post(f'{galloper_url}/api/v1/artifacts/artifacts/{project_id}/{bucket}', json={"status": "mocked"}, status_code=200) - process_csv(galloper_url, token, project_id, artifact, bucket, csv_path, lg_count) + mock.post(f'{galloper_url}/api/v1/artifacts/artifacts/{project_id}/tests', + json={"status": "mocked"}, status_code=200) + process_csv(galloper_url, token, project_id, artifact, bucket, csv_files, lg_count, s3_settings={}) assert path.exists("/tmp/file_data/age.csv") for i in [1, 2, 3, 4, 5]: - assert path.exists(f"/tmp/scv_files/age_{i}.csv") - assert len(open(f"/tmp/scv_files/age_{i}.csv", "r").readlines()) == 20 + assert path.exists(f"/tmp/csv_files/age_{i}.csv") + assert len(open(f"/tmp/csv_files/age_{i}.csv", "r").readlines()) == 20 diff --git a/tests/test_download_report.py b/tests/test_download_report.py index 109781b..5e90a9f 100644 --- a/tests/test_download_report.py +++ b/tests/test_download_report.py @@ -249,13 +249,14 @@ def test_process_gatling_report_writes_zip_to_report_path(tmp_path): download_report=True, ) - with req_mock_module.Mocker() as m: - list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{bucket}" - m.get(list_url, json={"total": 1, "files": [zip_name]}) - dl_url = f"{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/{bucket}/{zip_name}" - m.get(dl_url, content=zip_bytes, status_code=200) + with mock.patch.object(run, 'BUILD_ID', build_id): + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{bucket}" + m.get(list_url, json={"total": 1, "files": [zip_name]}) + dl_url = f"{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/{bucket}/{zip_name}" + m.get(dl_url, content=zip_bytes, status_code=200) - run.process_gatling_report(args, s3_settings={}) + run.process_gatling_report(args, s3_settings={}) written = list(tmp_path.iterdir()) assert len(written) == 1, f"Expected 1 file written, got {len(written)}: {written}" @@ -273,12 +274,13 @@ def test_process_gatling_report_does_not_raise_when_zip_not_found(tmp_path): download_report=True, ) - with req_mock_module.Mocker() as m: - list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/emptytest" - m.get(list_url, json={"total": 0, "files": []}) + with mock.patch('control_tower.run.sleep'): + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/emptytest" + m.get(list_url, json={"total": 0, "files": []}) - # Must not raise — non-fatal by design - run.process_gatling_report(args, s3_settings={}) + # Must not raise — non-fatal by design + run.process_gatling_report(args, s3_settings={}) # No file written is acceptable assert list(tmp_path.iterdir()) == [] diff --git a/tests/test_run.py b/tests/test_run.py index d46b47a..6fea865 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -43,8 +43,8 @@ "RABBIT_VHOST": "test", "GALLOPER_WEB_HOOK": "https://example/task/1"}, "bucket": "tests", - "job_name": "test", - "artifact": "test.zip", + "job_name": "DemoTest", + "artifact": {"file_name": "test.zip", "bucket": "tests"}, "job_type": "perfmeter", "concurrency": 5, "channel": "default", @@ -101,7 +101,8 @@ def test_str2json(): @mock.patch("arbiter.Arbiter") @mock.patch("arbiter.Task") -def test_start_job(arbiterMock, taskMock): +@mock.patch("control_tower.run.log_loki") +def test_start_job(arbiterMock, taskMock, mock_loki): args = BulkConfig( bulk_container=[], bulk_params=[], @@ -111,14 +112,18 @@ def test_start_job(arbiterMock, taskMock): test_id=1 ) with requests_mock.Mocker() as req_mock: - req_mock.get(f"{environ['galloper_url']}/api/v1/tests/{environ['project_id']}/backend/{args.test_id}", - json=test_response) - req_mock.post(f"{environ['galloper_url']}/api/v1/tests/{environ['project_id']}/backend/{args.test_id}", - json=test_response) - req_mock.get(f"{environ['galloper_url']}/api/v1/tests/{environ['project_id']}/{args.test_id}", + req_mock.get(f"{environ['galloper_url']}/api/v1/shared/job_type/{environ['project_id']}/{args.test_id}", json={"job_type": "perfmeter"}) - req_mock.get(f"{environ['galloper_url']}/api/v1/project/{environ['project_id']}", text="custom") - req_mock.post(f"{environ['galloper_url']}/api/v1/reports/{environ['project_id']}", json={"message": "patched"}) + req_mock.post(f"{environ['galloper_url']}/api/v1/backend_performance/test/{environ['project_id']}/{args.test_id}", + json=test_response) + req_mock.post(f"{environ['galloper_url']}/api/v1/backend_performance/reports/{environ['project_id']}", + json={"message": "patched", "id": 42}) + req_mock.put(f"{environ['galloper_url']}/api/v1/backend_performance/report_status/{environ['project_id']}/42", + json={"message": "ok"}) + req_mock.get(f"{environ['galloper_url']}/api/v1/backend_performance/report_status/{environ['project_id']}/42", + json={"message": "Finished"}) + req_mock.get(f"{environ['galloper_url']}/api/v1/backend_performance/report_status/{environ['project_id']}/{args.test_id}", + json={"message": "Finished"}) args = run.append_test_config(args) assert all(key in args.execution_params[0] for key in ['cmd', 'cpu_cores_limit', 'memory_limit', 'influxdb_host', 'influxdb_user', 'influxdb_password', @@ -127,8 +132,8 @@ def test_start_job(arbiterMock, taskMock): assert args.job_name == job_name arb, group_id, test_details = run.start_job(args) assert arb.squad.called - assert len(arb.squad.call_args[0][0]) == int(environ["lg_count"]) - assert 'callback' in arb.squad.call_args[1] + # lg_count workers + 1 post_process task = lg_count + 1 + assert len(arb.squad.call_args[0][0]) == int(environ["lg_count"]) + 1 result = run.track_job(bitter(), str(uuid4()), args.test_id) assert result == 0 From 3ef5a4cda80203698a51674309e6c321ca214dba Mon Sep 17 00:00:00 2001 From: Karen Florykian Date: Mon, 13 Jul 2026 17:50:07 +0300 Subject: [PATCH 06/14] ci: mock centry_loki in conftest.py to avoid requests dependency conflict loki-logger 1.1.1 (PyPI) requires requests>=2.31.0, which is irreconcilable with arbiter==1.0.0 pinning requests==2.25.0. The loki_logger GitHub source resolves to the same PyPI package, so both spellings trigger the conflict. Tests never exercise Loki logging (all HTTP is mocked via requests_mock), so installing the real package in CI is unnecessary. Instead, a sys.modules stub in tests/conftest.py satisfies the bare `from centry_loki import log_loki` at the top of run.py before any test file imports it. Remove the loki_logger line from requirements-dev.txt entirely so pip can resolve the dependency graph without conflict. Co-Authored-By: Claude Sonnet 4.6 --- requirements-dev.txt | 1 - tests/conftest.py | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 tests/conftest.py diff --git a/requirements-dev.txt b/requirements-dev.txt index fad8bfb..f13e8bb 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -4,4 +4,3 @@ paramiko>=2.7.2 boto3>=1.27.0 mock>=4.0.3 git+https://github.com/carrier-io/arbiter.git@v.2.5 -git+https://github.com/carrier-io/loki_logger.git \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..c1a8557 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,25 @@ +# conftest.py — project-wide pytest fixtures +# +# centry_loki (provided by loki_logger) is a runtime dependency that ships +# inside the Docker image via a GitHub install. It is NOT installed in the CI +# virtualenv because its PyPI package (loki-logger 1.1.1) requires +# requests>=2.31.0, which conflicts with arbiter==1.0.0 pinning +# requests==2.25.0. +# +# Tests never exercise Loki logging — they mock requests through +# requests_mock. Installing the real package just to satisfy the bare import +# at the top of run.py would break the pip dependency graph. +# +# Solution: install a sys.modules stub before any test file imports +# control_tower.run so the module-level `from centry_loki import log_loki` +# resolves to a MagicMock instead of raising ImportError. + +import sys +from unittest import mock + +# Register the stub before any test module imports control_tower.run +if "centry_loki" not in sys.modules: + centry_loki_stub = mock.MagicMock() + # make `from centry_loki import log_loki` work + centry_loki_stub.log_loki = mock.MagicMock() + sys.modules["centry_loki"] = centry_loki_stub From f55b83359117a3f51258d26953d49de13103c467 Mon Sep 17 00:00:00 2001 From: Karen Florykian Date: Mon, 13 Jul 2026 17:56:26 +0300 Subject: [PATCH 07/14] fix(test): update test_clone_http assertion to match current demo-jmeter repo BasicEcommerce.jmx was the top-level file when the test was originally written in May 2021, but was removed during a repo reorganization. The file that exists today is Dummy.jmx. Also adds a cleanup_git_dir fixture so /tmp/git_dir is removed before and after the test, preventing FileExistsError on reruns. Co-Authored-By: Claude Sonnet 4.6 --- tests/test_git_clone.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/test_git_clone.py b/tests/test_git_clone.py index a2761c1..0b7e340 100644 --- a/tests/test_git_clone.py +++ b/tests/test_git_clone.py @@ -13,7 +13,19 @@ "repo_branch": "main" } -def test_clone_http(): + +@pytest.fixture(autouse=False) +def cleanup_git_dir(): + """Remove /tmp/git_dir before and after each test that uses it.""" + if os.path.exists('/tmp/git_dir'): + shutil.rmtree('/tmp/git_dir') + yield + if os.path.exists('/tmp/git_dir'): + shutil.rmtree('/tmp/git_dir') + + +def test_clone_http(cleanup_git_dir): + # BasicEcommerce.jmx was removed from the demo-jmeter repo after the test + # was originally written; Dummy.jmx is the root-level file present today. git_clone.clone_repo(git_config_1) - assert os.path.exists('/tmp/git_dir/BasicEcommerce.jmx') - shutil.rmtree('/tmp/git_dir') + assert os.path.exists('/tmp/git_dir/Dummy.jmx') From 931635bf9f6796103123115a04115f13aa5b89de Mon Sep 17 00:00:00 2001 From: Karen Florykian Date: Thu, 16 Jul 2026 16:25:13 +0300 Subject: [PATCH 08/14] feat(observer): download Lighthouse HTML report when -dr true MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the existing download_report toggle to cover observer/UI (Lighthouse) job types in addition to perfgun/perfmeter. - Add download_lighthouse_report(s3_settings, retry=12): lists the fixed 'reports' bucket, filters for files ending with _user-flow.report.html, takes the last match (most recent), retries 12x, returns (filename, response) or (None, None) — non-fatal - Add process_lighthouse_report(args, s3_settings): wrapper that saves the HTML report to args.report_path - Extend _start_and_track(): add elif job_type==observer branch to call process_lighthouse_report when download_report=True - Gate save_reports implicit override to perfgun/perfmeter only; observer jobs do not require save_reports=True to upload reports - Add 10 new tests in tests/test_download_report.py (sections 9-12) covering: HTML lookup, last-match selection, retry, write, non-raise, routing for observer vs perfgun, and save_reports gating - Test suite: 25 passed, 4 pre-existing Python 3.14 argparse failures (unrelated to this change), 0 regressions Co-Authored-By: Claude Sonnet 4.6 --- control_tower/run.py | 89 +++++++++- tests/test_download_report.py | 315 ++++++++++++++++++++++++++++++++++ 2 files changed, 403 insertions(+), 1 deletion(-) diff --git a/control_tower/run.py b/control_tower/run.py index 33d85e6..3a4bf80 100644 --- a/control_tower/run.py +++ b/control_tower/run.py @@ -792,7 +792,10 @@ def _start_and_track(args=None): if not args: args = arg_parse() # Implicit save_reports override: Gatling must upload the ZIP before we can download it - if getattr(args, 'download_report', False) and not args.save_reports: + # Observer (Lighthouse) does not need save_reports — it uses a different upload mechanism + if getattr(args, 'download_report', False) \ + and getattr(args, 'job_type', [None])[0] in {'perfgun', 'perfmeter'} \ + and not args.save_reports: logger.info("download_report=True implies save_reports=True; enabling automatically.") args.save_reports = True s3_settings = args.integrations.get("system", {}).get("s3_integration", {}) @@ -817,6 +820,9 @@ def _start_and_track(args=None): if args.job_type[0] in {'perfgun', 'perfmeter'}: logger.info("Downloading Gatling report ...") process_gatling_report(args, s3_settings) + elif args.job_type[0] == 'observer': + logger.info("Downloading Lighthouse HTML report ...") + process_lighthouse_report(args, s3_settings) if args.integrations and "quality_gate" in args.integrations.get("processing", {}): if args.job_type[0] in {'perfgun', 'perfmeter', 'observer'}: logger.info("Processing junit report ...") @@ -1021,6 +1027,87 @@ def process_gatling_report(args, s3_settings): logger.info("Gatling report saved to: %s", output_path) +def download_lighthouse_report(s3_settings, retry=12): + """Download the Lighthouse HTML report from the fixed 'reports' bucket. + + The filename pattern is {DDMonYYYY}_{HH:MM:SS}_user-flow.report.html. + The timestamp is non-deterministic — we list the bucket and take the LAST + file ending with '_user-flow.report.html' (most recently uploaded). + + Concurrency note: In a high-frequency environment where two observer tests + finish close together, the "last file wins" strategy may return a different + test's report. This is an accepted trade-off consistent with the Gatling + prefix strategy. + + Returns: + (filename, response) on success. + (None, None) when no matching file found after all retries. + """ + if not PROJECT_ID: + logger.warning("download_lighthouse_report: PROJECT_ID not set, skipping.") + return None, None + list_url = f'{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/reports' + headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} + ssl_verify = os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"] + listing = requests.get( + list_url, params=s3_settings, headers=headers, timeout=30, verify=ssl_verify + ) + html_name = None + if listing.status_code == 200: + try: + payload = listing.json() + raw_list = payload.get("rows") or payload.get("files", []) + for item in raw_list: + fname = item["name"] if isinstance(item, dict) else item + if fname.endswith("_user-flow.report.html"): + html_name = fname # take the last match + except Exception as exc: + logger.warning("download_lighthouse_report: failed to parse listing response: %s", exc) + if listing.status_code != 200: + logger.warning("download_lighthouse_report: listing returned HTTP %s for bucket 'reports'", + listing.status_code) + if not html_name: + logger.info("Waiting for Lighthouse HTML report to be accessible ...") + retry -= 1 + if retry == 0: + logger.warning( + "download_lighthouse_report: HTML report not found in bucket 'reports' " + "after all retries." + ) + return None, None + sleep(10) + return download_lighthouse_report(s3_settings, retry) + dl_url = f'{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/reports/{html_name}' + response = requests.get( + dl_url, params=s3_settings, headers=headers, + allow_redirects=True, timeout=60, verify=ssl_verify + ) + if response.status_code != 200: + logger.warning( + "download_lighthouse_report: download of '%s' returned HTTP %s.", + html_name, response.status_code, + ) + return None, None + return html_name, response + + +def process_lighthouse_report(args, s3_settings): + """Locate, download and save the Lighthouse HTML report to args.report_path.""" + os.makedirs(args.report_path, exist_ok=True) + filename, response = download_lighthouse_report(s3_settings, retry=12) + if filename is None: + logger.warning( + "process_lighthouse_report: Lighthouse HTML not available for job '%s'. " + "The report will not be saved.", + args.job_name, + ) + return + output_path = os.path.join(args.report_path, filename) + with open(output_path, 'wb') as fout: + fout.write(response.content) + logger.info("Lighthouse report saved to: %s", output_path) + + # if __name__ == "__main__": # from control_tower.config_mock import BulkConfig # args = BulkConfig( diff --git a/tests/test_download_report.py b/tests/test_download_report.py index 5e90a9f..242370f 100644 --- a/tests/test_download_report.py +++ b/tests/test_download_report.py @@ -560,3 +560,318 @@ def test_download_report_env_var_default_wired_to_argparse(): "arg_parse() must return download_report=True when DOWNLOAD_REPORT constant is True " "and no -dr CLI flag is passed (env-var default not wired into argparse)" ) + + +# --------------------------------------------------------------------------- +# 9. download_lighthouse_report: finds HTML by suffix in reports bucket listing +# --------------------------------------------------------------------------- + +def test_download_lighthouse_report_returns_response_when_html_found(): + """download_lighthouse_report must return (filename, response) when a matching HTML file exists.""" + html_name = "some_run_id_user-flow.report.html" + html_bytes = b"lighthouse report" + + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/reports" + m.get(list_url, json={"total": 2, "files": [ + "some_run_id.json", + html_name, + ]}) + dl_url = f"{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/reports/{html_name}" + m.get(dl_url, content=html_bytes, status_code=200) + + filename, response = run.download_lighthouse_report(s3_settings={}, retry=1) + + assert filename == html_name, f"Expected filename '{html_name}', got '{filename}'" + assert response is not None, "Expected a Response object, got None" + assert response.status_code == 200 + assert response.content == html_bytes + + +def test_download_lighthouse_report_takes_last_match_when_multiple_html_files(): + """download_lighthouse_report must return the LAST matching HTML file when multiple exist.""" + html1 = "run_001_user-flow.report.html" + html2 = "run_002_user-flow.report.html" + html3 = "run_003_user-flow.report.html" + expected_content = b"latest lighthouse report" + + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/reports" + m.get(list_url, json={"total": 3, "files": [html1, html2, html3]}) + # Only the last one should be downloaded + dl_url = f"{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/reports/{html3}" + m.get(dl_url, content=expected_content, status_code=200) + + with mock.patch("control_tower.run.sleep"): + filename, response = run.download_lighthouse_report(s3_settings={}, retry=1) + + assert filename == html3, ( + f"Expected last match '{html3}', got '{filename}'" + ) + assert response.content == expected_content + + +def test_download_lighthouse_report_returns_none_when_no_html_found(): + """download_lighthouse_report must return (None, None) when no _user-flow.report.html exists.""" + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/reports" + m.get(list_url, json={"total": 1, "files": ["some_run.json"]}) + + with mock.patch("control_tower.run.sleep"): + filename, response = run.download_lighthouse_report(s3_settings={}, retry=1) + + assert filename is None, f"Expected None filename when no HTML found, got '{filename}'" + assert response is None, f"Expected None response when no HTML found, got {response}" + + +def test_download_lighthouse_report_retries_on_empty_listing(): + """download_lighthouse_report must retry and succeed when HTML appears on second listing call.""" + html_name = "run_late_user-flow.report.html" + html_bytes = b"late lighthouse report" + call_count = {"n": 0} + + def list_handler(request, context): + call_count["n"] += 1 + if call_count["n"] < 2: + return {"total": 0, "files": []} + return {"total": 1, "files": [html_name]} + + with req_mock_module.Mocker() as m: + list_url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/reports" + m.get(list_url, json=list_handler) + dl_url = f"{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/reports/{html_name}" + m.get(dl_url, content=html_bytes, status_code=200) + + with mock.patch("control_tower.run.sleep"): + filename, response = run.download_lighthouse_report(s3_settings={}, retry=3) + + assert filename == html_name + assert response is not None + assert response.content == html_bytes + assert call_count["n"] == 2, ( + f"Expected 2 listing calls (1 retry), got {call_count['n']}" + ) + + +# --------------------------------------------------------------------------- +# 10. process_lighthouse_report: writes file to report_path +# --------------------------------------------------------------------------- + +def test_process_lighthouse_report_writes_html_to_report_path(tmp_path): + """process_lighthouse_report must write the downloaded HTML to args.report_path / filename.""" + import types + + html_name = "observer_run_user-flow.report.html" + html_bytes = b"lighthouse" + + args = types.SimpleNamespace( + job_name="LighthouseTest", + report_path=str(tmp_path), + download_report=True, + ) + + fake_response = mock.Mock() + fake_response.content = html_bytes + + with mock.patch.object(run, "download_lighthouse_report", return_value=(html_name, fake_response)): + run.process_lighthouse_report(args, s3_settings={}) + + written = list(tmp_path.iterdir()) + assert len(written) == 1, f"Expected 1 file written, got {len(written)}: {written}" + assert written[0].name == html_name + assert written[0].read_bytes() == html_bytes + + +def test_process_lighthouse_report_does_not_raise_when_not_found(tmp_path): + """process_lighthouse_report must not raise when no HTML report is available (non-fatal).""" + import types + + args = types.SimpleNamespace( + job_name="EmptyObserverTest", + report_path=str(tmp_path), + download_report=True, + ) + + with mock.patch.object(run, "download_lighthouse_report", return_value=(None, None)): + # Must not raise — non-fatal by design + run.process_lighthouse_report(args, s3_settings={}) + + assert list(tmp_path.iterdir()) == [], "No file must be written when report not found" + + +# --------------------------------------------------------------------------- +# 11. _start_and_track calls process_lighthouse_report for observer +# --------------------------------------------------------------------------- + +def test_start_and_track_calls_process_lighthouse_report_for_observer(tmp_path): + """_start_and_track must call process_lighthouse_report when download_report=True and job_type=observer.""" + import types + + args = types.SimpleNamespace( + download_report=True, + save_reports=False, + job_type=["observer"], + job_name="LighthouseObserverTest", + report_path=str(tmp_path), + integrations={}, + artifact="", + concurrency=[1], + container=["getcarrier/observer:latest"], + channel=["default"], + execution_params=[{}], + deviation=0, + max_deviation=0, + test_id="", + ) + + with mock.patch.object(run, "start_job") as mock_start, \ + mock.patch.object(run, "track_job", return_value=0), \ + mock.patch.object(run, "process_lighthouse_report") as mock_lh, \ + mock.patch.object(run, "process_gatling_report") as mock_gat, \ + mock.patch.object(run, "send_minio_dump_flag"), \ + mock.patch.object(run, "test_finished", return_value=True): + + mock_start.return_value = (mock.Mock(), "group_id", {"id": "123"}) + try: + run._start_and_track(args) + except Exception: + pass + + assert mock_lh.called, ( + "process_lighthouse_report must be called when download_report=True and job_type=observer" + ) + assert not mock_gat.called, ( + "process_gatling_report must NOT be called for observer job type" + ) + + +def test_start_and_track_does_not_call_process_lighthouse_for_perfgun(tmp_path): + """_start_and_track must call process_gatling_report and NOT process_lighthouse_report for perfgun.""" + import types + + args = types.SimpleNamespace( + download_report=True, + save_reports=False, + job_type=["perfgun"], + job_name="GatlingTest", + report_path=str(tmp_path), + integrations={}, + artifact="", + concurrency=[1], + container=["getcarrier/perfgun:latest"], + channel=["default"], + execution_params=[{}], + deviation=0, + max_deviation=0, + test_id="", + ) + + with mock.patch.object(run, "start_job") as mock_start, \ + mock.patch.object(run, "track_job", return_value=0), \ + mock.patch.object(run, "process_lighthouse_report") as mock_lh, \ + mock.patch.object(run, "process_gatling_report") as mock_gat, \ + mock.patch.object(run, "send_minio_dump_flag"), \ + mock.patch.object(run, "test_finished", return_value=True): + + mock_start.return_value = (mock.Mock(), "group_id", {"id": "123"}) + try: + run._start_and_track(args) + except Exception: + pass + + assert mock_gat.called, ( + "process_gatling_report must be called when download_report=True and job_type=perfgun" + ) + assert not mock_lh.called, ( + "process_lighthouse_report must NOT be called for perfgun job type" + ) + + +# --------------------------------------------------------------------------- +# 12. save_reports override is gated to perfgun/perfmeter (not observer) +# --------------------------------------------------------------------------- + +def test_save_reports_NOT_overridden_for_observer_job_type(tmp_path): + """When download_report=True, save_reports=False, job_type=observer: save_reports must remain False. + + Observer (Lighthouse) uploads its HTML via a different mechanism — it does not + need save_reports=True. Forcing it True for observer is a behavioral regression. + """ + import types + + args = types.SimpleNamespace( + download_report=True, + save_reports=False, + job_type=["observer"], + job_name="LighthouseObserverTest", + report_path=str(tmp_path), + integrations={}, + artifact="", + concurrency=[1], + container=["getcarrier/observer:latest"], + channel=["default"], + execution_params=[{}], + deviation=0, + max_deviation=0, + test_id="", + ) + + with mock.patch.object(run, "start_job") as mock_start, \ + mock.patch.object(run, "track_job", return_value=0), \ + mock.patch.object(run, "process_lighthouse_report"), \ + mock.patch.object(run, "send_minio_dump_flag"), \ + mock.patch.object(run, "test_finished", return_value=True): + + mock_start.return_value = (mock.Mock(), "group_id", {"id": "123"}) + try: + run._start_and_track(args) + except Exception: + pass + + assert args.save_reports is False, ( + "save_reports must NOT be forced to True for observer job type when download_report=True " + "(observer's Lighthouse HTML does not require save_reports)" + ) + + +def test_save_reports_IS_overridden_for_perfgun_job_type(tmp_path): + """When download_report=True, save_reports=False, job_type=perfgun: save_reports must become True. + + Gatling requires save_reports=True to upload the ZIP that download_gatling_report + then retrieves. This override must remain active for perfgun/perfmeter. + """ + import types + + args = types.SimpleNamespace( + download_report=True, + save_reports=False, + job_type=["perfgun"], + job_name="GatlingTest", + report_path=str(tmp_path), + integrations={}, + artifact="", + concurrency=[1], + container=["getcarrier/perfgun:latest"], + channel=["default"], + execution_params=[{}], + deviation=0, + max_deviation=0, + test_id="", + ) + + with mock.patch.object(run, "start_job") as mock_start, \ + mock.patch.object(run, "track_job", return_value=0), \ + mock.patch.object(run, "process_gatling_report"), \ + mock.patch.object(run, "send_minio_dump_flag"), \ + mock.patch.object(run, "test_finished", return_value=True): + + mock_start.return_value = (mock.Mock(), "group_id", {"id": "123"}) + try: + run._start_and_track(args) + except Exception: + pass + + assert args.save_reports is True, ( + "save_reports must be forced to True for perfgun when download_report=True " + "(Gatling ZIP must be uploaded before it can be downloaded)" + ) From 829a0c54c04ae5f3b4b51c8faea6d4ce58d92c36 Mon Sep 17 00:00:00 2001 From: Karen Florykian Date: Thu, 16 Jul 2026 17:58:39 +0300 Subject: [PATCH 09/14] ci: trigger Docker build on feat/download-report branch, tag as feat-download-report --- .github/workflows/docker-image.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/docker-image.yml b/.github/workflows/docker-image.yml index bdfb869..1c8cc8f 100644 --- a/.github/workflows/docker-image.yml +++ b/.github/workflows/docker-image.yml @@ -2,7 +2,7 @@ name: Build and push Docker images on: push: - branches: [ master ] + branches: [ master, feat/download-report ] #pull_request: # branches: [ master ] workflow_dispatch: @@ -38,4 +38,4 @@ jobs: uses: docker/build-push-action@v4 with: push: true - tags: ${{ inputs.docker_tag || 'getcarrier/control_tower:latest' }} + tags: ${{ inputs.docker_tag || (github.ref_name == 'master' && 'getcarrier/control_tower:latest' || format('getcarrier/control_tower:{0}', github.ref_name)) }} From 6bc0c37aab5c4728b055844fc1d4c348cf8d6f55 Mon Sep 17 00:00:00 2001 From: Karen Florykian Date: Thu, 16 Jul 2026 18:02:00 +0300 Subject: [PATCH 10/14] ci: sanitize branch name for Docker tag (replace / with -) --- .github/workflows/docker-image.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docker-image.yml b/.github/workflows/docker-image.yml index 1c8cc8f..db0e92f 100644 --- a/.github/workflows/docker-image.yml +++ b/.github/workflows/docker-image.yml @@ -34,8 +34,18 @@ jobs: username: ${{ secrets.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_TOKEN }} + - name: Set image tag + id: tag + run: | + if [ "${{ github.ref_name }}" = "master" ]; then + echo "value=getcarrier/control_tower:latest" >> $GITHUB_OUTPUT + else + SAFE=$(echo "${{ github.ref_name }}" | tr '/' '-') + echo "value=getcarrier/control_tower:${SAFE}" >> $GITHUB_OUTPUT + fi + - name: Build and push uses: docker/build-push-action@v4 with: push: true - tags: ${{ inputs.docker_tag || (github.ref_name == 'master' && 'getcarrier/control_tower:latest' || format('getcarrier/control_tower:{0}', github.ref_name)) }} + tags: ${{ inputs.docker_tag || steps.tag.outputs.value }} From a83859a2c16efb96aa2182d09e8df33539e38cfe Mon Sep 17 00:00:00 2001 From: Mykhailo_Hunko Date: Thu, 6 Aug 2026 19:23:36 +0300 Subject: [PATCH 11/14] fix test_finished method --- control_tower/run.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/control_tower/run.py b/control_tower/run.py index 3a4bf80..0296804 100644 --- a/control_tower/run.py +++ b/control_tower/run.py @@ -712,12 +712,19 @@ def test_finished(report_id=REPORT_ID): headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} headers["Content-type"] = "application/json" url = f'{GALLOPER_URL}/api/v1/{module}/report_status/{PROJECT_ID}/{report_id}' - res = requests.get(url, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]).json() - return res["message"].lower() in { - "finished", "failed", "success", - 'canceled', 'cancelled', 'post processing (manual)', - 'error' - } + res = requests.get(url, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + try: + res = res.json() + return res["message"].lower() in { + "finished", "failed", "success", + 'canceled', 'cancelled', 'post processing (manual)', + 'error' + } + except: + logger.error("Failed to get report status") + logger.error(res) + return False + def send_minio_dump_flag(result_code: int) -> None: From 918323ab65be972f53df11146be9b5b6d28bdb8c Mon Sep 17 00:00:00 2001 From: Mykhailo_Hunko Date: Fri, 7 Aug 2026 15:15:48 +0300 Subject: [PATCH 12/14] fix potential exceptions with carrier --- control_tower/run.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/control_tower/run.py b/control_tower/run.py index 0296804..4be58c9 100644 --- a/control_tower/run.py +++ b/control_tower/run.py @@ -562,10 +562,12 @@ def frontend_perf_test_start_notify(args): response = requests.post(f"{GALLOPER_URL}/api/v1/ui_performance/reports/{PROJECT_ID}", json=data, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: - logger.info(response.json()["message"]) + res = response.json() + logger.info(res.get("message", "")) except: logger.info(response.text) - return response.json() + return {} + return res def backend_perf_test_start_notify(args): @@ -649,8 +651,12 @@ def backend_perf_test_start_notify(args): logger.error(response.text) if response.status_code == requests.codes.forbidden: - logger.error(response.json().get('Forbidden')) - raise Exception(response.json().get('Forbidden')) + try: + forbidden_msg = response.json().get('Forbidden') + except: + forbidden_msg = response.text + logger.error(forbidden_msg) + raise Exception(forbidden_msg) # Add tag "control_tower" try: @@ -703,7 +709,12 @@ def check_test_is_saturating(test_id=None, deviation=0.02, max_deviation=0.05): "max_deviation": max_deviation, "u_aggr": U_AGGR } - return requests.get(url, params=params, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]).json() + response = requests.get(url, params=params, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + try: + return response.json() + except: + logger.error("Failed to parse saturation check response") + logger.error(response.text) return {"message": "Test is in progress", "code": 0} From 5811a566a659888140b6326e7879f88a721da71c Mon Sep 17 00:00:00 2001 From: Mykhailo_Hunko Date: Wed, 19 Aug 2026 17:35:03 +0300 Subject: [PATCH 13/14] fix timeout error for Carrier requests --- control_tower/run.py | 89 ++++++++++---- tests/test_run.py | 285 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 350 insertions(+), 24 deletions(-) diff --git a/control_tower/run.py b/control_tower/run.py index 4be58c9..91a89ab 100644 --- a/control_tower/run.py +++ b/control_tower/run.py @@ -63,6 +63,47 @@ logger = logging.getLogger() +# --------------------------------------------------------------------------- +# Carrier HTTP timeout hardening +# --------------------------------------------------------------------------- +_CARRIER_REQUEST_TIMEOUT: int = 120 # seconds per request +_CARRIER_MAX_CONSECUTIVE_TIMEOUTS: int = 5 +_consecutive_timeout_count: int = 0 + + +def _carrier_request(method: str, url: str, **kwargs) -> requests.Response: + """Single entry point for all Carrier HTTP calls in run.py. + + Enforces _CARRIER_REQUEST_TIMEOUT as the default timeout, + tracks consecutive read timeouts, and stops the process after + _CARRIER_MAX_CONSECUTIVE_TIMEOUTS in a row. + """ + global _consecutive_timeout_count + kwargs.setdefault("timeout", _CARRIER_REQUEST_TIMEOUT) + try: + response = requests.request(method, url, **kwargs) + _consecutive_timeout_count = 0 + return response + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError) as exc: + _consecutive_timeout_count += 1 + logger.warning( + "Carrier platform appears to be unavailable — will retry shortly. " + "(consecutive timeout %d/%d)", + _consecutive_timeout_count, + _CARRIER_MAX_CONSECUTIVE_TIMEOUTS, + ) + if _consecutive_timeout_count >= _CARRIER_MAX_CONSECUTIVE_TIMEOUTS: + logger.critical( + f"Carrier platform has not recovered after {_CARRIER_MAX_CONSECUTIVE_TIMEOUTS} consecutive timeouts " + f"(~{(_CARRIER_MAX_CONSECUTIVE_TIMEOUTS * _CARRIER_REQUEST_TIMEOUT) // 60} minutes). Stopping execution." + ) + raise SystemExit( + "Test did not finish — Carrier platform is unresponsive. " + "Please try again later or contact the platform admins." + ) from exc + raise + + def str2bool(v): if isinstance(v, bool): return v @@ -130,7 +171,7 @@ def append_test_config(args): headers['Authorization'] = f'bearer {TOKEN}' url = f"{GALLOPER_URL}/api/v1/shared/job_type/{PROJECT_ID}/{args.test_id}" # get job_type - test_config = requests.get(url, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + test_config = _carrier_request("GET", url, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: test_config = test_config.json() except Exception as exc: @@ -180,7 +221,7 @@ def append_test_config(args): "type": "config" } # merge params with test config - test_config = requests.post(url, json=data, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + test_config = _carrier_request("POST", url, json=data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: test_config = test_config.json() except Exception as exc: @@ -442,7 +483,7 @@ def start_job(args=None): # upload artifact url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/sast/" file_payload = {"file": (f"{BUILD_ID}.zip", src_file)} - requests.post(url, params=s3_settings, headers=headers, files=file_payload, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + _carrier_request("POST", url, params=s3_settings, headers=headers, files=file_payload, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) if kubernetes_settings: task_kwargs = { @@ -527,7 +568,7 @@ def update_test_status(status, percentage, description): "description": description}} headers = {'content-type': 'application/json', 'Authorization': f'bearer {TOKEN}'} url = f'{GALLOPER_URL}/api/v1/{module}/report_status/{PROJECT_ID}/{REPORT_ID}' - response = requests.put(url, json=data, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + response = _carrier_request("PUT", url, json=data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: logger.info(response.json()["message"]) except: @@ -559,8 +600,8 @@ def frontend_perf_test_start_notify(args): if TOKEN: headers['Authorization'] = f'bearer {TOKEN}' - response = requests.post(f"{GALLOPER_URL}/api/v1/ui_performance/reports/{PROJECT_ID}", json=data, - headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + response = _carrier_request("POST", f"{GALLOPER_URL}/api/v1/ui_performance/reports/{PROJECT_ID}", json=data, + headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: res = response.json() logger.info(res.get("message", "")) @@ -637,7 +678,7 @@ def backend_perf_test_start_notify(args): headers['Authorization'] = f'bearer {TOKEN}' url = f'{GALLOPER_URL}/api/v1/backend_performance/reports/{PROJECT_ID}' - response = requests.post(url, json=data, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + response = _carrier_request("POST", url, json=data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) res = {} try: res = response.json() @@ -664,7 +705,7 @@ def backend_perf_test_start_notify(args): tags_data = {'tags': [{'title': 'ci/cd', 'hex': '#5933c6' }]} - requests.post(tags_url, json=tags_data, headers=headers, timeout=30, + _carrier_request("POST", tags_url, json=tags_data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) except: logger.error("Failed to add report tag") @@ -676,7 +717,7 @@ def get_project_package(): try: url = f"{GALLOPER_URL}/api/v1/projects/project/{PROJECT_ID}" headers = {'content-type': 'application/json', 'Authorization': f'bearer {TOKEN}'} - package = requests.get(url, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]).json()["package"] + package = _carrier_request("GET", url, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]).json()["package"] except: package = "custom" return package @@ -709,7 +750,7 @@ def check_test_is_saturating(test_id=None, deviation=0.02, max_deviation=0.05): "max_deviation": max_deviation, "u_aggr": U_AGGR } - response = requests.get(url, params=params, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + response = _carrier_request("GET", url, params=params, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: return response.json() except: @@ -723,7 +764,7 @@ def test_finished(report_id=REPORT_ID): headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} headers["Content-type"] = "application/json" url = f'{GALLOPER_URL}/api/v1/{module}/report_status/{PROJECT_ID}/{report_id}' - res = requests.get(url, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + res = _carrier_request("GET", url, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: res = res.json() return res["message"].lower() in { @@ -746,7 +787,7 @@ def send_minio_dump_flag(result_code: int) -> None: headers = {'Content-type': 'application/json'} if TOKEN: headers['Authorization'] = f'bearer {TOKEN}' - requests.patch(url, headers=headers, json={'build_id': BUILD_ID, 'result_code': result_code}, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + _carrier_request("PATCH", url, headers=headers, json={'build_id': BUILD_ID, 'result_code': result_code}, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) def track_job(bitter, group_id, test_id=None, deviation=0.02, max_deviation=0.05): @@ -799,7 +840,7 @@ def test_was_canceled(test_id): url = f'{GALLOPER_URL}/api/v1/{module}/report_status/{PROJECT_ID}/{test_id}' headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} headers["Content-type"] = "application/json" - status = requests.get(url, headers=headers, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]).json()['message'] + status = _carrier_request("GET", url, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]).json()['message'] return status in {'Cancelled', "Canceled", "post processing (manual)"} return False except: @@ -930,7 +971,7 @@ def download_junit_report(s3_settings, results_bucket, file_name, retry): else: url = f'{GALLOPER_URL}/artifacts/{results_bucket}/{file_name}' headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} - junit_report = requests.get(url, params=s3_settings, headers=headers, allow_redirects=True, timeout=30, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + junit_report = _carrier_request("GET", url, params=s3_settings, headers=headers, allow_redirects=True, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) if junit_report.status_code != 200 or 'botocore.errorfactory.NoSuchKey' in junit_report.text: logger.info("Waiting for report to be accessible ...") retry -= 1 @@ -957,8 +998,8 @@ def download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix list_url = f'{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{results_bucket}' headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} ssl_verify = os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"] - listing = requests.get( - list_url, params=s3_settings, headers=headers, timeout=30, verify=ssl_verify + listing = _carrier_request( + "GET", list_url, params=s3_settings, headers=headers, verify=ssl_verify ) zip_name = None if listing.status_code == 200: @@ -990,9 +1031,9 @@ def download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix sleep(10) return download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix, retry) dl_url = f'{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/{results_bucket}/{zip_name}' - response = requests.get( - dl_url, params=s3_settings, headers=headers, - allow_redirects=True, timeout=60, verify=ssl_verify + response = _carrier_request( + "GET", dl_url, params=s3_settings, headers=headers, + allow_redirects=True, verify=ssl_verify ) if response.status_code != 200: logger.warning( @@ -1067,8 +1108,8 @@ def download_lighthouse_report(s3_settings, retry=12): list_url = f'{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/reports' headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} ssl_verify = os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"] - listing = requests.get( - list_url, params=s3_settings, headers=headers, timeout=30, verify=ssl_verify + listing = _carrier_request( + "GET", list_url, params=s3_settings, headers=headers, verify=ssl_verify ) html_name = None if listing.status_code == 200: @@ -1096,9 +1137,9 @@ def download_lighthouse_report(s3_settings, retry=12): sleep(10) return download_lighthouse_report(s3_settings, retry) dl_url = f'{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/reports/{html_name}' - response = requests.get( - dl_url, params=s3_settings, headers=headers, - allow_redirects=True, timeout=60, verify=ssl_verify + response = _carrier_request( + "GET", dl_url, params=s3_settings, headers=headers, + allow_redirects=True, verify=ssl_verify ) if response.status_code != 200: logger.warning( diff --git a/tests/test_run.py b/tests/test_run.py index 6fea865..04cd917 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -14,7 +14,9 @@ environ["lg_count"] = "5" import mock +import requests import requests_mock +import urllib3 import argparse from control_tower.config_mock import BulkConfig from control_tower import run @@ -137,3 +139,286 @@ def test_start_job(arbiterMock, taskMock, mock_loki): result = run.track_job(bitter(), str(uuid4()), args.test_id) assert result == 0 + +# ============================================================================= +# HTTP Timeout Hardening Tests +# +# Feature: _carrier_request wrapper that enforces a 120-second default timeout +# on every Carrier API call and raises SystemExit after 5 consecutive timeouts. +# +# ALL tests below MUST FAIL before the implementation is added to run.py. +# Expected failure mode: AttributeError — module 'control_tower.run' has no +# attribute '_carrier_request' (or '_CARRIER_REQUEST_TIMEOUT'). +# ============================================================================= + + +@pytest.fixture +def reset_timeout_counter(): + """Reset the module-level consecutive timeout counter around each test. + + Pre-implementation: _consecutive_timeout_count does not exist — fixture is + a no-op so that the test body, not the fixture, produces the failure. + Post-implementation: resets to 0 before and after each test to prevent + counter state from bleeding between tests. + """ + if hasattr(run, '_consecutive_timeout_count'): + run._consecutive_timeout_count = 0 + yield + if hasattr(run, '_consecutive_timeout_count'): + run._consecutive_timeout_count = 0 + + +# --------------------------------------------------------------------------- +# Level 4 — Packaging: new symbols must be importable from control_tower.run +# --------------------------------------------------------------------------- + +def test_carrier_request_function_importable(): + assert hasattr(run, '_carrier_request'), ( + "_carrier_request is not defined in control_tower.run — " + "the implementation has not been added yet." + ) + + +def test_carrier_timeout_constants_importable(): + assert hasattr(run, '_CARRIER_REQUEST_TIMEOUT'), ( + "_CARRIER_REQUEST_TIMEOUT is not defined in control_tower.run" + ) + assert hasattr(run, '_CARRIER_MAX_CONSECUTIVE_TIMEOUTS'), ( + "_CARRIER_MAX_CONSECUTIVE_TIMEOUTS is not defined in control_tower.run" + ) + + +# --------------------------------------------------------------------------- +# Level 2 — Schema / Contract: constant values match the agreed SAD +# --------------------------------------------------------------------------- + +def test_timeout_value_is_120(): + assert run._CARRIER_REQUEST_TIMEOUT == 120, ( + f"Expected _CARRIER_REQUEST_TIMEOUT == 120, " + f"got {run._CARRIER_REQUEST_TIMEOUT!r}" + ) + + +def test_max_consecutive_timeouts_is_5(): + assert run._CARRIER_MAX_CONSECUTIVE_TIMEOUTS == 5, ( + f"Expected _CARRIER_MAX_CONSECUTIVE_TIMEOUTS == 5, " + f"got {run._CARRIER_MAX_CONSECUTIVE_TIMEOUTS!r}" + ) + + +def test_carrier_request_sets_default_timeout(reset_timeout_counter): + """When no timeout kwarg is passed, the outgoing request must use 120 s. + + _carrier_request calls requests.request(method, url, **kwargs) directly. + mock.patch('requests.request') intercepts that attribute lookup on the + requests module object — run.py accesses it as requests.request, so the + patch is effective. + """ + mock_response = mock.MagicMock() + mock_response.status_code = 200 + with mock.patch('requests.request', return_value=mock_response) as mock_req: + run._carrier_request('get', 'http://example.com/api/test') + assert mock_req.called, "_carrier_request did not call requests.request" + _, call_kwargs = mock_req.call_args + assert call_kwargs.get('timeout') == 120, ( + f"Expected timeout=120 to be set by default, " + f"got {call_kwargs.get('timeout')!r}" + ) + + +def test_carrier_request_explicit_timeout_wins(reset_timeout_counter): + """When an explicit timeout kwarg is passed, setdefault must NOT override it.""" + mock_response = mock.MagicMock() + mock_response.status_code = 200 + with mock.patch('requests.request', return_value=mock_response) as mock_req: + run._carrier_request('get', 'http://example.com/api/test', timeout=999) + _, call_kwargs = mock_req.call_args + assert call_kwargs.get('timeout') == 999, ( + f"Expected explicit timeout=999 to be preserved by setdefault, " + f"got {call_kwargs.get('timeout')!r}" + ) + + +# --------------------------------------------------------------------------- +# Level 1 — Unit: pure logic, no live HTTP +# --------------------------------------------------------------------------- + +def test_consecutive_timeout_counter_increments(reset_timeout_counter): + """Each Timeout exception must increment _consecutive_timeout_count by 1.""" + with mock.patch( + 'requests.request', + side_effect=requests.exceptions.Timeout("simulated timeout"), + ): + with pytest.raises(requests.exceptions.Timeout): + run._carrier_request('get', 'http://example.com/api/test') + assert run._consecutive_timeout_count == 1, ( + f"Expected counter == 1 after one timeout, " + f"got {run._consecutive_timeout_count!r}" + ) + + +def test_consecutive_timeout_counter_resets_on_success(reset_timeout_counter): + """A successful 200 response must reset _consecutive_timeout_count to 0.""" + run._consecutive_timeout_count = 3 + mock_response = mock.MagicMock() + mock_response.status_code = 200 + with mock.patch('requests.request', return_value=mock_response): + run._carrier_request('get', 'http://example.com/api/test') + assert run._consecutive_timeout_count == 0, ( + f"Expected counter reset to 0 after success, " + f"got {run._consecutive_timeout_count!r}" + ) + + +def test_five_consecutive_timeouts_raises_systemexit(reset_timeout_counter): + """Exactly 5 consecutive timeouts must raise SystemExit on the 5th call.""" + with mock.patch( + 'requests.request', + side_effect=requests.exceptions.Timeout("simulated timeout"), + ): + for _ in range(4): + with pytest.raises(requests.exceptions.Timeout): + run._carrier_request('get', 'http://example.com/api/test') + with pytest.raises(SystemExit): + run._carrier_request('get', 'http://example.com/api/test') + + +def test_systemexit_message_content(reset_timeout_counter): + """The SystemExit message must contain the agreed human-readable text.""" + run._consecutive_timeout_count = 4 + with mock.patch( + 'requests.request', + side_effect=requests.exceptions.Timeout("simulated timeout"), + ): + with pytest.raises(SystemExit) as exc_info: + run._carrier_request('get', 'http://example.com/api/test') + message = str(exc_info.value) + assert 'Test did not finish' in message, ( + f"Expected 'Test did not finish' in SystemExit message, got: {message!r}" + ) + assert 'Carrier platform is unresponsive' in message, ( + f"Expected 'Carrier platform is unresponsive' in SystemExit message, " + f"got: {message!r}" + ) + + +def test_critical_log_on_fifth_timeout(reset_timeout_counter): + """logger.critical must be called exactly once on the 5th consecutive timeout.""" + run._consecutive_timeout_count = 4 + with mock.patch( + 'requests.request', + side_effect=requests.exceptions.Timeout("simulated timeout"), + ): + with mock.patch.object(run, 'logger') as mock_logger: + with pytest.raises(SystemExit): + run._carrier_request('get', 'http://example.com/api/test') + mock_logger.critical.assert_called_once() + critical_msg = mock_logger.critical.call_args[0][0] + assert '5 consecutive timeouts' in critical_msg, ( + f"Expected '5 consecutive timeouts' in critical log message, " + f"got: {critical_msg!r}" + ) + + +def test_warning_log_on_each_timeout(reset_timeout_counter): + """logger.warning must be called with the platform-unavailable message on every timeout.""" + with mock.patch( + 'requests.request', + side_effect=requests.exceptions.Timeout("simulated timeout"), + ): + with mock.patch.object(run, 'logger') as mock_logger: + with pytest.raises(requests.exceptions.Timeout): + run._carrier_request('get', 'http://example.com/api/test') + mock_logger.warning.assert_called_once() + warning_msg = mock_logger.warning.call_args[0][0] + assert 'unavailable' in warning_msg.lower(), ( + f"Expected 'unavailable' in warning message, got: {warning_msg!r}" + ) + + +def test_readtimeout_error_is_handled(reset_timeout_counter): + """urllib3.exceptions.ReadTimeoutError must be caught by _carrier_request. + + The wrapper's except clause covers both requests.exceptions.Timeout and + urllib3.exceptions.ReadTimeoutError. This test exercises the urllib3 path. + The exception must be re-raised (or escalated to SystemExit if counter >= 5), + and the counter must be incremented. + """ + read_timeout_exc = urllib3.exceptions.ReadTimeoutError( + None, 'http://example.com/api/test', 'Read timed out.' + ) + with mock.patch('requests.request', side_effect=read_timeout_exc): + with pytest.raises((urllib3.exceptions.ReadTimeoutError, SystemExit)): + run._carrier_request('get', 'http://example.com/api/test') + assert run._consecutive_timeout_count >= 1, ( + f"Expected counter >= 1 after ReadTimeoutError, " + f"got {run._consecutive_timeout_count!r}" + ) + + +# --------------------------------------------------------------------------- +# Level 3 — Fixture-based Integration (mock HTTP, no live Carrier calls) +# +# Strategy: patch requests.Session.request (the universal sink for ALL requests +# calls — both old-style requests.put/get and the new requests.request path) +# to capture the timeout kwarg without making real network connections. +# +# Pre-implementation: calls requests.put(..., timeout=30, ...) → assertion +# fails because 30 != 120. +# Post-implementation: calls _carrier_request → requests.request(..., timeout=120, ...) +# → Session.request captures timeout=120 → assertion passes. +# --------------------------------------------------------------------------- + +def test_update_test_status_uses_120s_timeout(reset_timeout_counter): + """update_test_status must forward timeout=120 to the HTTP layer via _carrier_request.""" + mock_response = mock.MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {'message': 'ok'} + with mock.patch.object(run, 'REPORT_ID', '42'), \ + mock.patch.object(requests.Session, 'request', return_value=mock_response) as mock_session_req: + run.update_test_status(status='Running', percentage=50, description='integration test') + assert mock_session_req.called, "update_test_status did not make any HTTP request" + # Session.request is an unbound method — self is the first positional arg. + # method/url are passed as keyword args by requests.api.request, so they + # appear in call_args[1] alongside timeout. + timeout_used = mock_session_req.call_args[1].get('timeout') + assert timeout_used == 120, ( + f"update_test_status must use timeout=120 (via _carrier_request), " + f"got timeout={timeout_used!r}" + ) + + +def test_test_finished_uses_120s_timeout(reset_timeout_counter): + """test_finished must forward timeout=120 to the HTTP layer via _carrier_request.""" + mock_response = mock.MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {'message': 'Finished'} + with mock.patch.object(requests.Session, 'request', return_value=mock_response) as mock_session_req: + run.test_finished(report_id='42') + assert mock_session_req.called, "test_finished did not make any HTTP request" + timeout_used = mock_session_req.call_args[1].get('timeout') + assert timeout_used == 120, ( + f"test_finished must use timeout=120 (via _carrier_request), " + f"got timeout={timeout_used!r}" + ) + + +def test_counter_shared_across_calls(reset_timeout_counter): + """The timeout counter is module-level state shared across all call sites. + + Two consecutive Timeout exceptions from different URL paths must produce + _consecutive_timeout_count == 2, confirming the counter is not per-call-site. + """ + with mock.patch( + 'requests.request', + side_effect=requests.exceptions.Timeout("simulated timeout"), + ): + with pytest.raises(requests.exceptions.Timeout): + run._carrier_request('get', 'http://example.com/api/endpoint1') + with pytest.raises(requests.exceptions.Timeout): + run._carrier_request('post', 'http://example.com/api/endpoint2') + assert run._consecutive_timeout_count == 2, ( + f"Expected counter == 2 after two consecutive timeouts from different " + f"endpoints, got {run._consecutive_timeout_count!r}" + ) + From 2488807b71f85a7c7fa22df5e5df2cda45f5808b Mon Sep 17 00:00:00 2001 From: Mykhailo_Hunko Date: Tue, 25 Aug 2026 11:50:15 +0300 Subject: [PATCH 14/14] fix timeout errors. Add retries for critical Carrier calls --- control_tower/run.py | 146 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 119 insertions(+), 27 deletions(-) diff --git a/control_tower/run.py b/control_tower/run.py index 91a89ab..6658006 100644 --- a/control_tower/run.py +++ b/control_tower/run.py @@ -104,6 +104,33 @@ def _carrier_request(method: str, url: str, **kwargs) -> requests.Response: raise +def _carrier_request_with_retry(method: str, url: str, **kwargs) -> requests.Response: + """Critical-path wrapper: retries _carrier_request on Timeout up to + _CARRIER_MAX_CONSECUTIVE_TIMEOUTS times with a 30-second sleep between + attempts, then re-raises. + + On the final attempt the underlying _carrier_request raises SystemExit + (not Timeout) once the global consecutive-timeout counter is saturated — + that exception is not caught here and propagates naturally. + """ + for attempt in range(1, _CARRIER_MAX_CONSECUTIVE_TIMEOUTS + 1): + try: + return _carrier_request(method, url, **kwargs) + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError) as exc: + if attempt >= _CARRIER_MAX_CONSECUTIVE_TIMEOUTS: + logger.error( + "Critical request timed out after %d attempts — giving up.", + _CARRIER_MAX_CONSECUTIVE_TIMEOUTS, + ) + raise + logger.warning( + "Critical request timed out (attempt %d/%d) — retrying in 30 s ...", + attempt, + _CARRIER_MAX_CONSECUTIVE_TIMEOUTS, + ) + sleep(30) + + def str2bool(v): if isinstance(v, bool): return v @@ -171,7 +198,7 @@ def append_test_config(args): headers['Authorization'] = f'bearer {TOKEN}' url = f"{GALLOPER_URL}/api/v1/shared/job_type/{PROJECT_ID}/{args.test_id}" # get job_type - test_config = _carrier_request("GET", url, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + test_config = _carrier_request_with_retry("GET", url, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: test_config = test_config.json() except Exception as exc: @@ -221,7 +248,7 @@ def append_test_config(args): "type": "config" } # merge params with test config - test_config = _carrier_request("POST", url, json=data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + test_config = _carrier_request_with_retry("POST", url, json=data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: test_config = test_config.json() except Exception as exc: @@ -483,7 +510,13 @@ def start_job(args=None): # upload artifact url = f"{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/sast/" file_payload = {"file": (f"{BUILD_ID}.zip", src_file)} - _carrier_request("POST", url, params=s3_settings, headers=headers, files=file_payload, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + try: + _carrier_request("POST", url, params=s3_settings, headers=headers, files=file_payload, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError): + logger.warning( + "SAST artifact upload timed out — skipping. " + "The scan worker may not have the source code." + ) if kubernetes_settings: task_kwargs = { @@ -568,7 +601,7 @@ def update_test_status(status, percentage, description): "description": description}} headers = {'content-type': 'application/json', 'Authorization': f'bearer {TOKEN}'} url = f'{GALLOPER_URL}/api/v1/{module}/report_status/{PROJECT_ID}/{REPORT_ID}' - response = _carrier_request("PUT", url, json=data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + response = _carrier_request_with_retry("PUT", url, json=data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: logger.info(response.json()["message"]) except: @@ -600,7 +633,7 @@ def frontend_perf_test_start_notify(args): if TOKEN: headers['Authorization'] = f'bearer {TOKEN}' - response = _carrier_request("POST", f"{GALLOPER_URL}/api/v1/ui_performance/reports/{PROJECT_ID}", json=data, + response = _carrier_request_with_retry("POST", f"{GALLOPER_URL}/api/v1/ui_performance/reports/{PROJECT_ID}", json=data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) try: res = response.json() @@ -678,7 +711,7 @@ def backend_perf_test_start_notify(args): headers['Authorization'] = f'bearer {TOKEN}' url = f'{GALLOPER_URL}/api/v1/backend_performance/reports/{PROJECT_ID}' - response = _carrier_request("POST", url, json=data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + response = _carrier_request_with_retry("POST", url, json=data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) res = {} try: res = response.json() @@ -707,7 +740,7 @@ def backend_perf_test_start_notify(args): }]} _carrier_request("POST", tags_url, json=tags_data, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) - except: + except Exception: logger.error("Failed to add report tag") return res return {} @@ -718,7 +751,7 @@ def get_project_package(): url = f"{GALLOPER_URL}/api/v1/projects/project/{PROJECT_ID}" headers = {'content-type': 'application/json', 'Authorization': f'bearer {TOKEN}'} package = _carrier_request("GET", url, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]).json()["package"] - except: + except Exception: package = "custom" return package @@ -750,7 +783,11 @@ def check_test_is_saturating(test_id=None, deviation=0.02, max_deviation=0.05): "max_deviation": max_deviation, "u_aggr": U_AGGR } - response = _carrier_request("GET", url, params=params, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + try: + response = _carrier_request("GET", url, params=params, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError): + logger.warning("Saturation check timed out — assuming test is in progress.") + return {"message": "Test is in progress", "code": 0} try: return response.json() except: @@ -764,7 +801,11 @@ def test_finished(report_id=REPORT_ID): headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} headers["Content-type"] = "application/json" url = f'{GALLOPER_URL}/api/v1/{module}/report_status/{PROJECT_ID}/{report_id}' - res = _carrier_request("GET", url, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + try: + res = _carrier_request("GET", url, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError): + logger.warning("Report status check timed out — assuming test is still running.") + return False try: res = res.json() return res["message"].lower() in { @@ -787,7 +828,13 @@ def send_minio_dump_flag(result_code: int) -> None: headers = {'Content-type': 'application/json'} if TOKEN: headers['Authorization'] = f'bearer {TOKEN}' - _carrier_request("PATCH", url, headers=headers, json={'build_id': BUILD_ID, 'result_code': result_code}, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + try: + _carrier_request("PATCH", url, headers=headers, json={'build_id': BUILD_ID, 'result_code': result_code}, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError): + logger.warning( + "Log-dump PATCH to Carrier timed out — Carrier platform may be unavailable. " + "Logs for this run may be missing in Carrier." + ) def track_job(bitter, group_id, test_id=None, deviation=0.02, max_deviation=0.05): @@ -843,7 +890,7 @@ def test_was_canceled(test_id): status = _carrier_request("GET", url, headers=headers, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]).json()['message'] return status in {'Cancelled', "Canceled", "post processing (manual)"} return False - except: + except Exception: return False @@ -971,7 +1018,15 @@ def download_junit_report(s3_settings, results_bucket, file_name, retry): else: url = f'{GALLOPER_URL}/artifacts/{results_bucket}/{file_name}' headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} - junit_report = _carrier_request("GET", url, params=s3_settings, headers=headers, allow_redirects=True, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + try: + junit_report = _carrier_request("GET", url, params=s3_settings, headers=headers, allow_redirects=True, verify=os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"]) + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError): + logger.info("JUnit report download timed out — retrying ...") + retry -= 1 + if retry == 0: + return None + sleep(10) + return download_junit_report(s3_settings, results_bucket, file_name, retry) if junit_report.status_code != 200 or 'botocore.errorfactory.NoSuchKey' in junit_report.text: logger.info("Waiting for report to be accessible ...") retry -= 1 @@ -998,9 +1053,21 @@ def download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix list_url = f'{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/{results_bucket}' headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} ssl_verify = os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"] - listing = _carrier_request( - "GET", list_url, params=s3_settings, headers=headers, verify=ssl_verify - ) + try: + listing = _carrier_request( + "GET", list_url, params=s3_settings, headers=headers, verify=ssl_verify + ) + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError): + logger.info("Gatling artifact listing timed out — retrying ...") + retry -= 1 + if retry == 0: + logger.warning( + "download_gatling_report: listing timed out for bucket '%s' after all retries.", + results_bucket, + ) + return None + sleep(10) + return download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix, retry) zip_name = None if listing.status_code == 200: try: @@ -1031,10 +1098,17 @@ def download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix sleep(10) return download_gatling_report(s3_settings, results_bucket, distributed_mode_prefix, retry) dl_url = f'{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/{results_bucket}/{zip_name}' - response = _carrier_request( - "GET", dl_url, params=s3_settings, headers=headers, - allow_redirects=True, verify=ssl_verify - ) + try: + response = _carrier_request( + "GET", dl_url, params=s3_settings, headers=headers, + allow_redirects=True, verify=ssl_verify + ) + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError): + logger.warning( + "download_gatling_report: download of '%s' timed out.", + zip_name, + ) + return None if response.status_code != 200: logger.warning( "download_gatling_report: download of '%s' returned HTTP %s.", @@ -1108,9 +1182,20 @@ def download_lighthouse_report(s3_settings, retry=12): list_url = f'{GALLOPER_URL}/api/v1/artifacts/artifacts/{PROJECT_ID}/reports' headers = {'Authorization': f'bearer {TOKEN}'} if TOKEN else {} ssl_verify = os.environ.get("SSL_VERIFY", "").lower() in ["yes", "true"] - listing = _carrier_request( - "GET", list_url, params=s3_settings, headers=headers, verify=ssl_verify - ) + try: + listing = _carrier_request( + "GET", list_url, params=s3_settings, headers=headers, verify=ssl_verify + ) + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError): + logger.info("Lighthouse report listing timed out — retrying ...") + retry -= 1 + if retry == 0: + logger.warning( + "download_lighthouse_report: listing timed out for bucket 'reports' after all retries." + ) + return None, None + sleep(10) + return download_lighthouse_report(s3_settings, retry) html_name = None if listing.status_code == 200: try: @@ -1137,10 +1222,17 @@ def download_lighthouse_report(s3_settings, retry=12): sleep(10) return download_lighthouse_report(s3_settings, retry) dl_url = f'{GALLOPER_URL}/api/v1/artifacts/artifact/{PROJECT_ID}/reports/{html_name}' - response = _carrier_request( - "GET", dl_url, params=s3_settings, headers=headers, - allow_redirects=True, verify=ssl_verify - ) + try: + response = _carrier_request( + "GET", dl_url, params=s3_settings, headers=headers, + allow_redirects=True, verify=ssl_verify + ) + except (requests.exceptions.Timeout, urllib3.exceptions.ReadTimeoutError): + logger.warning( + "download_lighthouse_report: download of '%s' timed out.", + html_name, + ) + return None, None if response.status_code != 200: logger.warning( "download_lighthouse_report: download of '%s' returned HTTP %s.",