From fb86bc2bfa3a3a81d57ae45e03652269c01193bb Mon Sep 17 00:00:00 2001 From: junchao Date: Wed, 26 Aug 2020 15:33:52 +0800 Subject: [PATCH 1/6] Update pmon daemons for SONiC Physical Entity MIB feature --- sonic-psud/scripts/psud | 67 ++++++++++++---- sonic-thermalctld/scripts/thermalctld | 108 ++++++++++++++++++++++---- sonic-xcvrd/scripts/xcvrd | 13 +++- 3 files changed, 156 insertions(+), 32 deletions(-) diff --git a/sonic-psud/scripts/psud b/sonic-psud/scripts/psud index 9ea271c53..d9acc9964 100644 --- a/sonic-psud/scripts/psud +++ b/sonic-psud/scripts/psud @@ -29,18 +29,25 @@ PLATFORM_SPECIFIC_MODULE_NAME = "psuutil" PLATFORM_SPECIFIC_CLASS_NAME = "PsuUtil" CHASSIS_INFO_TABLE = 'CHASSIS_INFO' -CHASSIS_INFO_KEY_TEMPLATE = 'chassis {}' +CHASSIS_INFO_KEY = 'chassis 1' CHASSIS_INFO_PSU_NUM_FIELD = 'psu_num' PSU_INFO_TABLE = 'PSU_INFO' PSU_INFO_KEY_TEMPLATE = 'PSU {}' PSU_INFO_PRESENCE_FIELD = 'presence' +PSU_INFO_MODEL_FIELD = 'model' +PSU_INFO_SERIAL_FIELD = 'serial' PSU_INFO_STATUS_FIELD = 'status' PSU_INFO_TEMP_FIELD = 'temp' PSU_INFO_TEMP_TH_FIELD = 'temp_threshold' PSU_INFO_VOLTAGE_FIELD = 'voltage' PSU_INFO_VOLTAGE_MAX_TH_FIELD = 'voltage_max_threshold' PSU_INFO_VOLTAGE_MIN_TH_FIELD = 'voltage_min_threshold' +PSU_INFO_CURRENT_FIELD = 'current' +PSU_INFO_POWER_FIELD = 'power' +PSU_INFO_FRU_FIELD = 'is_replaceable' + +PHYSICAL_ENTITY_INFO_TABLE = 'PHYSICAL_ENTITY_INFO' FAN_INFO_TABLE = 'FAN_INFO' FAN_INFO_PRESENCE_FIELD = 'presence' @@ -93,13 +100,17 @@ def _wrapper_get_psus_status(psu_index): # Helper functions ============================================================= # +def get_psu_key(psu_index): + return PSU_INFO_KEY_TEMPLATE.format(psu_index) + + def psu_db_update(psu_tbl, psu_num): for psu_index in range(1, psu_num + 1): fvs = swsscommon.FieldValuePairs([(PSU_INFO_PRESENCE_FIELD, 'true' if _wrapper_get_psus_presence(psu_index) else 'false'), (PSU_INFO_STATUS_FIELD, 'true' if _wrapper_get_psus_status(psu_index) else 'false')]) - psu_tbl.set(PSU_INFO_KEY_TEMPLATE.format(psu_index), fvs) + psu_tbl.set(get_psu_key(psu_index), fvs) # try get information from platform API and return a default value if caught NotImplementedError @@ -256,16 +267,18 @@ class DaemonPsud(daemon_base.DaemonBase): chassis_tbl = swsscommon.Table(state_db, CHASSIS_INFO_TABLE) 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) # Post psu number info to STATE_DB psu_num = _wrapper_get_num_psus() fvs = swsscommon.FieldValuePairs([(CHASSIS_INFO_PSU_NUM_FIELD, str(psu_num))]) - chassis_tbl.set(CHASSIS_INFO_KEY_TEMPLATE.format(1), fvs) + chassis_tbl.set(CHASSIS_INFO_KEY, fvs) # Start main loop self.log_info("Start daemon main loop") while not self.stop.wait(PSU_INFO_UPDATE_PERIOD_SECS): + self._update_psu_entity_info() psu_db_update(psu_tbl, psu_num) self.update_psu_data(psu_tbl) self._update_led_color(psu_tbl) @@ -274,9 +287,9 @@ class DaemonPsud(daemon_base.DaemonBase): # Delete all the information from DB and then exit for psu_index in range(1, psu_num + 1): - psu_tbl._del(PSU_INFO_KEY_TEMPLATE.format(psu_index)) + psu_tbl._del(get_psu_key(psu_index)) - chassis_tbl._del(CHASSIS_INFO_KEY_TEMPLATE.format(1)) + chassis_tbl._del(CHASSIS_INFO_KEY) self.log_info("Shutting down...") @@ -291,9 +304,7 @@ class DaemonPsud(daemon_base.DaemonBase): self.log_warning("Failed to update PSU data - {}".format(e)) def _update_single_psu_data(self, index, psu, psu_tbl): - name = try_get(psu.get_name) - if not name: - name = PSU_INFO_KEY_TEMPLATE.format(index) + name = get_psu_key(index) presence = _wrapper_get_psus_presence(index) power_good = False voltage = None @@ -301,6 +312,9 @@ class DaemonPsud(daemon_base.DaemonBase): voltage_low_threshold = None temperature = None temperature_threshold = None + current = None + power = None + is_replaceable = try_get(psu.is_replaceable, False) if presence: power_good = _wrapper_get_psus_status(index) voltage = try_get(psu.get_voltage) @@ -308,6 +322,8 @@ class DaemonPsud(daemon_base.DaemonBase): voltage_low_threshold = try_get(psu.get_voltage_low_threshold) temperature = try_get(psu.get_temperature) temperature_threshold = try_get(psu.get_temperature_high_threshold) + current = try_get(psu.get_current) + power = try_get(psu.get_power) if index not in self.psu_status_dict: self.psu_status_dict[index] = PsuStatus(psu) @@ -354,22 +370,44 @@ class DaemonPsud(daemon_base.DaemonBase): self._set_psu_led(psu, psu_status) fvs = swsscommon.FieldValuePairs( - [(PSU_INFO_TEMP_FIELD, str(temperature)), + [(PSU_INFO_MODEL_FIELD, str(try_get(psu.get_model))), + (PSU_INFO_SERIAL_FIELD, str(try_get(psu.get_serial))), + (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_FRU_FIELD, str(is_replaceable)), + ]) + psu_tbl.set(name, fvs) + + def _update_psu_entity_info(self): + if not platform_chassis: + return + + for index, psu in enumerate(platform_chassis.get_all_psus()): + try: + self._update_single_psu_entity_info(index + 1, psu) + except Exception as e: + self.log_warning("Failed to update PSU data - {}".format(e)) + + def _update_single_psu_entity_info(self, psu_index, psu): + position = try_get(psu.get_position_in_parent, psu_index) + fvs = swsscommon.FieldValuePairs( + [('position_in_parent', str(position)), + ('parent_name', CHASSIS_INFO_KEY), ]) - psu_tbl.set(PSU_INFO_KEY_TEMPLATE.format(index), fvs) + self.phy_entity_tbl.set(get_psu_key(psu_index), fvs) def _update_psu_fan_data(self, psu, psu_index): """ - :param psu: :param psu_index: :return: """ - psu_name = try_get(psu.get_name, 'PSU {}'.format(psu_index)) + psu_name = get_psu_key(psu_index) presence = _wrapper_get_psus_presence(psu_index) fan_list = psu.get_all_fans() for index, fan in enumerate(fan_list): @@ -407,11 +445,11 @@ class DaemonPsud(daemon_base.DaemonBase): fvs = swsscommon.FieldValuePairs([ ('led_status', NOT_AVAILABLE) ]) - psu_tbl.set(PSU_INFO_KEY_TEMPLATE.format(index), fvs) + psu_tbl.set(get_psu_key(index), fvs) self._update_psu_fan_led_status(psu_status.psu, index) def _update_psu_fan_led_status(self, psu, psu_index): - psu_name = try_get(psu.get_name, 'PSU {}'.format(psu_index)) + psu_name = get_psu_key(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)) @@ -426,7 +464,6 @@ class DaemonPsud(daemon_base.DaemonBase): ]) self.fan_tbl.set(fan_name, fvs) - # # Main ========================================================================= # diff --git a/sonic-thermalctld/scripts/thermalctld b/sonic-thermalctld/scripts/thermalctld index d70c9c69d..36a70ae93 100644 --- a/sonic-thermalctld/scripts/thermalctld +++ b/sonic-thermalctld/scripts/thermalctld @@ -23,6 +23,7 @@ except ImportError as e: SYSLOG_IDENTIFIER = 'thermalctld' NOT_AVAILABLE = 'N/A' +CHASSIS_INFO_KEY = 'chassis 1' # utility functions @@ -44,6 +45,13 @@ def try_get(callback, default=NOT_AVAILABLE): return ret +def update_entity_info(table, parent_name, key, device, device_index): + fvs = swsscommon.FieldValuePairs( + [('position_in_parent', str(try_get(device.get_position_in_parent, device_index))), + ('parent_name', parent_name)]) + table.set(key, fvs) + + class FanStatus(logger.Logger): absence_fan_count = 0 fault_fan_count = 0 @@ -171,6 +179,7 @@ class FanStatus(logger.Logger): class FanUpdater(logger.Logger): # Fan information table name in database FAN_INFO_TABLE_NAME = 'FAN_INFO' + FAN_DRAWER_INFO_TABLE_NAME = 'FAN_DRAWER_INFO' def __init__(self, log_identifier, chassis): """ @@ -183,6 +192,8 @@ class FanUpdater(logger.Logger): self.fan_status_dict = {} state_db = daemon_base.db_connect("STATE_DB") self.table = swsscommon.Table(state_db, FanUpdater.FAN_INFO_TABLE_NAME) + self.drawer_table = swsscommon.Table(state_db, FanUpdater.FAN_DRAWER_INFO_TABLE_NAME) + self.phy_entity_table = swsscommon.Table(state_db, PHYSICAL_ENTITY_INFO_TABLE) def deinit(self): """ @@ -214,20 +225,18 @@ class FanUpdater(logger.Logger): old_bad_fan_count = FanStatus.get_bad_fan_count() FanStatus.reset_fan_counter() - fan_index = 0 - for drawer in self.chassis.get_all_fan_drawers(): - for fan in drawer.get_all_fans(): + for drawer_index, drawer in enumerate(self.chassis.get_all_fan_drawers()): + self._refresh_fan_drawer_status(drawer, drawer_index) + for fan_index, fan in enumerate(drawer.get_all_fans()): try: - self._refresh_fan_status(drawer, fan, fan_index) + self._refresh_fan_status(drawer, drawer_index, fan, fan_index) except Exception as e: self.log_warning('Failed to update FAN status - {}'.format(e)) - fan_index += 1 for psu_index, psu in enumerate(self.chassis.get_all_psus()): - psu_name = try_get(psu.get_name, 'PSU {}'.format(psu_index)) for fan_index, fan in enumerate(psu.get_all_fans()): try: - self._refresh_fan_status(None, fan, fan_index, '{} FAN'.format(psu_name), True) + self._refresh_fan_status(psu, psu_index, fan, fan_index, True) except Exception as e: self.log_warning('Failed to update PSU FAN status - {}'.format(e)) @@ -243,17 +252,40 @@ class FanUpdater(logger.Logger): self.log_debug("End fan updating") - def _refresh_fan_status(self, fan_drawer, fan, index, name_prefix='FAN', is_psu_fan=False): + def _refresh_fan_drawer_status(self, fan_drawer, drawer_index): + drawer_name = try_get(fan_drawer.get_name) + if drawer_name == NOT_AVAILABLE: + return + + update_entity_info(self.phy_entity_table, CHASSIS_INFO_KEY, drawer_name, fan_drawer, drawer_index) + + fvs = swsscommon.FieldValuePairs( + [('presence', str(try_get(fan_drawer.get_presence, False))), + ('model', str(try_get(fan_drawer.get_model))), + ('serial', str(try_get(fan_drawer.get_serial))), + ('status', str(try_get(fan_drawer.get_status))), + ('is_replaceable', str(try_get(fan_drawer.is_replaceable, False))), + ]) + + self.drawer_table.set(drawer_name, fvs) + + def _refresh_fan_status(self, parent, parent_index, fan, fan_index, is_psu_fan=False): """ Get Fan status by platform API and write to database for a given Fan - :param fan_drawer: Object representing a platform Fan drawer + :param parent: Parent device of this fan + :param parent_index: Parent device index :param fan: Object representing a platform Fan - :param index: Index of the Fan object in the platform + :param fan_index: Index of the Fan object in its parent device :param name_prefix: name prefix of Fan object if Fan.get_name not presented :return: """ - 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)) + drawer_name = NOT_AVAILABLE if is_psu_fan else str(try_get(parent.get_name)) + if is_psu_fan: + parent_name = 'PSU {}'.format(parent_index) + else: + parent_name = drawer_name if drawer_name != NOT_AVAILABLE else CHASSIS_INFO_KEY + fan_name = try_get(fan.get_name, '{} FAN {}'.format(parent_name, fan_index + 1)) + update_entity_info(self.phy_entity_table, parent_name, fan_name, fan, fan_index + 1) if fan_name not in self.fan_status_dict: self.fan_status_dict[fan_name] = FanStatus(SYSLOG_IDENTIFIER, fan, is_psu_fan) @@ -264,6 +296,7 @@ class FanUpdater(logger.Logger): speed_target = NOT_AVAILABLE fan_fault_status = NOT_AVAILABLE fan_direction = NOT_AVAILABLE + is_replaceable = try_get(fan.is_replaceable, False) presence = try_get(fan.get_presence, False) if presence: speed = try_get(fan.get_speed) @@ -321,6 +354,7 @@ class FanUpdater(logger.Logger): ('speed', str(speed)), ('speed_tolerance', str(speed_tolerance)), ('speed_target', str(speed_target)), + ('is_replaceable', str(is_replaceable)), ('timestamp', datetime.now().strftime('%Y%m%d %H:%M:%S')) ]) @@ -360,6 +394,21 @@ class FanUpdater(logger.Logger): ]) self.table.set(fan_name, fvs) + for drawer in self.chassis.get_all_fan_drawers(): + drawer_name = try_get(drawer.get_name) + if drawer_name == NOT_AVAILABLE: + continue + try: + fvs = swsscommon.FieldValuePairs([ + ('led_status', str(try_get(drawer.get_status_led))) + ]) + except Exception as e: + self.log_warning('Failed to get led status for fan drawer') + fvs = swsscommon.FieldValuePairs([ + ('led_status', NOT_AVAILABLE) + ]) + self.drawer_table.set(drawer_name, fvs) + class TemperatureStatus(logger.Logger): TEMPERATURE_DIFF_THRESHOLD = 10 @@ -459,6 +508,7 @@ class TemperatureUpdater(logger.Logger): self.temperature_status_dict = {} state_db = daemon_base.db_connect("STATE_DB") self.table = swsscommon.Table(state_db, TemperatureUpdater.TEMPER_INFO_TABLE_NAME) + self.phy_entity_table = swsscommon.Table(state_db, PHYSICAL_ENTITY_INFO_TABLE) def deinit(self): """ @@ -489,20 +539,44 @@ class TemperatureUpdater(logger.Logger): self.log_debug("Start temperature updating") for index, thermal in enumerate(self.chassis.get_all_thermals()): try: - self._refresh_temperature_status(thermal, index) + self._refresh_temperature_status(CHASSIS_INFO_KEY, thermal, index) except Exception as e: self.log_warning('Failed to update thermal status - {}'.format(e)) + for psu_index, psu in enumerate(self.chassis.get_all_psus()): + parent_name = 'PSU {}'.format(psu_index + 1) + for thermal_index, thermal in enumerate(psu.get_all_thermals()): + try: + self._refresh_temperature_status(parent_name, thermal, thermal_index) + except Exception as e: + self.log_warning('Failed to update thermal status - {}'.format(e)) + + for sfp_index, sfp in enumerate(self.chassis.get_all_sfps()): + parent_name = 'SFP {}'.format(sfp_index + 1) + for thermal_index, thermal in enumerate(sfp.get_all_thermals()): + try: + self._refresh_temperature_status(parent_name, thermal, thermal_index) + except Exception as e: + self.log_warning('Failed to update thermal status - {}'.format(e)) + self.log_debug("End temperature updating") - def _refresh_temperature_status(self, thermal, index): + def _refresh_temperature_status(self, parent_name, thermal, thermal_index): """ Get temperature status by platform API and write to database + :param parent_name: Name of parent device of the thermal object :param thermal: Object representing a platform thermal zone - :param index: Index of the thermal object in platform chassis + :param thermal_index: Index of the thermal object in platform chassis :return: """ - name = try_get(thermal.get_name, 'Thermal {}'.format(index + 1)) + name = try_get(thermal.get_name, '{} Thermal {}'.format(parent_name, thermal_index + 1)) + + # Only save entity info for thermals that belong to chassis + # for PSU and SFP thermal, they don't need save entity info because snmp can deduce the relation from PSU_INFO + # and TRANSCEIVER_DOM_SENSOR + if parent_name == CHASSIS_INFO_KEY: + update_entity_info(self.phy_entity_table, parent_name, name, thermal, thermal_index + 1) + if name not in self.temperature_status_dict: self.temperature_status_dict[name] = TemperatureStatus(SYSLOG_IDENTIFIER) @@ -513,6 +587,7 @@ class TemperatureUpdater(logger.Logger): high_critical_threshold = NOT_AVAILABLE low_critical_threshold = NOT_AVAILABLE temperature = try_get(thermal.get_temperature) + is_replaceable = try_get(thermal.is_replaceable, False) if temperature != NOT_AVAILABLE: temperature_status.set_temperature(name, temperature) high_threshold = try_get(thermal.get_high_threshold) @@ -546,6 +621,7 @@ class TemperatureUpdater(logger.Logger): ('warning_status', str(warning)), ('critical_high_threshold', str(high_critical_threshold)), ('critical_low_threshold', str(low_critical_threshold)), + ('is_replaceable', str(is_replaceable)), ('timestamp', datetime.now().strftime('%Y%m%d %H:%M:%S')) ]) diff --git a/sonic-xcvrd/scripts/xcvrd b/sonic-xcvrd/scripts/xcvrd index 3ed032272..36dab37c5 100644 --- a/sonic-xcvrd/scripts/xcvrd +++ b/sonic-xcvrd/scripts/xcvrd @@ -138,6 +138,14 @@ def _wrapper_get_presence(physical_port): pass return platform_sfputil.get_presence(physical_port) +def _wrapper_is_replaceable(physical_port): + if platform_chassis is not None: + try: + return platform_chassis.get_sfp(physical_port).is_replaceable() + except NotImplementedError: + pass + return False + def _wrapper_get_transceiver_info(physical_port): if platform_chassis is not None: try: @@ -250,6 +258,7 @@ def post_port_sfp_info_to_db(logical_port_name, table, transceiver_dict, try: port_info_dict = _wrapper_get_transceiver_info(physical_port) if port_info_dict is not None: + is_replaceable = _wrapper_is_replaceable(physical_port) transceiver_dict[physical_port]=port_info_dict fvs = swsscommon.FieldValuePairs([('type', port_info_dict['type']), ('hardware_rev', port_info_dict['hardware_rev']), @@ -266,7 +275,9 @@ def post_port_sfp_info_to_db(logical_port_name, table, transceiver_dict, ('cable_length',port_info_dict['cable_length']), ('specification_compliance',port_info_dict['specification_compliance']), ('nominal_bit_rate',port_info_dict['nominal_bit_rate']), - ('application_advertisement',port_info_dict['application_advertisement'] if 'application_advertisement' in port_info_dict else 'N/A')]) + ('application_advertisement',port_info_dict['application_advertisement'] if 'application_advertisement' in port_info_dict else 'N/A'), + ('is_replaceable',str(is_replaceable)), + ]) table.set(port_name, fvs) else: return SFP_EEPROM_NOT_READY From 068d09bd6e5a334b719ea0bbf46f1fe7ee7441d9 Mon Sep 17 00:00:00 2001 From: junchao Date: Fri, 28 Aug 2020 10:20:00 +0800 Subject: [PATCH 2/6] Fix unit test failures --- sonic-thermalctld/scripts/thermalctld | 5 ++-- sonic-thermalctld/tests/mock_platform.py | 32 +++++++++++++++------ sonic-thermalctld/tests/test_thermalctld.py | 2 +- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/sonic-thermalctld/scripts/thermalctld b/sonic-thermalctld/scripts/thermalctld index 36a70ae93..e4b8c8802 100644 --- a/sonic-thermalctld/scripts/thermalctld +++ b/sonic-thermalctld/scripts/thermalctld @@ -24,6 +24,7 @@ except ImportError as e: SYSLOG_IDENTIFIER = 'thermalctld' NOT_AVAILABLE = 'N/A' CHASSIS_INFO_KEY = 'chassis 1' +PHYSICAL_ENTITY_INFO_TABLE = 'PHYSICAL_ENTITY_INFO' # utility functions @@ -342,7 +343,7 @@ class FanUpdater(logger.Logger): # We don't set PSU led here, PSU led will be handled in psud if set_led: if not is_psu_fan: - self._set_fan_led(fan_drawer, fan, fan_name, fan_status) + self._set_fan_led(parent, fan, fan_name, fan_status) fvs = swsscommon.FieldValuePairs( [('presence', str(presence)), @@ -398,7 +399,7 @@ class FanUpdater(logger.Logger): drawer_name = try_get(drawer.get_name) if drawer_name == NOT_AVAILABLE: continue - try: + try: fvs = swsscommon.FieldValuePairs([ ('led_status', str(try_get(drawer.get_status_led))) ]) diff --git a/sonic-thermalctld/tests/mock_platform.py b/sonic-thermalctld/tests/mock_platform.py index b6fae1eaf..a3eec225f 100644 --- a/sonic-thermalctld/tests/mock_platform.py +++ b/sonic-thermalctld/tests/mock_platform.py @@ -17,6 +17,15 @@ def get_model(self): def get_serial(self): return self.serial + def get_position_in_parent(self): + return 1 + + def is_replaceable(self): + return True + + def get_status(self): + return True + class MockFan(MockDevice): STATUS_LED_COLOR_RED = 'red' @@ -75,6 +84,7 @@ def get_speed(self): class MockPsu(MockDevice): def __init__(self): + MockDevice.__init__(self) self.fan_list = [] def get_all_fans(self): @@ -82,8 +92,9 @@ def get_all_fans(self): class MockFanDrawer(MockDevice): - def __init__(self): - self.name = 'FanDrawer' + def __init__(self, index): + MockDevice.__init__(self) + self.name = 'FanDrawer {}'.format(index) self.fan_list = [] self.led_status = 'red' @@ -100,8 +111,9 @@ def set_status_led(self, value): self.led_status = value -class MockThermal: +class MockThermal(MockDevice): def __init__(self): + MockDevice.__init__(self) self.name = None self.temperature = 2 self.high_threshold = 3 @@ -154,6 +166,7 @@ def __init__(self): self.psu_list = [] self.thermal_list = [] self.fan_drawer_list = [] + self.sfp_list = [] def get_all_fans(self): return self.fan_list @@ -167,10 +180,13 @@ def get_all_thermals(self): def get_all_fan_drawers(self): return self.fan_drawer_list + def get_all_sfps(self): + return self.sfp_list + def make_absence_fan(self): fan = MockFan() fan.presence = False - fan_drawer = MockFanDrawer() + fan_drawer = MockFanDrawer(len(self.fan_drawer_list)) fan_drawer.fan_list.append(fan) self.fan_list.append(fan) self.fan_drawer_list.append(fan_drawer) @@ -178,7 +194,7 @@ def make_absence_fan(self): def make_fault_fan(self): fan = MockFan() fan.status = False - fan_drawer = MockFanDrawer() + fan_drawer = MockFanDrawer(len(self.fan_drawer_list)) fan_drawer.fan_list.append(fan) self.fan_list.append(fan) self.fan_drawer_list.append(fan_drawer) @@ -186,7 +202,7 @@ def make_fault_fan(self): def make_under_speed_fan(self): fan = MockFan() fan.make_under_speed() - fan_drawer = MockFanDrawer() + fan_drawer = MockFanDrawer(len(self.fan_drawer_list)) fan_drawer.fan_list.append(fan) self.fan_list.append(fan) self.fan_drawer_list.append(fan_drawer) @@ -194,14 +210,14 @@ def make_under_speed_fan(self): def make_over_speed_fan(self): fan = MockFan() fan.make_over_speed() - fan_drawer = MockFanDrawer() + fan_drawer = MockFanDrawer(len(self.fan_drawer_list)) fan_drawer.fan_list.append(fan) self.fan_list.append(fan) self.fan_drawer_list.append(fan_drawer) def make_error_fan(self): fan = MockErrorFan() - fan_drawer = MockFanDrawer() + fan_drawer = MockFanDrawer(len(self.fan_drawer_list)) fan_drawer.fan_list.append(fan) self.fan_list.append(fan) self.fan_drawer_list.append(fan_drawer) diff --git a/sonic-thermalctld/tests/test_thermalctld.py b/sonic-thermalctld/tests/test_thermalctld.py index fbfcf3c7e..38cc8b064 100644 --- a/sonic-thermalctld/tests/test_thermalctld.py +++ b/sonic-thermalctld/tests/test_thermalctld.py @@ -179,7 +179,7 @@ def test_insufficient_fan_number(): chassis.make_fault_fan() fan_updater = FanUpdater(SYSLOG_IDENTIFIER, chassis) fan_updater.update() - assert fan_updater.log_warning.call_count == 3 + 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() From e057e3545d207c0bf3aab394d810df442ccc90b2 Mon Sep 17 00:00:00 2001 From: junchao Date: Mon, 31 Aug 2020 16:17:17 +0800 Subject: [PATCH 3/6] Fix issues found in manual test --- sonic-psud/scripts/psud | 26 ++++++++++++++------------ sonic-thermalctld/scripts/thermalctld | 2 +- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/sonic-psud/scripts/psud b/sonic-psud/scripts/psud index d9acc9964..470d3f155 100644 --- a/sonic-psud/scripts/psud +++ b/sonic-psud/scripts/psud @@ -14,7 +14,7 @@ try: import sys import threading - from sonic_py_common import daemon_base + from sonic_py_common import daemon_base, logger from swsscommon import swsscommon except ImportError as e: raise ImportError (str(e) + " - required module not found") @@ -131,26 +131,28 @@ def try_get(callback, default=None): return ret -def log_on_status_changed(normal_status, normal_log, abnormal_log): +def log_on_status_changed(logger, normal_status, normal_log, abnormal_log): """ Log when any status changed + :param logger: Logger object. :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) + logger.log_notice(normal_log) else: - self.log_warning(abnormal_log) + logger.log_warning(abnormal_log) # # PSU status =================================================================== # -class PsuStatus(object): - def __init__(self, psu): +class PsuStatus(logger.Logger): + def __init__(self, psu, log_identifier): + super(PsuStatus, self).__init__(log_identifier) self.psu = psu self.presence = True self.power_good = True @@ -326,13 +328,13 @@ class DaemonPsud(daemon_base.DaemonBase): power = try_get(psu.get_power) if index not in self.psu_status_dict: - self.psu_status_dict[index] = PsuStatus(psu) + self.psu_status_dict[index] = PsuStatus(psu, SYSLOG_IDENTIFIER) psu_status = self.psu_status_dict[index] set_led = False if psu_status.set_presence(presence): set_led = True - log_on_status_changed(psu_status.presence, + log_on_status_changed(self, psu_status.presence, 'PSU absence warning cleared: {} is inserted back.'.format(name), 'PSU absence warning: {} is not present.'.format(name) ) @@ -345,14 +347,14 @@ class DaemonPsud(daemon_base.DaemonBase): if presence and psu_status.set_power_good(power_good): set_led = True - log_on_status_changed(psu_status.power_good, + log_on_status_changed(self, psu_status.power_good, 'Power absence warning cleared: {} power is back to normal.'.format(name), 'Power absence warning: {} is out of power.'.format(name) ) if presence and psu_status.set_voltage(voltage, voltage_high_threshold, voltage_low_threshold): set_led = True - log_on_status_changed(psu_status.voltage_good, + log_on_status_changed(self, psu_status.voltage_good, 'PSU voltage warning cleared: {} voltage is back to normal.'.format(name), 'PSU voltage warning: {} voltage out of range, current voltage={}, valid range=[{}, {}].'.format( name, voltage, voltage_high_threshold, voltage_low_threshold) @@ -360,7 +362,7 @@ class DaemonPsud(daemon_base.DaemonBase): if presence and psu_status.set_temperature(temperature, temperature_threshold): set_led = True - log_on_status_changed(psu_status.temperature_good, + log_on_status_changed(self, psu_status.temperature_good, 'PSU temperature warning cleared: {} temperature is back to normal.'.format(name), 'PSU temperature warning: {} temperature too hot, temperature={}, threshold={}.'.format( name, temperature, temperature_threshold) @@ -458,7 +460,7 @@ class DaemonPsud(daemon_base.DaemonBase): (FAN_INFO_LED_STATUS_FIELD, str(try_get(fan.get_status_led))) ]) except Exception as e: - logger.log_warning('Failed to get led status for fan {}'.format(fan_name)) + self.log_warning('Failed to get led status for fan {}'.format(fan_name)) fvs = swsscommon.FieldValuePairs([ (FAN_INFO_LED_STATUS_FIELD, NOT_AVAILABLE) ]) diff --git a/sonic-thermalctld/scripts/thermalctld b/sonic-thermalctld/scripts/thermalctld index e4b8c8802..5e67e3d68 100644 --- a/sonic-thermalctld/scripts/thermalctld +++ b/sonic-thermalctld/scripts/thermalctld @@ -282,7 +282,7 @@ class FanUpdater(logger.Logger): """ drawer_name = NOT_AVAILABLE if is_psu_fan else str(try_get(parent.get_name)) if is_psu_fan: - parent_name = 'PSU {}'.format(parent_index) + parent_name = 'PSU {}'.format(parent_index + 1) else: parent_name = drawer_name if drawer_name != NOT_AVAILABLE else CHASSIS_INFO_KEY fan_name = try_get(fan.get_name, '{} FAN {}'.format(parent_name, fan_index + 1)) From ca414df6a4732e59a12b210b3f7b30d73f1637fd Mon Sep 17 00:00:00 2001 From: junchao Date: Wed, 9 Sep 2020 10:17:39 +0800 Subject: [PATCH 4/6] Put PSU thermal to PHYSICAL_ENTITY_INFO table --- sonic-thermalctld/scripts/thermalctld | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sonic-thermalctld/scripts/thermalctld b/sonic-thermalctld/scripts/thermalctld index 5e67e3d68..39d944cc2 100644 --- a/sonic-thermalctld/scripts/thermalctld +++ b/sonic-thermalctld/scripts/thermalctld @@ -572,10 +572,11 @@ class TemperatureUpdater(logger.Logger): """ name = try_get(thermal.get_name, '{} Thermal {}'.format(parent_name, thermal_index + 1)) - # Only save entity info for thermals that belong to chassis - # for PSU and SFP thermal, they don't need save entity info because snmp can deduce the relation from PSU_INFO - # and TRANSCEIVER_DOM_SENSOR - if parent_name == CHASSIS_INFO_KEY: + # Only save entity info for thermals that belong to chassis and PSU + # for SFP thermal, they don't need save entity info because snmp can deduce the relation from TRANSCEIVER_DOM_SENSOR + # and as we save logical port in TRANSCEIVER_INFO table, for split cable, a SFP thermal might have multiple parent + # logical port + if 'SFP' not in parent_name: update_entity_info(self.phy_entity_table, parent_name, name, thermal, thermal_index + 1) if name not in self.temperature_status_dict: From 8ae63a7c69b4f5967dd9996bcdfc1832b7701f99 Mon Sep 17 00:00:00 2001 From: junchao Date: Fri, 16 Oct 2020 14:39:20 +0800 Subject: [PATCH 5/6] Fix LGTM warning --- sonic-psud/scripts/psud | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sonic-psud/scripts/psud b/sonic-psud/scripts/psud index ccfa1814e..1ca4dbf56 100644 --- a/sonic-psud/scripts/psud +++ b/sonic-psud/scripts/psud @@ -14,7 +14,7 @@ try: import sys import threading - from sonic_py_common import daemon_base, logger + from sonic_py_common import daemon_base from swsscommon import swsscommon except ImportError as e: raise ImportError (str(e) + " - required module not found") From e9cb6d43f7d63fff89b50bf4b5a2783790011d88 Mon Sep 17 00:00:00 2001 From: junchao Date: Tue, 3 Nov 2020 15:46:23 +0800 Subject: [PATCH 6/6] Fix review comment --- sonic-thermalctld/tests/test_thermalctld.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sonic-thermalctld/tests/test_thermalctld.py b/sonic-thermalctld/tests/test_thermalctld.py index 38cc8b064..fbfcf3c7e 100644 --- a/sonic-thermalctld/tests/test_thermalctld.py +++ b/sonic-thermalctld/tests/test_thermalctld.py @@ -179,7 +179,7 @@ def test_insufficient_fan_number(): chassis.make_fault_fan() fan_updater = FanUpdater(SYSLOG_IDENTIFIER, chassis) fan_updater.update() - assert fan_updater.log_warning.call_count == 3 + 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()