From 8342c325c7ea6ed3aa7f380dad78c629757a089d Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Mon, 10 Jan 2011 20:40:43 +0100
Subject: [PATCH] utils: Change RemovePidFile to take path, not name

This avoids having to monkey-patch the utils module for unittests.
Monkey patching is evil and caused a bug while moving code around.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
 lib/daemon.py                 |  2 +-
 lib/utils/__init__.py         | 13 ++++++-------
 test/ganeti.utils_unittest.py |  7 +++----
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/lib/daemon.py b/lib/daemon.py
index c3c476178..99f7dd322 100644
--- a/lib/daemon.py
+++ b/lib/daemon.py
@@ -681,4 +681,4 @@ def GenericMain(daemon_name, optionparser,
 
     exec_fn(options, args, prep_results)
   finally:
-    utils.RemovePidFile(daemon_name)
+    utils.RemovePidFile(utils.DaemonPidFileName(daemon_name))
diff --git a/lib/utils/__init__.py b/lib/utils/__init__.py
index 5e1eaf98e..06b5e87d7 100644
--- a/lib/utils/__init__.py
+++ b/lib/utils/__init__.py
@@ -2025,7 +2025,7 @@ def StopDaemon(name):
 def WritePidFile(pidfile):
   """Write the current process pidfile.
 
-  @type pidfile: sting
+  @type pidfile: string
   @param pidfile: the path to the file to be written
   @raise errors.LockError: if the pid file already exists and
       points to a live process
@@ -2049,20 +2049,19 @@ def WritePidFile(pidfile):
   return fd_pidfile
 
 
-def RemovePidFile(name):
+def RemovePidFile(pidfile):
   """Remove the current process pidfile.
 
   Any errors are ignored.
 
-  @type name: str
-  @param name: the daemon name used to derive the pidfile name
+  @type pidfile: string
+  @param pidfile: Path to the file to be removed
 
   """
-  pidfilename = DaemonPidFileName(name)
   # TODO: we could check here that the file contains our pid
   try:
-    RemoveFile(pidfilename)
-  except: # pylint: disable-msg=W0702
+    RemoveFile(pidfile)
+  except Exception: # pylint: disable-msg=W0703
     pass
 
 
diff --git a/test/ganeti.utils_unittest.py b/test/ganeti.utils_unittest.py
index 574edf906..65431f840 100755
--- a/test/ganeti.utils_unittest.py
+++ b/test/ganeti.utils_unittest.py
@@ -174,7 +174,6 @@ class TestPidFileFunctions(unittest.TestCase):
   def setUp(self):
     self.dir = tempfile.mkdtemp()
     self.f_dpn = lambda name: os.path.join(self.dir, "%s.pid" % name)
-    utils.DaemonPidFileName = self.f_dpn
 
   def testPidFileFunctions(self):
     pid_file = self.f_dpn('test')
@@ -187,7 +186,7 @@ class TestPidFileFunctions(unittest.TestCase):
     self.failUnlessRaises(errors.LockError, utils.WritePidFile,
                           self.f_dpn('test'))
     os.close(fd)
-    utils.RemovePidFile('test')
+    utils.RemovePidFile(self.f_dpn("test"))
     self.failIf(os.path.exists(pid_file),
                 "PID file should not exist anymore")
     self.failUnlessEqual(utils.ReadPidFile(pid_file), 0,
@@ -200,7 +199,7 @@ class TestPidFileFunctions(unittest.TestCase):
     # but now, even with the file existing, we should be able to lock it
     fd = utils.WritePidFile(self.f_dpn('test'))
     os.close(fd)
-    utils.RemovePidFile('test')
+    utils.RemovePidFile(self.f_dpn("test"))
     self.failIf(os.path.exists(pid_file),
                 "PID file should not exist anymore")
 
@@ -222,7 +221,7 @@ class TestPidFileFunctions(unittest.TestCase):
     self.failUnless(utils.IsProcessAlive(new_pid))
     utils.KillProcess(new_pid, waitpid=True)
     self.failIf(utils.IsProcessAlive(new_pid))
-    utils.RemovePidFile('child')
+    utils.RemovePidFile(self.f_dpn('child'))
     self.failUnlessRaises(errors.ProgrammerError, utils.KillProcess, 0)
 
   def tearDown(self):
-- 
GitLab