Merge branch 'sonic-net:master' into master

Author: i-davydenko

scripts/featured

@@ -77,7 +77,7 @@ class Feature(object):
         self.state = self._get_feature_table_key_render_value(feature_cfg.get('state'), device_config or {}, ['enabled', 'disabled', 'always_enabled', 'always_disabled'])
         self.auto_restart = feature_cfg.get('auto_restart', 'disabled')
         self.delayed = safe_eval(feature_cfg.get('delayed', 'False'))
-        self.has_global_scope = safe_eval(feature_cfg.get('has_global_scope', 'True'))
+        self.has_global_scope = safe_eval(self._get_feature_table_key_render_value(feature_cfg.get('has_global_scope', 'True'), device_config or {}, ['True', 'False']))
         self.has_per_asic_scope = safe_eval(self._get_feature_table_key_render_value(feature_cfg.get('has_per_asic_scope', 'False'), device_config or {}, ['True', 'False']))
         self.has_per_dpu_scope = safe_eval(feature_cfg.get('has_per_dpu_scope', 'False'))
 
@@ -198,7 +198,7 @@ class FeatureHandler(object):
         # Enable/disable the container service if the feature state was changed from its previous state.
         if self._cached_config[feature_name].state != feature.state:
             if self.update_feature_state(feature):
-                self.sync_feature_asic_scope(feature)
+                self.sync_feature_scope(feature)
                 self._cached_config[feature_name] = feature
             else:
                 self.resync_feature_state(self._cached_config[feature_name])
@@ -222,7 +222,7 @@ class FeatureHandler(object):
             self._cached_config.setdefault(feature_name, feature)
             self.update_systemd_config(feature)
             self.update_feature_state(feature)
-            self.sync_feature_asic_scope(feature)
+            self.sync_feature_scope(feature)
             self.resync_feature_state(feature)
 
     def update_feature_state(self, feature):
@@ -270,10 +270,10 @@ class FeatureHandler(object):
 
         return True
 
-    def sync_feature_asic_scope(self, feature_config):
-        """Updates the has_per_asic_scope field in the FEATURE|* tables as the field
+    def sync_feature_scope(self, feature_config):
+        """Updates the has_global_scope or has_per_asic_scope field in the FEATURE|* tables as the field
         might have to be rendered based on DEVICE_METADATA table or Device Running configuration.
-        Disable the ASIC instance service unit file it the render value is False and update config
+        Disable the Global/ASIC instance service unit file it the render value is False and update config
 
         Args:
             feature: An object represents a feature's configuration in `FEATURE`
@@ -284,12 +284,11 @@ class FeatureHandler(object):
         """
         feature_names, feature_suffixes = self.get_multiasic_feature_instances(feature_config, True)
         for feature_name in feature_names:
-            if '@' not in feature_name:
-                continue
             unit_file_state = self.get_systemd_unit_state("{}.{}".format(feature_name, feature_suffixes[-1]))
             if not unit_file_state:
                 continue
-            if unit_file_state != "disabled" and not feature_config.has_per_asic_scope:
+            if unit_file_state != "disabled" and \
+              ((not feature_config.has_per_asic_scope and '@' in feature_name) or (not feature_config.has_global_scope and '@' not in feature_name)):
                 cmds = []
                 for suffix in reversed(feature_suffixes):
                     cmds.append(["sudo", "systemctl", "stop", "{}.{}".format(feature_name, suffix)])
@@ -305,10 +304,12 @@ class FeatureHandler(object):
                         self.set_feature_state(feature_config, self.FEATURE_STATE_FAILED)
                         return
         self._config_db.mod_entry(FEATURE_TBL, feature_config.name, {'has_per_asic_scope': str(feature_config.has_per_asic_scope)})
+        self._config_db.mod_entry(FEATURE_TBL, feature_config.name, {'has_global_scope': str(feature_config.has_global_scope)})
 
         # sync has_per_asic_scope to CONFIG_DB in namespaces in multi-asic platform
         for ns, db in self.ns_cfg_db.items():
             db.mod_entry(FEATURE_TBL, feature_config.name, {'has_per_asic_scope': str(feature_config.has_per_asic_scope)})
+            db.mod_entry(FEATURE_TBL, feature_config.name, {'has_global_scope': str(feature_config.has_global_scope)})
 
     def update_systemd_config(self, feature_config):
         """Updates `Restart=` field in feature's systemd configuration file
@@ -364,7 +365,7 @@ class FeatureHandler(object):
     def get_multiasic_feature_instances(self, feature, all_instance=False):
         # Create feature name suffix depending feature is running in host or namespace or in both
         feature_names = (
-            ([feature.name] if feature.has_global_scope or not self.is_multi_npu else []) +
+            ([feature.name] if feature.has_global_scope or all_instance or not self.is_multi_npu else []) +
             ([(feature.name + '@' + str(asic_inst)) for asic_inst in range(device_info.get_num_npus())
                 if self.is_multi_npu and (all_instance or feature.has_per_asic_scope)]) + 
             ([(feature.name + '@' + device_info.DPU_NAME_PREFIX + str(dpu_inst)) for dpu_inst in range(self.num_dpus)

tests/featured/featured_test.py

@@ -262,7 +262,7 @@ def test_feature_config_parsing_defaults(self):
 
     @mock.patch('featured.FeatureHandler.update_systemd_config', mock.MagicMock())
     @mock.patch('featured.FeatureHandler.update_feature_state', mock.MagicMock())
-    @mock.patch('featured.FeatureHandler.sync_feature_asic_scope', mock.MagicMock())
+    @mock.patch('featured.FeatureHandler.sync_feature_scope', mock.MagicMock())
     def test_feature_resync(self):
         mock_db = mock.MagicMock()
         mock_db.get_entry = mock.MagicMock()

tests/featured/test_vectors.py

@@ -1083,7 +1083,7 @@
                     "lldp": {
                         "state": "enabled",
                         "delayed": "False",
-                        "has_global_scope": "True",
+                        "has_global_scope": "{% if ('CHASSIS_METADATA' in DEVICE_RUNTIME_METADATA and DEVICE_RUNTIME_METADATA['CHASSIS_METADATA']['module_type'] in ['linecard']) %}False{% else %}True{% endif %}",
                         "has_per_asic_scope": "{% if not DEVICE_RUNTIME_METADATA['ETHERNET_PORTS_PRESENT'] or ('CHASSIS_METADATA' in DEVICE_RUNTIME_METADATA and DEVICE_RUNTIME_METADATA['CHASSIS_METADATA']['module_type'] in ['supervisor']) %}False{% else %}True{% endif %}",
                         "auto_restart": "enabled",
                         "high_mem_alert": "disabled"
@@ -1118,7 +1118,7 @@
                     },
                     "lldp": {
                         "auto_restart": "enabled",
-                        "has_global_scope": "True",
+                        "has_global_scope": "False",
                         "has_per_asic_scope": "True",
                         "delayed": "False",
                         "high_mem_alert": "disabled",
@@ -1147,9 +1147,9 @@
                 call(['sudo', 'systemctl', 'unmask', '[email protected]']),
                 call(['sudo', 'systemctl', 'enable', '[email protected]']),
                 call(['sudo', 'systemctl', 'start', '[email protected]']),
-                call(['sudo', 'systemctl', 'unmask', 'lldp.service']),
-                call(['sudo', 'systemctl', 'enable', 'lldp.service']),
-                call(['sudo', 'systemctl', 'start', 'lldp.service']),
+                call(['sudo', 'systemctl', 'mask', 'lldp.service']),
+                call(['sudo', 'systemctl', 'disable', 'lldp.service']),
+                call(['sudo', 'systemctl', 'stop', 'lldp.service']),
                 call(['sudo', 'systemctl', 'unmask', '[email protected]']),
                 call(['sudo', 'systemctl', 'enable', '[email protected]']),
                 call(['sudo', 'systemctl', 'start', '[email protected]']),