Skip to content

Commit eeb3ae1

Browse files
maipbuimssonicbld
authored andcommitted
Revert "[system-health] Remove subprocess with shell=True (#12572)" (#13505)
This reverts commit b3a8167. Due to issue #13432
1 parent aea96da commit eeb3ae1

File tree

4 files changed

+9
-10
lines changed

4 files changed

+9
-10
lines changed

src/system-health/health_checker/service_checker.py

+5-6
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ class ServiceChecker(HealthChecker):
2626
CRITICAL_PROCESSES_PATH = 'etc/supervisor/critical_processes'
2727

2828
# Command to get merged directory of a container
29-
GET_CONTAINER_FOLDER_CMD = ['docker', 'inspect', '', '--format', "{{.GraphDriver.Data.MergedDir}}"]
29+
GET_CONTAINER_FOLDER_CMD = 'docker inspect {} --format "{{{{.GraphDriver.Data.MergedDir}}}}"'
3030

3131
# Command to query the status of monit service.
32-
CHECK_MONIT_SERVICE_CMD = ['systemctl', 'is-active', 'monit.service']
32+
CHECK_MONIT_SERVICE_CMD = 'systemctl is-active monit.service'
3333

3434
# Command to get summary of critical system service.
35-
CHECK_CMD = ['monit', 'summary', '-B']
35+
CHECK_CMD = 'monit summary -B'
3636
MIN_CHECK_CMD_LINES = 3
3737

3838
# Expect status for different system service category.
@@ -186,8 +186,7 @@ def _update_container_critical_processes(self, container, critical_process_list)
186186
self.need_save_cache = True
187187

188188
def _get_container_folder(self, container):
189-
ServiceChecker.GET_CONTAINER_FOLDER_CMD[2] = str(container)
190-
container_folder = utils.run_command(ServiceChecker.GET_CONTAINER_FOLDER_CMD)
189+
container_folder = utils.run_command(ServiceChecker.GET_CONTAINER_FOLDER_CMD.format(container))
191190
if container_folder is None:
192191
return container_folder
193192

@@ -353,7 +352,7 @@ def check_process_existence(self, container_name, critical_process_list, config,
353352
# We are using supervisorctl status to check the critical process status. We cannot leverage psutil here because
354353
# it not always possible to get process cmdline in supervisor.conf. E.g, cmdline of orchagent is "/usr/bin/orchagent",
355354
# however, in supervisor.conf it is "/usr/bin/orchagent.sh"
356-
cmd = ['docker', 'exec', str(container_name), 'bash', '-c', "supervisorctl status"]
355+
cmd = 'docker exec {} bash -c "supervisorctl status"'.format(container_name)
357356
process_status = utils.run_command(cmd)
358357
if process_status is None:
359358
for process_name in critical_process_list:

src/system-health/health_checker/sysmonitor.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ def get_app_ready_status(self, service):
234234

235235
#Gets the service properties
236236
def run_systemctl_show(self, service):
237-
command = ['systemctl', 'show', str(service), '--property=Id,LoadState,UnitFileState,Type,ActiveState,SubState,Result']
237+
command = ('systemctl show {} --property=Id,LoadState,UnitFileState,Type,ActiveState,SubState,Result'.format(service))
238238
output = utils.run_command(command)
239239
srv_properties = output.split('\n')
240240
prop_dict = {}

src/system-health/health_checker/utils.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ def run_command(command):
88
:return: Output of the shell command.
99
"""
1010
try:
11-
process = subprocess.Popen(command, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
11+
process = subprocess.Popen(command, shell=True, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
1212
return process.communicate()[0]
1313
except Exception:
1414
return None

src/system-health/tests/test_system_health.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -536,10 +536,10 @@ def test_manager(mock_hw_info, mock_service_info, mock_udc_info):
536536
manager._set_system_led(chassis, manager.config, 'normal')
537537

538538
def test_utils():
539-
output = utils.run_command(['some', 'invalid', 'command'])
539+
output = utils.run_command('some invalid command')
540540
assert not output
541541

542-
output = utils.run_command(['ls'])
542+
output = utils.run_command('ls')
543543
assert output
544544

545545

0 commit comments

Comments
 (0)