Revert "[system-health] Remove subprocess with shell=True (#12572)" (#13505)

This reverts commit b3a8167968.
Due to issue https://github.com/sonic-net/sonic-buildimage/issues/13432
This commit is contained in:
Mai Bui 2023-01-25 16:41:08 -05:00 committed by mssonicbld
parent aea96da04d
commit eeb3ae17a6
4 changed files with 9 additions and 10 deletions

View File

@ -26,13 +26,13 @@ class ServiceChecker(HealthChecker):
CRITICAL_PROCESSES_PATH = 'etc/supervisor/critical_processes' CRITICAL_PROCESSES_PATH = 'etc/supervisor/critical_processes'
# Command to get merged directory of a container # 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. # 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. # Command to get summary of critical system service.
CHECK_CMD = ['monit', 'summary', '-B'] CHECK_CMD = 'monit summary -B'
MIN_CHECK_CMD_LINES = 3 MIN_CHECK_CMD_LINES = 3
# Expect status for different system service category. # Expect status for different system service category.
@ -186,8 +186,7 @@ class ServiceChecker(HealthChecker):
self.need_save_cache = True self.need_save_cache = True
def _get_container_folder(self, container): def _get_container_folder(self, container):
ServiceChecker.GET_CONTAINER_FOLDER_CMD[2] = str(container) container_folder = utils.run_command(ServiceChecker.GET_CONTAINER_FOLDER_CMD.format(container))
container_folder = utils.run_command(ServiceChecker.GET_CONTAINER_FOLDER_CMD)
if container_folder is None: if container_folder is None:
return container_folder return container_folder
@ -353,7 +352,7 @@ class ServiceChecker(HealthChecker):
# We are using supervisorctl status to check the critical process status. We cannot leverage psutil here because # 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", # 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" # however, in supervisor.conf it is "/usr/bin/orchagent.sh"
cmd = ['docker', 'exec', str(container_name), 'bash', '-c', "supervisorctl status"] cmd = 'docker exec {} bash -c "supervisorctl status"'.format(container_name)
process_status = utils.run_command(cmd) process_status = utils.run_command(cmd)
if process_status is None: if process_status is None:
for process_name in critical_process_list: for process_name in critical_process_list:

View File

@ -234,7 +234,7 @@ class Sysmonitor(ProcessTaskBase):
#Gets the service properties #Gets the service properties
def run_systemctl_show(self, service): def run_systemctl_show(self, service):
command = ['systemctl', 'show', str(service), '--property=Id,LoadState,UnitFileState,Type,ActiveState,SubState,Result'] command = ('systemctl show {} --property=Id,LoadState,UnitFileState,Type,ActiveState,SubState,Result'.format(service))
output = utils.run_command(command) output = utils.run_command(command)
srv_properties = output.split('\n') srv_properties = output.split('\n')
prop_dict = {} prop_dict = {}

View File

@ -8,7 +8,7 @@ def run_command(command):
:return: Output of the shell command. :return: Output of the shell command.
""" """
try: try:
process = subprocess.Popen(command, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) process = subprocess.Popen(command, shell=True, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
return process.communicate()[0] return process.communicate()[0]
except Exception: except Exception:
return None return None

View File

@ -536,10 +536,10 @@ def test_manager(mock_hw_info, mock_service_info, mock_udc_info):
manager._set_system_led(chassis, manager.config, 'normal') manager._set_system_led(chassis, manager.config, 'normal')
def test_utils(): def test_utils():
output = utils.run_command(['some', 'invalid', 'command']) output = utils.run_command('some invalid command')
assert not output assert not output
output = utils.run_command(['ls']) output = utils.run_command('ls')
assert output assert output