[master] Merge branch 'trac3591' (logging lockfile fix)

Conflicts:
	ChangeLog
	Makefile.am
	src/lib/dhcpsrv/daemon.cc

ChangeLog

@@ -1,3 +1,9 @@
+838.	[bug]		tomek
+	Kea components now use the KEA_LOCKFILE_DIR environment variable
+	to specify the directory of the logging lockfile. Locking can be
+	disabled completely by setting the variable to 'none'.
+	(Trac #3591, git d4556e1d21766b94f2f0cda59df15e47e6f2676e)
+
 837.	[bug,doc]	tomek
 	Logging configuration examples in kea.conf fixed. Also updated
 	Kea documentation for logging.

Makefile.am

@@ -122,6 +122,7 @@ cppcheck:
 # These steps are necessary during installation
 install-exec-hook:
 	mkdir -p $(DESTDIR)${localstatedir}/log/
+	mkdir -p $(DESTDIR)${localstatedir}/run/${PACKAGE_NAME}
 
 ### include tool to generate documentation from log message specifications
 ### in the distributed tarball:

doc/guide/logging.xml

@@ -20,7 +20,7 @@
     <title>Logging</title>
 
     <section>
-      <title>Logging configuration</title>
+      <title>Logging Configuration</title>
 
       <para>
         During its operation Kea may produce many messages. They differ in
@@ -426,7 +426,7 @@ TODO; there's a ticket to determine these levels, see #1074
       </section>
 
       <section>
-        <title>Example Logger configurations</title>
+        <title>Example Logger Configurations</title>
 
         <para>
           In this example we want to set the global logging to
@@ -561,4 +561,76 @@ file be created.</para>
 
     </section>
 
+    <section>
+      <title>Logging During Kea Startup</title>
+
+      <para>
+        The logging configuration is specified in the configuration file.
+        However, when Kea starts, the file is not read until some way into the
+        initialization process.  Prior to that, the logging settings are
+        set to default values, although it is possible to modify some
+        aspects of the settings by means of environment variables. Note
+        that in the absence of any logging configuration in the configuration
+        file, the settings of (possibly modified) default configuration will
+        persist while the program is running.
+      </para>
+      <para>
+        The following environment variables can be used to control the
+        behavio of logging during startup:
+      </para>
+
+          <variablelist>
+          <varlistentry>
+          <term>KEA_LOCKFILE_DIR</term>
+          <listitem><para>
+              Specifies a directory where logging system should create its
+              lock file. If not specified, it is
+              <replaceable>prefix</replaceable>/var/run/kea, where
+              <replaceable>prefix</replaceable> defaults to /usr/local.
+              This variable must not end
+              with a slash. There is one special value: "none", which
+              instructs Kea to not create lock file at all. This may cause
+              issues if several processes log to the same file.
+          </para></listitem>
+          </varlistentry>
+
+          <varlistentry>
+          <term>KEA_LOGGER_DESTINATION</term>
+          <listitem><para>
+              Specifies logging output. There are several special values.
+              <variablelist>
+                  <varlistentry>
+                      <term>stdout</term>
+                      <listitem><para>
+                          Log to standard output.
+                      </para></listitem>
+                  </varlistentry>
+                  <varlistentry>
+                      <term>stderr</term>
+                      <listitem><para>
+                          Log to standard error.
+                      </para></listitem>
+                  </varlistentry>
+                  <varlistentry>
+                      <term>syslog<optional>:<replaceable>fac</replaceable></optional></term>
+                      <listitem><para>
+                          Log via syslog. The optional
+                          <replaceable>fac</replaceable> (which is
+                          separated from the word "syslog" by a colon)
+                          specifies the
+                          facility to be used for the log messages. Unless
+                          specified, messages will be logged using the
+                          facility "local0".
+                      </para></listitem>
+                  </varlistentry>
+              </variablelist>
+              Any other value is treated as a name
+              of the output file. If not specified otherwise, Kea will log to
+              standard output.
+          </para></listitem>
+          </varlistentry>
+
+          </variablelist>
+    </section>
+
   </chapter>

src/bin/d2/d_controller.cc

@@ -453,17 +453,6 @@ DControllerBase::~DControllerBase() {
 
 }; // namespace isc::d2
 
-// Provide an implementation until we figure out a better way to do this.
-void
-dhcp::Daemon::loggerInit(const char* log_name, bool verbose) {
-    setenv("KEA_LOCKFILE_DIR_FROM_BUILD", "/tmp", 1);
-    setenv("KEA_LOGGER_ROOT", log_name, 0);
-    setenv("KEA_LOGGER_SEVERITY", (verbose ? "DEBUG":"INFO"), 0);
-    setenv("KEA_LOGGER_DBGLEVEL", "99", 0);
-    setenv("KEA_LOGGER_DESTINATION",  "stdout", 0);
-    isc::log::initLogger();
-}
-
 }; // namespace isc
 
 std::string

src/bin/d2/tests/Makefile.am

@@ -14,7 +14,7 @@ EXTRA_DIST += testdata/d2_cfg_tests.json
 check-local:
 	for shtest in $(SHTESTS) ; do \
 	echo Running test: $$shtest ; \
-	export B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir); \
+	export KEA_LOCKFILE_DIR=$(abs_top_builddir); \
 	${SHELL} $(abs_builddir)/$$shtest || exit ; \
 	done
 

src/bin/d2/tests/d2_process_tests.sh.in

@@ -239,3 +239,4 @@ dynamic_reconfiguration_test
 shutdown_test "dhcp-ddns.sigterm_test" 15
 shutdown_test "dhcp-ddns.sigint_test" 2
 version_test "dhcp-ddns.version"
+logger_vars_test "dhcp-ddns.variables"

src/bin/dhcp4/bundy_controller.cc

@@ -211,12 +211,5 @@ void ControlledDhcpv4Srv::cleanup() {
     }
 }
 
-void
-Daemon::loggerInit(const char* log_name, bool verbose) {
-    isc::log::initLogger(log_name,
-                         (verbose ? isc::log::DEBUG : isc::log::INFO),
-                         isc::log::MAX_DEBUG_LEVEL, NULL, true);
-}
-
 };
 };

src/bin/dhcp4/kea_controller.cc

@@ -178,18 +178,5 @@ void ControlledDhcpv4Srv::cleanup() {
     // Nothing to do here. No need to disconnect from anything.
 }
 
-/// This is a logger initialization for JSON file backend.
-/// For now, it's just setting log messages to be printed on stdout.
-/// @todo: Implement this properly (see #3427)
-void Daemon::loggerInit(const char* logger_name, bool verbose) {
-
-    setenv("KEA_LOCKFILE_DIR_FROM_BUILD", "/tmp", 1);
-    setenv("KEA_LOGGER_ROOT", logger_name, 0);
-    setenv("KEA_LOGGER_SEVERITY", (verbose ? "DEBUG":"INFO"), 0);
-    setenv("KEA_LOGGER_DBGLEVEL", "99", 0);
-    setenv("KEA_LOGGER_DESTINATION",  "stdout", 0);
-    isc::log::initLogger();
-}
-
 };
 };

src/bin/dhcp4/tests/Makefile.am

@@ -13,7 +13,7 @@ EXTRA_DIST  = dhcp4_process_tests.sh.in
 check-local:
 	for shtest in $(SHTESTS) ; do \
 	echo Running test: $$shtest ; \
-	export B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir); \
+	export KEA_LOCKFILE_DIR=$(abs_top_builddir); \
 	${SHELL} $(abs_builddir)/$$shtest || exit ; \
 	done
 

src/bin/dhcp4/tests/dhcp4_process_tests.sh.in

@@ -272,3 +272,4 @@ dynamic_reconfiguration_test
 shutdown_test "dhcpv4.sigterm_test" 15
 shutdown_test "dhcpv4.sigint_test" 2
 version_test "dhcpv4.version"
+logger_vars_test "dhcpv4.variables"

src/bin/dhcp6/bundy_controller.cc

@@ -222,12 +222,5 @@ void ControlledDhcpv6Srv::cleanup() {
     }
 }
 
-void
-Daemon::loggerInit(const char* log_name, bool verbose) {
-    isc::log::initLogger(log_name,
-                         (verbose ? isc::log::DEBUG : isc::log::INFO),
-                         isc::log::MAX_DEBUG_LEVEL, NULL, true);
-}
-
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/bin/dhcp6/kea_controller.cc

@@ -185,18 +185,5 @@ void ControlledDhcpv6Srv::cleanup() {
     // Nothing to do here. No need to disconnect from anything.
 }
 
-/// This is a logger initialization for JSON file backend.
-/// For now, it's just setting log messages to be printed on stdout.
-/// @todo: Implement this properly (see #3427)
-void Daemon::loggerInit(const char* logger_name, bool verbose) {
-
-    setenv("KEA_LOCKFILE_DIR_FROM_BUILD", "/tmp", 1);
-    setenv("KEA_LOGGER_ROOT", logger_name, 0);
-    setenv("KEA_LOGGER_SEVERITY", (verbose ? "DEBUG":"INFO"), 0);
-    setenv("KEA_LOGGER_DBGLEVEL", "99", 0);
-    setenv("KEA_LOGGER_DESTINATION",  "stdout", 0);
-    isc::log::initLogger();
-}
-
 };
 };

src/bin/dhcp6/tests/Makefile.am

@@ -13,7 +13,7 @@ EXTRA_DIST = $(SHTESTS) dhcp6_process_tests.sh.in
 check-local:
 	for shtest in $(SHTESTS) ; do \
 	echo Running test: $$shtest ; \
-	export B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir); \
+	export KEA_LOCKFILE_DIR=$(abs_top_builddir); \
 	${SHELL} $(abs_builddir)/$$shtest || exit ; \
 	done
 

src/bin/dhcp6/tests/dhcp6_process_tests.sh.in

@@ -276,3 +276,4 @@ dynamic_reconfiguration_test
 shutdown_test "dhcpv6.sigterm_test" 15
 shutdown_test "dhcpv6.sigint_test" 2
 version_test "dhcpv6.version"
+logger_vars_test "dhcpv6.variables"

src/bin/keactrl/tests/Makefile.am

@@ -14,6 +14,7 @@ check-local:
 	for shtest in $(SHTESTS) ; do \
 	echo Running test: $$shtest ; \
 	chmod +x $(abs_builddir)/$$shtest ; \
+	export KEA_LOCKFILE_DIR=$(abs_top_builddir); \
 	export KEACTRL_BUILD_DIR=$(abs_top_builddir); \
 	export KEACTRL_CONF=$(abs_top_builddir)/src/bin/keactrl/tests/keactrl_test.conf; \
 	${SHELL} $(abs_builddir)/$$shtest || exit ; \

src/lib/dhcpsrv/daemon.cc

@@ -19,6 +19,8 @@
 #include <cc/data.h>
 #include <boost/bind.hpp>
 #include <logging.h>
+#include <log/logger_name.h>
+#include <log/logger_support.h>
 #include <errno.h>
 
 /// @brief provides default implementation for basic daemon operations
@@ -78,5 +80,15 @@ Daemon::getVerbose() const {
     return (CfgMgr::instance().isVerbose());
 }
 
+void Daemon::loggerInit(const char* name, bool verbose) {
+
+    // Initialize logger system
+    isc::log::initLogger(name, isc::log::DEBUG, isc::log::MAX_DEBUG_LEVEL,
+                         NULL);
+
+    // Apply default configuration (log INFO or DEBUG to stdout)
+    isc::log::setDefaultLoggingOutput(verbose);
+}
+
 };
 };

src/lib/dhcpsrv/daemon.h

@@ -111,8 +111,10 @@ public:
 
     /// @brief Initializes logger
     ///
-    /// This method initializes logger. Currently its implementation is specific
-    /// to each configuration backend.
+    /// This method initializes logging system. It also sets the default
+    /// output to stdout. This is used in early stages of the startup
+    /// phase before config file and parsed and proper logging details
+    /// are known.
     ///
     /// @param log_name name used in logger initialization
     /// @param verbose verbose mode (true usually enables DEBUG messages)

src/lib/dhcpsrv/logging.cc

@@ -17,6 +17,7 @@
 #include <boost/foreach.hpp>
 #include <boost/lexical_cast.hpp>
 #include <log/logger_specification.h>
+#include <log/logger_support.h>
 #include <log/logger_manager.h>
 #include <log/logger_name.h>
 
@@ -156,9 +157,6 @@ void LogConfigParser::applyConfiguration() {
     static const std::string SYSLOG = "syslog";
     static const std::string SYSLOG_COLON = "syslog:";
 
-    // Set locking directory to /tmp
-    setenv("B10_LOCKFILE_DIR_FROM_BUILD", "/tmp", 1);
-
     std::vector<LoggerSpecification> specs;
 
     // Now iterate through all specified loggers
@@ -174,7 +172,7 @@ void LogConfigParser::applyConfiguration() {
 
         for (std::vector<LoggingDestination>::const_iterator dest = it->destinations_.begin();
              dest != it->destinations_.end(); ++dest) {
-            
+
             // Set up output option according to destination specification
             if (dest->output_ == STDOUT) {
                 option.destination = OutputOption::DEST_CONSOLE;
@@ -219,19 +217,5 @@ void LogConfigParser::applyConfiguration() {
     manager.process(specs.begin(), specs.end());
 }
 
-void LogConfigParser::applyDefaultConfiguration(bool verbose) {
-    LoggerSpecification spec("kea", (verbose?isc::log::DEBUG : isc::log::INFO),
-                             (verbose?99:0));
-
-    OutputOption option;
-    option.destination = OutputOption::DEST_CONSOLE;
-    option.stream = OutputOption::STR_STDOUT;
-
-    spec.addOutputOption(option);
-
-    LoggerManager manager;
-    manager.process(spec);
-}
-
 } // namespace isc::dhcp
 } // namespace isc

src/lib/dhcpsrv/logging.h

@@ -67,13 +67,6 @@ public:
     /// @brief Applies stored configuration
     void applyConfiguration();
 
-    /// @brief Configures default logging
-    ///
-    /// This method is static,
-    ///
-    /// @param verbose specifies verbose mode (true forces DEBUG, debuglevel = 99)
-    void applyDefaultConfiguration(bool verbose = false);
-
 private:
 
     /// @brief Parses one JSON structure in Logging/loggers" array

src/lib/log/interprocess/Makefile.am

@@ -1,7 +1,7 @@
 SUBDIRS = . tests
 
 AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
-AM_CPPFLAGS += -DLOCKFILE_DIR=\"${localstatedir}/${PACKAGE_NAME}\"
+AM_CPPFLAGS += -DLOCKFILE_DIR=\"${localstatedir}/run/${PACKAGE_NAME}\"
 AM_CPPFLAGS += $(BOOST_INCLUDES)
 
 AM_CXXFLAGS = $(KEA_CXXFLAGS)

src/lib/log/interprocess/interprocess_sync_file.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012,2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -12,11 +12,16 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+// This file requires LOCKFILE_DIR to be defined. It points to the default
+// directory where lockfile will be created.
+
 #include <log/interprocess/interprocess_sync_file.h>
 
 #include <string>
 #include <cerrno>
 #include <cstring>
+#include <sstream>
+#include <iostream>
 
 #include <stdlib.h>
 #include <string.h>
@@ -46,33 +51,32 @@ InterprocessSyncFile::do_lock(int cmd, short l_type) {
     if (fd_ == -1) {
         std::string lockfile_path = LOCKFILE_DIR;
 
-        const char* const env = getenv("KEA_FROM_BUILD");
+        const char* const env = getenv("KEA_LOCKFILE_DIR");
         if (env != NULL) {
             lockfile_path = env;
         }
 
-        const char* const env2 = getenv("KEA_FROM_BUILD_LOCALSTATEDIR");
-        if (env2 != NULL) {
-            lockfile_path = env2;
-        }
-
-        const char* const env3 = getenv("KEA_LOCKFILE_DIR_FROM_BUILD");
-        if (env3 != NULL) {
-            lockfile_path = env3;
-        }
-
         lockfile_path += "/" + task_name_ + "_lockfile";
 
         // Open the lockfile in the constructor so it doesn't do the access
         // checks every time a message is logged.
-        const mode_t mode = umask(0111);
-        fd_ = open(lockfile_path.c_str(), O_CREAT | O_RDWR, 0660);
+        const mode_t mode = umask(S_IXUSR | S_IXGRP | S_IXOTH); // 0111
+        fd_ = open(lockfile_path.c_str(), O_CREAT | O_RDWR,
+                   S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP); // 0660
         umask(mode);
 
         if (fd_ == -1) {
-            isc_throw(InterprocessSyncFileError,
-                      "Unable to use interprocess sync lockfile ("
-                      << std::strerror(errno) << "): " << lockfile_path);
+            std::stringstream tmp;
+
+            // We failed to create a lockfile. This means that the logging
+            // system is unusable. We need to report the issue using plain
+            // print to stderr.
+            tmp << "Unable to use interprocess sync lockfile ("
+                << std::strerror(errno) << "): " << lockfile_path;
+            std::cerr << tmp.str() << std::endl;
+
+            // And then throw exception as usual.
+            isc_throw(InterprocessSyncFileError, tmp.str());
         }
     }
 

src/lib/log/interprocess/tests/run_unittests.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -16,10 +16,13 @@
 #include <util/unittests/run_all.h>
 #include <stdlib.h>
 
+// This file uses TEST_DATA_TOPBUILDDIR macro, which must point to a writeable
+// directory. It will be used for creating a logger lockfile.
+
 int
 main(int argc, char* argv[]) {
     ::testing::InitGoogleTest(&argc, argv);
 
-    setenv("KEA_LOCKFILE_DIR_FROM_BUILD", TEST_DATA_TOPBUILDDIR, 1);
+    setenv("KEA_LOCKFILE_DIR", TEST_DATA_TOPBUILDDIR, 1);
     return (isc::util::unittests::run_all());
 }

src/lib/log/logger_impl.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011,2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -18,8 +18,10 @@
 
 #include <stdarg.h>
 #include <stdio.h>
+#include <cstring>
 #include <boost/lexical_cast.hpp>
 #include <boost/static_assert.hpp>
+#include <boost/algorithm/string.hpp>
 
 #include <log4cplus/configurator.h>
 #include <log4cplus/loggingmacros.h>
@@ -33,6 +35,7 @@
 #include <log/message_dictionary.h>
 #include <log/message_types.h>
 #include <log/interprocess/interprocess_sync_file.h>
+#include <log/interprocess/interprocess_sync_null.h>
 
 #include <util/strutil.h>
 
@@ -45,6 +48,20 @@ using namespace std;
 namespace isc {
 namespace log {
 
+/// @brief detects whether file locking is enabled or disabled
+///
+/// The lockfile is enabled by default. The only way to disable it is to
+/// set KEA_LOCKFILE_DIR variable to 'none'.
+/// @return true if lockfile is enabled, false otherwise
+bool lockfileEnabled() {
+    const char* const env = getenv("KEA_LOCKFILE_DIR");
+    if (env && boost::iequals(string(env), string("none"))) {
+        return (false);
+    }
+
+    return (true);
+}
+
 // Constructor.  The setting of logger_ must be done when the variable is
 // constructed (instead of being left to the body of the function); at least
 // one compiler requires that all member variables be constructed before the
@@ -52,9 +69,13 @@ namespace log {
 // default constructor.
 LoggerImpl::LoggerImpl(const string& name) :
     name_(expandLoggerName(name)),
-    logger_(log4cplus::Logger::getInstance(name_)),
-    sync_(new interprocess::InterprocessSyncFile("logger"))
+    logger_(log4cplus::Logger::getInstance(name_))
 {
+    if (lockfileEnabled()) {
+        sync_ = new interprocess::InterprocessSyncFile("logger");
+    } else {
+        sync_ = new interprocess::InterprocessSyncNull("logger");
+    }
 }
 
 // Destructor. (Here because of virtual declaration.)

src/lib/log/logger_support.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011,2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -15,6 +15,7 @@
 #include <string>
 #include <log/logger_support.h>
 #include <log/logger_manager.h>
+#include <log/logger_name.h>
 
 using namespace std;
 
@@ -50,5 +51,75 @@ initLogger(const string& root, isc::log::Severity severity, int dbglevel,
     LoggerManager::init(root, severity, dbglevel, file, buffer);
 }
 
+// Reset characteristics of the root logger to that set by the environment
+// variables KEA_LOGGER_SEVERITY, KEA_LOGGER_DBGLEVEL and KEA_LOGGER_DESTINATION.
+
+void
+setDefaultLoggingOutput(bool verbose) {
+
+    using namespace isc::log;
+
+    // Constants: not declared static as this is function is expected to be
+    // called once only
+    const string DEVNULL = "/dev/null";
+    const string STDOUT = "stdout";
+    const string STDERR = "stderr";
+    const string SYSLOG = "syslog";
+    const string SYSLOG_COLON = "syslog:";
+
+    // Get the destination.  If not specified, assume /dev/null. (The default
+    // severity for unit tests is DEBUG, which generates a lot of output.
+    // Routing the logging to /dev/null will suppress that, whilst still
+    // ensuring that the code paths are tested.)
+    const char* destination = getenv("KEA_LOGGER_DESTINATION");
+    const string dest((destination == NULL) ? DEVNULL : destination);
+
+    // Prepare the objects to define the logging specification
+    LoggerSpecification spec(getRootLoggerName(),
+                             keaLoggerSeverity(verbose ? isc::log::DEBUG :
+                                               isc::log::INFO),
+                             keaLoggerDbglevel(isc::log::MAX_DEBUG_LEVEL));
+    OutputOption option;
+
+    // Set up output option according to destination specification
+    if (dest == STDOUT) {
+        option.destination = OutputOption::DEST_CONSOLE;
+        option.stream = OutputOption::STR_STDOUT;
+
+    } else if (dest == STDERR) {
+        option.destination = OutputOption::DEST_CONSOLE;
+        option.stream = OutputOption::STR_STDERR;
+
+    } else if (dest == SYSLOG) {
+        option.destination = OutputOption::DEST_SYSLOG;
+        // Use default specified in OutputOption constructor for the
+        // syslog destination
+
+    } else if (dest.find(SYSLOG_COLON) == 0) {
+        option.destination = OutputOption::DEST_SYSLOG;
+        // Must take account of the string actually being "syslog:"
+        if (dest == SYSLOG_COLON) {
+            cerr << "**ERROR** value for KEA_LOGGER_DESTINATION of " <<
+                    SYSLOG_COLON << " is invalid, " << SYSLOG <<
+                    " will be used instead\n";
+            // Use default for logging facility
+
+        } else {
+            // Everything else in the string is the facility name
+            option.facility = dest.substr(SYSLOG_COLON.size());
+        }
+
+    } else {
+        // Not a recognised destination, assume a file.
+        option.destination = OutputOption::DEST_FILE;
+        option.filename = dest;
+    }
+
+    // ... and set the destination
+    spec.addOutputOption(option);
+    LoggerManager manager;
+    manager.process(spec);
+}
+
 } // namespace log
 } // namespace isc

src/lib/log/logger_support.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011,2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -69,6 +69,17 @@ void initLogger(const std::string& root,
                 int dbglevel = 0, const char* file = NULL,
                 bool buffer = false);
 
+/// \brief Reset root logger characteristics
+///
+/// This is a simplified interface into the resetting of the characteristics
+/// of the root logger.  It is aimed for use in unit tests and initial
+/// phase of bring up before logging configuration is parsed and applied.
+/// It uses KEA_LOGGER_DESTINATION environment variable to specify
+/// logging destination.
+/// @param verbose defines whether logging should be verbose or not
+void setDefaultLoggingOutput(bool verbose = true);
+
+
 } // namespace log
 } // namespace isc
 

src/lib/log/logger_unittest_support.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011,2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -75,77 +75,6 @@ keaLoggerDbglevel(int defdbglevel) {
     return (defdbglevel);
 }
 
-
-// Reset characteristics of the root logger to that set by the environment
-// variables KEA_LOGGER_SEVERITY, KEA_LOGGER_DBGLEVEL and KEA_LOGGER_DESTINATION.
-
-void
-resetUnitTestRootLogger() {
-
-    using namespace isc::log;
-
-    // Constants: not declared static as this is function is expected to be
-    // called once only
-    const string DEVNULL = "/dev/null";
-    const string STDOUT = "stdout";
-    const string STDERR = "stderr";
-    const string SYSLOG = "syslog";
-    const string SYSLOG_COLON = "syslog:";
-
-    // Get the destination.  If not specified, assume /dev/null. (The default
-    // severity for unit tests is DEBUG, which generates a lot of output.
-    // Routing the logging to /dev/null will suppress that, whilst still
-    // ensuring that the code paths are tested.)
-    const char* destination = getenv("KEA_LOGGER_DESTINATION");
-    const string dest((destination == NULL) ? DEVNULL : destination);
-
-    // Prepare the objects to define the logging specification
-    LoggerSpecification spec(getRootLoggerName(), 
-                             keaLoggerSeverity(isc::log::DEBUG),
-                             keaLoggerDbglevel(isc::log::MAX_DEBUG_LEVEL));
-    OutputOption option;
-
-    // Set up output option according to destination specification
-    if (dest == STDOUT) {
-        option.destination = OutputOption::DEST_CONSOLE;
-        option.stream = OutputOption::STR_STDOUT;
-
-    } else if (dest == STDERR) {
-        option.destination = OutputOption::DEST_CONSOLE;
-        option.stream = OutputOption::STR_STDERR;
-
-    } else if (dest == SYSLOG) {
-        option.destination = OutputOption::DEST_SYSLOG;
-        // Use default specified in OutputOption constructor for the
-        // syslog destination
-
-    } else if (dest.find(SYSLOG_COLON) == 0) {
-        option.destination = OutputOption::DEST_SYSLOG;
-        // Must take account of the string actually being "syslog:"
-        if (dest == SYSLOG_COLON) {
-            cerr << "**ERROR** value for KEA_LOGGER_DESTINATION of " <<
-                    SYSLOG_COLON << " is invalid, " << SYSLOG <<
-                    " will be used instead\n";
-            // Use default for logging facility
-
-        } else {
-            // Everything else in the string is the facility name
-            option.facility = dest.substr(SYSLOG_COLON.size());
-        }
-
-    } else {
-        // Not a recognised destination, assume a file.
-        option.destination = OutputOption::DEST_FILE;
-        option.filename = dest;
-    }
-
-    // ... and set the destination
-    spec.addOutputOption(option);
-    LoggerManager manager;
-    manager.process(spec);
-}
-
-
 // Logger Run-Time Initialization via Environment Variables
 void initLogger(isc::log::Severity severity, int dbglevel) {
 
@@ -162,7 +91,7 @@ void initLogger(isc::log::Severity severity, int dbglevel) {
     const char* localfile = getenv("KEA_LOGGER_LOCALMSG");
 
     // Set a directory for creating lockfiles when running tests
-    setenv("KEA_LOCKFILE_DIR_FROM_BUILD", TOP_BUILDDIR, 1);
+    setenv("KEA_LOCKFILE_DIR", TOP_BUILDDIR, 0);
 
     // Initialize logging
     initLogger(root, isc::log::DEBUG, isc::log::MAX_DEBUG_LEVEL, localfile);
@@ -172,7 +101,7 @@ void initLogger(isc::log::Severity severity, int dbglevel) {
     // in the environment variables.  (The two-step approach is used as the
     // setUnitTestRootLoggerCharacteristics() function is used in several
     // places in the Kea tests, and it avoid duplicating code.)
-    resetUnitTestRootLogger();
+    isc::log::setDefaultLoggingOutput();
 } 
 
 } // namespace log

src/lib/log/logger_unittest_support.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011,2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -70,6 +70,10 @@ namespace log {
 /// be overridden by the tester.  It is not intended for use in production
 /// code.
 ///
+/// @note: Do NOT use this function in production code as it creates
+/// lockfile in the build dir. That's ok for tests, but not
+/// ok for production code.
+///
 /// @todo: Rename. This function overloads the initLogger() function that can
 ///       be used to initialize production programs.  This may lead to confusion.
 void initLogger(isc::log::Severity severity = isc::log::DEBUG,
@@ -107,17 +111,6 @@ isc::log::Severity keaLoggerSeverity(isc::log::Severity defseverity);
 /// \return Debug level to use.
 int keaLoggerDbglevel(int defdbglevel);
 
-
-/// \brief Reset root logger characteristics
-///
-/// This is a simplified interface into the resetting of the characteristics
-/// of the root logger.  It is aimed for use in unit tests and resets the
-/// characteristics of the root logger to use a severity, debug level and
-/// destination set by the environment variables KEA_LOGGER_SEVERITY,
-/// KEA_LOGGER_DBGLEVEL and KEA_LOGGER_DESTINATION.
-void
-resetUnitTestRootLogger();
-
 } // namespace log
 } // namespace isc
 

src/lib/log/tests/logger_level_impl_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011,2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -30,7 +30,7 @@ class LoggerLevelImplTest : public ::testing::Test {
 protected:
     LoggerLevelImplTest() {
         // Ensure logging set to default for unit tests
-        resetUnitTestRootLogger();
+        setDefaultLoggingOutput();
     }
 
     ~LoggerLevelImplTest()

src/lib/log/tests/logger_level_unittest.cc

@@ -31,7 +31,7 @@ protected:
     LoggerLevelTest() {
         // Logger initialization is done in main().  As logging tests may
         // alter the default logging output, it is reset here.
-        resetUnitTestRootLogger();
+        setDefaultLoggingOutput();
     }
     ~LoggerLevelTest() {
         LoggerManager::reset();

src/lib/log/tests/logger_support_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011,2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -23,7 +23,7 @@ protected:
     LoggerSupportTest() {
         // Logger initialization is done in main().  As logging tests may
         // alter the default logging output, it is reset here.
-        resetUnitTestRootLogger();
+        setDefaultLoggingOutput();
     }
     ~LoggerSupportTest() {
     }

src/lib/testutils/Makefile.am

@@ -4,6 +4,8 @@ AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES)
 AM_CXXFLAGS=$(KEA_CXXFLAGS)
 
+noinst_SCRIPTS = dhcp_test_lib.sh
+
 if HAVE_GTEST
 noinst_LTLIBRARIES = libkea-testutils.la
 

src/lib/testutils/dhcp_test_lib.sh.in

@@ -421,3 +421,94 @@ version_test() {
         test_finish 1
     fi
 }
+
+# This test verifies that the server is using logger variable
+# KEA_LOCKFILE_DIR properly (it should be used to point out to the directory,
+# where lockfile should be created. Also, "none" value means to not create
+# the lockfile at all).
+logger_vars_test() {
+    test_name=${1}  # Test name
+
+    # Log the start of the test and print test name.
+    test_start ${test_name}
+    # Remove dangling Kea instances and remove log files.
+    cleanup
+
+    # Create bogus configuration file. We don't really want the server to start,
+    # just want it to log something and die. Empty config is an easy way to
+    # enforce that behavior.
+    create_config "{ }"
+    printf "Please ignore any config error messages.\n"
+
+    # Remember old KEA_LOCKFILE_DIR
+    KEA_LOCKFILE_DIR_OLD=${KEA_LOCKFILE_DIR}
+
+    # Set lockfile directory to current directory.
+    KEA_LOCKFILE_DIR=.
+
+    # Start Kea.
+    start_kea ${bin_path}/${bin}
+
+    # Wait for Kea to process the invalid configuration and die.
+    sleep 1
+
+    # Check if it is still running. It should have terminated.
+    get_pids ${bin}
+    if [ ${_GET_PIDS_NUM} -ne 0 ]; then
+        printf "ERROR: expected Kea process to not start. Found %d processes"
+        printf " running.\n" ${_GET_PIDS_NUM}
+
+        # Revert to the old KEA_LOCKFILE_DIR value
+        KEA_LOCKFILE_DIR=${KEA_LOCKFILE_DIR_OLD}
+        clean_exit 1
+    fi
+
+    if [ ! -f "./logger_lockfile" ]; then
+        printf "ERROR: Expect ${bin} to create logger_lockfile in the\n"
+        printf "current directory, but no such file exists.\n"
+
+        # Revert to the old KEA_LOCKFILE_DIR value
+        KEA_LOCKFILE_DIR=${KEA_LOCKFILE_DIR__OLD}
+        clean_exit 1
+    fi
+
+    # Remove the lock file
+    rm -f ./logger_lockfile
+
+    # Tell Kea to NOT create logfiles at all
+    KEA_LOCKFILE_DIR="none"
+
+    # Start Kea.
+    start_kea ${bin_path}/${bin}
+
+    # Wait for Kea to process the invalid configuration and die.
+    sleep 1
+
+    # Check if it is still running. It should have terminated.
+    get_pids ${bin}
+    if [ ${_GET_PIDS_NUM} -ne 0 ]; then
+        printf "ERROR: expected Kea process to not start. Found %d processes"
+        printf " running.\n" ${_GET_PIDS_NUM}
+
+        # Revert to the old KEA_LOCKFILE_DIR value
+        KEA_LOCKFILE_DIR=${KEA_LOCKFILE_DIR_OLD}
+
+        clean_exit 1
+    fi
+
+    if [ -f "./logger_lockfile" ]; then
+        printf "ERROR: Expect ${bin} to NOT create logger_lockfile in the\n"
+        printf "current directory, but the file exists."
+
+        # Revert to the old KEA_LOCKFILE_DIR value
+        KEA_LOCKFILE_DIR=${KEA_LOCKFILE_DIR_OLD}
+
+        clean_exit 1
+    fi
+
+    # Revert to the old KEA_LOCKFILE_DIR value
+    printf "Reverting KEA_LOCKFILE_DIR to ${KEA_LOCKFILE_DIR_OLD}\n"
+    KEA_LOCKFILE_DIR=${KEA_LOCKFILE_DIR_OLD}
+
+    test_finish 0
+}

src/lib/util/threads/tests/run_unittests.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012,2014  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -16,10 +16,13 @@
 #include <util/unittests/run_all.h>
 #include <stdlib.h>
 
+// This file uses TEST_DATA_TOPBUILDDIR macro, which must point to a writeable
+// directory. It will be used for creating a logger lockfile.
+
 int
 main(int argc, char* argv[]) {
     ::testing::InitGoogleTest(&argc, argv);
 
-    setenv("KEA_LOCKFILE_DIR_FROM_BUILD", TEST_DATA_TOPBUILDDIR, 1);
+    setenv("KEA_LOCKFILE_DIR", TEST_DATA_TOPBUILDDIR, 1);
     return (isc::util::unittests::run_all());
 }