Merge branch 'devel-2.4'
authorMichael Hanselmann <hansmi@google.com>
Mon, 25 Jul 2011 13:02:23 +0000 (15:02 +0200)
committerMichael Hanselmann <hansmi@google.com>
Mon, 25 Jul 2011 13:06:56 +0000 (15:06 +0200)
* devel-2.4:
  Reopen daemon's stdio on SIGHUP
  Reopen log file only once after SIGHUP
  Don't leak file descriptors when setting up daemon output
  Fix aliases in bash completion

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Makefile.am
lib/daemon.py
lib/utils/log.py
lib/utils/process.py
test/ganeti.utils.log_unittest.py

index 2aa34af..38a335c 100644 (file)
@@ -960,7 +960,9 @@ $(PYTHON_BOOTSTRAP): Makefile | $(all_dirfiles)
          echo '# Temporarily alias commands until bash completion'; \
          echo '# generator is changed'; \
          echo 'if hasattr(main, "commands"):'; \
-               echo '  commands = main.commands # pylint: disable-msg=E1101'; \
+         echo '  commands = main.commands # pylint: disable-msg=E1101'; \
+         echo 'if hasattr(main, "aliases"):'; \
+         echo '  aliases = main.aliases # pylint: disable-msg=E1101'; \
          echo; \
          echo 'if __name__ == "__main__":'; \
          echo '  sys.exit(main.Main())'; \
index 974c8d8..7f5e9ef 100644 (file)
@@ -544,15 +544,17 @@ def _BeautifyError(err):
     return "%s" % str(err)
 
 
-def _HandleSigHup(reopen_cb, signum, frame): # pylint: disable-msg=W0613
+def _HandleSigHup(reopen_fn, signum, frame): # pylint: disable-msg=W0613
   """Handler for SIGHUP.
 
-  @param reopen_cb: Callback function for reopening log files
+  @param reopen_fn: List of callback functions for reopening log files
 
   """
-  assert callable(reopen_cb)
   logging.info("Reopening log files after receiving SIGHUP")
-  reopen_cb()
+
+  for fn in reopen_fn:
+    if fn:
+      fn()
 
 
 def GenericMain(daemon_name, optionparser,
@@ -668,9 +670,10 @@ def GenericMain(daemon_name, optionparser,
 
   if options.fork:
     utils.CloseFDs()
-    wpipe = utils.Daemonize(logfile=constants.DAEMONS_LOGFILES[daemon_name])
+    (wpipe, stdio_reopen_fn) = \
+      utils.Daemonize(logfile=constants.DAEMONS_LOGFILES[daemon_name])
   else:
-    wpipe = None
+    (wpipe, stdio_reopen_fn) = (None, None)
 
   log_reopen_fn = \
     utils.SetupLogging(constants.DAEMONS_LOGFILES[daemon_name], daemon_name,
@@ -681,7 +684,8 @@ def GenericMain(daemon_name, optionparser,
                        console_logging=console_logging)
 
   # Reopen log file(s) on SIGHUP
-  signal.signal(signal.SIGHUP, compat.partial(_HandleSigHup, log_reopen_fn))
+  signal.signal(signal.SIGHUP,
+                compat.partial(_HandleSigHup, [log_reopen_fn, stdio_reopen_fn]))
 
   utils.WritePidFile(utils.DaemonPidFileName(daemon_name))
   try:
index 2ddd0f9..1cc6d82 100644 (file)
@@ -70,6 +70,9 @@ class _ReopenableLogHandler(logging.handlers.BaseRotatingHandler):
     # TODO: Handle errors?
     self.stream = open(self.baseFilename, "a")
 
+    # Don't reopen on the next message
+    self._reopen = False
+
   def RequestReopen(self):
     """Register a request to reopen the file.
 
index 6d57b7c..f174776 100644 (file)
@@ -36,6 +36,7 @@ from cStringIO import StringIO
 
 from ganeti import errors
 from ganeti import constants
+from ganeti import compat
 
 from ganeti.utils import retry as utils_retry
 from ganeti.utils import wrapper as utils_wrapper
@@ -256,8 +257,10 @@ def SetupDaemonFDs(output_file, output_fd):
   # Open /dev/null (read-only, only for stdin)
   devnull_fd = os.open(os.devnull, os.O_RDONLY)
 
+  output_close = True
+
   if output_fd is not None:
-    pass
+    output_close = False
   elif output_file is not None:
     # Open output file
     try:
@@ -273,6 +276,12 @@ def SetupDaemonFDs(output_file, output_fd):
   os.dup2(output_fd, 1)
   os.dup2(output_fd, 2)
 
+  if devnull_fd > 2:
+    utils_wrapper.CloseFdNoError(devnull_fd)
+
+  if output_close and output_fd > 2:
+    utils_wrapper.CloseFdNoError(output_fd)
+
 
 def StartDaemon(cmd, env=None, cwd="/", output=None, output_fd=None,
                 pidfile=None):
@@ -837,8 +846,9 @@ def Daemonize(logfile):
 
   @type logfile: str
   @param logfile: the logfile to which we should redirect stdout/stderr
-  @rtype: int
-  @return: the value zero
+  @rtype: tuple; (int, callable)
+  @return: File descriptor of pipe(2) which must be closed to notify parent
+    process and a callable to reopen log files
 
   """
   # pylint: disable-msg=W0212
@@ -874,8 +884,12 @@ def Daemonize(logfile):
       rcode = 0
     os._exit(rcode) # Exit parent of the first child.
 
-  SetupDaemonFDs(logfile, None)
-  return wpipe
+  reopen_fn = compat.partial(SetupDaemonFDs, logfile, None)
+
+  # Open logs for the first time
+  reopen_fn()
+
+  return (wpipe, reopen_fn)
 
 
 def KillProcess(pid, signal_=signal.SIGTERM, timeout=30,
index 8594fb8..5ae860b 100755 (executable)
@@ -35,7 +35,7 @@ import testutils
 
 
 class TestLogHandler(unittest.TestCase):
-  def test(self):
+  def testNormal(self):
     tmpfile = tempfile.NamedTemporaryFile()
 
     handler = utils.log._ReopenableLogHandler(tmpfile.name)
@@ -84,6 +84,10 @@ class TestLogHandler(unittest.TestCase):
     # Write another message, should reopen
     for _ in range(4):
       logger.info("Test message INFO")
+
+      # Flag must be reset
+      self.assertFalse(handler._reopen)
+
       self.assertFalse(utils.VerifyFileID(utils.GetFileID(tmpfile.name),
                                           before_id))