From 59f187eb9ec7d5065d9b974aeff08f0e57ad5b02 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Wed, 30 Jul 2008 12:32:42 +0000
Subject: [PATCH] Unify SetupDaemon/SetupLogging

The 'old-style' info, error, debug logs do not make much sense. This
patch unifies the SetupLogging and SetupDaemon functions. As a result,
all the commands logs to a 'commands.log' file.

The patch also changes the log setup to keep going if there's an error
in setting up the file logging but we're logging to stderr.

Also, burnin now logs to its own file (burnin.log).

Reviewed-by: ultrotter
---
 daemons/ganeti-masterd |  4 +-
 daemons/ganeti-noded   |  4 +-
 daemons/ganeti-watcher |  2 +-
 lib/cli.py             |  3 +-
 lib/constants.py       |  1 +
 lib/logger.py          | 97 ++++++++++++++----------------------------
 tools/burnin           |  2 +-
 7 files changed, 40 insertions(+), 73 deletions(-)

diff --git a/daemons/ganeti-masterd b/daemons/ganeti-masterd
index 56b00f74a..cdaff4eed 100755
--- a/daemons/ganeti-masterd
+++ b/daemons/ganeti-masterd
@@ -306,8 +306,8 @@ def main():
 
   utils.WritePidFile(constants.MASTERD_PID)
 
-  logger.SetupDaemon(constants.LOG_MASTERDAEMON, debug=options.debug,
-                     stderr_logging=not options.fork)
+  logger.SetupLogging(constants.LOG_MASTERDAEMON, debug=options.debug,
+                      stderr_logging=not options.fork)
 
   logging.info("ganeti master daemon startup")
 
diff --git a/daemons/ganeti-noded b/daemons/ganeti-noded
index 449e010b8..50cd34c87 100755
--- a/daemons/ganeti-noded
+++ b/daemons/ganeti-noded
@@ -617,8 +617,8 @@ def main():
 
   utils.WritePidFile(constants.NODED_PID)
 
-  logger.SetupDaemon(logfile=constants.LOG_NODESERVER, debug=options.debug,
-                     stderr_logging=not options.fork)
+  logger.SetupLogging(logfile=constants.LOG_NODESERVER, debug=options.debug,
+                      stderr_logging=not options.fork)
   logging.info("ganeti node daemon startup")
 
   if options.fork:
diff --git a/daemons/ganeti-watcher b/daemons/ganeti-watcher
index b88e38748..033a49271 100755
--- a/daemons/ganeti-watcher
+++ b/daemons/ganeti-watcher
@@ -438,7 +438,7 @@ def main():
   """
   options, args = ParseOptions()
 
-  logger.SetupDaemon(constants.LOG_WATCHER, debug=options.debug)
+  logger.SetupLogging(constants.LOG_WATCHER, debug=options.debug)
 
   try:
     try:
diff --git a/lib/cli.py b/lib/cli.py
index 1eab65649..f77f15bd6 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -512,7 +512,8 @@ def GenericMain(commands, override=None, aliases=None):
     for key, val in override.iteritems():
       setattr(options, key, val)
 
-  logger.SetupLogging(program=binary, debug=options.debug)
+  logger.SetupLogging(constants.LOG_COMMANDS, debug=options.debug,
+                      stderr_logging=True, program=binary)
 
   utils.debug = options.debug
 
diff --git a/lib/constants.py b/lib/constants.py
index 3ab3885af..eb3275abc 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -116,6 +116,7 @@ LOG_MASTERDAEMON = LOG_DIR + "master-daemon.log"
 LOG_RAPISERVER = LOG_DIR + "rapi-daemon.log"
 LOG_RAPIACCESS = LOG_DIR + "rapi-access.log"
 LOG_COMMANDS = LOG_DIR + "commands.log"
+LOG_BURNING = LOG_DIR + "burnin.log"
 
 OS_SEARCH_PATH = _autoconf.OS_SEARCH_PATH
 EXPORT_DIR = _autoconf.EXPORT_DIR
diff --git a/lib/logger.py b/lib/logger.py
index f68355785..b90c9531d 100644
--- a/lib/logger.py
+++ b/lib/logger.py
@@ -35,84 +35,49 @@ import os, os.path
 from ganeti import constants
 
 
-def _CreateFileHandler(name):
-  return logging.FileHandler(os.path.join(constants.LOG_DIR, name))
-
-
-def SetupLogging(program='ganeti', debug=False):
-  """Setup logging for ganeti
-
-  On failure, a check is made whether process is run by root or not,
-  and an appropriate error message is printed on stderr, then process
-  exits.
-
-  Args:
-    debug: Whether to enable verbose logging
-    program: Program name
+def SetupLogging(logfile, debug=False, stderr_logging=False, program=""):
+  """Configures the logging module.
 
   """
-  fmt = "%(asctime)s " + program + ": %(message)s"
-  formatter = logging.Formatter(fmt)
-
-  stderr_fmt = "%(asctime)s: %(message)s"
-  stderr_formatter = logging.Formatter(stderr_fmt)
-
-  info_file = _CreateFileHandler("info")
-  info_file.setLevel(logging.INFO)
-  info_file.setFormatter(formatter)
-
-  errors_file = _CreateFileHandler("errors")
-  errors_file.setLevel(logging.ERROR)
-  errors_file.setFormatter(formatter)
-
-  debug_file = _CreateFileHandler("debug")
-  debug_file.setLevel(logging.DEBUG)
-  debug_file.setFormatter(formatter)
-
-  stderr_file = logging.StreamHandler()
-  stderr_file.setFormatter(stderr_formatter)
+  fmt = "%(asctime)s: " + program + " "
   if debug:
-    stderr_file.setLevel(logging.NOTSET)
-  else:
-    stderr_file.setLevel(logging.ERROR)
-
-  root_logger = logging.getLogger("")
-  root_logger.setLevel(logging.NOTSET)
-  root_logger.addHandler(info_file)
-  root_logger.addHandler(errors_file)
-  root_logger.addHandler(debug_file)
-  root_logger.addHandler(stderr_file)
-
-
-def SetupDaemon(logfile, debug=False, stderr_logging=False):
-  """Configures the logging module for daemons
-
-  """
-  if debug:
-    fmt = ("%(asctime)s: pid=%(process)d/%(threadName)s %(levelname)s"
+    fmt += ("pid=%(process)d/%(threadName)s %(levelname)s"
            " %(module)s:%(lineno)s %(message)s")
   else:
-    fmt = "%(asctime)s: pid=%(process)d %(levelname)s %(message)s"
+    fmt += "pid=%(process)d %(levelname)s %(message)s"
   formatter = logging.Formatter(fmt)
 
-  logfile_handler = logging.FileHandler(logfile)
-  logfile_handler.setFormatter(formatter)
-
-  stderr_handler = logging.StreamHandler()
-  stderr_handler.setFormatter(formatter)
-  if debug:
-    logfile_handler.setLevel(logging.DEBUG)
-    stderr_handler.setLevel(logging.NOTSET)
-  else:
-    logfile_handler.setLevel(logging.INFO)
-    stderr_handler.setLevel(logging.CRITICAL)
-
   root_logger = logging.getLogger("")
   root_logger.setLevel(logging.NOTSET)
-  root_logger.addHandler(logfile_handler)
+
   if stderr_logging:
+    stderr_handler = logging.StreamHandler()
+    stderr_handler.setFormatter(formatter)
+    if debug:
+      stderr_handler.setLevel(logging.NOTSET)
+    else:
+      stderr_handler.setLevel(logging.CRITICAL)
     root_logger.addHandler(stderr_handler)
 
+  # this can fail, if the logging directories are not setup or we have
+  # a permisssion problem; in this case, it's best to log but ignore
+  # the error if stderr_logging is True, and if false we re-raise the
+  # exception since otherwise we could run but without any logs at all
+  try:
+    logfile_handler = logging.FileHandler(logfile)
+    logfile_handler.setFormatter(formatter)
+    if debug:
+      logfile_handler.setLevel(logging.DEBUG)
+    else:
+      logfile_handler.setLevel(logging.INFO)
+    root_logger.addHandler(logfile_handler)
+  except EnvironmentError, err:
+    if stderr_logging:
+      logging.exception("Failed to enable logging to file '%s'", logfile)
+    else:
+      # we need to re-raise the exception
+      raise
+
 
 # Backwards compatibility
 Error = logging.error
diff --git a/tools/burnin b/tools/burnin
index cfa848aec..04a1bf742 100755
--- a/tools/burnin
+++ b/tools/burnin
@@ -63,7 +63,7 @@ class Burner(object):
 
   def __init__(self):
     """Constructor."""
-    logger.SetupLogging(program="ganeti/burnin", debug=False)
+    logger.SetupLogging(constants.LOG_BURNIN, debug=False, stderr_logging=True)
     self._feed_buf = StringIO()
     self.nodes = []
     self.instances = []
-- 
GitLab