From d9f311d78e983c98343954c85e7415115fb22885 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Tue, 29 Jul 2008 08:49:34 +0000
Subject: [PATCH] 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
---
lib/utils.py | 37 ++++++++++++++++++++---------------
test/ganeti.utils_unittest.py | 30 +++++++++++++++++++++++-----
2 files changed, 46 insertions(+), 21 deletions(-)
diff --git a/lib/utils.py b/lib/utils.py
index a9cdfc0b8..a9b71b5fb 100644
--- a/lib/utils.py
+++ b/lib/utils.py
@@ -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)
diff --git a/test/ganeti.utils_unittest.py b/test/ganeti.utils_unittest.py
index 7a9c64f5c..23a8b6b75 100755
--- a/test/ganeti.utils_unittest.py
+++ b/test/ganeti.utils_unittest.py
@@ -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)
--
GitLab