Commit d9f311d7 authored by Iustin Pop's avatar Iustin Pop
Browse files

Change IsPidFileAlive into ReadPidFile

We already have a function to test if a PID is alive, so it makes more
sense to use function composition that force calling (since we need to
read PIDs from files in other places too). Now IsProcessAlive returns
False for PIDs <= 0, since this is the error return from ReadPidFile.

The patch also adds a unittest for checking that WriteFile raises the
correct exception, and checks that an invalid or missing file causes
ReadPidFile to return zero. The unittest tearDown method will try to
cleanup the temp directory too (otherwise it leaves stuff after it).

Reviewed-by: ultrotter
parent f71245a0
......@@ -268,9 +268,13 @@ def IsProcessAlive(pid):
Returns: true or false, depending on if the pid exists or not
Remarks: zombie processes treated as not alive
Remarks: zombie processes treated as not alive, and giving a pid <=
0 makes the function to return False.
"""
if pid <= 0:
return False
try:
f = open("/proc/%d/status" % pid)
except IOError, err:
......@@ -290,29 +294,30 @@ def IsProcessAlive(pid):
return alive
def IsPidFileAlive(pidfile):
"""Check whether the given pidfile points to a live process.
def ReadPidFile(pidfile):
"""Read the pid from a file.
@param pidfile: Path to a file containing the pid to be checked
@type pidfile: string (filename)
@param pidfile: Path to a file containing the pid to be checked
@type pidfile: string (filename)
@return: The process id, if the file exista and contains a valid PID,
otherwise 0
@rtype: int
"""
try:
pf = open(pidfile, 'r')
except EnvironmentError, open_err:
if open_err.errno == errno.ENOENT:
return False
else:
raise errors.GenericError("Cannot open file %s. Error: %s" %
(pidfile, str(open_err)))
except EnvironmentError, err:
if err.errno != errno.ENOENT:
logging.exception("Can't read pid file?!")
return 0
try:
pid = int(pf.read())
except ValueError:
raise errors.GenericError("Invalid pid string in %s" %
(pidfile,))
except ValueError, err:
logging.info("Can't parse pid file contents", exc_info=err)
return 0
return IsProcessAlive(pid)
return pid
def MatchNameComponent(key, name_list):
......@@ -1066,7 +1071,7 @@ def WritePidFile(name):
"""
pid = os.getpid()
pidfilename = _DaemonPidFileName(name)
if IsPidFileAlive(pidfilename):
if IsProcessAlive(ReadPidFile(pidfilename)):
raise errors.GenericError("%s contains a live process" % pidfilename)
WriteFile(pidfilename, data="%d\n" % pid)
......
......@@ -43,12 +43,13 @@ from ganeti.utils import IsProcessAlive, RunCmd, \
ParseUnit, AddAuthorizedKey, RemoveAuthorizedKey, \
ShellQuote, ShellQuoteArgs, TcpPing, ListVisibleFiles, \
SetEtcHostsEntry, RemoveEtcHostsEntry, FirstFree
from ganeti.errors import LockError, UnitParseError
from ganeti.errors import LockError, UnitParseError, GenericError
def _ChildHandler(signal, stack):
global _ChildFlag
_ChildFlag = True
class TestIsProcessAlive(unittest.TestCase):
"""Testing case for IsProcessAlive"""
......@@ -99,8 +100,9 @@ class TestIsProcessAlive(unittest.TestCase):
self.assert_(not IsProcessAlive(self.pid_non_existing),
"noexisting process detected")
class TestPidFileFunctions(unittest.TestCase):
"""Tests for WritePidFile, RemovePidFile and IsPidFileAlive"""
"""Tests for WritePidFile, RemovePidFile and ReadPidFile"""
def setUp(self):
self.dir = tempfile.mkdtemp()
......@@ -108,13 +110,31 @@ class TestPidFileFunctions(unittest.TestCase):
utils._DaemonPidFileName = self.f_dpn
def testPidFileFunctions(self):
pid_file = self.f_dpn('test')
utils.WritePidFile('test')
self.assert_(os.path.exists(self.f_dpn('test')))
self.assert_(utils.IsPidFileAlive(self.f_dpn('test')))
self.failUnless(os.path.exists(pid_file),
"PID file should have been created")
read_pid = utils.ReadPidFile(pid_file)
self.failUnlessEqual(read_pid, os.getpid())
self.failUnless(utils.IsProcessAlive(read_pid))
self.failUnlessRaises(GenericError, utils.WritePidFile, 'test')
utils.RemovePidFile('test')
self.failIf(os.path.exists(pid_file),
"PID file should not exist anymore")
self.failUnlessEqual(utils.ReadPidFile(pid_file), 0,
"ReadPidFile should return 0 for missing pid file")
fh = open(pid_file, "w")
fh.write("blah\n")
fh.close()
self.failUnlessEqual(utils.ReadPidFile(pid_file), 0,
"ReadPidFile should return 0 for invalid pid file")
utils.RemovePidFile('test')
self.assert_(not os.path.exists(self.f_dpn('test')))
self.failIf(os.path.exists(pid_file),
"PID file should not exist anymore")
def tearDown(self):
for name in os.listdir(self.dir):
os.unlink(os.path.join(self.dir, name))
os.rmdir(self.dir)
......
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