From ea8e5c767fc69d3bdc155f85c6caf1438beb0251 Mon Sep 17 00:00:00 2001 From: Patrick MacArthur Date: Wed, 19 Jul 2023 17:30:31 -0400 Subject: [PATCH] [202211] chassisd: Fix crash on exit on linecard (#352) * Chassisd do an explicit stop of the config_manager (#328) * Fix to explicit stop the config_manager * Add tests for chassisd run method. * 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. --------- Co-authored-by: judyjoseph <53951155+judyjoseph@users.noreply.github.com> --- sonic-chassisd/scripts/chassisd | 5 ++++ sonic-chassisd/tests/mock_swsscommon.py | 18 ++++++++++++- .../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 | 23 ++++++++++++++++ 6 files changed, 89 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..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") @@ -445,6 +447,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..2da88e1e4 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): @@ -29,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/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..6986059be 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,21 @@ 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_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()