diff --git a/sonic-thermalctld/scripts/thermalctld b/sonic-thermalctld/scripts/thermalctld index 2dd6ffe450e5..26d5b0def023 100644 --- a/sonic-thermalctld/scripts/thermalctld +++ b/sonic-thermalctld/scripts/thermalctld @@ -11,7 +11,7 @@ try: import time from datetime import datetime - from sonic_py_common import daemon_base + from sonic_py_common import daemon_base, logger from sonic_py_common.task_base import ProcessTaskBase except ImportError as e: raise ImportError(str(e) + " - required module not found") @@ -24,7 +24,6 @@ except ImportError as e: SYSLOG_IDENTIFIER = 'thermalctld' NOT_AVAILABLE = 'N/A' - # utility functions # try get information from platform API and return a default value if caught NotImplementedError @@ -45,28 +44,16 @@ def try_get(callback, default=NOT_AVAILABLE): return ret -def log_on_status_changed(normal_status, normal_log, abnormal_log): - """ - Log when any status changed - :param normal_status: Expected status. - :param normal_log: Log string for expected status. - :param abnormal_log: Log string for unexpected status - :return: - """ - if normal_status: - self.log_notice(normal_log) - else: - self.log_warning(abnormal_log) - - -class FanStatus(object): +class FanStatus(logger.Logger): absence_fan_count = 0 fault_fan_count = 0 - def __init__(self, fan=None, is_psu_fan=False): + def __init__(self, log_identifier, fan=None, is_psu_fan=False): """ Constructor of FanStatus """ + super(FanStatus, self).__init__(log_identifier) + self.fan = fan self.is_psu_fan = is_psu_fan self.presence = True @@ -181,15 +168,17 @@ class FanStatus(object): # # FanUpdater =================================================================== # -class FanUpdater(object): +class FanUpdater(logger.Logger): # Fan information table name in database FAN_INFO_TABLE_NAME = 'FAN_INFO' - def __init__(self, chassis): + def __init__(self, log_identifier, chassis): """ Constructor for FanUpdater :param chassis: Object representing a platform chassis """ + super(FanUpdater, self).__init__(log_identifier) + self.chassis = chassis self.fan_status_dict = {} state_db = daemon_base.db_connect("STATE_DB") @@ -203,6 +192,19 @@ class FanUpdater(object): for name in self.fan_status_dict.keys(): self.table._del(name) + def _log_on_status_changed(self, normal_status, normal_log, abnormal_log): + """ + Log when any status changed + :param normal_status: Expected status. + :param normal_log: Log string for expected status. + :param abnormal_log: Log string for unexpected status + :return: + """ + if normal_status: + self.log_notice(normal_log) + else: + self.log_warning(abnormal_log) + def update(self): """ Update all Fan information to database @@ -253,7 +255,7 @@ class FanUpdater(object): drawer_name = NOT_AVAILABLE if is_psu_fan else str(try_get(fan_drawer.get_name)) fan_name = try_get(fan.get_name, '{} {}'.format(name_prefix, index + 1)) if fan_name not in self.fan_status_dict: - self.fan_status_dict[fan_name] = FanStatus(fan, is_psu_fan) + self.fan_status_dict[fan_name] = FanStatus(SYSLOG_IDENTIFIER, fan, is_psu_fan) fan_status = self.fan_status_dict[fan_name] @@ -273,34 +275,34 @@ class FanUpdater(object): set_led = False if fan_status.set_presence(presence): set_led = True - log_on_status_changed(fan_status.presence, - 'Fan removed warning cleared: {} was inserted.'.format(fan_name), - 'Fan removed warning: {} was removed from ' - 'the system, potential overheat hazard'.format(fan_name) - ) + self._log_on_status_changed(fan_status.presence, + 'Fan removed warning cleared: {} was inserted.'.format(fan_name), + 'Fan removed warning: {} was removed from ' + 'the system, potential overheat hazard'.format(fan_name) + ) if presence and fan_status.set_fault_status(fan_fault_status): set_led = True - log_on_status_changed(fan_status.status, - 'Fan fault warning cleared: {} is back to normal.'.format(fan_name), - 'Fan fault warning: {} is broken.'.format(fan_name) - ) + self._log_on_status_changed(fan_status.status, + 'Fan fault warning cleared: {} is back to normal.'.format(fan_name), + 'Fan fault warning: {} is broken.'.format(fan_name) + ) if presence and fan_status.set_under_speed(speed, speed_target, speed_tolerance): set_led = True - log_on_status_changed(not fan_status.under_speed, - 'Fan low speed warning cleared: {} speed is back to normal.'.format(fan_name), - 'Fan low speed warning: {} current speed={}, target speed={}, tolerance={}.'. - format(fan_name, speed, speed_target, speed_tolerance) - ) + self._log_on_status_changed(not fan_status.under_speed, + 'Fan low speed warning cleared: {} speed is back to normal.'.format(fan_name), + 'Fan low speed warning: {} current speed={}, target speed={}, tolerance={}.'. + format(fan_name, speed, speed_target, speed_tolerance) + ) if presence and fan_status.set_over_speed(speed, speed_target, speed_tolerance): set_led = True - log_on_status_changed(not fan_status.over_speed, - 'Fan high speed warning cleared: {} speed is back to normal.'.format(fan_name), - 'Fan high speed warning: {} target speed={}, current speed={}, tolerance={}.'. - format(fan_name, speed_target, speed, speed_tolerance) - ) + self._log_on_status_changed(not fan_status.over_speed, + 'Fan high speed warning cleared: {} speed is back to normal.'.format(fan_name), + 'Fan high speed warning: {} target speed={}, current speed={}, tolerance={}.'. + format(fan_name, speed_target, speed, speed_tolerance) + ) # TODO: handle invalid fan direction @@ -359,10 +361,12 @@ class FanUpdater(object): self.table.set(fan_name, fvs) -class TemperatureStatus(object): +class TemperatureStatus(logger.Logger): TEMPERATURE_DIFF_THRESHOLD = 10 - def __init__(self): + def __init__(self, log_identifier): + super(TemperatureStatus, self).__init__(log_identifier) + self.temperature = None self.over_temperature = False self.under_temperature = False @@ -440,15 +444,17 @@ class TemperatureStatus(object): # # TemperatureUpdater ====================================================================== # -class TemperatureUpdater(object): +class TemperatureUpdater(logger.Logger): # Temperature information table name in database TEMPER_INFO_TABLE_NAME = 'TEMPERATURE_INFO' - def __init__(self, chassis): + def __init__(self, log_identifier, chassis): """ Constructor of TemperatureUpdater :param chassis: Object representing a platform chassis """ + super(TemperatureUpdater, self).__init__(log_identifier) + self.chassis = chassis self.temperature_status_dict = {} state_db = daemon_base.db_connect("STATE_DB") @@ -462,6 +468,19 @@ class TemperatureUpdater(object): for name in self.temperature_status_dict.keys(): self.table._del(name) + def _log_on_status_changed(self, normal_status, normal_log, abnormal_log): + """ + Log when any status changed + :param normal_status: Expected status. + :param normal_log: Log string for expected status. + :param abnormal_log: Log string for unexpected status + :return: + """ + if normal_status: + self.log_notice(normal_log) + else: + self.log_warning(abnormal_log) + def update(self): """ Update all temperature information to database @@ -485,7 +504,7 @@ class TemperatureUpdater(object): """ name = try_get(thermal.get_name, 'Thermal {}'.format(index + 1)) if name not in self.temperature_status_dict: - self.temperature_status_dict[name] = TemperatureStatus() + self.temperature_status_dict[name] = TemperatureStatus(SYSLOG_IDENTIFIER) temperature_status = self.temperature_status_dict[name] @@ -503,21 +522,21 @@ class TemperatureUpdater(object): warning = False if temperature != NOT_AVAILABLE and temperature_status.set_over_temperature(temperature, high_threshold): - log_on_status_changed(not temperature_status.over_temperature, - 'High temperature warning cleared: {} temperature restore to {}C, high threshold {}C.'. - format(name, temperature, high_threshold), - 'High temperature warning: {} current temperature {}C, high threshold {}C'. - format(name, temperature, high_threshold) - ) + self._log_on_status_changed(not temperature_status.over_temperature, + 'High temperature warning cleared: {} temperature restore to {}C, high threshold {}C.'. + format(name, temperature, high_threshold), + 'High temperature warning: {} current temperature {}C, high threshold {}C'. + format(name, temperature, high_threshold) + ) warning = warning | temperature_status.over_temperature if temperature != NOT_AVAILABLE and temperature_status.set_under_temperature(temperature, low_threshold): - log_on_status_changed(not temperature_status.under_temperature, - 'Low temperature warning cleared: {} temperature restore to {}C, low threshold {}C.'. - format(name, temperature, low_threshold), - 'Low temperature warning: {} current temperature {}C, low threshold {}C'. - format(name, temperature, low_threshold) - ) + self._log_on_status_changed(not temperature_status.under_temperature, + 'Low temperature warning cleared: {} temperature restore to {}C, low threshold {}C.'. + format(name, temperature, low_threshold), + 'Low temperature warning: {} current temperature {}C, low threshold {}C'. + format(name, temperature, low_threshold) + ) warning = warning | temperature_status.under_temperature fvs = swsscommon.FieldValuePairs( @@ -547,8 +566,8 @@ class ThermalMonitor(ProcessTaskBase): :param chassis: Object representing a platform chassis """ ProcessTaskBase.__init__(self) - self.fan_updater = FanUpdater(chassis) - self.temperature_updater = TemperatureUpdater(chassis) + self.fan_updater = FanUpdater(SYSLOG_IDENTIFIER, chassis) + self.temperature_updater = TemperatureUpdater(SYSLOG_IDENTIFIER, chassis) def task_worker(self): """ diff --git a/sonic-thermalctld/tests/test_thermalctld.py b/sonic-thermalctld/tests/test_thermalctld.py index 3d91fb816c9b..fbfcf3c7ed21 100644 --- a/sonic-thermalctld/tests/test_thermalctld.py +++ b/sonic-thermalctld/tests/test_thermalctld.py @@ -6,6 +6,7 @@ from .mock_platform import MockChassis, MockFan, MockThermal +SYSLOG_IDENTIFIER = 'thermalctld_test' NOT_AVAILABLE = 'N/A' daemon_base.db_connect = MagicMock() @@ -22,17 +23,29 @@ def setup_function(): - thermal_control.log_notice = MagicMock() - thermal_control.log_warning = MagicMock() + FanStatus.log_notice = MagicMock() + FanStatus.log_warning = MagicMock() + FanUpdater.log_notice = MagicMock() + FanUpdater.log_warning = MagicMock() + TemperatureStatus.log_notice = MagicMock() + TemperatureStatus.log_warning = MagicMock() + TemperatureUpdater.log_notice = MagicMock() + TemperatureUpdater.log_warning = MagicMock() def teardown_function(): - thermal_control.log_notice.reset() - thermal_control.log_warning.reset() + FanStatus.log_notice.reset() + FanStatus.log_warning.reset() + FanUpdater.log_notice.reset() + FanUpdater.log_notice.reset() + TemperatureStatus.log_notice.reset() + TemperatureStatus.log_warning.reset() + TemperatureUpdater.log_warning.reset() + TemperatureUpdater.log_warning.reset() def test_fanstatus_set_presence(): - fan_status = FanStatus() + fan_status = FanStatus(SYSLOG_IDENTIFIER) ret = fan_status.set_presence(True) assert fan_status.presence assert not ret @@ -43,7 +56,7 @@ def test_fanstatus_set_presence(): def test_fanstatus_set_under_speed(): - fan_status = FanStatus() + fan_status = FanStatus(SYSLOG_IDENTIFIER) ret = fan_status.set_under_speed(NOT_AVAILABLE, NOT_AVAILABLE, NOT_AVAILABLE) assert not ret @@ -68,7 +81,7 @@ def test_fanstatus_set_under_speed(): def test_fanstatus_set_over_speed(): - fan_status = FanStatus() + fan_status = FanStatus(SYSLOG_IDENTIFIER) ret = fan_status.set_over_speed(NOT_AVAILABLE, NOT_AVAILABLE, NOT_AVAILABLE) assert not ret @@ -95,66 +108,66 @@ def test_fanstatus_set_over_speed(): def test_fanupdater_fan_absence(): chassis = MockChassis() chassis.make_absence_fan() - fan_updater = FanUpdater(chassis) + fan_updater = FanUpdater(SYSLOG_IDENTIFIER, chassis) fan_updater.update() fan_list = chassis.get_all_fans() assert fan_list[0].get_status_led() == MockFan.STATUS_LED_COLOR_RED - thermal_control.log_warning.assert_called() + fan_updater.log_warning.assert_called() fan_list[0].presence = True fan_updater.update() assert fan_list[0].get_status_led() == MockFan.STATUS_LED_COLOR_GREEN - thermal_control.log_notice.assert_called() + fan_updater.log_notice.assert_called() def test_fanupdater_fan_fault(): chassis = MockChassis() chassis.make_fault_fan() - fan_updater = FanUpdater(chassis) + fan_updater = FanUpdater(SYSLOG_IDENTIFIER, chassis) fan_updater.update() fan_list = chassis.get_all_fans() assert fan_list[0].get_status_led() == MockFan.STATUS_LED_COLOR_RED - thermal_control.log_warning.assert_called() + fan_updater.log_warning.assert_called() fan_list[0].status = True fan_updater.update() assert fan_list[0].get_status_led() == MockFan.STATUS_LED_COLOR_GREEN - thermal_control.log_notice.assert_called() + fan_updater.log_notice.assert_called() def test_fanupdater_fan_under_speed(): chassis = MockChassis() chassis.make_under_speed_fan() - fan_updater = FanUpdater(chassis) + fan_updater = FanUpdater(SYSLOG_IDENTIFIER, chassis) fan_updater.update() fan_list = chassis.get_all_fans() assert fan_list[0].get_status_led() == MockFan.STATUS_LED_COLOR_RED - thermal_control.log_warning.assert_called_once() + fan_updater.log_warning.assert_called_once() fan_list[0].make_normal_speed() fan_updater.update() assert fan_list[0].get_status_led() == MockFan.STATUS_LED_COLOR_GREEN - thermal_control.log_notice.assert_called_once() + fan_updater.log_notice.assert_called_once() def test_fanupdater_fan_over_speed(): chassis = MockChassis() chassis.make_over_speed_fan() - fan_updater = FanUpdater(chassis) + fan_updater = FanUpdater(SYSLOG_IDENTIFIER, chassis) fan_updater.update() fan_list = chassis.get_all_fans() assert fan_list[0].get_status_led() == MockFan.STATUS_LED_COLOR_RED - thermal_control.log_warning.assert_called_once() + fan_updater.log_warning.assert_called_once() fan_list[0].make_normal_speed() fan_updater.update() assert fan_list[0].get_status_led() == MockFan.STATUS_LED_COLOR_GREEN - thermal_control.log_notice.assert_called_once() + fan_updater.log_notice.assert_called_once() def test_insufficient_fan_number(): - fan_status1 = FanStatus() - fan_status2 = FanStatus() + fan_status1 = FanStatus(SYSLOG_IDENTIFIER) + fan_status2 = FanStatus(SYSLOG_IDENTIFIER) fan_status1.set_presence(False) fan_status2.set_fault_status(False) assert FanStatus.get_bad_fan_count() == 2 @@ -164,25 +177,25 @@ def test_insufficient_fan_number(): chassis = MockChassis() chassis.make_absence_fan() chassis.make_fault_fan() - fan_updater = FanUpdater(chassis) + fan_updater = FanUpdater(SYSLOG_IDENTIFIER, chassis) fan_updater.update() - assert thermal_control.log_warning.call_count == 3 - thermal_control.log_warning.assert_called_with('Insufficient number of working fans warning: 2 fans are not working.') + assert fan_updater.log_warning.call_count == 3 + fan_updater.log_warning.assert_called_with('Insufficient number of working fans warning: 2 fans are not working.') fan_list = chassis.get_all_fans() fan_list[0].presence = True fan_updater.update() - assert thermal_control.log_notice.call_count == 1 - thermal_control.log_warning.assert_called_with('Insufficient number of working fans warning: 1 fans are not working.') + assert fan_updater.log_notice.call_count == 1 + fan_updater.log_warning.assert_called_with('Insufficient number of working fans warning: 1 fans are not working.') fan_list[1].status = True fan_updater.update() - assert thermal_control.log_notice.call_count == 3 - thermal_control.log_notice.assert_called_with('Insufficient number of working fans warning cleared: all fans are back to normal.') + assert fan_updater.log_notice.call_count == 3 + fan_updater.log_notice.assert_called_with('Insufficient number of working fans warning cleared: all fans are back to normal.') def test_temperature_status_set_over_temper(): - temperatue_status = TemperatureStatus() + temperatue_status = TemperatureStatus(SYSLOG_IDENTIFIER) ret = temperatue_status.set_over_temperature(NOT_AVAILABLE, NOT_AVAILABLE) assert not ret @@ -202,7 +215,7 @@ def test_temperature_status_set_over_temper(): def test_temperstatus_set_under_temper(): - temperature_status = TemperatureStatus() + temperature_status = TemperatureStatus(SYSLOG_IDENTIFIER) ret = temperature_status.set_under_temperature(NOT_AVAILABLE, NOT_AVAILABLE) assert not ret @@ -224,27 +237,27 @@ def test_temperstatus_set_under_temper(): def test_temperupdater_over_temper(): chassis = MockChassis() chassis.make_over_temper_thermal() - temperature_updater = TemperatureUpdater(chassis) + temperature_updater = TemperatureUpdater(SYSLOG_IDENTIFIER, chassis) temperature_updater.update() thermal_list = chassis.get_all_thermals() - thermal_control.log_warning.assert_called_once() + temperature_updater.log_warning.assert_called_once() thermal_list[0].make_normal_temper() temperature_updater.update() - thermal_control.log_notice.assert_called_once() + temperature_updater.log_notice.assert_called_once() def test_temperupdater_under_temper(): chassis = MockChassis() chassis.make_under_temper_thermal() - temperature_updater = TemperatureUpdater(chassis) + temperature_updater = TemperatureUpdater(SYSLOG_IDENTIFIER, chassis) temperature_updater.update() thermal_list = chassis.get_all_thermals() - thermal_control.log_warning.assert_called_once() + temperature_updater.log_warning.assert_called_once() thermal_list[0].make_normal_temper() temperature_updater.update() - thermal_control.log_notice.assert_called_once() + temperature_updater.log_notice.assert_called_once() def test_update_fan_with_exception(): @@ -254,10 +267,10 @@ def test_update_fan_with_exception(): fan.make_over_speed() chassis.get_all_fans().append(fan) - fan_updater = FanUpdater(chassis) + fan_updater = FanUpdater(SYSLOG_IDENTIFIER, chassis) fan_updater.update() assert fan.get_status_led() == MockFan.STATUS_LED_COLOR_RED - thermal_control.log_warning.assert_called() + fan_updater.log_warning.assert_called() def test_update_thermal_with_exception(): @@ -267,6 +280,6 @@ def test_update_thermal_with_exception(): thermal.make_over_temper() chassis.get_all_thermals().append(thermal) - temperature_updater = TemperatureUpdater(chassis) + temperature_updater = TemperatureUpdater(SYSLOG_IDENTIFIER, chassis) temperature_updater.update() - thermal_control.log_warning.assert_called() + temperature_updater.log_warning.assert_called()