Commit 24154b6c authored by Marcin Siodelski's avatar Marcin Siodelski

[3534] Logger configuration can be rolled back.

Previously, the logger configuration was applied (to log4cplus) as soon as
it was parsed. We decided in the jabber discussion that the logger
configuration should only be applied if the whole configuration goes
through. So, the logger configuration is now 2-stage: parsing and commit
as for other configuration parameters.
parent 615df6d2
......@@ -18,7 +18,7 @@
#include <d2/d_controller.h>
#include <exceptions/exceptions.h>
#include <log/logger_support.h>
#include <dhcpsrv/configuration.h>
#include <dhcpsrv/cfgmgr.h>
#include <sstream>
#include <unistd.h>
......@@ -59,6 +59,16 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) {
throw; // rethrow it
}
// It is important that we set a default logger name because this name
// will be used when the user doesn't provide the logging configuration
// in the Kea configuration file.
isc::dhcp::CfgMgr::instance().setDefaultLoggerName(bin_name_);
// Logger's default configuration depends on whether we are in the
// verbose mode or not. CfgMgr manages the logger configuration so
// the verbose mode is set for CfgMgr.
isc::dhcp::CfgMgr::instance().setVerbose(verbose_);
// Do not initialize logger here if we are running unit tests. It would
// replace an instance of unit test specific logger.
if (!test_mode) {
......@@ -218,8 +228,13 @@ DControllerBase::initProcess() {
isc::data::ConstElementPtr
DControllerBase::configFromFile() {
// Rollback any previous staging configuration. For D2, only a
// logger configuration is used here.
isc::dhcp::CfgMgr::instance().rollback();
// Will hold configuration.
isc::data::ConstElementPtr module_config;
// Will receive configuration result.
isc::data::ConstElementPtr answer;
try {
std::string config_file = getConfigFile();
if (config_file.empty()) {
......@@ -236,11 +251,12 @@ DControllerBase::configFromFile() {
// so we can log things during configuration process.
// Temporary storage for logging configuration
isc::dhcp::ConfigurationPtr storage(new isc::dhcp::Configuration());
isc::dhcp::ConfigurationPtr storage =
isc::dhcp::CfgMgr::instance().getStagingCfg();
// Get 'Logging' element from the config and use it to set up
// logging. If there's no such element, we'll just pass NULL.
Daemon::configureLogger(whole_config->get("Logging"), storage, verbose_);
Daemon::configureLogger(whole_config->get("Logging"), storage);
// Extract derivation-specific portion of the configuration.
module_config = whole_config->get(getAppName());
......@@ -249,7 +265,20 @@ DControllerBase::configFromFile() {
" does not include '" <<
getAppName() << "' entry.");
}
answer = updateConfig(module_config);
int rcode = 0;
isc::config::parseAnswer(rcode, answer);
if (!rcode) {
// Configuration successful, so apply the logging configuration
// to log4cplus.
isc::dhcp::CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
isc::dhcp::CfgMgr::instance().commit();
}
} catch (const std::exception& ex) {
// Rollback logging configuration.
isc::dhcp::CfgMgr::instance().rollback();
// build an error result
isc::data::ConstElementPtr error =
isc::config::createAnswer(1,
......@@ -257,7 +286,7 @@ DControllerBase::configFromFile() {
return (error);
}
return (updateConfig(module_config));
return (answer);
}
......
......@@ -65,14 +65,10 @@ void configure(const std::string& file_name) {
isc_throw(isc::BadValue, "no configuration found");
}
// Let's configure logging before applying the configuration,
// so we can log things during configuration process.
// If there's no logging element, we'll just pass NULL pointer,
// which will be handled by configureLogger().
Daemon::configureLogger(json->get("Logging"),
CfgMgr::instance().getStagingCfg(),
ControlledDhcpv4Srv::getInstance()->getVerbose());
CfgMgr::instance().getStagingCfg());
// Get Dhcp4 component from the config
dhcp4 = json->get("Dhcp4");
......@@ -102,7 +98,12 @@ void configure(const std::string& file_name) {
isc_throw(isc::BadValue, reason);
}
// Configuration successful.
// If configuration was parsed successfully, apply the new logger
// configuration to log4cplus. It is done before commit in case
// something goes wrong.
CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
// Use new configuration.
CfgMgr::instance().commit();
} catch (const std::exception& ex) {
......
......@@ -16,6 +16,7 @@
#include <dhcp4/ctrl_dhcp4_srv.h>
#include <dhcp4/dhcp4_log.h>
#include <dhcpsrv/cfgmgr.h>
#include <log/logger_support.h>
#include <log/logger_manager.h>
......@@ -122,6 +123,11 @@ main(int argc, char* argv[]) {
int ret = EXIT_SUCCESS;
try {
// It is important that we set a default logger name because this name
// will be used when the user doesn't provide the logging configuration
// in the Kea configuration file.
CfgMgr::instance().setDefaultLoggerName(DHCP4_LOGGER_NAME);
// Initialize logging. If verbose, we'll use maximum verbosity.
Daemon::loggerInit(DHCP4_LOGGER_NAME, verbose_mode);
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_START_INFO)
......
......@@ -74,8 +74,7 @@ void configure(const std::string& file_name) {
// If there's no logging element, we'll just pass NULL pointer,
// which will be handled by configureLogger().
Daemon::configureLogger(json->get("Logging"),
CfgMgr::instance().getStagingCfg(),
ControlledDhcpv6Srv::getInstance()->getVerbose());
CfgMgr::instance().getStagingCfg());
// Get Dhcp6 component from the config
dhcp6 = json->get("Dhcp6");
......@@ -106,7 +105,12 @@ void configure(const std::string& file_name) {
isc_throw(isc::BadValue, reason);
}
// Configuration successful.
// If configuration was parsed successfully, apply the new logger
// configuration to log4cplus. It is done before commit in case
// something goes wrong.
CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
// Use new configuration.
CfgMgr::instance().commit();
} catch (const std::exception& ex) {
......
// Copyright (C) 2011-2013 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
......@@ -16,6 +16,7 @@
#include <dhcp6/ctrl_dhcp6_srv.h>
#include <dhcp6/dhcp6_log.h>
#include <dhcpsrv/cfgmgr.h>
#include <log/logger_support.h>
#include <log/logger_manager.h>
#include <exceptions/exceptions.h>
......@@ -122,6 +123,11 @@ main(int argc, char* argv[]) {
int ret = EXIT_SUCCESS;
try {
// It is important that we set a default logger name because this name
// will be used when the user doesn't provide the logging configuration
// in the Kea configuration file.
CfgMgr::instance().setDefaultLoggerName(DHCP6_LOGGER_NAME);
// Initialize logging. If verbose, we'll use maximum verbosity.
Daemon::loggerInit(DHCP6_LOGGER_NAME, verbose_mode);
......
......@@ -447,11 +447,10 @@ CfgMgr::getStagingCfg() {
CfgMgr::CfgMgr()
: datadir_(DHCP_DATA_DIR), echo_v4_client_id_(true),
d2_client_mgr_() {
d2_client_mgr_(), verbose_mode_(false) {
// DHCP_DATA_DIR must be set set with -DDHCP_DATA_DIR="..." in Makefile.am
// Note: the definition of DHCP_DATA_DIR needs to include quotation marks
// See AM_CPPFLAGS definition in Makefile.am
ensureCurrentAllocated();
}
CfgMgr::~CfgMgr() {
......
......@@ -466,6 +466,39 @@ public:
//@}
/// @name Methods setting/accessing global configuration for the process.
///
//@{
/// @brief Sets verbose mode.
///
/// @param verbose A boolean value indicating if the process should run
/// in verbose (true) or non-verbose mode.
void setVerbose(const bool verbose) {
verbose_mode_ = verbose;
}
/// @brief Checks if the process has been run in verbose mode.
///
/// @return true if verbose mode enabled, false otherwise.
bool isVerbose() const {
return (verbose_mode_);
}
/// @brief Sets the default logger name.
///
/// This name is used in cases when a user doesn't provide a configuration
/// for logger in the Kea configuration file.
void setDefaultLoggerName(const std::string& name) {
default_logger_name_ = name;
}
/// @brief Returns default logger name.
std::string getDefaultLoggerName() const {
return (default_logger_name_);
}
//@}
protected:
/// @brief Protected constructor.
......@@ -557,6 +590,12 @@ private:
/// @brief Container holding all previous and current configurations.
ConfigurationList configs_;
//@}
/// @brief Indicates if a process has been ran in the verbose mode.
bool verbose_mode_;
/// @brief Default logger name.
std::string default_logger_name_;
};
} // namespace isc::dhcp
......
......@@ -14,8 +14,13 @@
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/configuration.h>
#include <log/logger_manager.h>
#include <log/logger_specification.h>
#include <list>
#include <sstream>
using namespace isc::log;
namespace isc {
namespace dhcp {
......@@ -85,6 +90,20 @@ Configuration::copy(Configuration& new_config) const {
new_config.setCfgIface(cfg_iface_);
}
void
Configuration::applyLoggingCfg() const {
/// @todo Remove the hardcoded location.
setenv("B10_LOCKFILE_DIR_FROM_BUILD", "/tmp", 1);
std::list<LoggerSpecification> specs;
for (LoggingInfoStorage::const_iterator it = logging_info_.begin();
it != logging_info_.end(); ++it) {
specs.push_back(it->toSpec());
}
LoggerManager manager;
manager.process(specs.begin(), specs.end());
}
bool
Configuration::equals(const Configuration& other) const {
// If number of loggers is different, then configurations aren't equal.
......
......@@ -143,6 +143,9 @@ public:
/// be copied.
void copy(Configuration& new_config) const;
/// @brief Apply logging configuration to log4cplus.
void applyLoggingCfg() const;
/// @name Methods and operators used to compare configurations.
///
//@{
......
......@@ -13,6 +13,7 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <config.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/daemon.h>
#include <exceptions/exceptions.h>
#include <cc/data.h>
......@@ -31,7 +32,7 @@ namespace dhcp {
std::string Daemon::config_file_ = "";
Daemon::Daemon()
: signal_set_(), signal_handler_(), verbose_(false) {
: signal_set_(), signal_handler_() {
}
Daemon::~Daemon() {
......@@ -56,40 +57,25 @@ void Daemon::handleSignal() {
}
void Daemon::configureLogger(const isc::data::ConstElementPtr& log_config,
const ConfigurationPtr& storage,
bool verbose) {
// This is utility class that translates JSON structures into formats
// understandable by log4cplus.
LogConfigParser parser(storage);
if (!log_config) {
// There was no logger configuration. Let's clear any config
// and revert to the default.
parser.applyDefaultConfiguration(verbose); // Set up default logging
return;
const ConfigurationPtr& storage) {
if (log_config) {
isc::data::ConstElementPtr loggers = log_config->get("loggers");
if (loggers) {
LogConfigParser parser(storage);
parser.parseConfiguration(loggers, CfgMgr::instance().isVerbose());
}
}
}
isc::data::ConstElementPtr loggers;
loggers = log_config->get("loggers");
if (!loggers) {
// There is Logging structure, but it doesn't have loggers
// array in it. Let's clear any old logging configuration
// we may have and revert to the default.
parser.applyDefaultConfiguration(verbose); // Set up default logging
return;
}
// Translate JSON structures into log4cplus formats
parser.parseConfiguration(loggers, verbose);
// Apply the configuration
void
Daemon::setVerbose(bool verbose) {
CfgMgr::instance().setVerbose(verbose);
}
/// @todo: Once configuration unrolling is implemented,
/// this call will be moved to a separate method.
parser.applyConfiguration();
bool
Daemon::getVerbose() const {
return (CfgMgr::instance().isVerbose());
}
};
......
......@@ -128,10 +128,8 @@ public:
///
/// @param log_config JSON structures that describe logging
/// @param storage configuration will be stored here
/// @param verbose specifies if verbose mode should be enabled
static void configureLogger(const isc::data::ConstElementPtr& log_config,
const isc::dhcp::ConfigurationPtr& storage,
bool verbose);
const isc::dhcp::ConfigurationPtr& storage);
/// @brief Sets or clears verbose mode
///
......@@ -140,16 +138,12 @@ public:
/// config file are ignored.
///
/// @param verbose specifies if verbose should be set or not
void setVerbose(bool verbose) {
verbose_ = verbose;
}
void setVerbose(const bool verbose);
/// @brief Returns if running in verbose mode
///
/// @return verbose mode
bool getVerbose() const {
return (verbose_);
}
bool getVerbose() const;
/// @brief returns Kea version on stdout and exits.
///
......@@ -195,8 +189,6 @@ private:
/// @brief Config file name or empty if config file not used.
static std::string config_file_;
/// @brief Verbose mode
bool verbose_;
};
}; // end of isc::dhcp namespace
......
......@@ -55,6 +55,8 @@ void LogConfigParser::parseConfigEntry(isc::data::ConstElementPtr entry) {
}
LoggingInfo info;
// Remove default destinations as we are going to replace them.
info.clearDestinations();
// Get a name
isc::data::ConstElementPtr name_ptr = entry->get("name");
......@@ -121,6 +123,7 @@ void LogConfigParser::parseOutputOptions(std::vector<LoggingDestination>& destin
if (!output_options) {
isc_throw(BadValue, "Missing 'output_options' structure in 'loggers'");
}
BOOST_FOREACH(ConstElementPtr output_option, output_options->listValue()) {
LoggingDestination dest;
......
......@@ -13,7 +13,11 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/logging_info.h>
#include <log/logger_name.h>
using namespace isc::log;
namespace isc {
namespace dhcp {
......@@ -25,6 +29,29 @@ LoggingDestination::equals(const LoggingDestination& other) const {
maxsize_ == other.maxsize_);
}
LoggingInfo::LoggingInfo()
: name_("kea"), severity_(isc::log::INFO), debuglevel_(0) {
// If configuration Manager is in the verbose mode, we need to modify the
// default settings.
if (CfgMgr::instance().isVerbose()) {
severity_ = isc::log::DEBUG;
debuglevel_ = 99;
}
// If the process has set the non-empty name for the default logger,
// let's use this name.
std::string logger_name = CfgMgr::instance().getDefaultLoggerName();
if (!logger_name.empty()) {
name_ = logger_name;
}
// Add a default logging destination in case use hasn't provided a
// logger specification.
LoggingDestination dest;
dest.output_ = "stdout";
destinations_.push_back(dest);
}
bool
LoggingInfo::equals(const LoggingInfo& other) const {
// If number of destinations aren't equal, the objects are not equal.
......@@ -60,5 +87,59 @@ LoggingInfo::equals(const LoggingInfo& other) const {
debuglevel_ == other.debuglevel_);
}
LoggerSpecification
LoggingInfo::toSpec() const {
static const std::string STDOUT = "stdout";
static const std::string STDERR = "stderr";
static const std::string SYSLOG = "syslog";
static const std::string SYSLOG_COLON = "syslog:";
LoggerSpecification spec(name_, severity_, debuglevel_);
// Go over logger destinations and create output options accordinly.
for (std::vector<LoggingDestination>::const_iterator dest =
destinations_.begin(); dest != destinations_.end(); ++dest) {
OutputOption option;
// Set up output option according to destination specification
if (dest->output_ == STDOUT) {
option.destination = OutputOption::DEST_CONSOLE;
option.stream = OutputOption::STR_STDOUT;
} else if (dest->output_ == STDERR) {
option.destination = OutputOption::DEST_CONSOLE;
option.stream = OutputOption::STR_STDERR;
} else if (dest->output_ == SYSLOG) {
option.destination = OutputOption::DEST_SYSLOG;
// Use default specified in OutputOption constructor for the
// syslog destination
} else if (dest->output_.find(SYSLOG_COLON) == 0) {
option.destination = OutputOption::DEST_SYSLOG;
// Must take account of the string actually being "syslog:"
if (dest->output_ == SYSLOG_COLON) {
// The expected syntax is syslog:facility. User skipped
// the logging name, so we'll just use the default ("kea")
option.facility = isc::log::getDefaultRootLoggerName();
} else {
// Everything else in the string is the facility name
option.facility = dest->output_.substr(SYSLOG_COLON.size());
}
} else {
// Not a recognised destination, assume a file.
option.destination = OutputOption::DEST_FILE;
option.filename = dest->output_;
}
// ... and set the destination
spec.addOutputOption(option);
}
return (spec);
}
} // end of namespace isc::dhcp
} // end of namespace isc
......@@ -16,6 +16,7 @@
#define DHCPSRV_LOGGING_INFO_H
#include <log/logger_level.h>
#include <log/logger_specification.h>
#include <stdint.h>
#include <vector>
......@@ -85,8 +86,11 @@ struct LoggingInfo {
std::vector<LoggingDestination> destinations_;
/// @brief Default constructor.
LoggingInfo()
: name_("kea"), severity_(isc::log::INFO), debuglevel_(99) {
LoggingInfo();
/// @brief Removes logging destinations.
void clearDestinations() {
destinations_.clear();
}
/// @brief Compares two objects for equality.
......@@ -113,6 +117,9 @@ struct LoggingInfo {
bool operator!=(const LoggingInfo& other) const {
return (!equals(other));
}
/// @brief Converts logger configuration to a spec.
isc::log::LoggerSpecification toSpec() const;
};
/// @brief storage for logging information in log4cplus format
......
......@@ -281,6 +281,7 @@ public:
}
void clear() {
CfgMgr::instance().setVerbose(false);
CfgMgr::instance().deleteSubnets4();
CfgMgr::instance().deleteSubnets6();
CfgMgr::instance().deleteOptionDefs();
......@@ -1257,6 +1258,18 @@ TEST_F(CfgMgrTest, revert) {
EXPECT_EQ(12, cfg_mgr.getCurrentCfg()->getLoggingInfo()[0].debuglevel_);
}
// This test verifies that the verbosity can be set and obtained from the
// configuration manager.
TEST_F(CfgMgrTest, verbosity) {
ASSERT_FALSE(CfgMgr::instance().isVerbose());
CfgMgr::instance().setVerbose(true);
ASSERT_TRUE(CfgMgr::instance().isVerbose());
CfgMgr::instance().setVerbose(false);
EXPECT_FALSE(CfgMgr::instance().isVerbose());
}
/// @todo Add unit-tests for testing:
/// - addActiveIface() with invalid interface name
/// - addActiveIface() with the same interface twice
......
......@@ -169,6 +169,7 @@ TEST_F(ConfigurationTest, basic) {
// Check that Configuration can store logging information.
TEST_F(ConfigurationTest, loggingInfo) {
LoggingInfo log1;
log1.clearDestinations();
log1.name_ = "foo";
log1.severity_ = isc::log::WARN;
log1.debuglevel_ = 77;
......
......@@ -14,6 +14,7 @@
#include <config.h>
#include <exceptions/exceptions.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/daemon.h>
#include <dhcpsrv/logging.h>
#include <log/logger_unittest_support.h>
......@@ -49,6 +50,7 @@ TEST(DaemonTest, constructor) {
// More dedicated tests are availablef for LogConfigParser class.
// See logger_unittest.cc
TEST(DaemonTest, parsingConsoleOutput) {
CfgMgr::instance().setVerbose(false);
// Storage - parsed configuration will be stored here
ConfigurationPtr storage(new Configuration());
......@@ -70,7 +72,7 @@ TEST(DaemonTest, parsingConsoleOutput) {
// Spawn a daemon and tell it to configure logger
Daemon x;
EXPECT_NO_THROW(x.configureLogger(config, storage, false));
EXPECT_NO_THROW(x.configureLogger(config, storage));
// configureLogger will modify the logging options of the log4cplus logger.
// We need to reset the logger settings so as the following tests are
......
......@@ -13,6 +13,7 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <config.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/logging_info.h>
#include <gtest/gtest.h>
......@@ -51,8 +52,43 @@ TEST(LoggingDestintaion, equals) {
EXPECT_TRUE(dest1.equals(dest2));
}
/// @brief Test fixture class for testing @c LoggingInfo.
class LoggingInfoTest : public ::testing::Test {
public:
/// @brief Setup the test.
virtual void SetUp() {
CfgMgr::instance().setVerbose(false);
}
/// @brief Clear after the test.
virtual void TearDown() {
CfgMgr::instance().setVerbose(false);
}
};
// Checks if default logging configuration is correct.
TEST_F(LoggingInfoTest, defaults) {
LoggingInfo info_non_verbose;
EXPECT_EQ("kea", info_non_verbose.name_);
EXPECT_EQ(isc::log::INFO, info_non_verbose.severity_);
EXPECT_EQ(0, info_non_verbose.debuglevel_);
ASSERT_EQ(1, info_non_verbose.destinations_.size());
EXPECT_EQ("stdout", info_non_verbose.destinations_[0].output_);
CfgMgr::instance().setVerbose(true);
LoggingInfo info_verbose;
EXPECT_EQ("kea", info_verbose.name_);
EXPECT_EQ(isc::log::DEBUG, info_verbose.severity_);
EXPECT_EQ(99, info_verbose.debuglevel_);
ASSERT_EQ(1, info_verbose.destinations_.size());
EXPECT_EQ("stdout", info_verbose.destinations_[0].output_);
}
// Checks if (in)equality operators work for LoggingInfo.
TEST(LoggingInfo, equality) {
TEST_F(LoggingInfoTest, equalityOperators) {
LoggingInfo info1;
LoggingInfo info2;
......@@ -113,7 +149,7 @@ TEST(LoggingInfo, equality) {
LoggingDestination dest3;
dest3.output_ = "foobar";
info2.destinations_[1] = dest3;
info2.destinations_[2] = dest3;
// The should now be unequal.
EXPECT_FALSE(info1 == info2);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment