matthew/sonic-buildimage
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

[hostcfgd] Fix issue: FeatureHandler might override user configuration (#16766)

Browse Source 
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
This commit is contained in:
Nazarii Hnydyn 2023-10-11 19:14:00 +03:00 committed by GitHub
parent 6f93832a03
commit 7b06d9b982
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 90 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/sonic-host-services/scripts/hostcfgd
View File

@ -352,7 +352,7 @@ class FeatureHandler(object):
# 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', feature_config.name, {'has_per_asic_scope': str(feature_config.has_per_asic_scope)})
def update_systemd_config(self, feature_config):
"""Updates `Restart=` field in feature's systemd configuration file
according to the value of `auto_restart` field in `FEATURE` table of `CONFIG_DB`.
@ -427,7 +427,7 @@ class FeatureHandler(object):
unit_file_state = self.get_systemd_unit_state("{}.{}".format(feature_name, feature_suffixes[-1]))
if unit_file_state == "enabled" or not unit_file_state:
continue
cmds = []
cmds = []
for suffix in feature_suffixes:
cmds.append("sudo systemctl unmask {}.{}".format(feature_name, suffix))
@ -474,11 +474,28 @@ class FeatureHandler(object):
self.set_feature_state(feature, self.FEATURE_STATE_DISABLED)
def resync_feature_state(self, feature):
self._config_db.mod_entry('FEATURE', feature.name, {'state': feature.state})
current_entry = self._config_db.get_entry('FEATURE', feature.name)
current_feature_state = current_entry.get('state') if current_entry else None
# resync the feature state to CONFIG_DB in namespaces in multi-asic platform
for ns, db in self.ns_cfg_db.items():
db.mod_entry('FEATURE', feature.name, {'state': feature.state})
if feature.state == current_feature_state:
return
# feature.state might be rendered from a template, so that it should resync CONFIG DB
# FEATURE table to override the template value to valid state value
# ('always_enabled', 'always_disabled', 'disabled', 'enabled'). However, we should only
# resync feature state in two cases:
# 1. the rendered feature state is always_enabled or always_disabled, it means that the
# feature state is immutable and potential state change during HostConfigDaemon.load
# in redis should be skipped;
# 2. the current feature state in DB is a template which should be replaced by rendered feature
# state
# For other cases, we should not resync feature.state to CONFIG DB to avoid overriding user configuration.
if self._feature_state_is_immutable(feature.state) or self._feature_state_is_template(current_feature_state):
self._config_db.mod_entry('FEATURE', feature.name, {'state': feature.state})
# resync the feature state to CONFIG_DB in namespaces in multi-asic platform
for ns, db in self.ns_cfg_db.items():
db.mod_entry('FEATURE', feature.name, {'state': feature.state})
def set_feature_state(self, feature, state):
self._feature_state_table.set(feature.name, [('state', state)])
@ -487,6 +504,12 @@ class FeatureHandler(object):
for ns, tbl in self.ns_feature_state_tbl.items():
tbl.set(feature.name, [('state', state)])
def _feature_state_is_template(self, feature_state):
return feature_state not in ('always_enabled', 'always_disabled', 'disabled', 'enabled')
def _feature_state_is_immutable(self, feature_state):
return feature_state in ('always_enabled', 'always_disabled')
class Iptables(object):
def __init__(self):
'''

61
src/sonic-host-services/tests/hostcfgd/hostcfgd_test.py
View File

@ -241,6 +241,67 @@ class TestFeatureHandler(TestCase):
assert swss_feature.has_global_scope
assert not swss_feature.has_per_asic_scope
@mock.patch('hostcfgd.FeatureHandler.update_systemd_config', mock.MagicMock())
@mock.patch('hostcfgd.FeatureHandler.update_feature_state', mock.MagicMock())
@mock.patch('hostcfgd.FeatureHandler.sync_feature_asic_scope', mock.MagicMock())
def test_feature_resync(self):
mock_db = mock.MagicMock()
mock_db.get_entry = mock.MagicMock()
mock_db.mod_entry = mock.MagicMock()
mock_feature_state_table = mock.MagicMock()
feature_handler = hostcfgd.FeatureHandler(mock_db, mock_feature_state_table, {})
feature_table = {
'sflow': {
'state': 'enabled',
'auto_restart': 'enabled',
'delayed': 'True',
'has_global_scope': 'False',
'has_per_asic_scope': 'True',
}
}
mock_db.get_entry.return_value = None
feature_handler.sync_state_field(feature_table)
mock_db.mod_entry.assert_called_with('FEATURE', 'sflow', {'state': 'enabled'})
mock_db.mod_entry.reset_mock()
feature_handler = hostcfgd.FeatureHandler(mock_db, mock_feature_state_table, {})
mock_db.get_entry.return_value = {
'state': 'disabled',
}
feature_handler.sync_state_field(feature_table)
mock_db.mod_entry.assert_not_called()
feature_handler = hostcfgd.FeatureHandler(mock_db, mock_feature_state_table, {})
feature_table = {
'sflow': {
'state': 'always_enabled',
'auto_restart': 'enabled',
'delayed': 'True',
'has_global_scope': 'False',
'has_per_asic_scope': 'True',
}
}
feature_handler.sync_state_field(feature_table)
mock_db.mod_entry.assert_called_with('FEATURE', 'sflow', {'state': 'always_enabled'})
mock_db.mod_entry.reset_mock()
feature_handler = hostcfgd.FeatureHandler(mock_db, mock_feature_state_table, {})
mock_db.get_entry.return_value = {
'state': 'some template',
}
feature_table = {
'sflow': {
'state': 'enabled',
'auto_restart': 'enabled',
'delayed': 'True',
'has_global_scope': 'False',
'has_per_asic_scope': 'True',
}
}
feature_handler.sync_state_field(feature_table)
mock_db.mod_entry.assert_called_with('FEATURE', 'sflow', {'state': 'enabled'})
class TesNtpCfgd(TestCase):
"""