From 5f8f31cccbbe26d0a375d7d1a86a0ef13f9d97f7 Mon Sep 17 00:00:00 2001 From: judyjoseph <53951155+judyjoseph@users.noreply.github.com> Date: Tue, 17 Jan 2023 13:10:09 -0800 Subject: [PATCH 1/2] Chassisd do an explicit stop of the config_manager (#328) * Fix to explicit stop the config_manager * Add tests for chassisd run method. --- sonic-chassisd/scripts/chassisd | 3 +++ sonic-chassisd/tests/mock_swsscommon.py | 3 ++- .../mocked_libs/sonic_platform/__init__.py | 6 +++++ .../mocked_libs/sonic_platform/chassis.py | 26 +++++++++++++++++++ .../mocked_libs/sonic_platform/platform.py | 12 +++++++++ sonic-chassisd/tests/test_chassisd.py | 12 +++++++++ 6 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 sonic-chassisd/tests/mocked_libs/sonic_platform/__init__.py create mode 100644 sonic-chassisd/tests/mocked_libs/sonic_platform/chassis.py create mode 100644 sonic-chassisd/tests/mocked_libs/sonic_platform/platform.py diff --git a/sonic-chassisd/scripts/chassisd b/sonic-chassisd/scripts/chassisd index c0cc4c23a..4347bb399 100644 --- a/sonic-chassisd/scripts/chassisd +++ b/sonic-chassisd/scripts/chassisd @@ -445,6 +445,9 @@ class ChassisdDaemon(daemon_base.DaemonBase): self.log_info("Stop daemon main loop") + if config_manager is not None: + config_manager.task_stop() + # Delete all the information from DB and then exit self.module_updater.deinit() diff --git a/sonic-chassisd/tests/mock_swsscommon.py b/sonic-chassisd/tests/mock_swsscommon.py index 589084225..dbc42e13f 100644 --- a/sonic-chassisd/tests/mock_swsscommon.py +++ b/sonic-chassisd/tests/mock_swsscommon.py @@ -7,7 +7,8 @@ def __init__(self, db, table_name): self.mock_dict = {} def _del(self, key): - del self.mock_dict[key] + if key in self.mock_dict: + del self.mock_dict[key] pass def set(self, key, fvs): diff --git a/sonic-chassisd/tests/mocked_libs/sonic_platform/__init__.py b/sonic-chassisd/tests/mocked_libs/sonic_platform/__init__.py new file mode 100644 index 000000000..e491d5b52 --- /dev/null +++ b/sonic-chassisd/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-chassisd/tests/mocked_libs/sonic_platform/chassis.py b/sonic-chassisd/tests/mocked_libs/sonic_platform/chassis.py new file mode 100644 index 000000000..2410afde2 --- /dev/null +++ b/sonic-chassisd/tests/mocked_libs/sonic_platform/chassis.py @@ -0,0 +1,26 @@ +""" + Mock implementation of sonic_platform package for unit testing +""" + +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 + + def get_my_slot(self): + return 1 + + def get_supervisor_slot(self): + return 1 diff --git a/sonic-chassisd/tests/mocked_libs/sonic_platform/platform.py b/sonic-chassisd/tests/mocked_libs/sonic_platform/platform.py new file mode 100644 index 000000000..a1b61e13e --- /dev/null +++ b/sonic-chassisd/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-chassisd/tests/test_chassisd.py b/sonic-chassisd/tests/test_chassisd.py index 80655c50c..ba92f1233 100644 --- a/sonic-chassisd/tests/test_chassisd.py +++ b/sonic-chassisd/tests/test_chassisd.py @@ -14,6 +14,11 @@ daemon_base.db_connect = MagicMock() test_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(test_path, 'mocked_libs') +sys.path.insert(0, mocked_libs_path) + modules_path = os.path.dirname(test_path) scripts_path = os.path.join(modules_path, "scripts") sys.path.insert(0, modules_path) @@ -501,3 +506,10 @@ def test_signal_handler(): assert daemon_chassisd.log_info.call_count == 0 assert daemon_chassisd.stop.set.call_count == 0 assert exit_code == 0 + +def test_daemon_run(): + # Test the chassisd run + daemon_chassisd = ChassisdDaemon(SYSLOG_IDENTIFIER) + daemon_chassisd.stop = MagicMock() + daemon_chassisd.stop.wait.return_value = True + daemon_chassisd.run() From cb4ad629c618c724731234e731b5466fedc37b44 Mon Sep 17 00:00:00 2001 From: Patrick MacArthur Date: Thu, 9 Mar 2023 18:59:00 -0500 Subject: [PATCH 2/2] chassisd: Fix crash on exit on linecard Set the `config_manager` variable to `None` if we are running on a linecard and thus don't need to set up the config manager. During cleanup, the chassid service tries to clean up the `config_manager`, but the `config_manager` variable is only ever initialized if we are on the supervisor. Thus, checking if it is `None` is insufficient because this results in an `UnboundLocalError` that prevents the cleanup from succeeding on a linecard. --- sonic-chassisd/scripts/chassisd | 2 ++ sonic-chassisd/tests/mock_swsscommon.py | 15 +++++++++++++++ sonic-chassisd/tests/test_chassisd.py | 13 ++++++++++++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/sonic-chassisd/scripts/chassisd b/sonic-chassisd/scripts/chassisd index 4347bb399..fea1853d4 100644 --- a/sonic-chassisd/scripts/chassisd +++ b/sonic-chassisd/scripts/chassisd @@ -435,6 +435,8 @@ class ChassisdDaemon(daemon_base.DaemonBase): if self.module_updater.supervisor_slot == self.module_updater.my_slot: config_manager = ConfigManagerTask() config_manager.task_run() + else: + config_manager = None # Start main loop self.log_info("Start daemon main loop") diff --git a/sonic-chassisd/tests/mock_swsscommon.py b/sonic-chassisd/tests/mock_swsscommon.py index dbc42e13f..2da88e1e4 100644 --- a/sonic-chassisd/tests/mock_swsscommon.py +++ b/sonic-chassisd/tests/mock_swsscommon.py @@ -30,3 +30,18 @@ class FieldValuePairs: def __init__(self, fvs): self.fv_dict = dict(fvs) pass + +class Select: + TIMEOUT = 1 + + def addSelectable(self, selectable): + pass + + def removeSelectable(self, selectable): + pass + + def select(self, timeout=-1, interrupt_on_signal=False): + return self.TIMEOUT, None + +class SubscriberStateTable(Table): + pass diff --git a/sonic-chassisd/tests/test_chassisd.py b/sonic-chassisd/tests/test_chassisd.py index ba92f1233..6986059be 100644 --- a/sonic-chassisd/tests/test_chassisd.py +++ b/sonic-chassisd/tests/test_chassisd.py @@ -507,9 +507,20 @@ def test_signal_handler(): assert daemon_chassisd.stop.set.call_count == 0 assert exit_code == 0 -def test_daemon_run(): +def test_daemon_run_supervisor(): # Test the chassisd run daemon_chassisd = ChassisdDaemon(SYSLOG_IDENTIFIER) daemon_chassisd.stop = MagicMock() daemon_chassisd.stop.wait.return_value = True daemon_chassisd.run() + +def test_daemon_run_linecard(): + # Test the chassisd run + daemon_chassisd = ChassisdDaemon(SYSLOG_IDENTIFIER) + daemon_chassisd.stop = MagicMock() + daemon_chassisd.stop.wait.return_value = True + + import sonic_platform.platform + with patch.object(sonic_platform.platform.Chassis, 'get_my_slot') as mock: + mock.return_value = sonic_platform.platform.Platform().get_chassis().get_supervisor_slot() + 1 + daemon_chassisd.run()