From ccc5e8c264bce8cfa5e6c2d50248dfb4070c5484 Mon Sep 17 00:00:00 2001 From: sutter Date: Thu, 23 Jul 2026 11:12:03 -0700 Subject: [PATCH 01/16] xcvrd draft Signed-off-by: sutter --- .../process_restart/test_xcvrd_restart.py | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 tests/transceiver/system/process_restart/test_xcvrd_restart.py diff --git a/tests/transceiver/system/process_restart/test_xcvrd_restart.py b/tests/transceiver/system/process_restart/test_xcvrd_restart.py new file mode 100644 index 00000000000..4c0fbc53cd0 --- /dev/null +++ b/tests/transceiver/system/process_restart/test_xcvrd_restart.py @@ -0,0 +1,158 @@ +"""System / Process Restart - xcvrd daemon restart validation. + +Implements xcvrd daemon restart impact, xcvrd restart with I2C errors, +and xcvrd crash recovery test from + ``docs/testplan/transceiver/system_test_plan.md``. + +Execution order:: + + session start + `- check_links_up() <- session-scoped via + ``links_verified`` in + tests/transceiver/conftest.py + (failure skips every System test) + `- test_system_xcvrd_restart_simple + |- run_pre_check (xcvrd RUNNING) <- _per_test_health_check + |- : restart xcvrd -> verify all ports recovery + `- run_post_check + `- test_system_xcvrd_restart_with_i2c_errors + |- run_pre_check (xcvrd RUNNING) <- _per_test_health_check + |- : induce I2C errors -> restart xcvrd -> verify recovery + `- run_post_check + `- test_system_xcvrd_crash_recovery + |- run_pre_check (xcvrd RUNNING) <- _per_test_health_check + |- : inject crash -> monitor restart -> verify recovery + `- run_post_check + session end + `- _system_post_session_checks (system/conftest.py) + |- post_state_restoration() + |- STATE_DB consistency check + `- final link + LLDP check + +Failure handling: failures are accumulated per test case and reported in a single +pytest.fail at the end, so a single run surfaces all issues across all ports. +""" +import logging +import time +# import re +# import time +import pytest + +# from tests.transceiver.attribute_parser.attribute_keys import SYSTEM_ATTRIBUTES_KEY +from tests.transceiver.common.health_checks import ( + capture_baseline +) +from tests.transceiver.common.prerequisites import ( + check_links_up +) +from tests.transceiver.common.verification import ( + standard_port_recovery_and_verification +) +import tests.transceiver.common.process_restart_helpers as prh +from tests.common.helpers.dut_utils import get_program_info + +logger = logging.getLogger(__name__) + +@pytest.mark.disable_loganalyzer +def test_system_xcvrd_restart(duthost, port_attributes_dict, expected_pid_changes): + """Restart xcvrd and verify all ports recover cleanly. + + See the module docstring for the full execution tree. Steps: + + * verify all ports are oper-up and record xcvrd uptime, + * restart xcvrd via ``docker exec pmon supervisorctl restart xcvrd``, + * wait for ``xcvrd_restart_settle_sec`` then run Standard Port Recovery + and Verification for every port. + + All (port, step) failures are accumulated and reported in a single + ``pytest.fail`` so one run surfaces every issue. + """ + expected_pid_changes.add("xcvrd") + ports = sorted(port_attributes_dict.keys()) + xcvrd_wait = prh.sys_attr(port_attributes_dict[ports[0]], "xcvrd_restart_settle_sec", 120) + assert ports, "port_attributes_dict is empty - nothing to validate" + health_baseline = capture_baseline(duthost) + failures = [] # collected across every (port, step) tuple + + logger.info("Recording link states and uptime for %d port(s)", len(ports)) + logger.info("Recording initialXcvrD uptime: %s", prh.get_xcvrd_uptime(duthost)) + if not check_links_up(duthost, port_attributes_dict): + logger.warning("Validation on Start FAILED: some ports are down") + logger.info("Restarting xcvrd...") + prh.restart_process(duthost, 'xcvrd') + time.sleep(xcvrd_wait) + + # Wait for settle time and verify + time.sleep(xcvrd_wait) + logger.info("Running Standard Port Recovery and Verification for %d port(s)", len(ports)) + result = standard_port_recovery_and_verification( + duthost, ports, port_attributes_dict, + link_up_timeout_sec=xcvrd_wait, + health_baseline = health_baseline, + shared_state=None, + expected_pid_changes='xcvrd' + ) + if not result["passed"]: + failures.append(f"[post-restart] {result['details']}") + logger.warning("Post-restart validation FAILED: %s", result["details"]) + else: + logger.info("Post-restart validation PASSED: %s", result["details"]) + + if failures: + pytest.fail( + f"TC1: xcvrd restart recovery FAILED on {len(failures)} port(s):\n - " + + "\n - ".join(failures) + ) + +@pytest.mark.disable_loganalyzer +def test_system_xcvrd_crash_recovery(duthost, port_attributes_dict, expected_pid_changes): + """Inject an xcvrd crash and verify automatic restart and port recovery. + + See the module docstring for the full execution tree. Steps: + + * verify all ports are oper-up and record xcvrd uptime, + * inject a crash into the xcvrd script via ``inject_xcvrd_crash``, + * monitor automatic restart behavior, + * wait for ``xcvrd_restart_settle_sec`` then run Standard Port Recovery + and Verification for every port. + + All (port, step) failures are accumulated and reported in a single + ``pytest.fail`` so one run surfaces every issue. + """ + expected_pid_changes.add("xcvrd") + ports = sorted(port_attributes_dict.keys()) + assert ports, "port_attributes_dict is empty - nothing to validate" + xcvrd_wait = prh.sys_attr(port_attributes_dict[ports[0]], "xcvrd_restart_settle_sec", 120) + health_baseline = capture_baseline(duthost) + failures = [] # collected across every (port, step) tuple + + logger.info("Recording initial link states for %d port(s)", len(ports)) + logger.info("Recording initial XcvrD uptime: %s", prh.get_xcvrd_uptime(duthost)) + if not check_links_up(duthost, port_attributes_dict): + logger.warning("Validation on Start FAILED: some ports are down") + + logger.info("Using SIGKILL to crash xcvrd") + status, pid = get_program_info(duthost, 'pmon', 'xcvrd') + duthost.kill_pmon_daemon_pid_w_sig(pid, -9) + + # Wait, then run Standard Port Recovery and Verification for all ports + time.sleep(xcvrd_wait) + logger.info("Running Standard Port Recovery and Verification for %d port(s)", len(ports)) + result = standard_port_recovery_and_verification( + duthost, ports, port_attributes_dict, + link_up_timeout_sec=xcvrd_wait, + health_baseline = health_baseline, + shared_state=None, + expected_pid_changes='xcvrd' + ) + if not result["passed"]: + failures.append(f"[post-crash] {result['details']}") + logger.warning("Post-crash validation FAILED: %s", result["details"]) + else: + logger.info("Post-crash validation PASSED: %s", result["details"]) + + if failures: + pytest.fail( + f"xcvrd crash recovery FAILED on {len(failures)} port(s):\n - " + + "\n - ".join(failures) + ) \ No newline at end of file From 8ce4483b119558b5c92905d4615880b5e1ca26d0 Mon Sep 17 00:00:00 2001 From: sutter Date: Tue, 28 Jul 2026 13:36:21 -0700 Subject: [PATCH 02/16] Addressed GH copilot comments - still non functional Signed-off-by: sutter --- .../process_restart/test_xcvrd_restart.py | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/tests/transceiver/system/process_restart/test_xcvrd_restart.py b/tests/transceiver/system/process_restart/test_xcvrd_restart.py index 4c0fbc53cd0..0980b7d4e6a 100644 --- a/tests/transceiver/system/process_restart/test_xcvrd_restart.py +++ b/tests/transceiver/system/process_restart/test_xcvrd_restart.py @@ -11,11 +11,12 @@ ``links_verified`` in tests/transceiver/conftest.py (failure skips every System test) - `- test_system_xcvrd_restart_simple + `- test_system_xcvrd_restart |- run_pre_check (xcvrd RUNNING) <- _per_test_health_check |- : restart xcvrd -> verify all ports recovery `- run_post_check - `- test_system_xcvrd_restart_with_i2c_errors + `- test_system_xcvrd_restart_with_i2c_errors + NOTE: this test has been skipped during initial development due to library issues |- run_pre_check (xcvrd RUNNING) <- _per_test_health_check |- : induce I2C errors -> restart xcvrd -> verify recovery `- run_post_check @@ -47,7 +48,7 @@ ) from tests.transceiver.common.verification import ( standard_port_recovery_and_verification -) +) import tests.transceiver.common.process_restart_helpers as prh from tests.common.helpers.dut_utils import get_program_info @@ -69,26 +70,25 @@ def test_system_xcvrd_restart(duthost, port_attributes_dict, expected_pid_change """ expected_pid_changes.add("xcvrd") ports = sorted(port_attributes_dict.keys()) - xcvrd_wait = prh.sys_attr(port_attributes_dict[ports[0]], "xcvrd_restart_settle_sec", 120) assert ports, "port_attributes_dict is empty - nothing to validate" + xcvrd_wait = prh.sys_attr(port_attributes_dict[ports[0]], "xcvrd_restart_settle_sec", 120) health_baseline = capture_baseline(duthost) failures = [] # collected across every (port, step) tuple logger.info("Recording link states and uptime for %d port(s)", len(ports)) logger.info("Recording initialXcvrD uptime: %s", prh.get_xcvrd_uptime(duthost)) - if not check_links_up(duthost, port_attributes_dict): + link_check = check_links_up(duthost, port_attributes_dict) + if not link_check["passed"]: logger.warning("Validation on Start FAILED: some ports are down") logger.info("Restarting xcvrd...") prh.restart_process(duthost, 'xcvrd') time.sleep(xcvrd_wait) - # Wait for settle time and verify - time.sleep(xcvrd_wait) logger.info("Running Standard Port Recovery and Verification for %d port(s)", len(ports)) result = standard_port_recovery_and_verification( duthost, ports, port_attributes_dict, link_up_timeout_sec=xcvrd_wait, - health_baseline = health_baseline, + health_baseline=health_baseline, shared_state=None, expected_pid_changes='xcvrd' ) @@ -128,12 +128,13 @@ def test_system_xcvrd_crash_recovery(duthost, port_attributes_dict, expected_pid logger.info("Recording initial link states for %d port(s)", len(ports)) logger.info("Recording initial XcvrD uptime: %s", prh.get_xcvrd_uptime(duthost)) - if not check_links_up(duthost, port_attributes_dict): + link_check = check_links_up(duthost, port_attributes_dict) + if not link_check["passed"]: logger.warning("Validation on Start FAILED: some ports are down") logger.info("Using SIGKILL to crash xcvrd") status, pid = get_program_info(duthost, 'pmon', 'xcvrd') - duthost.kill_pmon_daemon_pid_w_sig(pid, -9) + duthost.kill_pmon_daemon_pid_w_sig(pid, "-9") # Wait, then run Standard Port Recovery and Verification for all ports time.sleep(xcvrd_wait) @@ -141,7 +142,7 @@ def test_system_xcvrd_crash_recovery(duthost, port_attributes_dict, expected_pid result = standard_port_recovery_and_verification( duthost, ports, port_attributes_dict, link_up_timeout_sec=xcvrd_wait, - health_baseline = health_baseline, + health_baseline=health_baseline, shared_state=None, expected_pid_changes='xcvrd' ) From 85fea0d7c9628e821da889488007b2b6e089fa54 Mon Sep 17 00:00:00 2001 From: sutter Date: Wed, 5 Aug 2026 11:07:02 -0700 Subject: [PATCH 03/16] Combined restart tcs and refactored to work w new verification func Signed-off-by: sutter --- tests/transceiver/common/verification.py | 533 ++++++++++++++++++ .../system/process_restart/__init__.py | 0 .../system/process_restart/conftest.py | 14 + .../process_restart/test_pmon_restart.py | 111 ++++ .../process_restart/test_swss_restart.py | 121 ++++ .../process_restart/test_syncd_restart.py | 125 ++++ .../process_restart/test_xcvrd_restart.py | 109 ++-- 7 files changed, 976 insertions(+), 37 deletions(-) create mode 100644 tests/transceiver/common/verification.py create mode 100644 tests/transceiver/system/process_restart/__init__.py create mode 100644 tests/transceiver/system/process_restart/conftest.py create mode 100644 tests/transceiver/system/process_restart/test_pmon_restart.py create mode 100644 tests/transceiver/system/process_restart/test_swss_restart.py create mode 100644 tests/transceiver/system/process_restart/test_syncd_restart.py diff --git a/tests/transceiver/common/verification.py b/tests/transceiver/common/verification.py new file mode 100644 index 00000000000..2c5172167d0 --- /dev/null +++ b/tests/transceiver/common/verification.py @@ -0,0 +1,533 @@ +"""Standard Port Recovery and Verification Procedures. + +Implements the Standard Port and Verification function, +as well as the related child functions. All parent and +child functions will return an output following the format: + +dict: ``{port: {'passed': bool, 'details': str}}`` + +""" +import logging +import re +import time + +from tests.common.platform.interface_utils import wait_ports_oper_status +from tests.transceiver.attribute_parser.attribute_keys import ( + EEPROM_ATTRIBUTES_KEY, + SYSTEM_ATTRIBUTES_KEY, +) +from tests.transceiver.common import db_helpers, health_checks +from tests.transceiver.common.eeprom_decode import is_cmis_active_optical + +logger = logging.getLogger(__name__) + +# Post-recovery observation window (seconds) for the mandatory Link +# Flap/Stability "Stability (always)" sub-check in system_test_plan.md, which +# does not pin an exact duration ("a short post-recovery observation window"). +# Kept small since this runs once per port on every Standard Port Recovery +# call. +DEFAULT_STABILITY_WINDOW_SEC = 5 + + +# ────────────────────────────────────────────────────────────────────── +# LLDP neighbor poll +# ────────────────────────────────────────────────────────────────────── + +_LLDP_POLL_INTERVAL_SEC = 3 + + +def check_lldp_neighbors_present(duthost, port_timeouts, namespaces=None): + """Poll APPL_DB ``LLDP_ENTRY_TABLE:`` until every port has a neighbor. + + Why: System tests need to confirm the far end re-converged after a + disruptive operation; polling all ports together (each with its own + timeout) lets waits overlap instead of summing per-port timeouts. + + Args: + duthost: SONiC DUT host fixture. + port_timeouts: dict of ``{port: timeout_sec}``. + namespaces: optional dict of ``{port: namespace}``. + + Returns: + dict: ``{port: {'passed': bool, 'details': str}}``, one entry per + ``port_timeouts``. + """ + if namespaces is None: + namespaces = {} + + def _namespace_for(port): + if port in namespaces: + return namespaces[port] + return db_helpers.resolve_namespace(duthost, port) + + ports_by_ns = {} + for port in port_timeouts: + ports_by_ns.setdefault(_namespace_for(port), []).append(port) + + start = time.monotonic() + deadlines = {port: start + max(0, int(timeout_sec)) for port, timeout_sec in port_timeouts.items()} + remaining = set(port_timeouts) + passed_ports = set() + + while remaining: + for ns, ports_in_ns in ports_by_ns.items(): + pending_in_ns = [port for port in ports_in_ns if port in remaining] + if not pending_in_ns: + continue + by_key, err = db_helpers.get_db_table(duthost, "APPL_DB", "LLDP_ENTRY_TABLE", namespace=ns, sep=":") + if err: + continue + for port in pending_in_ns: + if port in by_key: + passed_ports.add(port) + remaining.discard(port) + + now = time.monotonic() + for port in list(remaining): + if now >= deadlines[port]: + remaining.discard(port) + if not remaining: + break + # Cap the sleep to whatever's left before the earliest still-remaining + # deadline, so a full _LLDP_POLL_INTERVAL_SEC never overshoots a + # deadline that falls inside this interval - keeps the reported + # "no LLDP neighbor after Ns" honest instead of running up to one + # poll interval past N. + sleep_for = min(_LLDP_POLL_INTERVAL_SEC, max(0, min(deadlines[port] for port in remaining) - now)) + time.sleep(sleep_for) + + per_port = {} + for port, timeout_sec in port_timeouts.items(): + if port in passed_ports: + details = f"{port}: LLDP neighbor present within {timeout_sec}s" + logger.info("LLDP check PASSED: %s", details) + per_port[port] = {"passed": True, "details": details} + else: + details = f"{port}: no LLDP neighbor after {timeout_sec}s" + logger.warning("LLDP check FAILED: %s", details) + per_port[port] = {"passed": False, "details": details} + return per_port + + +# ────────────────────────────────────────────────────────────────────── +# Link Flap / Stability check +# ────────────────────────────────────────────────────────────────────── + + +def capture_flap_sentinels(duthost, ports, namespaces=None): + """Snapshot every port's APPL_DB ``PORT_TABLE:`` ``flap_count``/``last_up_time`` once. + + Why: provides the shared baseline that :func:`assert_no_flap_since` later + compares against, regardless of how the elapsed time in between is spent. + + Args: + duthost: SONiC DUT host fixture. + ports: list of logical interface names. + namespaces: optional dict of ``{port: namespace}``. + + Returns: + dict: ``{port: (flap_count, last_up_time)}`` - both raw APPL_DB + strings (or ``None`` if either field is absent), one entry per ``ports``. + """ + if namespaces is None: + namespaces = {} + + def _namespace_for(port): + if port in namespaces: + return namespaces[port] + return db_helpers.resolve_namespace(duthost, port) + + sentinels = {} + for port in ports: + port_table = db_helpers.hgetall_dict( + duthost, "APPL_DB", f"PORT_TABLE:{port}", namespace=_namespace_for(port) + ) + sentinels[port] = (port_table.get("flap_count"), port_table.get("last_up_time")) + return sentinels + + +def assert_no_flap_since(duthost, ports, sentinels, namespaces=None, elapsed_sec=None): + """Verify no port in ``ports`` has flapped since its ``sentinels`` snapshot. + + Why: the comparison step of a flap-freeness check, decoupled from how the + sentinel was captured or how much time has elapsed, so callers can overlap + the observation window with other work instead of a flat sleep. + + Args: + duthost: SONiC DUT host fixture. + ports: list of logical interface names. + sentinels: dict of ``{port: (flap_count, last_up_time)}``, from + :func:`capture_flap_sentinels`. + namespaces: optional dict of ``{port: namespace}``. + elapsed_sec: optional, for the details message only. + + Returns: + dict: ``{port: {'passed': bool, 'details': str}}``, one entry per ``ports``. + """ + if namespaces is None: + namespaces = {} + + def _namespace_for(port): + if port in namespaces: + return namespaces[port] + return db_helpers.resolve_namespace(duthost, port) + + window_desc = f"{elapsed_sec}s window" if elapsed_sec is not None else "observation window" + + per_port = {} + for port in ports: + baseline_flap, baseline_up = sentinels.get(port, (None, None)) + port_table = db_helpers.hgetall_dict( + duthost, "APPL_DB", f"PORT_TABLE:{port}", namespace=_namespace_for(port) + ) + current_flap = port_table.get("flap_count") + current_up = port_table.get("last_up_time") + + if baseline_flap is None and baseline_up is None: + details = ( + f"{port}: no flap_count/last_up_time sentinel captured - " + "cannot verify stability (schema mismatch or partial publish)" + ) + logger.warning("Stability check FAILED: %s", details) + per_port[port] = {"passed": False, "details": details} + elif current_flap != baseline_flap or current_up != baseline_up: + details = ( + f"{port}: flap detected during {window_desc} " + f"(flap_count {baseline_flap}->{current_flap}, " + f"last_up_time {baseline_up}->{current_up})" + ) + logger.warning("Stability check FAILED: %s", details) + per_port[port] = {"passed": False, "details": details} + else: + details = f"{port}: stable for {window_desc} (flap_count={current_flap}, last_up_time={current_up})" + logger.info("Stability check PASSED: %s", details) + per_port[port] = {"passed": True, "details": details} + return per_port + + +def check_ports_stability(duthost, ports, window_sec, namespaces=None): + """Verify no port in ``ports`` flaps over one shared post-recovery observation window. + + Why: standalone implementation of the mandatory "Stability (always)" + sub-check from system_test_plan.md, for callers with no other work to + overlap the observation window with (composes + :func:`capture_flap_sentinels` + sleep + :func:`assert_no_flap_since`). + + Args: + duthost: SONiC DUT host fixture. + ports: list of logical interface names. + window_sec: shared observation window, in seconds. + namespaces: optional dict of ``{port: namespace}``. + + Returns: + dict: ``{port: {'passed': bool, 'details': str}}``, one entry per ``ports``. + """ + sentinels = capture_flap_sentinels(duthost, ports, namespaces=namespaces) + time.sleep(window_sec) + return assert_no_flap_since(duthost, ports, sentinels, namespaces=namespaces, elapsed_sec=window_sec) + + +# ────────────────────────────────────────────────────────────────────── +# Standard Port Recovery and Verification Procedure +# (see docs/testplan/transceiver/system_test_plan.md) +# ────────────────────────────────────────────────────────────────────── + + +# TRANSCEIVER_STATUS field names, verified against a live DUT +# (``redis-cli -n 6 hgetall 'TRANSCEIVER_STATUS|Ethernet0'``) and against the +# xcvrd writer - CmisApi.get_transceiver_status() in sonic-platform-common +# (sonic_platform_base/sonic_xcvr/api/public/cmis.py): datapath state is +# published as "DPState" (value "DataPathActivated"), config state as +# "config_state_hostlane" (value "ConfigSuccess") - NOT +# "host_lane_datapath_state"/"host_lane_config_state". +_CMIS_DATAPATH_STATE_RE = re.compile(r'^DP(\d+)State$') +_CMIS_CONFIG_STATE_RE = re.compile(r'^config_state_hostlane(\d+)$') + + +def check_cmis_state(duthost, ports, lport_to_first_subport_mapping, namespaces=None): + """Verify CMIS DataPathState=DataPathActivated and ConfigState=ConfigSuccess, for every port in ``ports``. + + Why: ``TRANSCEIVER_STATUS`` is published once per physical module (under + the first sub-port of a breakout group) and carries every host lane of + the module, so a breakout sub-port must be checked only against its own + active lanes - not a sibling's - to avoid a false pass/fail; this also + batches the underlying DB reads per namespace instead of per port. + + Args: + duthost: SONiC DUT host fixture. + ports: list of logical interface names. + lport_to_first_subport_mapping: the value of the session-scoped + fixture of the same name (``tests/transceiver/conftest.py``). + namespaces: optional dict of ``{port: namespace}``. + + Returns: + dict: ``{port: {'passed': bool, 'details': str}}``, one entry per ``ports``. + """ + if namespaces is None: + namespaces = {} + + def _namespace_for(port): + if port in namespaces: + return namespaces[port] + return db_helpers.resolve_namespace(duthost, port) + + ports_by_ns = {} + for port in ports: + ports_by_ns.setdefault(_namespace_for(port), []).append(port) + + status_by_parent = {} + port_table_by_port = {} + for ns in ports_by_ns: + status_dump, status_err = db_helpers.get_state_db_table(duthost, "TRANSCEIVER_STATUS", namespace=ns) + if status_err is None: + status_by_parent.update(status_dump) + port_table_dump, port_table_err = db_helpers.get_db_table( + duthost, "APPL_DB", "PORT_TABLE", namespace=ns, sep=":" + ) + if port_table_err is None: + port_table_by_port.update(port_table_dump) + + per_port = {} + for port in ports: + parent = lport_to_first_subport_mapping.get(port, port) + status = status_by_parent.get(parent) + if not status: + per_port[port] = { + "passed": False, + "details": f"{port}: TRANSCEIVER_STATUS|{parent} missing or empty", + } + continue + + port_table = port_table_by_port.get(port, {}) + lanes_field = port_table.get("lanes") + if not lanes_field: + per_port[port] = { + "passed": False, + "details": ( + f"{port}: PORT_TABLE:{port} has no 'lanes' field - " + "cannot determine this port's active host lanes" + ), + } + continue + host_lane_count = len(lanes_field.split(",")) + subport = int(port_table.get("subport") or 1) + lane_start = host_lane_count * max(0, subport - 1) + active_lanes = set(range(lane_start + 1, lane_start + host_lane_count + 1)) + + bad_datapath = [] + bad_config = [] + datapath_fields_seen = 0 + config_fields_seen = 0 + for k, v in status.items(): + datapath_match = _CMIS_DATAPATH_STATE_RE.match(k) + if datapath_match: + if int(datapath_match.group(1)) not in active_lanes: + continue + datapath_fields_seen += 1 + if v != "DataPathActivated": + bad_datapath.append(f"{k}={v}") + continue + config_match = _CMIS_CONFIG_STATE_RE.match(k) + if config_match: + if int(config_match.group(1)) not in active_lanes: + continue + config_fields_seen += 1 + if v != "ConfigSuccess": + bad_config.append(f"{k}={v}") + + if datapath_fields_seen == 0 and config_fields_seen == 0: + per_port[port] = { + "passed": False, + "details": ( + f"{port} (parent {parent}) TRANSCEIVER_STATUS|{parent} has no " + f"DPState or config_state_hostlane fields for this port's " + f"active host lanes {sorted(active_lanes)} - cannot confirm CMIS " + "state (schema mismatch, partial publish, or lane-range mismatch)" + ), + } + continue + + if bad_datapath or bad_config: + problems = [] + if bad_datapath: + problems.append("datapath: " + ", ".join(bad_datapath)) + if bad_config: + problems.append("config: " + ", ".join(bad_config)) + per_port[port] = { + "passed": False, + "details": f"{port} (parent {parent}) CMIS state NOT activated - " + "; ".join(problems), + } + continue + + per_port[port] = { + "passed": True, + "details": f"{port} (parent {parent}) CMIS DataPathActivated + ConfigSuccess", + } + return per_port + + +def standard_port_recovery_and_verification( + duthost, ports, port_attributes_dict, link_up_timeout_sec, health_baseline, + lport_to_first_subport_mapping, + stability_window_sec=DEFAULT_STABILITY_WINDOW_SEC, + expected_pid_changes=None, + flap_count_baseline=None, + assert_no_flap_across_op=False, +): + """Run the Standard Port Recovery and Verification Procedure on a batch of ports. + + Why: implements the common post-recovery verification steps + (link status, flap/stability, LLDP, CMIS state, docker/process health) + that every disruptive Link Behavior System test needs to run afterward, + batched across ``ports`` so fixed per-call costs aren't multiplied by + port count and every port's failures are surfaced in one call. + + Args: + duthost: SONiC DUT host fixture. + ports: list of logical interface names to validate. + port_attributes_dict: dict of ``{port: port_attrs}`` (as produced by + the ``port_attributes_dict`` fixture), with one entry per port in + ``ports``. + link_up_timeout_sec: budget for waiting on oper-up + health_baseline: the dict returned by + :func:`tests.transceiver.common.health_checks.capture_baseline` + lport_to_first_subport_mapping: the value of the session-scoped + fixture of the same name (``tests/transceiver/conftest.py``). + stability_window_sec: shared post-recovery observation window + (seconds) for the stability sub-check. + expected_pid_changes: set of monitored process names whose PID is + expected to differ from ``health_baseline`` in the health check. + flap_count_baseline: optional dict of ``{port: flap_count}`` + assert_no_flap_across_op: whether to additionally assert no flap + occurred across the whole operation (only valid where the link + stays up and the flap counter survives, e.g. xcvrd/pmon restart). + + Returns: + dict: ``{'passed': bool, 'per_port': {port: {'passed': bool, 'details': str}}, 'details': str}`` + """ + # Owning ASIC namespace per port, resolved once and reused for every + # per-namespace DB read below (LLDP / TRANSCEIVER_STATUS / stability). + # ``None`` on single-ASIC -> no ``-n`` flag. + namespaces = {port: db_helpers.resolve_namespace(duthost, port) for port in ports} + + per_port_failures = {port: [] for port in ports} + checks_ran = {port: [] for port in ports} # human-readable checks that ran, per port + + # 1. Link status - one batched poll covers every port. + down_ports = wait_ports_oper_status(duthost, ports, "up", link_up_timeout_sec) + for port in ports: + checks_ran[port].append("link up") + for port in down_ports: + per_port_failures[port].append(f"port {port} did not reach oper-up within {link_up_timeout_sec}s") + + up_ports = [port for port in ports if port not in down_ports] + + # 2a/2b setup - one shared flap/last_up_time sentinel per up port, captured + # right after link-up. + recovery_t0 = time.monotonic() + post_recovery_sentinels = capture_flap_sentinels(duthost, up_ports, namespaces=namespaces) if up_ports else {} + + # 2b. No flap across the operation - only when assert_no_flap_across_op is + # set (flap counter must survive the op, e.g. xcvrd/pmon restart). + if assert_no_flap_across_op: + for port in up_ports: + checks_ran[port].append("no-flap-across-op") + baseline_flap = (flap_count_baseline or {}).get(port) + current_flap, _current_up = post_recovery_sentinels.get(port, (None, None)) + if baseline_flap is None or current_flap is None: + per_port_failures[port].append( + f"{port}: cannot assert across-op no-flap - flap_count baseline/current missing" + ) + elif current_flap != baseline_flap: + per_port_failures[port].append( + f"{port}: flapped across operation (flap_count {baseline_flap} -> {current_flap})" + ) + + # 3. LLDP - only for up ports that request it (otherwise LLDP is moot); + # per-port timeouts honored, polls interleaved across the batch. + lldp_port_timeouts = {} + for port in up_ports: + sys_attrs = port_attributes_dict.get(port, {}).get(SYSTEM_ATTRIBUTES_KEY, {}) + if sys_attrs.get("verify_lldp_on_link_up", True): + if "lldp_neighbor_wait_sec" not in sys_attrs: + raise ValueError( + f"{port}: 'lldp_neighbor_wait_sec' is not defined in SYSTEM_ATTRIBUTES " + "(system.json 'defaults', or a more specific override) - required " + "whenever verify_lldp_on_link_up is True" + ) + lldp_port_timeouts[port] = sys_attrs["lldp_neighbor_wait_sec"] + if lldp_port_timeouts: + lldp_results = check_lldp_neighbors_present( + duthost, lldp_port_timeouts, namespaces=namespaces + ) + for port, result in lldp_results.items(): + checks_ran[port].append("LLDP") + if not result["passed"]: + per_port_failures[port].append(result["details"]) + + # 5. CMIS state - only for up, CMIS active-optical ports, in one batched + # check_cmis_state call. + cmis_active_ports = [ + port for port in up_ports + if is_cmis_active_optical(port_attributes_dict.get(port, {}).get(EEPROM_ATTRIBUTES_KEY, {})) + ] + if cmis_active_ports: + cmis_results = check_cmis_state( + duthost, cmis_active_ports, lport_to_first_subport_mapping, namespaces=namespaces + ) + for port, result in cmis_results.items(): + checks_ran[port].append("CMIS state") + if not result["passed"]: + per_port_failures[port].append(result["details"]) + + # 2a end-gate. Mandatory stability sub-check, only for up ports. Asserted + # here (after steps 3/5) so the window overlaps that work instead of a + # leading sleep; tops up to stability_window_sec if the rest ran short. + if up_ports: + elapsed = time.monotonic() - recovery_t0 + time.sleep(max(0, stability_window_sec - elapsed)) + stability_results = assert_no_flap_since( + duthost, up_ports, post_recovery_sentinels, namespaces=namespaces, + elapsed_sec=stability_window_sec, + ) + for port, result in stability_results.items(): + checks_ran[port].append("stability") + if not result["passed"]: + per_port_failures[port].append(result["details"]) + + # 7. Docker/process health - delegates to health_checks.verify_health + # against the same health_baseline the per-test fixture uses. Runs + # once, host-wide, unconditionally. + if health_baseline is None: + health_failure = ( + "health_baseline not provided - caller must pass the 'health_baseline' " + "pytest fixture value (tests/transceiver/conftest.py)" + ) + else: + health_result = health_checks.verify_health( + duthost, health_baseline, expect_pid_change=expected_pid_changes, + ) + health_failure = None if health_result["passed"] else "; ".join(health_result["failures"]) + for port in ports: + checks_ran[port].append("health") + if health_failure is not None: + per_port_failures[port].append(f"health: {health_failure}") + + per_port = {} + overall_passed = True + for port in ports: + failures = per_port_failures[port] + if failures: + overall_passed = False + details = f"{port}: " + "; ".join(failures) + logger.warning("Standard Port Recovery FAILED: %s", details) + else: + details = f"{port}: " + " + ".join(checks_ran[port]) + " all OK" + logger.info("Standard Port Recovery PASSED: %s", details) + per_port[port] = {"passed": not failures, "details": details} + + return { + "passed": overall_passed, + "per_port": per_port, + "details": "; ".join(per_port[port]["details"] for port in ports), + } \ No newline at end of file diff --git a/tests/transceiver/system/process_restart/__init__.py b/tests/transceiver/system/process_restart/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/transceiver/system/process_restart/conftest.py b/tests/transceiver/system/process_restart/conftest.py new file mode 100644 index 00000000000..8b4384040f2 --- /dev/null +++ b/tests/transceiver/system/process_restart/conftest.py @@ -0,0 +1,14 @@ +"""Process Restart category conftest. +Opts the Process Restart test category into the cross-category session-level +prerequisites defined in ``tests/transceiver/conftest.py``. +Per the prerequisite matrix in ``docs/testplan/transceiver/test_plan.md``, +Process Restart consumes the ``presence_verified, gold_fw_verified, links_verified`` gates. +""" + +import pytest + + +# Opt into the cross-category session gates this category consumes. +@pytest.fixture(autouse=True, scope="session") +def _category_session_prerequisites(presence_verified, gold_fw_verified, links_verified): + return \ No newline at end of file diff --git a/tests/transceiver/system/process_restart/test_pmon_restart.py b/tests/transceiver/system/process_restart/test_pmon_restart.py new file mode 100644 index 00000000000..32f0704a7e5 --- /dev/null +++ b/tests/transceiver/system/process_restart/test_pmon_restart.py @@ -0,0 +1,111 @@ +"""System / Process Restart - pmon daemon restart validation. + +Implements the pmon restart test from + ``docs/testplan/transceiver/system_test_plan.md``. + +Execution order:: + + session start + `- check_links_up() <- session-scoped via + ``links_verified`` in + tests/transceiver/conftest.py + (failure skips every System test) + `- test_system_pmon_restart + |- : restart pmon -> verify all ports recovery + session end + `- _system_post_session_checks (system/conftest.py) + |- post_state_restoration() + `- final link + LLDP check + +Failure handling: failures are accumulated per test case and reported in a +single ``pytest.fail`` at the end, so a single run surfaces all issues across +all ports. +""" +import logging +import time +import pytest + +from tests.transceiver.attribute_parser.attribute_keys import ( + SYSTEM_ATTRIBUTES_KEY +) +from tests.transceiver.common.health_checks import capture_baseline +from tests.transceiver.common.prerequisites import ( + check_links_up +) +from tests.transceiver.common.verification import ( + standard_port_recovery_and_verification +) +from tests.common.helpers.sonic_db import AppDbCli as sdbHelp + +logger = logging.getLogger(__name__) + + +@pytest.mark.disable_loganalyzer +def test_system_pmon_restart( + duthost, port_attributes_dict, expected_pid_changes, + lport_to_first_subport_mapping, +): + """Restart pmon and verify all ports recover cleanly. + + Implements the test described in + ``docs/testplan/transceiver/system_test_plan.md``. + + Simple pmon restart: + 1. Verify all ports are operationally up and record link up time + 2. Restart pmon using 'sudo systemctl restart pmon' + 3. Wait for pmon_restart_settle_sec + 4. Execute Standard Port Recovery and Verification Procedure + for all ports + 5. Verify pmon has been running for at least pmon_restart_settle_sec + """ + expected_pid_changes.add("xcvrd") + ports = sorted(port_attributes_dict.keys()) + assert ports, "port_attributes_dict is empty - nothing to validate" + health_baseline = capture_baseline(duthost) + failures = [] + + logger.info("Recording link states and uptime for %d port(s)", len(ports)) + if not check_links_up(duthost, port_attributes_dict): + logger.warning("Validation on Start FAILED: some ports are down") + else: + appl_db = sdbHelp(duthost) + for port in ports: + logger.info( + "Recording initial link uptime: %s", + appl_db.hget_key_value( + "PORT_TABLE:{}".format(port), "last_up_time" + ), + ) + + logger.info("Restarting pmon...") + duthost.restart_service('pmon', 'pmon') + pmon_wait = port_attributes_dict[ports[0]].get( + SYSTEM_ATTRIBUTES_KEY, + {} + ).get( + "pmon_restart_settle_sec", + 120 + ) + + # accounts for minimum timeout behavior of SPRaV + time.sleep(pmon_wait + 60) + + # Wait for settle time and verify + result = standard_port_recovery_and_verification( + duthost, ports, port_attributes_dict, + link_up_timeout_sec=pmon_wait, + health_baseline=health_baseline, + lport_to_first_subport_mapping=lport_to_first_subport_mapping, + expected_pid_changes=expected_pid_changes, + ) + if not result["passed"]: + failures.append(f"[startup] {result['details']}") + logger.warning("Startup validation FAILED: %s", result["details"]) + else: + logger.info("Startup validation PASSED: %s", result["details"]) + + if failures: + pytest.fail( + f"pmon restart recovery FAILED on {len(failures)} port(s): \n - " + + "\n - ".join(failures) + ) diff --git a/tests/transceiver/system/process_restart/test_swss_restart.py b/tests/transceiver/system/process_restart/test_swss_restart.py new file mode 100644 index 00000000000..7ac427c5cf5 --- /dev/null +++ b/tests/transceiver/system/process_restart/test_swss_restart.py @@ -0,0 +1,121 @@ +"""System / Process Restart - swss daemon restart validation. + +Implements the swss restart test from + ``docs/testplan/transceiver/system_test_plan.md``. + +Execution order:: + + session start + `- check_links_up() <- session-scoped via + ``links_verified`` in + tests/transceiver/conftest.py + (failure skips every System test) + `- test_system_swss_restart + |- : restart swss -> verify pmon restart (if expected) + `- -> verify all ports recovery + session end + `- _system_post_session_checks (system/conftest.py) + |- post_state_restoration() + `- final link + LLDP check + +Failure handling: failures are accumulated per test case and reported in a +single ``pytest.fail`` at the end, so a single run surfaces all issues across +all ports. +""" +import logging +import time +import pytest + +from tests.transceiver.attribute_parser.attribute_keys import ( + SYSTEM_ATTRIBUTES_KEY +) +from tests.transceiver.common.health_checks import capture_baseline +from tests.transceiver.common.prerequisites import ( + check_links_up +) +from tests.transceiver.common.verification import ( + standard_port_recovery_and_verification +) +from tests.common.helpers.sonic_db import AppDbCli as sdbHelp +from tests.common.platform.processes_utils import check_process_up + +logger = logging.getLogger(__name__) + + +@pytest.mark.disable_loganalyzer +def test_system_swss_restart( + duthost, port_attributes_dict, expected_pid_changes, + lport_to_first_subport_mapping, +): + """Restart swss and verify all ports recover cleanly. + + See the module docstring for the full execution tree. Steps: + + * verify all ports are oper-up and record link-up timestamps, + * restart swss via ``sudo systemctl restart swss``, + * if ``expect_pmon_restart_with_swss_or_syncd`` is set, verify pmon + restarted as expected, + * wait for ``swss_restart_settle_sec`` then run Standard Port Recovery + and Verification for every port. + + All (port, step) failures are accumulated and reported in a single + ``pytest.fail`` so one run surfaces every issue. + """ + expected_pid_changes.add("xcvrd") + ports = sorted(port_attributes_dict.keys()) + assert ports, "port_attributes_dict is empty - nothing to validate" + health_baseline = capture_baseline(duthost) + failures = [] + + logger.info("Recording link states and uptime for %d port(s)", len(ports)) + if not check_links_up(duthost, port_attributes_dict): + logger.warning("Validation on Start FAILED: some ports are down") + else: + appl_db = sdbHelp(duthost) + for port in ports: + logger.info( + "Recording initial link uptime: %s", + appl_db.hget_key_value( + "PORT_TABLE:{}".format(port), "last_up_time" + ), + ) + + logger.info("Restarting swss...") + duthost.restart_service('swss', 'swss') + swss_wait = port_attributes_dict[ports[0]].get( + SYSTEM_ATTRIBUTES_KEY, {} + ).get("swss_restart_settle_sec", 180) + time.sleep(swss_wait) + + # Check whether pmon restarted alongside swss + if port_attributes_dict[ports[0]].get( + SYSTEM_ATTRIBUTES_KEY, {} + ).get("expect_pmon_restart_with_swss_or_syncd", False): + time.sleep(15) + logger.info("Verifying pmon restart after swss restart...") + if check_process_up(duthost, 'pmon'): + failures.append("[pmon] pmon did not restart as expected") + logger.warning( + "pmon FAILED to Restart when" + " expect_pmon_restart_with_swss_or_syncd is True" + ) + + # Wait for settle time and verify + result = standard_port_recovery_and_verification( + duthost, ports, port_attributes_dict, + link_up_timeout_sec=swss_wait, + health_baseline=health_baseline, + lport_to_first_subport_mapping=lport_to_first_subport_mapping, + expected_pid_changes=expected_pid_changes, + ) + if not result["passed"]: + failures.append(f"[startup] {result['details']}") + logger.warning("Startup validation FAILED: %s", result["details"]) + else: + logger.info("Startup validation PASSED: %s", result["details"]) + + if failures: + pytest.fail( + f"swss restart recovery FAILED on {len(failures)} port(s):\n - " + + "\n - ".join(failures) + ) diff --git a/tests/transceiver/system/process_restart/test_syncd_restart.py b/tests/transceiver/system/process_restart/test_syncd_restart.py new file mode 100644 index 00000000000..6cb847e1c5d --- /dev/null +++ b/tests/transceiver/system/process_restart/test_syncd_restart.py @@ -0,0 +1,125 @@ +"""System / Process Restart - syncd daemon restart validation. + +Implements the syncd restart test from + ``docs/testplan/transceiver/system_test_plan.md``. + +Execution order:: + + session start + `- check_links_up() <- session-scoped via + ``links_verified`` in + tests/transceiver/conftest.py + (failure skips every System test) + `- test_system_syncd_restart + |- : restart syncd -> verify pmon restart (if expected) + `- -> verify all ports recover + session end + `- _system_post_session_checks (system/conftest.py) + |- post_state_restoration() + `- final link + LLDP check + +Failure handling: failures are accumulated per test case and reported in a +single ``pytest.fail`` at the end, so a single run surfaces all issues across +all ports. +""" +import logging +import time +import pytest + +from tests.transceiver.attribute_parser.attribute_keys import ( + SYSTEM_ATTRIBUTES_KEY +) +from tests.transceiver.common.health_checks import capture_baseline +from tests.transceiver.common.prerequisites import ( + check_links_up +) +from tests.transceiver.common.verification import ( + standard_port_recovery_and_verification +) +from tests.common.helpers.sonic_db import AppDbCli as sdbHelp +from tests.common.platform.processes_utils import check_process_up + +logger = logging.getLogger(__name__) + + +@pytest.mark.disable_loganalyzer +def test_system_syncd_restart( + duthost, port_attributes_dict, expected_pid_changes, + lport_to_first_subport_mapping, +): + """Restart syncd and verify all ports recover cleanly. + + See the module docstring for the full execution tree. Steps: + + * verify all ports are oper-up and record link-up timestamps, + * restart syncd via ``sudo systemctl restart syncd``, + * if ``expect_pmon_restart_with_swss_or_syncd`` is set, verify pmon + restarted as expected, + * wait for ``syncd_restart_settle_sec`` then run Standard Port Recovery + and Verification for every port. + + All (port, step) failures are accumulated and reported in a single + ``pytest.fail`` so one run surfaces every issue. + """ + expected_pid_changes.add("xcvrd") + ports = sorted(port_attributes_dict.keys()) + assert ports, "port_attributes_dict is empty - nothing to validate" + failures = [] + health_baseline = capture_baseline(duthost) + + logger.info("Recording link states and uptime for %d port(s)", len(ports)) + if not check_links_up(duthost, port_attributes_dict): + logger.warning("Validation on Start FAILED: some ports are down") + else: + appl_db = sdbHelp(duthost) + for port in ports: + logger.info( + "Recording initial link uptime: %s", + appl_db.hget_key_value( + "PORT_TABLE:{}".format(port), "last_up_time" + ), + ) + + logger.info("Restarting syncd...") + duthost.restart_service("syncd", "syncd") + syncd_wait = port_attributes_dict[ports[0]].get( + SYSTEM_ATTRIBUTES_KEY, {} + ).get("syncd_restart_settle_sec", 240) + time.sleep(syncd_wait) + + # Check whether pmon restarted alongside syncd + if port_attributes_dict[ports[0]].get( + SYSTEM_ATTRIBUTES_KEY, {} + ).get("expect_pmon_restart_with_swss_or_syncd", False): + time.sleep(15) + logger.info("Verifying pmon restart after syncd restart...") + if check_process_up(duthost, 'pmon'): + failures.append("[pmon] pmon did not restart as expected") + logger.warning( + "pmon FAILED to restart when" + " expect_pmon_restart_with_swss_or_syncd is True" + ) + + # Wait for settle time and verify + result = standard_port_recovery_and_verification( + duthost, ports, port_attributes_dict, + link_up_timeout_sec=syncd_wait, + health_baseline=health_baseline, + lport_to_first_subport_mapping=lport_to_first_subport_mapping, + expected_pid_changes=expected_pid_changes, + ) + if not result["passed"]: + failures.append(f"[startup] {result['details']}") + logger.warning( + "Post-restart validation FAILED: %s", result["details"] + ) + else: + logger.info( + "Post-restart validation PASSED: %s", result["details"] + ) + + if failures: + pytest.fail( + f"syncd restart recovery FAILED on {len(failures)} port(s):\n - " + + "\n - ".join(failures) + ) diff --git a/tests/transceiver/system/process_restart/test_xcvrd_restart.py b/tests/transceiver/system/process_restart/test_xcvrd_restart.py index 0980b7d4e6a..fb3a656ec56 100644 --- a/tests/transceiver/system/process_restart/test_xcvrd_restart.py +++ b/tests/transceiver/system/process_restart/test_xcvrd_restart.py @@ -10,36 +10,40 @@ `- check_links_up() <- session-scoped via ``links_verified`` in tests/transceiver/conftest.py - (failure skips every System test) + (failure skips every + System test) `- test_system_xcvrd_restart |- run_pre_check (xcvrd RUNNING) <- _per_test_health_check |- : restart xcvrd -> verify all ports recovery - `- run_post_check - `- test_system_xcvrd_restart_with_i2c_errors - NOTE: this test has been skipped during initial development due to library issues + `- run_post_check + `- test_system_xcvrd_restart_with_i2c_errors + NOTE: this test has been skipped during initial development + due to library issues |- run_pre_check (xcvrd RUNNING) <- _per_test_health_check |- : induce I2C errors -> restart xcvrd -> verify recovery - `- run_post_check + `- run_post_check `- test_system_xcvrd_crash_recovery |- run_pre_check (xcvrd RUNNING) <- _per_test_health_check |- : inject crash -> monitor restart -> verify recovery - `- run_post_check + `- run_post_check session end `- _system_post_session_checks (system/conftest.py) |- post_state_restoration() |- STATE_DB consistency check `- final link + LLDP check -Failure handling: failures are accumulated per test case and reported in a single -pytest.fail at the end, so a single run surfaces all issues across all ports. +Failure handling: failures are accumulated per test case and reported in +a single pytest.fail at the end, so a single run surfaces all issues +across all ports. """ import logging import time -# import re -# import time + import pytest -# from tests.transceiver.attribute_parser.attribute_keys import SYSTEM_ATTRIBUTES_KEY +from tests.transceiver.attribute_parser.attribute_keys import ( + SYSTEM_ATTRIBUTES_KEY +) from tests.transceiver.common.health_checks import ( capture_baseline ) @@ -49,13 +53,16 @@ from tests.transceiver.common.verification import ( standard_port_recovery_and_verification ) -import tests.transceiver.common.process_restart_helpers as prh from tests.common.helpers.dut_utils import get_program_info logger = logging.getLogger(__name__) + @pytest.mark.disable_loganalyzer -def test_system_xcvrd_restart(duthost, port_attributes_dict, expected_pid_changes): +def test_system_xcvrd_restart( + duthost, port_attributes_dict, expected_pid_changes, + lport_to_first_subport_mapping, +): """Restart xcvrd and verify all ports recover cleanly. See the module docstring for the full execution tree. Steps: @@ -71,41 +78,58 @@ def test_system_xcvrd_restart(duthost, port_attributes_dict, expected_pid_change expected_pid_changes.add("xcvrd") ports = sorted(port_attributes_dict.keys()) assert ports, "port_attributes_dict is empty - nothing to validate" - xcvrd_wait = prh.sys_attr(port_attributes_dict[ports[0]], "xcvrd_restart_settle_sec", 120) + xcvrd_wait = port_attributes_dict[ports[0]].get( + SYSTEM_ATTRIBUTES_KEY, {} + ).get("xcvrd_restart_settle_sec", 120) health_baseline = capture_baseline(duthost) failures = [] # collected across every (port, step) tuple logger.info("Recording link states and uptime for %d port(s)", len(ports)) - logger.info("Recording initialXcvrD uptime: %s", prh.get_xcvrd_uptime(duthost)) + status, pid, uptime = get_program_info( + duthost, 'pmon', 'xcvrd', include_uptime=True + ) + logger.info( + "Recording initial xcvrd status: %s (pid %s, uptime %s)", + status, pid, uptime, + ) link_check = check_links_up(duthost, port_attributes_dict) if not link_check["passed"]: logger.warning("Validation on Start FAILED: some ports are down") logger.info("Restarting xcvrd...") - prh.restart_process(duthost, 'xcvrd') + duthost.stop_pmon_daemon_service('xcvrd') + duthost.start_pmon_daemon('xcvrd') time.sleep(xcvrd_wait) - - logger.info("Running Standard Port Recovery and Verification for %d port(s)", len(ports)) + + logger.info( + "Running Standard Port Recovery and Verification for %d port(s)", + len(ports), + ) result = standard_port_recovery_and_verification( - duthost, ports, port_attributes_dict, - link_up_timeout_sec=xcvrd_wait, + duthost, ports, port_attributes_dict, + link_up_timeout_sec=xcvrd_wait, health_baseline=health_baseline, - shared_state=None, - expected_pid_changes='xcvrd' + lport_to_first_subport_mapping=lport_to_first_subport_mapping, + expected_pid_changes=expected_pid_changes, ) if not result["passed"]: failures.append(f"[post-restart] {result['details']}") logger.warning("Post-restart validation FAILED: %s", result["details"]) else: logger.info("Post-restart validation PASSED: %s", result["details"]) - + if failures: pytest.fail( - f"TC1: xcvrd restart recovery FAILED on {len(failures)} port(s):\n - " + f"TC1: xcvrd restart recovery FAILED on {len(failures)} " + "port(s):\n - " + "\n - ".join(failures) ) + @pytest.mark.disable_loganalyzer -def test_system_xcvrd_crash_recovery(duthost, port_attributes_dict, expected_pid_changes): +def test_system_xcvrd_crash_recovery( + duthost, port_attributes_dict, expected_pid_changes, + lport_to_first_subport_mapping, +): """Inject an xcvrd crash and verify automatic restart and port recovery. See the module docstring for the full execution tree. Steps: @@ -122,38 +146,49 @@ def test_system_xcvrd_crash_recovery(duthost, port_attributes_dict, expected_pid expected_pid_changes.add("xcvrd") ports = sorted(port_attributes_dict.keys()) assert ports, "port_attributes_dict is empty - nothing to validate" - xcvrd_wait = prh.sys_attr(port_attributes_dict[ports[0]], "xcvrd_restart_settle_sec", 120) + xcvrd_wait = port_attributes_dict[ports[0]].get( + SYSTEM_ATTRIBUTES_KEY, {} + ).get("xcvrd_restart_settle_sec", 120) health_baseline = capture_baseline(duthost) failures = [] # collected across every (port, step) tuple - logger.info("Recording initial link states for %d port(s)", len(ports)) - logger.info("Recording initial XcvrD uptime: %s", prh.get_xcvrd_uptime(duthost)) + logger.info("Recording initial link states for %d port(s)", len(ports)) + status, pid, uptime = get_program_info( + duthost, 'pmon', 'xcvrd', include_uptime=True + ) + logger.info( + "Recording initial xcvrd status: %s (pid %s, uptime %s)", + status, pid, uptime, + ) link_check = check_links_up(duthost, port_attributes_dict) if not link_check["passed"]: logger.warning("Validation on Start FAILED: some ports are down") - + logger.info("Using SIGKILL to crash xcvrd") status, pid = get_program_info(duthost, 'pmon', 'xcvrd') duthost.kill_pmon_daemon_pid_w_sig(pid, "-9") - + # Wait, then run Standard Port Recovery and Verification for all ports time.sleep(xcvrd_wait) - logger.info("Running Standard Port Recovery and Verification for %d port(s)", len(ports)) + logger.info( + "Running Standard Port Recovery and Verification for %d port(s)", + len(ports), + ) result = standard_port_recovery_and_verification( - duthost, ports, port_attributes_dict, - link_up_timeout_sec=xcvrd_wait, + duthost, ports, port_attributes_dict, + link_up_timeout_sec=xcvrd_wait, health_baseline=health_baseline, - shared_state=None, - expected_pid_changes='xcvrd' + lport_to_first_subport_mapping=lport_to_first_subport_mapping, + expected_pid_changes=expected_pid_changes, ) if not result["passed"]: failures.append(f"[post-crash] {result['details']}") logger.warning("Post-crash validation FAILED: %s", result["details"]) else: logger.info("Post-crash validation PASSED: %s", result["details"]) - + if failures: pytest.fail( f"xcvrd crash recovery FAILED on {len(failures)} port(s):\n - " + "\n - ".join(failures) - ) \ No newline at end of file + ) From 2a78003c225011f995f0ac1a58d47f3661db6f26 Mon Sep 17 00:00:00 2001 From: sutter Date: Wed, 5 Aug 2026 11:08:49 -0700 Subject: [PATCH 04/16] Added optional flag to return uptime in get_program_info Signed-off-by: sutter --- tests/common/helpers/dut_utils.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/tests/common/helpers/dut_utils.py b/tests/common/helpers/dut_utils.py index e6c9ef38343..453b948ea22 100644 --- a/tests/common/helpers/dut_utils.py +++ b/tests/common/helpers/dut_utils.py @@ -238,7 +238,9 @@ def get_group_program_info(duthost, container_name, group_name): return group_program_info -def get_program_info(duthost, container_name, program_name): +def get_program_info( + duthost, container_name, program_name, include_uptime=False +): """Gets program running status and its pid by analyzing the command output of "docker exec supervisorctl status" @@ -246,26 +248,39 @@ def get_program_info(duthost, container_name, program_name): duthost: Hostname of DUT. container_name: A string shows container name. program_name: A string shows process name. + include_uptime: When True, also return the uptime field supervisorctl + reports for a RUNNING program (e.g. "0:12:34", or "37 days, + 17:55:12" past the first day). Defaults to False so existing + callers keep unpacking a 2-tuple unchanged. Return: - Program running status and its pid. + Program running status and its pid. When include_uptime is True, a + third value (uptime string, or None if not RUNNING) is also returned. """ program_status = None program_pid = -1 + program_uptime = None program_list = duthost.shell("docker exec {} supervisorctl status" .format(container_name), module_ignore_errors=True) for program_info in program_list["stdout_lines"]: if program_info.find(program_name) != -1: - program_status = program_info.split()[1].strip() + fields = program_info.split() + program_status = fields[1].strip() if program_status == "RUNNING": - program_pid = int(program_info.split()[3].strip(',')) + program_pid = int(fields[3].strip(',')) + if "uptime" in fields: + program_uptime = " ".join( + fields[fields.index("uptime") + 1:] + ) break if program_pid != -1: logger.info("Found program '{}' in the '{}' state with pid {}" .format(program_name, program_status, program_pid)) + if include_uptime: + return program_status, program_pid, program_uptime return program_status, program_pid From e738a54925f87060ca549da737ab900c94adc0cb Mon Sep 17 00:00:00 2001 From: sutter Date: Wed, 5 Aug 2026 15:21:35 -0700 Subject: [PATCH 05/16] SPRaV streamlining and flake8 fixes Signed-off-by: sutter --- tests/transceiver/common/verification.py | 321 ++++++++++++----------- 1 file changed, 174 insertions(+), 147 deletions(-) diff --git a/tests/transceiver/common/verification.py b/tests/transceiver/common/verification.py index 2c5172167d0..915ee0f3d02 100644 --- a/tests/transceiver/common/verification.py +++ b/tests/transceiver/common/verification.py @@ -1,7 +1,7 @@ """Standard Port Recovery and Verification Procedures. Implements the Standard Port and Verification function, -as well as the related child functions. All parent and +as well as the related child functions. All parent and child functions will return an output following the format: dict: ``{port: {'passed': bool, 'details': str}}`` @@ -12,32 +12,19 @@ import time from tests.common.platform.interface_utils import wait_ports_oper_status -from tests.transceiver.attribute_parser.attribute_keys import ( - EEPROM_ATTRIBUTES_KEY, - SYSTEM_ATTRIBUTES_KEY, -) from tests.transceiver.common import db_helpers, health_checks -from tests.transceiver.common.eeprom_decode import is_cmis_active_optical logger = logging.getLogger(__name__) -# Post-recovery observation window (seconds) for the mandatory Link -# Flap/Stability "Stability (always)" sub-check in system_test_plan.md, which -# does not pin an exact duration ("a short post-recovery observation window"). -# Kept small since this runs once per port on every Standard Port Recovery -# call. DEFAULT_STABILITY_WINDOW_SEC = 5 - - -# ────────────────────────────────────────────────────────────────────── -# LLDP neighbor poll -# ────────────────────────────────────────────────────────────────────── - _LLDP_POLL_INTERVAL_SEC = 3 +_CMIS_DATAPATH_STATE_RE = re.compile(r'^DP(\d+)State$') +_CMIS_CONFIG_STATE_RE = re.compile(r'^config_state_hostlane(\d+)$') def check_lldp_neighbors_present(duthost, port_timeouts, namespaces=None): - """Poll APPL_DB ``LLDP_ENTRY_TABLE:`` until every port has a neighbor. + """Poll APPL_DB ``LLDP_ENTRY_TABLE:`` until every port has a + neighbor. Why: System tests need to confirm the far end re-converged after a disruptive operation; polling all ports together (each with its own @@ -65,7 +52,10 @@ def _namespace_for(port): ports_by_ns.setdefault(_namespace_for(port), []).append(port) start = time.monotonic() - deadlines = {port: start + max(0, int(timeout_sec)) for port, timeout_sec in port_timeouts.items()} + deadlines = { + port: start + max(0, int(timeout_sec)) + for port, timeout_sec in port_timeouts.items() + } remaining = set(port_timeouts) passed_ports = set() @@ -74,7 +64,9 @@ def _namespace_for(port): pending_in_ns = [port for port in ports_in_ns if port in remaining] if not pending_in_ns: continue - by_key, err = db_helpers.get_db_table(duthost, "APPL_DB", "LLDP_ENTRY_TABLE", namespace=ns, sep=":") + by_key, err = db_helpers.get_db_table( + duthost, "APPL_DB", "LLDP_ENTRY_TABLE", namespace=ns, sep=":" + ) if err: continue for port in pending_in_ns: @@ -88,12 +80,10 @@ def _namespace_for(port): remaining.discard(port) if not remaining: break - # Cap the sleep to whatever's left before the earliest still-remaining - # deadline, so a full _LLDP_POLL_INTERVAL_SEC never overshoots a - # deadline that falls inside this interval - keeps the reported - # "no LLDP neighbor after Ns" honest instead of running up to one - # poll interval past N. - sleep_for = min(_LLDP_POLL_INTERVAL_SEC, max(0, min(deadlines[port] for port in remaining) - now)) + sleep_for = min( + _LLDP_POLL_INTERVAL_SEC, + max(0, min(deadlines[port] for port in remaining) - now), + ) time.sleep(sleep_for) per_port = {} @@ -115,10 +105,10 @@ def _namespace_for(port): def capture_flap_sentinels(duthost, ports, namespaces=None): - """Snapshot every port's APPL_DB ``PORT_TABLE:`` ``flap_count``/``last_up_time`` once. - - Why: provides the shared baseline that :func:`assert_no_flap_since` later - compares against, regardless of how the elapsed time in between is spent. + """ + Snapshot every port's APPL_DB ``PORT_TABLE:`` ``flap_count``/ + ``last_up_time`` once. Creates shared baseline that + :func:`assert_no_flap_since` compares against, Args: duthost: SONiC DUT host fixture. @@ -127,7 +117,8 @@ def capture_flap_sentinels(duthost, ports, namespaces=None): Returns: dict: ``{port: (flap_count, last_up_time)}`` - both raw APPL_DB - strings (or ``None`` if either field is absent), one entry per ``ports``. + strings (or ``None`` if either field is absent), one entry per + ``ports``. """ if namespaces is None: namespaces = {} @@ -140,18 +131,20 @@ def _namespace_for(port): sentinels = {} for port in ports: port_table = db_helpers.hgetall_dict( - duthost, "APPL_DB", f"PORT_TABLE:{port}", namespace=_namespace_for(port) + duthost, "APPL_DB", f"PORT_TABLE:{port}", + namespace=_namespace_for(port) + ) + sentinels[port] = ( + port_table.get("flap_count"), port_table.get("last_up_time") ) - sentinels[port] = (port_table.get("flap_count"), port_table.get("last_up_time")) return sentinels -def assert_no_flap_since(duthost, ports, sentinels, namespaces=None, elapsed_sec=None): - """Verify no port in ``ports`` has flapped since its ``sentinels`` snapshot. - - Why: the comparison step of a flap-freeness check, decoupled from how the - sentinel was captured or how much time has elapsed, so callers can overlap - the observation window with other work instead of a flat sleep. +def assert_no_flap_since( + duthost, ports, sentinels, namespaces=None, elapsed_sec=None +): + """ + Verify no port in ``ports`` has flapped since its ``sentinels`` snapshot. Args: duthost: SONiC DUT host fixture. @@ -162,7 +155,8 @@ def assert_no_flap_since(duthost, ports, sentinels, namespaces=None, elapsed_sec elapsed_sec: optional, for the details message only. Returns: - dict: ``{port: {'passed': bool, 'details': str}}``, one entry per ``ports``. + dict: ``{port: {'passed': bool, 'details': str}}``, one entry per + ``ports``. """ if namespaces is None: namespaces = {} @@ -172,13 +166,17 @@ def _namespace_for(port): return namespaces[port] return db_helpers.resolve_namespace(duthost, port) - window_desc = f"{elapsed_sec}s window" if elapsed_sec is not None else "observation window" + window_desc = ( + f"{elapsed_sec}s window" if elapsed_sec is not None + else "observation window" + ) per_port = {} for port in ports: baseline_flap, baseline_up = sentinels.get(port, (None, None)) port_table = db_helpers.hgetall_dict( - duthost, "APPL_DB", f"PORT_TABLE:{port}", namespace=_namespace_for(port) + duthost, "APPL_DB", f"PORT_TABLE:{port}", + namespace=_namespace_for(port) ) current_flap = port_table.get("flap_count") current_up = port_table.get("last_up_time") @@ -199,19 +197,18 @@ def _namespace_for(port): logger.warning("Stability check FAILED: %s", details) per_port[port] = {"passed": False, "details": details} else: - details = f"{port}: stable for {window_desc} (flap_count={current_flap}, last_up_time={current_up})" + details = ( + f"{port}: stable for {window_desc} " + f"(flap_count={current_flap}, last_up_time={current_up})" + ) logger.info("Stability check PASSED: %s", details) per_port[port] = {"passed": True, "details": details} return per_port def check_ports_stability(duthost, ports, window_sec, namespaces=None): - """Verify no port in ``ports`` flaps over one shared post-recovery observation window. - - Why: standalone implementation of the mandatory "Stability (always)" - sub-check from system_test_plan.md, for callers with no other work to - overlap the observation window with (composes - :func:`capture_flap_sentinels` + sleep + :func:`assert_no_flap_since`). + """Verify no port in ``ports`` flaps over one shared post-recovery + observation window. Args: duthost: SONiC DUT host fixture. @@ -220,11 +217,15 @@ def check_ports_stability(duthost, ports, window_sec, namespaces=None): namespaces: optional dict of ``{port: namespace}``. Returns: - dict: ``{port: {'passed': bool, 'details': str}}``, one entry per ``ports``. + dict: ``{port: {'passed': bool, 'details': str}}``, one entry per + ``ports``. """ sentinels = capture_flap_sentinels(duthost, ports, namespaces=namespaces) time.sleep(window_sec) - return assert_no_flap_since(duthost, ports, sentinels, namespaces=namespaces, elapsed_sec=window_sec) + return assert_no_flap_since( + duthost, ports, sentinels, namespaces=namespaces, + elapsed_sec=window_sec, + ) # ────────────────────────────────────────────────────────────────────── @@ -233,19 +234,11 @@ def check_ports_stability(duthost, ports, window_sec, namespaces=None): # ────────────────────────────────────────────────────────────────────── -# TRANSCEIVER_STATUS field names, verified against a live DUT -# (``redis-cli -n 6 hgetall 'TRANSCEIVER_STATUS|Ethernet0'``) and against the -# xcvrd writer - CmisApi.get_transceiver_status() in sonic-platform-common -# (sonic_platform_base/sonic_xcvr/api/public/cmis.py): datapath state is -# published as "DPState" (value "DataPathActivated"), config state as -# "config_state_hostlane" (value "ConfigSuccess") - NOT -# "host_lane_datapath_state"/"host_lane_config_state". -_CMIS_DATAPATH_STATE_RE = re.compile(r'^DP(\d+)State$') -_CMIS_CONFIG_STATE_RE = re.compile(r'^config_state_hostlane(\d+)$') - - -def check_cmis_state(duthost, ports, lport_to_first_subport_mapping, namespaces=None): - """Verify CMIS DataPathState=DataPathActivated and ConfigState=ConfigSuccess, for every port in ``ports``. +def check_cmis_state( + duthost, ports, lport_to_first_subport_mapping, namespaces=None +): + """Verify CMIS DataPathState=DataPathActivated and + ConfigState=ConfigSuccess, for every port in ``ports``. Why: ``TRANSCEIVER_STATUS`` is published once per physical module (under the first sub-port of a breakout group) and carries every host lane of @@ -261,7 +254,8 @@ def check_cmis_state(duthost, ports, lport_to_first_subport_mapping, namespaces= namespaces: optional dict of ``{port: namespace}``. Returns: - dict: ``{port: {'passed': bool, 'details': str}}``, one entry per ``ports``. + dict: ``{port: {'passed': bool, 'details': str}}``, one entry per + ``ports``. """ if namespaces is None: namespaces = {} @@ -278,7 +272,9 @@ def _namespace_for(port): status_by_parent = {} port_table_by_port = {} for ns in ports_by_ns: - status_dump, status_err = db_helpers.get_state_db_table(duthost, "TRANSCEIVER_STATUS", namespace=ns) + status_dump, status_err = db_helpers.get_state_db_table( + duthost, "TRANSCEIVER_STATUS", namespace=ns + ) if status_err is None: status_by_parent.update(status_dump) port_table_dump, port_table_err = db_helpers.get_db_table( @@ -294,7 +290,8 @@ def _namespace_for(port): if not status: per_port[port] = { "passed": False, - "details": f"{port}: TRANSCEIVER_STATUS|{parent} missing or empty", + "details": f"{port}: TRANSCEIVER_STATUS|{parent} missing " + "or empty", } continue @@ -312,7 +309,9 @@ def _namespace_for(port): host_lane_count = len(lanes_field.split(",")) subport = int(port_table.get("subport") or 1) lane_start = host_lane_count * max(0, subport - 1) - active_lanes = set(range(lane_start + 1, lane_start + host_lane_count + 1)) + active_lanes = set( + range(lane_start + 1, lane_start + host_lane_count + 1) + ) bad_datapath = [] bad_config = [] @@ -339,10 +338,12 @@ def _namespace_for(port): per_port[port] = { "passed": False, "details": ( - f"{port} (parent {parent}) TRANSCEIVER_STATUS|{parent} has no " - f"DPState or config_state_hostlane fields for this port's " - f"active host lanes {sorted(active_lanes)} - cannot confirm CMIS " - "state (schema mismatch, partial publish, or lane-range mismatch)" + f"{port} (parent {parent}) TRANSCEIVER_STATUS|{parent} " + f"has no DPState or config_state_hostlane fields " + f"for this port's active host lanes " + f"{sorted(active_lanes)} - cannot confirm CMIS state " + "(schema mismatch, partial publish, or lane-range " + "mismatch)" ), } continue @@ -355,13 +356,15 @@ def _namespace_for(port): problems.append("config: " + ", ".join(bad_config)) per_port[port] = { "passed": False, - "details": f"{port} (parent {parent}) CMIS state NOT activated - " + "; ".join(problems), + "details": f"{port} (parent {parent}) CMIS state NOT " + "activated - " + "; ".join(problems), } continue per_port[port] = { "passed": True, - "details": f"{port} (parent {parent}) CMIS DataPathActivated + ConfigSuccess", + "details": f"{port} (parent {parent}) CMIS DataPathActivated " + "+ ConfigSuccess", } return per_port @@ -374,13 +377,11 @@ def standard_port_recovery_and_verification( flap_count_baseline=None, assert_no_flap_across_op=False, ): - """Run the Standard Port Recovery and Verification Procedure on a batch of ports. - - Why: implements the common post-recovery verification steps - (link status, flap/stability, LLDP, CMIS state, docker/process health) - that every disruptive Link Behavior System test needs to run afterward, - batched across ``ports`` so fixed per-call costs aren't multiplied by - port count and every port's failures are surfaced in one call. + """Run the Standard Port Recovery and Verification Procedure on a + batch of ports (link status, flap/stability, LLDP, CMIS state, + docker/process health), batched across ``ports`` so fixed per-call + costs aren't multiplied by port count and every port's failures are + surfaced in one call. Args: duthost: SONiC DUT host fixture. @@ -397,88 +398,110 @@ def standard_port_recovery_and_verification( (seconds) for the stability sub-check. expected_pid_changes: set of monitored process names whose PID is expected to differ from ``health_baseline`` in the health check. - flap_count_baseline: optional dict of ``{port: flap_count}`` + flap_count_baseline: optional dict of ``{port: flap_count}`` assert_no_flap_across_op: whether to additionally assert no flap occurred across the whole operation (only valid where the link stays up and the flap counter survives, e.g. xcvrd/pmon restart). Returns: - dict: ``{'passed': bool, 'per_port': {port: {'passed': bool, 'details': str}}, 'details': str}`` + dict: ``{'passed': bool, 'per_port': {port: {'passed': bool, + 'details': str}}, 'details': str}`` """ - # Owning ASIC namespace per port, resolved once and reused for every - # per-namespace DB read below (LLDP / TRANSCEIVER_STATUS / stability). # ``None`` on single-ASIC -> no ``-n`` flag. - namespaces = {port: db_helpers.resolve_namespace(duthost, port) for port in ports} + namespaces = { + port: db_helpers.resolve_namespace(duthost, port) for port in ports + } per_port_failures = {port: [] for port in ports} - checks_ran = {port: [] for port in ports} # human-readable checks that ran, per port + checks_ran = {port: [] for port in ports} # human-readable checks ran # 1. Link status - one batched poll covers every port. - down_ports = wait_ports_oper_status(duthost, ports, "up", link_up_timeout_sec) + down_ports = wait_ports_oper_status( + duthost, ports, "up", link_up_timeout_sec + ) for port in ports: checks_ran[port].append("link up") for port in down_ports: - per_port_failures[port].append(f"port {port} did not reach oper-up within {link_up_timeout_sec}s") + per_port_failures[port].append( + f"port {port} did not reach oper-up within {link_up_timeout_sec}s" + ) up_ports = [port for port in ports if port not in down_ports] # 2a/2b setup - one shared flap/last_up_time sentinel per up port, captured # right after link-up. recovery_t0 = time.monotonic() - post_recovery_sentinels = capture_flap_sentinels(duthost, up_ports, namespaces=namespaces) if up_ports else {} - - # 2b. No flap across the operation - only when assert_no_flap_across_op is - # set (flap counter must survive the op, e.g. xcvrd/pmon restart). - if assert_no_flap_across_op: - for port in up_ports: - checks_ran[port].append("no-flap-across-op") - baseline_flap = (flap_count_baseline or {}).get(port) - current_flap, _current_up = post_recovery_sentinels.get(port, (None, None)) - if baseline_flap is None or current_flap is None: - per_port_failures[port].append( - f"{port}: cannot assert across-op no-flap - flap_count baseline/current missing" - ) - elif current_flap != baseline_flap: - per_port_failures[port].append( - f"{port}: flapped across operation (flap_count {baseline_flap} -> {current_flap})" - ) - - # 3. LLDP - only for up ports that request it (otherwise LLDP is moot); - # per-port timeouts honored, polls interleaved across the batch. - lldp_port_timeouts = {} - for port in up_ports: - sys_attrs = port_attributes_dict.get(port, {}).get(SYSTEM_ATTRIBUTES_KEY, {}) - if sys_attrs.get("verify_lldp_on_link_up", True): - if "lldp_neighbor_wait_sec" not in sys_attrs: - raise ValueError( - f"{port}: 'lldp_neighbor_wait_sec' is not defined in SYSTEM_ATTRIBUTES " - "(system.json 'defaults', or a more specific override) - required " - "whenever verify_lldp_on_link_up is True" - ) - lldp_port_timeouts[port] = sys_attrs["lldp_neighbor_wait_sec"] - if lldp_port_timeouts: - lldp_results = check_lldp_neighbors_present( - duthost, lldp_port_timeouts, namespaces=namespaces - ) - for port, result in lldp_results.items(): - checks_ran[port].append("LLDP") - if not result["passed"]: - per_port_failures[port].append(result["details"]) - - # 5. CMIS state - only for up, CMIS active-optical ports, in one batched - # check_cmis_state call. - cmis_active_ports = [ - port for port in up_ports - if is_cmis_active_optical(port_attributes_dict.get(port, {}).get(EEPROM_ATTRIBUTES_KEY, {})) - ] - if cmis_active_ports: - cmis_results = check_cmis_state( - duthost, cmis_active_ports, lport_to_first_subport_mapping, namespaces=namespaces - ) - for port, result in cmis_results.items(): - checks_ran[port].append("CMIS state") - if not result["passed"]: - per_port_failures[port].append(result["details"]) + post_recovery_sentinels = ( + capture_flap_sentinels(duthost, up_ports, namespaces=namespaces) + if up_ports else {} + ) + + # NOTE: Temporarily disabled for PR streamlining + # All commented code below is UNTESTED + # + # # 2b. No flap across the operation - only when assert_no_flap_across_op + # # is set (flap counter must survive the op, e.g. xcvrd/pmon + # # restart). + # if assert_no_flap_across_op: + # for port in up_ports: + # checks_ran[port].append("no-flap-across-op") + # baseline_flap = (flap_count_baseline or {}).get(port) + # current_flap, _current_up = post_recovery_sentinels.get( + # port, (None, None) + # ) + # if baseline_flap is None or current_flap is None: + # per_port_failures[port].append( + # f"{port}: cannot assert across-op no-flap - " + # "flap_count baseline/current missing" + # ) + # elif current_flap != baseline_flap: + # per_port_failures[port].append( + # f"{port}: flapped across operation (flap_count " + # f"{baseline_flap} -> {current_flap})" + # ) + + # # 3. LLDP - only for up ports that request it (otherwise LLDP is moot); + # # per-port timeouts honored, polls interleaved across the batch. + # lldp_port_timeouts = {} + # for port in up_ports: + # sys_attrs = port_attributes_dict.get(port, {}).get( + # SYSTEM_ATTRIBUTES_KEY, {} + # ) + # if sys_attrs.get("verify_lldp_on_link_up", True): + # if "lldp_neighbor_wait_sec" not in sys_attrs: + # raise ValueError( + # f"{port}: 'lldp_neighbor_wait_sec' is not defined in " + # "SYSTEM_ATTRIBUTES (system.json 'defaults', or a more " + # "specific override) - required whenever " + # "verify_lldp_on_link_up is True" + # ) + # lldp_port_timeouts[port] = sys_attrs["lldp_neighbor_wait_sec"] + # if lldp_port_timeouts: + # lldp_results = check_lldp_neighbors_present( + # duthost, lldp_port_timeouts, namespaces=namespaces + # ) + # for port, result in lldp_results.items(): + # checks_ran[port].append("LLDP") + # if not result["passed"]: + # per_port_failures[port].append(result["details"]) + + # # 5. CMIS state - only for up, CMIS active-optical ports, in one + # # batched check_cmis_state call. + # cmis_active_ports = [ + # port for port in up_ports + # if is_cmis_active_optical( + # port_attributes_dict.get(port, {}).get(EEPROM_ATTRIBUTES_KEY, {}) + # ) + # ] + # if cmis_active_ports: + # cmis_results = check_cmis_state( + # duthost, cmis_active_ports, lport_to_first_subport_mapping, + # namespaces=namespaces + # ) + # for port, result in cmis_results.items(): + # checks_ran[port].append("CMIS state") + # if not result["passed"]: + # per_port_failures[port].append(result["details"]) # 2a end-gate. Mandatory stability sub-check, only for up ports. Asserted # here (after steps 3/5) so the window overlaps that work instead of a @@ -500,14 +523,18 @@ def standard_port_recovery_and_verification( # once, host-wide, unconditionally. if health_baseline is None: health_failure = ( - "health_baseline not provided - caller must pass the 'health_baseline' " - "pytest fixture value (tests/transceiver/conftest.py)" + "health_baseline not provided - caller must pass the " + "'health_baseline' pytest fixture value " + "(tests/transceiver/conftest.py)" ) else: health_result = health_checks.verify_health( duthost, health_baseline, expect_pid_change=expected_pid_changes, ) - health_failure = None if health_result["passed"] else "; ".join(health_result["failures"]) + health_failure = ( + None if health_result["passed"] + else "; ".join(health_result["failures"]) + ) for port in ports: checks_ran[port].append("health") if health_failure is not None: @@ -530,4 +557,4 @@ def standard_port_recovery_and_verification( "passed": overall_passed, "per_port": per_port, "details": "; ".join(per_port[port]["details"] for port in ports), - } \ No newline at end of file + } From a9fe265498327a6bedeaefbbc51beef3921f07cb Mon Sep 17 00:00:00 2001 From: sutter Date: Wed, 5 Aug 2026 15:49:18 -0700 Subject: [PATCH 06/16] Updated to simon vers Signed-off-by: sutter --- tests/transceiver/common/db_helpers.py | 67 ++++++++++++++++++++------ 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/tests/transceiver/common/db_helpers.py b/tests/transceiver/common/db_helpers.py index cfded120172..d65502a13c4 100644 --- a/tests/transceiver/common/db_helpers.py +++ b/tests/transceiver/common/db_helpers.py @@ -181,36 +181,43 @@ def get_state_db_hash_field(duthost, table, key, field, namespace=None): return get_db_hash_field(duthost, STATE_DB, table, key, field, namespace=namespace) -def get_state_db_table(duthost, table, namespace=None): - """Read every STATE_DB ``|*`` entry in a single ``sonic-db-dump`` call. +def get_db_table(duthost, db, table, namespace=None, sep="|"): + """Read every ``
*`` entry in ``db`` in a single ``sonic-db-dump`` call. + + This replaces one per-key read (``hget``/``hgetall``) per port with one bulk + dump — the right shape when a test/poll needs many ports' entries (e.g. + verifying ``vdm_supported`` across the whole ``TRANSCEIVER_INFO`` table, or + polling ``LLDP_ENTRY_TABLE`` presence across a batch of ports) instead of one + round-trip per port. - This replaces one ``hget`` per port with one bulk dump — the right shape when - a test needs many ports' fields (e.g. verifying ``vdm_supported`` across the - whole ``TRANSCEIVER_INFO`` table) instead of a single field. + ``sep`` is the table/key separator: ``"|"`` for STATE_DB and CONFIG_DB, + ``":"`` for APPL_DB (see :func:`get_db_hash_field`). ``namespace`` scopes the dump to one ASIC on a multi-ASIC DUT. NOTE the - mechanism differs from :func:`get_state_db_hash_field`: ``sonic-db-dump``'s - own ``-n`` is the *database* name (here ``STATE_DB``), not a namespace, so a - namespaced read is done by running the dump inside the ASIC's network - namespace via ``sudo ip netns exec ...`` — the same wrapper the - framework's ASIC host uses (see ``sonic_asic.py`` ``ns_arg``). The prefix is - added only when ``namespace`` is truthy (``asicN``, e.g. from + mechanism differs from :func:`get_db_hash_field`: ``sonic-db-dump``'s own + ``-n`` is the *database* name (here ``db``), not a namespace, so a namespaced + read is done by running the dump inside the ASIC's network namespace via + ``sudo ip netns exec ...`` — the same wrapper the framework's ASIC host + uses (see ``sonic_asic.py`` ``ns_arg``). The prefix is added only when + ``namespace`` is truthy (``asicN``, e.g. from ``duthost.get_namespace_from_asic_id``); on a single-ASIC DUT the value is ``None``/``""`` and the command stays byte-identical to the pre-namespace form. Returns ``(by_key, err)``: - - ``({key_suffix: {field: value}}, None)`` on success. The ``
|`` + - ``({key_suffix: {field: value}}, None)`` on success. The ``
`` prefix is stripped, so for ``TRANSCEIVER_INFO`` ``key_suffix`` is the port name and the value is that port's published field map (an empty - dict if the entry carries no fields). + dict if the entry carries no fields). Redis never stores an empty hash, + so ``key_suffix in by_key`` is equivalent to "that key's hash is + non-empty" — the same truth a per-key ``if entry:`` test gives. - ``(None, " failed ...")`` on a non-zero rc or unparseable output. ``sonic-db-dump -y`` emits JSON keyed by full Redis key, with the hash fields nested under each key's ``"value"`` block; this unwraps that into a flat - ``{port: {field: value}}`` map. + ``{key_suffix: {field: value}}`` map. """ ns_prefix = f"sudo ip netns exec {namespace} " if namespace else "" - cmd = f"{ns_prefix}sonic-db-dump -n STATE_DB -y -k '{table}|*'" + cmd = f"{ns_prefix}sonic-db-dump -n {db} -y -k '{table}{sep}*'" result = duthost.shell(cmd, module_ignore_errors=True) if result.get("rc", RC_FAILURE) != 0: return None, ( @@ -221,7 +228,7 @@ def get_state_db_table(duthost, table, namespace=None): raw = json.loads(result.get("stdout") or "{}") except ValueError as exc: return None, f"{cmd}: could not parse sonic-db-dump JSON ({exc})" - prefix = f"{table}|" + prefix = f"{table}{sep}" return { full_key[len(prefix):]: entry.get("value", {}) for full_key, entry in raw.items() @@ -229,6 +236,34 @@ def get_state_db_table(duthost, table, namespace=None): }, None +def get_state_db_table(duthost, table, namespace=None): + """Thin wrapper over :func:`get_db_table` pinned to ``STATE_DB`` (``|`` separator). + + See that function for the ``namespace``/``(by_key, err)`` semantics; this + preserves the existing STATE_DB call sites unchanged. + """ + return get_db_table(duthost, "STATE_DB", table, namespace=namespace, sep="|") + + +def resolve_namespace(duthost, port): + """Return the ASIC network namespace owning ``port`` (``None`` on single-ASIC). + + ``duthost.get_port_asic_instance(port).namespace`` directly - that + ``SonicAsic`` attribute is already set to exactly what + ``duthost.get_namespace_from_asic_id(asic_index)`` would recompute from + the same instance's own ``asic_index`` (see ``tests/common/devices/ + sonic_asic.py``'s ``__init__`` and ``tests/common/devices/multi_asic.py``'s + ``get_namespace_from_asic_id``), so going through ``get_namespace_from_asic_id`` + is pure indirection for a port that's already resolved to its ASIC. + + Multi-ASIC DBs (STATE_DB / APPL_DB, including LLDP) are per-namespace, so + every per-port DB read scopes to the owning ASIC; on a single-ASIC DUT + this is ``None`` (``DEFAULT_NAMESPACE``) and the ``hgetall_dict``/ + ``get_db_hash_field`` wrappers above emit no ``-n`` flag. + """ + return duthost.get_port_asic_instance(port).namespace + + def get_config_db_port_names(duthost): """Return the set of port names in the CONFIG_DB PORT table. From 42892871300bbc799bba7feb2978b8884aae9eaf Mon Sep 17 00:00:00 2001 From: sutter Date: Wed, 5 Aug 2026 17:14:33 -0700 Subject: [PATCH 07/16] Updated misc files to bring in line with simon changes, plus compatibilty fixes Signed-off-by: sutter --- tests/common/platform/interface_utils.py | 35 +++- tests/transceiver/common/state_management.py | 150 +++++++++++++++++ tests/transceiver/common/verification.py | 6 +- tests/transceiver/system/__init__.py | 0 tests/transceiver/system/conftest.py | 155 ++++++++++++++++++ .../process_restart/test_xcvrd_restart.py | 6 +- 6 files changed, 337 insertions(+), 15 deletions(-) create mode 100644 tests/transceiver/common/state_management.py create mode 100644 tests/transceiver/system/__init__.py create mode 100644 tests/transceiver/system/conftest.py diff --git a/tests/common/platform/interface_utils.py b/tests/common/platform/interface_utils.py index ec1e3040f09..dad7d205249 100644 --- a/tests/common/platform/interface_utils.py +++ b/tests/common/platform/interface_utils.py @@ -1,6 +1,5 @@ """ Helper script for checking status of interfaces - This script contains re-usable functions for checking status of interfaces on SONiC. """ @@ -94,6 +93,34 @@ def expect_interface_status(dut, interface_name, expected_op_status): return status['oper'] == expected_op_status +def wait_ports_oper_status(duthost, ports, status, wait_sec, poll_interval_sec=2): + """Poll until every port in ``ports`` reaches oper-``status``; return failures. + Issues a single ``show interface description`` per poll (one full-table dump, + parsed once) and checks every port against that snapshot, rather than one CLI + call per port -- the latter is O(N) redundant dumps per poll and does not + scale to hundreds of ports. + Returns a list with one string per port still not at oper-``status`` after + ``wait_sec``; empty once all reach it. A port absent from the dump is reported + as a failure (rather than raising) so a missing/renamed port aggregates like + any other laggard. + """ + # Imported lazily to avoid a module-load import cycle + # (tests.common.utilities <-> tests.common.platform.interface_utils). + from tests.common.utilities import wait_until + + def _ports_not_at_status(): + snapshot = get_dut_interfaces_status(duthost) + return [port for port in ports + if (snapshot.get(port) or {}).get("oper") != status] + + if wait_until(wait_sec, poll_interval_sec, 0, lambda: not _ports_not_at_status()): + return [] + return [ + "port {} did not reach oper-{} within {}s".format(port, status, wait_sec) + for port in _ports_not_at_status() + ] + + def check_interface_status(dut, asic_index, interfaces, xcvr_skip_list): """ @summary: Check the admin and oper status of the specified interfaces on DUT. @@ -266,11 +293,9 @@ def get_dpu_npu_ports_from_hwsku(duthost): def get_fec_eligible_interfaces(duthost, supported_speeds): """ Get interfaces that are operationally up, SFP present and have supported speeds. - Args: duthost: The device under test. supported_speeds (list): A list of supported speeds for validation. - Returns: interfaces (list): A list of interface names with SFP present, oper status up and speed in supported_speeds. @@ -307,11 +332,9 @@ def get_fec_eligible_interfaces(duthost, supported_speeds): def clear_interface_counters_and_wait(duthost, wait_time=60): """ Clear SONiC interface counters and wait before validating them. - Args: duthost: The device under test. wait_time (int): Number of seconds to wait after clearing counters. - Returns: None """ @@ -439,4 +462,4 @@ def get_port_indexes_with_flat_memory(dut): port_indexes_with_flat_memory = dut.shell("python3 get_port_indexes_with_flat_memory.py")["stdout"] port_indexes_with_flat_memory = ast.literal_eval(port_indexes_with_flat_memory) logging.info(f"Port indexes with flat memory: {port_indexes_with_flat_memory}") - return port_indexes_with_flat_memory + return port_indexes_with_flat_memory \ No newline at end of file diff --git a/tests/transceiver/common/state_management.py b/tests/transceiver/common/state_management.py new file mode 100644 index 00000000000..3e97654b2a4 --- /dev/null +++ b/tests/transceiver/common/state_management.py @@ -0,0 +1,150 @@ +"""State Preservation and Restoration helpers for transceiver System tests. + +Lives at the location reserved by +``docs/testplan/transceiver/diagrams/file_organization.md`` for the +"State Preservation and Restoration helpers". + +Implements the post-session / post-test restoration described in +``docs/testplan/transceiver/system_test_plan.md`` (§ State Preservation and +Restoration): bring every port back to its known-good steady state — admin-up ++ oper-up, high power mode (low-power off), and CMIS DataPath Activated. + +This is the restorative counterpart to +:mod:`tests.transceiver.common.verification` (which is diagnostic): it reuses +``verification.check_cmis_state`` to decide whether a CMIS datapath needs +recycling and ``prerequisites.check_links_up`` for the final recovery verdict, +and issues port shutdown/startup through +:mod:`tests.transceiver.common.cli_helpers`. +""" +import logging + +from tests.common.platform.interface_utils import ( + get_dut_interfaces_status, + get_lport_to_first_subport_mapping, +) +from tests.transceiver.attribute_parser.attribute_keys import EEPROM_ATTRIBUTES_KEY +from tests.transceiver.common import cli_helpers +from tests.transceiver.common.prerequisites import check_links_up +from tests.transceiver.common.verification import check_cmis_state + +logger = logging.getLogger(__name__) + + +def _port_namespace(duthost, port): + """Return the ASIC network namespace owning ``port``. + + Resolved the same way as the System tests + (``tests/transceiver/system/link_behavior/test_port_link_toggle.py`` and the + EEPROM tests): map the port to its ASIC instance, then to that ASIC's + namespace. On a single-ASIC DUT this is ``""``, so ``cli_helpers`` emits no + ``-n`` flag and the command stays ``config interface startup ``. + """ + return duthost.get_namespace_from_asic_id( + duthost.get_port_asic_instance(port).asic_index + ) + + +def _is_oper_up(duthost, port): + intf_status = get_dut_interfaces_status(duthost) + s = intf_status.get(port, {}) or {} + return s.get("admin") == "up" and s.get("oper") == "up" + + +def _is_lpmode_high(duthost, port): + """Return True iff sfputil reports low-power mode is OFF (i.e. high power).""" + out = duthost.shell(f"sfputil show lpmode -p {port}", module_ignore_errors=True) + if out.get("rc", 1) != 0: + return True # can't tell - don't try to "fix" what we can't observe + for line in (out.get("stdout_lines") or []): + parts = line.strip().split() + if len(parts) >= 2 and parts[0] == port: + return parts[1].lower() == "off" + return True + + +def post_state_restoration(duthost, port_attributes_dict): + """Restore every port in ``port_attributes_dict`` to known-good state. + + Per ``system_test_plan.md`` post-session State Restoration: + * admin-up + oper-up, + * high power mode (low-power off), + * DataPath Activated. + + The function is restorative, not diagnostic - it does the minimum + needed to bring each port back, then reports which ports it touched + and which ones still failed to recover. + + Returns: + dict: ``{ + 'admin_up_restored': [str], # ports we issued 'startup' on + 'lpmode_high_restored': [str], # ports we toggled out of LPMode + 'datapath_recycled': [str], # ports we shutdown+startup'd to + # force a CMIS datapath re-init + 'still_failing': [str], # 'port: reason' for ports that + # didn't recover after all of + # the above + }`` + """ + summary = { + "admin_up_restored": [], + "lpmode_high_restored": [], + "datapath_recycled": [], + "still_failing": [], + } + if not port_attributes_dict: + return summary + + # Pass 1: admin-up everything that's down. + for port in sorted(port_attributes_dict.keys()): + if not _is_oper_up(duthost, port): + logger.info("Restoration: issuing 'config interface startup %s'", port) + cli_helpers.config_interface_startup( + duthost, port, namespace=_port_namespace(duthost, port) + ) + summary["admin_up_restored"].append(port) + + # Pass 2: turn off low-power mode on anything still in LPMode. + for port in sorted(port_attributes_dict.keys()): + if not _is_lpmode_high(duthost, port): + logger.info("Restoration: turning off LPMode on %s", port) + duthost.shell(f"sfputil lpmode off {port}", module_ignore_errors=True) + summary["lpmode_high_restored"].append(port) + + # Pass 3: for CMIS active-optical ports, recycle the datapath if still + # not DPActivated. Batched into a single check_cmis_state call across + # every such port (it does its own fresh DB reads per call, so this + # naturally picks up the admin-up/LPMode actions from passes 1 and 2 + # above without needing a separate cache to invalidate). + cmis_ports = [ + port for port in sorted(port_attributes_dict.keys()) + if port_attributes_dict[port].get(EEPROM_ATTRIBUTES_KEY, {}).get( + "cmis_active_optical" + ) + ] + if cmis_ports: + lport_to_first_subport_mapping = get_lport_to_first_subport_mapping( + duthost + ) + cmis_results = check_cmis_state( + duthost, cmis_ports, lport_to_first_subport_mapping + ) + for port in cmis_ports: + if not cmis_results[port]["passed"]: + logger.info( + "Restoration: recycling datapath on %s (shutdown+startup)", + port, + ) + namespace = _port_namespace(duthost, port) + cli_helpers.config_interface_shutdown( + duthost, port, namespace=namespace + ) + cli_helpers.config_interface_startup( + duthost, port, namespace=namespace + ) + summary["datapath_recycled"].append(port) + + final_link = check_links_up(duthost, port_attributes_dict) + if not final_link["passed"]: + summary["still_failing"].extend(final_link["down"]) + + return summary diff --git a/tests/transceiver/common/verification.py b/tests/transceiver/common/verification.py index 915ee0f3d02..c8ac4247e18 100644 --- a/tests/transceiver/common/verification.py +++ b/tests/transceiver/common/verification.py @@ -26,10 +26,6 @@ def check_lldp_neighbors_present(duthost, port_timeouts, namespaces=None): """Poll APPL_DB ``LLDP_ENTRY_TABLE:`` until every port has a neighbor. - Why: System tests need to confirm the far end re-converged after a - disruptive operation; polling all ports together (each with its own - timeout) lets waits overlap instead of summing per-port timeouts. - Args: duthost: SONiC DUT host fixture. port_timeouts: dict of ``{port: timeout_sec}``. @@ -241,7 +237,7 @@ def check_cmis_state( ConfigState=ConfigSuccess, for every port in ``ports``. Why: ``TRANSCEIVER_STATUS`` is published once per physical module (under - the first sub-port of a breakout group) and carries every host lane of + the first sub-port of a breakou t group) and carries every host lane of the module, so a breakout sub-port must be checked only against its own active lanes - not a sibling's - to avoid a false pass/fail; this also batches the underlying DB reads per namespace instead of per port. diff --git a/tests/transceiver/system/__init__.py b/tests/transceiver/system/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/transceiver/system/conftest.py b/tests/transceiver/system/conftest.py new file mode 100644 index 00000000000..23e3484276f --- /dev/null +++ b/tests/transceiver/system/conftest.py @@ -0,0 +1,155 @@ +"""System category conftest. + +Opts the System test category into the cross-category session-level +prerequisites defined in ``tests/transceiver/conftest.py`` and runs the +post-session checks called out in +``docs/testplan/transceiver/system_test_plan.md``. + +Per the prerequisite matrix in ``docs/testplan/transceiver/test_plan.md``, +System consumes all three gates: ``presence_verified``, +``gold_fw_verified``, and ``links_verified``. Requesting them here means +the gates fire once per session before any System test runs, and on +failure every System test is skipped with a clear reason. +""" +import logging + +import pytest + +from tests.transceiver.attribute_parser.attribute_keys import SYSTEM_ATTRIBUTES_KEY +from tests.transceiver.common.prerequisites import check_links_up +from tests.transceiver.common.state_management import post_state_restoration +from tests.transceiver.common.verification import check_lldp_neighbors_present + +logger = logging.getLogger(__name__) + + +@pytest.fixture(autouse=True, scope="session") +def _system_session_prerequisites(presence_verified, gold_fw_verified, links_verified): + """Autouse wrapper that pulls in every session-scoped prerequisite gate + consumed by System tests. + + All three gates are session-scoped fixtures defined in + ``tests/transceiver/conftest.py``; each one calls ``pytest.skip(...)`` + on failure so every System test is skipped with a clear reason. + """ + return + + +# ────────────────────────────────────────────────────────────────────── +# Post-Session Checks (system_test_plan.md). +# +# After the full System suite has run we do, in order: +# 0. State Restoration - run post_state_restoration() to put the +# testbed back into a known-good state +# (admin-up + high power mode + DPActivated) +# before we verify anything below. +# 1. STATE_DB consistency - TRANSCEIVER_INFO and TRANSCEIVER_DOM_SENSOR +# entries exist for every port in +# port_attributes_dict. +# 2. End-to-end link + LLDP - every port back oper-up and (if enabled) +# its LLDP neighbor rediscovered after the +# disruptive test sequence. +# +# Failures are reported as warnings instead of fixture errors so they +# don't mask the actual test results that are already on the report. +# ────────────────────────────────────────────────────────────────────── + + +def _state_db_key_exists(duthost, key): + cmd = f'sonic-db-cli STATE_DB hgetall "{key}"' + out = duthost.shell(cmd, module_ignore_errors=True) + if out.get("rc", 1) != 0: + return False + return bool((out.get("stdout") or "").strip()) + + +@pytest.fixture(autouse=True, scope="session") +def _system_post_session_checks(duthost, port_attributes_dict): + """Run the Post-Session State Restoration + Checks from + system_test_plan.md at session teardown. + + Order matters: restoration runs FIRST so the STATE_DB / link / LLDP + checks that follow observe the restored steady state, not whatever + transient mid-failure state the suite happened to end in. + """ + yield + + if not port_attributes_dict: + return + + logger.info("System suite: running post-session state restoration on %d port(s)", + len(port_attributes_dict)) + + # 0. State Restoration - admin-up, high power mode, DPActivated. + try: + restoration_summary = post_state_restoration(duthost, port_attributes_dict) + except Exception as e: + # Defensive: this should never throw, but if it does we don't + # want to mask the actual test results. + logger.warning("post_state_restoration raised unexpectedly: %s", e) + restoration_summary = None + + if restoration_summary: + actions_taken = ( + restoration_summary["admin_up_restored"] + or restoration_summary["lpmode_high_restored"] + or restoration_summary["datapath_recycled"] + ) + if actions_taken: + logger.warning( + "Post-session restoration actions taken: " + "startup=%s lpmode_off=%s transceivers_recycled=%s", + restoration_summary["admin_up_restored"], + restoration_summary["lpmode_high_restored"], + restoration_summary["datapath_recycled"], + ) + if restoration_summary["still_failing"]: + logger.warning( + "Post-session restoration: %d port(s) did NOT recover: %s", + len(restoration_summary["still_failing"]), + "; ".join(restoration_summary["still_failing"]), + ) + + logger.info("System suite: running post-session consistency checks on %d port(s)", + len(port_attributes_dict)) + + # 1. STATE_DB consistency. + missing_info = [] + missing_dom = [] + for port in sorted(port_attributes_dict.keys()): + if not _state_db_key_exists(duthost, f"TRANSCEIVER_INFO|{port}"): + missing_info.append(port) + if not _state_db_key_exists(duthost, f"TRANSCEIVER_DOM_SENSOR|{port}"): + missing_dom.append(port) + if missing_info: + logger.warning("Post-session: TRANSCEIVER_INFO missing in STATE_DB for: %s", + ", ".join(missing_info)) + if missing_dom: + logger.warning("Post-session: TRANSCEIVER_DOM_SENSOR missing in STATE_DB for: %s", + ", ".join(missing_dom)) + if not missing_info and not missing_dom: + logger.info("Post-session: STATE_DB consistency check PASSED") + + # 2. Final link + LLDP. + link_result = check_links_up(duthost, port_attributes_dict) + if not link_result["passed"]: + logger.warning("Post-session link check FAILED: %s", link_result["details"]) + else: + logger.info("Post-session link check PASSED: %s", link_result["details"]) + + # short poll budget here - LLDP should already be settled by now + lldp_port_timeouts = { + port: 30 + for port, attrs in port_attributes_dict.items() + if attrs.get(SYSTEM_ATTRIBUTES_KEY, {}).get( + "verify_lldp_on_link_up", True + ) + } + lldp_results = check_lldp_neighbors_present(duthost, lldp_port_timeouts) + lldp_failed = [port for port, r in lldp_results.items() if not r["passed"]] + if lldp_failed: + logger.warning( + "Post-session LLDP check FAILED for: %s", ", ".join(lldp_failed) + ) + else: + logger.info("Post-session LLDP check PASSED") \ No newline at end of file diff --git a/tests/transceiver/system/process_restart/test_xcvrd_restart.py b/tests/transceiver/system/process_restart/test_xcvrd_restart.py index fb3a656ec56..bb69d7ebf1b 100644 --- a/tests/transceiver/system/process_restart/test_xcvrd_restart.py +++ b/tests/transceiver/system/process_restart/test_xcvrd_restart.py @@ -38,7 +38,6 @@ """ import logging import time - import pytest from tests.transceiver.attribute_parser.attribute_keys import ( @@ -135,10 +134,9 @@ def test_system_xcvrd_crash_recovery( See the module docstring for the full execution tree. Steps: * verify all ports are oper-up and record xcvrd uptime, - * inject a crash into the xcvrd script via ``inject_xcvrd_crash``, + * inject a crash into the xcvrd script via SIGKILL, * monitor automatic restart behavior, - * wait for ``xcvrd_restart_settle_sec`` then run Standard Port Recovery - and Verification for every port. + * wait for ``xcvrd_restart_settle_sec`` then run SPRaV All (port, step) failures are accumulated and reported in a single ``pytest.fail`` so one run surfaces every issue. From 9e391a11baba70195e8c36d481640c16d69967b7 Mon Sep 17 00:00:00 2001 From: sutter Date: Thu, 6 Aug 2026 14:49:58 -0700 Subject: [PATCH 08/16] Updated helper function calls Signed-off-by: sutter --- .../system/process_restart/test_pmon_restart.py | 11 ++++------- .../system/process_restart/test_swss_restart.py | 3 ++- .../system/process_restart/test_syncd_restart.py | 3 ++- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/transceiver/system/process_restart/test_pmon_restart.py b/tests/transceiver/system/process_restart/test_pmon_restart.py index 32f0704a7e5..2b5f9f5ba02 100644 --- a/tests/transceiver/system/process_restart/test_pmon_restart.py +++ b/tests/transceiver/system/process_restart/test_pmon_restart.py @@ -65,7 +65,8 @@ def test_system_pmon_restart( failures = [] logger.info("Recording link states and uptime for %d port(s)", len(ports)) - if not check_links_up(duthost, port_attributes_dict): + link_check = check_links_up(duthost, port_attributes_dict) + if not link_check["passed"]: logger.warning("Validation on Start FAILED: some ports are down") else: appl_db = sdbHelp(duthost) @@ -80,12 +81,8 @@ def test_system_pmon_restart( logger.info("Restarting pmon...") duthost.restart_service('pmon', 'pmon') pmon_wait = port_attributes_dict[ports[0]].get( - SYSTEM_ATTRIBUTES_KEY, - {} - ).get( - "pmon_restart_settle_sec", - 120 - ) + SYSTEM_ATTRIBUTES_KEY, {} + ).get("pmon_restart_settle_sec", 120) # accounts for minimum timeout behavior of SPRaV time.sleep(pmon_wait + 60) diff --git a/tests/transceiver/system/process_restart/test_swss_restart.py b/tests/transceiver/system/process_restart/test_swss_restart.py index 7ac427c5cf5..35d3c952dd7 100644 --- a/tests/transceiver/system/process_restart/test_swss_restart.py +++ b/tests/transceiver/system/process_restart/test_swss_restart.py @@ -68,7 +68,8 @@ def test_system_swss_restart( failures = [] logger.info("Recording link states and uptime for %d port(s)", len(ports)) - if not check_links_up(duthost, port_attributes_dict): + link_check = check_links_up(duthost, port_attributes_dict) + if not link_check["passed"]: logger.warning("Validation on Start FAILED: some ports are down") else: appl_db = sdbHelp(duthost) diff --git a/tests/transceiver/system/process_restart/test_syncd_restart.py b/tests/transceiver/system/process_restart/test_syncd_restart.py index 6cb847e1c5d..3072c4670da 100644 --- a/tests/transceiver/system/process_restart/test_syncd_restart.py +++ b/tests/transceiver/system/process_restart/test_syncd_restart.py @@ -68,7 +68,8 @@ def test_system_syncd_restart( health_baseline = capture_baseline(duthost) logger.info("Recording link states and uptime for %d port(s)", len(ports)) - if not check_links_up(duthost, port_attributes_dict): + link_check = check_links_up(duthost, port_attributes_dict) + if not link_check["passed"]: logger.warning("Validation on Start FAILED: some ports are down") else: appl_db = sdbHelp(duthost) From 52badb8b031762b753711fe2f0d3e490bb0c173a Mon Sep 17 00:00:00 2001 From: sutter Date: Fri, 7 Aug 2026 11:26:00 -0700 Subject: [PATCH 09/16] Backported Simon's db helpers Signed-off-by: sutter --- tests/transceiver/common/db_helpers.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/tests/transceiver/common/db_helpers.py b/tests/transceiver/common/db_helpers.py index 20bcb0b533a..b08d03fba7c 100644 --- a/tests/transceiver/common/db_helpers.py +++ b/tests/transceiver/common/db_helpers.py @@ -423,17 +423,5 @@ def get_config_db_port_names(duthost): table directly rather than the ``(value, err)`` tuple the per-port wrappers use; a facts-gather failure is an infra-level error and is allowed to raise. """ - port_table = {} - for namespace in duthost.get_frontend_asic_namespace_list(): - config_facts = duthost.config_facts( - host=duthost.hostname, - source="running", - namespace=namespace, - )["ansible_facts"] - port_table.update(config_facts.get("PORT") or {}) - return port_table - - -def get_config_db_port_names(duthost): - """Return the set of port names in the CONFIG_DB PORT table.""" - return set(get_config_db_port_table(duthost).keys()) + config_facts = duthost.get_running_config_facts() + return set(config_facts.get("PORT", {}).keys()) \ No newline at end of file From 80694b51fcec9a34cf946c365c9cd2dcef1f1c8f Mon Sep 17 00:00:00 2001 From: sutter Date: Fri, 7 Aug 2026 11:47:52 -0700 Subject: [PATCH 10/16] db update Signed-off-by: sutter --- tests/transceiver/common/db_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/transceiver/common/db_helpers.py b/tests/transceiver/common/db_helpers.py index b08d03fba7c..22cad7bf8f5 100644 --- a/tests/transceiver/common/db_helpers.py +++ b/tests/transceiver/common/db_helpers.py @@ -424,4 +424,4 @@ def get_config_db_port_names(duthost): use; a facts-gather failure is an infra-level error and is allowed to raise. """ config_facts = duthost.get_running_config_facts() - return set(config_facts.get("PORT", {}).keys()) \ No newline at end of file + return set(config_facts.get("PORT", {}).keys()) From c709d7cd8c356f0058502510a76f3ac6032a2fc5 Mon Sep 17 00:00:00 2001 From: sutter Date: Fri, 7 Aug 2026 11:51:14 -0700 Subject: [PATCH 11/16] Updating db_helpers to simons latest PR - dupe bc last one had merge issues Signed-off-by: sutter --- tests/transceiver/common/db_helpers.py | 169 ++----------------------- 1 file changed, 11 insertions(+), 158 deletions(-) diff --git a/tests/transceiver/common/db_helpers.py b/tests/transceiver/common/db_helpers.py index 22cad7bf8f5..267e6864eb3 100644 --- a/tests/transceiver/common/db_helpers.py +++ b/tests/transceiver/common/db_helpers.py @@ -21,168 +21,21 @@ continue Bulk/once-per-test accessors read many rows in one shot: -:func:`get_config_db_port_table` and :func:`get_config_db_port_names` return -their values directly, while :func:`get_state_db_table` keeps the ``(value, -err)`` tuple so a dump failure can be surfaced as a clean per-test failure. +:func:`get_config_db_port_names` returns its value directly, while +:func:`get_state_db_table` keeps the ``(value, err)`` tuple so a dump failure can +be surfaced as a clean per-test failure. """ import ast import json import logging -import re -from datetime import datetime -from tests.common.helpers.sonic_db import STATE_DB from tests.transceiver.common.cli_parser_helper import RC_FAILURE logger = logging.getLogger(__name__) -STATE_DB_UPDATE_TIME_FIELD = "last_update_time" -STATE_DB_UPDATE_TIME_FUTURE_TOLERANCE_MIN = 0.1 -XCVRD_UPDATE_TIME_FORMAT = "%a %b %d %H:%M:%S %Y" - -_FLOAT_PATTERN = re.compile( - r"[-+]?(?:inf(?:inity)?|\d*\.?\d+(?:[eE][-+]?\d+)?)", - re.IGNORECASE, -) - - -def parse_numeric(value): - """Parse the first numeric token from a DB value. - - Supports regular floats plus ``inf`` / ``-inf`` forms such as ``-infdBm``. - Returns ``None`` for absent, N/A-like, or unparseable values. - """ - if value is None: - return None - - text = str(value).strip() - if not text or text.upper() in ("N/A", "NA", "NONE"): - return None - - match = _FLOAT_PATTERN.search(text) - if not match: - logger.debug("Could not parse numeric value from %r", value) - return None - - token = match.group(0).lower() - if token in ("inf", "+inf", "infinity", "+infinity"): - return float("inf") - if token in ("-inf", "-infinity"): - return float("-inf") - - try: - return float(match.group(0)) - except ValueError: - logger.debug( - "Could not convert numeric token %r from %r", - match.group(0), - value, - ) - return None - - -def parse_update_time(value): - """Parse an xcvrd UTC update timestamp.""" - if value is None: - return None - - raw = str(value).strip() - if not raw: - return None - - normalized = " ".join(raw.split()) - try: - return datetime.strptime(normalized, XCVRD_UPDATE_TIME_FORMAT) - except ValueError: - logger.debug( - "Could not parse xcvrd update timestamp %r with format %s", - raw, - XCVRD_UPDATE_TIME_FORMAT, - ) - return None - - -def resolve_port_namespace(duthost, port): - """Return the ASIC namespace for a logical port, or ``None`` on single-ASIC.""" - return duthost.get_port_asic_instance(port).namespace - - -def _entry_field_age_minutes(entry, now_utc): - """Return the configured update timestamp age in minutes, or ``None``.""" - if not entry: - return None - - parsed_time = parse_update_time(entry.get(STATE_DB_UPDATE_TIME_FIELD)) - if parsed_time is None: - return None - - return (now_utc - parsed_time).total_seconds() / 60.0 - - -def check_entry_freshness( - entry, - max_age_min, - now_utc, - table_name="STATE_DB entry", -): - """Validate entry freshness and return failures plus the computed age. - - The timestamp is parsed once, and callers can use the returned age for - logging without re-parsing the same entry value. - """ - result = { - "failures": [], - "age_minutes": _entry_field_age_minutes(entry, now_utc), - } - - if max_age_min is None: - return result - - if not entry: - result["failures"].append( - "missing {} data for {} freshness check".format( - table_name, - STATE_DB_UPDATE_TIME_FIELD, - ) - ) - return result - - try: - max_age = float(max_age_min) - except (TypeError, ValueError): - result["failures"].append( - "invalid data_max_age_min={!r}".format(max_age_min) - ) - return result - - age_minutes = result["age_minutes"] - if age_minutes is None: - result["failures"].append( - "{} missing or unparsable while data_max_age_min is configured".format( - STATE_DB_UPDATE_TIME_FIELD - ) - ) - return result - - if age_minutes < -float(STATE_DB_UPDATE_TIME_FUTURE_TOLERANCE_MIN): - result["failures"].append( - "{} is in the future (age_min={:.2f}, tolerance_min={:.2f})".format( - STATE_DB_UPDATE_TIME_FIELD, - age_minutes, - float(STATE_DB_UPDATE_TIME_FUTURE_TOLERANCE_MIN), - ) - ) - elif age_minutes > max_age: - result["failures"].append( - "{} too old (age_min={:.2f}, limit={})".format( - STATE_DB_UPDATE_TIME_FIELD, - age_minutes, - max_age_min, - ) - ) - - return result +# sonic-db-cli database identifiers (the first positional arg to sonic-db-cli). +STATE_DB = "STATE_DB" def parse_state_db_bool(value): @@ -414,14 +267,14 @@ def resolve_namespace(duthost, port): def get_config_db_port_names(duthost): """Return the set of port names in the CONFIG_DB PORT table. - Reads every frontend ASIC namespace so multi-ASIC DUTs include front-panel - Ethernet ports whose PORT entries live in per-ASIC CONFIG_DB instances. On - single-ASIC DUTs, ``get_frontend_asic_namespace_list`` returns ``[None]``, - so this keeps the default-namespace behavior. + Thin accessor over ``duthost.get_running_config_facts()`` (the ansible-facts + path SONiC exposes for the running CONFIG_DB). Returns an empty set when the + PORT table is absent/empty so the caller can decide whether that is a skip or + a failure. This is a once-per-test bulk read (not a per-port query), so it returns the - table directly rather than the ``(value, err)`` tuple the per-port wrappers + set directly rather than the ``(value, err)`` tuple the per-port wrappers use; a facts-gather failure is an infra-level error and is allowed to raise. """ config_facts = duthost.get_running_config_facts() - return set(config_facts.get("PORT", {}).keys()) + return set(config_facts.get("PORT", {}).keys()) \ No newline at end of file From 66942a5511323e4d4d6787df95557c205d7b8572 Mon Sep 17 00:00:00 2001 From: sutter Date: Fri, 7 Aug 2026 13:47:46 -0700 Subject: [PATCH 12/16] Trailing whitespace fixes Signed-off-by: sutter --- tests/common/platform/interface_utils.py | 2 +- tests/transceiver/common/db_helpers.py | 2 +- tests/transceiver/system/conftest.py | 2 +- tests/transceiver/system/process_restart/conftest.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/common/platform/interface_utils.py b/tests/common/platform/interface_utils.py index dad7d205249..b3a9eab59b1 100644 --- a/tests/common/platform/interface_utils.py +++ b/tests/common/platform/interface_utils.py @@ -462,4 +462,4 @@ def get_port_indexes_with_flat_memory(dut): port_indexes_with_flat_memory = dut.shell("python3 get_port_indexes_with_flat_memory.py")["stdout"] port_indexes_with_flat_memory = ast.literal_eval(port_indexes_with_flat_memory) logging.info(f"Port indexes with flat memory: {port_indexes_with_flat_memory}") - return port_indexes_with_flat_memory \ No newline at end of file + return port_indexes_with_flat_memory diff --git a/tests/transceiver/common/db_helpers.py b/tests/transceiver/common/db_helpers.py index 267e6864eb3..d65502a13c4 100644 --- a/tests/transceiver/common/db_helpers.py +++ b/tests/transceiver/common/db_helpers.py @@ -277,4 +277,4 @@ def get_config_db_port_names(duthost): use; a facts-gather failure is an infra-level error and is allowed to raise. """ config_facts = duthost.get_running_config_facts() - return set(config_facts.get("PORT", {}).keys()) \ No newline at end of file + return set(config_facts.get("PORT", {}).keys()) diff --git a/tests/transceiver/system/conftest.py b/tests/transceiver/system/conftest.py index 23e3484276f..6573ecfe99f 100644 --- a/tests/transceiver/system/conftest.py +++ b/tests/transceiver/system/conftest.py @@ -152,4 +152,4 @@ def _system_post_session_checks(duthost, port_attributes_dict): "Post-session LLDP check FAILED for: %s", ", ".join(lldp_failed) ) else: - logger.info("Post-session LLDP check PASSED") \ No newline at end of file + logger.info("Post-session LLDP check PASSED") diff --git a/tests/transceiver/system/process_restart/conftest.py b/tests/transceiver/system/process_restart/conftest.py index 8b4384040f2..a24cada02b6 100644 --- a/tests/transceiver/system/process_restart/conftest.py +++ b/tests/transceiver/system/process_restart/conftest.py @@ -11,4 +11,4 @@ # Opt into the cross-category session gates this category consumes. @pytest.fixture(autouse=True, scope="session") def _category_session_prerequisites(presence_verified, gold_fw_verified, links_verified): - return \ No newline at end of file + return From 71dcc91f3383e72d2fbfe9f3df3b3fae843f7a06 Mon Sep 17 00:00:00 2001 From: sutter Date: Fri, 7 Aug 2026 15:35:20 -0700 Subject: [PATCH 13/16] Updated pmon polling function calls Signed-off-by: sutter --- tests/transceiver/system/process_restart/test_swss_restart.py | 4 ++-- .../transceiver/system/process_restart/test_syncd_restart.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/transceiver/system/process_restart/test_swss_restart.py b/tests/transceiver/system/process_restart/test_swss_restart.py index 35d3c952dd7..364fe5ed059 100644 --- a/tests/transceiver/system/process_restart/test_swss_restart.py +++ b/tests/transceiver/system/process_restart/test_swss_restart.py @@ -37,7 +37,7 @@ standard_port_recovery_and_verification ) from tests.common.helpers.sonic_db import AppDbCli as sdbHelp -from tests.common.platform.processes_utils import check_process_up +from tests.common.platform.processes_utils import check_pmon_uptime_minutes logger = logging.getLogger(__name__) @@ -94,7 +94,7 @@ def test_system_swss_restart( ).get("expect_pmon_restart_with_swss_or_syncd", False): time.sleep(15) logger.info("Verifying pmon restart after swss restart...") - if check_process_up(duthost, 'pmon'): + if check_pmon_uptime_minutes(duthost, minimal_runtime=3): failures.append("[pmon] pmon did not restart as expected") logger.warning( "pmon FAILED to Restart when" diff --git a/tests/transceiver/system/process_restart/test_syncd_restart.py b/tests/transceiver/system/process_restart/test_syncd_restart.py index 3072c4670da..27c53141238 100644 --- a/tests/transceiver/system/process_restart/test_syncd_restart.py +++ b/tests/transceiver/system/process_restart/test_syncd_restart.py @@ -37,7 +37,7 @@ standard_port_recovery_and_verification ) from tests.common.helpers.sonic_db import AppDbCli as sdbHelp -from tests.common.platform.processes_utils import check_process_up +from tests.common.platform.processes_utils import check_pmon_uptime_minutes logger = logging.getLogger(__name__) @@ -94,7 +94,7 @@ def test_system_syncd_restart( ).get("expect_pmon_restart_with_swss_or_syncd", False): time.sleep(15) logger.info("Verifying pmon restart after syncd restart...") - if check_process_up(duthost, 'pmon'): + if check_pmon_uptime_minutes(duthost, minimal_runtime=3): failures.append("[pmon] pmon did not restart as expected") logger.warning( "pmon FAILED to restart when" From 820a0293670203f9a26ead5a18e8fe4f5f94a908 Mon Sep 17 00:00:00 2001 From: sutter Date: Fri, 7 Aug 2026 16:08:07 -0700 Subject: [PATCH 14/16] Restoring deleted functions Signed-off-by: sutter --- tests/transceiver/common/db_helpers.py | 151 +++++++++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/tests/transceiver/common/db_helpers.py b/tests/transceiver/common/db_helpers.py index d65502a13c4..62e448fa8c1 100644 --- a/tests/transceiver/common/db_helpers.py +++ b/tests/transceiver/common/db_helpers.py @@ -28,11 +28,162 @@ import ast import json import logging +import re +from datetime import datetime + +from tests.common.helpers.sonic_db import STATE_DB from tests.transceiver.common.cli_parser_helper import RC_FAILURE logger = logging.getLogger(__name__) +STATE_DB_UPDATE_TIME_FIELD = "last_update_time" +STATE_DB_UPDATE_TIME_FUTURE_TOLERANCE_MIN = 0.1 +XCVRD_UPDATE_TIME_FORMAT = "%a %b %d %H:%M:%S %Y" + +_FLOAT_PATTERN = re.compile( + r"[-+]?(?:inf(?:inity)?|\d*\.?\d+(?:[eE][-+]?\d+)?)", + re.IGNORECASE, +) + + +def parse_numeric(value): + """Parse the first numeric token from a DB value. + + Supports regular floats plus ``inf`` / ``-inf`` forms such as ``-infdBm``. + Returns ``None`` for absent, N/A-like, or unparseable values. + """ + if value is None: + return None + + text = str(value).strip() + if not text or text.upper() in ("N/A", "NA", "NONE"): + return None + + match = _FLOAT_PATTERN.search(text) + if not match: + logger.debug("Could not parse numeric value from %r", value) + return None + + token = match.group(0).lower() + if token in ("inf", "+inf", "infinity", "+infinity"): + return float("inf") + if token in ("-inf", "-infinity"): + return float("-inf") + + try: + return float(match.group(0)) + except ValueError: + logger.debug( + "Could not convert numeric token %r from %r", + match.group(0), + value, + ) + return None + + +def parse_update_time(value): + """Parse an xcvrd UTC update timestamp.""" + if value is None: + return None + + raw = str(value).strip() + if not raw: + return None + + normalized = " ".join(raw.split()) + try: + return datetime.strptime(normalized, XCVRD_UPDATE_TIME_FORMAT) + except ValueError: + logger.debug( + "Could not parse xcvrd update timestamp %r with format %s", + raw, + XCVRD_UPDATE_TIME_FORMAT, + ) + return None + + +def resolve_port_namespace(duthost, port): + """Return the ASIC namespace for a logical port, or ``None`` on single-ASIC.""" + return duthost.get_port_asic_instance(port).namespace + + +def _entry_field_age_minutes(entry, now_utc): + """Return the configured update timestamp age in minutes, or ``None``.""" + if not entry: + return None + + parsed_time = parse_update_time(entry.get(STATE_DB_UPDATE_TIME_FIELD)) + if parsed_time is None: + return None + + return (now_utc - parsed_time).total_seconds() / 60.0 + + +def check_entry_freshness( + entry, + max_age_min, + now_utc, + table_name="STATE_DB entry", +): + """Validate entry freshness and return failures plus the computed age. + + The timestamp is parsed once, and callers can use the returned age for + logging without re-parsing the same entry value. + """ + result = { + "failures": [], + "age_minutes": _entry_field_age_minutes(entry, now_utc), + } + + if max_age_min is None: + return result + + if not entry: + result["failures"].append( + "missing {} data for {} freshness check".format( + table_name, + STATE_DB_UPDATE_TIME_FIELD, + ) + ) + return result + + try: + max_age = float(max_age_min) + except (TypeError, ValueError): + result["failures"].append( + "invalid data_max_age_min={!r}".format(max_age_min) + ) + return result + + age_minutes = result["age_minutes"] + if age_minutes is None: + result["failures"].append( + "{} missing or unparsable while data_max_age_min is configured".format( + STATE_DB_UPDATE_TIME_FIELD + ) + ) + return result + + if age_minutes < -float(STATE_DB_UPDATE_TIME_FUTURE_TOLERANCE_MIN): + result["failures"].append( + "{} is in the future (age_min={:.2f}, tolerance_min={:.2f})".format( + STATE_DB_UPDATE_TIME_FIELD, + age_minutes, + float(STATE_DB_UPDATE_TIME_FUTURE_TOLERANCE_MIN), + ) + ) + elif age_minutes > max_age: + result["failures"].append( + "{} too old (age_min={:.2f}, limit={})".format( + STATE_DB_UPDATE_TIME_FIELD, + age_minutes, + max_age_min, + ) + ) + + return result + # sonic-db-cli database identifiers (the first positional arg to sonic-db-cli). STATE_DB = "STATE_DB" From 9d63b5b8ea56e6e56c4c4e7f7f640fcbba2e66c8 Mon Sep 17 00:00:00 2001 From: sutter Date: Fri, 7 Aug 2026 16:19:55 -0700 Subject: [PATCH 15/16] removed dupe definition Signed-off-by: sutter --- tests/transceiver/common/db_helpers.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/transceiver/common/db_helpers.py b/tests/transceiver/common/db_helpers.py index 62e448fa8c1..bbc96aba399 100644 --- a/tests/transceiver/common/db_helpers.py +++ b/tests/transceiver/common/db_helpers.py @@ -184,11 +184,6 @@ def check_entry_freshness( return result - -# sonic-db-cli database identifiers (the first positional arg to sonic-db-cli). -STATE_DB = "STATE_DB" - - def parse_state_db_bool(value): """Parse a STATE_DB string into a Python bool, or ``None`` if unrecognized. From 84720c773927ce6da94050c1886d15e10592a25b Mon Sep 17 00:00:00 2001 From: sutter Date: Fri, 7 Aug 2026 16:26:12 -0700 Subject: [PATCH 16/16] Added a blank line Signed-off-by: sutter --- tests/transceiver/common/db_helpers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/transceiver/common/db_helpers.py b/tests/transceiver/common/db_helpers.py index bbc96aba399..2de4fe9cd84 100644 --- a/tests/transceiver/common/db_helpers.py +++ b/tests/transceiver/common/db_helpers.py @@ -184,6 +184,7 @@ def check_entry_freshness( return result + def parse_state_db_bool(value): """Parse a STATE_DB string into a Python bool, or ``None`` if unrecognized.