From 8f8d81201d886d857c0d4f11053f7e45af1cd538 Mon Sep 17 00:00:00 2001 From: GauravNagesh Date: Mon, 3 Nov 2025 04:13:14 +0000 Subject: [PATCH 1/3] psud: redis performance improvements --- sonic-psud/scripts/psud | 140 ++++++++++++++++++++++++++++------------ 1 file changed, 98 insertions(+), 42 deletions(-) diff --git a/sonic-psud/scripts/psud b/sonic-psud/scripts/psud index 3f8c29eab..9469174a3 100644 --- a/sonic-psud/scripts/psud +++ b/sonic-psud/scripts/psud @@ -251,7 +251,10 @@ class PsuChassisInfo(logger.Logger): # Record in state DB in chassis table fvs[dict_index] = (CHASSIS_INFO_TOTAL_POWER_SUPPLIED_FIELD, str(self.total_supplied_power)) fvs[dict_index + 1] = (CHASSIS_INFO_TOTAL_POWER_CONSUMED_FIELD, str(self.total_consumed_power)) - chassis_tbl.set(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1), fvs) + try: + chassis_tbl.set(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1), fvs) + except Exception as e: + self.log_error(f"Exception occurred while writing chassis power info to Redis : {type(e).__name__} : {e}") def update_master_status(self): set_led = self.first_run @@ -296,6 +299,7 @@ class PsuStatus(object): self.check_psu_power_threshold = False self.power_exceeded_threshold = False self.logger = logger + self.led_status = NOT_AVAILABLE def set_presence(self, presence): """ @@ -358,6 +362,18 @@ class PsuStatus(object): self.power_exceeded_threshold = power_exceeded_threshold return True + def set_led_status(self, led_status): + """ + Set and cache PSU LED status + :param led_status: PSU LED status + :return: True if status changed else False + """ + if led_status == self.led_status: + return False + + self.led_status = led_status + return True + def is_ok(self): return self.presence and self.power_good and self.voltage_good and self.temperature_good @@ -401,25 +417,36 @@ class DaemonPsud(daemon_base.DaemonBase): self.log_error("Failed to load psuutil: %s" % (str(e)), True) sys.exit(PSUUTIL_LOAD_ERROR) - # Connect to STATE_DB and create psu/chassis info tables - state_db = daemon_base.db_connect("STATE_DB") - self.chassis_tbl = swsscommon.Table(state_db, CHASSIS_INFO_TABLE) - self.psu_tbl = swsscommon.Table(state_db, PSU_INFO_TABLE) - self.fan_tbl = swsscommon.Table(state_db, FAN_INFO_TABLE) - self.phy_entity_tbl = swsscommon.Table(state_db, PHYSICAL_ENTITY_INFO_TABLE) + try: + # Connect to STATE_DB and create psu/chassis info tables + state_db = daemon_base.db_connect("STATE_DB") + self.statedb_redisPipeline = swsscommon.RedisPipeline(state_db, 10) + self.chassis_tbl = swsscommon.Table(self.statedb_redisPipeline, CHASSIS_INFO_TABLE, True) + self.psu_tbl = swsscommon.Table(self.statedb_redisPipeline, PSU_INFO_TABLE, True) + self.fan_tbl = swsscommon.Table(self.statedb_redisPipeline, FAN_INFO_TABLE, True) + self.phy_entity_tbl = swsscommon.Table(self.statedb_redisPipeline, PHYSICAL_ENTITY_INFO_TABLE, True) + except Exception as e: + self.log_error(f"Failed to connect to STATE_DB or create RedisPipeline : {type(e).__name__} : {e}", True) + sys.exit(exit_code+2) # Post psu number info to STATE_DB self.num_psus = _wrapper_get_num_psus() fvs = swsscommon.FieldValuePairs([(CHASSIS_INFO_PSU_NUM_FIELD, str(self.num_psus))]) - self.chassis_tbl.set(CHASSIS_INFO_KEY, fvs) + try: + self.chassis_tbl.set(CHASSIS_INFO_KEY, fvs) + except Exception as e: + self.log_error(f"Exception occurred while writing PSU number info to Redis : {type(e).__name__} : {e}") def __del__(self): # Delete all the information from DB and then exit - for psu_index in range(1, self.num_psus + 1): - self.psu_tbl._del(get_psu_key(psu_index)) + try: + for psu_index in range(1, self.num_psus + 1): + self.psu_tbl._del(get_psu_key(psu_index)) - self.chassis_tbl._del(CHASSIS_INFO_KEY) - self.chassis_tbl._del(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1)) + self.chassis_tbl._del(CHASSIS_INFO_KEY) + self.chassis_tbl._del(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1)) + except Exception as e: + self.log_error(f"Exception occurred while deleting tables in Redis : {type(e).__name__} : {e}") # Override signal handler from DaemonBase def signal_handler(self, sig, frame): @@ -459,6 +486,11 @@ class DaemonPsud(daemon_base.DaemonBase): if self.first_run: self.first_run = False + # flush any remaining requests on the pipeline to STAT_DB at end of every cycle + try: + self.statedb_redisPipeline.flush() + except Exception as e: + self.log_error(f"Exception occurred while flushing Redis pipeline : {type(e).__name__} : {e}") return True def update_psu_data(self): @@ -542,6 +574,7 @@ class DaemonPsud(daemon_base.DaemonBase): power_exceeded_threshold = psu_status.power_exceeded_threshold power_warning_suppress_threshold = try_get(psu.get_psu_power_warning_suppress_threshold, NOT_AVAILABLE) power_critical_threshold = try_get(psu.get_psu_power_critical_threshold, NOT_AVAILABLE) + power_exceeded_changed = False if psu_status.check_psu_power_threshold: # Calculate total power system_power = float(power) @@ -568,7 +601,8 @@ class DaemonPsud(daemon_base.DaemonBase): # Raise alarm power_exceeded_threshold = True - if psu_status.set_power_exceed_threshold(power_exceeded_threshold) and not self.psu_threshold_exceeded_logged: + power_exceeded_changed = psu_status.set_power_exceed_threshold(power_exceeded_threshold) + if power_exceeded_changed and not self.psu_threshold_exceeded_logged: # Since this is a system level PSU power exceeding check, we do not need to log it for each PSU log_on_status_changed(self, not psu_status.power_exceeded_threshold, 'PSU power warning cleared: system power {} is back to normal, below the warning suppress threshold {}.'.format(system_power, power_warning_suppress_threshold), @@ -594,28 +628,36 @@ class DaemonPsud(daemon_base.DaemonBase): if set_led: self._set_psu_led(psu, psu_status) - fvs = swsscommon.FieldValuePairs( - [(PSU_INFO_MODEL_FIELD, str(try_get(psu.get_model, NOT_AVAILABLE))), - (PSU_INFO_SERIAL_FIELD, str(try_get(psu.get_serial, NOT_AVAILABLE))), - (PSU_INFO_REV_FIELD, str(try_get(psu.get_revision, NOT_AVAILABLE))), - (PSU_INFO_TEMP_FIELD, str(temperature)), - (PSU_INFO_TEMP_TH_FIELD, str(temperature_threshold)), - (PSU_INFO_VOLTAGE_FIELD, str(voltage)), - (PSU_INFO_VOLTAGE_MIN_TH_FIELD, str(voltage_low_threshold)), - (PSU_INFO_VOLTAGE_MAX_TH_FIELD, str(voltage_high_threshold)), - (PSU_INFO_CURRENT_FIELD, str(current)), - (PSU_INFO_POWER_FIELD, str(power)), - (PSU_INFO_POWER_WARNING_SUPPRESS_THRESHOLD, str(power_warning_suppress_threshold)), - (PSU_INFO_POWER_CRITICAL_THRESHOLD, str(power_critical_threshold)), - (PSU_INFO_POWER_OVERLOAD, str(power_exceeded_threshold)), - (PSU_INFO_FRU_FIELD, str(is_replaceable)), - (PSU_INFO_IN_CURRENT_FIELD, str(in_current)), - (PSU_INFO_IN_VOLTAGE_FIELD, str(in_voltage)), - (PSU_INFO_POWER_MAX_FIELD, str(max_power)), - (PSU_INFO_PRESENCE_FIELD, 'true' if _wrapper_get_psu_presence(index) else 'false'), - (PSU_INFO_STATUS_FIELD, 'true' if _wrapper_get_psu_status(index) else 'false'), - ]) - self.psu_tbl.set(name, fvs) + # Only add rarely-changing fields if changed or first run while always adding the frequently-changing fields + fvs = swsscommon.FieldValuePairs() + if presence_changed or self.first_run: + fvs.append((PSU_INFO_MODEL_FIELD, str(try_get(psu.get_model, NOT_AVAILABLE)))) + fvs.append((PSU_INFO_SERIAL_FIELD, str(try_get(psu.get_serial, NOT_AVAILABLE)))) + fvs.append((PSU_INFO_REV_FIELD, str(try_get(psu.get_revision, NOT_AVAILABLE)))) + fvs.append((PSU_INFO_PRESENCE_FIELD, 'true' if presence else 'false')) + fvs.append((PSU_INFO_FRU_FIELD, str(is_replaceable))) + fvs.append((PSU_INFO_TEMP_TH_FIELD, str(temperature_threshold))) + fvs.append((PSU_INFO_VOLTAGE_MIN_TH_FIELD, str(voltage_low_threshold))) + fvs.append((PSU_INFO_VOLTAGE_MAX_TH_FIELD, str(voltage_high_threshold))) + if power_good_changed or self.first_run: + fvs.append((PSU_INFO_STATUS_FIELD, 'true' if power_good else 'false')) + if power_exceeded_changed or self.first_run: + fvs.append((PSU_INFO_POWER_OVERLOAD, str(power_exceeded_threshold))) + + fvs.append((PSU_INFO_TEMP_FIELD, str(temperature))) + fvs.append((PSU_INFO_VOLTAGE_FIELD, str(voltage))) + fvs.append((PSU_INFO_CURRENT_FIELD, str(current))) + fvs.append((PSU_INFO_POWER_FIELD, str(power))) + fvs.append((PSU_INFO_POWER_WARNING_SUPPRESS_THRESHOLD, str(power_warning_suppress_threshold))) + fvs.append((PSU_INFO_POWER_CRITICAL_THRESHOLD, str(power_critical_threshold))) + fvs.append((PSU_INFO_IN_CURRENT_FIELD, str(in_current))) + fvs.append((PSU_INFO_IN_VOLTAGE_FIELD, str(in_voltage))) + fvs.append((PSU_INFO_POWER_MAX_FIELD, str(max_power))) + + try: + self.psu_tbl.set(name, fvs) + except Exception as e: + self.log_error(f"Exception occurred while updating PSU data for {name} to Redis : {type(e).__name__} : {e}") def _update_psu_entity_info(self): if not platform_chassis: @@ -633,7 +675,10 @@ class DaemonPsud(daemon_base.DaemonBase): [('position_in_parent', str(position)), ('parent_name', CHASSIS_INFO_KEY), ]) - self.phy_entity_tbl.set(get_psu_key(psu_index), fvs) + try: + self.phy_entity_tbl.set(get_psu_key(psu_index), fvs) + except Exception as e: + self.log_error(f"Exception occurred while writing PSU entity info to Redis : {type(e).__name__} : {e}") def _update_psu_fan_data(self, psu, psu_index): """ @@ -656,7 +701,10 @@ class DaemonPsud(daemon_base.DaemonBase): (FAN_INFO_SPEED_FIELD, str(speed)), (FAN_INFO_TIMESTAMP_FIELD, datetime.now().strftime('%Y%m%d %H:%M:%S')) ]) - self.fan_tbl.set(fan_name, fvs) + try: + self.fan_tbl.set(fan_name, fvs) + except Exception as e: + self.log_error(f"Exception occurred while writing PSU fan info to Redis : {type(e).__name__} : {e}") def _set_psu_led(self, psu, psu_status): try: @@ -670,10 +718,15 @@ class DaemonPsud(daemon_base.DaemonBase): return for index, psu_status in self.psu_status_dict.items(): - fvs = swsscommon.FieldValuePairs([ - ('led_status', str(try_get(psu_status.psu.get_status_led, NOT_AVAILABLE))) - ]) - self.psu_tbl.set(get_psu_key(index), fvs) + led_status = try_get(psu_status.psu.get_status_led, NOT_AVAILABLE) + if psu_status.set_led_status(led_status): + fvs = swsscommon.FieldValuePairs([ + ('led_status', str(led_status)) + ]) + try: + self.psu_tbl.set(get_psu_key(index), fvs) + except Exception as e: + self.log_error(f"Exception occurred while writing PSU LED status to Redis : {type(e).__name__} : {e}") self._update_psu_fan_led_status(psu_status.psu, index) def _update_psu_fan_led_status(self, psu, psu_index): @@ -684,7 +737,10 @@ class DaemonPsud(daemon_base.DaemonBase): fvs = swsscommon.FieldValuePairs([ (FAN_INFO_LED_STATUS_FIELD, str(try_get(fan.get_status_led, NOT_AVAILABLE))) ]) - self.fan_tbl.set(fan_name, fvs) + try: + self.fan_tbl.set(fan_name, fvs) + except Exception as e: + self.log_error(f"Exception occurred while writing PSU fan LED status to Redis : {type(e).__name__} : {e}") def update_psu_chassis_info(self): if not platform_chassis: From 4644924fd27a4494067778b3304de80cd10d8062 Mon Sep 17 00:00:00 2001 From: GauravNagesh Date: Mon, 3 Nov 2025 04:18:05 +0000 Subject: [PATCH 2/3] psud: modifying testcases for new functionality and modifying mocked_libs for RedisPipeline and FieldValuePairs --- .../mocked_libs/swsscommon/swsscommon.py | 27 ++- sonic-psud/tests/test_DaemonPsud.py | 172 ++++++++++++++---- 2 files changed, 160 insertions(+), 39 deletions(-) diff --git a/sonic-psud/tests/mocked_libs/swsscommon/swsscommon.py b/sonic-psud/tests/mocked_libs/swsscommon/swsscommon.py index 1da16aa2e..a7d1792dc 100644 --- a/sonic-psud/tests/mocked_libs/swsscommon/swsscommon.py +++ b/sonic-psud/tests/mocked_libs/swsscommon/swsscommon.py @@ -6,9 +6,20 @@ STATE_DB = '' +class RedisPipeline: + def __init__(self, db, batch_size=128): + self.db = db + self.batch_size = batch_size + self.queue = [] + + def flush(self): + # Mock flush operation - just clear the queue + self.queue.clear() + pass class Table: - def __init__(self, db, table_name): + def __init__(self, db_or_pipeline, table_name, buffered=False): + # Mock to support both both constructors (db, table_name) and (pipeline, table_name, buffered) self.table_name = table_name self.mock_dict = {} @@ -34,11 +45,17 @@ def get_size(self): class FieldValuePairs: - fv_dict = {} - - def __init__(self, tuple_list): - if isinstance(tuple_list, list) and isinstance(tuple_list[0], tuple): + def __init__(self, tuple_list=None): + # Mock to support both both constructors FieldValuePairs() and FieldValuePairs(tuple_list) + if tuple_list is None: + self.fv_dict = {} + elif isinstance(tuple_list, list) and len(tuple_list) > 0 and isinstance(tuple_list[0], tuple): self.fv_dict = dict(tuple_list) + else: + self.fv_dict = {} + + def append(self, kv_tuple): + self.fv_dict[kv_tuple[0]] = kv_tuple[1] def __setitem__(self, key, kv_tuple): self.fv_dict[kv_tuple[0]] = kv_tuple[1] diff --git a/sonic-psud/tests/test_DaemonPsud.py b/sonic-psud/tests/test_DaemonPsud.py index ca866eca0..5a21ce9d0 100644 --- a/sonic-psud/tests/test_DaemonPsud.py +++ b/sonic-psud/tests/test_DaemonPsud.py @@ -143,28 +143,45 @@ def test_update_psu_data(self): expected_calls = [mock.call("Failed to update PSU data - Test message")] * 2 assert daemon_psud.log_warning.mock_calls == expected_calls - def _construct_expected_fvp(self, power=100.0, power_warning_suppress_threshold='N/A', power_critical_threshold='N/A', power_overload=False): - expected_fvp = psud.swsscommon.FieldValuePairs( - [(psud.PSU_INFO_MODEL_FIELD, 'Fake Model'), - (psud.PSU_INFO_SERIAL_FIELD, '12345678'), - (psud.PSU_INFO_REV_FIELD, '1234'), - (psud.PSU_INFO_TEMP_FIELD, '30.0'), - (psud.PSU_INFO_TEMP_TH_FIELD, '50.0'), - (psud.PSU_INFO_VOLTAGE_FIELD, '12.0'), - (psud.PSU_INFO_VOLTAGE_MIN_TH_FIELD, '11.0'), - (psud.PSU_INFO_VOLTAGE_MAX_TH_FIELD, '13.0'), - (psud.PSU_INFO_CURRENT_FIELD, '8.0'), - (psud.PSU_INFO_POWER_FIELD, str(power)), - (psud.PSU_INFO_POWER_WARNING_SUPPRESS_THRESHOLD, str(power_warning_suppress_threshold)), - (psud.PSU_INFO_POWER_CRITICAL_THRESHOLD, str(power_critical_threshold)), - (psud.PSU_INFO_POWER_OVERLOAD, str(power_overload)), - (psud.PSU_INFO_FRU_FIELD, 'True'), - (psud.PSU_INFO_IN_VOLTAGE_FIELD, '220.25'), - (psud.PSU_INFO_IN_CURRENT_FIELD, '0.72'), - (psud.PSU_INFO_POWER_MAX_FIELD, 'N/A'), - (psud.PSU_INFO_PRESENCE_FIELD, 'true'), - (psud.PSU_INFO_STATUS_FIELD, 'true'), - ]) + def _construct_expected_fvp(self, power=100.0, power_warning_suppress_threshold='N/A', power_critical_threshold='N/A', power_overload=False, first_run=True, power_overload_changed=False, power_good=True, power_good_changed=False, presence=True, presence_changed=False): + # Build field list based on what should be included + fv_list = [] + + # Rarely-changing fields (only on first run or when presence changed) + if first_run or presence_changed: + fv_list.extend([ + (psud.PSU_INFO_MODEL_FIELD, 'Fake Model'), + (psud.PSU_INFO_SERIAL_FIELD, '12345678'), + (psud.PSU_INFO_REV_FIELD, '1234'), + (psud.PSU_INFO_PRESENCE_FIELD, 'true' if presence else 'false'), + (psud.PSU_INFO_FRU_FIELD, 'True'), + (psud.PSU_INFO_TEMP_TH_FIELD, '50.0'), + (psud.PSU_INFO_VOLTAGE_MIN_TH_FIELD, '11.0'), + (psud.PSU_INFO_VOLTAGE_MAX_TH_FIELD, '13.0'), + ]) + + # Power good status (only on first run or when power_good changed) + if first_run or power_good_changed: + fv_list.append((psud.PSU_INFO_STATUS_FIELD, 'true' if power_good else 'false')) + + # Power overload (only on first run or when power_exceeded changed) + if first_run or power_overload_changed: + fv_list.append((psud.PSU_INFO_POWER_OVERLOAD, str(power_overload))) + + # Frequently-changing fields (always included) + fv_list.extend([ + (psud.PSU_INFO_TEMP_FIELD, '30.0'), + (psud.PSU_INFO_VOLTAGE_FIELD, '12.0'), + (psud.PSU_INFO_CURRENT_FIELD, '8.0'), + (psud.PSU_INFO_POWER_FIELD, str(power)), + (psud.PSU_INFO_POWER_WARNING_SUPPRESS_THRESHOLD, str(power_warning_suppress_threshold)), + (psud.PSU_INFO_POWER_CRITICAL_THRESHOLD, str(power_critical_threshold)), + (psud.PSU_INFO_IN_CURRENT_FIELD, '0.72'), + (psud.PSU_INFO_IN_VOLTAGE_FIELD, '220.25'), + (psud.PSU_INFO_POWER_MAX_FIELD, 'N/A'), + ]) + + expected_fvp = psud.swsscommon.FieldValuePairs(fv_list) return expected_fvp @mock.patch('psud._wrapper_get_psu_presence', mock.MagicMock()) @@ -205,7 +222,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(100.0, 120.0, 130.0, False) + expected_fvp = self._construct_expected_fvp(100.0, 120.0, 130.0, False, first_run=True, power_overload_changed=False) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -218,7 +235,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(115.0, 120.0, 130.0, False) + expected_fvp = self._construct_expected_fvp(115.0, 120.0, 130.0, False, first_run=False, power_overload_changed=False) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -229,7 +246,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(125.0, 120.0, 130.0, True) + expected_fvp = self._construct_expected_fvp(125.0, 120.0, 130.0, True, first_run=False, power_overload_changed=True) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -240,7 +257,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(115.0, 120.0, 130.0, True) + expected_fvp = self._construct_expected_fvp(115.0, 120.0, 130.0, True, first_run=False, power_overload_changed=False) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -251,7 +268,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False) + expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False, first_run=False, power_overload_changed=True) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() daemon_psud._update_led_color() @@ -262,7 +279,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(125.0, 120.0, 130.0, True) + expected_fvp = self._construct_expected_fvp(125.0, 120.0, 130.0, True, first_run=False, power_overload_changed=True) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -273,7 +290,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False) + expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False, first_run=False, power_overload_changed=True, power_good=True, power_good_changed=False) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -281,49 +298,90 @@ def test_power_threshold(self): # PSU power becomes down psu.set_status(False) daemon_psud._update_single_psu_data(1, psu) - daemon_psud._update_led_color() assert not daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold + expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False, first_run=False, power_overload_changed=False, power_good=False, power_good_changed=True) + daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_RED == psu.get_status_led() # PSU power becomes up psu.set_status(True) daemon_psud._update_single_psu_data(1, psu) - daemon_psud._update_led_color() assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold + expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False, first_run=False, power_overload_changed=False, power_good=True, power_good_changed=True) + daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() # PSU becomes absent psu.set_presence(False) daemon_psud._update_single_psu_data(1, psu) - daemon_psud._update_led_color() assert not daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold + # When presence is False, most values become NOT_AVAILABLE + # BUT power thresholds are still fetched from PSU has these data fields according to psu object we created in our testcase above + expected_fvp = psud.swsscommon.FieldValuePairs([ + (psud.PSU_INFO_MODEL_FIELD, 'Fake Model'), + (psud.PSU_INFO_SERIAL_FIELD, '12345678'), + (psud.PSU_INFO_REV_FIELD, '1234'), + (psud.PSU_INFO_PRESENCE_FIELD, 'false'), + (psud.PSU_INFO_FRU_FIELD, 'True'), + (psud.PSU_INFO_TEMP_TH_FIELD, 'N/A'), + (psud.PSU_INFO_VOLTAGE_MIN_TH_FIELD, 'N/A'), + (psud.PSU_INFO_VOLTAGE_MAX_TH_FIELD, 'N/A'), + (psud.PSU_INFO_STATUS_FIELD, 'false'), + (psud.PSU_INFO_TEMP_FIELD, 'N/A'), + (psud.PSU_INFO_VOLTAGE_FIELD, 'N/A'), + (psud.PSU_INFO_CURRENT_FIELD, 'N/A'), + (psud.PSU_INFO_POWER_FIELD, 'N/A'), + (psud.PSU_INFO_POWER_WARNING_SUPPRESS_THRESHOLD, '120.0'), + (psud.PSU_INFO_POWER_CRITICAL_THRESHOLD, '130.0'), + (psud.PSU_INFO_IN_CURRENT_FIELD, 'N/A'), + (psud.PSU_INFO_IN_VOLTAGE_FIELD, 'N/A'), + (psud.PSU_INFO_POWER_MAX_FIELD, 'N/A'), + ]) + daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_RED == psu.get_status_led() # PSU becomes present psu.set_presence(True) daemon_psud._update_single_psu_data(1, psu) - daemon_psud._update_led_color() assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold + expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False, first_run=False, power_overload_changed=False, power_good=True, power_good_changed=True, presence=True, presence_changed=True) + daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() # Thresholds become invalid on the fly + # Critical threshold becomes invalid (NotImplementedError) psu.get_psu_power_critical_threshold = mock.MagicMock(side_effect=NotImplementedError('')) daemon_psud._update_single_psu_data(1, psu) assert not daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold + # When threshold becomes N/A, it's still written to the table + expected_fvp = self._construct_expected_fvp(105.0, 120.0, 'N/A', False, first_run=False, power_overload_changed=False, power_good=True, power_good_changed=False, presence=True, presence_changed=False) + daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + + # Critical threshold becomes valid again psu.get_psu_power_critical_threshold = mock.MagicMock(return_value=120.0) daemon_psud.psu_status_dict[1].check_psu_power_threshold = True daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold + expected_fvp = self._construct_expected_fvp(105.0, 120.0, 120.0, False, first_run=False, power_overload_changed=False, power_good=True, power_good_changed=False, presence=True, presence_changed=False) + daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + + # Warning threshold becomes invalid (NotImplementedError) psu.get_psu_power_warning_suppress_threshold = mock.MagicMock(side_effect=NotImplementedError('')) daemon_psud._update_single_psu_data(1, psu) assert not daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold + expected_fvp = self._construct_expected_fvp(105.0, 'N/A', 120.0, False, first_run=False, power_overload_changed=False, power_good=True, power_good_changed=False, presence=True, presence_changed=False) + daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) def test_set_psu_led(self): mock_logger = mock.MagicMock() @@ -389,6 +447,7 @@ def test_update_led_color(self): assert daemon_psud.psu_tbl.set.call_count == 0 assert daemon_psud._update_psu_fan_led_status.call_count == 0 + # First run: LED status is OFF, should write to DB (initial state) psud.platform_chassis = MockChassis() daemon_psud.psu_status_dict[1] = psu_status expected_fvp = psud.swsscommon.FieldValuePairs([('led_status', MockPsu.STATUS_LED_COLOR_OFF)]) @@ -401,6 +460,15 @@ def test_update_led_color(self): daemon_psud.psu_tbl.reset_mock() daemon_psud._update_psu_fan_led_status.reset_mock() + # Second run: LED status is still OFF, should NOT write to DB (no change) + daemon_psud._update_led_color() + assert daemon_psud.psu_tbl.set.call_count == 0 + assert daemon_psud._update_psu_fan_led_status.call_count == 1 # Fan LED status still checked + + daemon_psud.psu_tbl.reset_mock() + daemon_psud._update_psu_fan_led_status.reset_mock() + + # LED changes from OFF to GREEN, should write to DB mock_psu.set_status_led(MockPsu.STATUS_LED_COLOR_GREEN) expected_fvp = psud.swsscommon.FieldValuePairs([('led_status', MockPsu.STATUS_LED_COLOR_GREEN)]) daemon_psud._update_led_color() @@ -412,6 +480,15 @@ def test_update_led_color(self): daemon_psud.psu_tbl.reset_mock() daemon_psud._update_psu_fan_led_status.reset_mock() + # LED status is still GREEN, should NOT write to DB (no change) + daemon_psud._update_led_color() + assert daemon_psud.psu_tbl.set.call_count == 0 + assert daemon_psud._update_psu_fan_led_status.call_count == 1 + + daemon_psud.psu_tbl.reset_mock() + daemon_psud._update_psu_fan_led_status.reset_mock() + + # LED changes from GREEN to RED, should write to DB mock_psu.set_status_led(MockPsu.STATUS_LED_COLOR_RED) expected_fvp = psud.swsscommon.FieldValuePairs([('led_status', MockPsu.STATUS_LED_COLOR_RED)]) daemon_psud._update_led_color() @@ -423,7 +500,26 @@ def test_update_led_color(self): daemon_psud.psu_tbl.reset_mock() daemon_psud._update_psu_fan_led_status.reset_mock() - # Test exception handling + # LED status is still RED, should NOT write to DB (no change) + daemon_psud._update_led_color() + assert daemon_psud.psu_tbl.set.call_count == 0 + assert daemon_psud._update_psu_fan_led_status.call_count == 1 + + daemon_psud.psu_tbl.reset_mock() + daemon_psud._update_psu_fan_led_status.reset_mock() + + # LED changes back from RED to GREEN, should write to DB + mock_psu.set_status_led(MockPsu.STATUS_LED_COLOR_GREEN) + expected_fvp = psud.swsscommon.FieldValuePairs([('led_status', MockPsu.STATUS_LED_COLOR_GREEN)]) + daemon_psud._update_led_color() + assert daemon_psud.psu_tbl.set.call_count == 1 + daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + assert daemon_psud._update_psu_fan_led_status.call_count == 1 + + daemon_psud.psu_tbl.reset_mock() + daemon_psud._update_psu_fan_led_status.reset_mock() + + # Test exception handling - LED status becomes N/A, should write to DB mock_psu.get_status_led = mock.Mock(side_effect=NotImplementedError) expected_fvp = psud.swsscommon.FieldValuePairs([('led_status', psud.NOT_AVAILABLE)]) daemon_psud._update_led_color() @@ -432,6 +528,14 @@ def test_update_led_color(self): assert daemon_psud._update_psu_fan_led_status.call_count == 1 daemon_psud._update_psu_fan_led_status.assert_called_with(mock_psu, 1) + daemon_psud.psu_tbl.reset_mock() + daemon_psud._update_psu_fan_led_status.reset_mock() + + # LED status is still N/A, should NOT write to DB (no change) + daemon_psud._update_led_color() + assert daemon_psud.psu_tbl.set.call_count == 0 + assert daemon_psud._update_psu_fan_led_status.call_count == 1 + def test_update_psu_fan_led_status(self): mock_fan = MockFan("PSU 1 Test Fan 1", MockFan.FAN_DIRECTION_INTAKE) mock_psu = MockPsu("PSU 1", 0, True, True) From e2a42501c32f36a40f1535136beb858a77ed1680 Mon Sep 17 00:00:00 2001 From: GauravNagesh Date: Wed, 12 Nov 2025 18:01:43 +0000 Subject: [PATCH 3/3] psud unittest fix: mock DaemonPsud.__del__ to avoid cleanup errors during test teardown --- sonic-psud/tests/test_DaemonPsud.py | 2 ++ sonic-psud/tests/test_psud.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/sonic-psud/tests/test_DaemonPsud.py b/sonic-psud/tests/test_DaemonPsud.py index 5a21ce9d0..b5f5ae0c4 100644 --- a/sonic-psud/tests/test_DaemonPsud.py +++ b/sonic-psud/tests/test_DaemonPsud.py @@ -33,6 +33,8 @@ load_source('psud', os.path.join(scripts_path, 'psud')) import psud +# Mock __del__ at module level to prevent issues during garbage collection +psud.DaemonPsud.__del__ = mock.MagicMock() class TestDaemonPsud(object): """ diff --git a/sonic-psud/tests/test_psud.py b/sonic-psud/tests/test_psud.py index 463ee9edd..2e4686849 100644 --- a/sonic-psud/tests/test_psud.py +++ b/sonic-psud/tests/test_psud.py @@ -26,6 +26,8 @@ load_source('psud', os.path.join(scripts_path, 'psud')) import psud +# Mock __del__ at module level to prevent issues during garbage collection +psud.DaemonPsud.__del__ = mock.MagicMock() daemon_base.db_connect = mock.MagicMock()