From 3c9ecea88514b090a4e16fe5373662c9251e3251 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Thu, 23 Jul 2026 11:11:50 -0400 Subject: [PATCH 1/3] Fix APIC database collection false passes (#327) Report missing, empty, unavailable, or malformed mitmocounters data as an explicit error instead of passing incomplete collection. Preserve any oversized-class findings and add regression coverage for sorting and threshold behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci-preupgrade-validation-script.py | 68 ++++++- .../test_apic_database_size_check.py | 187 ++++++++++++++++-- 2 files changed, 227 insertions(+), 28 deletions(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index 5e79f56e..2436a530 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6102,6 +6102,7 @@ def apic_database_size_check(cversion, **kwargs): dme_svc_list = ['vmmmgr', 'policymgr', 'eventmgr', 'policydist'] unique_list = {} + collection_errors = [] apic_id_to_name = {} apic_node_mo = icurl('class', 'infraWiNode.json') for apic in apic_node_mo: @@ -6119,15 +6120,49 @@ def apic_database_size_check(cversion, **kwargs): for dme in dme_svc_list: for id in apic_id_to_name: apic_hostname = apic_id_to_name[id] - collect_stats_cmd = 'cat /debug/'+apic_hostname+'/'+dme+'/mitmocounters/mo | grep -v ALL | sort -rn -k3' - top_class_stats = run_cmd(collect_stats_cmd, splitlines=True) - - for svc_stats in top_class_stats[:4]: - if ":" in svc_stats: - class_name = svc_stats.split(":")[0].strip() - mo_count = svc_stats.split(":")[1].strip() - if int(mo_count) > 1000*1000*1.5: - unique_list[class_name] = {"id": id, "dme": dme, "checked_val": mo_count} + counter_file = '/debug/'+apic_hostname+'/'+dme+'/mitmocounters/mo' + collect_stats_cmd = 'cat ' + counter_file + ' 2>/dev/null' + try: + class_stats = run_cmd(collect_stats_cmd, splitlines=True) + except subprocess.CalledProcessError: + collection_errors.append([id, dme, 'Counter file is unavailable']) + continue + + parsed_class_stats = [] + malformed_stats = False + for stats in class_stats: + stats = stats.strip() + if not stats or 'ALL' in stats: + continue + if ':' not in stats: + malformed_stats = True + continue + class_name, mo_count = stats.split(':', 1) + class_name = class_name.strip() + if not class_name: + malformed_stats = True + continue + try: + mo_count = int(mo_count.strip()) + except ValueError: + malformed_stats = True + continue + parsed_class_stats.append((mo_count, class_name)) + + if malformed_stats: + collection_errors.append([id, dme, 'Counter data is malformed']) + if not parsed_class_stats and not malformed_stats: + collection_errors.append([id, dme, 'Counter file is missing or empty']) + continue + + top_class_stats = sorted(parsed_class_stats, reverse=True) + for mo_count, class_name in top_class_stats[:4]: + if mo_count > 1000*1000*1.5: + unique_list[class_name] = { + "id": id, + "dme": dme, + "checked_val": str(mo_count), + } else: headers = ["APIC ID", "DME", "Shard", "Size"] recommended_action = 'Contact Cisco TAC to investigate all flagged large DB sizes' @@ -6155,6 +6190,21 @@ def apic_database_size_check(cversion, **kwargs): checked_val = details['checked_val'] data.append([apic_id, dme, unique_key, checked_val]) + if collection_errors: + return Result( + result=ERROR, + msg='Unable to collect APIC database object counters', + headers=['APIC ID', 'DME', 'Collection Error'], + data=collection_errors, + unformatted_headers=headers, + unformatted_data=data, + recommended_action=( + 'Retry the check. Contact Cisco TAC to investigate any flagged ' + 'high object counts or persistent collection errors.' + ), + doc_url=doc_url, + ) + if data: result = FAIL_UF return Result(result=result, headers=headers, data=data, recommended_action=recommended_action, doc_url=doc_url) diff --git a/tests/checks/apic_database_size_check/test_apic_database_size_check.py b/tests/checks/apic_database_size_check/test_apic_database_size_check.py index 8a651bc7..1aa2d943 100644 --- a/tests/checks/apic_database_size_check/test_apic_database_size_check.py +++ b/tests/checks/apic_database_size_check/test_apic_database_size_check.py @@ -13,25 +13,25 @@ apic_node_api = 'infraWiNode.json' -apic1_pm_cat = "cat /debug/apic1/policymgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic1_pd_cat = "cat /debug/apic1/policydist/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic1_vmm_cat = "cat /debug/apic1/vmmmgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic1_evm_cat = "cat /debug/apic1/eventmgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" - -apic2_pm_cat = "cat /debug/apic2/policymgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic2_pd_cat = "cat /debug/apic2/policydist/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic2_vmm_cat = "cat /debug/apic2/vmmmgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic2_evm_cat = "cat /debug/apic2/eventmgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" - -apic3_pm_cat = "cat /debug/apic3/policymgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic3_pd_cat = "cat /debug/apic3/policydist/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic3_vmm_cat = "cat /debug/apic3/vmmmgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic3_evm_cat = "cat /debug/apic3/eventmgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" - -apic4_pm_cat = "cat /debug/apic4/policymgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic4_pd_cat = "cat /debug/apic4/policydist/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic4_vmm_cat = "cat /debug/apic4/vmmmgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" -apic4_evm_cat = "cat /debug/apic4/eventmgr/mitmocounters/mo | grep -v ALL | sort -rn -k3" +apic1_pm_cat = "cat /debug/apic1/policymgr/mitmocounters/mo 2>/dev/null" +apic1_pd_cat = "cat /debug/apic1/policydist/mitmocounters/mo 2>/dev/null" +apic1_vmm_cat = "cat /debug/apic1/vmmmgr/mitmocounters/mo 2>/dev/null" +apic1_evm_cat = "cat /debug/apic1/eventmgr/mitmocounters/mo 2>/dev/null" + +apic2_pm_cat = "cat /debug/apic2/policymgr/mitmocounters/mo 2>/dev/null" +apic2_pd_cat = "cat /debug/apic2/policydist/mitmocounters/mo 2>/dev/null" +apic2_vmm_cat = "cat /debug/apic2/vmmmgr/mitmocounters/mo 2>/dev/null" +apic2_evm_cat = "cat /debug/apic2/eventmgr/mitmocounters/mo 2>/dev/null" + +apic3_pm_cat = "cat /debug/apic3/policymgr/mitmocounters/mo 2>/dev/null" +apic3_pd_cat = "cat /debug/apic3/policydist/mitmocounters/mo 2>/dev/null" +apic3_vmm_cat = "cat /debug/apic3/vmmmgr/mitmocounters/mo 2>/dev/null" +apic3_evm_cat = "cat /debug/apic3/eventmgr/mitmocounters/mo 2>/dev/null" + +apic4_pm_cat = "cat /debug/apic4/policymgr/mitmocounters/mo 2>/dev/null" +apic4_pd_cat = "cat /debug/apic4/policydist/mitmocounters/mo 2>/dev/null" +apic4_vmm_cat = "cat /debug/apic4/vmmmgr/mitmocounters/mo 2>/dev/null" +apic4_evm_cat = "cat /debug/apic4/eventmgr/mitmocounters/mo 2>/dev/null" apic1_acidiag = "acidiag dbsize --topshard --apic 1 -f json" apic2_acidiag = "acidiag dbsize --topshard --apic 2 -f json" @@ -334,3 +334,152 @@ def test_permission_logic(run_check, mock_icurl, mock_run_cmd, cversion, expecte cversion=script.AciVersion(cversion) if cversion else None ) assert result.result == expected_result + + +@pytest.mark.parametrize( + "failure_details,expected_error", + [ + ({"splitlines": True, "output": ""}, "Counter file is missing or empty"), + ({"CalledProcessError": True}, "Counter file is unavailable"), + ], +) +def test_missing_mitmocounters_returns_error( + run_check, + mock_icurl, + mock_run_cmd, + icurl_outputs, + cmd_outputs, + failure_details, + expected_error, +): + icurl_outputs.clear() + icurl_outputs.update({ + apic_node_api: read_data(dir, 'infraWiNode_3.json'), + }) + cmd_outputs.clear() + cmd_outputs.update({ + apic2_pm_cat: failure_details, + apic2_pd_cat: failure_details, + apic2_vmm_cat: failure_details, + apic2_evm_cat: failure_details, + }) + + result = run_check(cversion=script.AciVersion("6.0(8f)")) + + assert result.result == script.ERROR + assert result.msg == "Unable to collect APIC database object counters" + assert result.headers == ["APIC ID", "DME", "Collection Error"] + assert len(result.data) == 4 + assert all(row[2] == expected_error for row in result.data) + + +def test_collection_error_preserves_oversized_classes( + run_check, mock_icurl, mock_run_cmd, icurl_outputs, cmd_outputs +): + icurl_outputs.clear() + icurl_outputs.update({ + apic_node_api: read_data(dir, 'infraWiNode_3.json'), + }) + cmd_outputs.clear() + cmd_outputs.update({ + apic2_vmm_cat: {"splitlines": True, "output": mitcounters_vmmmgr_pos}, + apic2_pm_cat: {"CalledProcessError": True}, + apic2_evm_cat: {"splitlines": True, "output": mitcounters_neg}, + apic2_pd_cat: {"splitlines": True, "output": mitcounters_neg}, + }) + + result = run_check(cversion=script.AciVersion("6.0(8f)")) + + assert result.result == script.ERROR + assert result.data == [["2", "policymgr", "Counter file is unavailable"]] + assert result.unformatted_headers == [ + "APIC ID", "DME", "Class Name", "Object Count" + ] + assert sorted(result.unformatted_data) == sorted([ + ["2", "vmmmgr", "compProv", "1800000"], + ["2", "vmmmgr", "compatCtlrFw", "1700000"], + ["2", "vmmmgr", "aaaIRbacRule", "1600000"], + ]) + assert "high object counts" in result.recommended_action + + +def test_object_counters_are_sorted_before_top_four_and_thresholded( + run_check, mock_icurl, mock_run_cmd, icurl_outputs, cmd_outputs +): + unsorted_counters = """ +belowOne : 1 +atThreshold : 1500000 +belowTwo : 2 +highest : 1600000 +aboveThreshold : 1500001 +""" + icurl_outputs.clear() + icurl_outputs.update({ + apic_node_api: read_data(dir, 'infraWiNode_3.json'), + }) + cmd_outputs.clear() + cmd_outputs.update({ + apic2_vmm_cat: {"splitlines": True, "output": unsorted_counters}, + apic2_pm_cat: {"splitlines": True, "output": mitcounters_neg}, + apic2_evm_cat: {"splitlines": True, "output": mitcounters_neg}, + apic2_pd_cat: {"splitlines": True, "output": mitcounters_neg}, + }) + + result = run_check(cversion=script.AciVersion("6.0(8f)")) + + assert result.result == script.FAIL_UF + assert result.headers == ["APIC ID", "DME", "Class Name", "Object Count"] + assert sorted(result.data) == sorted([ + ["2", "vmmmgr", "highest", "1600000"], + ["2", "vmmmgr", "aboveThreshold", "1500001"], + ]) + + +def test_malformed_counter_preserves_oversized_classes( + run_check, mock_icurl, mock_run_cmd, icurl_outputs, cmd_outputs +): + icurl_outputs.clear() + icurl_outputs.update({ + apic_node_api: read_data(dir, 'infraWiNode_3.json'), + }) + cmd_outputs.clear() + cmd_outputs.update({ + apic2_vmm_cat: {"splitlines": True, "output": mitcounters_vmmmgr_pos}, + apic2_pm_cat: {"splitlines": True, "output": "brokenClass :"}, + apic2_evm_cat: {"splitlines": True, "output": mitcounters_neg}, + apic2_pd_cat: {"splitlines": True, "output": mitcounters_neg}, + }) + + result = run_check(cversion=script.AciVersion("6.0(8f)")) + + assert result.result == script.ERROR + assert result.data == [["2", "policymgr", "Counter data is malformed"]] + assert sorted(result.unformatted_data) == sorted([ + ["2", "vmmmgr", "compProv", "1800000"], + ["2", "vmmmgr", "compatCtlrFw", "1700000"], + ["2", "vmmmgr", "aaaIRbacRule", "1600000"], + ]) + + +def test_colonless_counter_data_returns_error( + run_check, mock_icurl, mock_run_cmd, icurl_outputs, cmd_outputs +): + icurl_outputs.clear() + icurl_outputs.update({ + apic_node_api: read_data(dir, 'infraWiNode_3.json'), + }) + cmd_outputs.clear() + cmd_outputs.update({ + apic2_vmm_cat: { + "splitlines": True, + "output": "validClass : 10\ntruncatedClass", + }, + apic2_pm_cat: {"splitlines": True, "output": mitcounters_neg}, + apic2_evm_cat: {"splitlines": True, "output": mitcounters_neg}, + apic2_pd_cat: {"splitlines": True, "output": mitcounters_neg}, + }) + + result = run_check(cversion=script.AciVersion("6.0(8f)")) + + assert result.result == script.ERROR + assert result.data == [["2", "vmmmgr", "Counter data is malformed"]] From 029d4d41594873f4a24f8738012946851b2f678f Mon Sep 17 00:00:00 2001 From: Gabriel Date: Thu, 23 Jul 2026 12:36:21 -0400 Subject: [PATCH 2/3] Retry transient APIC database counter reads (#327) Retry each legacy mitmocounters read up to three times and capture the final command error when collection remains unavailable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci-preupgrade-validation-script.py | 40 ++++++- .../test_apic_database_size_check.py | 101 ++++++++++++++---- tests/checks/conftest.py | 6 +- 3 files changed, 121 insertions(+), 26 deletions(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index 2436a530..418dfd88 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6101,6 +6101,8 @@ def apic_database_size_check(cversion, **kwargs): doc_url = 'https://datacenter.github.io/ACI-Pre-Upgrade-Validation-Script/validations/#apic-database-size' dme_svc_list = ['vmmmgr', 'policymgr', 'eventmgr', 'policydist'] + counter_read_attempts = 3 + counter_read_retry_delay = 1 unique_list = {} collection_errors = [] apic_id_to_name = {} @@ -6121,11 +6123,39 @@ def apic_database_size_check(cversion, **kwargs): for id in apic_id_to_name: apic_hostname = apic_id_to_name[id] counter_file = '/debug/'+apic_hostname+'/'+dme+'/mitmocounters/mo' - collect_stats_cmd = 'cat ' + counter_file + ' 2>/dev/null' - try: - class_stats = run_cmd(collect_stats_cmd, splitlines=True) - except subprocess.CalledProcessError: - collection_errors.append([id, dme, 'Counter file is unavailable']) + collect_stats_cmd = 'cat ' + counter_file + ' 2>&1' + class_stats = None + final_error = None + for attempt in range(1, counter_read_attempts + 1): + try: + class_stats = run_cmd(collect_stats_cmd, splitlines=True) + break + except subprocess.CalledProcessError as error: + error_output = error.output + if isinstance(error_output, bytes): + error_output = error_output.decode('utf-8', 'replace') + final_error = (error_output or str(error)).strip() + if attempt < counter_read_attempts: + log.warning( + 'Counter read failed for APIC %s %s ' + '(attempt %s/%s): %s', + id, + dme, + attempt, + counter_read_attempts, + final_error, + ) + time.sleep(counter_read_retry_delay) + + if class_stats is None: + collection_errors.append([ + id, + dme, + 'Counter file is unavailable after %s attempts: %s' % ( + counter_read_attempts, + final_error, + ), + ]) continue parsed_class_stats = [] diff --git a/tests/checks/apic_database_size_check/test_apic_database_size_check.py b/tests/checks/apic_database_size_check/test_apic_database_size_check.py index 1aa2d943..f1cbeb54 100644 --- a/tests/checks/apic_database_size_check/test_apic_database_size_check.py +++ b/tests/checks/apic_database_size_check/test_apic_database_size_check.py @@ -2,6 +2,7 @@ import pytest import logging import importlib +from subprocess import CalledProcessError from helpers.utils import read_data script = importlib.import_module("aci-preupgrade-validation-script") @@ -13,25 +14,25 @@ apic_node_api = 'infraWiNode.json' -apic1_pm_cat = "cat /debug/apic1/policymgr/mitmocounters/mo 2>/dev/null" -apic1_pd_cat = "cat /debug/apic1/policydist/mitmocounters/mo 2>/dev/null" -apic1_vmm_cat = "cat /debug/apic1/vmmmgr/mitmocounters/mo 2>/dev/null" -apic1_evm_cat = "cat /debug/apic1/eventmgr/mitmocounters/mo 2>/dev/null" +apic1_pm_cat = "cat /debug/apic1/policymgr/mitmocounters/mo 2>&1" +apic1_pd_cat = "cat /debug/apic1/policydist/mitmocounters/mo 2>&1" +apic1_vmm_cat = "cat /debug/apic1/vmmmgr/mitmocounters/mo 2>&1" +apic1_evm_cat = "cat /debug/apic1/eventmgr/mitmocounters/mo 2>&1" -apic2_pm_cat = "cat /debug/apic2/policymgr/mitmocounters/mo 2>/dev/null" -apic2_pd_cat = "cat /debug/apic2/policydist/mitmocounters/mo 2>/dev/null" -apic2_vmm_cat = "cat /debug/apic2/vmmmgr/mitmocounters/mo 2>/dev/null" -apic2_evm_cat = "cat /debug/apic2/eventmgr/mitmocounters/mo 2>/dev/null" +apic2_pm_cat = "cat /debug/apic2/policymgr/mitmocounters/mo 2>&1" +apic2_pd_cat = "cat /debug/apic2/policydist/mitmocounters/mo 2>&1" +apic2_vmm_cat = "cat /debug/apic2/vmmmgr/mitmocounters/mo 2>&1" +apic2_evm_cat = "cat /debug/apic2/eventmgr/mitmocounters/mo 2>&1" -apic3_pm_cat = "cat /debug/apic3/policymgr/mitmocounters/mo 2>/dev/null" -apic3_pd_cat = "cat /debug/apic3/policydist/mitmocounters/mo 2>/dev/null" -apic3_vmm_cat = "cat /debug/apic3/vmmmgr/mitmocounters/mo 2>/dev/null" -apic3_evm_cat = "cat /debug/apic3/eventmgr/mitmocounters/mo 2>/dev/null" +apic3_pm_cat = "cat /debug/apic3/policymgr/mitmocounters/mo 2>&1" +apic3_pd_cat = "cat /debug/apic3/policydist/mitmocounters/mo 2>&1" +apic3_vmm_cat = "cat /debug/apic3/vmmmgr/mitmocounters/mo 2>&1" +apic3_evm_cat = "cat /debug/apic3/eventmgr/mitmocounters/mo 2>&1" -apic4_pm_cat = "cat /debug/apic4/policymgr/mitmocounters/mo 2>/dev/null" -apic4_pd_cat = "cat /debug/apic4/policydist/mitmocounters/mo 2>/dev/null" -apic4_vmm_cat = "cat /debug/apic4/vmmmgr/mitmocounters/mo 2>/dev/null" -apic4_evm_cat = "cat /debug/apic4/eventmgr/mitmocounters/mo 2>/dev/null" +apic4_pm_cat = "cat /debug/apic4/policymgr/mitmocounters/mo 2>&1" +apic4_pd_cat = "cat /debug/apic4/policydist/mitmocounters/mo 2>&1" +apic4_vmm_cat = "cat /debug/apic4/vmmmgr/mitmocounters/mo 2>&1" +apic4_evm_cat = "cat /debug/apic4/eventmgr/mitmocounters/mo 2>&1" apic1_acidiag = "acidiag dbsize --topshard --apic 1 -f json" apic2_acidiag = "acidiag dbsize --topshard --apic 2 -f json" @@ -340,7 +341,17 @@ def test_permission_logic(run_check, mock_icurl, mock_run_cmd, cversion, expecte "failure_details,expected_error", [ ({"splitlines": True, "output": ""}, "Counter file is missing or empty"), - ({"CalledProcessError": True}, "Counter file is unavailable"), + ( + { + "CalledProcessError": True, + "returncode": 1, + "error_output": b"cat: file: No such file or directory\n", + }, + ( + "Counter file is unavailable after 3 attempts: " + "cat: file: No such file or directory" + ), + ), ], ) def test_missing_mitmocounters_returns_error( @@ -351,6 +362,7 @@ def test_missing_mitmocounters_returns_error( cmd_outputs, failure_details, expected_error, + monkeypatch, ): icurl_outputs.clear() icurl_outputs.update({ @@ -363,6 +375,7 @@ def test_missing_mitmocounters_returns_error( apic2_vmm_cat: failure_details, apic2_evm_cat: failure_details, }) + monkeypatch.setattr(script.time, "sleep", lambda _: None) result = run_check(cversion=script.AciVersion("6.0(8f)")) @@ -374,7 +387,7 @@ def test_missing_mitmocounters_returns_error( def test_collection_error_preserves_oversized_classes( - run_check, mock_icurl, mock_run_cmd, icurl_outputs, cmd_outputs + run_check, mock_icurl, mock_run_cmd, icurl_outputs, cmd_outputs, monkeypatch ): icurl_outputs.clear() icurl_outputs.update({ @@ -383,15 +396,26 @@ def test_collection_error_preserves_oversized_classes( cmd_outputs.clear() cmd_outputs.update({ apic2_vmm_cat: {"splitlines": True, "output": mitcounters_vmmmgr_pos}, - apic2_pm_cat: {"CalledProcessError": True}, + apic2_pm_cat: { + "CalledProcessError": True, + "error_output": b"cat: file: No such file or directory\n", + }, apic2_evm_cat: {"splitlines": True, "output": mitcounters_neg}, apic2_pd_cat: {"splitlines": True, "output": mitcounters_neg}, }) + monkeypatch.setattr(script.time, "sleep", lambda _: None) result = run_check(cversion=script.AciVersion("6.0(8f)")) assert result.result == script.ERROR - assert result.data == [["2", "policymgr", "Counter file is unavailable"]] + assert result.data == [[ + "2", + "policymgr", + ( + "Counter file is unavailable after 3 attempts: " + "cat: file: No such file or directory" + ), + ]] assert result.unformatted_headers == [ "APIC ID", "DME", "Class Name", "Object Count" ] @@ -403,6 +427,43 @@ def test_collection_error_preserves_oversized_classes( assert "high object counts" in result.recommended_action +def test_transient_counter_read_succeeds_on_retry( + run_check, mock_icurl, mock_run_cmd, icurl_outputs, cmd_outputs, monkeypatch +): + icurl_outputs.clear() + icurl_outputs.update({ + apic_node_api: read_data(dir, 'infraWiNode_3.json'), + }) + successful_outputs = { + apic2_vmm_cat: mitcounters_neg, + apic2_pm_cat: mitcounters_neg, + apic2_evm_cat: mitcounters_neg, + apic2_pd_cat: mitcounters_neg, + } + call_counts = {} + sleep_calls = [] + + def transient_run_cmd(cmd, splitlines=False): + call_counts[cmd] = call_counts.get(cmd, 0) + 1 + if call_counts[cmd] == 1: + raise CalledProcessError( + 1, + cmd, + output=b"cat: file: No such file or directory\n", + ) + output = successful_outputs[cmd] + return output.splitlines() if splitlines else output + + monkeypatch.setattr(script, "run_cmd", transient_run_cmd) + monkeypatch.setattr(script.time, "sleep", sleep_calls.append) + + result = run_check(cversion=script.AciVersion("6.0(8f)")) + + assert result.result == script.PASS + assert all(call_count == 2 for call_count in call_counts.values()) + assert sleep_calls.count(1) == 4 + + def test_object_counters_are_sorted_before_top_four_and_thresholded( run_check, mock_icurl, mock_run_cmd, icurl_outputs, cmd_outputs ): diff --git a/tests/checks/conftest.py b/tests/checks/conftest.py index 01cd8870..8f883dcf 100644 --- a/tests/checks/conftest.py +++ b/tests/checks/conftest.py @@ -121,7 +121,11 @@ def _mock_run_cmd(cmd, splitlines=False): log.error("Command `%s` not found in test data", cmd) return "" if details.get("CalledProcessError"): - raise CalledProcessError(127, cmd) + raise CalledProcessError( + details.get("returncode", 127), + cmd, + output=details.get("error_output"), + ) splitlines = details.get("splitlines", False) output = details.get("output") From 9a8448e41465e0edf2b630b8b48c9322c5b60bd1 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Wed, 12 Aug 2026 18:37:30 -0400 Subject: [PATCH 3/3] Retry empty APIC database counter reads --- aci-preupgrade-validation-script.py | 13 +++++++- .../test_apic_database_size_check.py | 33 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index 418dfd88..f9306a72 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6129,7 +6129,18 @@ def apic_database_size_check(cversion, **kwargs): for attempt in range(1, counter_read_attempts + 1): try: class_stats = run_cmd(collect_stats_cmd, splitlines=True) - break + if class_stats: + break + if attempt < counter_read_attempts: + log.warning( + 'Counter read returned no data for APIC %s %s ' + '(attempt %s/%s)', + id, + dme, + attempt, + counter_read_attempts, + ) + time.sleep(counter_read_retry_delay) except subprocess.CalledProcessError as error: error_output = error.output if isinstance(error_output, bytes): diff --git a/tests/checks/apic_database_size_check/test_apic_database_size_check.py b/tests/checks/apic_database_size_check/test_apic_database_size_check.py index f1cbeb54..a4b058b1 100644 --- a/tests/checks/apic_database_size_check/test_apic_database_size_check.py +++ b/tests/checks/apic_database_size_check/test_apic_database_size_check.py @@ -464,6 +464,39 @@ def transient_run_cmd(cmd, splitlines=False): assert sleep_calls.count(1) == 4 +def test_empty_counter_read_succeeds_on_retry( + run_check, mock_icurl, mock_run_cmd, icurl_outputs, cmd_outputs, monkeypatch +): + icurl_outputs.clear() + icurl_outputs.update({ + apic_node_api: read_data(dir, 'infraWiNode_3.json'), + }) + successful_outputs = { + apic2_vmm_cat: mitcounters_neg, + apic2_pm_cat: mitcounters_neg, + apic2_evm_cat: mitcounters_neg, + apic2_pd_cat: mitcounters_neg, + } + call_counts = {} + sleep_calls = [] + + def transient_run_cmd(cmd, splitlines=False): + call_counts[cmd] = call_counts.get(cmd, 0) + 1 + if call_counts[cmd] == 1: + return [] + output = successful_outputs[cmd] + return output.splitlines() if splitlines else output + + monkeypatch.setattr(script, "run_cmd", transient_run_cmd) + monkeypatch.setattr(script.time, "sleep", sleep_calls.append) + + result = run_check(cversion=script.AciVersion("6.0(8f)")) + + assert result.result == script.PASS + assert all(call_count == 2 for call_count in call_counts.values()) + assert sleep_calls.count(1) == 4 + + def test_object_counters_are_sorted_before_top_four_and_thresholded( run_check, mock_icurl, mock_run_cmd, icurl_outputs, cmd_outputs ):