Commit 36117c2b authored by Iustin Pop's avatar Iustin Pop
Browse files

Modify utils.RunCmd to write output to file

Currently we launch processes via the shell in a few places only to
redirect standard output and error to a log file ("&> $file"). It is
better to do such redirection from within RunCmd itself.

This patch splits RunCmd in two parts, the setup and the execution part,
the latter being implemented in two different functions depending on
whether we write to a file or not.

We also update the unittests with this new case.

Reviewed-by: imsnah
parent f2a6fc9e
......@@ -103,18 +103,22 @@ class RunResult(object):
output = property(_GetOutput, None, None, "Return full output")
def RunCmd(cmd, env=None):
def RunCmd(cmd, env=None, output=None):
"""Execute a (shell) command.
The command should not read from its standard input, as it will be
closed.
@param cmd: Command to run
@type cmd: string or list
@param env: Additional environment
@param cmd: Command to run
@type env: dict
@keyword env: Additional environment
@type output: str
@keyword output: if desired, the output of the command can be
saved in a file instead of the RunResult instance; this
parameter denotes the file name (if not None)
@rtype: L{RunResult}
@return: `RunResult` instance
@rtype: RunResult
"""
if no_fork:
......@@ -134,12 +138,40 @@ def RunCmd(cmd, env=None):
if env is not None:
cmd_env.update(env)
if output is None:
out, err, status = _RunCmdPipe(cmd, cmd_env, shell)
else:
status = _RunCmdFile(cmd, cmd_env, shell, output)
out = err = ""
if status >= 0:
exitcode = status
signal_ = None
else:
exitcode = None
signal_ = -status
return RunResult(exitcode, signal_, out, err, strcmd)
def _RunCmdPipe(cmd, env, via_shell):
"""Run a command and return its output.
@type cmd: string or list
@param cmd: Command to run
@type env: dict
@param env: The environment to use
@type via_shell: bool
@param via_shell: if we should run via the shell
@rtype: tuple
@return: (out, err, status)
"""
poller = select.poll()
child = subprocess.Popen(cmd, shell=shell,
child = subprocess.Popen(cmd, shell=via_shell,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
stdin=subprocess.PIPE,
close_fds=True, env=cmd_env)
close_fds=True, env=env)
child.stdin.close()
poller.register(child.stdout, select.POLLIN)
......@@ -173,14 +205,37 @@ def RunCmd(cmd, env=None):
err = err.getvalue()
status = child.wait()
if status >= 0:
exitcode = status
signal_ = None
else:
exitcode = None
signal_ = -status
return out, err, status
return RunResult(exitcode, signal_, out, err, strcmd)
def _RunCmdFile(cmd, env, via_shell, output):
"""Run a command and save its output to a file.
@type cmd: string or list
@param cmd: Command to run
@type env: dict
@param env: The environment to use
@type via_shell: bool
@param via_shell: if we should run via the shell
@type output: str
@param output: the filename in which to save the output
@rtype: int
@return: the exit status
"""
fh = open(output, "a")
try:
child = subprocess.Popen(cmd, shell=via_shell,
stderr=subprocess.STDOUT,
stdout=fh,
stdin=subprocess.PIPE,
close_fds=True, env=env)
child.stdin.close()
status = child.wait()
finally:
fh.close()
return status
def RemoveFile(filename):
......
......@@ -150,47 +150,63 @@ class TestPidFileFunctions(unittest.TestCase):
os.rmdir(self.dir)
class TestRunCmd(unittest.TestCase):
class TestRunCmd(testutils.GanetiTestCase):
"""Testing case for the RunCmd function"""
def setUp(self):
self.magic = time.ctime() + " ganeti test"
fh, self.fname = tempfile.mkstemp()
os.close(fh)
def tearDown(self):
if self.fname:
utils.RemoveFile(self.fname)
def testOk(self):
"""Test successful exit code"""
result = RunCmd("/bin/sh -c 'exit 0'")
self.assertEqual(result.exit_code, 0)
self.assertEqual(result.output, "")
def testFail(self):
"""Test fail exit code"""
result = RunCmd("/bin/sh -c 'exit 1'")
self.assertEqual(result.exit_code, 1)
self.assertEqual(result.output, "")
def testStdout(self):
"""Test standard output"""
cmd = 'echo -n "%s"' % self.magic
result = RunCmd("/bin/sh -c '%s'" % cmd)
self.assertEqual(result.stdout, self.magic)
result = RunCmd("/bin/sh -c '%s'" % cmd, output=self.fname)
self.assertEqual(result.output, "")
self.assertFileContent(self.fname, self.magic)
def testStderr(self):
"""Test standard error"""
cmd = 'echo -n "%s"' % self.magic
result = RunCmd("/bin/sh -c '%s' 1>&2" % cmd)
self.assertEqual(result.stderr, self.magic)
result = RunCmd("/bin/sh -c '%s' 1>&2" % cmd, output=self.fname)
self.assertEqual(result.output, "")
self.assertFileContent(self.fname, self.magic)
def testCombined(self):
"""Test combined output"""
cmd = 'echo -n "A%s"; echo -n "B%s" 1>&2' % (self.magic, self.magic)
expected = "A" + self.magic + "B" + self.magic
result = RunCmd("/bin/sh -c '%s'" % cmd)
self.assertEqual(result.output, "A" + self.magic + "B" + self.magic)
self.assertEqual(result.output, expected)
result = RunCmd("/bin/sh -c '%s'" % cmd, output=self.fname)
self.assertEqual(result.output, "")
self.assertFileContent(self.fname, expected)
def testSignal(self):
"""Test signal"""
result = RunCmd(["python", "-c", "import os; os.kill(os.getpid(), 15)"])
self.assertEqual(result.signal, 15)
self.assertEqual(result.output, "")
def testListRun(self):
"""Test list runs"""
......@@ -205,6 +221,13 @@ class TestRunCmd(unittest.TestCase):
self.assertEqual(result.exit_code, 0)
self.assertEqual(result.stdout, self.magic)
def testFileEmptyOutput(self):
"""Test file output"""
result = RunCmd(["true"], output=self.fname)
self.assertEqual(result.signal, None)
self.assertEqual(result.exit_code, 0)
self.assertFileContent(self.fname, "")
def testLang(self):
"""Test locale environment"""
old_env = os.environ.copy()
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment