From ef80d32f102dbe01f9f5ea5b1ae7bf7fd676fa5c Mon Sep 17 00:00:00 2001
From: "Marty Y. Lok" <76118573+mlok-nokia@users.noreply.github.com>
Date: Tue, 17 Dec 2024 22:18:07 -0500
Subject: [PATCH] [chassisd] Address the chassisd crash issue and add UT for it
 (#573)

Description
On Nokia platform, slot name of Supervisor is string "A" instead of a number. Using "int" to convert it could cause issue backtrace. We should use slot value to any checking without any conversion. This will fixes sonic-net/sonic-buildimage#21131

Motivation and Context
Modify the _get_module_info not to convert "slot" to a string value. And also modify the code not to convert slot value to an to do any checking. Just directly use the returned value of get_slot(). Also add UT test_moduleupdater_check_slot_string() to valid it.

How Has This Been Tested?
Tested on 202405 branch


Signed-off-by: mlok <marty.lok@nokia.com>
---
 sonic-chassisd/scripts/chassisd       |  8 ++---
 sonic-chassisd/tests/test_chassisd.py | 48 +++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/sonic-chassisd/scripts/chassisd b/sonic-chassisd/scripts/chassisd
index 0f31ce52c..28f89503f 100755
--- a/sonic-chassisd/scripts/chassisd
+++ b/sonic-chassisd/scripts/chassisd
@@ -276,7 +276,7 @@ class ModuleUpdater(logger.Logger):
 
         for module_index in range(0, self.num_modules):
             module_info_dict = self._get_module_info(module_index)
-            if self.my_slot == int(module_info_dict['slot']):
+            if self.my_slot == module_info_dict['slot']:
                 my_index = module_index
 
             if module_info_dict is not None:
@@ -293,7 +293,7 @@ class ModuleUpdater(logger.Logger):
 
                 fvs = swsscommon.FieldValuePairs([(CHASSIS_MODULE_INFO_DESC_FIELD, module_info_dict[CHASSIS_MODULE_INFO_DESC_FIELD]),
                                                   (CHASSIS_MODULE_INFO_SLOT_FIELD,
-                                                   module_info_dict[CHASSIS_MODULE_INFO_SLOT_FIELD]),
+                                                   str(module_info_dict[CHASSIS_MODULE_INFO_SLOT_FIELD])),
                                                   (CHASSIS_MODULE_INFO_OPERSTATUS_FIELD, module_info_dict[CHASSIS_MODULE_INFO_OPERSTATUS_FIELD]),
                                                   (CHASSIS_MODULE_INFO_NUM_ASICS_FIELD, str(len(module_info_dict[CHASSIS_MODULE_INFO_ASICS]))),
                                                   (CHASSIS_MODULE_INFO_SERIAL_FIELD, module_info_dict[CHASSIS_MODULE_INFO_SERIAL_FIELD])])
@@ -350,8 +350,8 @@ class ModuleUpdater(logger.Logger):
 
         # In line card push the hostname of the module and num_asics to the chassis state db.
         # The hostname is used as key to access chassis app db entries 
-        module_info_dict = self._get_module_info(my_index)
         if not self._is_supervisor():
+           module_info_dict = self._get_module_info(my_index)
            hostname_key = "{}{}".format(ModuleBase.MODULE_TYPE_LINE, int(self.my_slot) - 1)
            hostname = try_get(device_info.get_hostname, default="None")
            hostname_fvs = swsscommon.FieldValuePairs([(CHASSIS_MODULE_INFO_SLOT_FIELD, str(self.my_slot)), 
@@ -386,7 +386,7 @@ class ModuleUpdater(logger.Logger):
 
         module_info_dict[CHASSIS_MODULE_INFO_NAME_FIELD] = name
         module_info_dict[CHASSIS_MODULE_INFO_DESC_FIELD] = str(desc)
-        module_info_dict[CHASSIS_MODULE_INFO_SLOT_FIELD] = str(slot)
+        module_info_dict[CHASSIS_MODULE_INFO_SLOT_FIELD] = slot
         module_info_dict[CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] = str(status)
         module_info_dict[CHASSIS_MODULE_INFO_ASICS] = asics
         module_info_dict[CHASSIS_MODULE_INFO_SERIAL_FIELD] = str(serial)
diff --git a/sonic-chassisd/tests/test_chassisd.py b/sonic-chassisd/tests/test_chassisd.py
index 417f5d878..83418a9d1 100644
--- a/sonic-chassisd/tests/test_chassisd.py
+++ b/sonic-chassisd/tests/test_chassisd.py
@@ -280,6 +280,54 @@ def test_configupdater_check_num_modules():
     fvs = module_updater.chassis_table.get(CHASSIS_INFO_KEY_TEMPLATE.format(1))
     assert fvs == None
 
+def test_moduleupdater_check_string_slot():
+    chassis = MockChassis()
+
+    #Supervisor
+    index = 0
+    name = "SUPERVISOR0"
+    desc = "Supervisor card"
+    slot = "A"
+    serial = "RP1000101"
+    module_type = ModuleBase.MODULE_TYPE_SUPERVISOR
+    supervisor = MockModule(index, name, desc, module_type, slot, serial)
+    supervisor.set_midplane_ip()
+    chassis.module_list.append(supervisor)
+
+    #Linecard
+    index = 1
+    name = "LINE-CARD0"
+    desc = "36 port 400G card"
+    slot = "1"
+    serial = "LC1000101"
+    module_type = ModuleBase.MODULE_TYPE_LINE
+    module = MockModule(index, name, desc, module_type, slot, serial)
+    module.set_midplane_ip()
+    chassis.module_list.append(module)
+
+    #Fabric-card
+    index = 1
+    name = "FABRIC-CARD0"
+    desc = "Switch fabric card"
+    slot = "17"
+    serial = "FC1000101"
+    module_type = ModuleBase.MODULE_TYPE_FABRIC
+    fabric = MockModule(index, name, desc, module_type, slot, serial)
+    chassis.module_list.append(fabric)
+
+    #Run on supervisor
+    module_updater = ModuleUpdater(SYSLOG_IDENTIFIER, chassis, slot,
+                                   module.supervisor_slot)
+    module_updater.supervisor_slot = supervisor.get_slot()
+    module_updater.my_slot = supervisor.get_slot()
+    module_updater.modules_num_update()
+    module_updater.module_db_update()
+    module_updater.check_midplane_reachability()
+
+    midplane_table = module_updater.midplane_table
+    #Check only one entry in database
+    assert 1 == midplane_table.size()
+    
 def test_midplane_presence_modules():
     chassis = MockChassis()