diff --git a/src/bin/dhcp6/Makefile.am b/src/bin/dhcp6/Makefile.am index 60e9d0f33cb084a704ea60c5eae58a2603c8da5c..8f0dd76f1240d885b15f25dabba9f8c090d15b40 100644 --- a/src/bin/dhcp6/Makefile.am +++ b/src/bin/dhcp6/Makefile.am @@ -3,7 +3,6 @@ SUBDIRS = . tests 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/lib/cc -I$(top_builddir)/src/lib/cc -AM_CPPFLAGS += -DTOP_BUILDDIR="\"$(top_builddir)\"" AM_CPPFLAGS += $(BOOST_INCLUDES) AM_CXXFLAGS = $(B10_CXXFLAGS) @@ -63,7 +62,7 @@ b10_dhcp6_SOURCES += bundy_controller.cc endif if CONFIG_BACKEND_JSON -b10_dhcp6_SOURCES += jsonfile_controller.cc +b10_dhcp6_SOURCES += kea_controller.cc endif nodist_b10_dhcp6_SOURCES = dhcp6_messages.h dhcp6_messages.cc diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index 632007a041eb07cf97b73727df2bf27e10afa311..c0a5a5709c4e13e9e07bf8d4cd442e650f8c43ff 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -54,7 +54,7 @@ public: /// *_backend.cc /// /// @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 /// diff --git a/src/bin/dhcp6/kea_controller.cc b/src/bin/dhcp6/kea_controller.cc index 2196d23462358364c7257515c97b40b60a4c965d..7e42df9ede3e805beb1db9d7dac78c5afdcedfcb 100644 --- a/src/bin/dhcp6/kea_controller.cc +++ b/src/bin/dhcp6/kea_controller.cc @@ -48,7 +48,7 @@ using namespace std; namespace isc { namespace dhcp { -bool +void ControlledDhcpv6Srv::init(const std::string& file_name) { // This is a configuration backend implementation that reads the // configuration from a JSON file. @@ -58,11 +58,13 @@ ControlledDhcpv6Srv::init(const std::string& file_name) { isc::data::ConstElementPtr result; // Basic sanity check: file name must not be empty. - if (file_name.empty()) { - isc_throw(BadValue, "JSON configuration file not specified"); - } - 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 json = Element::fromJSONFile(file_name, true); @@ -99,7 +101,7 @@ ControlledDhcpv6Srv::init(const std::string& file_name) { LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL) .arg("Configuration failed: Undefined result of configureDhcp6Server" "() function after attempting to read " + file_name); - return (false); + return; } // Now check is the returned result is successful (rcode=0) or not @@ -112,14 +114,12 @@ ControlledDhcpv6Srv::init(const std::string& file_name) { reason = string(" (") + comment->stringValue() + string(")"); } 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 // methods are called in processConfig() which is called by // processCommand("reload-config", ...) - - return (true); } void ControlledDhcpv6Srv::cleanup() { @@ -129,34 +129,14 @@ void ControlledDhcpv6Srv::cleanup() { /// This is a logger initialization for JSON file backend. /// For now, it's just setting log messages to be printed on stdout. /// @todo: Implement this properly (see #3427) -void Daemon::loggerInit(const char* log_name, 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 - // configuration. - - using namespace isc::log; - - Severity severity = b10LoggerSeverity(verbose ? isc::log::DEBUG : isc::log::INFO); - - // 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); +void Daemon::loggerInit(const char*, bool verbose, bool ) { + + setenv("B10_LOCKFILE_DIR_FROM_BUILD", "/tmp", 1); + setenv("B10_LOGGER_ROOT", "kea", 0); + setenv("B10_LOGGER_SEVERITY", (verbose ? "DEBUG":"INFO"), 0); + setenv("B10_LOGGER_DBGLEVEL", "99", 0); + setenv("B10_LOGGER_DESTINATION", "stdout", 0); + isc::log::initLogger(); } }; diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc index 08740eb7324833aec7ee499f5a9f46aefa0a66f1..49c70ec319227d5c334a91adb649e94e8c1e52a9 100644 --- a/src/bin/dhcp6/main.cc +++ b/src/bin/dhcp6/main.cc @@ -105,7 +105,6 @@ main(int argc, char* argv[]) { int ret = EXIT_SUCCESS; - bool status = true; try { // Initialize logging. If verbose, we'll use maximum verbosity. // If standalone is enabled, do not buffer initial log messages @@ -123,8 +122,8 @@ main(int argc, char* argv[]) { try { // Initialize the server, i.e. establish control session // if BIND10 backend is used or read a configuration file - // - status = server.init(config_file); + server.init(config_file); + } catch (const std::exception& ex) { LOG_ERROR(dhcp6_logger, DHCP6_INIT_FAIL).arg(ex.what()); @@ -139,9 +138,6 @@ main(int argc, char* argv[]) { } else { LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_STANDALONE); } - if (!status) { - isc_throw(isc::Unexpected, "Failed to initialize configuration backend."); - } server.run(); LOG_INFO(dhcp6_logger, DHCP6_SHUTDOWN); diff --git a/src/bin/dhcp6/tests/Makefile.am b/src/bin/dhcp6/tests/Makefile.am index f09eb3c18b2e84f941c8dfe1e624614aef4a4e66..1c8b6e5a69b32b11ccbe792570ca6fcbc31a3018 100644 --- a/src/bin/dhcp6/tests/Makefile.am +++ b/src/bin/dhcp6/tests/Makefile.am @@ -96,8 +96,8 @@ dhcp6_unittests_SOURCES += bundy_controller_unittest.cc endif if CONFIG_BACKEND_JSON -dhcp6_unittests_SOURCES += ../jsonfile_controller.cc -dhcp6_unittests_SOURCES += jsonfile_controller_unittest.cc +dhcp6_unittests_SOURCES += ../kea_controller.cc +dhcp6_unittests_SOURCES += kea_controller_unittest.cc endif nodist_dhcp6_unittests_SOURCES = ../dhcp6_messages.h ../dhcp6_messages.cc diff --git a/src/bin/dhcp6/tests/kea_controller_unittest.cc b/src/bin/dhcp6/tests/kea_controller_unittest.cc index 1f70c042e1ea784114a9c15a5b74c6bdf98350a4..3198561345742e7e54981fd2dd394203f74ebc88 100644 --- a/src/bin/dhcp6/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp6/tests/kea_controller_unittest.cc @@ -101,7 +101,7 @@ TEST_F(JSONFileBackendTest, jsonFile) { ); // 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. const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6(); @@ -171,7 +171,7 @@ TEST_F(JSONFileBackendTest, comments) { ); // 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. const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6(); @@ -190,4 +190,49 @@ TEST_F(JSONFileBackendTest, comments) { 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 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 diff --git a/src/lib/dhcpsrv/daemon.cc b/src/lib/dhcpsrv/daemon.cc index c4ccfb86e8559c08f29de5018dfab2a27652a1ff..bb9566745e3d8f7bdd6d82d0abed346917629133 100644 --- a/src/lib/dhcpsrv/daemon.cc +++ b/src/lib/dhcpsrv/daemon.cc @@ -27,7 +27,7 @@ namespace dhcp { Daemon::Daemon() { } -bool Daemon::init(const std::string&) { +void Daemon::init(const std::string&) { return false; } diff --git a/src/lib/dhcpsrv/daemon.h b/src/lib/dhcpsrv/daemon.h index fe17938ca576c30fc7f494169a22ef7e5798fd5f..7c144890447242fc3b782e715446474fd77ee354 100644 --- a/src/lib/dhcpsrv/daemon.h +++ b/src/lib/dhcpsrv/daemon.h @@ -61,8 +61,8 @@ public: /// likely that the D2 will be started, it will analyze its config file, /// decide that it is not needed and will shut down. /// - /// @return true if initialization was successful, false if it was not - virtual bool init(const std::string& config_file); + /// @note this method may throw + virtual void init(const std::string& config_file); /// @brief Performs final deconfiguration. ///