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

[hostcfgd] Enhance hostcfgd to check feature state and run less system calls (#7987)

Browse Source 
Why I did it
Currently hostcfgd is implemented in a way each feature which is enabled/disabled triggering execution of systemctl enable/unmask commands which eventually trigger 'systemctl daemon-reload' command.
Each call like this cost 0.6s and overall add a overhead of ~12 seconds of CPU time.
This change will verify the desired state of a feature and the current state of this feature on systemd and trigger a system call only when must.

How I did it
Check each feature status on systemd before executing a system call to enable and reload the systemctl daemon.

How to verify it
Build an image with this change and observe less system calls are executed.

Signed-off-by: Shlomi Bitton <shlomibi@nvidia.com>
This commit is contained in:
shlomibitton 2021-07-04 07:12:28 +03:00 committed by Judy Joseph
parent 7916276330
commit 0f3657fc02
3 changed files with 115 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -240,6 +240,13 @@ class FeatureHandler(object):
cmds = []
feature_names, feature_suffixes = self.get_feature_attribute(feature)
for feature_name in feature_names:
# Check if it is already enabled, if yes skip the system call
cmd = "sudo systemctl status {}.{} | grep Loaded".format(feature_name, feature_suffixes[-1])
proc = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
(stdout, stderr) = proc.communicate()
if "enabled" in str(stdout):
continue
for suffix in feature_suffixes:
cmds.append("sudo systemctl unmask {}.{}".format(feature_name, suffix))
# If feature has timer associated with it, start/enable corresponding systemd .timer unit
@ -259,6 +266,13 @@ class FeatureHandler(object):
cmds = []
feature_names, feature_suffixes = self.get_feature_attribute(feature)
for feature_name in feature_names:
# Check if it is already disabled, if yes skip the system call
cmd = "sudo systemctl status {}.{} | grep Loaded".format(feature_name, feature_suffixes[-1])
proc = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
(stdout, stderr) = proc.communicate()
if "disabled" in str(stdout) or "masked" in str(stdout):
continue
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]))

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

@ -97,6 +97,11 @@ class TestHostcfgd(TestCase):
fs.create_dir(hostcfgd.FeatureHandler.SYSTEMD_SYSTEM_DIR)
MockConfigDb.set_config_db(test_data["config_db"])
with mock.patch("hostcfgd.subprocess") as mocked_subprocess:
popen_mock = mock.Mock()
attrs = test_data["popen_attributes"]
popen_mock.configure_mock(**attrs)
mocked_subprocess.Popen.return_value = popen_mock
host_config_daemon = hostcfgd.HostConfigDaemon()
host_config_daemon.feature_handler.update_all_features_config()
assert self.__verify_table(

96
src/sonic-host-services/tests/hostcfgd/test_vectors.py
View File

@ -96,6 +96,9 @@ HOSTCFGD_TEST_VECTOR = [
call("sudo systemctl enable telemetry.timer", shell=True),
call("sudo systemctl start telemetry.timer", shell=True),
],
"popen_attributes": {
'communicate.return_value': ('output', 'error')
},
},
],
[
@ -189,6 +192,9 @@ HOSTCFGD_TEST_VECTOR = [
call("sudo systemctl enable telemetry.timer", shell=True),
call("sudo systemctl start telemetry.timer", shell=True),
],
"popen_attributes": {
'communicate.return_value': ('output', 'error')
},
},
],
[
@ -282,6 +288,96 @@ HOSTCFGD_TEST_VECTOR = [
call("sudo systemctl enable telemetry.timer", shell=True),
call("sudo systemctl start telemetry.timer", shell=True),
],
"popen_attributes": {
'communicate.return_value': ('output', 'error')
},
},
],
[
"DualTorCaseWithNoSystemCalls",
{
"config_db": {
"DEVICE_METADATA": {
"localhost": {
"subtype": "DualToR",
"type": "ToRRouter",
}
},
"KDUMP": {
"config": {
"enabled": "false",
"num_dumps": "3",
"memory": "0M-2G:256M,2G-4G:320M,4G-8G:384M,8G-:448M"
}
},
"FEATURE": {
"dhcp_relay": {
"auto_restart": "enabled",
"has_global_scope": "True",
"has_per_asic_scope": "False",
"has_timer": "False",
"high_mem_alert": "disabled",
"set_owner": "kube",
"state": "enabled"
},
"mux": {
"auto_restart": "enabled",
"has_global_scope": "True",
"has_per_asic_scope": "False",
"has_timer": "False",
"high_mem_alert": "disabled",
"set_owner": "local",
"state": "{% if 'subtype' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['subtype'] == 'DualToR' %}enabled{% else %}always_disabled{% endif %}"
},
"telemetry": {
"auto_restart": "enabled",
"has_global_scope": "True",
"has_per_asic_scope": "False",
"has_timer": "True",
"high_mem_alert": "disabled",
"set_owner": "kube",
"state": "enabled",
"status": "enabled"
},
},
},
"expected_config_db": {
"FEATURE": {
"dhcp_relay": {
"auto_restart": "enabled",
"has_global_scope": "True",
"has_per_asic_scope": "False",
"has_timer": "False",
"high_mem_alert": "disabled",
"set_owner": "kube",
"state": "enabled"
},
"mux": {
"auto_restart": "enabled",
"has_global_scope": "True",
"has_per_asic_scope": "False",
"has_timer": "False",
"high_mem_alert": "disabled",
"set_owner": "local",
"state": "enabled"
},
"telemetry": {
"auto_restart": "enabled",
"has_global_scope": "True",
"has_per_asic_scope": "False",
"has_timer": "True",
"high_mem_alert": "disabled",
"set_owner": "kube",
"state": "enabled",
"status": "enabled"
},
},
},
"expected_subprocess_calls": [
],
"popen_attributes": {
'communicate.return_value': ('enabled', 'error')
},
}
]
]