[3880] Support for control-socket implemented in DHCPv4.

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -19,9 +19,11 @@
 #include <hooks/hooks_manager.h>
 #include <dhcp4/json_config_parser.h>
 #include <dhcpsrv/cfgmgr.h>
+#include <config/command_mgr.h>
 
 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.
 }

src/bin/dhcp4/json_config_parser.cc

@@ -27,6 +27,7 @@
 #include <dhcpsrv/parsers/host_reservation_parser.h>
 #include <dhcpsrv/parsers/host_reservations_list_parser.h>
 #include <dhcpsrv/parsers/ifaces_config_parser.h>
+#include <config/command_mgr.h>
 #include <util/encode/hex.h>
 #include <util/strutil.h>
 
@@ -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<std::string, ConstElementPtr>::const_iterator leases_config =
             values_map.find("lease-database");

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() {

src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc

@@ -15,6 +15,7 @@
 #include <config.h>
 
 #include <cc/command_interpreter.h>
+#include <config/command_mgr.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <hooks/hooks_manager.h>
@@ -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<ControlledDhcpv4Srv> 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<ControlledDhcpv4Srv> 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

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -17,6 +17,7 @@
 
 #include <asiolink/io_address.h>
 #include <cc/command_interpreter.h>
+#include <config/command_mgr.h>
 #include <dhcp4/tests/dhcp4_test_utils.h>
 #include <dhcp/tests/pkt_captures.h>
 #include <dhcp/dhcp4.h>
@@ -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;
 

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;
     }
 

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<int> connections_;
 };
 
 }; // end of isc::config namespace

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)
 

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

src/lib/config/tests/command_socket_factory_unittests.cc

@@ -17,6 +17,7 @@
 #include <cc/data.h>
 #include <config/command_mgr.h>
 #include <config/command_socket_factory.h>
+#include <cstdio>
 
 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));
 }
+

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)

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
 ///

src/lib/dhcpsrv/srv_config.h

@@ -24,6 +24,7 @@
 #include <dhcpsrv/cfg_subnets6.h>
 #include <dhcpsrv/cfg_mac_source.h>
 #include <dhcpsrv/logging_info.h>
+#include <cc/data.h>
 #include <boost/shared_ptr.hpp>
 #include <vector>
 #include <stdint.h>
@@ -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.

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).