gluster / gluster-health-report

Gluster Health Report Tool
GNU Lesser General Public License v3.0
23 stars 20 forks source link

coredump-report: Fix OSError for ulimit command #17

Closed pdvian closed 6 years ago

pdvian commented 6 years ago

The ulimit needs to be executed through shell but passing cmd as list preventing it to be executed and getting OSError. The command_output func sets shell to True in popen call if cmd is a string object.

Signed-off-by: Prashant D pdhange@redhat.com

pdvian commented 6 years ago

Getting below error: [2017-10-09 10:45:11.95014] E [main:191:main] : Report failure report=report_coredump Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/gluster_health_report-0.1-py2.7.egg/glusterhealth/main.py", line 188, in main func(context) File "/usr/lib/python2.7/site-packages/gluster_health_report-0.1-py2.7.egg/glusterhealth/reports/coredump.py", line 18, in report_coredump out = command_output(cmd) File "/usr/lib/python2.7/site-packages/gluster_health_report-0.1-py2.7.egg/glusterhealth/reports/utils.py", line 23, in command_output p = Popen(cmd, stdout=PIPE, stderr=PIPE, shell=shell) File "/usr/lib64/python2.7/subprocess.py", line 711, in init errread, errwrite) File "/usr/lib64/python2.7/subprocess.py", line 1327, in _execute_child raise child_exception OSError: [Errno 2] No such file or directory

pdvian commented 6 years ago

Should we also set "shell = True" if cmd is list object ? or accept cmd as str object only ?

aravindavk commented 6 years ago

Should we also set "shell = True" if cmd is list object ? or accept cmd as str object only ?

command_output is already doing that. If input is list then shell=False, else True

pdvian commented 6 years ago

Yes, then this patch makes sense. What say ?

aravindavk commented 6 years ago

Yes, then this patch makes sense. What say ?

It should work for both the approach, I am wondering why it is not working in your setup. May be PATH is different?

@prashanthpai any idea why this is failing?

prashanthpai commented 6 years ago

@aravindavk Not sure. It's recommended that the args be a list when shell = False and command_output() already takes care of it. This works fine for me.

pdvian commented 6 years ago

@prashanthpai @aravindavk with current master branch, I am having this error on RHEL7.3 node. after comparing the strace logs in str and list object case, it seems with list object in Popen call python could not find ulimit (ulimit is part of bash binary). Popen call with list object: 27770 execve("/usr/bin/python", ["python", "ulimit.py"], [/ 26 vars /]) = 0 27770 readlink("ulimit.py", 0x7ffed3acb350, 4096) = -1 EINVAL (Invalid argument) 27770 lstat("/root/ulimit.py", {st_mode=S_IFREG|0644, st_size=386, ...}) = 0 27770 stat("ulimit.py", {st_mode=S_IFREG|0644, st_size=386, ...}) = 0 27770 open("ulimit.py", O_RDONLY) = 3 27770 stat("ulimit.py", {st_mode=S_IFREG|0644, st_size=386, ...}) = 0 27770 open("ulimit.py", O_RDONLY) = 3 27771 execve("/usr/local/sbin/ulimit", ["ulimit", "-c"], [/ 26 vars /]) = -1 ENOENT (No such file or directory) 27771 execve("/usr/local/bin/ulimit", ["ulimit", "-c"], [/ 26 vars /]) = -1 ENOENT (No such file or directory) 27771 execve("/usr/sbin/ulimit", ["ulimit", "-c"], [/ 26 vars /]) = -1 ENOENT (No such file or directory) 27771 execve("/usr/bin/ulimit", ["ulimit", "-c"], [/ 26 vars /]) = -1 ENOENT (No such file or directory) 27771 execve("/root/bin/ulimit", ["ulimit", "-c"], [/ 26 vars /]) = -1 ENOENT (No such file or directory) 27770 write(2, " File \"ulimit.py\", line 14, in "..., 41) = 41 27770 open("ulimit.py", O_RDONLY) = 4 27770 write(2, " File \"ulimit.py\", line 5, in c"..., 46) = 46 27770 open("ulimit.py", O_RDONLY) = 4

Popen call with str object: 27765 execve("/usr/bin/python", ["python", "ulimit.py"], [/ 26 vars /]) = 0 27765 readlink("ulimit.py", 0x7ffd740d9b00, 4096) = -1 EINVAL (Invalid argument) 27765 lstat("/root/ulimit.py", {st_mode=S_IFREG|0644, st_size=386, ...}) = 0 27765 stat("ulimit.py", {st_mode=S_IFREG|0644, st_size=386, ...}) = 0 27765 open("ulimit.py", O_RDONLY) = 3 27765 stat("ulimit.py", {st_mode=S_IFREG|0644, st_size=386, ...}) = 0 27765 open("ulimit.py", O_RDONLY) = 3 27766 execve("/bin/sh", ["/bin/sh", "-c", "ulimit -c"], [/ 26 vars /] <unfinished ...>

So in case of ulimit command, the behavior is bit different here. @prashanthpai which system are you using ? Can you check the strace logs for below python script on your system:

from subprocess import Popen, PIPE

def command_output(cmd): shell = True if isinstance(cmd, str) else False p = Popen(cmd, stdout=PIPE, stderr=PIPE, shell=shell) out, err = p.communicate() if p.returncode != 0: print "command failed" return out

if name =='main': cmd=["ulimit","-c"]

cmd="ulimit -c"

print "out: ", command_output(cmd)
aravindavk commented 6 years ago

I will merge this since both approaches should work. and string approach is working even if any PATH issues.