Added UT for the changes

Signed-off-by: Abhishek Dosi <[email protected]>

Author: abdosi

scripts/hostcfgd

@@ -232,7 +232,7 @@ class FeatureHandler(object):
         """
         Summary:
         Updates the state field in the FEATURE|* tables as the state field
-        might have to be rendered based on DEVICE_METADATA table and generated Device Running Metadata
+        might have to be rendere dbased on DEVICE_METADATA table and generated Device Running Metadata
         """
         for feature_name in feature_table.keys():
             if not feature_name:
@@ -285,8 +285,7 @@ class FeatureHandler(object):
             self.disable_feature(feature)
             syslog.syslog(syslog.LOG_INFO, "Feature {} is stopped and disabled".format(feature.name))
 
-        if self.is_multi_npu:
-            self.sync_feature_asic_scope(feature)
+        self.sync_feature_asic_scope(feature)
 
         return True
 
@@ -311,7 +310,7 @@ class FeatureHandler(object):
             unit_file_state = self.get_systemd_unit_state("{}.{}".format(feature_name, feature_suffixes[-1]))
             if not unit_file_state:
                 continue
-            if unit_file_state == "enabled" and not feature_config.has_per_asic_scope:
+            if unit_file_state != "disabled" and not feature_config.has_per_asic_scope:
                 for suffix in reversed(feature_suffixes):
                     cmds.append("sudo systemctl stop {}.{}".format(feature_name, suffix))
                     cmds.append("sudo systemctl disable {}.{}".format(feature_name, feature_suffixes[-1]))
@@ -325,7 +324,7 @@ class FeatureHandler(object):
                                       .format(feature.name, feature_suffixes[-1]))
                         self.set_feature_state(feature, self.FEATURE_STATE_FAILED)
                         return
-        self._config_db.mod_entry('FEATURE', feature_config.name, {'has_per_asic_scope': feature_config.has_per_asic_scope})
+        self._config_db.mod_entry('FEATURE', feature_config.name, {'has_per_asic_scope': str(feature_config.has_per_asic_scope)})
 
 
 

tests/hostcfgd/hostcfgd_test.py

@@ -1,6 +1,7 @@
 import os
 import sys
 import swsscommon as swsscommon_package
+from sonic_py_common import device_info
 from swsscommon import swsscommon
 
 from parameterized import parameterized
@@ -45,7 +46,7 @@ def checks_config_table(self, feature_table, expected_table):
 
         return True if not ddiff else False
 
-    def checks_systemd_config_file(self, feature_table):
+    def checks_systemd_config_file(self, feature_table, feature_systemd_name_map=None):
         """Checks whether the systemd configuration file of each feature was created or not
         and whether the `Restart=` field in the file is set correctly or not.
 
@@ -68,13 +69,16 @@ def checks_systemd_config_file(self, feature_table):
             elif "disabled" in auto_restart_status:
                 auto_restart_status = "disabled"
 
-            feature_systemd_config_file_path = systemd_config_file_path.format(feature_name)
-            is_config_file_existing = os.path.exists(feature_systemd_config_file_path)
-            assert is_config_file_existing, "Systemd configuration file of feature '{}' does not exist!".format(feature_name)
+            feature_systemd_list = feature_systemd_name_map[feature_name] if feature_systemd_name_map else [feature_name]
 
-            with open(feature_systemd_config_file_path) as systemd_config_file:
-                status = systemd_config_file.read().strip()
-            assert status == '[Service]\nRestart={}'.format(truth_table[auto_restart_status])
+            for feature_systemd in feature_systemd_list:
+                feature_systemd_config_file_path = systemd_config_file_path.format(feature_systemd)
+                is_config_file_existing = os.path.exists(feature_systemd_config_file_path)
+                assert is_config_file_existing, "Systemd configuration file of feature '{}' does not exist!".format(feature_systemd)
+
+                with open(feature_systemd_config_file_path) as systemd_config_file:
+                    status = systemd_config_file.read().strip()
+                assert status == '[Service]\nRestart={}'.format(truth_table[auto_restart_status])
 
     def get_state_db_set_calls(self, feature_table):
         """Returns a Mock call objects which recorded the `set` calls to `FEATURE` table in `STATE_DB`.
@@ -119,31 +123,40 @@ def test_sync_state_field(self, test_scenario_name, config_data, fs):
         MockConfigDb.set_config_db(config_data['config_db'])
         feature_state_table_mock = mock.Mock()
         with mock.patch('hostcfgd.subprocess') as mocked_subprocess:
-            popen_mock = mock.Mock()
-            attrs = config_data['popen_attributes']
-            popen_mock.configure_mock(**attrs)
-            mocked_subprocess.Popen.return_value = popen_mock
-
-            device_config = {}
-            device_config['DEVICE_METADATA'] = MockConfigDb.CONFIG_DB['DEVICE_METADATA']
-            feature_handler = hostcfgd.FeatureHandler(MockConfigDb(), feature_state_table_mock, device_config)
-
-            feature_table = MockConfigDb.CONFIG_DB['FEATURE']
-            feature_handler.sync_state_field(feature_table)
-
-            is_any_difference = self.checks_config_table(MockConfigDb.get_config_db()['FEATURE'],
-                                                         config_data['expected_config_db']['FEATURE'])
-            assert is_any_difference, "'FEATURE' table in 'CONFIG_DB' is modified unexpectedly!"
-
-            feature_table_state_db_calls = self.get_state_db_set_calls(feature_table)
-
-            self.checks_systemd_config_file(config_data['config_db']['FEATURE'])
-            mocked_subprocess.check_call.assert_has_calls(config_data['enable_feature_subprocess_calls'],
-                                                          any_order=True)
-            mocked_subprocess.check_call.assert_has_calls(config_data['daemon_reload_subprocess_call'],
-                                                          any_order=True)
-            feature_state_table_mock.set.assert_has_calls(feature_table_state_db_calls)
-            self.checks_systemd_config_file(config_data['config_db']['FEATURE'])
+            with mock.patch("sonic_py_common.device_info.get_device_runtime_metadata", return_value=config_data['device_runtime_metadata']):
+                with mock.patch("sonic_py_common.device_info.is_multi_npu", return_value=True if 'num_npu' in config_data else False):
+                    with mock.patch("sonic_py_common.device_info.get_num_npus", return_value=config_data['num_npu'] if 'num_npu' in config_data else 1):
+                        popen_mock = mock.Mock()
+                        attrs = config_data['popen_attributes']
+                        popen_mock.configure_mock(**attrs)
+                        mocked_subprocess.Popen.return_value = popen_mock
+
+                        device_config = {}
+                        device_config['DEVICE_METADATA'] = MockConfigDb.CONFIG_DB['DEVICE_METADATA']
+                        feature_handler = hostcfgd.FeatureHandler(MockConfigDb(), feature_state_table_mock, device_config)
+
+                        feature_table = MockConfigDb.CONFIG_DB['FEATURE']
+                        feature_handler.sync_state_field(feature_table)
+
+                        feature_systemd_name_map = {}
+                        for feature_name in feature_table.keys():
+                            feature = hostcfgd.Feature(feature_name, feature_table[feature_name], device_config | config_data['device_runtime_metadata'])
+                            feature_names, _ = feature_handler.get_multiasic_feature_instances(feature)
+                            feature_systemd_name_map[feature_name] = feature_names
+
+                        is_any_difference = self.checks_config_table(MockConfigDb.get_config_db()['FEATURE'],
+                                                                     config_data['expected_config_db']['FEATURE'])
+                        assert is_any_difference, "'FEATURE' table in 'CONFIG_DB' is modified unexpectedly!"
+
+                        feature_table_state_db_calls = self.get_state_db_set_calls(feature_table)
+
+                        self.checks_systemd_config_file(config_data['config_db']['FEATURE'], feature_systemd_name_map)
+                        mocked_subprocess.check_call.assert_has_calls(config_data['enable_feature_subprocess_calls'],
+                                                                      any_order=True)
+                        mocked_subprocess.check_call.assert_has_calls(config_data['daemon_reload_subprocess_call'],
+                                                                      any_order=True)
+                        feature_state_table_mock.set.assert_has_calls(feature_table_state_db_calls)
+                        self.checks_systemd_config_file(config_data['config_db']['FEATURE'], feature_systemd_name_map)
 
     @parameterized.expand(HOSTCFGD_TEST_VECTOR)
     @patchfs
@@ -164,25 +177,32 @@ def test_handler(self, test_scenario_name, config_data, fs):
         MockConfigDb.set_config_db(config_data['config_db'])
         feature_state_table_mock = mock.Mock()
         with mock.patch('hostcfgd.subprocess') as mocked_subprocess:
-            popen_mock = mock.Mock()
-            attrs = config_data['popen_attributes']
-            popen_mock.configure_mock(**attrs)
-            mocked_subprocess.Popen.return_value = popen_mock
-
-            device_config = {}
-            device_config['DEVICE_METADATA'] = MockConfigDb.CONFIG_DB['DEVICE_METADATA']
-            feature_handler = hostcfgd.FeatureHandler(MockConfigDb(), feature_state_table_mock, device_config)
-
-            feature_table = MockConfigDb.CONFIG_DB['FEATURE']
-
-            for feature_name, feature_config in feature_table.items():
-                feature_handler.handler(feature_name, 'SET', feature_config)
-
-            self.checks_systemd_config_file(config_data['config_db']['FEATURE'])
-            mocked_subprocess.check_call.assert_has_calls(config_data['enable_feature_subprocess_calls'],
-                                                          any_order=True)
-            mocked_subprocess.check_call.assert_has_calls(config_data['daemon_reload_subprocess_call'],
-                                                          any_order=True)
+            with mock.patch("sonic_py_common.device_info.get_device_runtime_metadata", return_value=config_data['device_runtime_metadata']):
+                with mock.patch("sonic_py_common.device_info.is_multi_npu", return_value=True if 'num_npu' in config_data else False):
+                    with mock.patch("sonic_py_common.device_info.get_num_npus", return_value=config_data['num_npu'] if 'num_npu' in config_data else 1):
+                        popen_mock = mock.Mock()
+                        attrs = config_data['popen_attributes']
+                        popen_mock.configure_mock(**attrs)
+                        mocked_subprocess.Popen.return_value = popen_mock
+
+                        device_config = {}
+                        device_config['DEVICE_METADATA'] = MockConfigDb.CONFIG_DB['DEVICE_METADATA']
+                        feature_handler = hostcfgd.FeatureHandler(MockConfigDb(), feature_state_table_mock, device_config)
+
+                        feature_table = MockConfigDb.CONFIG_DB['FEATURE']
+
+                        feature_systemd_name_map = {}
+                        for feature_name, feature_config in feature_table.items():
+                            feature_handler.handler(feature_name, 'SET', feature_config)
+                            feature = hostcfgd.Feature(feature_name, feature_table[feature_name], device_config | config_data['device_runtime_metadata'])
+                            feature_names, _ = feature_handler.get_multiasic_feature_instances(feature)
+                            feature_systemd_name_map[feature_name] = feature_names
+
+                        self.checks_systemd_config_file(config_data['config_db']['FEATURE'], feature_systemd_name_map)
+                        mocked_subprocess.check_call.assert_has_calls(config_data['enable_feature_subprocess_calls'],
+                                                                      any_order=True)
+                        mocked_subprocess.check_call.assert_has_calls(config_data['daemon_reload_subprocess_call'],
+                                                                      any_order=True)
 
     def test_feature_config_parsing(self):
         swss_feature = hostcfgd.Feature('swss', {