From cfa600f578624ad80820b8eebe6b04f8fe97f83c Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com> Date: Tue, 23 Mar 2021 01:14:09 +0800 Subject: [PATCH 01/11] [thermalctld] Initialize fan led in thermalctld for the first run (#167) Initialize fan led in thermalcltd for the first run. Add a flag "led_initialized" in FanStatus and set it as False on __init__ function. FanUpdater will use this flag to determine if fan led should be set even if no fan event detected. --- sonic-thermalctld/scripts/thermalctld | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/sonic-thermalctld/scripts/thermalctld b/sonic-thermalctld/scripts/thermalctld index 1da7d4328..bf2285558 100644 --- a/sonic-thermalctld/scripts/thermalctld +++ b/sonic-thermalctld/scripts/thermalctld @@ -79,6 +79,7 @@ class FanStatus(logger.Logger): self.under_speed = False self.over_speed = False self.invalid_direction = False + self.led_initialized = False @classmethod def get_bad_fan_count(cls): @@ -315,7 +316,7 @@ class FanUpdater(logger.Logger): fan_fault_status = try_get(fan.get_status, False) fan_direction = try_get(fan.get_direction) - set_led = False + set_led = not fan_status.led_initialized if fan_status.set_presence(presence): set_led = True self._log_on_status_changed(fan_status.presence, @@ -383,16 +384,16 @@ class FanUpdater(logger.Logger): :return: """ try: - if fan_status.is_ok(): - fan.set_status_led(fan.STATUS_LED_COLOR_GREEN) - fan_drawer.set_status_led(fan.STATUS_LED_COLOR_GREEN) - else: - # TODO: wait for Kebo to define the mapping of fan status to led color, - # just set it to red so far - fan.set_status_led(fan.STATUS_LED_COLOR_RED) - fan_drawer.set_status_led(fan.STATUS_LED_COLOR_RED) + led_color = fan.STATUS_LED_COLOR_GREEN if fan_status.is_ok() else fan.STATUS_LED_COLOR_RED + fan.set_status_led(led_color) + fan_drawer.set_status_led(led_color) except NotImplementedError as e: self.log_warning('Failed to set status LED for fan {}, set_status_led not implemented'.format(fan_name)) + + # Set led_initialized to True even if there is NotImplementedError as it is not neccessary to + # print the warning log again and again. But if there is other exception, we could not + # reach this line, and it will retry setting led color in the next run. + fan_status.led_initialized = True def _update_led_color(self): for fan_name, fan_status in self.fan_status_dict.items(): From 260cf2dedb74880a44713b2b37cae6595caeb445 Mon Sep 17 00:00:00 2001 From: vdahiya12 <67608553+vdahiya12@users.noreply.github.com> Date: Mon, 22 Mar 2021 12:26:45 -0700 Subject: [PATCH 02/11] [xcvrd] change firmware information fields name inside MUX_CABLE_INFO table for Y cable (#165) This PR fixes the naming convention for firmware related fields for Y cable. In particular all the fields are now named as tor_self and tor_peer in place of tor1 and tor 2 Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com Description This PR fixes the naming convention for firmware related fields for Y cable. In particular all the fields are now named as tor_self and tor_peer in place of tor1 and tor 2 Motivation and Context Required by telemetry team as part of their initial schema Signed-off-by: vaibhav-dahiya --- sonic-xcvrd/tests/test_xcvrd.py | 40 ++++++++-------- .../xcvrd/xcvrd_utilities/y_cable_helper.py | 48 ++++++++++--------- 2 files changed, 46 insertions(+), 42 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index a560de82d..36140a14c 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -252,26 +252,26 @@ def test_init_port_sfp_status_tbl(self): 'commit_slot2_nic': 'False', 'empty_slot1_nic': 'True', 'empty_slot2_nic': 'False', - 'build_slot1_tor1': 'MS', - 'build_slot2_tor1': 'MS', - 'version_slot1_tor1': '1.7', - 'version_slot2_tor1': '1.7', - 'run_slot1_tor1': 'True', - 'run_slot2_tor1': 'False', - 'commit_slot1_tor1': 'True', - 'commit_slot2_tor1': 'False', - 'empty_slot1_tor1': 'True', - 'empty_slot2_tor1': 'False', - 'build_slot1_tor2': 'MS', - 'build_slot2_tor2': 'MS', - 'version_slot1_tor2': '1.7', - 'version_slot2_tor2': '1.7', - 'run_slot1_tor2': 'True', - 'run_slot2_tor2': 'False', - 'commit_slot1_tor2': 'True', - 'commit_slot2_tor2': 'False', - 'empty_slot1_tor2': 'True', - 'empty_slot2_tor2': 'False'})) + 'build_slot1_tor_self': 'MS', + 'build_slot2_tor_self': 'MS', + 'version_slot1_tor_self': '1.7', + 'version_slot2_tor_self': '1.7', + 'run_slot1_tor_self': 'True', + 'run_slot2_tor_self': 'False', + 'commit_slot1_tor_self': 'True', + 'commit_slot2_tor_self': 'False', + 'empty_slot1_tor_self': 'True', + 'empty_slot2_tor_self': 'False', + 'build_slot1_tor_peer': 'MS', + 'build_slot2_tor_peer': 'MS', + 'version_slot1_tor_peer': '1.7', + 'version_slot2_tor_peer': '1.7', + 'run_slot1_tor_peer': 'True', + 'run_slot2_tor_peer': 'False', + 'commit_slot1_tor_peer': 'True', + 'commit_slot2_tor_peer': 'False', + 'empty_slot1_tor_peer': 'True', + 'empty_slot2_tor_peer': 'False'})) def test_post_port_mux_info_to_db(self): logical_port_name = "Ethernet0" mux_tbl = Table("STATE_DB", y_cable_helper.MUX_CABLE_INFO_TABLE) diff --git a/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py b/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py index 1953aa40f..bd55bb64f 100644 --- a/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py +++ b/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py @@ -714,8 +714,12 @@ def get_muxcable_info(physical_port, logical_port_name): mux_info_dict["link_status_nic"] = "down" get_firmware_dict(physical_port, 0, "nic", mux_info_dict) - get_firmware_dict(physical_port, 1, "tor1", mux_info_dict) - get_firmware_dict(physical_port, 2, "tor2", mux_info_dict) + if read_side == 1: + get_firmware_dict(physical_port, 1, "tor_self", mux_info_dict) + get_firmware_dict(physical_port, 2, "tor_peer", mux_info_dict) + else: + get_firmware_dict(physical_port, 1, "tor_peer", mux_info_dict) + get_firmware_dict(physical_port, 2, "tor_self", mux_info_dict) res = y_cable.get_internal_voltage_temp(physical_port) @@ -885,26 +889,26 @@ def post_port_mux_info_to_db(logical_port_name, table): ('commit_slot2_nic', str(mux_info_dict["commit_slot2_nic"])), ('empty_slot1_nic', str(mux_info_dict["empty_slot1_nic"])), ('empty_slot2_nic', str(mux_info_dict["empty_slot2_nic"])), - ('build_slot1_tor1', str(mux_info_dict["build_slot1_tor1"])), - ('build_slot2_tor1', str(mux_info_dict["build_slot2_tor1"])), - ('version_slot1_tor1', str(mux_info_dict["version_slot1_tor1"])), - ('version_slot2_tor1', str(mux_info_dict["version_slot2_tor1"])), - ('run_slot1_tor1', str(mux_info_dict["run_slot1_tor1"])), - ('run_slot2_tor1', str(mux_info_dict["run_slot2_tor1"])), - ('commit_slot1_tor1', str(mux_info_dict["commit_slot1_tor1"])), - ('commit_slot2_tor1', str(mux_info_dict["commit_slot2_tor1"])), - ('empty_slot1_tor1', str(mux_info_dict["empty_slot1_tor1"])), - ('empty_slot2_tor1', str(mux_info_dict["empty_slot2_tor1"])), - ('build_slot1_tor2', str(mux_info_dict["build_slot1_tor2"])), - ('build_slot2_tor2', str(mux_info_dict["build_slot2_tor2"])), - ('version_slot1_tor2', str(mux_info_dict["version_slot1_tor2"])), - ('version_slot2_tor2', str(mux_info_dict["version_slot2_tor2"])), - ('run_slot1_tor2', str(mux_info_dict["run_slot1_tor2"])), - ('run_slot2_tor2', str(mux_info_dict["run_slot2_tor2"])), - ('commit_slot1_tor2', str(mux_info_dict["commit_slot1_tor2"])), - ('commit_slot2_tor2', str(mux_info_dict["commit_slot2_tor2"])), - ('empty_slot1_tor2', str(mux_info_dict["empty_slot1_tor2"])), - ('empty_slot2_tor2', str(mux_info_dict["empty_slot2_tor2"])) + ('build_slot1_tor_self', str(mux_info_dict["build_slot1_tor_self"])), + ('build_slot2_tor_self', str(mux_info_dict["build_slot2_tor_self"])), + ('version_slot1_tor_self', str(mux_info_dict["version_slot1_tor_self"])), + ('version_slot2_tor_self', str(mux_info_dict["version_slot2_tor_self"])), + ('run_slot1_tor_self', str(mux_info_dict["run_slot1_tor_self"])), + ('run_slot2_tor_self', str(mux_info_dict["run_slot2_tor_self"])), + ('commit_slot1_tor_self', str(mux_info_dict["commit_slot1_tor_self"])), + ('commit_slot2_tor_self', str(mux_info_dict["commit_slot2_tor_self"])), + ('empty_slot1_tor_self', str(mux_info_dict["empty_slot1_tor_self"])), + ('empty_slot2_tor_self', str(mux_info_dict["empty_slot2_tor_self"])), + ('build_slot1_tor_peer', str(mux_info_dict["build_slot1_tor_peer"])), + ('build_slot2_tor_peer', str(mux_info_dict["build_slot2_tor_peer"])), + ('version_slot1_tor_peer', str(mux_info_dict["version_slot1_tor_peer"])), + ('version_slot2_tor_peer', str(mux_info_dict["version_slot2_tor_peer"])), + ('run_slot1_tor_peer', str(mux_info_dict["run_slot1_tor_peer"])), + ('run_slot2_tori_peer', str(mux_info_dict["run_slot2_tor_peer"])), + ('commit_slot1_tor_peer', str(mux_info_dict["commit_slot1_tor_peer"])), + ('commit_slot2_tor_peer', str(mux_info_dict["commit_slot2_tor_peer"])), + ('empty_slot1_tor_peer', str(mux_info_dict["empty_slot1_tor_peer"])), + ('empty_slot2_tor_peer', str(mux_info_dict["empty_slot2_tor_peer"])) ]) table.set(logical_port_name, fvs) else: From c5be3ca4c450a83d4e6e11c71da6f767d2a1bd17 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Mon, 29 Mar 2021 10:37:12 -0700 Subject: [PATCH 03/11] [psud] Increase unit test coverage; Refactor mock platform (#154) - Refactor psud such that the `run()` method does not contain an infinite loop, thus allowing us to unit test it - Refactor mock_platform.py such that it inherits from sonic-platform-common in order to ensure it is aligned with the current API definitions (this introduces a test-time dependency on the sonic-platform-common package) - Eliminate the need to check for a `PSUD_UNIT_TESTING` environment variable in daemon code - Increase overall unit test unit test coverage from 78% to 97% --- sonic-psud/pytest.ini | 2 +- sonic-psud/scripts/psud | 146 +++--- sonic-psud/setup.py | 3 +- sonic-psud/tests/mock_device_base.py | 11 - sonic-psud/tests/mock_platform.py | 421 +++++++++++++----- .../mocked_libs/sonic_platform/__init__.py | 5 + .../tests/mocked_libs/sonic_platform/psu.py | 10 + .../tests/mocked_libs/swsscommon/__init__.py | 5 + .../swsscommon/swsscommon.py} | 7 + sonic-psud/tests/test_DaemonPsud.py | 243 ++++++---- sonic-psud/tests/test_PsuChassisInfo.py | 91 ++-- sonic-psud/tests/test_PsuStatus.py | 26 +- sonic-psud/tests/test_psud.py | 36 +- 13 files changed, 663 insertions(+), 343 deletions(-) delete mode 100644 sonic-psud/tests/mock_device_base.py create mode 100644 sonic-psud/tests/mocked_libs/sonic_platform/__init__.py create mode 100644 sonic-psud/tests/mocked_libs/sonic_platform/psu.py create mode 100644 sonic-psud/tests/mocked_libs/swsscommon/__init__.py rename sonic-psud/tests/{mock_swsscommon.py => mocked_libs/swsscommon/swsscommon.py} (89%) diff --git a/sonic-psud/pytest.ini b/sonic-psud/pytest.ini index aa4fe636e..d90ee9ed9 100644 --- a/sonic-psud/pytest.ini +++ b/sonic-psud/pytest.ini @@ -1,2 +1,2 @@ [pytest] -addopts = --cov=scripts --cov-report html --cov-report term --cov-report xml --junitxml=test-results.xml -v +addopts = --cov=scripts --cov-report html --cov-report term --cov-report xml --junitxml=test-results.xml -vv diff --git a/sonic-psud/scripts/psud b/sonic-psud/scripts/psud index bb7ed567f..d33ad1861 100644 --- a/sonic-psud/scripts/psud +++ b/sonic-psud/scripts/psud @@ -9,21 +9,14 @@ The loop interval is PSU_INFO_UPDATE_PERIOD_SECS in seconds. """ -import os import signal import sys import threading from datetime import datetime +from sonic_platform.psu import Psu from sonic_py_common import daemon_base, logger - -# If unit testing is occurring, mock swsscommon and module_base -if os.getenv("PSUD_UNIT_TESTING") == "1": - from tests.mock_platform import MockPsu as Psu - from tests import mock_swsscommon as swsscommon -else: - from sonic_platform.psu import Psu - from swsscommon import swsscommon +from swsscommon import swsscommon # @@ -32,8 +25,8 @@ else: # TODO: Once we no longer support Python 2, we can eliminate this and get the # name using the 'name' field (e.g., `signal.SIGINT.name`) starting with Python 3.5 -SIGNALS_TO_NAMES_DICT = dict((getattr(signal, n), n) \ - for n in dir(signal) if n.startswith('SIG') and '_' not in n ) +SIGNALS_TO_NAMES_DICT = dict((getattr(signal, n), n) + for n in dir(signal) if n.startswith('SIG') and '_' not in n) SYSLOG_IDENTIFIER = "psud" @@ -85,8 +78,11 @@ PSUUTIL_LOAD_ERROR = 1 platform_psuutil = None platform_chassis = None +exit_code = 0 # temporary wrappers that are compliable with both new platform api and old-style plugin mode + + def _wrapper_get_num_psus(): if platform_chassis is not None: try: @@ -131,7 +127,7 @@ def psu_db_update(psu_tbl, psu_num): psu_tbl.set(get_psu_key(psu_index), fvs) -# try get information from platform API and return a default value if caught NotImplementedError +# try get information from platform API and return a default value if we catch NotImplementedError def try_get(callback, default=None): """ Handy function to invoke the callback and catch NotImplementedError @@ -257,11 +253,10 @@ class PsuChassisInfo(logger.Logger): self.master_status_good = master_status_good # Update the PSU master status LED - try: - color = Psu.STATUS_LED_COLOR_GREEN if master_status_good else Psu.STATUS_LED_COLOR_RED - Psu.set_status_master_led(color) - except NotImplementedError: - self.log_warning("set_status_master_led() not implemented") + # set_status_master_led() is a class method implemented in PsuBase + # so we do not need to catch NotImplementedError here + color = Psu.STATUS_LED_COLOR_GREEN if master_status_good else Psu.STATUS_LED_COLOR_RED + Psu.set_status_master_led(color) log_on_status_changed(self, self.master_status_good, 'PSU supplied power warning cleared: supplied power is back to normal.', @@ -351,32 +346,21 @@ class DaemonPsud(daemon_base.DaemonBase): def __init__(self, log_identifier): super(DaemonPsud, self).__init__(log_identifier) - self.stop = threading.Event() + # Set minimum logging level to INFO + self.set_min_log_priority_info() + + self.stop_event = threading.Event() + self.num_psus = 0 self.psu_status_dict = {} + self.chassis_tbl = None self.fan_tbl = None + self.psu_tbl = None self.psu_chassis_info = None self.first_run = True - # Signal handler - def signal_handler(self, sig, frame): - if sig == signal.SIGHUP: - self.log_info("Caught SIGHUP - ignoring...") - elif sig == signal.SIGINT: - self.log_info("Caught SIGINT - exiting...") - self.stop.set() - elif sig == signal.SIGTERM: - self.log_info("Caught SIGTERM - exiting...") - self.stop.set() - else: - self.log_warning("Caught unhandled signal '{}'".format(SIGNALS_TO_NAMES_DICT[sig])) - - # Run daemon - def run(self): global platform_psuutil global platform_chassis - self.log_info("Starting up...") - # Load new platform api class try: import sonic_platform.platform @@ -394,52 +378,70 @@ class DaemonPsud(daemon_base.DaemonBase): # Connect to STATE_DB and create psu/chassis info tables state_db = daemon_base.db_connect("STATE_DB") - chassis_tbl = swsscommon.Table(state_db, CHASSIS_INFO_TABLE) - psu_tbl = swsscommon.Table(state_db, PSU_INFO_TABLE) + 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) # 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, fvs) + 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) - # Start main loop - self.log_info("Start daemon main loop") + 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)) - 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) + self.chassis_tbl._del(CHASSIS_INFO_KEY) + self.chassis_tbl._del(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1)) - if platform_chassis and platform_chassis.is_modular_chassis(): - self.update_psu_chassis_info(chassis_tbl) + # Override signal handler from DaemonBase + def signal_handler(self, sig, frame): + FATAL_SIGNALS = [signal.SIGINT, signal.SIGTERM] + NONFATAL_SIGNALS = [signal.SIGHUP] - self.first_run = False + global exit_code - self.log_info("Stop daemon main loop") + if sig in FATAL_SIGNALS: + self.log_info("Caught signal '{}' - exiting...".format(SIGNALS_TO_NAMES_DICT[sig])) + exit_code = 128 + sig # Make sure we exit with a non-zero code so that supervisor will try to restart us + self.stop_event.set() + elif sig in NONFATAL_SIGNALS: + self.log_info("Caught signal '{}' - ignoring...".format(SIGNALS_TO_NAMES_DICT[sig])) + else: + self.log_warning("Caught unhandled signal '{}' - ignoring...".format(SIGNALS_TO_NAMES_DICT[sig])) - # Delete all the information from DB and then exit - for psu_index in range(1, psu_num + 1): - psu_tbl._del(get_psu_key(psu_index)) + # Main daemon logic + def run(self): + if self.stop_event.wait(PSU_INFO_UPDATE_PERIOD_SECS): + # We received a fatal signal + return False - chassis_tbl._del(CHASSIS_INFO_KEY) - chassis_tbl._del(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1)) + self._update_psu_entity_info() + psu_db_update(self.psu_tbl, self.num_psus) + self.update_psu_data() + self._update_led_color() - self.log_info("Shutting down...") + if platform_chassis and platform_chassis.is_modular_chassis(): + self.update_psu_chassis_info() - def update_psu_data(self, psu_tbl): + if self.first_run: + self.first_run = False + + return True + + def update_psu_data(self): if not platform_chassis: return for index, psu in enumerate(platform_chassis.get_all_psus()): try: - self._update_single_psu_data(index + 1, psu, psu_tbl) + self._update_single_psu_data(index + 1, psu) except Exception as e: self.log_warning("Failed to update PSU data - {}".format(e)) - def _update_single_psu_data(self, index, psu, psu_tbl): + def _update_single_psu_data(self, index, psu): name = get_psu_key(index) presence = _wrapper_get_psu_presence(index) power_good = False @@ -517,7 +519,7 @@ class DaemonPsud(daemon_base.DaemonBase): (PSU_INFO_POWER_FIELD, str(power)), (PSU_INFO_FRU_FIELD, str(is_replaceable)), ]) - psu_tbl.set(name, fvs) + self.psu_tbl.set(name, fvs) def _update_psu_entity_info(self): if not platform_chassis: @@ -567,15 +569,15 @@ class DaemonPsud(daemon_base.DaemonBase): except NotImplementedError: self.log_warning("set_status_led() not implemented") - def _update_led_color(self, psu_tbl): + def _update_led_color(self): if not platform_chassis: 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))) - ]) - psu_tbl.set(get_psu_key(index), fvs) + ]) + self.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): @@ -588,14 +590,14 @@ class DaemonPsud(daemon_base.DaemonBase): ]) self.fan_tbl.set(fan_name, fvs) - def update_psu_chassis_info(self, chassis_tbl): + def update_psu_chassis_info(self): if not platform_chassis: return if not self.psu_chassis_info: self.psu_chassis_info = PsuChassisInfo(SYSLOG_IDENTIFIER, platform_chassis) - self.psu_chassis_info.run_power_budget(chassis_tbl) + self.psu_chassis_info.run_power_budget(self.chassis_tbl) self.psu_chassis_info.update_master_status() @@ -606,8 +608,16 @@ class DaemonPsud(daemon_base.DaemonBase): def main(): psud = DaemonPsud(SYSLOG_IDENTIFIER) - psud.run() + + psud.log_info("Starting up...") + + while psud.run(): + pass + + psud.log_info("Shutting down...") + + return exit_code if __name__ == '__main__': - main() + sys.exit(main()) diff --git a/sonic-psud/setup.py b/sonic-psud/setup.py index 732968123..236607bac 100644 --- a/sonic-psud/setup.py +++ b/sonic-psud/setup.py @@ -23,7 +23,8 @@ tests_require=[ 'mock>=2.0.0; python_version < "3.3"', 'pytest', - 'pytest-cov' + 'pytest-cov', + 'sonic_platform_common' ], classifiers=[ 'Development Status :: 4 - Beta', diff --git a/sonic-psud/tests/mock_device_base.py b/sonic-psud/tests/mock_device_base.py deleted file mode 100644 index 2aa9b9349..000000000 --- a/sonic-psud/tests/mock_device_base.py +++ /dev/null @@ -1,11 +0,0 @@ -class DeviceBase(): - # Device-types - DEVICE_TYPE_PSU = "PSU" - DEVICE_TYPE_FAN = "FAN" - DEVICE_TYPE_FANDRAWER = "FAN-DRAWER" - - # LED colors - STATUS_LED_COLOR_GREEN = "green" - STATUS_LED_COLOR_AMBER = "amber" - STATUS_LED_COLOR_RED = "red" - STATUS_LED_COLOR_OFF = "off" diff --git a/sonic-psud/tests/mock_platform.py b/sonic-psud/tests/mock_platform.py index d1530d666..6a0bb3517 100644 --- a/sonic-psud/tests/mock_platform.py +++ b/sonic-psud/tests/mock_platform.py @@ -1,165 +1,378 @@ -from .mock_device_base import DeviceBase +from sonic_platform_base import chassis_base +from sonic_platform_base import fan_base +from sonic_platform_base import fan_drawer_base +from sonic_platform_base import module_base +from sonic_platform_base import psu_base + + +class MockChassis(chassis_base.ChassisBase): + def __init__(self, + name='Fixed Chassis', + position_in_parent=0, + presence=True, + model='Module Model', + serial='Module Serial', + status=True): + super(MockChassis, self).__init__() + self._name = name + self._presence = presence + self._model = model + self._serial = serial + self._status = status + self._position_in_parent = position_in_parent + + self._psu_list = [] + self._fan_drawer_list = [] + self._module_list = [] + def get_num_psus(self): + return len(self._psu_list) -class MockDevice: - STATUS_LED_COLOR_GREEN = "green" - STATUS_LED_COLOR_AMBER = "amber" - STATUS_LED_COLOR_RED = "red" - STATUS_LED_COLOR_OFF = "off" + def get_all_psus(self): + return self._psu_list - def __init__(self): - self.name = None - self.presence = True - self.model = 'Module Model' - self.serial = 'Module Serial' + def get_psu(self, index): + return self._psu_list[index] - def get_name(self): - return self.name + def get_num_fan_drawers(self): + return len(self._fan_drawer_list) - def set_presence(self, presence): - self.presence = presence + def get_all_fan_drawers(self): + return self._fan_drawer_list + + def get_num_modules(self): + return len(self._module_list) + + def get_all_modules(self): + return self._module_list + + def get_status_led(self): + return self._status_led_color + + def set_status_led(self, color): + self._status_led_color = color + return True + + # Methods inherited from DeviceBase class and related setters + def get_name(self): + return self._name def get_presence(self): - return self.presence + return self._presence + + def set_presence(self, presence): + self._presence = presence def get_model(self): - return self.model + return self._model def get_serial(self): - return self.serial + return self._serial + def get_status(self): + return self._status -class MockPsu(MockDevice): - master_led_color = MockDevice.STATUS_LED_COLOR_OFF + def set_status(self, status): + self._status = status - def __init__(self, presence, status, name, position_in_parent): - self.name = name - self.presence = True - self.status = status - self.status_led_color = self.STATUS_LED_COLOR_OFF - self.position_in_parent = position_in_parent - self._fan_list = [] + def get_position_in_parent(self): + return self._position_in_parent + + def is_replaceable(self): + return self._replaceable + + +class MockFan(fan_base.FanBase): + def __init__(self, + name, + position_in_parent, + presence=True, + model='Module Model', + serial='Module Serial', + status=True, + direction=fan_base.FanBase.FAN_DIRECTION_INTAKE, + speed=50): + super(MockFan, self).__init__() + self._name = name + self._presence = presence + self._model = model + self._serial = serial + self._status = status + self._position_in_parent = position_in_parent + + self._direction = direction + self._speed = speed + self._status_led_color = self.STATUS_LED_COLOR_OFF - def get_all_fans(self): - return self._fan_list + def get_direction(self): + return self._direction - def get_powergood_status(self): - return self.status + def get_speed(self): + return self._speed + + def get_status_led(self): + return self._status_led_color def set_status_led(self, color): - self.status_led_color = color + self._status_led_color = color return True - def get_status_led(self): - return self.status_led_color + # Methods inherited from DeviceBase class and related setters + def get_name(self): + return self._name + + def get_presence(self): + return self._presence + + def set_presence(self, presence): + self._presence = presence + + def get_model(self): + return self._model + + def get_serial(self): + return self._serial + + def get_status(self): + return self._status def set_status(self, status): - self.status = status + self._status = status def get_position_in_parent(self): - return self.position_in_parent + return self._position_in_parent + + def is_replaceable(self): + return self._replaceable + + +class MockFanDrawer(fan_drawer_base.FanDrawerBase): + def __init__(self, + name, + position_in_parent, + presence=True, + model='Module Model', + serial='Module Serial', + status=True): + super(MockFanDrawer, self).__init__() + self._name = name + self._presence = presence + self._model = model + self._serial = serial + self._status = status + self._position_in_parent = position_in_parent + + self._max_consumed_power = 500.0 + self._status_led_color = self.STATUS_LED_COLOR_OFF - def set_maximum_supplied_power(self, supplied_power): - self.max_supplied_power = supplied_power + def get_status(self): + return self._status - def get_maximum_supplied_power(self): - return self.max_supplied_power + def set_status(self, status): + self._status = status - @classmethod - def set_status_master_led(cls, color): - cls.master_led_color = color + def get_maximum_consumed_power(self): + return self._max_consumed_power - @classmethod - def get_status_master_led(cls): - return cls.master_led_color + def set_maximum_consumed_power(self, consumed_power): + self._max_consumed_power = consumed_power + def get_status_led(self): + return self._status_led_color + + def set_status_led(self, color): + self._status_led_color = color + return True -class MockFanDrawer(MockDevice): - def __init__(self, fan_drawer_presence, fan_drawer_status, fan_drawer_name): - self.name = fan_drawer_name - self.presence = True - self.fan_drawer_status = fan_drawer_status + # Methods inherited from DeviceBase class and related setters + def get_name(self): + return self._name + + def get_presence(self): + return self._presence + + def set_presence(self, presence): + self._presence = presence + + def get_model(self): + return self._model + + def get_serial(self): + return self._serial def get_status(self): - return self.fan_drawer_status + return self._status def set_status(self, status): - self.fan_drawer_status = status + self._status = status + + def get_position_in_parent(self): + return self._position_in_parent + + def is_replaceable(self): + return self._replaceable + + +class MockModule(module_base.ModuleBase): + def __init__(self, + name, + position_in_parent, + presence=True, + model='Module Model', + serial='Module Serial', + status=True): + super(MockModule, self).__init__() + self._name = name + self._presence = presence + self._model = model + self._serial = serial + self._status = status + self._position_in_parent = position_in_parent + + self._max_consumed_power = 500.0 def set_maximum_consumed_power(self, consumed_power): - self.max_consumed_power = consumed_power + self._max_consumed_power = consumed_power def get_maximum_consumed_power(self): - return self.max_consumed_power + return self._max_consumed_power + # Methods inherited from DeviceBase class and related setters + def get_name(self): + return self._name -class MockFan(MockDevice): - FAN_DIRECTION_INTAKE = "intake" - FAN_DIRECTION_EXHAUST = "exhaust" + def get_presence(self): + return self._presence - def __init__(self, name, direction, speed=50): - self.name = name - self.direction = direction - self.speed = speed - self.status_led_color = self.STATUS_LED_COLOR_OFF + def set_presence(self, presence): + self._presence = presence - def get_direction(self): - return self.direction + def get_model(self): + return self._model - def get_speed(self): - return self.speed + def get_serial(self): + return self._serial - def set_status_led(self, color): - self.status_led_color = color - return True + def get_status(self): + return self._status - def get_status_led(self): - return self.status_led_color + def set_status(self, status): + self._status = status + def get_position_in_parent(self): + return self._position_in_parent + + def is_replaceable(self): + return self._replaceable + + +class MockPsu(psu_base.PsuBase): + def __init__(self, + name, + position_in_parent, + presence=True, + model='Module Model', + serial='Module Serial', + status=True, + voltage=12.0, + current=8.0, + power=100.0, + temp=30.00, + temp_high_th=50.0, + voltage_low_th=11.0, + voltage_high_th=13.0, + replaceable=True): + super(MockPsu, self).__init__() + self._name = name + self._presence = presence + self._model = model + self._serial = serial + self._status = status + self._position_in_parent = position_in_parent + self._replaceable = replaceable + + self._voltage = voltage + self._current = current + self._power = power + self._temp = temp + self._temp_high_th = temp_high_th + self._voltage_low_th = voltage_low_th + self._voltage_high_th = voltage_high_th + self._status_led_color = self.STATUS_LED_COLOR_OFF + + def get_voltage(self): + return self._voltage + + def set_voltage(self, voltage): + self._voltage = voltage + + def get_current(self): + return self._current + + def set_current(self, current): + self._current = current + + def get_power(self): + return self._power + + def set_power(self, power): + self._power = power + def get_powergood_status(self): + return self._status -class MockModule(MockDevice): - def __init__(self, module_presence, module_status, module_name): - self.name = module_name - self.presence = True - self.module_status = module_status + def get_temperature(self): + return self._temp - def get_status(self): - return self.module_status + def set_temperature(self, power): + self._temp = temp - def set_status(self, status): - self.module_status = status + def get_temperature_high_threshold(self): + return self._temp_high_th - def set_maximum_consumed_power(self, consumed_power): - self.max_consumed_power = consumed_power + def get_voltage_high_threshold(self): + return self._voltage_high_th - def get_maximum_consumed_power(self): - return self.max_consumed_power + def get_voltage_low_threshold(self): + return self._voltage_low_th + def get_maximum_supplied_power(self): + return self._max_supplied_power -class MockChassis: - def __init__(self): - self.psu_list = [] - self.fan_drawer_list = [] - self.module_list = [] + def set_maximum_supplied_power(self, supplied_power): + self._max_supplied_power = supplied_power - def get_num_psus(self): - return len(self.psu_list) + def get_status_led(self): + return self._status_led_color - def get_all_psus(self): - return self.psu_list + def set_status_led(self, color): + self._status_led_color = color + return True - def get_psu(self, index): - return self.psu_list[index] + # Methods inherited from DeviceBase class and related setters + def get_name(self): + return self._name - def get_num_fan_drawers(self): - return len(self.fan_drawer_list) + def get_presence(self): + return self._presence - def get_all_fan_drawers(self): - return self.fan_drawer_list + def set_presence(self, presence): + self._presence = presence - def get_num_modules(self): - return len(self.module_list) + def get_model(self): + return self._model - def get_all_modules(self): - return self.module_list + def get_serial(self): + return self._serial + + def get_status(self): + return self._status + + def set_status(self, status): + self._status = status + + def get_position_in_parent(self): + return self._position_in_parent + + def is_replaceable(self): + return self._replaceable diff --git a/sonic-psud/tests/mocked_libs/sonic_platform/__init__.py b/sonic-psud/tests/mocked_libs/sonic_platform/__init__.py new file mode 100644 index 000000000..188f03fce --- /dev/null +++ b/sonic-psud/tests/mocked_libs/sonic_platform/__init__.py @@ -0,0 +1,5 @@ +""" + Mock implementation of sonic_platform package for unit testing +""" + +from . import psu diff --git a/sonic-psud/tests/mocked_libs/sonic_platform/psu.py b/sonic-psud/tests/mocked_libs/sonic_platform/psu.py new file mode 100644 index 000000000..0e3555c07 --- /dev/null +++ b/sonic-psud/tests/mocked_libs/sonic_platform/psu.py @@ -0,0 +1,10 @@ +""" + Mock implementation of sonic_platform package for unit testing +""" + +from sonic_platform_base.psu_base import PsuBase + + +class Psu(PsuBase): + def __init__(self): + super(PsuBase, self).__init__() diff --git a/sonic-psud/tests/mocked_libs/swsscommon/__init__.py b/sonic-psud/tests/mocked_libs/swsscommon/__init__.py new file mode 100644 index 000000000..012af621e --- /dev/null +++ b/sonic-psud/tests/mocked_libs/swsscommon/__init__.py @@ -0,0 +1,5 @@ +''' + Mock implementation of swsscommon package for unit testing +''' + +from . import swsscommon diff --git a/sonic-psud/tests/mock_swsscommon.py b/sonic-psud/tests/mocked_libs/swsscommon/swsscommon.py similarity index 89% rename from sonic-psud/tests/mock_swsscommon.py rename to sonic-psud/tests/mocked_libs/swsscommon/swsscommon.py index ba8c9d1b2..6947a8601 100644 --- a/sonic-psud/tests/mock_swsscommon.py +++ b/sonic-psud/tests/mocked_libs/swsscommon/swsscommon.py @@ -1,3 +1,7 @@ +''' + Mock implementation of swsscommon package for unit testing +''' + STATE_DB = '' @@ -19,6 +23,9 @@ def get(self, key): return self.mock_dict[key] return None + def get_size(self): + return (len(self.mock_dict)) + class FieldValuePairs: fv_dict = {} diff --git a/sonic-psud/tests/test_DaemonPsud.py b/sonic-psud/tests/test_DaemonPsud.py index c905686e4..116886657 100644 --- a/sonic-psud/tests/test_DaemonPsud.py +++ b/sonic-psud/tests/test_DaemonPsud.py @@ -1,7 +1,7 @@ import datetime import os import sys -from imp import load_source +from imp import load_source # Replace with importlib once we no longer need to support Python 2 import pytest @@ -12,108 +12,167 @@ import mock from sonic_py_common import daemon_base -from . import mock_platform -from . import mock_swsscommon +from .mock_platform import MockChassis, MockFan, MockPsu SYSLOG_IDENTIFIER = 'psud_test' NOT_AVAILABLE = 'N/A' daemon_base.db_connect = mock.MagicMock() -test_path = os.path.dirname(os.path.abspath(__file__)) -modules_path = os.path.dirname(test_path) +tests_path = os.path.dirname(os.path.abspath(__file__)) + +# Add mocked_libs path so that the file under test can load mocked modules from there +mocked_libs_path = os.path.join(tests_path, "mocked_libs") +sys.path.insert(0, mocked_libs_path) + +# Add path to the file under test so that we can load it +modules_path = os.path.dirname(tests_path) scripts_path = os.path.join(modules_path, "scripts") sys.path.insert(0, modules_path) - -os.environ["PSUD_UNIT_TESTING"] = "1" -load_source('psud', scripts_path + '/psud') +load_source('psud', os.path.join(scripts_path, 'psud')) import psud + class TestDaemonPsud(object): """ Test cases to cover functionality in DaemonPsud class """ def test_signal_handler(self): + psud.platform_chassis = MockChassis() daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER) - daemon_psud.stop.set = mock.MagicMock() + daemon_psud.stop_event.set = mock.MagicMock() daemon_psud.log_info = mock.MagicMock() daemon_psud.log_warning = mock.MagicMock() # Test SIGHUP daemon_psud.signal_handler(psud.signal.SIGHUP, None) assert daemon_psud.log_info.call_count == 1 - daemon_psud.log_info.assert_called_with("Caught SIGHUP - ignoring...") + daemon_psud.log_info.assert_called_with("Caught signal 'SIGHUP' - ignoring...") assert daemon_psud.log_warning.call_count == 0 - assert daemon_psud.stop.set.call_count == 0 + assert daemon_psud.stop_event.set.call_count == 0 + assert psud.exit_code == 0 - # Test SIGINT + # Reset daemon_psud.log_info.reset_mock() daemon_psud.log_warning.reset_mock() - daemon_psud.stop.set.reset_mock() - daemon_psud.signal_handler(psud.signal.SIGINT, None) + daemon_psud.stop_event.set.reset_mock() + + # Test SIGINT + test_signal = psud.signal.SIGINT + daemon_psud.signal_handler(test_signal, None) assert daemon_psud.log_info.call_count == 1 - daemon_psud.log_info.assert_called_with("Caught SIGINT - exiting...") + daemon_psud.log_info.assert_called_with("Caught signal 'SIGINT' - exiting...") assert daemon_psud.log_warning.call_count == 0 - assert daemon_psud.stop.set.call_count == 1 + assert daemon_psud.stop_event.set.call_count == 1 + assert psud.exit_code == (128 + test_signal) - # Test SIGTERM + # Reset daemon_psud.log_info.reset_mock() daemon_psud.log_warning.reset_mock() - daemon_psud.stop.set.reset_mock() - daemon_psud.signal_handler(psud.signal.SIGTERM, None) + daemon_psud.stop_event.set.reset_mock() + + # Test SIGTERM + test_signal = psud.signal.SIGTERM + daemon_psud.signal_handler(test_signal, None) assert daemon_psud.log_info.call_count == 1 - daemon_psud.log_info.assert_called_with("Caught SIGTERM - exiting...") + daemon_psud.log_info.assert_called_with("Caught signal 'SIGTERM' - exiting...") assert daemon_psud.log_warning.call_count == 0 - assert daemon_psud.stop.set.call_count == 1 + assert daemon_psud.stop_event.set.call_count == 1 + assert psud.exit_code == (128 + test_signal) - # Test an unhandled signal + # Reset daemon_psud.log_info.reset_mock() daemon_psud.log_warning.reset_mock() - daemon_psud.stop.set.reset_mock() + daemon_psud.stop_event.set.reset_mock() + psud.exit_code = 0 + + # Test an unhandled signal daemon_psud.signal_handler(psud.signal.SIGUSR1, None) assert daemon_psud.log_warning.call_count == 1 - daemon_psud.log_warning.assert_called_with("Caught unhandled signal 'SIGUSR1'") + daemon_psud.log_warning.assert_called_with("Caught unhandled signal 'SIGUSR1' - ignoring...") assert daemon_psud.log_info.call_count == 0 - assert daemon_psud.stop.set.call_count == 0 + assert daemon_psud.stop_event.set.call_count == 0 + assert psud.exit_code == 0 + + def test_run(self): + psud.platform_chassis = MockChassis() + daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER) + daemon_psud._update_psu_entity_info = mock.MagicMock() + daemon_psud.update_psu_data = mock.MagicMock() + daemon_psud._update_led_color = mock.MagicMock() + daemon_psud.update_psu_chassis_info = mock.MagicMock() + + daemon_psud.run() + assert daemon_psud.first_run is False def test_update_psu_data(self): - mock_psu1 = mock_platform.MockPsu(True, True, "PSU 1", 0) - mock_psu2 = mock_platform.MockPsu(True, True, "PSU 2", 1) - mock_psu_tbl = mock.MagicMock() + mock_psu1 = MockPsu("PSU 1", 0, True, True) + mock_psu2 = MockPsu("PSU 2", 1, True, True) + psud.platform_chassis = MockChassis() daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER) daemon_psud._update_single_psu_data = mock.MagicMock() daemon_psud.log_warning = mock.MagicMock() # Test platform_chassis is None psud.platform_chassis = None - daemon_psud.update_psu_data(mock_psu_tbl) + daemon_psud.update_psu_data() assert daemon_psud._update_single_psu_data.call_count == 0 assert daemon_psud.log_warning.call_count == 0 # Test with mocked platform_chassis - psud.platform_chassis = mock_platform.MockChassis() - psud.platform_chassis.psu_list = [mock_psu1, mock_psu2] - daemon_psud.update_psu_data(mock_psu_tbl) + psud.platform_chassis = MockChassis() + psud.platform_chassis._psu_list = [mock_psu1, mock_psu2] + daemon_psud.update_psu_data() assert daemon_psud._update_single_psu_data.call_count == 2 - daemon_psud._update_single_psu_data.assert_called_with(2, mock_psu2, mock_psu_tbl) + expected_calls = [mock.call(1, mock_psu1), mock.call(2, mock_psu2)] + assert daemon_psud._update_single_psu_data.mock_calls == expected_calls assert daemon_psud.log_warning.call_count == 0 - daemon_psud.log_warning = mock.MagicMock() daemon_psud._update_single_psu_data.reset_mock() # Test _update_single_psu_data() throws exception daemon_psud._update_single_psu_data.side_effect = Exception("Test message") - daemon_psud.update_psu_data(mock_psu_tbl) + daemon_psud.update_psu_data() assert daemon_psud._update_single_psu_data.call_count == 2 - daemon_psud._update_single_psu_data.assert_called_with(2, mock_psu2, mock_psu_tbl) + expected_calls = [mock.call(1, mock_psu1), mock.call(2, mock_psu2)] + assert daemon_psud._update_single_psu_data.mock_calls == expected_calls assert daemon_psud.log_warning.call_count == 2 - daemon_psud.log_warning.assert_called_with("Failed to update PSU data - Test message") + expected_calls = [mock.call("Failed to update PSU data - Test message")] * 2 + assert daemon_psud.log_warning.mock_calls == expected_calls + + @mock.patch('psud._wrapper_get_psu_presence', mock.MagicMock()) + @mock.patch('psud._wrapper_get_psu_status', mock.MagicMock()) + def test_update_single_psu_data(self): + psud._wrapper_get_psu_presence.return_value = True + psud._wrapper_get_psu_status.return_value = True + + psu1 = MockPsu('PSU 1', 0, True, 'Fake Model', '12345678') + psud.platform_chassis = MockChassis() + psud.platform_chassis._psu_list.append(psu1) + + expected_fvp = psud.swsscommon.FieldValuePairs( + [(psud.PSU_INFO_MODEL_FIELD, 'Fake Model'), + (psud.PSU_INFO_SERIAL_FIELD, '12345678'), + (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, '100.0'), + (psud.PSU_INFO_FRU_FIELD, 'True'), + ]) + + daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER) + daemon_psud.psu_tbl = mock.MagicMock() + daemon_psud._update_single_psu_data(1, psu1) + 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() - mock_psu = mock_platform.MockPsu(True, True, "PSU 1", 0) + mock_psu = MockPsu("PSU 1", 0, True, True) psu_status = psud.PsuStatus(mock_logger, mock_psu) daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER) @@ -158,82 +217,82 @@ def test_set_psu_led(self): assert mock_psu.get_status_led() == mock_psu.STATUS_LED_COLOR_GREEN # Test set_status_led not implemented - mock_psu.set_status_led = mock.MagicMock(side_effect = NotImplementedError) + mock_psu.set_status_led = mock.MagicMock(side_effect=NotImplementedError) daemon_psud.log_warning = mock.MagicMock() daemon_psud._set_psu_led(mock_psu, psu_status) assert daemon_psud.log_warning.call_count == 1 daemon_psud.log_warning.assert_called_with("set_status_led() not implemented") def test_update_led_color(self): - mock_psu = mock_platform.MockPsu(True, True, "PSU 1", 0) - mock_psu_tbl = mock.MagicMock() + mock_psu = MockPsu("PSU 1", 0, True, True) mock_logger = mock.MagicMock() psu_status = psud.PsuStatus(mock_logger, mock_psu) daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER) + daemon_psud.psu_tbl = mock.MagicMock() daemon_psud._update_psu_fan_led_status = mock.MagicMock() # If psud.platform_chassis is None, _update_psu_fan_led_status() should do nothing psud.platform_chassis = None - daemon_psud._update_led_color(mock_psu_tbl) - assert mock_psu_tbl.set.call_count == 0 + daemon_psud._update_led_color() + assert daemon_psud.psu_tbl.set.call_count == 0 assert daemon_psud._update_psu_fan_led_status.call_count == 0 - psud.platform_chassis = mock_platform.MockChassis() + psud.platform_chassis = MockChassis() daemon_psud.psu_status_dict[1] = psu_status - expected_fvp = psud.swsscommon.FieldValuePairs([('led_status', mock_platform.MockPsu.STATUS_LED_COLOR_OFF)]) - daemon_psud._update_led_color(mock_psu_tbl) - assert mock_psu_tbl.set.call_count == 1 - mock_psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + expected_fvp = psud.swsscommon.FieldValuePairs([('led_status', MockPsu.STATUS_LED_COLOR_OFF)]) + 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._update_psu_fan_led_status.assert_called_with(mock_psu, 1) - mock_psu_tbl.set.reset_mock() + daemon_psud.psu_tbl.reset_mock() daemon_psud._update_psu_fan_led_status.reset_mock() - mock_psu.set_status_led(mock_platform.MockPsu.STATUS_LED_COLOR_GREEN) - expected_fvp = psud.swsscommon.FieldValuePairs([('led_status', mock_platform.MockPsu.STATUS_LED_COLOR_GREEN)]) - daemon_psud._update_led_color(mock_psu_tbl) - assert mock_psu_tbl.set.call_count == 1 - mock_psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + 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._update_psu_fan_led_status.assert_called_with(mock_psu, 1) - mock_psu_tbl.set.reset_mock() + daemon_psud.psu_tbl.reset_mock() daemon_psud._update_psu_fan_led_status.reset_mock() - mock_psu.set_status_led(mock_platform.MockPsu.STATUS_LED_COLOR_RED) - expected_fvp = psud.swsscommon.FieldValuePairs([('led_status', mock_platform.MockPsu.STATUS_LED_COLOR_RED)]) - daemon_psud._update_led_color(mock_psu_tbl) - assert mock_psu_tbl.set.call_count == 1 - mock_psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + 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() + 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._update_psu_fan_led_status.assert_called_with(mock_psu, 1) - mock_psu_tbl.set.reset_mock() + daemon_psud.psu_tbl.reset_mock() daemon_psud._update_psu_fan_led_status.reset_mock() # Test exception handling - mock_psu.get_status_led = mock.Mock(side_effect = NotImplementedError) + 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(mock_psu_tbl) - assert mock_psu_tbl.set.call_count == 1 - mock_psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) + 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._update_psu_fan_led_status.assert_called_with(mock_psu, 1) def test_update_psu_fan_led_status(self): - mock_fan = mock_platform.MockFan("PSU 1 Test Fan 1", mock_platform.MockFan.FAN_DIRECTION_INTAKE) - mock_psu = mock_platform.MockPsu(True, True, "PSU 1", 0) + mock_fan = MockFan("PSU 1 Test Fan 1", MockFan.FAN_DIRECTION_INTAKE) + mock_psu = MockPsu("PSU 1", 0, True, True) mock_psu._fan_list = [mock_fan] mock_logger = mock.MagicMock() - psud.platform_chassis = mock_platform.MockChassis() + psud.platform_chassis = MockChassis() daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER) daemon_psud.fan_tbl = mock.MagicMock() - expected_fvp = psud.swsscommon.FieldValuePairs([(psud.FAN_INFO_LED_STATUS_FIELD, mock_platform.MockFan.STATUS_LED_COLOR_OFF)]) + expected_fvp = psud.swsscommon.FieldValuePairs([(psud.FAN_INFO_LED_STATUS_FIELD, MockFan.STATUS_LED_COLOR_OFF)]) daemon_psud._update_psu_fan_led_status(mock_psu, 1) assert daemon_psud.fan_tbl.set.call_count == 1 daemon_psud.fan_tbl.set.assert_called_with("PSU 1 Test Fan 1", expected_fvp) @@ -241,7 +300,7 @@ def test_update_psu_fan_led_status(self): daemon_psud.fan_tbl.set.reset_mock() # Test Fan.get_status_led not implemented - mock_fan.get_status_led = mock.Mock(side_effect = NotImplementedError) + mock_fan.get_status_led = mock.Mock(side_effect=NotImplementedError) expected_fvp = psud.swsscommon.FieldValuePairs([(psud.FAN_INFO_LED_STATUS_FIELD, psud.NOT_AVAILABLE)]) daemon_psud._update_psu_fan_led_status(mock_psu, 1) assert daemon_psud.fan_tbl.set.call_count == 1 @@ -250,7 +309,7 @@ def test_update_psu_fan_led_status(self): daemon_psud.fan_tbl.set.reset_mock() # Test Fan.get_name not implemented - mock_fan.get_name = mock.Mock(side_effect = NotImplementedError) + mock_fan.get_name = mock.Mock(side_effect=NotImplementedError) daemon_psud._update_psu_fan_led_status(mock_psu, 1) assert daemon_psud.fan_tbl.set.call_count == 1 daemon_psud.fan_tbl.set.assert_called_with("PSU 1 FAN 1", expected_fvp) @@ -262,19 +321,18 @@ def test_update_psu_chassis_info(self): # If psud.platform_chassis is None, update_psu_chassis_info() should do nothing psud.platform_chassis = None daemon_psud.psu_chassis_info = None - daemon_psud.update_psu_chassis_info(None) + daemon_psud.update_psu_chassis_info() assert daemon_psud.psu_chassis_info is None # Now we mock platform_chassis, so that daemon_psud.psu_chassis_info should be instantiated and run_power_budget() should be called - psud.platform_chassis = mock_platform.MockChassis() - daemon_psud.update_psu_chassis_info(None) + psud.platform_chassis = MockChassis() + daemon_psud.update_psu_chassis_info() assert daemon_psud.psu_chassis_info is not None assert daemon_psud.psu_chassis_info.run_power_budget.call_count == 1 - daemon_psud.psu_chassis_info.run_power_budget.assert_called_with(None) def test_update_psu_entity_info(self): - mock_psu1 = mock_platform.MockPsu(True, True, "PSU 1", 0) - mock_psu2 = mock_platform.MockPsu(True, True, "PSU 2", 1) + mock_psu1 = MockPsu("PSU 1", 0, True, True) + mock_psu2 = MockPsu("PSU 2", 1, True, True) daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER) daemon_psud._update_single_psu_entity_info = mock.MagicMock() @@ -284,23 +342,24 @@ def test_update_psu_entity_info(self): daemon_psud._update_psu_entity_info() assert daemon_psud._update_single_psu_entity_info.call_count == 0 - psud.platform_chassis = mock_platform.MockChassis() - psud.platform_chassis.psu_list = [mock_psu1] + psud.platform_chassis = MockChassis() + psud.platform_chassis._psu_list = [mock_psu1] daemon_psud._update_psu_entity_info() assert daemon_psud._update_single_psu_entity_info.call_count == 1 daemon_psud._update_single_psu_entity_info.assert_called_with(1, mock_psu1) daemon_psud._update_single_psu_entity_info.reset_mock() - psud.platform_chassis.psu_list = [mock_psu1, mock_psu2] + psud.platform_chassis._psu_list = [mock_psu1, mock_psu2] daemon_psud._update_psu_entity_info() assert daemon_psud._update_single_psu_entity_info.call_count == 2 - daemon_psud._update_single_psu_entity_info.assert_called_with(2, mock_psu2) + expected_calls = [mock.call(1, mock_psu1), mock.call(2, mock_psu2)] + assert daemon_psud._update_single_psu_entity_info.mock_calls == expected_calls # Test behavior if _update_single_psu_entity_info raises an exception daemon_psud._update_single_psu_entity_info.reset_mock() daemon_psud._update_single_psu_entity_info.side_effect = Exception("Test message") daemon_psud.log_warning = mock.MagicMock() - psud.platform_chassis.psu_list = [mock_psu1] + psud.platform_chassis._psu_list = [mock_psu1] daemon_psud._update_psu_entity_info() assert daemon_psud._update_single_psu_entity_info.call_count == 1 daemon_psud._update_single_psu_entity_info.assert_called_with(1, mock_psu1) @@ -309,7 +368,7 @@ def test_update_psu_entity_info(self): @mock.patch('psud.try_get', mock.MagicMock(return_value=0)) def test_update_single_psu_entity_info(self): - mock_psu1 = mock_platform.MockPsu(True, True, "PSU 1", 0) + mock_psu1 = MockPsu("PSU 1", 0, True, True) expected_fvp = psud.swsscommon.FieldValuePairs( [('position_in_parent', '0'), @@ -327,23 +386,23 @@ def test_update_psu_fan_data(self, mock_datetime): fake_time = datetime.datetime(2021, 1, 1, 12, 34, 56) mock_datetime.now.return_value = fake_time - mock_fan = mock_platform.MockFan("PSU 1 Test Fan 1", mock_platform.MockFan.FAN_DIRECTION_INTAKE) - mock_psu1 = mock_platform.MockPsu(True, True, "PSU 1", 0) + mock_fan = MockFan("PSU 1 Test Fan 1", MockFan.FAN_DIRECTION_INTAKE) + mock_psu1 = MockPsu("PSU 1", 0, True, True) mock_psu1._fan_list = [mock_fan] mock_logger = mock.MagicMock() - psud.platform_chassis = mock_platform.MockChassis() - psud.platform_chassis.psu_list = [mock_psu1] + psud.platform_chassis = MockChassis() + psud.platform_chassis._psu_list = [mock_psu1] daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER) daemon_psud.fan_tbl = mock.MagicMock() expected_fvp = psud.swsscommon.FieldValuePairs( - [(psud.FAN_INFO_PRESENCE_FIELD, "True"), - (psud.FAN_INFO_STATUS_FIELD, "True"), - (psud.FAN_INFO_DIRECTION_FIELD, mock_fan.get_direction()), - (psud.FAN_INFO_SPEED_FIELD, str(mock_fan.get_speed())), - (psud.FAN_INFO_TIMESTAMP_FIELD, fake_time.strftime('%Y%m%d %H:%M:%S')) + [(psud.FAN_INFO_PRESENCE_FIELD, "True"), + (psud.FAN_INFO_STATUS_FIELD, "True"), + (psud.FAN_INFO_DIRECTION_FIELD, mock_fan.get_direction()), + (psud.FAN_INFO_SPEED_FIELD, str(mock_fan.get_speed())), + (psud.FAN_INFO_TIMESTAMP_FIELD, fake_time.strftime('%Y%m%d %H:%M:%S')) ]) daemon_psud._update_psu_fan_data(mock_psu1, 1) assert daemon_psud.fan_tbl.set.call_count == 1 diff --git a/sonic-psud/tests/test_PsuChassisInfo.py b/sonic-psud/tests/test_PsuChassisInfo.py index 2757f3e55..4231b497e 100644 --- a/sonic-psud/tests/test_PsuChassisInfo.py +++ b/sonic-psud/tests/test_PsuChassisInfo.py @@ -1,6 +1,6 @@ import os import sys -from imp import load_source +from imp import load_source # Replace with importlib once we no longer need to support Python 2 import pytest @@ -11,7 +11,6 @@ import mock from sonic_py_common import daemon_base -from . import mock_swsscommon from .mock_platform import MockChassis, MockPsu, MockFanDrawer, MockModule SYSLOG_IDENTIFIER = 'test_PsuChassisInfo' @@ -19,15 +18,24 @@ daemon_base.db_connect = mock.MagicMock() -test_path = os.path.dirname(os.path.abspath(__file__)) -modules_path = os.path.dirname(test_path) +tests_path = os.path.dirname(os.path.abspath(__file__)) + +# Add mocked_libs path so that the file under test can load mocked modules from there +mocked_libs_path = os.path.join(tests_path, "mocked_libs") +sys.path.insert(0, mocked_libs_path) + +# We also need to load the mocked swsscommon locally for use below +load_source('swsscommon', os.path.join(mocked_libs_path, 'swsscommon', 'swsscommon.py')) +import swsscommon as mock_swsscommon + +# Add path to the file under test so that we can load it +modules_path = os.path.dirname(tests_path) scripts_path = os.path.join(modules_path, "scripts") sys.path.insert(0, modules_path) - -os.environ["PSUD_UNIT_TESTING"] = "1" -load_source('psud', scripts_path + '/psud') +load_source('psud', os.path.join(scripts_path, 'psud')) import psud + CHASSIS_INFO_TABLE = 'CHASSIS_INFO' CHASSIS_INFO_KEY_TEMPLATE = 'chassis {}' @@ -52,6 +60,7 @@ class TestPsuChassisInfo(object): """ Test cases to cover functionality in PsuChassisInfo class """ + def test_update_master_status(self): chassis = MockChassis() chassis_info = psud.PsuChassisInfo(SYSLOG_IDENTIFIER, chassis) @@ -97,33 +106,22 @@ def test_update_master_status(self): assert ret == False assert chassis_info.master_status_good == False - # Test set_status_master_led not implemented - with mock.patch.object(MockPsu, "set_status_master_led", mock.Mock(side_effect = NotImplementedError)): - chassis_info.total_supplied_power = 510.0 - chassis_info.total_consumed_power = 350.0 - chassis_info.master_status_good = False - chassis_info.log_warning = mock.MagicMock() - ret = chassis_info.update_master_status() - assert ret == True - assert chassis_info.master_status_good == True - chassis_info.log_warning.assert_called_with("set_status_master_led() not implemented") - def test_supplied_power(self): chassis = MockChassis() - psu1 = MockPsu(True, True, "PSU 1", 0) + psu1 = MockPsu("PSU 1", 0, True, True) psu1_power = 510.0 psu1.set_maximum_supplied_power(psu1_power) - chassis.psu_list.append(psu1) + chassis._psu_list.append(psu1) - psu2 = MockPsu(True, True, "PSU 2", 1) + psu2 = MockPsu("PSU 2", 1, True, True) psu2_power = 800.0 psu2.set_maximum_supplied_power(psu2_power) - chassis.psu_list.append(psu2) + chassis._psu_list.append(psu2) - psu3 = MockPsu(True, True, "PSU 3", 2) + psu3 = MockPsu("PSU 3", 2, True, True) psu3_power = 350.0 psu3.set_maximum_supplied_power(psu3_power) - chassis.psu_list.append(psu3) + chassis._psu_list.append(psu3) total_power = psu1_power + psu2_power + psu3_power state_db = daemon_base.db_connect("STATE_DB") @@ -151,15 +149,15 @@ def test_supplied_power(self): def test_consumed_power(self): chassis = MockChassis() - fan_drawer1 = MockFanDrawer(True, True, "FanDrawer 1") + fan_drawer1 = MockFanDrawer("FanDrawer 1", 0, True, True) fan_drawer1_power = 510.0 fan_drawer1.set_maximum_consumed_power(fan_drawer1_power) - chassis.fan_drawer_list.append(fan_drawer1) + chassis._fan_drawer_list.append(fan_drawer1) - module1 = MockFanDrawer(True, True, "Module 1") + module1 = MockModule("Module 1", 0, True, True) module1_power = 700.0 module1.set_maximum_consumed_power(module1_power) - chassis.module_list.append(module1) + chassis._module_list.append(module1) total_power = fan_drawer1_power + module1_power state_db = daemon_base.db_connect("STATE_DB") @@ -186,58 +184,63 @@ def test_consumed_power(self): fvs = chassis_tbl.get(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1)) assert total_power == float(fvs[CHASSIS_INFO_TOTAL_POWER_CONSUMED_FIELD]) - def test_power_budget(self): chassis = MockChassis() - psu = MockPsu(True, True, "PSU 1", 0) + psu1 = MockPsu("PSU 1", 0, True, True) psu1_power = 510.0 - psu.set_maximum_supplied_power(psu1_power) - chassis.psu_list.append(psu) + psu1.set_maximum_supplied_power(psu1_power) + chassis._psu_list.append(psu1) - fan_drawer1 = MockFanDrawer(True, True, "FanDrawer 1") + fan_drawer1 = MockFanDrawer("FanDrawer 1", 0, True, True) fan_drawer1_power = 510.0 fan_drawer1.set_maximum_consumed_power(fan_drawer1_power) - chassis.fan_drawer_list.append(fan_drawer1) + chassis._fan_drawer_list.append(fan_drawer1) - module1 = MockFanDrawer(True, True, "Module 1") + module1 = MockModule("Module 1", 0, True, True) module1_power = 700.0 module1.set_maximum_consumed_power(module1_power) - chassis.module_list.append(module1) + chassis._module_list.append(module1) state_db = daemon_base.db_connect("STATE_DB") chassis_tbl = mock_swsscommon.Table(state_db, CHASSIS_INFO_TABLE) chassis_info = psud.PsuChassisInfo(SYSLOG_IDENTIFIER, chassis) - # Check if supplied_power < consumed_power + # Check case where supplied_power < consumed_power chassis_info.run_power_budget(chassis_tbl) chassis_info.update_master_status() fvs = chassis_tbl.get(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1)) assert float(fvs[CHASSIS_INFO_TOTAL_POWER_SUPPLIED_FIELD]) < float(fvs[CHASSIS_INFO_TOTAL_POWER_CONSUMED_FIELD]) assert chassis_info.master_status_good == False - assert MockPsu.get_status_master_led() == MockPsu.STATUS_LED_COLOR_RED + + # We cannot call get_status_master_led() on our mocked PSUs, because + # they are not instantiated from the same Psu class loaded in psud, + # so we must call it on the class there. + assert psud.Psu.get_status_master_led() == MockPsu.STATUS_LED_COLOR_RED # Add a PSU - psu = MockPsu(True, True, "PSU 2", 1) + psu2 = MockPsu("PSU 2", 1, True, True) psu2_power = 800.0 - psu.set_maximum_supplied_power(psu2_power) - chassis.psu_list.append(psu) + psu2.set_maximum_supplied_power(psu2_power) + chassis._psu_list.append(psu2) - # Check if supplied_power > consumed_power + # Check case where supplied_power > consumed_power chassis_info.run_power_budget(chassis_tbl) chassis_info.update_master_status() fvs = chassis_tbl.get(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1)) assert float(fvs[CHASSIS_INFO_TOTAL_POWER_SUPPLIED_FIELD]) > float(fvs[CHASSIS_INFO_TOTAL_POWER_CONSUMED_FIELD]) assert chassis_info.master_status_good == True - assert MockPsu.get_status_master_led() == MockPsu.STATUS_LED_COLOR_GREEN + # We cannot call get_status_master_led() on our mocked PSUs, because + # they are not instantiated from the same Psu class loaded in psud, + # so we must call it on the class there. + assert psud.Psu.get_status_master_led() == MockPsu.STATUS_LED_COLOR_GREEN def test_get_psu_key(self): assert psud.get_psu_key(0) == psud.PSU_INFO_KEY_TEMPLATE.format(0) assert psud.get_psu_key(1) == psud.PSU_INFO_KEY_TEMPLATE.format(1) - def test_try_get(self): # Test a proper, working callback GOOD_CALLBACK_RETURN_VALUE = "This is a test" diff --git a/sonic-psud/tests/test_PsuStatus.py b/sonic-psud/tests/test_PsuStatus.py index e55384d8d..de7fca542 100644 --- a/sonic-psud/tests/test_PsuStatus.py +++ b/sonic-psud/tests/test_PsuStatus.py @@ -1,6 +1,6 @@ import os import sys -from imp import load_source +from imp import load_source # Replace with importlib once we no longer need to support Python 2 # TODO: Clean this up once we no longer need to support Python 2 if sys.version_info.major == 3: @@ -10,13 +10,17 @@ from .mock_platform import MockPsu -test_path = os.path.dirname(os.path.abspath(__file__)) -modules_path = os.path.dirname(test_path) +tests_path = os.path.dirname(os.path.abspath(__file__)) + +# Add mocked_libs path so that the file under test can load mocked modules from there +mocked_libs_path = os.path.join(tests_path, "mocked_libs") +sys.path.insert(0, mocked_libs_path) + +# Add path to the file under test so that we can load it +modules_path = os.path.dirname(tests_path) scripts_path = os.path.join(modules_path, "scripts") sys.path.insert(0, modules_path) - -os.environ["PSUD_UNIT_TESTING"] = "1" -load_source('psud', scripts_path + '/psud') +load_source('psud', os.path.join(scripts_path, 'psud')) import psud @@ -27,7 +31,7 @@ class TestPsuStatus(object): def test_set_presence(self): mock_logger = mock.MagicMock() - mock_psu = MockPsu(True, True, "PSU 1", 0) + mock_psu = MockPsu("PSU 1", 0, True, True) psu_status = psud.PsuStatus(mock_logger, mock_psu) assert psu_status.presence == False @@ -49,7 +53,7 @@ def test_set_presence(self): def test_set_power_good(self): mock_logger = mock.MagicMock() - mock_psu = MockPsu(True, True, "PSU 1", 0) + mock_psu = MockPsu("PSU 1", 0, True, True) psu_status = psud.PsuStatus(mock_logger, mock_psu) assert psu_status.power_good == False @@ -76,7 +80,7 @@ def test_set_power_good(self): def test_set_voltage(self): mock_logger = mock.MagicMock() - mock_psu = MockPsu(True, True, "PSU 1", 0) + mock_psu = MockPsu("PSU 1", 0, True, True) psu_status = psud.PsuStatus(mock_logger, mock_psu) assert psu_status.voltage_good == False @@ -143,7 +147,7 @@ def test_set_voltage(self): def test_set_temperature(self): mock_logger = mock.MagicMock() - mock_psu = MockPsu(True, True, "PSU 1", 0) + mock_psu = MockPsu("PSU 1", 0, True, True) psu_status = psud.PsuStatus(mock_logger, mock_psu) assert psu_status.temperature_good == False @@ -193,7 +197,7 @@ def test_set_temperature(self): def test_is_ok(self): mock_logger = mock.MagicMock() - mock_psu = MockPsu(True, True, "PSU 1", 0) + mock_psu = MockPsu("PSU 1", 0, True, True) psu_status = psud.PsuStatus(mock_logger, mock_psu) psu_status.presence = True diff --git a/sonic-psud/tests/test_psud.py b/sonic-psud/tests/test_psud.py index a8d36fd84..a66a8c29a 100644 --- a/sonic-psud/tests/test_psud.py +++ b/sonic-psud/tests/test_psud.py @@ -1,6 +1,6 @@ import os import sys -from imp import load_source +from imp import load_source # Replace with importlib once we no longer need to support Python 2 import pytest @@ -11,23 +11,29 @@ import mock from sonic_py_common import daemon_base -from . import mock_swsscommon -from .mock_platform import MockChassis, MockPsu, MockFanDrawer, MockModule +from .mock_platform import MockPsu -SYSLOG_IDENTIFIER = 'psud_test' -NOT_AVAILABLE = 'N/A' +tests_path = os.path.dirname(os.path.abspath(__file__)) -daemon_base.db_connect = mock.MagicMock() +# Add mocked_libs path so that the file under test can load mocked modules from there +mocked_libs_path = os.path.join(tests_path, "mocked_libs") +sys.path.insert(0, mocked_libs_path) -test_path = os.path.dirname(os.path.abspath(__file__)) -modules_path = os.path.dirname(test_path) +# Add path to the file under test so that we can load it +modules_path = os.path.dirname(tests_path) scripts_path = os.path.join(modules_path, "scripts") sys.path.insert(0, modules_path) - -os.environ["PSUD_UNIT_TESTING"] = "1" -load_source('psud', scripts_path + '/psud') +load_source('psud', os.path.join(scripts_path, 'psud')) import psud + +daemon_base.db_connect = mock.MagicMock() + + +SYSLOG_IDENTIFIER = 'psud_test' +NOT_AVAILABLE = 'N/A' + + @mock.patch('psud.platform_chassis', mock.MagicMock()) @mock.patch('psud.platform_psuutil', mock.MagicMock()) def test_wrapper_get_num_psus(): @@ -171,3 +177,11 @@ def test_log_on_status_changed(): assert mock_logger.log_notice.call_count == 0 assert mock_logger.log_warning.call_count == 1 mock_logger.log_warning.assert_called_with(abnormal_log) + + +@mock.patch('psud.DaemonPsud.run') +def test_main(mock_run): + mock_run.return_value = False + + psud.main() + assert mock_run.call_count == 1 From 450b7d783e2705677127dca711c73105170635ba Mon Sep 17 00:00:00 2001 From: Stephen Sun <5379172+stephenxs@users.noreply.github.com> Date: Wed, 31 Mar 2021 00:17:53 +0800 Subject: [PATCH 04/11] Bug fix: the fields that are not supported by vendor should be "N/A" in STATE_DB (#168) - Initialize fields as "N/A" and set the field to "N/A" for those not supported by vendor API - Update set_voltage, set_temperature, treating "N/A" instead of None as invalid values - Update unit test cases accordingly Signed-off-by: Stephen Sun --- sonic-psud/scripts/psud | 36 +++++++++++++++--------------- sonic-psud/tests/test_PsuStatus.py | 20 ++++++++--------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/sonic-psud/scripts/psud b/sonic-psud/scripts/psud index d33ad1861..8a41fc054 100644 --- a/sonic-psud/scripts/psud +++ b/sonic-psud/scripts/psud @@ -305,7 +305,7 @@ class PsuStatus(object): return True def set_voltage(self, voltage, high_threshold, low_threshold): - if not voltage or not high_threshold or not low_threshold: + if voltage == NOT_AVAILABLE or high_threshold == NOT_AVAILABLE or low_threshold == NOT_AVAILABLE: if self.voltage_good is not True: self.logger.log_warning('PSU voltage or high_threshold or low_threshold become unavailable, ' 'voltage={}, high_threshold={}, low_threshold={}'.format(voltage, high_threshold, low_threshold)) @@ -320,7 +320,7 @@ class PsuStatus(object): return True def set_temperature(self, temperature, high_threshold): - if not temperature or not high_threshold: + if temperature == NOT_AVAILABLE or high_threshold == NOT_AVAILABLE: if self.temperature_good is not True: self.logger.log_warning('PSU temperature or high_threshold become unavailable, ' 'temperature={}, high_threshold={}'.format(temperature, high_threshold)) @@ -445,23 +445,23 @@ class DaemonPsud(daemon_base.DaemonBase): name = get_psu_key(index) presence = _wrapper_get_psu_presence(index) power_good = False - voltage = None - voltage_high_threshold = None - voltage_low_threshold = None - temperature = None - temperature_threshold = None - current = None - power = None + voltage = NOT_AVAILABLE + voltage_high_threshold = NOT_AVAILABLE + voltage_low_threshold = NOT_AVAILABLE + temperature = NOT_AVAILABLE + temperature_threshold = NOT_AVAILABLE + current = NOT_AVAILABLE + power = NOT_AVAILABLE is_replaceable = try_get(psu.is_replaceable, False) if presence: power_good = _wrapper_get_psu_status(index) - voltage = try_get(psu.get_voltage) - voltage_high_threshold = try_get(psu.get_voltage_high_threshold) - 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) + 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) + temperature = try_get(psu.get_temperature, NOT_AVAILABLE) + temperature_threshold = try_get(psu.get_temperature_high_threshold, NOT_AVAILABLE) + current = try_get(psu.get_current, NOT_AVAILABLE) + power = try_get(psu.get_power, NOT_AVAILABLE) if index not in self.psu_status_dict: self.psu_status_dict[index] = PsuStatus(self, psu) @@ -508,8 +508,8 @@ class DaemonPsud(daemon_base.DaemonBase): self._set_psu_led(psu, psu_status) fvs = swsscommon.FieldValuePairs( - [(PSU_INFO_MODEL_FIELD, str(try_get(psu.get_model))), - (PSU_INFO_SERIAL_FIELD, str(try_get(psu.get_serial))), + [(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_TEMP_FIELD, str(temperature)), (PSU_INFO_TEMP_TH_FIELD, str(temperature_threshold)), (PSU_INFO_VOLTAGE_FIELD, str(voltage)), diff --git a/sonic-psud/tests/test_PsuStatus.py b/sonic-psud/tests/test_PsuStatus.py index de7fca542..7505bc04c 100644 --- a/sonic-psud/tests/test_PsuStatus.py +++ b/sonic-psud/tests/test_PsuStatus.py @@ -121,27 +121,27 @@ def test_set_voltage(self): assert psu_status.voltage_good == True # Test passing parameters as None when voltage_good == True - ret = psu_status.set_voltage(None, 12.5, 11.5) + ret = psu_status.set_voltage(psud.NOT_AVAILABLE, 12.5, 11.5) assert ret == False assert psu_status.voltage_good == True - ret = psu_status.set_voltage(11.5, None, 11.5) + ret = psu_status.set_voltage(11.5, psud.NOT_AVAILABLE, 11.5) assert ret == False assert psu_status.voltage_good == True - ret = psu_status.set_voltage(11.5, 12.5, None) + ret = psu_status.set_voltage(11.5, 12.5, psud.NOT_AVAILABLE) assert ret == False assert psu_status.voltage_good == True # Test passing parameters as None when voltage_good == False psu_status.voltage_good = False - ret = psu_status.set_voltage(None, 12.5, 11.5) + ret = psu_status.set_voltage(psud.NOT_AVAILABLE, 12.5, 11.5) assert ret == False assert psu_status.voltage_good == True psu_status.voltage_good = False - ret = psu_status.set_voltage(11.5, None, 11.5) + ret = psu_status.set_voltage(11.5, psud.NOT_AVAILABLE, 11.5) assert ret == False assert psu_status.voltage_good == True psu_status.voltage_good = False - ret = psu_status.set_voltage(11.5, 12.5, None) + ret = psu_status.set_voltage(11.5, 12.5, psud.NOT_AVAILABLE) assert ret == False assert psu_status.voltage_good == True @@ -178,20 +178,20 @@ def test_set_temperature(self): assert psu_status.temperature_good == True # Test passing parameters as None when temperature_good == True - ret = psu_status.set_temperature(None, 50.0) + ret = psu_status.set_temperature(psud.NOT_AVAILABLE, 50.0) assert ret == False assert psu_status.temperature_good == True - ret = psu_status.set_temperature(20.123, None) + ret = psu_status.set_temperature(20.123, psud.NOT_AVAILABLE) assert ret == False assert psu_status.temperature_good == True # Test passing parameters as None when temperature_good == False psu_status.temperature_good = False - ret = psu_status.set_temperature(None, 50.0) + ret = psu_status.set_temperature(psud.NOT_AVAILABLE, 50.0) assert ret == False assert psu_status.temperature_good == True psu_status.temperature_good = False - ret = psu_status.set_temperature(20.123, None) + ret = psu_status.set_temperature(20.123, psud.NOT_AVAILABLE) assert ret == False assert psu_status.temperature_good == True From 7f01b2c8993c442568c0a07de1be2065bf79e383 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Mon, 5 Apr 2021 12:27:58 -0700 Subject: [PATCH 05/11] [xcvrd] Gracefully handle improper 'specification_compliance' field; also fix other potential bugs (#169) #### Description Gracefully handle improper 'specification_compliance' field; also fix other potential bugs and adjust some style #### Motivation and Context The 'specification_compliance' field of transceiver info is expected to be a string representation of a dictionary. However, there is a chance, upon some kind of platform issue that a vendor's platform API returns something like 'N/A'. In this case, xcrvd would crash. Rather than crash, xcvrd should handle this gracefully and log a warning message instead. Also fixed a couple potential bugs where `default_dict` was being compared to `0`, when it should have been `len(default_dict)` Also rename some constants using uppercase naming convention. --- sonic-xcvrd/tests/test_xcvrd.py | 22 ++++++++++ sonic-xcvrd/xcvrd/xcvrd.py | 78 +++++++++++++++++++-------------- 2 files changed, 68 insertions(+), 32 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 36140a14c..620ca3a96 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -317,3 +317,25 @@ def test_post_port_mux_static_info_to_db(self): mux_tbl = Table("STATE_DB", y_cable_helper.MUX_CABLE_STATIC_INFO_TABLE) rc = post_port_mux_static_info_to_db(logical_port_name, mux_tbl) assert(rc != -1) + + def test_get_media_settings_key(self): + xcvr_info_dict = { + 0: { + 'manufacturer': 'Molex', + 'model': '1064141421', + 'cable_type': 'Length Cable Assembly(m)', + 'cable_length': '255', + 'specification_compliance': "{'10/40G Ethernet Compliance Code': '10GBase-SR'}", + 'type_abbrv_name': 'QSFP+' + } + } + + # Test a good 'specification_compliance' value + result = get_media_settings_key(0, xcvr_info_dict) + assert result == ['MOLEX-1064141421', 'QSFP+-10GBase-SR-255M'] + + # Test a bad 'specification_compliance' value + xcvr_info_dict[0]['specification_compliance'] = 'N/A' + result = get_media_settings_key(0, xcvr_info_dict) + assert result == ['MOLEX-1064141421', 'QSFP+'] + # TODO: Ensure that error message was logged diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index cecb582b2..0ae0a0cfd 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -545,8 +545,9 @@ def recover_missing_sfp_table_entries(sfp_util, int_tbl, status_tbl, stop_event) def check_port_in_range(range_str, physical_port): - range_separator = '-' - range_list = range_str.split(range_separator) + RANGE_SEPARATOR = '-' + + range_list = range_str.split(RANGE_SEPARATOR) start_num = int(range_list[0].strip()) end_num = int(range_list[1].strip()) if start_num <= physical_port <= end_num: @@ -555,8 +556,11 @@ def check_port_in_range(range_str, physical_port): def get_media_settings_value(physical_port, key): - range_separator = '-' - comma_separator = ',' + GLOBAL_MEDIA_SETTINGS_KEY = 'GLOBAL_MEDIA_SETTINGS' + PORT_MEDIA_SETTINGS_KEY = 'PORT_MEDIA_SETTINGS' + DEFAULT_KEY = 'Default' + RANGE_SEPARATOR = '-' + COMMA_SEPARATOR = ',' media_dict = {} default_dict = {} @@ -566,22 +570,22 @@ def get_media_settings_value(physical_port, key): # 1,2,3,4,5 # 1-4,9-12 - if "GLOBAL_MEDIA_SETTINGS" in g_dict: - for keys in g_dict["GLOBAL_MEDIA_SETTINGS"]: - if comma_separator in keys: - port_list = keys.split(comma_separator) + if GLOBAL_MEDIA_SETTINGS_KEY in g_dict: + for keys in g_dict[GLOBAL_MEDIA_SETTINGS_KEY]: + if COMMA_SEPARATOR in keys: + port_list = keys.split(COMMA_SEPARATOR) for port in port_list: - if range_separator in port: + if RANGE_SEPARATOR in port: if check_port_in_range(port, physical_port): - media_dict = g_dict["GLOBAL_MEDIA_SETTINGS"][keys] + media_dict = g_dict[GLOBAL_MEDIA_SETTINGS_KEY][keys] break elif str(physical_port) == port: - media_dict = g_dict["GLOBAL_MEDIA_SETTINGS"][keys] + media_dict = g_dict[GLOBAL_MEDIA_SETTINGS_KEY][keys] break - elif range_separator in keys: + elif RANGE_SEPARATOR in keys: if check_port_in_range(keys, physical_port): - media_dict = g_dict["GLOBAL_MEDIA_SETTINGS"][keys] + media_dict = g_dict[GLOBAL_MEDIA_SETTINGS_KEY][keys] # If there is a match in the global profile for a media type, # fetch those values @@ -589,36 +593,38 @@ def get_media_settings_value(physical_port, key): return media_dict[key[0]] elif key[1] in media_dict: return media_dict[key[1]] - elif "Default" in media_dict: - default_dict = media_dict['Default'] + elif DEFAULT_KEY in media_dict: + default_dict = media_dict[DEFAULT_KEY] media_dict = {} - if "PORT_MEDIA_SETTINGS" in g_dict: - for keys in g_dict["PORT_MEDIA_SETTINGS"]: + if PORT_MEDIA_SETTINGS_KEY in g_dict: + for keys in g_dict[PORT_MEDIA_SETTINGS_KEY]: if int(keys) == physical_port: - media_dict = g_dict["PORT_MEDIA_SETTINGS"][keys] + media_dict = g_dict[PORT_MEDIA_SETTINGS_KEY][keys] break + if len(media_dict) == 0: - if default_dict != 0: + if len(default_dict) != 0: return default_dict else: helper_logger.log_error("Error: No values for physical port '{}'".format(physical_port)) return {} + if key[0] in media_dict: return media_dict[key[0]] elif key[1] in media_dict: return media_dict[key[1]] - elif "Default" in media_dict: - return media_dict['Default'] + elif DEFAULT_KEY in media_dict: + return media_dict[DEFAULT_KEY] elif len(default_dict) != 0: return default_dict - else: - return {} else: - if default_dict != 0: + if len(default_dict) != 0: return default_dict + return {} + def get_media_settings_key(physical_port, transceiver_dict): sup_compliance_str = '10/40G Ethernet Compliance Code' @@ -632,7 +638,13 @@ def get_media_settings_key(physical_port, transceiver_dict): media_len = transceiver_dict[physical_port]['cable_length'] media_compliance_dict_str = transceiver_dict[physical_port]['specification_compliance'] - media_compliance_dict = ast.literal_eval(media_compliance_dict_str) + + media_compliance_dict = {} + try: + media_compliance_dict = ast.literal_eval(media_compliance_dict_str) + except ValueError as e: + helper_logger.log_error("Invalid value for port {} 'specification_compliance': {}".format(physical_port, media_compliance_dict_str)) + media_compliance_code = '' media_type = '' @@ -654,24 +666,26 @@ def get_media_settings_key(physical_port, transceiver_dict): def get_media_val_str_from_dict(media_dict): + LANE_STR = 'lane' + LANE_SEPARATOR = ',' + media_str = '' - lane_str = 'lane' - lane_separator = ',' tmp_dict = {} for keys in media_dict: - lane_num = int(keys.strip()[len(lane_str):]) + lane_num = int(keys.strip()[len(LANE_STR):]) tmp_dict[lane_num] = media_dict[keys] for key in range(0, len(tmp_dict)): media_str += tmp_dict[key] if key != list(tmp_dict.keys())[-1]: - media_str += lane_separator + media_str += LANE_SEPARATOR return media_str def get_media_val_str(num_logical_ports, lane_dict, logical_idx): - lane_str = "lane" + LANE_STR = 'lane' + logical_media_dict = {} num_lanes_on_port = len(lane_dict) @@ -685,8 +699,8 @@ def get_media_val_str(num_logical_ports, lane_dict, logical_idx): for lane_idx in range(start_lane, start_lane + num_lanes_per_logical_port): - lane_idx_str = lane_str + str(lane_idx) - logical_lane_idx_str = lane_str + str(lane_idx - start_lane) + lane_idx_str = LANE_STR + str(lane_idx) + logical_lane_idx_str = LANE_STR + str(lane_idx - start_lane) logical_media_dict[logical_lane_idx_str] = lane_dict[lane_idx_str] media_val_str = get_media_val_str_from_dict(logical_media_dict) From 0bd9f698f7550f498af54a52199cb5ad5a45e306 Mon Sep 17 00:00:00 2001 From: noaOrMlnx <58519608+noaOrMlnx@users.noreply.github.com> Date: Wed, 7 Apr 2021 19:59:27 +0300 Subject: [PATCH 06/11] [thermalctld] Fix 'NameError("name 'chassis' is not defined")' error in log (#170) Changed the chassis definition to be a part of the class do the chassis will be found in run() function Without the change, an error appears in syslog: `ERR pmon#thermalctld: Caught exception while running thermal policy - NameError("name 'chassis' is not defined")` --- sonic-thermalctld/scripts/thermalctld | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sonic-thermalctld/scripts/thermalctld b/sonic-thermalctld/scripts/thermalctld index bf2285558..7dbc3d99f 100644 --- a/sonic-thermalctld/scripts/thermalctld +++ b/sonic-thermalctld/scripts/thermalctld @@ -745,18 +745,18 @@ class ThermalControlDaemon(daemon_base.DaemonBase): self.wait_time = self.INTERVAL - chassis = sonic_platform.platform.Platform().get_chassis() + self.chassis = sonic_platform.platform.Platform().get_chassis() - self.thermal_monitor = ThermalMonitor(chassis) + self.thermal_monitor = ThermalMonitor(self.chassis) self.thermal_monitor.task_run() self.thermal_manager = None try: - self.thermal_manager = chassis.get_thermal_manager() + self.thermal_manager = self.chassis.get_thermal_manager() if self.thermal_manager: self.thermal_manager.initialize() self.thermal_manager.load(ThermalControlDaemon.POLICY_FILE) - self.thermal_manager.init_thermal_algorithm(chassis) + self.thermal_manager.init_thermal_algorithm(self.chassis) except NotImplementedError: self.log_warning('Thermal manager is not supported on this platform') except Exception as e: @@ -810,7 +810,7 @@ class ThermalControlDaemon(daemon_base.DaemonBase): begin = time.time() try: if self.thermal_manager: - self.thermal_manager.run_policy(chassis) + self.thermal_manager.run_policy(self.chassis) except Exception as e: self.log_error('Caught exception while running thermal policy - {}'.format(repr(e))) From 5b6d9c062dfeda6afe5cb7081f4a4a3f7c1b3d52 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Thu, 8 Apr 2021 11:26:19 -0700 Subject: [PATCH 07/11] [syseepromd] Add unit tests; Refactor to allow for greater unit test coverage (#156) - Refactor syseepromd to eliminate infinite loops to allow for increased unit test coverage - Refactor signal handler and ensure daemon always exits with non-zero exit code so that supervisor will restart it - Add unit tests Unit test coverage increases from 0% to 90% --- sonic-syseepromd/pytest.ini | 2 + sonic-syseepromd/scripts/syseepromd | 187 ++++++++------- sonic-syseepromd/setup.cfg | 2 + sonic-syseepromd/setup.py | 6 + sonic-syseepromd/tests/__init__.py | 0 .../mocked_libs/sonic_platform/__init__.py | 6 + .../mocked_libs/sonic_platform/chassis.py | 21 ++ .../mocked_libs/sonic_platform/platform.py | 12 + .../tests/mocked_libs/swsscommon/__init__.py | 5 + .../mocked_libs/swsscommon/swsscommon.py | 51 ++++ sonic-syseepromd/tests/test_syseepromd.py | 221 ++++++++++++++++++ 11 files changed, 418 insertions(+), 95 deletions(-) create mode 100644 sonic-syseepromd/pytest.ini create mode 100644 sonic-syseepromd/setup.cfg create mode 100644 sonic-syseepromd/tests/__init__.py create mode 100644 sonic-syseepromd/tests/mocked_libs/sonic_platform/__init__.py create mode 100644 sonic-syseepromd/tests/mocked_libs/sonic_platform/chassis.py create mode 100644 sonic-syseepromd/tests/mocked_libs/sonic_platform/platform.py create mode 100644 sonic-syseepromd/tests/mocked_libs/swsscommon/__init__.py create mode 100644 sonic-syseepromd/tests/mocked_libs/swsscommon/swsscommon.py create mode 100644 sonic-syseepromd/tests/test_syseepromd.py diff --git a/sonic-syseepromd/pytest.ini b/sonic-syseepromd/pytest.ini new file mode 100644 index 000000000..d90ee9ed9 --- /dev/null +++ b/sonic-syseepromd/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +addopts = --cov=scripts --cov-report html --cov-report term --cov-report xml --junitxml=test-results.xml -vv diff --git a/sonic-syseepromd/scripts/syseepromd b/sonic-syseepromd/scripts/syseepromd index 8300f576f..3ccf9a077 100644 --- a/sonic-syseepromd/scripts/syseepromd +++ b/sonic-syseepromd/scripts/syseepromd @@ -8,83 +8,100 @@ With this daemon, show syseeprom CLI will be able to get data from state DB instead of access hw or cache. ''' -try: - import signal - import sys - import threading +import signal +import sys +import threading - from sonic_py_common import daemon_base - from swsscommon import swsscommon -except ImportError as e: - raise ImportError(str(e) + " - required module not found") +from sonic_py_common import daemon_base +from swsscommon import swsscommon + + +# TODO: Once we no longer support Python 2, we can eliminate this and get the +# name using the 'name' field (e.g., `signal.SIGINT.name`) starting with Python 3.5 +SIGNALS_TO_NAMES_DICT = dict((getattr(signal, n), n) + for n in dir(signal) if n.startswith('SIG') and '_' not in n) PLATFORM_SPECIFIC_MODULE_NAME = 'eeprom' PLATFORM_SPECIFIC_CLASS_NAME = 'board' EEPROM_INFO_UPDATE_PERIOD_SECS = 60 -POST_EEPROM_SUCCESS = 0 +ERR_NONE = 0 ERR_PLATFORM_NOT_SUPPORT = 1 ERR_FAILED_EEPROM = 2 ERR_FAILED_UPDATE_DB = 3 ERR_INVALID_PARAMETER = 4 -ERR_EEPROMUTIL_LOAD = 5 +ERR_EEPROM_LOAD = 5 EEPROM_TABLE_NAME = 'EEPROM_INFO' + SYSLOG_IDENTIFIER = 'syseepromd' +exit_code = 0 + class DaemonSyseeprom(daemon_base.DaemonBase): - def __init__(self, log_identifier): - super(DaemonSyseeprom, self).__init__(log_identifier) + def __init__(self): + super(DaemonSyseeprom, self).__init__(SYSLOG_IDENTIFIER) + + # Set minimum logging level to INFO + self.set_min_log_priority_info() self.stop_event = threading.Event() self.eeprom = None + self.eeprom_tbl = None - state_db = daemon_base.db_connect("STATE_DB") - self.eeprom_tbl = swsscommon.Table(state_db, EEPROM_TABLE_NAME) - self.eepromtbl_keys = [] + # First, try to load the new platform API + try: + import sonic_platform + self.eeprom = sonic_platform.platform.Platform().get_chassis().get_eeprom() + except Exception as e: + self.log_warning( + "Failed to load platform-specific eeprom from sonic_platform package due to {}. Trying deprecated plugin method ...".format(repr(e))) - def _wrapper_read_eeprom(self): - if self.eeprom is not None: + # If we didn't successfully load the class from the sonic_platform package, try loading the old plugin try: - return self.eeprom.read_eeprom() - except (NotImplementedError, IOError): - pass + self.eeprom = self.load_platform_util(PLATFORM_SPECIFIC_MODULE_NAME, PLATFORM_SPECIFIC_CLASS_NAME) + except Exception as e: + self.log_error("Failed to load platform-specific eeprom from deprecated plugin: {}".format(repr(e))) - try: - return self.eeprom.read_eeprom() - except IOError: - pass + if not self.eeprom: + sys.exit(ERR_EEPROM_LOAD) - def _wrapper_update_eeprom_db(self, eeprom): - if self.eeprom is not None: - try: - return self.eeprom.update_eeprom_db(eeprom) - except NotImplementedError: - pass + # Connect to STATE_DB + state_db = daemon_base.db_connect("STATE_DB") + self.eeprom_tbl = swsscommon.Table(state_db, EEPROM_TABLE_NAME) + self.eepromtbl_keys = [] - return self.eeprom.update_eeprom_db(eeprom) + # Post system EEPROM info to state DB once at start-up + rc = self.post_eeprom_to_db() + if rc != ERR_NONE: + self.log_error("Failed to post system EEPROM info to database") + + def __del__(self): + # Delete all the information from DB + self.clear_db() def post_eeprom_to_db(self): - eeprom = self._wrapper_read_eeprom() - if eeprom is None: - self.log_error("Failed to read eeprom") + eeprom_data = self.eeprom.read_eeprom() + if eeprom_data is None: + self.log_error("Failed to read EEPROM") return ERR_FAILED_EEPROM - err = self._wrapper_update_eeprom_db(eeprom) + err = self.eeprom.update_eeprom_db(eeprom_data) if err: - self.log_error("Failed to update eeprom info to database") + self.log_error("Failed to update EEPROM info in database") return ERR_FAILED_UPDATE_DB self.eepromtbl_keys = self.eeprom_tbl.getKeys() - return POST_EEPROM_SUCCESS + return ERR_NONE def clear_db(self): - keys = self.eeprom_tbl.getKeys() - for key in keys: - self.eeprom_tbl._del(key) + if self.eeprom_tbl: + keys = self.eeprom_tbl.getKeys() + for key in keys: + self.eeprom_tbl._del(key) def detect_eeprom_table_integrity(self): keys = self.eeprom_tbl.getKeys() @@ -98,65 +115,37 @@ class DaemonSyseeprom(daemon_base.DaemonBase): return True - # Signal handler + # Override signal handler from DaemonBase def signal_handler(self, sig, frame): - if sig == signal.SIGHUP: - self.log_info("Caught SIGHUP - ignoring...") - elif sig == signal.SIGINT: - self.log_info("Caught SIGINT - exiting...") - self.stop_event.set() - elif sig == signal.SIGTERM: - self.log_info("Caught SIGTERM - exiting...") + FATAL_SIGNALS = [signal.SIGINT, signal.SIGTERM] + NONFATAL_SIGNALS = [signal.SIGHUP] + + global exit_code + + if sig in FATAL_SIGNALS: + self.log_info("Caught signal '{}' - exiting...".format(SIGNALS_TO_NAMES_DICT[sig])) + exit_code = 128 + sig # Make sure we exit with a non-zero code so that supervisor will try to restart us self.stop_event.set() + elif sig in NONFATAL_SIGNALS: + self.log_info("Caught signal '{}' - ignoring...".format(SIGNALS_TO_NAMES_DICT[sig])) else: - self.log_warning("Caught unhandled signal '" + sig + "'") + self.log_warning("Caught unhandled signal '{}' - ignoring...".format(SIGNALS_TO_NAMES_DICT[sig])) - # Run daemon + # Main daemon logic def run(self): - self.log_info("Starting up...") - - # First, try to load the new platform API - try: - import sonic_platform - self.chassis = sonic_platform.platform.Platform().get_chassis() - self.eeprom = self.chassis.get_eeprom() - except Exception as e: - self.log_warning("Failed to load data from eeprom using sonic_platform package due to {}, retrying using deprecated plugin method".format(repr(e))) - - # If we didn't successfully load the class from the sonic_platform package, try loading the old plugin - if not self.eeprom: - try: - self.eeprom = self.load_platform_util(PLATFORM_SPECIFIC_MODULE_NAME, PLATFORM_SPECIFIC_CLASS_NAME) - except Exception as e: - self.log_error("Failed to load platform-specific eeprom implementation: {}".format(repr(e))) - - if not self.eeprom: - sys.exit(ERR_EEPROMUTIL_LOAD) - - # Connect to STATE_DB and post syseeprom info to state DB - rc = self.post_eeprom_to_db() - if rc != POST_EEPROM_SUCCESS: - self.log_error("Failed to post eeprom to database") - - # Start main loop - self.log_info("Start daemon main loop") - - while not self.stop_event.wait(EEPROM_INFO_UPDATE_PERIOD_SECS): - rc = self.detect_eeprom_table_integrity() - if not rc: - self.log_info("sys eeprom table was changed, need update") - self.clear_db() - rcs = self.post_eeprom_to_db() - if rcs != POST_EEPROM_SUCCESS: - self.log_error("Failed to post eeprom to database") - continue - - self.log_info("Stop daemon main loop") + if self.stop_event.wait(EEPROM_INFO_UPDATE_PERIOD_SECS): + # We received a fatal signal + return False - # Delete all the information from DB and then exit - self.clear_db() + rc = self.detect_eeprom_table_integrity() + if not rc: + self.log_info("System EEPROM table was changed, needs update") + self.clear_db() + rcs = self.post_eeprom_to_db() + if rcs != ERR_NONE: + self.log_error("Failed to post EEPROM to database") - self.log_info("Shutting down...") + return True # # Main ========================================================================= @@ -164,9 +153,17 @@ class DaemonSyseeprom(daemon_base.DaemonBase): def main(): - syseepromd = DaemonSyseeprom(SYSLOG_IDENTIFIER) - syseepromd.run() + syseepromd = DaemonSyseeprom() + + syseepromd.log_info("Starting up...") + + while syseepromd.run(): + pass + + syseepromd.log_info("Shutting down...") + + return exit_code if __name__ == '__main__': - main() + sys.exit(main()) diff --git a/sonic-syseepromd/setup.cfg b/sonic-syseepromd/setup.cfg new file mode 100644 index 000000000..b7e478982 --- /dev/null +++ b/sonic-syseepromd/setup.cfg @@ -0,0 +1,2 @@ +[aliases] +test=pytest diff --git a/sonic-syseepromd/setup.py b/sonic-syseepromd/setup.py index 1f485b2fb..4232e3d88 100644 --- a/sonic-syseepromd/setup.py +++ b/sonic-syseepromd/setup.py @@ -16,6 +16,12 @@ setup_requires=[ 'wheel' ], + tests_require=[ + 'mock>=2.0.0; python_version < "3.3"', + 'pytest', + 'pytest-cov', + 'sonic_platform_common' + ], classifiers=[ 'Development Status :: 4 - Beta', 'Environment :: No Input/Output (Daemon)', diff --git a/sonic-syseepromd/tests/__init__.py b/sonic-syseepromd/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/sonic-syseepromd/tests/mocked_libs/sonic_platform/__init__.py b/sonic-syseepromd/tests/mocked_libs/sonic_platform/__init__.py new file mode 100644 index 000000000..e491d5b52 --- /dev/null +++ b/sonic-syseepromd/tests/mocked_libs/sonic_platform/__init__.py @@ -0,0 +1,6 @@ +""" + Mock implementation of sonic_platform package for unit testing +""" + +from . import chassis +from . import platform diff --git a/sonic-syseepromd/tests/mocked_libs/sonic_platform/chassis.py b/sonic-syseepromd/tests/mocked_libs/sonic_platform/chassis.py new file mode 100644 index 000000000..918e01243 --- /dev/null +++ b/sonic-syseepromd/tests/mocked_libs/sonic_platform/chassis.py @@ -0,0 +1,21 @@ +""" + Mock implementation of sonic_platform package for unit testing +""" + +# TODO: Clean this up once we no longer need to support Python 2 +import sys +if sys.version_info.major == 3: + from unittest import mock +else: + import mock + +from sonic_platform_base.chassis_base import ChassisBase + + +class Chassis(ChassisBase): + def __init__(self): + ChassisBase.__init__(self) + self.eeprom = mock.MagicMock() + + def get_eeprom(self): + return self.eeprom diff --git a/sonic-syseepromd/tests/mocked_libs/sonic_platform/platform.py b/sonic-syseepromd/tests/mocked_libs/sonic_platform/platform.py new file mode 100644 index 000000000..a1b61e13e --- /dev/null +++ b/sonic-syseepromd/tests/mocked_libs/sonic_platform/platform.py @@ -0,0 +1,12 @@ +""" + Mock implementation of sonic_platform package for unit testing +""" + +from sonic_platform_base.platform_base import PlatformBase +from sonic_platform.chassis import Chassis + + +class Platform(PlatformBase): + def __init__(self): + PlatformBase.__init__(self) + self._chassis = Chassis() diff --git a/sonic-syseepromd/tests/mocked_libs/swsscommon/__init__.py b/sonic-syseepromd/tests/mocked_libs/swsscommon/__init__.py new file mode 100644 index 000000000..012af621e --- /dev/null +++ b/sonic-syseepromd/tests/mocked_libs/swsscommon/__init__.py @@ -0,0 +1,5 @@ +''' + Mock implementation of swsscommon package for unit testing +''' + +from . import swsscommon diff --git a/sonic-syseepromd/tests/mocked_libs/swsscommon/swsscommon.py b/sonic-syseepromd/tests/mocked_libs/swsscommon/swsscommon.py new file mode 100644 index 000000000..8a0a87692 --- /dev/null +++ b/sonic-syseepromd/tests/mocked_libs/swsscommon/swsscommon.py @@ -0,0 +1,51 @@ +''' + Mock implementation of swsscommon package for unit testing +''' + +STATE_DB = '' + + +class Table: + def __init__(self, db, table_name): + self.table_name = table_name + self.mock_dict = {} + + def _del(self, key): + del self.mock_dict[key] + pass + + def set(self, key, fvs): + self.mock_dict[key] = fvs.fv_dict + pass + + def get(self, key): + if key in self.mock_dict: + return self.mock_dict[key] + return None + + +class FieldValuePairs: + fv_dict = {} + + def __init__(self, tuple_list): + if isinstance(tuple_list, list) and isinstance(tuple_list[0], tuple): + self.fv_dict = dict(tuple_list) + + def __setitem__(self, key, kv_tuple): + self.fv_dict[kv_tuple[0]] = kv_tuple[1] + + def __getitem__(self, key): + return self.fv_dict[key] + + def __eq__(self, other): + if not isinstance(other, FieldValuePairs): + # don't attempt to compare against unrelated types + return NotImplemented + + return self.fv_dict == other.fv_dict + + def __repr__(self): + return repr(self.fv_dict) + + def __str__(self): + return repr(self.fv_dict) diff --git a/sonic-syseepromd/tests/test_syseepromd.py b/sonic-syseepromd/tests/test_syseepromd.py new file mode 100644 index 000000000..e25b94ce3 --- /dev/null +++ b/sonic-syseepromd/tests/test_syseepromd.py @@ -0,0 +1,221 @@ +import os +import sys +from imp import load_source # Replace with importlib once we no longer need to support Python 2 + +import pytest + +# TODO: Clean this up once we no longer need to support Python 2 +if sys.version_info.major == 3: + from unittest import mock +else: + import mock +from sonic_py_common import daemon_base + +SYSLOG_IDENTIFIER = 'syseepromd_test' +NOT_AVAILABLE = 'N/A' + +daemon_base.db_connect = mock.MagicMock() + +tests_path = os.path.dirname(os.path.abspath(__file__)) + +# Add mocked_libs path so that the file under test can load mocked modules from there +mocked_libs_path = os.path.join(tests_path, 'mocked_libs') +sys.path.insert(0, mocked_libs_path) + +# Add path to the file under test so that we can load it +modules_path = os.path.dirname(tests_path) +scripts_path = os.path.join(modules_path, 'scripts') +sys.path.insert(0, modules_path) + +load_source('syseepromd', os.path.join(scripts_path, 'syseepromd')) +import syseepromd + + +def test_post_eeprom_to_db_eeprom_read_fail(): + daemon_syseepromd = syseepromd.DaemonSyseeprom() + daemon_syseepromd.eeprom.read_eeprom = mock.MagicMock(return_value=None) + daemon_syseepromd.eeprom_tbl = mock.MagicMock() + daemon_syseepromd.log_error = mock.MagicMock() + + ret = daemon_syseepromd.post_eeprom_to_db() + assert ret == syseepromd.ERR_FAILED_EEPROM + assert daemon_syseepromd.log_error.call_count == 1 + daemon_syseepromd.log_error.assert_called_with('Failed to read EEPROM') + assert daemon_syseepromd.eeprom_tbl.getKeys.call_count == 0 + + +def test_post_eeprom_to_db_update_fail(): + daemon_syseepromd = syseepromd.DaemonSyseeprom() + daemon_syseepromd.eeprom.update_eeprom_db = mock.MagicMock(return_value=1) + daemon_syseepromd.eeprom_tbl = mock.MagicMock() + daemon_syseepromd.log_error = mock.MagicMock() + + ret = daemon_syseepromd.post_eeprom_to_db() + assert ret == syseepromd.ERR_FAILED_UPDATE_DB + assert daemon_syseepromd.log_error.call_count == 1 + daemon_syseepromd.log_error.assert_called_with('Failed to update EEPROM info in database') + assert daemon_syseepromd.eeprom_tbl.getKeys.call_count == 0 + + +def test_post_eeprom_to_db_ok(): + daemon_syseepromd = syseepromd.DaemonSyseeprom() + daemon_syseepromd.eeprom.update_eeprom_db = mock.MagicMock(return_value=0) + daemon_syseepromd.eeprom_tbl = mock.MagicMock() + daemon_syseepromd.log_error = mock.MagicMock() + + ret = daemon_syseepromd.post_eeprom_to_db() + assert ret == syseepromd.ERR_NONE + assert daemon_syseepromd.log_error.call_count == 0 + assert daemon_syseepromd.eeprom_tbl.getKeys.call_count == 1 + + +def test_clear_db(): + daemon_syseepromd = syseepromd.DaemonSyseeprom() + daemon_syseepromd.eeprom_tbl.getKeys = mock.MagicMock(return_value=['key1', 'key2']) + daemon_syseepromd.eeprom_tbl._del = mock.MagicMock() + + daemon_syseepromd.clear_db() + assert daemon_syseepromd.eeprom_tbl.getKeys.call_count == 1 + assert daemon_syseepromd.eeprom_tbl._del.call_count == 2 + + +def test_detect_eeprom_table_integrity(): + daemon_syseepromd = syseepromd.DaemonSyseeprom() + + # Test entries as expected + daemon_syseepromd.eeprom_tbl.getKeys = mock.MagicMock(return_value=['key1', 'key2']) + daemon_syseepromd.eepromtbl_keys = ['key1', 'key2'] + ret = daemon_syseepromd.detect_eeprom_table_integrity() + assert ret == True + + # Test differing amounts of entries + daemon_syseepromd.eeprom_tbl.getKeys = mock.MagicMock(return_value=['key1', 'key2']) + daemon_syseepromd.eepromtbl_keys = ['key1'] + ret = daemon_syseepromd.detect_eeprom_table_integrity() + assert ret == False + + # Test same amount of entries, but with different keys + daemon_syseepromd.eeprom_tbl.getKeys = mock.MagicMock(return_value=['key1', 'key2']) + daemon_syseepromd.eepromtbl_keys = ['key1', 'key3'] + ret = daemon_syseepromd.detect_eeprom_table_integrity() + assert ret == False + + +def test_signal_handler(): + daemon_syseepromd = syseepromd.DaemonSyseeprom() + daemon_syseepromd.stop_event.set = mock.MagicMock() + daemon_syseepromd.log_info = mock.MagicMock() + daemon_syseepromd.log_warning = mock.MagicMock() + + # Test SIGHUP + daemon_syseepromd.signal_handler(syseepromd.signal.SIGHUP, None) + assert daemon_syseepromd.log_info.call_count == 1 + daemon_syseepromd.log_info.assert_called_with("Caught signal 'SIGHUP' - ignoring...") + assert daemon_syseepromd.log_warning.call_count == 0 + assert daemon_syseepromd.stop_event.set.call_count == 0 + assert syseepromd.exit_code == 0 + + # Reset + daemon_syseepromd.log_info.reset_mock() + daemon_syseepromd.log_warning.reset_mock() + daemon_syseepromd.stop_event.set.reset_mock() + + # Test SIGINT + test_signal = syseepromd.signal.SIGINT + daemon_syseepromd.signal_handler(test_signal, None) + assert daemon_syseepromd.log_info.call_count == 1 + daemon_syseepromd.log_info.assert_called_with("Caught signal 'SIGINT' - exiting...") + assert daemon_syseepromd.log_warning.call_count == 0 + assert daemon_syseepromd.stop_event.set.call_count == 1 + assert syseepromd.exit_code == (128 + test_signal) + + # Reset + daemon_syseepromd.log_info.reset_mock() + daemon_syseepromd.log_warning.reset_mock() + daemon_syseepromd.stop_event.set.reset_mock() + + # Test SIGTERM + test_signal = syseepromd.signal.SIGTERM + daemon_syseepromd.signal_handler(test_signal, None) + assert daemon_syseepromd.log_info.call_count == 1 + daemon_syseepromd.log_info.assert_called_with("Caught signal 'SIGTERM' - exiting...") + assert daemon_syseepromd.log_warning.call_count == 0 + assert daemon_syseepromd.stop_event.set.call_count == 1 + assert syseepromd.exit_code == (128 + test_signal) + + # Reset + daemon_syseepromd.log_info.reset_mock() + daemon_syseepromd.log_warning.reset_mock() + daemon_syseepromd.stop_event.set.reset_mock() + syseepromd.exit_code = 0 + + # Test an unhandled signal + daemon_syseepromd.signal_handler(syseepromd.signal.SIGUSR1, None) + assert daemon_syseepromd.log_warning.call_count == 1 + daemon_syseepromd.log_warning.assert_called_with("Caught unhandled signal 'SIGUSR1' - ignoring...") + assert daemon_syseepromd.log_info.call_count == 0 + assert daemon_syseepromd.stop_event.set.call_count == 0 + assert syseepromd.exit_code == 0 + + +@mock.patch('syseepromd.EEPROM_INFO_UPDATE_PERIOD_SECS', 1) +def test_run(): + daemon_syseepromd = syseepromd.DaemonSyseeprom() + daemon_syseepromd.clear_db = mock.MagicMock() + daemon_syseepromd.log_info = mock.MagicMock() + daemon_syseepromd.log_error = mock.MagicMock() + daemon_syseepromd.post_eeprom_to_db = mock.MagicMock(return_value=syseepromd.ERR_NONE) + + # Test no change to EEPROM data + daemon_syseepromd.detect_eeprom_table_integrity = mock.MagicMock(return_value=True) + + ret = daemon_syseepromd.run() + assert ret == True + assert daemon_syseepromd.detect_eeprom_table_integrity.call_count == 1 + assert daemon_syseepromd.log_info.call_count == 0 + assert daemon_syseepromd.log_error.call_count == 0 + assert daemon_syseepromd.clear_db.call_count == 0 + assert daemon_syseepromd.post_eeprom_to_db.call_count == 0 + + # Reset mocks + daemon_syseepromd.detect_eeprom_table_integrity.reset_mock() + + # Test EEPROM data has changed, update succeeds + daemon_syseepromd.detect_eeprom_table_integrity = mock.MagicMock(return_value=False) + + ret = daemon_syseepromd.run() + assert ret == True + assert daemon_syseepromd.detect_eeprom_table_integrity.call_count == 1 + assert daemon_syseepromd.log_info.call_count == 1 + daemon_syseepromd.log_info.assert_called_with('System EEPROM table was changed, needs update') + assert daemon_syseepromd.clear_db.call_count == 1 + assert daemon_syseepromd.post_eeprom_to_db.call_count == 1 + assert daemon_syseepromd.log_error.call_count == 0 + + # Reset mocks + daemon_syseepromd.detect_eeprom_table_integrity.reset_mock() + daemon_syseepromd.log_info.reset_mock() + daemon_syseepromd.clear_db.reset_mock() + daemon_syseepromd.post_eeprom_to_db.reset_mock() + + # Test EEPROM data has changed, update fails + daemon_syseepromd.detect_eeprom_table_integrity = mock.MagicMock(return_value=False) + daemon_syseepromd.post_eeprom_to_db = mock.MagicMock(return_value=syseepromd.ERR_FAILED_UPDATE_DB) + + ret = daemon_syseepromd.run() + assert ret == True + assert daemon_syseepromd.detect_eeprom_table_integrity.call_count == 1 + assert daemon_syseepromd.log_info.call_count == 1 + daemon_syseepromd.log_info.assert_called_with('System EEPROM table was changed, needs update') + assert daemon_syseepromd.clear_db.call_count == 1 + assert daemon_syseepromd.post_eeprom_to_db.call_count == 1 + assert daemon_syseepromd.log_error.call_count == 1 + daemon_syseepromd.log_error.assert_called_with('Failed to post EEPROM to database') + + +@mock.patch('syseepromd.DaemonSyseeprom.run') +def test_main(mock_run): + mock_run.return_value = False + + syseepromd.main() + assert mock_run.call_count == 1 From eff5c1c83035a6f2bf07ba1a22c91db67e325a14 Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com> Date: Fri, 9 Apr 2021 08:56:07 +0800 Subject: [PATCH 08/11] [thermalctld] No need exit thermalcltd when loading invalid policy file (#172) Currently, when thermalctld loading an invalid policy file, it catches the exception and call sys.exit. However, there is a sub process created and sys.exit cause the daemon hang forever. Actually, it is not necessary to exit thermalctld here because: 1. Other than running thermal policy, there is a sub process which monitoring thermal status. So even if there is an invalid policy file, thermal monitoring can still work. 2. Even if we exit here, supervisord will restart thermalctld and it fall into the same exception again and again. --- sonic-thermalctld/scripts/thermalctld | 2 -- 1 file changed, 2 deletions(-) diff --git a/sonic-thermalctld/scripts/thermalctld b/sonic-thermalctld/scripts/thermalctld index 7dbc3d99f..5c1449984 100644 --- a/sonic-thermalctld/scripts/thermalctld +++ b/sonic-thermalctld/scripts/thermalctld @@ -29,7 +29,6 @@ PHYSICAL_ENTITY_INFO_TABLE = 'PHYSICAL_ENTITY_INFO' INVALID_SLOT = -1 ERR_UNKNOWN = 1 -ERR_INIT_FAILED = 2 # Thermal control daemon is designed to never exit, it must always # return non-zero exit code when exiting and so that supervisord will @@ -761,7 +760,6 @@ class ThermalControlDaemon(daemon_base.DaemonBase): self.log_warning('Thermal manager is not supported on this platform') except Exception as e: self.log_error('Caught exception while initializing thermal manager - {}'.format(repr(e))) - sys.exit(ERR_INIT_FAILED) def deinit(self): """ From be7f4e1c919ed4812c396b95118611a9f3a1a44a Mon Sep 17 00:00:00 2001 From: vganesan-nokia <67648637+vganesan-nokia@users.noreply.github.com> Date: Fri, 9 Apr 2021 19:03:50 -0400 Subject: [PATCH 09/11] [voqinband]Support for inband port as regular port (#145) Signed-off-by: vedganes Inband port is avaialable in PORT table. But regular port handlings are not applicable for Inband port. Changes in this PR are to avoid regular port handling on Inband port for ledd script. --- sonic-ledd/scripts/ledd | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sonic-ledd/scripts/ledd b/sonic-ledd/scripts/ledd index 87cea6287..88b4ae5cc 100644 --- a/sonic-ledd/scripts/ledd +++ b/sonic-ledd/scripts/ledd @@ -10,7 +10,7 @@ import sys from sonic_py_common import daemon_base from sonic_py_common import multi_asic -from sonic_py_common.interface import backplane_prefix +from sonic_py_common.interface import backplane_prefix, inband_prefix from swsscommon import swsscommon #============================= Constants ============================= @@ -96,7 +96,7 @@ class DaemonLedd(daemon_base.DaemonBase): fvp_dict = dict(fvp) if op == "SET" and "oper_status" in fvp_dict: - if not key.startswith(backplane_prefix()): + if not key.startswith(backplane_prefix()) and not key.startswith(inband_prefix()): self.led_control.port_link_state_change(key, fvp_dict["oper_status"]) else: return 4 From c4d47900571f68b1d854030c8d13e291850e4043 Mon Sep 17 00:00:00 2001 From: vdahiya12 <67608553+vdahiya12@users.noreply.github.com> Date: Fri, 9 Apr 2021 17:02:24 -0700 Subject: [PATCH 10/11] [xcvrd] refactor Y-Cable firmware information to conform with all vendors (#171) This PR refactors the firmware information MUX_CABLE_INFO output to return the following fields for each target. { "version_active": "", "version_inactive": "", "version_next": "", } So by calling this for all the 3 MCU's TOR1, TOR2 and NIC we can get the below result. which would be stored in state db table MUX_CABLE_INFO { "version_nic_active": "0.6MS", "version_nic_inactive": "0.5MS", "version_nic_next": "0.6MS", "version_self_active": "0.5MS", "version_self_inactive": "0.6MS", "version_self_next": "0.6MS", "version_peer_active": "0.6MS", "version_peer_inactive": "0.6MS", "version_peer_next": "0.6MS", } Signed-off-by: vaibhav-dahiya --- sonic-xcvrd/tests/test_xcvrd.py | 39 +++------- .../xcvrd/xcvrd_utilities/y_cable_helper.py | 74 +++++-------------- 2 files changed, 29 insertions(+), 84 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 620ca3a96..a4d64b68d 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -242,36 +242,15 @@ def test_init_port_sfp_status_tbl(self): 'internal_voltage': '3.3', 'nic_temperature': '20', 'nic_voltage': '2.7', - 'build_slot1_nic': 'MS', - 'build_slot2_nic': 'MS', - 'version_slot1_nic': '1.7', - 'version_slot2_nic': '1.7', - 'run_slot1_nic': 'True', - 'run_slot2_nic': 'False', - 'commit_slot1_nic': 'True', - 'commit_slot2_nic': 'False', - 'empty_slot1_nic': 'True', - 'empty_slot2_nic': 'False', - 'build_slot1_tor_self': 'MS', - 'build_slot2_tor_self': 'MS', - 'version_slot1_tor_self': '1.7', - 'version_slot2_tor_self': '1.7', - 'run_slot1_tor_self': 'True', - 'run_slot2_tor_self': 'False', - 'commit_slot1_tor_self': 'True', - 'commit_slot2_tor_self': 'False', - 'empty_slot1_tor_self': 'True', - 'empty_slot2_tor_self': 'False', - 'build_slot1_tor_peer': 'MS', - 'build_slot2_tor_peer': 'MS', - 'version_slot1_tor_peer': '1.7', - 'version_slot2_tor_peer': '1.7', - 'run_slot1_tor_peer': 'True', - 'run_slot2_tor_peer': 'False', - 'commit_slot1_tor_peer': 'True', - 'commit_slot2_tor_peer': 'False', - 'empty_slot1_tor_peer': 'True', - 'empty_slot2_tor_peer': 'False'})) + 'version_nic_active': '1.6MS', + 'version_nic_inactive': '1.7MS', + 'version_nic_next': '1.7MS', + 'version_self_active': '1.6MS', + 'version_self_inactive': '1.7MS', + 'version_self_next': '1.7MS', + 'version_peer_active': '1.6MS', + 'version_peer_inactive': '1.7MS', + 'version_peer_next': '1.7MS'})) def test_post_port_mux_info_to_db(self): logical_port_name = "Ethernet0" mux_tbl = Table("STATE_DB", y_cable_helper.MUX_CABLE_INFO_TABLE) diff --git a/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py b/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py index bd55bb64f..a7eb9e355 100644 --- a/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py +++ b/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py @@ -542,27 +542,14 @@ def get_firmware_dict(physical_port, target, side, mux_info_dict): result = y_cable.get_firmware_version(physical_port, target) if result is not None and isinstance(result, dict): - mux_info_dict[("build_slot1_{}".format(side))] = result.get("build_slot1", None) - mux_info_dict[("version_slot1_{}".format(side))] = result.get("version_slot1", None) - mux_info_dict[("build_slot2_{}".format(side))] = result.get("build_slot2", None) - mux_info_dict[("version_slot2_{}".format(side))] = result.get("version_slot2", None) - mux_info_dict[("run_slot1_{}".format(side))] = result.get("run_slot1", None) - mux_info_dict[("run_slot2_{}".format(side))] = result.get("run_slot2", None) - mux_info_dict[("commit_slot1_{}".format(side))] = result.get("commit_slot1", None) - mux_info_dict[("commit_slot2_{}".format(side))] = result.get("commit_slot2", None) - mux_info_dict[("empty_slot1_{}".format(side))] = result.get("empty_slot1", None) - mux_info_dict[("empty_slot2_{}".format(side))] = result.get("empty_slot2", None) + mux_info_dict[("version_{}_active".format(side))] = result.get("version_active", None) + mux_info_dict[("version_{}_inactive".format(side))] = result.get("version_inactive", None) + mux_info_dict[("version_{}_next".format(side))] = result.get("version_next", None) + else: - mux_info_dict[("build_slot1_{}".format(side))] = "N/A" - mux_info_dict[("version_slot1_{}".format(side))] = "N/A" - mux_info_dict[("build_slot2_{}".format(side))] = "N/A" - mux_info_dict[("version_slot2_{}".format(side))] = "N/A" - mux_info_dict[("run_slot1_{}".format(side))] = "N/A" - mux_info_dict[("run_slot2_{}".format(side))] = "N/A" - mux_info_dict[("commit_slot1_{}".format(side))] = "N/A" - mux_info_dict[("commit_slot2_{}".format(side))] = "N/A" - mux_info_dict[("empty_slot1_{}".format(side))] = "N/A" - mux_info_dict[("empty_slot2_{}".format(side))] = "N/A" + mux_info_dict[("version_{}_active".format(side))] = "N/A" + mux_info_dict[("version_{}_inactive".format(side))] = "N/A" + mux_info_dict[("version_{}_next".format(side))] = "N/A" def get_muxcable_info(physical_port, logical_port_name): @@ -715,11 +702,11 @@ def get_muxcable_info(physical_port, logical_port_name): get_firmware_dict(physical_port, 0, "nic", mux_info_dict) if read_side == 1: - get_firmware_dict(physical_port, 1, "tor_self", mux_info_dict) - get_firmware_dict(physical_port, 2, "tor_peer", mux_info_dict) + get_firmware_dict(physical_port, 1, "self", mux_info_dict) + get_firmware_dict(physical_port, 2, "peer", mux_info_dict) else: - get_firmware_dict(physical_port, 1, "tor_peer", mux_info_dict) - get_firmware_dict(physical_port, 2, "tor_self", mux_info_dict) + get_firmware_dict(physical_port, 1, "peer", mux_info_dict) + get_firmware_dict(physical_port, 2, "self", mux_info_dict) res = y_cable.get_internal_voltage_temp(physical_port) @@ -879,36 +866,15 @@ def post_port_mux_info_to_db(logical_port_name, table): ('internal_voltage', str(mux_info_dict["internal_voltage"])), ('nic_temperature', str(mux_info_dict["nic_temperature"])), ('nic_voltage', str(mux_info_dict["nic_voltage"])), - ('build_slot1_nic', str(mux_info_dict["build_slot1_nic"])), - ('build_slot2_nic', str(mux_info_dict["build_slot2_nic"])), - ('version_slot1_nic', str(mux_info_dict["version_slot1_nic"])), - ('version_slot2_nic', str(mux_info_dict["version_slot2_nic"])), - ('run_slot1_nic', str(mux_info_dict["run_slot1_nic"])), - ('run_slot2_nic', str(mux_info_dict["run_slot2_nic"])), - ('commit_slot1_nic', str(mux_info_dict["commit_slot1_nic"])), - ('commit_slot2_nic', str(mux_info_dict["commit_slot2_nic"])), - ('empty_slot1_nic', str(mux_info_dict["empty_slot1_nic"])), - ('empty_slot2_nic', str(mux_info_dict["empty_slot2_nic"])), - ('build_slot1_tor_self', str(mux_info_dict["build_slot1_tor_self"])), - ('build_slot2_tor_self', str(mux_info_dict["build_slot2_tor_self"])), - ('version_slot1_tor_self', str(mux_info_dict["version_slot1_tor_self"])), - ('version_slot2_tor_self', str(mux_info_dict["version_slot2_tor_self"])), - ('run_slot1_tor_self', str(mux_info_dict["run_slot1_tor_self"])), - ('run_slot2_tor_self', str(mux_info_dict["run_slot2_tor_self"])), - ('commit_slot1_tor_self', str(mux_info_dict["commit_slot1_tor_self"])), - ('commit_slot2_tor_self', str(mux_info_dict["commit_slot2_tor_self"])), - ('empty_slot1_tor_self', str(mux_info_dict["empty_slot1_tor_self"])), - ('empty_slot2_tor_self', str(mux_info_dict["empty_slot2_tor_self"])), - ('build_slot1_tor_peer', str(mux_info_dict["build_slot1_tor_peer"])), - ('build_slot2_tor_peer', str(mux_info_dict["build_slot2_tor_peer"])), - ('version_slot1_tor_peer', str(mux_info_dict["version_slot1_tor_peer"])), - ('version_slot2_tor_peer', str(mux_info_dict["version_slot2_tor_peer"])), - ('run_slot1_tor_peer', str(mux_info_dict["run_slot1_tor_peer"])), - ('run_slot2_tori_peer', str(mux_info_dict["run_slot2_tor_peer"])), - ('commit_slot1_tor_peer', str(mux_info_dict["commit_slot1_tor_peer"])), - ('commit_slot2_tor_peer', str(mux_info_dict["commit_slot2_tor_peer"])), - ('empty_slot1_tor_peer', str(mux_info_dict["empty_slot1_tor_peer"])), - ('empty_slot2_tor_peer', str(mux_info_dict["empty_slot2_tor_peer"])) + ('version_self_active', str(mux_info_dict["version_self_active"])), + ('version_self_inactive', str(mux_info_dict["version_self_inactive"])), + ('version_self_next', str(mux_info_dict["version_self_next"])), + ('version_peer_active', str(mux_info_dict["version_peer_active"])), + ('version_peer_inactive', str(mux_info_dict["version_peer_inactive"])), + ('version_peer_next', str(mux_info_dict["version_peer_next"])), + ('version_nic_active', str(mux_info_dict["version_nic_active"])), + ('version_nic_inactive', str(mux_info_dict["version_nic_inactive"])), + ('version_nic_next', str(mux_info_dict["version_nic_next"])) ]) table.set(logical_port_name, fvs) else: From 911601dfb17403f01bb2340f97b69ad57c2c2cb3 Mon Sep 17 00:00:00 2001 From: Tamer Ahmed Date: Thu, 15 Apr 2021 19:19:04 -0700 Subject: [PATCH 11/11] [muxcable] Fix Redis Selectable Processing (#173) Selectable object can return multiple fields, however current code processes only the first field and wait till the next selectable is triggered. This PR consumes all fields that were returned by a selectable object. This results in reducing processing time from order of seconds to order of milliseconds. signed-off-by: Tamer Ahmed --- .../xcvrd/xcvrd_utilities/y_cable_helper.py | 111 +++++++++--------- 1 file changed, 58 insertions(+), 53 deletions(-) diff --git a/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py b/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py index a7eb9e355..aacc264b3 100644 --- a/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py +++ b/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py @@ -1054,66 +1054,71 @@ def task_worker(self): namespace = redisSelectObj.getDbConnector().getNamespace() asic_index = multi_asic.get_asic_index_from_namespace(namespace) - (port, op, fvp) = status_tbl[asic_index].pop() - if fvp: - # This check might be redundant, to check, the presence of this Port in keys - # in logical_port_list but keep for now for coherency - # also skip checking in logical_port_list inside sfp_util - if port not in y_cable_tbl_keys[asic_index]: - continue - - fvp_dict = dict(fvp) - - if "state" in fvp_dict: - # got a state change - new_status = fvp_dict["state"] - (status, fvs) = y_cable_tbl[asic_index].get(port) - if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( - port, y_cable_tbl[asic_index])) + while True: + (port, op, fvp) = status_tbl[asic_index].pop() + if not port: + break + if fvp: + # This check might be redundant, to check, the presence of this Port in keys + # in logical_port_list but keep for now for coherency + # also skip checking in logical_port_list inside sfp_util + if port not in y_cable_tbl_keys[asic_index]: continue - mux_port_dict = dict(fvs) - old_status = mux_port_dict.get("state") - read_side = mux_port_dict.get("read_side") - # Now whatever is the state requested, toggle the mux appropriately - active_side = update_tor_active_side(read_side, new_status, port) - if active_side == -1: - helper_logger.log_warning("ERR: Got a change event for toggle but could not toggle the mux-direction for port {} state from {} to {}, writing unknown".format( - port, old_status, new_status)) - new_status = 'unknown' - - fvs_updated = swsscommon.FieldValuePairs([('state', new_status), - ('read_side', - read_side), - ('active_side', str(active_side))]) - y_cable_tbl[asic_index].set(port, fvs_updated) - helper_logger.log_info("Got a change event for toggle the mux-direction active side for port {} state from {} to {}".format( - port, old_status, new_status)) - else: - helper_logger.log_info("Got a change event on port {} of table {} that does not contain state".format( - port, swsscommon.APP_HW_MUX_CABLE_TABLE_NAME)) - - (port_m, op_m, fvp_m) = mux_cable_command_tbl[asic_index].pop() - if fvp_m: - - if port_m not in y_cable_tbl_keys[asic_index]: - continue - - fvp_dict = dict(fvp_m) - if "command" in fvp_dict: - # check if xcvrd got a probe command - probe_identifier = fvp_dict["command"] + fvp_dict = dict(fvp) - if probe_identifier == "probe": - (status, fv) = y_cable_tbl[asic_index].get(port_m) + if "state" in fvp_dict: + # got a state change + new_status = fvp_dict["state"] + (status, fvs) = y_cable_tbl[asic_index].get(port) if status is False: helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( - port_m, y_cable_tbl[asic_index])) + port, y_cable_tbl[asic_index])) continue - mux_port_dict = dict(fv) + mux_port_dict = dict(fvs) + old_status = mux_port_dict.get("state") read_side = mux_port_dict.get("read_side") - update_appdb_port_mux_cable_response_table(port_m, asic_index, appl_db, int(read_side)) + # Now whatever is the state requested, toggle the mux appropriately + active_side = update_tor_active_side(read_side, new_status, port) + if active_side == -1: + helper_logger.log_warning("ERR: Got a change event for toggle but could not toggle the mux-direction for port {} state from {} to {}, writing unknown".format( + port, old_status, new_status)) + new_status = 'unknown' + + fvs_updated = swsscommon.FieldValuePairs([('state', new_status), + ('read_side', read_side), + ('active_side', str(active_side))]) + y_cable_tbl[asic_index].set(port, fvs_updated) + helper_logger.log_info("Got a change event for toggle the mux-direction active side for port {} state from {} to {}".format( + port, old_status, new_status)) + else: + helper_logger.log_info("Got a change event on port {} of table {} that does not contain state".format( + port, swsscommon.APP_HW_MUX_CABLE_TABLE_NAME)) + + while True: + (port_m, op_m, fvp_m) = mux_cable_command_tbl[asic_index].pop() + if not port_m: + break + if fvp_m: + + if port_m not in y_cable_tbl_keys[asic_index]: + continue + + fvp_dict = dict(fvp_m) + + if "command" in fvp_dict: + # check if xcvrd got a probe command + probe_identifier = fvp_dict["command"] + + if probe_identifier == "probe": + (status, fv) = y_cable_tbl[asic_index].get(port_m) + if status is False: + helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( + port_m, y_cable_tbl[asic_index])) + continue + mux_port_dict = dict(fv) + read_side = mux_port_dict.get("read_side") + update_appdb_port_mux_cable_response_table(port_m, asic_index, appl_db, int(read_side)) def task_run(self): self.task_thread = threading.Thread(target=self.task_worker)