[5046] Avoid wiping logging when config is empty

src/lib/dhcpsrv/srv_config.cc
    SrvConfig::applyLoggingCfg() - now only calls LoggerManager::process()
    if the logger config isn't empty

src/bin/dhcp6/ctrl_dhcp6_srv.cc
src/bin/dhcp6/tests/dhcp6_test_utils.cc
    Minor clean up and commentary

src/bin/dhcp6/kea_controller.cc
    configure(const std::string& file_name)
    - Removed initial rollback, now done in commandSetConfigHandler()

src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
   - Removed unnecessary call to initLogger

diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
index 0e493c5adec215bff258763f1e4a24695343fd78..7bbf6733497d8f6e9d7b3897cf85ac424fe223b6 100644 (file)
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
@@ -77,15 +77,11 @@ ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
     ConstElementPtr dhcp6;
     string message;
 
-    // Throw out the preivous staging config that may be present
+    // We are starting the configuration process so we should remove any
+    // staging configuration that has been created during previous
+    // configuration attempts.
     CfgMgr::instance().rollback();
 
-    // Logging is a sibling element and so, has to be explicitly
-    // configured. We should call configureLogger even if there's no
-    // Logging element as this will revert it to default logging.
-    Daemon::configureLogger(args->get("Logging"),
-                            CfgMgr::instance().getStagingCfg());
-
     // Command arguments are expected to be:
     // { "Dhcp6": { ... }, "Logging": { ... } }
     // The Logging component is technically optional, but very recommended.
@@ -106,6 +102,15 @@ ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
                                                            message);
         return (result);
     }
+
+    // Logging is a sibling element and must be be parsed explicitly.
+    // The call to configureLogger parses the given Logging element if
+    // not null, into the staging config.  Note this DOES alter the
+    // current loggers, they remain in effect until we apply the
+    // logging config.
+    Daemon::configureLogger(args->get("Logging"),
+                            CfgMgr::instance().getStagingCfg());
+
     // Now we configure the server proper.
     return (processConfig(dhcp6));
 }

diff --git a/src/bin/dhcp6/kea_controller.cc b/src/bin/dhcp6/kea_controller.cc
index 0e7352ca818c8916b1a5aab32ad81c8496f121c2..8ee2b64b477dc76b8b9acd1c06e7817882946880 100644 (file)
--- a/src/bin/dhcp6/kea_controller.cc
+++ b/src/bin/dhcp6/kea_controller.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -37,11 +37,6 @@ void configure(const std::string& file_name) {
     // This is a configuration backend implementation that reads the
     // configuration from a JSON file.
 
-    // We are starting the configuration process so we should remove any
-    // staging configuration that has been created during previous
-    // configuration attempts.
-    CfgMgr::instance().rollback();
-
     isc::data::ConstElementPtr json;
     isc::data::ConstElementPtr dhcp6;
     isc::data::ConstElementPtr logger;
@@ -79,7 +74,7 @@ void configure(const std::string& file_name) {
             // happen, but as the configureDhcp6Server returns a pointer, it is
             // theoretically possible that it will return NULL.
             isc_throw(isc::BadValue, "undefined result of "
-                      "processCommand(\"set-config\", dhcp6)");
+                      "processCommand(\"set-config\", json)");
         }
 
         // Now check is the returned result is successful (rcode=0) or not

diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
index 0dea9e328065e26c6ef6fa67d0debabf738cf848..98c88b44d8977dcf14386a4652053ddd76ec9fa9 100644 (file)
--- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
@@ -151,11 +151,6 @@ public:
         ConstElementPtr answer = server_->processConfig(config);
         ASSERT_TRUE(answer);
 
-        // If the configuration doesn't contain logging config, processConfig()
-        // will revert the logging to default (stdout). We call initLogger()
-        // to restore unit test logging.
-        isc::log::initLogger();
-
         int status = 0;
         ConstElementPtr txt = isc::config::parseAnswer(status, answer);
         // This should succeed. If not, print the error message.

diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.cc b/src/bin/dhcp6/tests/dhcp6_test_utils.cc
index 5c4b70d30d45276cb9062ce786379868048658f5..a1c74dab108fb3d74e4d9f2a5d1b3071dfba1bc9 100644 (file)
--- a/src/bin/dhcp6/tests/dhcp6_test_utils.cc
+++ b/src/bin/dhcp6/tests/dhcp6_test_utils.cc
@@ -48,7 +48,7 @@ BaseServerTest::~BaseServerTest() {
     // Revert to original data directory.
     CfgMgr::instance().setDataDir(original_datadir_);
 
-    // Revert to unit test logging
+    // Revert to unit test logging in case the test reconfigured logging.
     isc::log::initLogger();
 }
 

diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc
index 7c7830966f5972e19d07db238114e9cc79cf9fd3..017d83767e6969e3c6b3d7922401b97506697043 100644 (file)
--- a/src/lib/dhcpsrv/srv_config.cc
+++ b/src/lib/dhcpsrv/srv_config.cc
@@ -117,8 +117,14 @@ SrvConfig::applyLoggingCfg() const {
          it != logging_info_.end(); ++it) {
         specs.push_back(it->toSpec());
     }
-    LoggerManager manager;
-    manager.process(specs.begin(), specs.end());
+
+    // If there are no specs, then leave logging alone. This will leave
+    // the existing logging setup intact. Calling process() with no
+    // specs will reset the logging heirarchy and so forth.
+    if (specs.size()) {
+        LoggerManager manager;
+        manager.process(specs.begin(), specs.end());
+    }
 }
 
 bool