Browse Source

[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
Thomas Markwalder 8 years ago
parent
commit
767bf1503f

+ 12 - 7
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));
 }

+ 2 - 7
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

+ 0 - 5
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.

+ 1 - 1
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();
 }
 

+ 8 - 2
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