From: Thomas Markwalder Date: Fri, 9 Dec 2016 21:06:38 +0000 (-0500) Subject: [5046] Avoid wiping logging when config is empty X-Git-Tag: trac5030_base~1^2~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=767bf1503f982d75f5c9fcca219f86a8345b5802;p=thirdparty%2Fkea.git [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 0e493c5ade..7bbf673349 100644 --- 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 0e7352ca81..8ee2b64b47 100644 --- 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 0dea9e3280..98c88b44d8 100644 --- 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 5c4b70d30d..a1c74dab10 100644 --- 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 7c7830966f..017d83767e 100644 --- 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