Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support to render Feature Table has_global_scope field. #120

Merged
merged 8 commits into from
May 21, 2024
21 changes: 11 additions & 10 deletions scripts/featured
Original file line number Diff line number Diff line change
Expand Up @@ -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'))

Expand Down Expand Up @@ -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])
Expand All @@ -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):
Expand Down Expand Up @@ -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`
Expand All @@ -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)])
Expand All @@ -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
Expand Down Expand Up @@ -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 []) +
judyjoseph marked this conversation as resolved.
Show resolved Hide resolved
([(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)
Expand Down
2 changes: 1 addition & 1 deletion tests/featured/featured_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
10 changes: 5 additions & 5 deletions tests/featured/test_vectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -1147,9 +1147,9 @@
call(['sudo', 'systemctl', 'unmask', 'teamd@1.service']),
call(['sudo', 'systemctl', 'enable', 'teamd@1.service']),
call(['sudo', 'systemctl', 'start', 'teamd@1.service']),
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', 'lldp@0.service']),
call(['sudo', 'systemctl', 'enable', 'lldp@0.service']),
call(['sudo', 'systemctl', 'start', 'lldp@0.service']),
Expand Down
Loading