diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 0b200a74e0..3a06c7e082 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -19,9 +19,11 @@ #include #include #include +#include using namespace isc::data; using namespace isc::hooks; +using namespace isc::config; using namespace std; namespace isc { @@ -121,7 +123,6 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { ConstElementPtr answer = configureDhcp4Server(*srv, config); - // Check that configuration was successful. If not, do not reopen sockets // and don't bother with DDNS stuff. try { @@ -164,6 +165,19 @@ ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/) "There is another Dhcpv4Srv instance already."); } server_ = this; // remember this instance for later use in handlers + + // Register supported commands in CommandMgr + CommandMgr::instance().registerCommand("shutdown", + boost::bind(&ControlledDhcpv4Srv::commandShutdownHandler, this, _1, _2)); + + /// @todo: register config-reload (see CtrlDhcpv4Srv::commandConfigReloadHandler) + /// @todo: register libreload (see CtrlDhcpv4Srv::commandLibReloadHandler) + /// @todo: register statistic-get (see StatsMgr::get(name)) + /// @todo: register statistic-reset (see StatsMgr::reset(name)) + /// @todo: register statistic-get-all (see StatsMgr::getAll()) + /// @todo: register statistic-reset-all (see StatsMgr::resetAll()) + /// @todo: register statistic-remove (see StatsMgr::del(name)) + /// @todo: register statistic-remove-all (see StatsMgr::removeAll()) } void ControlledDhcpv4Srv::shutdown() { @@ -174,6 +188,12 @@ void ControlledDhcpv4Srv::shutdown() { ControlledDhcpv4Srv::~ControlledDhcpv4Srv() { cleanup(); + // Close the command socket (if it exists). + CommandMgr::instance().closeCommandSocket(); + + // Deregister any registered commands + CommandMgr::instance().deregisterCommand("shutdown"); + server_ = NULL; // forget this instance. Noone should call any handlers at // this stage. } diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index ee7f5934ca..604f87d683 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -399,6 +400,8 @@ namespace dhcp { parser = new D2ClientConfigParser(config_id); } else if (config_id.compare("match-client-id") == 0) { parser = new BooleanParser(config_id, globalContext()->boolean_values_); + } else if (config_id.compare("control-socket") == 0) { + parser = new ControlSocketParser(config_id); } else { isc_throw(DhcpConfigError, "unsupported global configuration parameter: " @@ -532,6 +535,26 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { subnet_parser->build(subnet_config->second); } + // Get command socket configuration from the config file. + // This code expects the following structure: + // { + // "socket-type": "unix", + // "socket-name": "/tmp/kea4.sock" + // } + ConstElementPtr sock_cfg = + CfgMgr::instance().getStagingCfg()->getControlSocketInfo(); + + // Close existing socket (if any). + isc::config::CommandMgr::instance().closeCommandSocket(); + if (sock_cfg) { + // This will create a control socket and will install external socket + // in IfaceMgr. That socket will be monitored when Dhcp4Srv::receivePacket() + // calls IfaceMgr::receive4() and callback in CommandMgr will be called, + // if necessary. If there were previously open command socket, it will + // be closed. + isc::config::CommandMgr::instance().openCommandSocket(sock_cfg); + } + // the leases database parser is the last to be run. std::map::const_iterator leases_config = values_map.find("lease-database"); diff --git a/src/bin/dhcp4/kea_controller.cc b/src/bin/dhcp4/kea_controller.cc index 43e5e33011..a1c537022e 100644 --- a/src/bin/dhcp4/kea_controller.cc +++ b/src/bin/dhcp4/kea_controller.cc @@ -104,6 +104,8 @@ void configure(const std::string& file_name) { CfgMgr::instance().getStagingCfg()->applyLoggingCfg(); // Use new configuration. + /// @todo: This commit should be moved to + /// CtrlDhcp4Srv::commandConfigReloadHandler. CfgMgr::instance().commit(); } catch (const std::exception& ex) { @@ -171,7 +173,6 @@ ControlledDhcpv4Srv::init(const std::string& file_name) { signal_set_.reset(new isc::util::SignalSet(SIGINT, SIGHUP, SIGTERM)); // Set the pointer to the handler function. signal_handler_ = signalHandler; - } void ControlledDhcpv4Srv::cleanup() { diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index d4bfe21968..86b393ee16 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -156,4 +157,89 @@ TEST_F(CtrlDhcpv4SrvTest, libreload) { EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "1212")); } +// This test checks which commands are registered by the DHCPv4 server. +TEST_F(CtrlDhcpv4SrvTest, commandsRegistration) { + + ConstElementPtr list_cmds = createCommand("list-commands"); + ConstElementPtr answer; + + // By default the list should be empty (except the standard list-commands + // supported by the CommandMgr itself) + EXPECT_NO_THROW(answer = CommandMgr::instance().processCommand(list_cmds)); + ASSERT_TRUE(answer); + ASSERT_TRUE(answer->get("arguments")); + EXPECT_EQ("[ \"list-commands\" ]", answer->get("arguments")->str()); + + // Created server should register several additional commands. + boost::scoped_ptr srv; + ASSERT_NO_THROW( + srv.reset(new ControlledDhcpv4Srv(0)); + ); + + EXPECT_NO_THROW(answer = CommandMgr::instance().processCommand(list_cmds)); + ASSERT_TRUE(answer); + ASSERT_TRUE(answer->get("arguments")); + EXPECT_EQ("[ \"list-commands\", \"shutdown\" ]", answer->get("arguments")->str()); + + // Ok, and now delete the server. It should deregister its commands. + srv.reset(); + + // The list should be (almost) empty again. + EXPECT_NO_THROW(answer = CommandMgr::instance().processCommand(list_cmds)); + ASSERT_TRUE(answer); + ASSERT_TRUE(answer->get("arguments")); + EXPECT_EQ("[ \"list-commands\" ]", answer->get("arguments")->str()); +} + +// Checks if the server is able to parse control socket configuration and +// configures the command socket properly. +TEST_F(CtrlDhcpv4SrvTest, commandSocketBasic) { + + string socket_path = string(TEST_DATA_DIR) + "/kea4.sock"; + + // Just a simple config. The important part here is the socket + // location information. + std::string config_txt = + "{" + " \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + " }," + " \"rebind-timer\": 2000, " + " \"renew-timer\": 1000, " + " \"subnet4\": [ ]," + " \"valid-lifetime\": 4000," + " \"control-socket\": {" + " \"socket-type\": \"unix\"," + " \"socket-name\": \"" + socket_path + "\"" + " }" + "}"; + + boost::scoped_ptr srv; + ASSERT_NO_THROW( + srv.reset(new ControlledDhcpv4Srv(0)); + ); + + ConstElementPtr config = Element::fromJSON(config_txt); + + ConstElementPtr answer = srv->processConfig(config); + ASSERT_TRUE(answer); + + int status = 0; + isc::config::parseAnswer(status, answer); + EXPECT_EQ(0, status); + + // Now check that the socket was indeed open. + EXPECT_TRUE(isc::config::CommandMgr::instance().getControlSocketFD() > 0); +} + +/// @todo: Implement system tests for the control socket. +/// It is tricky in unit-tests, as it would require running two processes +/// (one for the server itself and a second one for the test that sends +/// command and receives an aswer). +/// +/// Alternatively, we could use shell tests. It would be much simpler, +/// but that requires using socat, a tool that is typically not installed. +/// So we'd need a check in configure to check if it's available and +/// fail configure process if missing (or disable the tests). + } // End of anonymous namespace diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index ecf2c7db7b..4937cce933 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -56,6 +57,7 @@ using namespace isc::dhcp; using namespace isc::data; using namespace isc::asiolink; using namespace isc::hooks; +using namespace isc::config; using namespace isc::dhcp::test; using namespace isc::test; diff --git a/src/lib/config/command_mgr.cc b/src/lib/config/command_mgr.cc index 7d9fd00c6b..184dfbb2aa 100644 --- a/src/lib/config/command_mgr.cc +++ b/src/lib/config/command_mgr.cc @@ -29,7 +29,7 @@ CommandMgr::CommandMgr() { boost::bind(&CommandMgr::listCommandsHandler, this, _1, _2)); } -int CommandMgr::openCtrlSocket(const isc::data::ConstElementPtr& socket_info) { +int CommandMgr::openCommandSocket(const isc::data::ConstElementPtr& socket_info) { if (socket_info_) { isc_throw(SocketError, "There is already a control socket open"); } @@ -44,7 +44,7 @@ int CommandMgr::openCtrlSocket(const isc::data::ConstElementPtr& socket_info) { return (socket_); } -void CommandMgr::closeCtrlSocket() { +void CommandMgr::closeCommandSocket() { if (socket_info_) { isc::dhcp::IfaceMgr::instance().deleteExternalSocket(socket_); @@ -125,6 +125,10 @@ CommandMgr::connectionAcceptor(int sockfd) { isc::dhcp::IfaceMgr::instance().addExternalSocket(fd2, boost::bind(&isc::config::CommandMgr::commandReader, fd2)); + // Remember this socket descriptor. It will be needed when we shut down the + // server. + instance().connections_.push_back(fd2); + LOG_INFO(command_logger, COMMAND_SOCKET_CONNECTION_OPENED).arg(fd2).arg(sockfd); } @@ -141,6 +145,9 @@ CommandMgr::commandReader(int sockfd) { if (rval < 0) { // Read failed LOG_WARN(command_logger, COMMAND_SOCKET_READ_FAIL).arg(rval).arg(sockfd); + + /// @todo: Should we close the connection, similar to what is already + /// being done for rval == 0? return; } else if (rval == 0) { @@ -154,6 +161,10 @@ CommandMgr::commandReader(int sockfd) { // Close the socket. close(sockfd); + + // Remove it from the active connections list. + instance().connections_.remove(sockfd); + return; } diff --git a/src/lib/config/command_mgr.h b/src/lib/config/command_mgr.h index 0a506bc48d..3eb64e0e98 100644 --- a/src/lib/config/command_mgr.h +++ b/src/lib/config/command_mgr.h @@ -92,14 +92,17 @@ public: /// Currently supported types are: /// - unix (required parameters: socket-type: unix, socket-name:/unix/path) /// - /// @throw CommandSocketError if socket creation fails + /// This method will close previously open command socket (if exists). + /// + /// @throw CommandSocketError if socket creation fails. + /// @throw SocketError if command socket is already open. /// /// @param socket_info describes control socket parameters /// @return socket descriptor of the socket created - int openCtrlSocket(const isc::data::ConstElementPtr& socket_info); + int openCommandSocket(const isc::data::ConstElementPtr& socket_info); /// @brief Shuts down any open control sockets - void closeCtrlSocket(); + void closeCommandSocket(); /// @brief Registers specified command handler for a given command /// @@ -147,6 +150,14 @@ public: /// handled at all times. void deregisterAll(); + + /// @brief Returns control socket descriptor + /// + /// This method should be used only in tests. + int getControlSocketFD() const { + return (socket_); + } + private: /// @brief Private constructor @@ -175,6 +186,9 @@ private: /// @brief Parameters for control socket isc::data::ConstElementPtr socket_info_; + + /// @brief Socket descriptors for open connections + std::list connections_; }; }; // end of isc::config namespace diff --git a/src/lib/config/tests/Makefile.am b/src/lib/config/tests/Makefile.am index 626dc67eb2..820135d10f 100644 --- a/src/lib/config/tests/Makefile.am +++ b/src/lib/config/tests/Makefile.am @@ -2,6 +2,7 @@ SUBDIRS = testdata . AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib AM_CPPFLAGS += $(BOOST_INCLUDES) +AM_CPPFLAGS += -DTEST_DATA_DIR=\"$(abs_top_srcdir)/src/lib/testutils/testdata\" AM_CXXFLAGS = $(KEA_CXXFLAGS) diff --git a/src/lib/config/tests/command_mgr_unittests.cc b/src/lib/config/tests/command_mgr_unittests.cc index 45e847462f..8a3b8bdd89 100644 --- a/src/lib/config/tests/command_mgr_unittests.cc +++ b/src/lib/config/tests/command_mgr_unittests.cc @@ -32,13 +32,13 @@ public: handler_called = false; CommandMgr::instance().deregisterAll(); - CommandMgr::instance().closeCtrlSocket(); + CommandMgr::instance().closeCommandSocket(); } /// Default destructor ~CommandMgrTest() { CommandMgr::instance().deregisterAll(); - CommandMgr::instance().closeCtrlSocket(); + CommandMgr::instance().closeCommandSocket(); } /// @brief A simple command handler that always returns an eror diff --git a/src/lib/config/tests/command_socket_factory_unittests.cc b/src/lib/config/tests/command_socket_factory_unittests.cc index 236add6643..65ab09d937 100644 --- a/src/lib/config/tests/command_socket_factory_unittests.cc +++ b/src/lib/config/tests/command_socket_factory_unittests.cc @@ -17,6 +17,7 @@ #include #include #include +#include using namespace isc::config; using namespace isc::data; @@ -27,18 +28,22 @@ public: /// Default constructor CommandSocketFactoryTest() { - unlink(SOCKET_NAME); + // Remove any stale socket files + remove(SOCKET_NAME.c_str()); } /// Default destructor ~CommandSocketFactoryTest() { - unlink(SOCKET_NAME); + + // Remove any stale socket files + remove(SOCKET_NAME.c_str()); } - static const char* SOCKET_NAME; + static const std::string SOCKET_NAME; }; -const char* CommandSocketFactoryTest::SOCKET_NAME = "test-socket"; +const std::string CommandSocketFactoryTest::SOCKET_NAME = + std::string(TEST_DATA_DIR) + "/test-socket"; TEST_F(CommandSocketFactoryTest, unixCreate) { // Null pointer is obviously a bad idea. @@ -67,3 +72,4 @@ TEST_F(CommandSocketFactoryTest, unixCreate) { // It should be possible to close the socket. EXPECT_NO_THROW(CommandSocketFactory::close(fd, socket_info)); } + diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index 68da05f44a..02f018f883 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -214,6 +214,27 @@ MACSourcesListConfigParser::commit() { // Nothing to do. } +// ******************** ControlSocketParser ************************* +ControlSocketParser::ControlSocketParser(const std::string& param_name) { + if (param_name != "control-socket") { + isc_throw(BadValue, "Internal error. Control socket parser called " + " for wrong parameter:" << param_name); + } +} + +void ControlSocketParser::build(isc::data::ConstElementPtr value) { + if (value->getType() != Element::map) { + isc_throw(BadValue, "Specified control-socket is expected to be a map" + ", i.e. a structure defined within { }"); + } + CfgMgr::instance().getStagingCfg()->setControlSocketInfo(value); +} + +/// @brief Does nothing. +void ControlSocketParser::commit() { + // Nothing to do. +} + // ******************** HooksLibrariesParser ************************* HooksLibrariesParser::HooksLibrariesParser(const std::string& param_name) diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index efed05e61a..a5b2de757f 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -420,6 +420,23 @@ private: ParserContextPtr global_context_; }; +/// @brief Parser for the control-socket structure +/// +/// It does not parse anything, simply stores the element in +/// the staging config. +class ControlSocketParser : public DhcpConfigParser { +public: + + ControlSocketParser(const std::string& param_name); + + /// @brief Stores contents of the control-socket map in the staging config. + /// + /// @param value pointer to the content of parsed values + virtual void build(isc::data::ConstElementPtr value); + + /// @brief Does nothing. + virtual void commit(); +}; /// @brief Parser for hooks library list /// diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index e27f9dd122..59609f44d9 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -276,6 +277,18 @@ public: return (cfg_mac_source_); } + /// @brief Returns information about control socket + /// @return pointer to the Element that holds control-socket map + const isc::data::ConstElementPtr getControlSocketInfo() const { + return (control_socket_); + } + + /// @brief Sets information about the control socket + /// @param control_socket Element that holds control-socket map + void setControlSocketInfo(const isc::data::ConstElementPtr& control_socket) { + control_socket_ = control_socket; + } + /// @brief Copies the currnet configuration to a new configuration. /// /// This method copies the parameters stored in the configuration to @@ -396,6 +409,9 @@ private: /// This object holds a set of RSOO-enabled options. See /// RFC 6422 for the definition of the RSOO-enabled option. CfgRSOOPtr cfg_rsoo_; + + /// @brief Pointer to the control-socket information + isc::data::ConstElementPtr control_socket_; }; /// @name Pointers to the @c SrvConfig object. diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 2ac95ae2d7..315467f63a 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -1757,3 +1757,7 @@ TEST_F(ParseConfigTest, validRelayInfo6) { // Unparseable text that looks like IPv6 address, but has too many colons EXPECT_THROW(parser->build(json_bogus2), DhcpConfigError); } + +// There's no test for ControlSocketParser, as it is tested in the DHCPv4 code +// (see CtrlDhcpv4SrvTest.commandSocketBasic in +// src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc).