diff --git a/sonic-psud/scripts/psud b/sonic-psud/scripts/psud index 3f8c29eab..b0296328d 100644 --- a/sonic-psud/scripts/psud +++ b/sonic-psud/scripts/psud @@ -90,31 +90,87 @@ exit_code = 0 # temporary wrappers that are compliable with both new platform api and old-style plugin mode -def _wrapper_get_num_psus(): +def _wrapper_get_num_psus(logger): if platform_chassis is not None: try: return platform_chassis.get_num_psus() except NotImplementedError: pass - return platform_psuutil.get_num_psus() + if platform_psuutil is not None: + return platform_psuutil.get_num_psus() + if logger: + logger.log_warning("PSU provider unavailable; defaulting to 0 PSUs") + return 0 -def _wrapper_get_psu_presence(psu_index): +def _wrapper_get_psu(logger, psu_index): + """ + Get PSU object from platform chassis + :param logger: Logger instance for error/warning messages + :param psu_index: PSU index (1-based) + :return: PSU object if available, None otherwise + + Note: + If logger is None, errors and warnings encountered during execution will not be logged. + """ if platform_chassis is not None: try: - return platform_chassis.get_psu(psu_index - 1).get_presence() - except NotImplementedError: - pass - return platform_psuutil.get_psu_presence(psu_index) + return platform_chassis.get_psu(psu_index - 1) + except NotImplementedError as e: + if logger: + logger.log_warning("get_psu() not implemented by platform chassis: {}".format(str(e))) + except Exception as e: + if logger: + logger.log_error("Failed to get PSU {} from platform chassis: {}".format(psu_index, str(e))) + return None -def _wrapper_get_psu_status(psu_index): +def _wrapper_get_psu_presence(logger, psu_index): if platform_chassis is not None: + psu = _wrapper_get_psu(logger, psu_index) + if psu: + try: + return psu.get_presence() + except NotImplementedError: + pass + except Exception as e: + if logger: + logger.log_warning("Exception in psu.get_presence() for PSU {}: {}".format(psu_index, str(e))) + return False + if platform_psuutil is not None: try: - return platform_chassis.get_psu(psu_index - 1).get_powergood_status() - except NotImplementedError: - pass - return platform_psuutil.get_psu_status(psu_index) + return platform_psuutil.get_psu_presence(psu_index) + except Exception as e: + if logger: + logger.log_warning("Exception in platform_psuutil.get_psu_presence({}): {}".format(psu_index, str(e))) + return False + if logger: + logger.log_error("Failed to get PSU {} presence".format(psu_index)) + return False + + +def _wrapper_get_psu_status(logger, psu_index): + if platform_chassis is not None: + psu = _wrapper_get_psu(logger, psu_index) + if psu: + try: + return psu.get_powergood_status() + except NotImplementedError: + pass + except Exception as e: + if logger: + logger.log_warning("Exception in psu.get_powergood_status() for PSU {}: {}".format(psu_index, str(e))) + return False + if platform_psuutil is not None: + try: + return platform_psuutil.get_psu_status(psu_index) + except Exception as e: + if logger: + logger.log_warning("Exception in platform_psuutil.get_psu_status({}): {}".format(psu_index, str(e))) + return False + if logger: + logger.log_error("Failed to get PSU {} status".format(psu_index)) + return False # @@ -123,13 +179,12 @@ def _wrapper_get_psu_status(psu_index): def get_psu_key(psu_index): if platform_chassis is not None: - try: - return platform_chassis.get_psu(psu_index - 1).get_name() - except NotImplementedError: - pass - except IndexError: - #some functionality is expectent on returning an expected key even if the psu object itself does not exist - pass + psu = _wrapper_get_psu(None, psu_index) + if psu: + try: + return psu.get_name() + except Exception: + pass return PSU_INFO_KEY_TEMPLATE.format(psu_index) @@ -409,7 +464,7 @@ class DaemonPsud(daemon_base.DaemonBase): self.phy_entity_tbl = swsscommon.Table(state_db, PHYSICAL_ENTITY_INFO_TABLE) # Post psu number info to STATE_DB - self.num_psus = _wrapper_get_num_psus() + self.num_psus = _wrapper_get_num_psus(self) fvs = swsscommon.FieldValuePairs([(CHASSIS_INFO_PSU_NUM_FIELD, str(self.num_psus))]) self.chassis_tbl.set(CHASSIS_INFO_KEY, fvs) @@ -418,8 +473,9 @@ class DaemonPsud(daemon_base.DaemonBase): 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)) + if self.chassis_tbl is not None: + self.chassis_tbl._del(CHASSIS_INFO_KEY) + self.chassis_tbl._del(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1)) # Override signal handler from DaemonBase def signal_handler(self, sig, frame): @@ -474,7 +530,7 @@ class DaemonPsud(daemon_base.DaemonBase): def _update_single_psu_data(self, index, psu): name = get_psu_key(index) - presence = _wrapper_get_psu_presence(index) + presence = _wrapper_get_psu_presence(self, index) power_good = False voltage = NOT_AVAILABLE voltage_high_threshold = NOT_AVAILABLE @@ -488,7 +544,7 @@ class DaemonPsud(daemon_base.DaemonBase): in_current = NOT_AVAILABLE max_power = NOT_AVAILABLE if presence: - power_good = _wrapper_get_psu_status(index) + power_good = _wrapper_get_psu_status(self, index) voltage = try_get(psu.get_voltage, NOT_AVAILABLE) voltage_high_threshold = try_get(psu.get_voltage_high_threshold, NOT_AVAILABLE) voltage_low_threshold = try_get(psu.get_voltage_low_threshold, NOT_AVAILABLE) @@ -612,8 +668,8 @@ class DaemonPsud(daemon_base.DaemonBase): (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'), + (PSU_INFO_PRESENCE_FIELD, 'true' if _wrapper_get_psu_presence(self, index) else 'false'), + (PSU_INFO_STATUS_FIELD, 'true' if _wrapper_get_psu_status(self, index) else 'false'), ]) self.psu_tbl.set(name, fvs) @@ -642,7 +698,7 @@ class DaemonPsud(daemon_base.DaemonBase): :return: """ psu_name = get_psu_key(psu_index) - presence = _wrapper_get_psu_presence(psu_index) + presence = _wrapper_get_psu_presence(self, psu_index) fan_list = psu.get_all_fans() for index, fan in enumerate(fan_list): fan_name = try_get(fan.get_name, '{} FAN {}'.format(psu_name, index + 1)) diff --git a/sonic-psud/tests/test_psud.py b/sonic-psud/tests/test_psud.py index 463ee9edd..7f54a94cb 100644 --- a/sonic-psud/tests/test_psud.py +++ b/sonic-psud/tests/test_psud.py @@ -37,64 +37,312 @@ @mock.patch('psud.platform_chassis', mock.MagicMock()) @mock.patch('psud.platform_psuutil', mock.MagicMock()) def test_wrapper_get_num_psus(): + mock_logger = mock.MagicMock() # Test new platform API is available and implemented - psud._wrapper_get_num_psus() + psud._wrapper_get_num_psus(mock_logger) assert psud.platform_chassis.get_num_psus.call_count == 1 assert psud.platform_psuutil.get_num_psus.call_count == 0 # Test new platform API is available but not implemented psud.platform_chassis.get_num_psus.side_effect = NotImplementedError - psud._wrapper_get_num_psus() + psud._wrapper_get_num_psus(mock_logger) assert psud.platform_chassis.get_num_psus.call_count == 2 assert psud.platform_psuutil.get_num_psus.call_count == 1 # Test new platform API not available psud.platform_chassis = None - psud._wrapper_get_num_psus() + psud._wrapper_get_num_psus(mock_logger) assert psud.platform_psuutil.get_num_psus.call_count == 2 + # Test with None logger - should not crash + psud.platform_chassis = mock.MagicMock() + psud.platform_psuutil = mock.MagicMock() + psud._wrapper_get_num_psus(None) + assert psud.platform_chassis.get_num_psus.call_count >= 1 + + # Test when both providers are unavailable + psud.platform_chassis = None + psud.platform_psuutil = None + mock_logger.reset_mock() + result = psud._wrapper_get_num_psus(mock_logger) + assert result == 0 + assert mock_logger.log_warning.call_count == 1 + mock_logger.log_warning.assert_called_with("PSU provider unavailable; defaulting to 0 PSUs") + @mock.patch('psud.platform_chassis', mock.MagicMock()) @mock.patch('psud.platform_psuutil', mock.MagicMock()) def test_wrapper_get_psu_presence(): - # Test new platform API is available - psud._wrapper_get_psu_presence(1) - assert psud.platform_chassis.get_psu(0).get_presence.call_count == 1 + mock_logger = mock.MagicMock() + mock_psu = mock.MagicMock() + mock_psu.get_presence.return_value = True + + # Test new platform API is available and working + psud.platform_chassis.get_psu.return_value = mock_psu + result = psud._wrapper_get_psu_presence(mock_logger, 1) + assert result is True + psud.platform_chassis.get_psu.assert_called_with(0) + mock_psu.get_presence.assert_called_once() assert psud.platform_psuutil.get_psu_presence.call_count == 0 + assert mock_logger.log_error.call_count == 0 - # Test new platform API is available but not implemented - psud.platform_chassis.get_psu(0).get_presence.side_effect = NotImplementedError - psud._wrapper_get_psu_presence(1) - assert psud.platform_chassis.get_psu(0).get_presence.call_count == 2 + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + mock_psu.get_presence.reset_mock() + mock_logger.log_error.reset_mock() + + # Test _wrapper_get_psu returns None (PSU object retrieval failed) + psud.platform_chassis.get_psu.side_effect = Exception("PSU retrieval failed") + psud._wrapper_get_psu_presence(mock_logger, 1) + # Should fallback to platform_psuutil + psud.platform_chassis.get_psu.assert_called_with(0) + assert psud.platform_psuutil.get_psu_presence.call_count == 1 + psud.platform_psuutil.get_psu_presence.assert_called_with(1) + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + psud.platform_chassis.get_psu.side_effect = None # Remove side effect + psud.platform_psuutil.get_psu_presence.reset_mock() + mock_logger.log_error.reset_mock() + + # Test new platform API PSU available but get_presence not implemented + psud.platform_chassis.get_psu.return_value = mock_psu + mock_psu.get_presence.side_effect = NotImplementedError + psud._wrapper_get_psu_presence(mock_logger, 1) + # Should fallback to platform_psuutil + psud.platform_chassis.get_psu.assert_called_with(0) + mock_psu.get_presence.assert_called_once() assert psud.platform_psuutil.get_psu_presence.call_count == 1 + psud.platform_psuutil.get_psu_presence.assert_called_with(1) + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + mock_psu.get_presence.reset_mock() + psud.platform_psuutil.get_psu_presence.reset_mock() + mock_logger.log_error.reset_mock() + + # Test psu.get_presence() raises general exception + psud.platform_chassis.get_psu.return_value = mock_psu + mock_psu.get_presence.side_effect = RuntimeError("Hardware error") + result = psud._wrapper_get_psu_presence(mock_logger, 1) + assert result is False + psud.platform_chassis.get_psu.assert_called_with(0) + mock_psu.get_presence.assert_called_once() + assert mock_logger.log_warning.call_count == 1 + mock_logger.log_warning.assert_called_with("Exception in psu.get_presence() for PSU 1: Hardware error") + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + mock_psu.get_presence.reset_mock() + psud.platform_psuutil.get_psu_presence.reset_mock() + mock_logger.log_warning.reset_mock() + mock_logger.log_error.reset_mock() # Test new platform API not available psud.platform_chassis = None - psud._wrapper_get_psu_presence(1) - assert psud.platform_psuutil.get_psu_presence.call_count == 2 + psud._wrapper_get_psu_presence(mock_logger, 1) + # Should use platform_psuutil + assert psud.platform_psuutil.get_psu_presence.call_count == 1 psud.platform_psuutil.get_psu_presence.assert_called_with(1) + # Reset mocks + psud.platform_psuutil.get_psu_presence.reset_mock() + mock_logger.log_error.reset_mock() + + # Test platform_psuutil.get_psu_presence raises exception + psud.platform_chassis = None + psud.platform_psuutil = mock.MagicMock() + psud.platform_psuutil.get_psu_presence.side_effect = Exception("PSU presence error") + result = psud._wrapper_get_psu_presence(mock_logger, 1) + assert result is False + assert psud.platform_psuutil.get_psu_presence.call_count == 1 + assert mock_logger.log_warning.call_count == 1 + mock_logger.log_warning.assert_called_with("Exception in platform_psuutil.get_psu_presence(1): PSU presence error") + + # Reset mocks + psud.platform_psuutil.get_psu_presence.reset_mock() + mock_logger.log_warning.reset_mock() + mock_logger.log_error.reset_mock() + + # Test both platform_chassis and platform_psuutil are None + psud.platform_chassis = None + psud.platform_psuutil = None + result = psud._wrapper_get_psu_presence(mock_logger, 1) + assert result is False + assert mock_logger.log_error.call_count == 1 + mock_logger.log_error.assert_called_with("Failed to get PSU 1 presence") + + +@mock.patch('psud.platform_chassis', mock.MagicMock()) +@mock.patch('psud.platform_psuutil', mock.MagicMock()) +def test_wrapper_get_psu(): + mock_logger = mock.MagicMock() + mock_psu = mock.MagicMock() + + # Test new platform API is available and working + psud.platform_chassis.get_psu.return_value = mock_psu + result = psud._wrapper_get_psu(mock_logger, 1) + assert result == mock_psu + psud.platform_chassis.get_psu.assert_called_with(0) # psu_index - 1 + assert mock_logger.log_warning.call_count == 0 + + # Reset mock + psud.platform_chassis.get_psu.reset_mock() + mock_logger.log_warning.reset_mock() + + # Test NotImplementedError + psud.platform_chassis.get_psu.side_effect = NotImplementedError("Not implemented") + result = psud._wrapper_get_psu(mock_logger, 1) + assert result is None + psud.platform_chassis.get_psu.assert_called_with(0) + assert mock_logger.log_warning.call_count == 1 + mock_logger.log_warning.assert_called_with("get_psu() not implemented by platform chassis: Not implemented") + + # Reset mock + psud.platform_chassis.get_psu.reset_mock() + mock_logger.log_warning.reset_mock() + + # Test general Exception + psud.platform_chassis.get_psu.side_effect = Exception("General error") + result = psud._wrapper_get_psu(mock_logger, 2) + assert result is None + psud.platform_chassis.get_psu.assert_called_with(1) # psu_index - 1 + assert mock_logger.log_error.call_count == 1 + mock_logger.log_error.assert_called_with("Failed to get PSU 2 from platform chassis: General error") + + # Reset mock + psud.platform_chassis.get_psu.reset_mock() + mock_logger.log_warning.reset_mock() + + # Test with logger as None + psud.platform_chassis.get_psu.side_effect = NotImplementedError("Not implemented") + result = psud._wrapper_get_psu(None, 1) + assert result is None + psud.platform_chassis.get_psu.assert_called_with(0) + + # Test with None logger and different exception types + mock_logger.reset_mock() + psud.platform_chassis.get_psu.side_effect = RuntimeError("Hardware error") + result = psud._wrapper_get_psu(None, 1) + assert result is None + + # Test with valid logger and RuntimeError - should log error + psud.platform_chassis = mock.MagicMock() + psud.platform_chassis.get_psu.side_effect = RuntimeError("Hardware error") + mock_logger.reset_mock() + result = psud._wrapper_get_psu(mock_logger, 2) + assert result is None + assert mock_logger.log_error.call_count == 1 + mock_logger.log_error.assert_called_with("Failed to get PSU 2 from platform chassis: Hardware error") + + # Test platform_chassis is None + psud.platform_chassis = None + result = psud._wrapper_get_psu(mock_logger, 1) + assert result is None + @mock.patch('psud.platform_chassis', mock.MagicMock()) @mock.patch('psud.platform_psuutil', mock.MagicMock()) def test_wrapper_get_psu_status(): - # Test new platform API is available - psud._wrapper_get_psu_status(1) - assert psud.platform_chassis.get_psu(0).get_powergood_status.call_count == 1 + mock_logger = mock.MagicMock() + mock_psu = mock.MagicMock() + mock_psu.get_powergood_status.return_value = True + + # Test new platform API is available and working + psud.platform_chassis.get_psu.return_value = mock_psu + result = psud._wrapper_get_psu_status(mock_logger, 1) + assert result is True + psud.platform_chassis.get_psu.assert_called_with(0) + mock_psu.get_powergood_status.assert_called_once() assert psud.platform_psuutil.get_psu_status.call_count == 0 + assert mock_logger.log_error.call_count == 0 - # Test new platform API is available but not implemented - psud.platform_chassis.get_psu(0).get_powergood_status.side_effect = NotImplementedError - psud._wrapper_get_psu_status(1) - assert psud.platform_chassis.get_psu(0).get_powergood_status.call_count == 2 + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + mock_psu.get_powergood_status.reset_mock() + mock_logger.log_error.reset_mock() + + # Test _wrapper_get_psu returns None (PSU object retrieval failed) + psud.platform_chassis.get_psu.side_effect = Exception("PSU retrieval failed") + psud._wrapper_get_psu_status(mock_logger, 1) + # Should fallback to platform_psuutil + psud.platform_chassis.get_psu.assert_called_with(0) + assert psud.platform_psuutil.get_psu_status.call_count == 1 + psud.platform_psuutil.get_psu_status.assert_called_with(1) + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + psud.platform_chassis.get_psu.side_effect = None # Remove side effect + psud.platform_psuutil.get_psu_status.reset_mock() + mock_logger.log_error.reset_mock() + + # Test new platform API PSU available but get_powergood_status not implemented + psud.platform_chassis.get_psu.return_value = mock_psu + mock_psu.get_powergood_status.side_effect = NotImplementedError + psud._wrapper_get_psu_status(mock_logger, 1) + # Should fallback to platform_psuutil + psud.platform_chassis.get_psu.assert_called_with(0) + mock_psu.get_powergood_status.assert_called_once() assert psud.platform_psuutil.get_psu_status.call_count == 1 + psud.platform_psuutil.get_psu_status.assert_called_with(1) + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + mock_psu.get_powergood_status.reset_mock() + psud.platform_psuutil.get_psu_status.reset_mock() + mock_logger.log_error.reset_mock() + + # Test psu.get_powergood_status() raises general exception + psud.platform_chassis.get_psu.return_value = mock_psu + mock_psu.get_powergood_status.side_effect = RuntimeError("Hardware error") + result = psud._wrapper_get_psu_status(mock_logger, 1) + assert result is False + psud.platform_chassis.get_psu.assert_called_with(0) + mock_psu.get_powergood_status.assert_called_once() + assert mock_logger.log_warning.call_count == 1 + mock_logger.log_warning.assert_called_with("Exception in psu.get_powergood_status() for PSU 1: Hardware error") + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + mock_psu.get_powergood_status.reset_mock() + psud.platform_psuutil.get_psu_status.reset_mock() + mock_logger.log_warning.reset_mock() + mock_logger.log_error.reset_mock() # Test new platform API not available psud.platform_chassis = None - psud._wrapper_get_psu_status(1) - assert psud.platform_psuutil.get_psu_status.call_count == 2 + psud._wrapper_get_psu_status(mock_logger, 1) + # Should use platform_psuutil + assert psud.platform_psuutil.get_psu_status.call_count == 1 psud.platform_psuutil.get_psu_status.assert_called_with(1) + # Reset mocks + psud.platform_psuutil.get_psu_status.reset_mock() + mock_logger.log_error.reset_mock() + + # Test platform_psuutil.get_psu_status raises exception + psud.platform_chassis = None + psud.platform_psuutil = mock.MagicMock() + psud.platform_psuutil.get_psu_status.side_effect = Exception("PSU status error") + result = psud._wrapper_get_psu_status(mock_logger, 1) + assert result is False + assert psud.platform_psuutil.get_psu_status.call_count == 1 + assert mock_logger.log_warning.call_count == 1 + mock_logger.log_warning.assert_called_with("Exception in platform_psuutil.get_psu_status(1): PSU status error") + + # Reset mocks + psud.platform_psuutil.get_psu_status.reset_mock() + mock_logger.log_warning.reset_mock() + mock_logger.log_error.reset_mock() + + # Test both platform_chassis and platform_psuutil are None + psud.platform_chassis = None + psud.platform_psuutil = None + result = psud._wrapper_get_psu_status(mock_logger, 1) + assert result is False + assert mock_logger.log_error.call_count == 1 + mock_logger.log_error.assert_called_with("Failed to get PSU 1 status") + def test_log_on_status_changed(): normal_log = "Normal log message" abnormal_log = "Abnormal log message" @@ -114,6 +362,72 @@ def test_log_on_status_changed(): mock_logger.log_warning.assert_called_with(abnormal_log) +@mock.patch('psud.platform_chassis', mock.MagicMock()) +def test_get_psu_key(): + mock_psu = mock.MagicMock() + mock_psu.get_name.return_value = "PSU-1" + + # Test platform_chassis is available and PSU get_name() works + psud.platform_chassis.get_psu.return_value = mock_psu + result = psud.get_psu_key(1) + assert result == "PSU-1" + psud.platform_chassis.get_psu.assert_called_with(0) # psu_index - 1 + mock_psu.get_name.assert_called_once() + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + mock_psu.get_name.reset_mock() + + # Test _wrapper_get_psu returns None (PSU object retrieval failed) + psud.platform_chassis.get_psu.side_effect = Exception("PSU retrieval failed") + result = psud.get_psu_key(2) + assert result == "PSU 2" + psud.platform_chassis.get_psu.assert_called_with(1) # psu_index - 1 + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + psud.platform_chassis.get_psu.side_effect = None # Remove side effect + + # Test PSU available but get_name() raises NotImplementedError + psud.platform_chassis.get_psu.return_value = mock_psu + mock_psu.get_name.side_effect = NotImplementedError + result = psud.get_psu_key(3) + assert result == "PSU 3" + psud.platform_chassis.get_psu.assert_called_with(2) # psu_index - 1 + mock_psu.get_name.assert_called_once() + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + mock_psu.get_name.reset_mock() + + # Test PSU available but get_name() raises IndexError + mock_psu.get_name.side_effect = IndexError + result = psud.get_psu_key(4) + assert result == "PSU 4" + psud.platform_chassis.get_psu.assert_called_with(3) # psu_index - 1 + mock_psu.get_name.assert_called_once() + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + mock_psu.get_name.reset_mock() + + # Test PSU available but get_name() raises general Exception + mock_psu.get_name.side_effect = RuntimeError("Hardware error") + result = psud.get_psu_key(4) + assert result == "PSU 4" + psud.platform_chassis.get_psu.assert_called_with(3) # psu_index - 1 + mock_psu.get_name.assert_called_once() + + # Reset mocks + psud.platform_chassis.get_psu.reset_mock() + mock_psu.get_name.reset_mock() + + # Test platform_chassis is None + psud.platform_chassis = None + result = psud.get_psu_key(5) + assert result == "PSU 5" + + @mock.patch('psud.platform_chassis', mock.MagicMock()) @mock.patch('psud.DaemonPsud.run') def test_main(mock_run):