diff --git a/src/system-health/health_checker/service_checker.py b/src/system-health/health_checker/service_checker.py index 674ad9944280..c81948a7ae25 100644 --- a/src/system-health/health_checker/service_checker.py +++ b/src/system-health/health_checker/service_checker.py @@ -24,13 +24,13 @@ class ServiceChecker(HealthChecker): CRITICAL_PROCESSES_PATH = 'etc/supervisor/critical_processes' # Command to get merged directory of a container - GET_CONTAINER_FOLDER_CMD = 'docker inspect {} --format "{{{{.GraphDriver.Data.MergedDir}}}}"' + GET_CONTAINER_FOLDER_CMD = ['docker', 'inspect', '', '--format', "{{.GraphDriver.Data.MergedDir}}"] # Command to query the status of monit service. - CHECK_MONIT_SERVICE_CMD = 'systemctl is-active monit.service' + CHECK_MONIT_SERVICE_CMD = ['systemctl', 'is-active', 'monit.service'] # Command to get summary of critical system service. - CHECK_CMD = 'monit summary -B' + CHECK_CMD = ['monit', 'summary', '-B'] MIN_CHECK_CMD_LINES = 3 # Expect status for different system service category. @@ -168,7 +168,8 @@ def _update_container_critical_processes(self, container, critical_process_list) self.need_save_cache = True def _get_container_folder(self, container): - container_folder = utils.run_command(ServiceChecker.GET_CONTAINER_FOLDER_CMD.format(container)) + ServiceChecker.GET_CONTAINER_FOLDER_CMD[2] = str(container) + container_folder = utils.run_command(ServiceChecker.GET_CONTAINER_FOLDER_CMD) if container_folder is None: return container_folder @@ -327,7 +328,7 @@ def check_process_existence(self, container_name, critical_process_list, config, # We are using supervisorctl status to check the critical process status. We cannot leverage psutil here because # it not always possible to get process cmdline in supervisor.conf. E.g, cmdline of orchagent is "/usr/bin/orchagent", # however, in supervisor.conf it is "/usr/bin/orchagent.sh" - cmd = 'docker exec {} bash -c "supervisorctl status"'.format(container_name) + cmd = ['docker', 'exec', str(container_name), 'bash', '-c', "supervisorctl status"] process_status = utils.run_command(cmd) if process_status is None: for process_name in critical_process_list: diff --git a/src/system-health/health_checker/sysmonitor.py b/src/system-health/health_checker/sysmonitor.py index e69d289fc537..e4dbc68ebf1d 100755 --- a/src/system-health/health_checker/sysmonitor.py +++ b/src/system-health/health_checker/sysmonitor.py @@ -235,7 +235,7 @@ def get_app_ready_status(self, service): #Gets the service properties def run_systemctl_show(self, service): - command = ('systemctl show {} --property=Id,LoadState,UnitFileState,Type,ActiveState,SubState,Result'.format(service)) + command = ['systemctl', 'show', str(service), '--property=Id,LoadState,UnitFileState,Type,ActiveState,SubState,Result'] output = utils.run_command(command) srv_properties = output.split('\n') prop_dict = {} diff --git a/src/system-health/health_checker/utils.py b/src/system-health/health_checker/utils.py index 00e7754e1ec2..338ef1d3afe5 100644 --- a/src/system-health/health_checker/utils.py +++ b/src/system-health/health_checker/utils.py @@ -8,7 +8,7 @@ def run_command(command): :return: Output of the shell command. """ try: - process = subprocess.Popen(command, shell=True, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + process = subprocess.Popen(command, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) return process.communicate()[0] except Exception: return None diff --git a/src/system-health/tests/test_system_health.py b/src/system-health/tests/test_system_health.py index d58c69bececa..ab0a3bcb1145 100644 --- a/src/system-health/tests/test_system_health.py +++ b/src/system-health/tests/test_system_health.py @@ -504,10 +504,10 @@ def test_manager(mock_hw_info, mock_service_info, mock_udc_info): manager._set_system_led(chassis, manager.config, 'normal') def test_utils(): - output = utils.run_command('some invalid command') + output = utils.run_command(['some', 'invalid', 'command']) assert not output - output = utils.run_command('ls') + output = utils.run_command(['ls']) assert output