Commit 5916bcf3 authored by Tomek Mrugalski's avatar Tomek Mrugalski 🛰

[3400] Changes after review:

 - files renamed to {kea,bundy}_controller.cc
 - Daemon::init() is now void
 - status variable removed from main
 - added unit-tests for broken configs
parent 3476b678
...@@ -3,7 +3,6 @@ SUBDIRS = . tests ...@@ -3,7 +3,6 @@ SUBDIRS = . tests
AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
AM_CPPFLAGS += -I$(top_srcdir)/src/bin -I$(top_builddir)/src/bin AM_CPPFLAGS += -I$(top_srcdir)/src/bin -I$(top_builddir)/src/bin
AM_CPPFLAGS += -I$(top_srcdir)/src/lib/cc -I$(top_builddir)/src/lib/cc AM_CPPFLAGS += -I$(top_srcdir)/src/lib/cc -I$(top_builddir)/src/lib/cc
AM_CPPFLAGS += -DTOP_BUILDDIR="\"$(top_builddir)\""
AM_CPPFLAGS += $(BOOST_INCLUDES) AM_CPPFLAGS += $(BOOST_INCLUDES)
AM_CXXFLAGS = $(B10_CXXFLAGS) AM_CXXFLAGS = $(B10_CXXFLAGS)
...@@ -63,7 +62,7 @@ b10_dhcp6_SOURCES += bundy_controller.cc ...@@ -63,7 +62,7 @@ b10_dhcp6_SOURCES += bundy_controller.cc
endif endif
if CONFIG_BACKEND_JSON if CONFIG_BACKEND_JSON
b10_dhcp6_SOURCES += jsonfile_controller.cc b10_dhcp6_SOURCES += kea_controller.cc
endif endif
nodist_b10_dhcp6_SOURCES = dhcp6_messages.h dhcp6_messages.cc nodist_b10_dhcp6_SOURCES = dhcp6_messages.h dhcp6_messages.cc
......
...@@ -54,7 +54,7 @@ public: ...@@ -54,7 +54,7 @@ public:
/// *_backend.cc /// *_backend.cc
/// ///
/// @return true if initialization was successful, false if it failed /// @return true if initialization was successful, false if it failed
bool init(const std::string& config_file); void init(const std::string& config_file);
/// @brief Performs cleanup, immediately before termination /// @brief Performs cleanup, immediately before termination
/// ///
......
...@@ -48,7 +48,7 @@ using namespace std; ...@@ -48,7 +48,7 @@ using namespace std;
namespace isc { namespace isc {
namespace dhcp { namespace dhcp {
bool void
ControlledDhcpv6Srv::init(const std::string& file_name) { ControlledDhcpv6Srv::init(const std::string& file_name) {
// This is a configuration backend implementation that reads the // This is a configuration backend implementation that reads the
// configuration from a JSON file. // configuration from a JSON file.
...@@ -58,11 +58,13 @@ ControlledDhcpv6Srv::init(const std::string& file_name) { ...@@ -58,11 +58,13 @@ ControlledDhcpv6Srv::init(const std::string& file_name) {
isc::data::ConstElementPtr result; isc::data::ConstElementPtr result;
// Basic sanity check: file name must not be empty. // Basic sanity check: file name must not be empty.
if (file_name.empty()) {
isc_throw(BadValue, "JSON configuration file not specified");
}
try { try {
if (file_name.empty()) {
// Basic sanity check: file name must not be empty.
isc_throw(BadValue, "JSON configuration file not specified. Please"
"use -c command line option.");
}
// Read contents of the file and parse it as JSON // Read contents of the file and parse it as JSON
json = Element::fromJSONFile(file_name, true); json = Element::fromJSONFile(file_name, true);
...@@ -99,7 +101,7 @@ ControlledDhcpv6Srv::init(const std::string& file_name) { ...@@ -99,7 +101,7 @@ ControlledDhcpv6Srv::init(const std::string& file_name) {
LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL) LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL)
.arg("Configuration failed: Undefined result of configureDhcp6Server" .arg("Configuration failed: Undefined result of configureDhcp6Server"
"() function after attempting to read " + file_name); "() function after attempting to read " + file_name);
return (false); return;
} }
// Now check is the returned result is successful (rcode=0) or not // Now check is the returned result is successful (rcode=0) or not
...@@ -112,14 +114,12 @@ ControlledDhcpv6Srv::init(const std::string& file_name) { ...@@ -112,14 +114,12 @@ ControlledDhcpv6Srv::init(const std::string& file_name) {
reason = string(" (") + comment->stringValue() + string(")"); reason = string(" (") + comment->stringValue() + string(")");
} }
LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL).arg(reason); LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL).arg(reason);
return (false); isc_throw(BadValue, "Failed to apply configuration:" << reason);
} }
// We don't need to call openActiveSockets() or startD2() as these // We don't need to call openActiveSockets() or startD2() as these
// methods are called in processConfig() which is called by // methods are called in processConfig() which is called by
// processCommand("reload-config", ...) // processCommand("reload-config", ...)
return (true);
} }
void ControlledDhcpv6Srv::cleanup() { void ControlledDhcpv6Srv::cleanup() {
...@@ -129,34 +129,14 @@ void ControlledDhcpv6Srv::cleanup() { ...@@ -129,34 +129,14 @@ void ControlledDhcpv6Srv::cleanup() {
/// This is a logger initialization for JSON file backend. /// This is a logger initialization for JSON file backend.
/// For now, it's just setting log messages to be printed on stdout. /// For now, it's just setting log messages to be printed on stdout.
/// @todo: Implement this properly (see #3427) /// @todo: Implement this properly (see #3427)
void Daemon::loggerInit(const char* log_name, bool verbose, bool ) { void Daemon::loggerInit(const char*, bool verbose, bool ) {
// This method configures logger. For now it is very simple.
// We'll make it more robust once we add support for JSON-based logging setenv("B10_LOCKFILE_DIR_FROM_BUILD", "/tmp", 1);
// configuration. setenv("B10_LOGGER_ROOT", "kea", 0);
setenv("B10_LOGGER_SEVERITY", (verbose ? "DEBUG":"INFO"), 0);
using namespace isc::log; setenv("B10_LOGGER_DBGLEVEL", "99", 0);
setenv("B10_LOGGER_DESTINATION", "stdout", 0);
Severity severity = b10LoggerSeverity(verbose ? isc::log::DEBUG : isc::log::INFO); isc::log::initLogger();
// Set a directory for creating lockfiles when running tests
// @todo: Find out why this is needed. Without this, the logger doesn't
// work.
setenv("B10_LOCKFILE_DIR_FROM_BUILD", TOP_BUILDDIR, 1);
// Initialize logging
initLogger(log_name, severity, isc::log::MAX_DEBUG_LEVEL, NULL);
// Now configure logger output to stdout.
/// @todo: Make this configurable as part of #3427.
LoggerSpecification spec(log_name, severity,
b10LoggerDbglevel(isc::log::MAX_DEBUG_LEVEL));
OutputOption option;
option.destination = OutputOption::DEST_CONSOLE;
option.stream = OutputOption::STR_STDOUT;
spec.addOutputOption(option);
LoggerManager manager;
manager.process(spec);
} }
}; };
......
...@@ -105,7 +105,6 @@ main(int argc, char* argv[]) { ...@@ -105,7 +105,6 @@ main(int argc, char* argv[]) {
int ret = EXIT_SUCCESS; int ret = EXIT_SUCCESS;
bool status = true;
try { try {
// Initialize logging. If verbose, we'll use maximum verbosity. // Initialize logging. If verbose, we'll use maximum verbosity.
// If standalone is enabled, do not buffer initial log messages // If standalone is enabled, do not buffer initial log messages
...@@ -123,8 +122,8 @@ main(int argc, char* argv[]) { ...@@ -123,8 +122,8 @@ main(int argc, char* argv[]) {
try { try {
// Initialize the server, i.e. establish control session // Initialize the server, i.e. establish control session
// if BIND10 backend is used or read a configuration file // if BIND10 backend is used or read a configuration file
// server.init(config_file);
status = server.init(config_file);
} catch (const std::exception& ex) { } catch (const std::exception& ex) {
LOG_ERROR(dhcp6_logger, DHCP6_INIT_FAIL).arg(ex.what()); LOG_ERROR(dhcp6_logger, DHCP6_INIT_FAIL).arg(ex.what());
...@@ -139,9 +138,6 @@ main(int argc, char* argv[]) { ...@@ -139,9 +138,6 @@ main(int argc, char* argv[]) {
} else { } else {
LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_STANDALONE); LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_STANDALONE);
} }
if (!status) {
isc_throw(isc::Unexpected, "Failed to initialize configuration backend.");
}
server.run(); server.run();
LOG_INFO(dhcp6_logger, DHCP6_SHUTDOWN); LOG_INFO(dhcp6_logger, DHCP6_SHUTDOWN);
......
...@@ -96,8 +96,8 @@ dhcp6_unittests_SOURCES += bundy_controller_unittest.cc ...@@ -96,8 +96,8 @@ dhcp6_unittests_SOURCES += bundy_controller_unittest.cc
endif endif
if CONFIG_BACKEND_JSON if CONFIG_BACKEND_JSON
dhcp6_unittests_SOURCES += ../jsonfile_controller.cc dhcp6_unittests_SOURCES += ../kea_controller.cc
dhcp6_unittests_SOURCES += jsonfile_controller_unittest.cc dhcp6_unittests_SOURCES += kea_controller_unittest.cc
endif endif
nodist_dhcp6_unittests_SOURCES = ../dhcp6_messages.h ../dhcp6_messages.cc nodist_dhcp6_unittests_SOURCES = ../dhcp6_messages.h ../dhcp6_messages.cc
......
...@@ -101,7 +101,7 @@ TEST_F(JSONFileBackendTest, jsonFile) { ...@@ -101,7 +101,7 @@ TEST_F(JSONFileBackendTest, jsonFile) {
); );
// And configure it using the config file. // And configure it using the config file.
EXPECT_TRUE(srv->init(TEST_FILE)); EXPECT_NO_THROW(srv->init(TEST_FILE));
// Now check if the configuration has been applied correctly. // Now check if the configuration has been applied correctly.
const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6(); const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6();
...@@ -171,7 +171,7 @@ TEST_F(JSONFileBackendTest, comments) { ...@@ -171,7 +171,7 @@ TEST_F(JSONFileBackendTest, comments) {
); );
// And configure it using config without // And configure it using config without
EXPECT_TRUE(srv->init(TEST_FILE)); EXPECT_NO_THROW(srv->init(TEST_FILE));
// Now check if the configuration has been applied correctly. // Now check if the configuration has been applied correctly.
const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6(); const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6();
...@@ -190,4 +190,49 @@ TEST_F(JSONFileBackendTest, comments) { ...@@ -190,4 +190,49 @@ TEST_F(JSONFileBackendTest, comments) {
EXPECT_EQ(Lease::TYPE_NA, pools1.at(0)->getType()); EXPECT_EQ(Lease::TYPE_NA, pools1.at(0)->getType());
} }
// This test checks if configuration detects failure when trying:
// - empty file
// - empty filename
// - no Dhcp6 element
// - Config file that contains Dhcp6 but has a content error
TEST_F(JSONFileBackendTest, configBroken) {
// Empty config is not allowed, because Dhcp6 element is missing
string config_empty = "";
// This config does not have mandatory Dhcp6 element
string config_v4 = "{ \"Dhcp4\": { \"interfaces\": [ \"*\" ],"
"\"preferred-lifetime\": 3000,"
"\"rebind-timer\": 2000, "
"\"renew-timer\": 1000, "
"\"subnet4\": [ { "
" \"pool\": [ \"192.0.2.0/24\" ],"
" \"subnet\": \"192.0.2.0/24\" "
" } ]}";
// This has Dhcp6 element, but it's utter nonsense
string config_nonsense = "{ \"Dhcp6\": { \"reviews\": \"are so much fun\" } }";
// Now initialize the server
boost::scoped_ptr<ControlledDhcpv6Srv> srv;
ASSERT_NO_THROW(
srv.reset(new ControlledDhcpv6Srv(0))
);
// Try to configure without filename. Should fail.
EXPECT_THROW(srv->init(""), BadValue);
// Try to configure it using empty file. Should fail.
writeFile(TEST_FILE, config_empty);
EXPECT_THROW(srv->init(TEST_FILE), BadValue);
// Now try to load a config that does not have Dhcp6 component.
writeFile(TEST_FILE, config_v4);
EXPECT_THROW(srv->init(TEST_FILE), BadValue);
// Now try to load a config with Dhcp6 full of nonsense.
writeFile(TEST_FILE, config_nonsense);
EXPECT_THROW(srv->init(TEST_FILE), BadValue);
}
} // End of anonymous namespace } // End of anonymous namespace
...@@ -27,7 +27,7 @@ namespace dhcp { ...@@ -27,7 +27,7 @@ namespace dhcp {
Daemon::Daemon() { Daemon::Daemon() {
} }
bool Daemon::init(const std::string&) { void Daemon::init(const std::string&) {
return false; return false;
} }
......
...@@ -61,8 +61,8 @@ public: ...@@ -61,8 +61,8 @@ public:
/// likely that the D2 will be started, it will analyze its config file, /// likely that the D2 will be started, it will analyze its config file,
/// decide that it is not needed and will shut down. /// decide that it is not needed and will shut down.
/// ///
/// @return true if initialization was successful, false if it was not /// @note this method may throw
virtual bool init(const std::string& config_file); virtual void init(const std::string& config_file);
/// @brief Performs final deconfiguration. /// @brief Performs final deconfiguration.
/// ///
......
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