Skip to content

Commit

Permalink
[202211] chassisd: Fix crash on exit on linecard (#352)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
patrickmacarthur and judyjoseph authored Jul 19, 2023
1 parent 4daa748 commit ea8e5c7
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 1 deletion.
5 changes: 5 additions & 0 deletions sonic-chassisd/scripts/chassisd
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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()

Expand Down
18 changes: 17 additions & 1 deletion sonic-chassisd/tests/mock_swsscommon.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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
6 changes: 6 additions & 0 deletions sonic-chassisd/tests/mocked_libs/sonic_platform/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""
Mock implementation of sonic_platform package for unit testing
"""

from . import chassis
from . import platform
26 changes: 26 additions & 0 deletions sonic-chassisd/tests/mocked_libs/sonic_platform/chassis.py
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions sonic-chassisd/tests/mocked_libs/sonic_platform/platform.py
Original file line number Diff line number Diff line change
@@ -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()
23 changes: 23 additions & 0 deletions sonic-chassisd/tests/test_chassisd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()

0 comments on commit ea8e5c7

Please sign in to comment.