diff --git a/src/sonic-host-services/scripts/hostcfgd b/src/sonic-host-services/scripts/hostcfgd index 3b207b7fd5..3d6202a15a 100755 --- a/src/sonic-host-services/scripts/hostcfgd +++ b/src/sonic-host-services/scripts/hostcfgd @@ -457,6 +457,13 @@ class HostConfigDaemon: def enable_feature(self, feature_names, feature_suffixes): start_cmds = [] 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: start_cmds.append("sudo systemctl unmask {}.{}".format(feature_name, suffix)) # If feature has timer associated with it, start/enable corresponding systemd .timer unit @@ -477,6 +484,13 @@ class HostConfigDaemon: def disable_feature(self, feature_names, feature_suffixes): stop_cmds = [] 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): stop_cmds.append("sudo systemctl stop {}.{}".format(feature_name, suffix)) stop_cmds.append("sudo systemctl disable {}.{}".format(feature_name, suffix)) diff --git a/src/sonic-host-services/tests/hostcfgd/hostcfgd_test.py b/src/sonic-host-services/tests/hostcfgd/hostcfgd_test.py index 2abbb7f7ca..8f369ef993 100644 --- a/src/sonic-host-services/tests/hostcfgd/hostcfgd_test.py +++ b/src/sonic-host-services/tests/hostcfgd/hostcfgd_test.py @@ -65,6 +65,11 @@ class TestHostcfgd(TestCase): """ 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.update_all_feature_states() assert self.__verify_table( diff --git a/src/sonic-host-services/tests/hostcfgd/test_vectors.py b/src/sonic-host-services/tests/hostcfgd/test_vectors.py index a8ac0219d1..123a5512ad 100644 --- a/src/sonic-host-services/tests/hostcfgd/test_vectors.py +++ b/src/sonic-host-services/tests/hostcfgd/test_vectors.py @@ -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') + }, + } + ] ]