Merge branch 'stable-2.8' into stable-2.9

* stable-2.8
  Add additional tests for utils.Retry
  Make retry tests independent of actual time
  Fix corner-case in handling of remaining retry time
  Perform proper cleanup on termination of Haskell daemons
Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

lib/utils/retry.py

@@ -170,11 +170,11 @@ def Retry(fn, delay, timeout, args=None, wait_fn=time.sleep,
 
     remaining_time = end_time - _time_fn()
 
-    if remaining_time < 0.0:
+    if remaining_time <= 0.0:
       # pylint: disable=W0142
       raise RetryTimeout(*retry_args)
 
-    assert remaining_time >= 0.0
+    assert remaining_time > 0.0
 
     if calc_delay is None:
       wait_fn(remaining_time)

src/Ganeti/Daemon.hs

@@ -45,6 +45,7 @@ module Ganeti.Daemon
   , genericMain
   ) where
 
+import Control.Concurrent
 import Control.Exception
 import Control.Monad
 import Data.Maybe (fromMaybe, listToMaybe)
@@ -53,6 +54,7 @@ import GHC.IO.Handle (hDuplicateTo)
 import Network.BSD (getHostName)
 import qualified Network.Socket as Socket
 import System.Console.GetOpt
+import System.Directory
 import System.Exit
 import System.Environment
 import System.IO
@@ -235,6 +237,19 @@ setupDaemonEnv cwd umask = do
   _ <- createSession
   return ()
 
+-- | Cleanup function, performing all the operations that need to be done prior
+-- to shutting down a daemon.
+finalCleanup :: FilePath -> IO ()
+finalCleanup = removeFile
+
+-- | Signal handler for the termination signal.
+handleSigTerm :: ThreadId -> IO ()
+handleSigTerm mainTID =
+  -- Throw termination exception to the main thread, so that the daemon is
+  -- actually stopped in the proper way, executing all the functions waiting on
+  -- "finally" statement.
+  Control.Exception.throwTo mainTID ExitSuccess
+
 -- | Signal handler for reopening log files.
 handleSigHup :: FilePath -> IO ()
 handleSigHup path = do
@@ -404,7 +419,7 @@ fullPrep :: GanetiDaemon  -- ^ The daemon we're running
          -> SyslogUsage   -- ^ Syslog mode
          -> a             -- ^ Check results
          -> PrepFn a b    -- ^ Prepare function
-         -> IO b
+         -> IO (FilePath, b)
 fullPrep daemon opts syslog check_result prep_fn = do
   logfile <- if optDaemonize opts
                then return Nothing
@@ -415,7 +430,10 @@ fullPrep daemon opts syslog check_result prep_fn = do
   _ <- describeError "writing PID file; already locked?"
          Nothing (Just pidfile) $ writePidFile pidfile
   logNotice $ dname ++ " daemon startup"
-  prep_fn opts check_result
+  prep_res <- prep_fn opts check_result
+  tid <- myThreadId
+  _ <- installHandler sigTERM (Catch $ handleSigTerm tid) Nothing
+  return (pidfile, prep_res)
 
 -- | Inner daemon function.
 --
@@ -429,11 +447,11 @@ innerMain :: GanetiDaemon  -- ^ The daemon we're running
           -> Maybe Fd      -- ^ Error reporting function
           -> IO ()
 innerMain daemon opts syslog check_result prep_fn exec_fn fd = do
-  prep_result <- fullPrep daemon opts syslog check_result prep_fn
+  (pidFile, prep_result) <- fullPrep daemon opts syslog check_result prep_fn
                  `Control.Exception.catch` handlePrepErr True fd
   -- no error reported, we should now close the fd
   maybeCloseFd fd
-  exec_fn opts check_result prep_result
+  finally (exec_fn opts check_result prep_result) (finalCleanup pidFile)
 
 -- | Daemon prepare error handling function.
 handlePrepErr :: Bool -> Maybe Fd -> IOError -> IO a

test/py/ganeti.utils.retry_unittest.py

@@ -35,6 +35,16 @@ class TestRetry(testutils.GanetiTestCase):
     testutils.GanetiTestCase.setUp(self)
     self.retries = 0
     self.called = 0
+    self.time = 1379601882.0
+    self.time_for_time_fn = 0
+    self.time_for_retry_and_succeed = 0
+
+  def _time_fn(self):
+    self.time += self.time_for_time_fn
+    return self.time
+
+  def _wait_fn(self, delay):
+    self.time += delay
 
   @staticmethod
   def _RaiseRetryAgain():
@@ -48,6 +58,7 @@ class TestRetry(testutils.GanetiTestCase):
     return utils.Retry(self._RaiseRetryAgain, 0.01, 0.02)
 
   def _RetryAndSucceed(self, retries):
+    self.time += self.time_for_retry_and_succeed
     if self.retries < retries:
       self.retries += 1
       raise utils.RetryAgain()
@@ -64,39 +75,77 @@ class TestRetry(testutils.GanetiTestCase):
 
   def testRaiseTimeout(self):
     self.failUnlessRaises(utils.RetryTimeout, utils.Retry,
-                          self._RaiseRetryAgain, 0.01, 0.02)
+                          self._RaiseRetryAgain, 0.01, 0.02,
+                          wait_fn = self._wait_fn, _time_fn = self._time_fn)
     self.failUnlessRaises(utils.RetryTimeout, utils.Retry,
-                          self._RetryAndSucceed, 0.01, 0, args=[1])
+                          self._RetryAndSucceed, 0.01, 0, args=[1],
+                          wait_fn = self._wait_fn, _time_fn = self._time_fn)
     self.failUnlessEqual(self.retries, 1)
 
   def testComplete(self):
-    self.failUnlessEqual(utils.Retry(lambda: True, 0, 1), True)
-    self.failUnlessEqual(utils.Retry(self._RetryAndSucceed, 0, 1, args=[2]),
+    self.failUnlessEqual(utils.Retry(lambda: True, 0, 1,
+                                     wait_fn = self._wait_fn,
+                                     _time_fn = self._time_fn),
+                         True)
+    self.failUnlessEqual(utils.Retry(self._RetryAndSucceed, 0, 1, args=[2],
+                                     wait_fn = self._wait_fn,
+                                     _time_fn = self._time_fn),
+                         True)
+    self.failUnlessEqual(self.retries, 2)
+
+  def testCompleteNontrivialTimes(self):
+    self.time_for_time_fn = 0.01
+    self.time_for_retry_and_succeed = 0.1
+    self.failUnlessEqual(utils.Retry(self._RetryAndSucceed, 0, 1, args=[2],
+                                     wait_fn = self._wait_fn,
+                                     _time_fn = self._time_fn),
                          True)
     self.failUnlessEqual(self.retries, 2)
 
   def testNestedLoop(self):
     try:
       self.failUnlessRaises(errors.ProgrammerError, utils.Retry,
-                            self._WrongNestedLoop, 0, 1)
+                            self._WrongNestedLoop, 0, 1,
+                            wait_fn = self._wait_fn, _time_fn = self._time_fn)
     except utils.RetryTimeout:
       self.fail("Didn't detect inner loop's exception")
 
   def testTimeoutArgument(self):
     retry_arg="my_important_debugging_message"
     try:
-      utils.Retry(self._RaiseRetryAgainWithArg, 0.01, 0.02, args=[[retry_arg]])
+      utils.Retry(self._RaiseRetryAgainWithArg, 0.01, 0.02, args=[[retry_arg]],
+                  wait_fn = self._wait_fn, _time_fn = self._time_fn)
     except utils.RetryTimeout, err:
       self.failUnlessEqual(err.args, (retry_arg, ))
     else:
       self.fail("Expected timeout didn't happen")
 
+  def testTimeout(self):
+    self.time_for_time_fn = 0.01
+    self.time_for_retry_and_succeed = 10
+    try:
+      utils.Retry(self._RetryAndSucceed, 1, 18, args=[2],
+                  wait_fn = self._wait_fn, _time_fn = self._time_fn)
+    except utils.RetryTimeout, err:
+      self.failUnlessEqual(err.args, ())
+    else:
+      self.fail("Expected timeout didn't happen")
+
+  def testNoTimeout(self):
+    self.time_for_time_fn = 0.01
+    self.time_for_retry_and_succeed = 8
+    self.failUnlessEqual(
+      utils.Retry(self._RetryAndSucceed, 1, 18, args=[2],
+                  wait_fn = self._wait_fn, _time_fn = self._time_fn),
+      True)
+
   def testRaiseInnerWithExc(self):
     retry_arg="my_important_debugging_message"
     try:
       try:
         utils.Retry(self._RaiseRetryAgainWithArg, 0.01, 0.02,
-                    args=[[errors.GenericError(retry_arg, retry_arg)]])
+                    args=[[errors.GenericError(retry_arg, retry_arg)]],
+                    wait_fn = self._wait_fn, _time_fn = self._time_fn)
       except utils.RetryTimeout, err:
         err.RaiseInner()
       else:
@@ -111,7 +160,8 @@ class TestRetry(testutils.GanetiTestCase):
     try:
       try:
         utils.Retry(self._RaiseRetryAgainWithArg, 0.01, 0.02,
-                    args=[[retry_arg, retry_arg]])
+                    args=[[retry_arg, retry_arg]],
+                    wait_fn = self._wait_fn, _time_fn = self._time_fn)
       except utils.RetryTimeout, err:
         err.RaiseInner()
       else:
@@ -122,17 +172,27 @@ class TestRetry(testutils.GanetiTestCase):
       self.fail("Expected RetryTimeout didn't happen")
 
   def testSimpleRetry(self):
-    self.assertFalse(utils.SimpleRetry(True, lambda: False, 0.01, 0.02))
-    self.assertFalse(utils.SimpleRetry(lambda x: x, lambda: False, 0.01, 0.02))
-    self.assertTrue(utils.SimpleRetry(True, lambda: True, 0, 1))
-    self.assertTrue(utils.SimpleRetry(lambda x: x, lambda: True, 0, 1))
-    self.assertTrue(utils.SimpleRetry(True, self._SimpleRetryAndSucceed,
-                                      0, 1, args=[1]))
+    self.assertFalse(utils.SimpleRetry(True, lambda: False, 0.01, 0.02,
+                                       wait_fn = self._wait_fn,
+                                       _time_fn = self._time_fn))
+    self.assertFalse(utils.SimpleRetry(lambda x: x, lambda: False, 0.01, 0.02,
+                                       wait_fn = self._wait_fn,
+                                       _time_fn = self._time_fn))
+    self.assertTrue(utils.SimpleRetry(True, lambda: True, 0, 1,
+                                      wait_fn = self._wait_fn,
+                                      _time_fn = self._time_fn))
+    self.assertTrue(utils.SimpleRetry(lambda x: x, lambda: True, 0, 1,
+                                      wait_fn = self._wait_fn,
+                                      _time_fn = self._time_fn))
+    self.assertTrue(utils.SimpleRetry(True, self._SimpleRetryAndSucceed, 0, 1,
+                                      args=[1], wait_fn = self._wait_fn,
+                                      _time_fn = self._time_fn))
     self.assertEqual(self.retries, 1)
     self.assertEqual(self.called, 2)
     self.called = self.retries = 0
-    self.assertTrue(utils.SimpleRetry(True, self._SimpleRetryAndSucceed,
-                                      0, 1, args=[2]))
+    self.assertTrue(utils.SimpleRetry(True, self._SimpleRetryAndSucceed, 0, 1,
+                                      args=[2], wait_fn = self._wait_fn,
+                                      _time_fn = self._time_fn))
     self.assertEqual(self.called, 3)