test.ganeti.process_unittest: Fix race condition

There was a race condition on heavily loaded testsystem causing randomly
to fail the timeout unittests as the signal handler is not yet setup but
the timeout has already hit.

Therefore we introduce a workaround to wait until a program reached a
certain point (for example after signal handling setup) before we
actually go for the real run. The wait of course has a timeout as well,
but it's pretty high. If we hit the 20 seconds we have really big issues
anyway.

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

diff --git a/lib/utils/process.py b/lib/utils/process.py
index be7d774..6d57b7c 100644 (file)
--- a/lib/utils/process.py
+++ b/lib/utils/process.py
@@ -141,7 +141,8 @@ def _BuildCmdEnvironment(env, reset):
 
 
 def RunCmd(cmd, env=None, output=None, cwd="/", reset_env=False,
-           interactive=False, timeout=None, noclose_fds=None):
+           interactive=False, timeout=None, noclose_fds=None,
+           _postfork_fn=None):
   """Execute a (shell) command.
 
   The command should not read from its standard input, as it will be
@@ -169,6 +170,7 @@ def RunCmd(cmd, env=None, output=None, cwd="/", reset_env=False,
   @type noclose_fds: list
   @param noclose_fds: list of additional (fd >=3) file descriptors to leave
                       open for the child process
+  @param _postfork_fn: Callback run after fork but before timeout (unittest)
   @rtype: L{RunResult}
   @return: RunResult instance
   @raise errors.ProgrammerError: if we call this when forks are disabled
@@ -200,8 +202,11 @@ def RunCmd(cmd, env=None, output=None, cwd="/", reset_env=False,
     if output is None:
       out, err, status, timeout_action = _RunCmdPipe(cmd, cmd_env, shell, cwd,
                                                      interactive, timeout,
-                                                     noclose_fds)
+                                                     noclose_fds,
+                                                     _postfork_fn=_postfork_fn)
     else:
+      assert _postfork_fn is None, \
+          "_postfork_fn not supported if output provided"
       timeout_action = _TIMEOUT_NONE
       status = _RunCmdFile(cmd, cmd_env, shell, output, cwd, noclose_fds)
       out = err = ""
@@ -468,7 +473,8 @@ def _WaitForProcess(child, timeout):
 
 
 def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout, noclose_fds,
-                _linger_timeout=constants.CHILD_LINGER_TIMEOUT):
+                _linger_timeout=constants.CHILD_LINGER_TIMEOUT,
+                _postfork_fn=None):
   """Run a command and return its output.
 
   @type  cmd: string or list
@@ -486,6 +492,7 @@ def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout, noclose_fds,
   @type noclose_fds: list
   @param noclose_fds: list of additional (fd >=3) file descriptors to leave
                       open for the child process
+  @param _postfork_fn: Function run after fork but before timeout (unittest)
   @rtype: tuple
   @return: (out, err, status)
 
@@ -514,6 +521,9 @@ def _RunCmdPipe(cmd, env, via_shell, cwd, interactive, timeout, noclose_fds,
                            cwd=cwd,
                            preexec_fn=preexec_fn)
 
+  if _postfork_fn:
+    _postfork_fn(child.pid)
+
   out = StringIO()
   err = StringIO()
 

diff --git a/test/ganeti.utils.process_unittest.py b/test/ganeti.utils.process_unittest.py
index 39e8485..5f11422 100755 (executable)
--- a/test/ganeti.utils.process_unittest.py
+++ b/test/ganeti.utils.process_unittest.py
@@ -27,6 +27,7 @@ import shutil
 import os
 import stat
 import time
+import select
 import signal
 
 from ganeti import constants
@@ -153,6 +154,45 @@ class TestIsProcessHandlingSignal(unittest.TestCase):
     self.assert_(utils.RunInSeparateProcess(self._TestRealProcess))
 
 
+class _PostforkProcessReadyHelper:
+  """A helper to use with _postfork_fn in RunCmd.
+
+  It makes sure a process has reached a certain state by reading from a fifo.
+
+  @ivar write_fd: The fd number to write to
+
+  """
+  def __init__(self, timeout):
+    """Initialize the helper.
+
+    @param fifo_dir: The dir where we can create the fifo
+    @param timeout: The time in seconds to wait before giving up
+
+    """
+    self.timeout = timeout
+    (self.read_fd, self.write_fd) = os.pipe()
+
+  def Ready(self, pid):
+    """Waits until the process is ready.
+
+    @param pid: The pid of the process
+
+    """
+    (read_ready, _, _) = select.select([self.read_fd], [], [], self.timeout)
+
+    if not read_ready:
+      # We hit the timeout
+      raise AssertionError("Timeout %d reached while waiting for process %d"
+                           " to become ready" % (self.timeout, pid))
+
+  def Cleanup(self):
+    """Cleans up the helper.
+
+    """
+    os.close(self.read_fd)
+    os.close(self.write_fd)
+
+
 class TestRunCmd(testutils.GanetiTestCase):
   """Testing case for the RunCmd function"""
 
@@ -164,7 +204,11 @@ class TestRunCmd(testutils.GanetiTestCase):
     self.fifo_file = os.path.join(self.fifo_tmpdir, "ganeti_test_fifo")
     os.mkfifo(self.fifo_file)
 
+    # If the process is not ready after 20 seconds we have bigger issues
+    self.proc_ready_helper = _PostforkProcessReadyHelper(20)
+
   def tearDown(self):
+    self.proc_ready_helper.Cleanup()
     shutil.rmtree(self.fifo_tmpdir)
     testutils.GanetiTestCase.tearDown(self)
 
@@ -216,22 +260,31 @@ class TestRunCmd(testutils.GanetiTestCase):
     self.assertEqual(result.output, "")
 
   def testTimeoutClean(self):
-    cmd = "trap 'exit 0' TERM; read < %s" % self.fifo_file
-    result = utils.RunCmd(["/bin/sh", "-c", cmd], timeout=0.2)
+    cmd = ("trap 'exit 0' TERM; echo >&%d; read < %s" %
+           (self.proc_ready_helper.write_fd, self.fifo_file))
+    result = utils.RunCmd(["/bin/sh", "-c", cmd], timeout=0.2,
+                          noclose_fds=[self.proc_ready_helper.write_fd],
+                          _postfork_fn=self.proc_ready_helper.Ready)
     self.assertEqual(result.exit_code, 0)
 
   def testTimeoutKill(self):
-    cmd = ["/bin/sh", "-c", "trap '' TERM; read < %s" % self.fifo_file]
+    cmd = ["/bin/sh", "-c", "trap '' TERM; echo >&%d; read < %s" %
+           (self.proc_ready_helper.write_fd, self.fifo_file)]
     timeout = 0.2
     (out, err, status, ta) = \
       utils.process._RunCmdPipe(cmd, {}, False, "/", False,
-                                timeout, None, _linger_timeout=0.2)
+                                timeout, [self.proc_ready_helper.write_fd],
+                                _linger_timeout=0.2,
+                                _postfork_fn=self.proc_ready_helper.Ready)
     self.assert_(status < 0)
     self.assertEqual(-status, signal.SIGKILL)
 
   def testTimeoutOutputAfterTerm(self):
-    cmd = "trap 'echo sigtermed; exit 1' TERM; read < %s" % self.fifo_file
-    result = utils.RunCmd(["/bin/sh", "-c", cmd], timeout=0.2)
+    cmd = ("trap 'echo sigtermed; exit 1' TERM; echo >&%d; read < %s" %
+           (self.proc_ready_helper.write_fd, self.fifo_file))
+    result = utils.RunCmd(["/bin/sh", "-c", cmd], timeout=0.2,
+                          noclose_fds=[self.proc_ready_helper.write_fd],
+                          _postfork_fn=self.proc_ready_helper.Ready)
     self.assert_(result.failed)
     self.assertEqual(result.stdout, "sigtermed\n")