From: Tomek Mrugalski Date: Fri, 30 May 2014 12:24:25 +0000 (+0200) Subject: [3399] Changes after review X-Git-Tag: trac3434_base~15^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6eb7220674f9eaa25e7afcffc7407360ab161ee7;p=thirdparty%2Fkea.git [3399] Changes after review - ChangLog updated (b10-dhcp4 => DHCPv4 server) - stand-alone mode removed in Kea4,Kea6 - getInstance() used instead of server_ - ctrl_dhcp4_srv.cc: one stringstream instance now shared by all error messages - ctrl_dhcp4_srv.h: comments cleaned up - Added log message for received configuration - kea_controller.cc: Unnecessary header/using namespace removed - *.json files are now cleaned up in src/bin/dhcp4/tests - added comment that explains lack of tests for Bundy backend - kea_controller_unittest.cc now uses the same filename everywhere Changes also applied to Kea6 (where applicable). --- diff --git a/ChangeLog b/ChangeLog index b3f0b7ac37..bb12ba4ff1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +7XX. [func] tomek + DHCPv4 server: New parameter added to configure.ac: --with-kea-config. + It allows selecting configuration backend and accepts one of two + values: BUNDY, which uses Bundy (former BIND10) framework as Kea + 0.8 did, or JSON, which reads configuration from a JSON file. + (Trac #3399, git TBD) + 784. [func] tmark DHCP_DDNS's configuration was changed. The unused parameter, "interface" was deleted. Three new parameters, "ncr_protocol", "ncr_format", and @@ -6,7 +13,7 @@ (Trac #3268, git bd60252e679f19b062f61926647f661ab169f21c) 783. [func]* tomek - b10-dhcp6: New parameter added to configure: --with-kea-config. + DHCPv6 server: New parameter added to configure: --with-kea-config. It allows selecting configuration backend and accepts one of two values: BUNDY, which uses Bundy (former BIND10 framework as Kea 0.8 did, or JSON, which reads configuration from a JSON file. diff --git a/doc/examples/kea4/single-subnet.json b/doc/examples/kea4/single-subnet.json index 9afef91ff4..b4717da393 100644 --- a/doc/examples/kea4/single-subnet.json +++ b/doc/examples/kea4/single-subnet.json @@ -1,4 +1,4 @@ -# This is an example configuration file for DHCPv4 server in Kea. +# This is an example configuration file for the DHCPv4 server in Kea. # It is a basic scenario with one IPv4 subnet configured. The subnet # contains a single pool of dynamically allocated addresses. diff --git a/src/bin/dhcp4/bundy_controller.cc b/src/bin/dhcp4/bundy_controller.cc index dec9689843..9e0984cee1 100644 --- a/src/bin/dhcp4/bundy_controller.cc +++ b/src/bin/dhcp4/bundy_controller.cc @@ -209,10 +209,10 @@ void ControlledDhcpv4Srv::cleanup() { } void -Daemon::loggerInit(const char* log_name, bool verbose, bool stand_alone) { +Daemon::loggerInit(const char* log_name, bool verbose) { isc::log::initLogger(log_name, (verbose ? isc::log::DEBUG : isc::log::INFO), - isc::log::MAX_DEBUG_LEVEL, NULL, !stand_alone); + isc::log::MAX_DEBUG_LEVEL, NULL, true); } }; diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index dd6fc4f9b9..dd7c943bcf 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -30,8 +30,8 @@ ControlledDhcpv4Srv* ControlledDhcpv4Srv::server_ = NULL; ConstElementPtr ControlledDhcpv4Srv::commandShutdownHandler(const string&, ConstElementPtr) { - if (ControlledDhcpv4Srv::server_) { - ControlledDhcpv4Srv::server_->shutdown(); + if (ControlledDhcpv4Srv::getInstance()) { + ControlledDhcpv4Srv::getInstance()->shutdown(); } else { LOG_WARN(dhcp4_logger, DHCP4_NOT_RUNNING); ConstElementPtr answer = isc::config::createAnswer(1, @@ -76,8 +76,8 @@ ControlledDhcpv4Srv::processCommand(const string& command, if (!srv) { ConstElementPtr no_srv = isc::config::createAnswer(1, - "Server object not initialized, can't process command '" + - command + "'."); + "Server object not initialized, so can't process command '" + + command + "', arguments: '" + args->str() + "'."); return (no_srv); } @@ -103,16 +103,23 @@ ControlledDhcpv4Srv::processCommand(const string& command, isc::data::ConstElementPtr ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { + + LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_RECEIVED) + .arg(config->str()); + ControlledDhcpv4Srv* srv = ControlledDhcpv4Srv::getInstance(); + // Single stream instance used in all error clauses + std::ostringstream err; + if (!srv) { - ConstElementPtr no_srv = isc::config::createAnswer(1, - "Server object not initialized, can't process config."); - return (no_srv); + err << "Server object not initialized, can't process config."; + return (isc::config::createAnswer(1, err.str())); } ConstElementPtr answer = configureDhcp4Server(*srv, config); + // Check that configuration was successful. If not, do not reopen sockets // and don't bother with DDNS stuff. try { @@ -122,29 +129,28 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { return (answer); } } catch (const std::exception& ex) { - return (isc::config::createAnswer(1, "Failed to process configuration:" - + string(ex.what()))); + err << "Failed to process configuration:" << ex.what(); + return (isc::config::createAnswer(1, err.str())); } // Server will start DDNS communications if its enabled. try { srv->startD2(); } catch (const std::exception& ex) { - std::ostringstream err; - err << "error starting DHCP_DDNS client " - " after server reconfiguration: " << ex.what(); + err << "Error starting DHCP_DDNS client after server reconfiguration: " + << ex.what(); return (isc::config::createAnswer(1, err.str())); } // Configuration may change active interfaces. Therefore, we have to reopen // sockets according to new configuration. This operation is not exception - // safe and we really don't want to emit exceptions to the callback caller. - // Instead, catch an exception and create appropriate answer. + // safe and we really don't want to emit exceptions to whoever called this + // method. Instead, catch an exception and create appropriate answer. try { - srv->openActiveSockets(srv->getPort(), server_->useBroadcast()); + srv->openActiveSockets(srv->getPort(), getInstance()->useBroadcast()); } catch (std::exception& ex) { - std::ostringstream err; - err << "failed to open sockets after server reconfiguration: " << ex.what(); + err << "failed to open sockets after server reconfiguration: " + << ex.what(); answer = isc::config::createAnswer(1, err.str()); } return (answer); @@ -152,11 +158,11 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/) :Dhcpv4Srv(port) { - if (server_) { + if (getInstance()) { isc_throw(InvalidOperation, "There is another Dhcpv4Srv instance already."); } - server_ = this; // remember this instance for use in callback + server_ = this; // remember this instance for later use in handlers } void ControlledDhcpv4Srv::shutdown() { @@ -167,15 +173,15 @@ void ControlledDhcpv4Srv::shutdown() { ControlledDhcpv4Srv::~ControlledDhcpv4Srv() { cleanup(); - server_ = NULL; // forget this instance. There should be no callback anymore - // at this stage anyway. + server_ = NULL; // forget this instance. Noone should call any handlers at + // this stage. } void ControlledDhcpv4Srv::sessionReader(void) { // Process one asio event. If there are more events, iface_mgr will call // this callback more than once. - if (server_) { - server_->io_service_.run_one(); + if (getInstance()) { + getInstance()->io_service_.run_one(); } } diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index d590772f1d..2717ae15c0 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -26,12 +26,10 @@ namespace dhcp { /// @brief Controlled version of the DHCPv4 server /// -/// This is a class that is responsible for establishing connection -/// with msqg (receving commands and configuration). This is an extended -/// version of Dhcpv4Srv class that is purely a DHCPv4 server, without -/// external control. ControlledDhcpv4Srv should be used in typical BIND10 -/// (i.e. featuring msgq) environment, while Dhcpv4Srv should be used in -/// embedded environments. +/// This is a class that is responsible for DHCPv4 server being controllable. +/// It does various things, depending on the configuration backend. +/// For Bundy backend it establishes a connection with msqg and later receives +/// commands over it. For Kea backend, it reads configuration file from disk. /// /// For detailed explanation or relations between main(), ControlledDhcpv4Srv, /// Dhcpv4Srv and other classes, see \ref dhcpv4Session. @@ -53,7 +51,8 @@ public: /// operation. For specific details, see actual implementation in /// *_backend.cc /// - /// @return true if initialization was successful, false if it failed + /// This method may throw if initialization fails. Exception types may be + /// specific to used configuration backend. void init(const std::string& config_file); /// @brief Performs cleanup, immediately before termination @@ -77,6 +76,11 @@ public: /// wrapper that calls process*Command() methods and catches exceptions /// in them. /// + /// Currently supported commands are: + /// - shutdown + /// - libreload + /// - config-reload + /// /// @note It never throws. /// /// @param command Text represenation of the command (e.g. "shutdown") @@ -88,7 +92,7 @@ public: /// @brief Configuration processor /// - /// This is a callback for handling incoming configuration updates. + /// This is a method for handling incoming configuration updates. /// This method should be called by all configuration backends when the /// server is starting up or when configuration has changed. /// @@ -103,7 +107,7 @@ public: /// @brief Returns pointer to the sole instance of Dhcpv4Srv /// - /// @note may return NULL, if called before server is spawned + /// @return server instance (may return NULL, if called before server is spawned) static ControlledDhcpv4Srv* getInstance() { return (server_); } diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 40215a2583..9777391b37 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -44,6 +44,10 @@ name was malformed or due to internal server error. A debug message listing the command (and possible arguments) received from the BIND 10 control system by the DHCPv4 server. +% DHCP4_CONFIG_RECEIVED received configuration %1 +A debug message listing the configuration received by the DHCPv4 server. +The source of that configuration depends on used configuration backend. + % DHCP4_CONFIG_COMPLETE DHCPv4 server has completed configuration: %1 This is an informational message announcing the successful processing of a new configuration. It is output during server startup, and when an updated @@ -339,17 +343,11 @@ core component within the DHCPv4 server (the Dhcpv4 server object) has failed. As a result, the server will exit. The reason for the failure is given within the message. -% DHCP4_STANDALONE skipping message queue, running standalone -This is a debug message indicating that the DHCPv4 server is running in -standalone mode, not connected to the message queue. Standalone mode -is only useful during program development, and should not be used in a -production environment. - % DHCP4_STARTING server starting This informational message indicates that the DHCPv4 server has processed any command-line switches and is starting. -% DHCP4_START_INFO pid: %1, port: %2, verbose: %3, standalone: %4 +% DHCP4_START_INFO pid: %1, port: %2, verbose: %3 This is a debug message issued during the DHCPv4 server startup. It lists some information about the parameters with which the server is running. diff --git a/src/bin/dhcp4/kea_controller.cc b/src/bin/dhcp4/kea_controller.cc index 53d601eb3d..3984fd2894 100644 --- a/src/bin/dhcp4/kea_controller.cc +++ b/src/bin/dhcp4/kea_controller.cc @@ -14,35 +14,15 @@ #include -#include -#include -#include -#include #include #include #include -#include -#include -#include -#include -#include -#include -#include #include -#include -#include -#include #include -#include using namespace isc::asiolink; -using namespace isc::cc; -using namespace isc::config; -using namespace isc::data; using namespace isc::dhcp; -using namespace isc::log; -using namespace isc::util; using namespace std; namespace isc { @@ -66,7 +46,7 @@ ControlledDhcpv4Srv::init(const std::string& file_name) { } // Read contents of the file and parse it as JSON - json = Element::fromJSONFile(file_name, true); + json = isc::data::Element::fromJSONFile(file_name, true); if (!json) { LOG_ERROR(dhcp4_logger, DHCP4_CONFIG_LOAD_FAIL) @@ -105,9 +85,9 @@ ControlledDhcpv4Srv::init(const std::string& file_name) { } // Now check is the returned result is successful (rcode=0) or not - ConstElementPtr comment; /// see @ref isc::config::parseAnswer + isc::data::ConstElementPtr comment; /// see @ref isc::config::parseAnswer int rcode; - comment = parseAnswer(rcode, result); + comment = isc::config::parseAnswer(rcode, result); if (rcode != 0) { string reason = ""; if (comment) { @@ -129,7 +109,7 @@ void ControlledDhcpv4Srv::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*, bool verbose, bool ) { +void Daemon::loggerInit(const char*, bool verbose) { setenv("B10_LOCKFILE_DIR_FROM_BUILD", "/tmp", 1); setenv("B10_LOGGER_ROOT", "kea", 0); diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc index 1508995d4e..99e49d42fb 100644 --- a/src/bin/dhcp4/main.cc +++ b/src/bin/dhcp4/main.cc @@ -43,9 +43,8 @@ const char* const DHCP4_LOGGER_NAME = "kea"; void usage() { - cerr << "Usage: " << DHCP4_NAME << " [-v] [-s] [-p number]" << endl; + cerr << "Usage: " << DHCP4_NAME << " [-v] [-p number] [-c file]" << endl; cerr << " -v: verbose output" << endl; - cerr << " -s: stand-alone mode (don't connect to BIND10)" << endl; cerr << " -p number: specify non-standard port number 1-65535 " << "(useful for testing only)" << endl; cerr << " -c file: specify configuration file" << endl; @@ -58,22 +57,17 @@ main(int argc, char* argv[]) { int ch; int port_number = DHCP4_SERVER_PORT; // The default. any other values are // useful for testing only. - bool stand_alone = false; // Should be connect to BIND10 msgq? bool verbose_mode = false; // Should server be verbose? // The standard config file std::string config_file(""); - while ((ch = getopt(argc, argv, "vsp:c:")) != -1) { + while ((ch = getopt(argc, argv, "vp:c:")) != -1) { switch (ch) { case 'v': verbose_mode = true; break; - case 's': - stand_alone = true; - break; - case 'p': try { port_number = boost::lexical_cast(optarg); @@ -108,35 +102,34 @@ main(int argc, char* argv[]) { try { // Initialize logging. If verbose, we'll use maximum verbosity. // If standalone is enabled, do not buffer initial log messages - Daemon::loggerInit(DHCP4_LOGGER_NAME, verbose_mode, stand_alone); + Daemon::loggerInit(DHCP4_LOGGER_NAME, verbose_mode); LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_START_INFO) - .arg(getpid()).arg(port_number).arg(verbose_mode ? "yes" : "no") - .arg(stand_alone ? "yes" : "no" ); - + .arg(getpid()).arg(port_number).arg(verbose_mode ? "yes" : "no"); + LOG_INFO(dhcp4_logger, DHCP4_STARTING); + // Create the server instance. ControlledDhcpv4Srv server(port_number); - if (!stand_alone) { - - try { - server.init(config_file); - } catch (const std::exception& ex) { - LOG_ERROR(dhcp4_logger, DHCP4_SESSION_FAIL).arg(ex.what()); + try { + // Initialize the server. + server.init(config_file); + } catch (const std::exception& ex) { + LOG_ERROR(dhcp4_logger, DHCP4_SESSION_FAIL).arg(ex.what()); - // We should not continue if were told to configure (either read - // config file or establish Bundy control session). + // We should not continue if were told to configure (either read + // config file or establish Bundy control session). - isc::log::LoggerManager log_manager; - log_manager.process(); + isc::log::LoggerManager log_manager; + log_manager.process(); - cerr << "Failed to initialize server: " << ex.what() << endl; - return (EXIT_FAILURE); - } - } else { - LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_STANDALONE); + cerr << "Failed to initialize server: " << ex.what() << endl; + return (EXIT_FAILURE); } + + // And run the main loop of the server. server.run(); + LOG_INFO(dhcp4_logger, DHCP4_SHUTDOWN); } catch (const std::exception& ex) { diff --git a/src/bin/dhcp4/tests/Makefile.am b/src/bin/dhcp4/tests/Makefile.am index 2c28fb14cd..c835aaa1bf 100644 --- a/src/bin/dhcp4/tests/Makefile.am +++ b/src/bin/dhcp4/tests/Makefile.am @@ -33,6 +33,7 @@ AM_CPPFLAGS += -DINSTALL_PROG=\"$(abs_top_srcdir)/install-sh\" CLEANFILES = $(builddir)/interfaces.txt $(builddir)/logger_lockfile CLEANFILES += $(builddir)/load_marker.txt $(builddir)/unload_marker.txt +CLEANFILES += *.json AM_CXXFLAGS = $(B10_CXXFLAGS) if USE_CLANGPP diff --git a/src/bin/dhcp4/tests/bundy_controller_unittest.cc b/src/bin/dhcp4/tests/bundy_controller_unittest.cc index f556560a14..0578ec6fe6 100644 --- a/src/bin/dhcp4/tests/bundy_controller_unittest.cc +++ b/src/bin/dhcp4/tests/bundy_controller_unittest.cc @@ -18,6 +18,10 @@ namespace { +/// As of May 2014, maintaining or extending Bundy support is very low +/// prority for Kea team. We are looking for contributors, who would +/// like to maintain this backend. + // Bundy framework specific tests should be added here. TEST(BundyBackendTest, dummy) { diff --git a/src/bin/dhcp4/tests/dhcp4_test.py b/src/bin/dhcp4/tests/dhcp4_test.py index 276456e01d..37e9629d6f 100644 --- a/src/bin/dhcp4/tests/dhcp4_test.py +++ b/src/bin/dhcp4/tests/dhcp4_test.py @@ -13,7 +13,7 @@ # NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -from init import ProcessInfo, parse_args, dump_pid, unlink_pid_file, _BASETIME +from init import ProcessInfo import unittest import sys @@ -207,22 +207,5 @@ class TestDhcpv4Daemon(unittest.TestCase): # Check that there is an error message about invalid port number printed on stderr self.assertEqual( str(error).count("Failed to parse port number"), 1) - def test_portnumber_nonroot(self): - print("Check that specifying unprivileged port number will work.") - - # Check that there is a message about running with an unprivileged port - (returncode, output, error) = self.runCommand(['../b10-dhcp4', '-v', '-s', '-p', '10057']) - output_text = str(output) + str(error) - self.assertEqual(output_text.count("DHCP4_OPEN_SOCKET opening sockets on port 10057"), 1) - - def test_skip_msgq(self): - print("Check that connection to BIND10 msgq can be disabled.") - - # Check that the system outputs a message on one of its streams about running - # standalone. - (returncode, output, error) = self.runCommand(['../b10-dhcp4', '-v', '-s', '-p', '10057']) - output_text = str(output) + str(error) - self.assertEqual(output_text.count("DHCP4_STANDALONE"), 1) - if __name__ == '__main__': unittest.main() diff --git a/src/bin/dhcp4/tests/kea_controller_unittest.cc b/src/bin/dhcp4/tests/kea_controller_unittest.cc index 538016e991..756314838b 100644 --- a/src/bin/dhcp4/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp4/tests/kea_controller_unittest.cc @@ -45,7 +45,9 @@ public: NakedControlledDhcpv4Srv():ControlledDhcpv4Srv(0) { } }; - +/// @brief test class for Kea configuration backend +/// +/// This class is used for testing class JSONFileBackendTest : public ::testing::Test { public: JSONFileBackendTest() { @@ -55,10 +57,10 @@ public: static_cast(unlink(TEST_FILE)); }; - void writeFile(const std::string& file_name, const std::string& content) { - static_cast(unlink(file_name.c_str())); + void writeFile(const std::string& content) { + static_cast(unlink(TEST_FILE)); - ofstream out(file_name.c_str(), ios::trunc); + ofstream out(TEST_FILE, ios::trunc); EXPECT_TRUE(out.is_open()); out << content; out.close(); @@ -92,7 +94,7 @@ TEST_F(JSONFileBackendTest, jsonFile) { "\"valid-lifetime\": 4000 }" "}"; - writeFile(TEST_FILE, config); + writeFile(config); // Now initialize the server boost::scoped_ptr srv; @@ -161,7 +163,7 @@ TEST_F(JSONFileBackendTest, comments) { /// @todo: Implement C++-style (// ...) comments /// @todo: Implement C-style (/* ... */) comments - writeFile(TEST_FILE, config_hash_comments); + writeFile(config_hash_comments); // Now initialize the server boost::scoped_ptr srv; @@ -169,7 +171,7 @@ TEST_F(JSONFileBackendTest, comments) { srv.reset(new ControlledDhcpv4Srv(0)) ); - // And configure it using config without + // And configure it using config with comments. EXPECT_NO_THROW(srv->init(TEST_FILE)); // Now check if the configuration has been applied correctly. @@ -222,34 +224,34 @@ TEST_F(JSONFileBackendTest, configBroken) { EXPECT_THROW(srv->init(""), BadValue); // Try to configure it using empty file. Should fail. - writeFile(TEST_FILE, config_empty); + writeFile(config_empty); EXPECT_THROW(srv->init(TEST_FILE), BadValue); // Now try to load a config that does not have Dhcp4 component. - writeFile(TEST_FILE, config_v4); + writeFile(config_v4); EXPECT_THROW(srv->init(TEST_FILE), BadValue); // Now try to load a config with Dhcp4 full of nonsense. - writeFile(TEST_FILE, config_nonsense); + writeFile(config_nonsense); EXPECT_THROW(srv->init(TEST_FILE), BadValue); } -// This unit-test reads all files enumerated in configs-test.txt file, loads -// each of them and verify that they can be loaded. -// -// @todo: Unfortunately, we have this test disabled, because all loaded -// configs use memfile, which attempts to create lease file in -// /usr/local/var/bind10/kea-leases4.csv. We have couple options here: -// a) disable persistence in example configs - a very bad thing to do -// as users will forget to reenable it and then will be surprised when their -// leases disappear -// b) change configs to store lease file in /tmp. It's almost as bad as the -// previous one. Users will then be displeased when all their leases are -// wiped. (most systems wipe /tmp during boot) -// c) read each config and rewrite it on the fly, so persistence is disabled. -// This is probably the way to go, but this is a work for a dedicated ticket. -// -// Hence I'm leaving the test in, but it is disabled. +/// This unit-test reads all files enumerated in configs-test.txt file, loads +/// each of them and verify that they can be loaded. +/// +/// @todo: Unfortunately, we have this test disabled, because all loaded +/// configs use memfile, which attempts to create lease file in +/// /usr/local/var/bind10/kea-leases4.csv. We have couple options here: +/// a) disable persistence in example configs - a very bad thing to do +/// as users will forget to reenable it and then will be surprised when their +/// leases disappear +/// b) change configs to store lease file in /tmp. It's almost as bad as the +/// previous one. Users will then be displeased when all their leases are +/// wiped. (most systems wipe /tmp during boot) +/// c) read each config and rewrite it on the fly, so persistence is disabled. +/// This is probably the way to go, but this is a work for a dedicated ticket. +/// +/// Hence I'm leaving the test in, but it is disabled. TEST_F(JSONFileBackendTest, DISABLED_loadAllConfigs) { // Create server first diff --git a/src/bin/dhcp6/bundy_controller.cc b/src/bin/dhcp6/bundy_controller.cc index 2a6b4092da..9a261ff7ec 100644 --- a/src/bin/dhcp6/bundy_controller.cc +++ b/src/bin/dhcp6/bundy_controller.cc @@ -217,10 +217,10 @@ void ControlledDhcpv6Srv::cleanup() { } void -Daemon::loggerInit(const char* log_name, bool verbose, bool stand_alone) { +Daemon::loggerInit(const char* log_name, bool verbose) { isc::log::initLogger(log_name, (verbose ? isc::log::DEBUG : isc::log::INFO), - isc::log::MAX_DEBUG_LEVEL, NULL, !stand_alone); + isc::log::MAX_DEBUG_LEVEL, NULL, true); } }; // end of isc::dhcp namespace diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index c5deadbf83..950c5baf55 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -99,9 +99,12 @@ ControlledDhcpv6Srv::processCommand(const std::string& command, } } - isc::data::ConstElementPtr ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { + + LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_RECEIVED) + .arg(config->str()); + ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance(); if (!srv) { diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index caad18b751..9789bb077f 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -26,12 +26,10 @@ namespace dhcp { /// @brief Controlled version of the DHCPv6 server /// -/// This is a class that is responsible for establishing connection -/// with msqg (receving commands and configuration). This is an extended -/// version of Dhcpv6Srv class that is purely a DHCPv6 server, without -/// external control. ControlledDhcpv6Srv should be used in typical BIND10 -/// (i.e. featuring msgq) environment, while Dhcpv6Srv should be used in -/// embedded environments. +/// This is a class that is responsible for DHCPv6 server being controllable. +/// It does various things, depending on the configuration backend. +/// For Bundy backend it establishes a connection with msqg and later receives +/// commands over it. For Kea backend, it reads configuration file from disk. /// /// For detailed explanation or relations between main(), ControlledDhcpv6Srv, /// Dhcpv6Srv and other classes, see \ref dhcpv6Session. @@ -53,7 +51,8 @@ public: /// operation. For specific details, see actual implementation in /// *_backend.cc /// - /// @return true if initialization was successful, false if it failed + /// This method may throw if initialization fails. Exception types may be + /// specific to used configuration backend. void init(const std::string& config_file); /// @brief Performs cleanup, immediately before termination @@ -77,6 +76,11 @@ public: /// wrapper that calls process*Command() methods and catches exceptions /// in them. /// + /// Currently supported commands are: + /// - shutdown + /// - libreload + /// - config-reload + /// /// @note It never throws. /// /// @param command Text represenation of the command (e.g. "shutdown") @@ -88,7 +92,7 @@ public: /// @brief configuration processor /// - /// This is a callback for handling incoming configuration updates. + /// This is a method for handling incoming configuration updates. /// This method should be called by all configuration backends when the /// server is starting up or when configuration has changed. /// @@ -103,7 +107,7 @@ public: /// @brief returns pointer to the sole instance of Dhcpv6Srv /// - /// @note may return NULL, if called before server is spawned + /// @return server instance (may return NULL, if called before server is spawned) static ControlledDhcpv6Srv* getInstance() { return (server_); } diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index 828ab65f04..2c39645aeb 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -40,6 +40,10 @@ address assignment. The most likely cause is a problem with the client. A debug message listing the command (and possible arguments) received from the BIND 10 control system by the IPv6 DHCP server. +% DHCP6_CONFIG_RECEIVED received configuration: %1 +A debug message listing the configuration received by the DHCPv6 server. +The source of that configuration depends on used configuration backend. + % DHCP6_CONFIG_COMPLETE DHCPv6 server has completed configuration: %1 This is an informational message announcing the successful processing of a new configuration. it is output during server startup, and when an updated @@ -532,7 +536,7 @@ production environment. This informational message indicates that the IPv6 DHCP server has processed any command-line switches and is starting. -% DHCP6_START_INFO pid: %1, port: %2, verbose: %3, standalone: %4 +% DHCP6_START_INFO pid: %1, port: %2, verbose: %3 This is a debug message issued during the IPv6 DHCP server startup. It lists some information about the parameters with which the server is running. diff --git a/src/bin/dhcp6/kea_controller.cc b/src/bin/dhcp6/kea_controller.cc index a5b357dd85..cd099009d6 100644 --- a/src/bin/dhcp6/kea_controller.cc +++ b/src/bin/dhcp6/kea_controller.cc @@ -15,34 +15,17 @@ #include #include -#include #include #include #include #include #include -#include -#include -#include -#include -#include -#include -#include #include -#include -#include -#include #include -#include using namespace isc::asiolink; -using namespace isc::cc; -using namespace isc::config; -using namespace isc::data; using namespace isc::dhcp; -using namespace isc::log; -using namespace isc::util; using namespace std; namespace isc { @@ -66,7 +49,7 @@ ControlledDhcpv6Srv::init(const std::string& file_name) { } // Read contents of the file and parse it as JSON - json = Element::fromJSONFile(file_name, true); + json = isc::data::Element::fromJSONFile(file_name, true); if (!json) { LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL) @@ -105,9 +88,9 @@ ControlledDhcpv6Srv::init(const std::string& file_name) { } // Now check is the returned result is successful (rcode=0) or not - ConstElementPtr comment; /// see @ref isc::config::parseAnswer + isc::data::ConstElementPtr comment; /// see @ref isc::config::parseAnswer int rcode; - comment = parseAnswer(rcode, result); + comment = isc::config::parseAnswer(rcode, result); if (rcode != 0) { string reason = ""; if (comment) { @@ -129,7 +112,7 @@ 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*, bool verbose, bool ) { +void Daemon::loggerInit(const char*, bool verbose) { setenv("B10_LOCKFILE_DIR_FROM_BUILD", "/tmp", 1); setenv("B10_LOGGER_ROOT", "kea", 0); diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc index 49c70ec319..4d49af9c39 100644 --- a/src/bin/dhcp6/main.cc +++ b/src/bin/dhcp6/main.cc @@ -43,9 +43,8 @@ const char* const DHCP6_LOGGER_NAME = "kea"; void usage() { - cerr << "Usage: " << DHCP6_NAME << " [-v] [-s] [-p port_number] [-c cfgfile]" << endl; + cerr << "Usage: " << DHCP6_NAME << " [-v] [-p port_number] [-c cfgfile]" << endl; cerr << " -v: verbose output" << endl; - cerr << " -s: skip configuration (don't connect to BIND10 or don't read config file)" << endl; cerr << " -p number: specify non-standard port number 1-65535 " << "(useful for testing only)" << endl; cerr << " -c file: specify configuration file" << endl; @@ -58,22 +57,17 @@ main(int argc, char* argv[]) { int ch; int port_number = DHCP6_SERVER_PORT; // The default. Any other values are // useful for testing only. - bool stand_alone = false; // Should be connect to BIND10 msgq? bool verbose_mode = false; // Should server be verbose? // The standard config file std::string config_file(""); - while ((ch = getopt(argc, argv, "vsp:c:")) != -1) { + while ((ch = getopt(argc, argv, "vp:c:")) != -1) { switch (ch) { case 'v': verbose_mode = true; break; - case 's': // stand-alone - stand_alone = true; - break; - case 'p': // port number try { port_number = boost::lexical_cast(optarg); @@ -107,39 +101,37 @@ main(int argc, char* argv[]) { int ret = EXIT_SUCCESS; try { // Initialize logging. If verbose, we'll use maximum verbosity. - // If standalone is enabled, do not buffer initial log messages - Daemon::loggerInit(DHCP6_LOGGER_NAME, verbose_mode, stand_alone); + Daemon::loggerInit(DHCP6_LOGGER_NAME, verbose_mode); LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_START_INFO) - .arg(getpid()).arg(port_number).arg(verbose_mode ? "yes" : "no") - .arg(stand_alone ? "yes" : "no" ); + .arg(getpid()).arg(port_number).arg(verbose_mode ? "yes" : "no"); LOG_INFO(dhcp6_logger, DHCP6_STARTING); + // Create the server instance. ControlledDhcpv6Srv server(port_number); - if (!stand_alone) { - try { - // Initialize the server, i.e. establish control session - // if BIND10 backend is used or read a configuration file - server.init(config_file); + try { + // Initialize the server, i.e. establish control session + // if BIND10 backend is used or read a configuration file + // if Kea backend is used. + server.init(config_file); - } catch (const std::exception& ex) { - LOG_ERROR(dhcp6_logger, DHCP6_INIT_FAIL).arg(ex.what()); + } catch (const std::exception& ex) { + LOG_ERROR(dhcp6_logger, DHCP6_INIT_FAIL).arg(ex.what()); - // We should not continue if were told to configure (either read - // config file or establish BIND10 control session). - isc::log::LoggerManager log_manager; - log_manager.process(); + // We should not continue if were told to configure (either read + // config file or establish BIND10 control session). + isc::log::LoggerManager log_manager; + log_manager.process(); - cerr << "Failed to initialize server: " << ex.what() << endl; - return (EXIT_FAILURE); - } - } else { - LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_STANDALONE); + cerr << "Failed to initialize server: " << ex.what() << endl; + return (EXIT_FAILURE); } + // And run the main loop of the server. server.run(); + LOG_INFO(dhcp6_logger, DHCP6_SHUTDOWN); } catch (const std::exception& ex) { diff --git a/src/bin/dhcp6/tests/bundy_controller_unittest.cc b/src/bin/dhcp6/tests/bundy_controller_unittest.cc index f556560a14..0578ec6fe6 100644 --- a/src/bin/dhcp6/tests/bundy_controller_unittest.cc +++ b/src/bin/dhcp6/tests/bundy_controller_unittest.cc @@ -18,6 +18,10 @@ namespace { +/// As of May 2014, maintaining or extending Bundy support is very low +/// prority for Kea team. We are looking for contributors, who would +/// like to maintain this backend. + // Bundy framework specific tests should be added here. TEST(BundyBackendTest, dummy) { diff --git a/src/bin/dhcp6/tests/dhcp6_test.py b/src/bin/dhcp6/tests/dhcp6_test.py index 3333111ba0..9b4af4791e 100644 --- a/src/bin/dhcp6/tests/dhcp6_test.py +++ b/src/bin/dhcp6/tests/dhcp6_test.py @@ -210,22 +210,5 @@ class TestDhcpv6Daemon(unittest.TestCase): # Check that there is an error message about invalid port number printed on stderr self.assertEqual( str(error).count("Failed to parse port number"), 1) - def test_portnumber_nonroot(self): - print("Check that specifying unprivileged port number will work.") - - # Check that there is a message about running with an unprivileged port - (returncode, output, error) = self.runCommand(['../b10-dhcp6', '-v', '-s', '-p', '10547']) - output_text = str(output) + str(error) - self.assertEqual(output_text.count("DHCP6_OPEN_SOCKET opening sockets on port 10547"), 1) - - def test_skip_msgq(self): - print("Check that connection to BIND10 msgq can be disabled.") - - # Check that the system outputs a message on one of its streams about running - # standalone. - (returncode, output, error) = self.runCommand(['../b10-dhcp6', '-v', '-s', '-p', '10547']) - output_text = str(output) + str(error) - self.assertEqual(output_text.count("DHCP6_STANDALONE"), 1) - if __name__ == '__main__': unittest.main() diff --git a/src/lib/dhcpsrv/daemon.h b/src/lib/dhcpsrv/daemon.h index 7c14489044..9bcbf7f80e 100644 --- a/src/lib/dhcpsrv/daemon.h +++ b/src/lib/dhcpsrv/daemon.h @@ -85,8 +85,12 @@ public: /// Initializez logger /// - /// This method initializes logger. I - static void loggerInit(const char* log_name, bool verbose, bool stand_alone); + /// This method initializes logger. Currently its implementation is specific + /// to each configuration backend. + /// + /// @param log_name name used in logger initialization + /// @param verbose verbose mode (true usually enables DEBUG messages) + static void loggerInit(const char* log_name, bool verbose); }; }; // end of isc::dhcp namespace