[master] Merge branch 'trac3329'

b10-dhcp4 now fully able to generate and send DHCP-DDNS requests

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -114,6 +114,16 @@ ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr new_config) {
         return (answer);
     }
 
+    // Server will start DDNS communications if its enabled.
+    try {
+        server_->startD2();
+    } catch (const std::exception& ex) {
+        std::ostringstream err;
+        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.
@@ -211,11 +221,15 @@ void ControlledDhcpv4Srv::establishSession() {
 
     try {
         configureDhcp4Server(*this, config_session_->getFullConfig());
+
+        // Server will start DDNS communications if its enabled.
+        server_->startD2();
+
         // Configuration may disable or enable interfaces so we have to
         // reopen sockets according to new configuration.
         openActiveSockets(getPort(), useBroadcast());
 
-    } catch (const DhcpConfigError& ex) {
+    } catch (const std::exception& ex) {
         LOG_ERROR(dhcp4_logger, DHCP4_CONFIG_LOAD_FAIL).arg(ex.what());
 
     }

src/bin/dhcp4/dhcp4_messages.mes

@@ -20,11 +20,11 @@ to receive DHCPv4 traffic. IPv4 socket on this interface will be opened once
 Interface Manager starts up procedure of opening sockets.
 
 % DHCP4_CCSESSION_STARTED control channel session started on socket %1
-A debug message issued during startup after the IPv4 DHCP server has
+A debug message issued during startup after the DHCPv4 server has
 successfully established a session with the BIND 10 control channel.
 
 % DHCP4_CCSESSION_STARTING starting control channel session, specfile: %1
-This debug message is issued just before the IPv4 DHCP server attempts
+This debug message is issued just before the DHCPv4 server attempts
 to establish a session with the BIND 10 control channel.
 
 % DHCP4_CLIENT_NAME_PROC_FAIL failed to process the fqdn or hostname sent by a client: %1
@@ -42,7 +42,7 @@ class or classes. This is a norma
 
 % DHCP4_COMMAND_RECEIVED received command %1, arguments: %2
 A debug message listing the command (and possible arguments) received
-from the BIND 10 control system by the IPv4 DHCP server.
+from the BIND 10 control system by the DHCPv4 server.
 
 % DHCP4_CONFIG_COMPLETE DHCPv4 server has completed configuration: %1
 This is an informational message announcing the successful processing of a
@@ -70,7 +70,7 @@ configuration. That happens at start up and also when a server configuration
 change is committed by the administrator.
 
 % DHCP4_CONFIG_UPDATE updated configuration received: %1
-A debug message indicating that the IPv4 DHCP server has received an
+A debug message indicating that the DHCPv4 server has received an
 updated configuration from the BIND 10 configuration system.
 
 % DHCP4_DB_BACKEND_STARTED lease database started (type: %1, name: %2)
@@ -194,12 +194,12 @@ which this message has been received. The IPv4 address assigned on this
 interface must belong to one of the configured subnets. Otherwise
 received message is dropped.
 
-% DHCP4_NOT_RUNNING IPv4 DHCP server is not running
+% DHCP4_NOT_RUNNING DHCPv4 server is not running
 A warning message is issued when an attempt is made to shut down the
-IPv4 DHCP server but it is not running.
+DHCPv4 server but it is not running.
 
 % DHCP4_OPEN_SOCKET opening sockets on port %1
-A debug message issued during startup, this indicates that the IPv4 DHCP
+A debug message issued during startup, this indicates that the DHCPv4
 server is about to open sockets on the specified port.
 
 % DHCP4_PACKET_NOT_FOR_US received DHCPv4 message (transid=%1, iface=%2) dropped because it contains foreign server identifier
@@ -214,7 +214,7 @@ A warning message issued when IfaceMgr fails to open and bind a socket. The reas
 for the failure is appended as an argument of the log message.
 
 % DHCP4_PACKET_PARSE_FAIL failed to parse incoming packet: %1
-The IPv4 DHCP server has received a packet that it is unable to
+The DHCPv4 server has received a packet that it is unable to
 interpret. The reason why the packet is invalid is included in the message.
 
 % DHCP4_PACKET_PROCESS_FAIL failed to process packet received from %1: %2
@@ -233,35 +233,35 @@ may well be a valid DHCP packet, just a type not expected by the server
 (e.g. it will report a received OFFER packet as UNKNOWN).
 
 % DHCP4_PACKET_RECEIVE_FAIL error on attempt to receive packet: %1
-The IPv4 DHCP server tried to receive a packet but an error
+The DHCPv4 server tried to receive a packet but an error
 occurred during this attempt. The reason for the error is included in
 the message.
 
 % DHCP4_PACKET_SEND_FAIL failed to send DHCPv4 packet: %1
-This error is output if the IPv4 DHCP server fails to send an assembled
+This error is output if the DHCPv4 server fails to send an assembled
 DHCP message to a client. The reason for the error is included in the
 message.
 
 % DHCP4_PARSER_COMMIT_EXCEPTION parser failed to commit changes
-On receipt of message containing details to a change of the IPv4 DHCP
+On receipt of message containing details to a change of the DHCPv4
 server configuration, a set of parsers were successfully created, but one
 of them failed to commit its changes due to a low-level system exception
 being raised.  Additional messages may be output indicating the reason.
 
 % DHCP4_PARSER_COMMIT_FAIL parser failed to commit changes: %1
-On receipt of message containing details to a change of the IPv4 DHCP
+On receipt of message containing details to a change of the DHCPv4
 server configuration, a set of parsers were successfully created, but
 one of them failed to commit its changes.  The reason for the failure
 is given in the message.
 
 % DHCP4_PARSER_CREATED created parser for configuration element %1
-A debug message output during a configuration update of the IPv4 DHCP
+A debug message output during a configuration update of the DHCPv4
 server, notifying that the parser for the specified configuration element
 has been successfully created.
 
 % DHCP4_PARSER_EXCEPTION failed to create or run parser for configuration element %1
 On receipt of message containing details to a change of its configuration,
-the IPv4 DHCP server failed to create a parser to decode the contents of
+the DHCPv4 server failed to create a parser to decode the contents of
 the named configuration element, or the creation succeeded but the parsing
 actions and committal of changes failed.  The message has been output in
 response to a non-BIND 10 exception being raised.  Additional messages
@@ -269,7 +269,7 @@ may give further information.
 
 % DHCP4_PARSER_FAIL failed to create or run parser for configuration element %1: %2
 On receipt of message containing details to a change of its configuration,
-the IPv4 DHCP server failed to create a parser to decode the contents
+the DHCPv4 server failed to create a parser to decode the contents
 of the named configuration element, or the creation succeeded but the
 parsing actions and committal of changes failed.  The reason for the
 failure is given in the message.
@@ -324,40 +324,40 @@ both clones use the same client-id.
 A debug message listing the data returned to the client.
 
 % DHCP4_SERVER_FAILED server failed: %1
-The IPv4 DHCP server has encountered a fatal error and is terminating.
+The DHCPv4 server has encountered a fatal error and is terminating.
 The reason for the failure is included in the message.
 
 % DHCP4_SESSION_FAIL failed to establish BIND 10 session (%1), running stand-alone
 The server has failed to establish communication with the rest of BIND
 10 and is running in stand-alone mode.  (This behavior will change once
-the IPv4 DHCP server is properly integrated with the rest of BIND 10.)
+the DHCPv4 server is properly integrated with the rest of BIND 10.)
 
 % DHCP4_SHUTDOWN server shutdown
-The IPv4 DHCP server has terminated normally.
+The DHCPv4 server has terminated normally.
 
 % DHCP4_SHUTDOWN_REQUEST shutdown of server requested
-This debug message indicates that a shutdown of the IPv4 server has
+This debug message indicates that a shutdown of the DHCPv4 server has
 been requested via a call to the 'shutdown' method of the core Dhcpv4Srv
 object.
 
 % DHCP4_SRV_CONSTRUCT_ERROR error creating Dhcpv4Srv object, reason: %1
 This error message indicates that during startup, the construction of a
-core component within the IPv4 DHCP server (the Dhcpv4 server object)
+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 IPv4 server is running in
+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 IPv4 DHCP server has
+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
-This is a debug message issued during the IPv4 DHCP server startup.
+This is a debug message issued during the DHCPv4 server startup.
 It lists some information about the parameters with which the server
 is running.
 
@@ -370,3 +370,8 @@ steps in the processing of incoming client message.
 This warning message is output when a packet was received from a subnet
 for which the DHCPv4 server has not been configured. The most probable
 cause is a misconfiguration of the server.
+
+% DHCP4_DDNS_REQUEST_SEND_FAILED failed sending a request to b10-dhcp-ddns, error: %1,  ncr: %2
+This error message indicates that DHCP4 server attempted to send a DDNS
+update reqeust to the DHCP-DDNS server.  This is most likely a configuration or
+networking error.

src/bin/dhcp4/dhcp4_srv.cc

@@ -420,9 +420,6 @@ Dhcpv4Srv::run() {
             LOG_ERROR(dhcp4_logger, DHCP4_PACKET_SEND_FAIL)
                 .arg(e.what());
         }
-
-        // Send NameChangeRequests to the b10-dhcp_ddns module.
-        sendNameChangeRequests();
     }
 
     return (true);
@@ -881,26 +878,23 @@ queueNameChangeRequest(const isc::dhcp_ddns::NameChangeType chg_type,
             .arg(ex.what());
         return;
     }
+
     // Create NameChangeRequest
-    NameChangeRequest ncr(chg_type, lease->fqdn_fwd_, lease->fqdn_rev_,
-                          lease->hostname_, lease->addr_.toText(),
-                          dhcid, lease->cltt_ + lease->valid_lft_,
-                          lease->valid_lft_);
-    // And queue it.
+    NameChangeRequestPtr ncr(new NameChangeRequest(chg_type, lease->fqdn_fwd_,
+                                                   lease->fqdn_rev_,
+                                                   lease->hostname_,
+                                                   lease->addr_.toText(),
+                                                   dhcid,
+                                                   (lease->cltt_ +
+                                                    lease->valid_lft_),
+                                                   lease->valid_lft_));
+
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_QUEUE_NCR)
         .arg(chg_type == CHG_ADD ? "add" : "remove")
-        .arg(ncr.toText());
-    name_change_reqs_.push(ncr);
-}
+        .arg(ncr->toText());
 
-void
-Dhcpv4Srv::sendNameChangeRequests() {
-    while (!name_change_reqs_.empty()) {
-        /// @todo Once next NameChangeRequest is picked from the queue
-        /// we should send it to the b10-dhcp_ddns module. Currently we
-        /// just drop it.
-        name_change_reqs_.pop();
-    }
+    // And pass it to the the manager.
+    CfgMgr::instance().getD2ClientMgr().sendRequest(ncr);
 }
 
 void
@@ -1869,5 +1863,29 @@ bool Dhcpv4Srv::classSpecificProcessing(const Pkt4Ptr& query, const Pkt4Ptr& rsp
     return (true);
 }
 
+void
+Dhcpv4Srv::startD2() {
+    D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr();
+    if (d2_mgr.ddnsEnabled()) {
+        // Updates are enabled, so lets start the sender, passing in
+        // our error handler.
+        // This may throw so wherever this is called needs to ready.
+        d2_mgr.startSender(boost::bind(&Dhcpv4Srv::d2ClientErrorHandler,
+                                       this, _1, _2));
+    }
+}
+
+void
+Dhcpv4Srv::d2ClientErrorHandler(const
+                                dhcp_ddns::NameChangeSender::Result result,
+                                dhcp_ddns::NameChangeRequestPtr& ncr) {
+    LOG_ERROR(dhcp4_logger, DHCP4_DDNS_REQUEST_SEND_FAILED).
+              arg(result).arg((ncr ? ncr->toText() : " NULL "));
+    // We cannot communicate with b10-dhcp-ddns, suspend futher updates.
+    /// @todo We may wish to revisit this, but for now we will simpy turn
+    /// them off.
+    CfgMgr::instance().getD2ClientMgr().suspendUpdates();
+}
+
 }   // namespace dhcp
 }   // namespace isc

src/bin/dhcp4/dhcp4_srv.h

@@ -21,6 +21,7 @@
 #include <dhcp/option4_client_fqdn.h>
 #include <dhcp/option_custom.h>
 #include <dhcp_ddns/ncr_msg.h>
+#include <dhcpsrv/d2_client_mgr.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/alloc_engine.h>
 #include <hooks/callout_handle.h>
@@ -165,6 +166,30 @@ public:
     /// @param use_bcast should broadcast flags be set on the sockets.
     static void openActiveSockets(const uint16_t port, const bool use_bcast);
 
+    /// @brief Starts DHCP_DDNS client IO if DDNS updates are enabled.
+    ///
+    /// If updates are enabled, it Instructs the D2ClientMgr singleton to
+    /// enter send mode.  If D2ClientMgr encounters errors it may throw
+    /// D2ClientErrors. This method does not catch exceptions.
+    void startD2();
+
+    /// @brief Implements the error handler for DHCP_DDNS IO errors
+    ///
+    /// Invoked when a NameChangeRequest send to b10-dhcp-ddns completes with
+    /// a failed status.  These are communications errors, not data related
+    /// failures.
+    ///
+    /// This method logs the failure and then suspends all further updates.
+    /// Updating can only be restored by reconfiguration or restarting the
+    /// server.  There is currently no retry logic so the first IO error that
+    /// occurs will suspend updates.
+    /// @todo We may wish to make this more robust or sophisticated.
+    ///
+    /// @param result Result code of the send operation.
+    /// @param ncr NameChangeRequest which failed to send.
+    virtual void d2ClientErrorHandler(const dhcp_ddns::
+                                      NameChangeSender::Result result,
+                                      dhcp_ddns::NameChangeRequestPtr& ncr);
 protected:
 
     /// @name Functions filtering and sanity-checking received messages.
@@ -455,10 +480,10 @@ protected:
     /// @brief Creates the NameChangeRequest and adds to the queue for
     /// processing.
     ///
-    /// This function adds the @c isc::dhcp_ddns::NameChangeRequest to the
-    /// queue and emits the debug message which indicates whether the request
-    /// being added is to remove DNS entry or add a new entry. This function
-    /// is exception free.
+    /// This creates the @c isc::dhcp_ddns::NameChangeRequest; emits a
+    /// the debug message which indicates whether the request being added is
+    /// to remove DNS entry or add a new entry; and then sends the request
+    /// to the D2ClientMgr for transmission to b10-dhcp-ddns.
     ///
     /// @param chg_type A type of the NameChangeRequest (ADD or REMOVE).
     /// @param lease A lease for which the NameChangeRequest is created and
@@ -466,17 +491,6 @@ protected:
     void queueNameChangeRequest(const isc::dhcp_ddns::NameChangeType chg_type,
                                 const Lease4Ptr& lease);
 
-    /// @brief Sends all outstanding NameChangeRequests to b10-dhcp-ddns module.
-    ///
-    /// The purpose of this function is to pick all outstanding
-    /// NameChangeRequests from the FIFO queue and send them to b10-dhcp-ddns
-    /// module.
-    ///
-    /// @todo Currently this function simply removes all requests from the
-    /// queue but doesn't send them anywhere. In the future, the
-    /// NameChangeSender will be used to deliver requests to the other module.
-    void sendNameChangeRequests();
-
     /// @brief Attempts to renew received addresses
     ///
     /// Attempts to renew existing lease. This typically includes finding a lease that
@@ -686,12 +700,6 @@ private:
     int hook_index_pkt4_receive_;
     int hook_index_subnet4_select_;
     int hook_index_pkt4_send_;
-
-protected:
-
-    /// Holds a list of @c isc::dhcp_ddns::NameChangeRequest objects which
-    /// are waiting for sending  to b10-dhcp-ddns module.
-    std::queue<isc::dhcp_ddns::NameChangeRequest> name_change_reqs_;
 };
 
 }; // namespace isc::dhcp

src/bin/dhcp4/tests/Makefile.am

@@ -76,6 +76,7 @@ TESTS += dhcp4_unittests
 dhcp4_unittests_SOURCES = ../dhcp4_srv.h ../dhcp4_srv.cc ../ctrl_dhcp4_srv.cc
 dhcp4_unittests_SOURCES += ../dhcp4_log.h ../dhcp4_log.cc
 dhcp4_unittests_SOURCES += ../config_parser.cc ../config_parser.h
+dhcp4_unittests_SOURCES += d2_unittest.h d2_unittest.cc
 dhcp4_unittests_SOURCES += dhcp4_test_utils.h
 dhcp4_unittests_SOURCES += dhcp4_unittests.cc
 dhcp4_unittests_SOURCES += dhcp4_srv_unittest.cc

src/bin/dhcp4/tests/d2_unittest.cc

@@ -0,0 +1,379 @@
+// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dhcp/iface_mgr.h>
+#include <dhcp4/config_parser.h>
+#include <dhcp4/tests/d2_unittest.h>
+#include <dhcpsrv/cfgmgr.h>
+
+#include <gtest/gtest.h>
+
+#include <string>
+
+using namespace isc;
+using namespace isc::asiolink;
+using namespace isc::data;
+
+namespace isc {
+namespace dhcp {
+namespace test {
+
+void
+D2Dhcpv4Srv::d2ClientErrorHandler(const
+                                dhcp_ddns::NameChangeSender::Result result,
+                                dhcp_ddns::NameChangeRequestPtr& ncr) {
+    ++error_count_;
+    // call base class error handler
+    Dhcpv4Srv::d2ClientErrorHandler(result, ncr);
+}
+
+const bool Dhcp4SrvD2Test::SHOULD_PASS;
+const bool Dhcp4SrvD2Test::SHOULD_FAIL;
+
+Dhcp4SrvD2Test::Dhcp4SrvD2Test() {
+}
+
+Dhcp4SrvD2Test::~Dhcp4SrvD2Test() {
+    reset();
+}
+
+dhcp_ddns::NameChangeRequestPtr
+Dhcp4SrvD2Test::buildTestNcr(uint32_t dhcid_id_num) {
+    // Build an NCR from json string.
+    std::ostringstream stream;
+
+    stream <<
+        "{"
+        " \"change_type\" : 0 , "
+        " \"forward_change\" : true , "
+        " \"reverse_change\" : false , "
+        " \"fqdn\" : \"myhost.example.com.\" , "
+        " \"ip_address\" : \"192.168.2.1\" , "
+        " \"dhcid\" : \""
+
+        << std::hex << std::setfill('0') << std::setw(16)
+        << dhcid_id_num << "\" , "
+
+        " \"lease_expires_on\" : \"20140121132405\" , "
+        " \"lease_length\" : 1300 "
+        "}";
+
+    return (dhcp_ddns::NameChangeRequest::fromJSON(stream.str()));
+}
+
+void
+Dhcp4SrvD2Test::reset() {
+    std::string config = "{ \"interfaces\": [ \"*\" ],"
+            "\"hooks-libraries\": [ ], "
+            "\"rebind-timer\": 2000, "
+            "\"renew-timer\": 1000, "
+            "\"valid-lifetime\": 4000, "
+            "\"subnet4\": [ ], "
+            "\"dhcp-ddns\": { \"enable-updates\" : false }, "
+            "\"option-def\": [ ], "
+            "\"option-data\": [ ] }";
+    configure(config, SHOULD_PASS);
+}
+
+void
+Dhcp4SrvD2Test::configureD2(bool enable_d2, const bool exp_result,
+                            const std::string& ip_address,
+                            const uint32_t port) {
+    std::ostringstream config;
+    config <<
+        "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        " \"dhcp-ddns\" : {"
+        "     \"enable-updates\" : " << (enable_d2 ? "true" : "false") <<  ", "
+        "     \"server-ip\" : \"" << ip_address << "\", "
+        "     \"server-port\" : " << port << ", "
+        "     \"ncr-protocol\" : \"UDP\", "
+        "     \"ncr-format\" : \"JSON\", "
+        "     \"always-include-fqdn\" : true, "
+        "     \"allow-client-update\" : true, "
+        "     \"override-no-update\" : true, "
+        "     \"override-client-update\" : true, "
+        "     \"replace-client-name\" : true, "
+        "     \"generated-prefix\" : \"test.prefix\", "
+        "     \"qualifying-suffix\" : \"test.suffix.\" },"
+        "\"valid-lifetime\": 4000 }";
+
+    configure(config.str(), exp_result);
+}
+
+void
+Dhcp4SrvD2Test::configure(const std::string& config, bool exp_result) {
+    ElementPtr json = Element::fromJSON(config);
+    ConstElementPtr status;
+
+    // Configure the server and make sure the config is accepted
+    EXPECT_NO_THROW(status = configureDhcp4Server(srv_, json));
+    ASSERT_TRUE(status);
+
+    int rcode;
+    ConstElementPtr comment = config::parseAnswer(rcode, status);
+    if (exp_result == SHOULD_PASS) {
+        ASSERT_EQ(0, rcode);
+    } else {
+        ASSERT_EQ(1, rcode);
+    }
+}
+
+// Tests ability to turn on and off ddns updates by submitting
+// by submitting the appropriate configuration to Dhcp4 server
+// and then invoking its startD2() method.
+TEST_F(Dhcp4SrvD2Test, enableDisable) {
+    // Grab the manager and verify that be default ddns is off
+    // and a sender was not started.
+    dhcp::D2ClientMgr& mgr = CfgMgr::instance().getD2ClientMgr();
+    ASSERT_FALSE(mgr.ddnsEnabled());
+    ASSERT_FALSE(mgr.amSending());
+
+    // Verify a valid config with ddns enabled configures ddns properly,
+    // but does not start the sender.
+    ASSERT_NO_FATAL_FAILURE(configureD2(true));
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_FALSE(mgr.amSending());
+
+    // Verify that calling start does not throw and starts the sender.
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+
+    // Verify a valid config with ddns disabled configures ddns properly.
+    // Sender should not have been started.
+    ASSERT_NO_FATAL_FAILURE(configureD2(false));
+    ASSERT_FALSE(mgr.ddnsEnabled());
+    ASSERT_FALSE(mgr.amSending());
+
+    // Verify that the sender does NOT get started when ddns is disabled.
+    srv_.startD2();
+    ASSERT_FALSE(mgr.amSending());
+}
+
+// Tests Dhcp4 server's ability to correctly handle a flawed dhcp-ddns configuration.
+// It does so by first enabling updates by submitting a valid configuration and then
+// ensuring they remain on after submitting a flawed configuration.
+// and then invoking its startD2() method.
+TEST_F(Dhcp4SrvD2Test, badConfig) {
+    // Grab the manager and verify that be default ddns is off
+    // and a sender was not started.
+    dhcp::D2ClientMgr& mgr = CfgMgr::instance().getD2ClientMgr();
+    ASSERT_FALSE(mgr.ddnsEnabled());
+
+    // Configure it enabled and start it.
+    ASSERT_NO_FATAL_FAILURE(configureD2(true));
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+
+    // Now attempt to give it an invalid configuration.
+    // Result should indicate failure.
+    ASSERT_NO_FATAL_FAILURE(configureD2(false, SHOULD_FAIL, "bogus_ip"));
+
+    // Configure was not altered, so ddns should be enabled and still sending.
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_TRUE(mgr.amSending());
+
+    // Verify that calling start does not throw or stop the sender.
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+
+}
+
+// Checks that submitting an identical dhcp-ddns configuration
+// is handled properly.  Not effect should be no change in
+// status for ddns updating.  Updates should still enabled and
+// in send mode.  This indicates that the sender was not stopped.
+TEST_F(Dhcp4SrvD2Test, sameConfig) {
+    // Grab the manager and verify that be default ddns is off
+    // and a sender was not started.
+    dhcp::D2ClientMgr& mgr = CfgMgr::instance().getD2ClientMgr();
+    ASSERT_FALSE(mgr.ddnsEnabled());
+
+    // Configure it enabled and start it.
+    ASSERT_NO_FATAL_FAILURE(configureD2(true));
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+
+    // Now submit an identical configuration.
+    ASSERT_NO_FATAL_FAILURE(configureD2(true));
+
+    // Configuration was not altered, so ddns should still enabled and sending.
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_TRUE(mgr.amSending());
+
+    // Verify that calling start does not throw or stop the sender.
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+}
+
+// Checks that submitting an different, but valid dhcp-ddns configuration
+// is handled properly.  Updates should be enabled, however they should
+// not yet be running.  This indicates that the sender was stopped and
+// replaced, but not yet started.
+TEST_F(Dhcp4SrvD2Test, differentConfig) {
+    // Grab the manager and verify that be default ddns is off
+    // and a sender was not started.
+    dhcp::D2ClientMgr& mgr = CfgMgr::instance().getD2ClientMgr();
+    ASSERT_FALSE(mgr.ddnsEnabled());
+
+    // Configure it enabled and start it.
+    ASSERT_NO_FATAL_FAILURE(configureD2(true));
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+
+    // Now enable it on a different port.
+    ASSERT_NO_FATAL_FAILURE(configureD2(true, SHOULD_PASS, "127.0.0.1", 54001));
+
+    // Configuration was altered, so ddns should still enabled but not sending.
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_FALSE(mgr.amSending());
+
+    // Verify that calling start starts the sender.
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+}
+
+// Checks that given a valid, enabled configuration and placing
+// sender in send mode, permits NCR requests to be sent via UPD
+// socket.  Note this test does not employ any sort of receiving
+// client to verify actual transmission.  These types of tests
+// are including under dhcp_ddns and d2 unit testing.
+TEST_F(Dhcp4SrvD2Test, simpleUDPSend) {
+    // Grab the manager and verify that be default ddns is off
+    // and a sender was not started.
+    dhcp::D2ClientMgr& mgr = CfgMgr::instance().getD2ClientMgr();
+    ASSERT_FALSE(mgr.ddnsEnabled());
+
+    // Configure it enabled and start it.
+    ASSERT_NO_FATAL_FAILURE(configureD2(true));
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+
+    // Verify that we can queue up a message.
+    dhcp_ddns::NameChangeRequestPtr ncr = buildTestNcr();
+    ASSERT_NO_THROW(mgr.sendRequest(ncr));
+    EXPECT_EQ(1, mgr.getQueueSize());
+
+    // Calling receive should detect the ready IO on the sender's select-fd,
+    // and invoke callback, which should complete the send.
+    ASSERT_NO_THROW(IfaceMgr::instance().receive4(0,0));
+
+    // Verify the queue is now empty.
+    EXPECT_EQ(0, mgr.getQueueSize());
+}
+
+// Checks that an IO error in sending a request to D2, results in ddns updates being
+// suspended.  This indicates that Dhcp4Srv's error handler has been invoked as expected.
+// Note that this unit test relies on an attempt to send to a server address of 0.0.0.0
+// port 0 fails under all OSs.
+TEST_F(Dhcp4SrvD2Test, forceUDPSendFailure) {
+    // Grab the manager and verify that be default ddns is off
+    // and a sender was not started.
+    dhcp::D2ClientMgr& mgr = CfgMgr::instance().getD2ClientMgr();
+    ASSERT_FALSE(mgr.ddnsEnabled());
+
+    // Configure it enabled and start it.
+    // Using server address of 0.0.0.0/0 should induce failure on send.
+    ASSERT_NO_FATAL_FAILURE(configureD2(true, SHOULD_PASS, "0.0.0.0", 0));
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+
+    // Queue up 3 messages.
+    for (int i = 0; i < 3; i++) {
+        dhcp_ddns::NameChangeRequestPtr ncr = buildTestNcr(i + 1);
+        ASSERT_NO_THROW(mgr.sendRequest(ncr));
+    }
+    EXPECT_EQ(3, mgr.getQueueSize());
+
+    // Calling receive should detect the ready IO on the sender's select-fd,
+    // and invoke callback, which should complete the send, which should
+    // fail.
+    ASSERT_NO_THROW(IfaceMgr::instance().receive4(0,0));
+
+    // Verify the error handler was invoked.
+    EXPECT_EQ(1, srv_.error_count_);
+
+    // Verify that updates are disabled and we are no longer sending.
+    ASSERT_FALSE(mgr.ddnsEnabled());
+    ASSERT_FALSE(mgr.amSending());
+
+    // Verify message is still in the queue.
+    EXPECT_EQ(3, mgr.getQueueSize());
+
+    // Verify that we can't just restart it.
+    /// @todo This may change if we add ability to resume.
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_FALSE(mgr.amSending());
+
+    // Configure it enabled and start it.
+    ASSERT_NO_FATAL_FAILURE(configureD2(true));
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+
+    // Verify message is still in the queue.
+    EXPECT_EQ(3, mgr.getQueueSize());
+
+    // This will finish sending the 1st message in queue
+    // and initiate send of 2nd message.
+    ASSERT_NO_THROW(IfaceMgr::instance().receive4(0,0));
+    EXPECT_EQ(1, srv_.error_count_);
+
+    // First message is off the queue.
+    EXPECT_EQ(2, mgr.getQueueSize());
+}
+
+// Tests error handling of D2ClientMgr::sendRequest() failure
+// by attempting to queue maximum number of messages.
+TEST_F(Dhcp4SrvD2Test, queueMaxError) {
+    // Configure it enabled and start it.
+    dhcp::D2ClientMgr& mgr = CfgMgr::instance().getD2ClientMgr();
+    ASSERT_NO_FATAL_FAILURE(configureD2(true));
+    ASSERT_TRUE(mgr.ddnsEnabled());
+    ASSERT_NO_THROW(srv_.startD2());
+    ASSERT_TRUE(mgr.amSending());
+
+    // Attempt to queue more then the maximum allowed.
+    int max_msgs = mgr.getQueueMaxSize();
+    for (int i = 0; i < max_msgs + 1; i++) {
+        dhcp_ddns::NameChangeRequestPtr ncr = buildTestNcr(i + 1);
+        ASSERT_NO_THROW(mgr.sendRequest(ncr));
+    }
+
+    // Stopping sender will complete the first message so there
+    // should be max less one.
+    EXPECT_EQ(max_msgs - 1, mgr.getQueueSize());
+
+    // Verify the error handler was invoked.
+    EXPECT_EQ(1, srv_.error_count_);
+
+    // Verify that updates are disabled and we are no longer sending.
+    ASSERT_FALSE(mgr.ddnsEnabled());
+    ASSERT_FALSE(mgr.amSending());
+}
+
+
+} // namespace test
+} // namespace dhcp
+} // namespace isc
+

src/bin/dhcp4/tests/d2_unittest.h

@@ -0,0 +1,117 @@
+// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+/// @file d2_unittest.h Defines classes for testing Dhcpv4srv with D2ClientMgr
+
+#ifndef D2_UNITTEST_H
+#define D2_UNITTEST_H
+
+#include <dhcp4/dhcp4_srv.h>
+#include <config/ccsession.h>
+
+#include <gtest/gtest.h>
+
+namespace isc {
+namespace dhcp {
+namespace test {
+
+/// @brief Test derivation of Dhcpv4Srv class used in D2 testing.
+/// Use of this class allows the intervention at strategic points in testing
+/// by permitting overridden methods and access to scope protected members.
+class D2Dhcpv4Srv : public  Dhcpv4Srv {
+public:
+    /// @brief Counts the number of times the client error handler is called.
+    int error_count_;
+
+    /// @brief Constructor
+    D2Dhcpv4Srv()
+        : Dhcpv4Srv(0, "type=memfile", false, false), error_count_(0) {
+    }
+
+    /// @brief virtual Destructor.
+    virtual ~D2Dhcpv4Srv() {
+    }
+
+    /// @brief Override the error handler.
+    virtual void d2ClientErrorHandler(const dhcp_ddns::NameChangeSender::
+                                      Result result,
+                                      dhcp_ddns::NameChangeRequestPtr& ncr);
+};
+
+/// @brief Test fixture which permits testing the interaction between the
+/// D2ClientMgr and Dhcpv4Srv.
+class Dhcp4SrvD2Test : public ::testing::Test {
+public:
+    /// @brief Mnemonic constants for calls to configuration methods.
+    static const bool SHOULD_PASS = true;
+    static const bool SHOULD_FAIL = false;
+
+    /// @brief Constructor
+    Dhcp4SrvD2Test();
+
+    /// @brief virtual Destructor
+    virtual ~Dhcp4SrvD2Test();
+
+    /// @brief Resets the CfgMgr singleton to defaults.
+    /// Primarily used in the test destructor as gtest doesn't exit between
+    /// tests.
+    /// @todo CfgMgr should provide a method to reset everything or maybe
+    /// reconstruct the singleton.
+    void reset();
+
+    /// @brief Configures the server with D2 enabled or disabled
+    ///
+    /// Constructs a configuration string including dhcp-ddns with the
+    /// parameters given and passes it into the server's configuration handler.
+    ///
+    /// @param enable_updates value to assign to the enable-updates parameter
+    /// @param exp_result indicates if configuration should pass or fail
+    /// @param ip_address IP address for the D2 server
+    /// @param port  port for the D2 server
+    void configureD2(bool enable_updates, bool exp_result = SHOULD_PASS,
+                     const std::string& ip_address = "127.0.0.1",
+                     const uint32_t port = 53001);
+
+    /// @brief Configures the server with the given configuration
+    ///
+    /// Passes the given configuration string into the server's configuration
+    /// handler.  It accepts a flag indicating whether or not the configuration
+    /// is expected to succeed or fail.  This permits testing the server's
+    /// response to both valid and invalid configurations.
+    ///
+    /// @param config JSON string containing the configuration
+    /// @param exp_result indicates if configuration should pass or fail
+    void configure(const std::string& config, bool exp_result = SHOULD_PASS);
+
+    /// @brief Contructs a NameChangeRequest message from a fixed JSON string.
+    ///
+    /// @param dhcid_id_num Integer value to use as the DHCID.
+    dhcp_ddns::NameChangeRequestPtr buildTestNcr(uint32_t
+                                                 dhcid_id_num = 0xdeadbeef);
+
+    /// @brief Stores the return code of the last configuration attempt.
+    int rcode_;
+
+    /// @brief Stores the message component of the last configuration tattempt.
+    isc::data::ConstElementPtr comment_;
+
+    /// @brief Server object under test.
+    D2Dhcpv4Srv srv_;
+};
+
+}; // end of isc::dhcp::test namespace
+}; // end of isc::dhcp namespace
+}; // end of isc namespace
+
+#endif // D2_UNITTEST_H

src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -195,7 +195,6 @@ public:
     using Dhcpv4Srv::sanityCheck;
     using Dhcpv4Srv::srvidToString;
     using Dhcpv4Srv::unpackOptions;
-    using Dhcpv4Srv::name_change_reqs_;
     using Dhcpv4Srv::classifyPacket;
     using Dhcpv4Srv::accept;
     using Dhcpv4Srv::acceptMessageType;

src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -34,13 +34,17 @@ namespace {
 
 class NameDhcpv4SrvTest : public Dhcpv4SrvTest {
 public:
+    // Reference to D2ClientMgr singleton
+    D2ClientMgr& d2_mgr_;
+
     // Bit Constants for turning on and off DDNS configuration options.
     static const uint16_t ALWAYS_INCLUDE_FQDN = 1;
     static const uint16_t OVERRIDE_NO_UPDATE = 2;
     static const uint16_t OVERRIDE_CLIENT_UPDATE = 4;
     static const uint16_t REPLACE_CLIENT_NAME = 8;
 
-    NameDhcpv4SrvTest() : Dhcpv4SrvTest() {
+    NameDhcpv4SrvTest() : Dhcpv4SrvTest(),
+        d2_mgr_(CfgMgr::instance().getD2ClientMgr()) {
         srv_ = new NakedDhcpv4Srv(0);
         // Config DDNS to be enabled, all controls off
         enableD2();
@@ -48,6 +52,9 @@ public:
 
     virtual ~NameDhcpv4SrvTest() {
         delete srv_;
+        // CfgMgr singleton doesn't get wiped between tests, so  we'll
+        // disable D2 explictly between tests.
+        disableD2();
     }
 
     /// @brief Sets the server's DDNS configuration to ddns updates disabled.
@@ -69,15 +76,15 @@ public:
         D2ClientConfigPtr cfg;
 
         ASSERT_NO_THROW(cfg.reset(new D2ClientConfig(true,
-                                  isc::asiolink::IOAddress("192.0.2.1"), 477,
+                                  isc::asiolink::IOAddress("127.0.0.1"), 53001,
                                   dhcp_ddns::NCR_UDP, dhcp_ddns::FMT_JSON,
                                   (mask & ALWAYS_INCLUDE_FQDN),
                                   (mask & OVERRIDE_NO_UPDATE),
                                   (mask & OVERRIDE_CLIENT_UPDATE),
                                   (mask & REPLACE_CLIENT_NAME),
                                   "myhost", "example.com")));
-
-        CfgMgr::instance().setD2ClientConfig(cfg);
+        ASSERT_NO_THROW(CfgMgr::instance().setD2ClientConfig(cfg));
+        ASSERT_NO_THROW(srv_->startD2());
     }
 
     // Create a lease to be used by various tests.
@@ -317,16 +324,19 @@ public:
                                  const time_t cltt,
                                  const uint16_t len,
                                  const bool not_strict_expire_check = false) {
-        NameChangeRequest ncr = srv_->name_change_reqs_.front();
-        EXPECT_EQ(type, ncr.getChangeType());
-        EXPECT_EQ(forward, ncr.isForwardChange());
-        EXPECT_EQ(reverse, ncr.isReverseChange());
-        EXPECT_EQ(addr, ncr.getIpAddress());
-        EXPECT_EQ(fqdn, ncr.getFqdn());
+        NameChangeRequestPtr ncr;
+        ASSERT_NO_THROW(ncr = d2_mgr_.peekAt(0));
+        ASSERT_TRUE(ncr);
+
+        EXPECT_EQ(type, ncr->getChangeType());
+        EXPECT_EQ(forward, ncr->isForwardChange());
+        EXPECT_EQ(reverse, ncr->isReverseChange());
+        EXPECT_EQ(addr, ncr->getIpAddress());
+        EXPECT_EQ(fqdn, ncr->getFqdn());
         // Compare dhcid if it is not empty. In some cases, the DHCID is
         // not known in advance and can't be compared.
         if (!dhcid.empty()) {
-            EXPECT_EQ(dhcid, ncr.getDhcid().toStr());
+            EXPECT_EQ(dhcid, ncr->getDhcid().toStr());
         }
         // In some cases, the test doesn't have access to the last transmission
         // time for the particular client. In such cases, the test can use the
@@ -334,13 +344,15 @@ public:
         // for equality but rather check that the lease expiration time is not
         // greater than the current time + lease lifetime.
         if (not_strict_expire_check) {
-            EXPECT_GE(cltt + len, ncr.getLeaseExpiresOn());
+            EXPECT_GE(cltt + len, ncr->getLeaseExpiresOn());
         } else {
-            EXPECT_EQ(cltt + len, ncr.getLeaseExpiresOn());
+            EXPECT_EQ(cltt + len, ncr->getLeaseExpiresOn());
         }
-        EXPECT_EQ(len, ncr.getLeaseLength());
-        EXPECT_EQ(isc::dhcp_ddns::ST_NEW, ncr.getStatus());
-        srv_->name_change_reqs_.pop();
+        EXPECT_EQ(len, ncr->getLeaseLength());
+        EXPECT_EQ(isc::dhcp_ddns::ST_NEW, ncr->getStatus());
+
+        // Process the message off the queue
+        ASSERT_NO_THROW(d2_mgr_.runReadyIO());
     }
 
 
@@ -373,20 +385,23 @@ public:
         checkResponse(reply, DHCPACK, 1234);
         checkFqdnFlags(reply, response_flags);
 
-        // There should be an NCR only if response S flag is 1.
-        /// @todo This logic will need to change if forward and reverse
-        /// updates are ever controlled independently.
-        if ((response_flags & Option4ClientFqdn::FLAG_S) == 0) {
-            ASSERT_EQ(0, srv_->name_change_reqs_.size());
-        } else {
-            // Verify that there is one NameChangeRequest generated as expected.
-            ASSERT_EQ(1, srv_->name_change_reqs_.size());
-            verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
-                                    reply->getYiaddr().toText(),
-                                    "myhost.example.com.",
-                                    "", // empty DHCID means don't check it
-                                    time(NULL) + subnet_->getValid(),
-                                    subnet_->getValid(), true);
+        // NCRs cannot be sent to the d2_mgr unless updates are enabled.
+        if (d2_mgr_.ddnsEnabled()) {
+            // There should be an NCR only if response S flag is 1.
+            /// @todo This logic will need to change if forward and reverse
+            /// updates are ever controlled independently.
+            if ((response_flags & Option4ClientFqdn::FLAG_S) == 0) {
+                ASSERT_EQ(0, d2_mgr_.getQueueSize());
+            } else {
+                // Verify that there is one NameChangeRequest as expected.
+                ASSERT_EQ(1, d2_mgr_.getQueueSize());
+                verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+                                        reply->getYiaddr().toText(),
+                                        "myhost.example.com.",
+                                        "", // empty DHCID means don't check it
+                                        time(NULL) + subnet_->getValid(),
+                                        subnet_->getValid(), true);
+            }
         }
     }
 
@@ -639,7 +654,7 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsNewLease) {
     Lease4Ptr old_lease;
 
     ASSERT_NO_THROW(srv_->createNameChangeRequests(lease, old_lease));
-    ASSERT_EQ(1, srv_->name_change_reqs_.size());
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
 
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             "192.0.2.3", "myhost.example.com.",
@@ -658,7 +673,7 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsRenewNoChange) {
     old_lease->valid_lft_ += 100;
 
     ASSERT_NO_THROW(srv_->createNameChangeRequests(lease, old_lease));
-    EXPECT_TRUE(srv_->name_change_reqs_.empty());
+    ASSERT_EQ(0, d2_mgr_.getQueueSize());
 }
 
 // Test that no NameChangeRequest is generated when forward and reverse
@@ -671,7 +686,7 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsNoUpdate) {
                                    "lease2.example.com.",
                                    false, false);
     ASSERT_NO_THROW(srv_->createNameChangeRequests(lease2, lease1));
-    EXPECT_EQ(1, srv_->name_change_reqs_.size());
+    EXPECT_EQ(1, d2_mgr_.getQueueSize());
 
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
                             "192.0.2.3", "lease1.example.com.",
@@ -683,7 +698,7 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsNoUpdate) {
     lease2->fqdn_rev_ = true;
     lease2->fqdn_fwd_ = true;
     ASSERT_NO_THROW(srv_->createNameChangeRequests(lease2, lease1));
-    EXPECT_EQ(1, srv_->name_change_reqs_.size());
+    EXPECT_EQ(1, d2_mgr_.getQueueSize());
 
 }
 
@@ -697,7 +712,7 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsRenew) {
                                    "lease2.example.com.",
                                    true, true);
     ASSERT_NO_THROW(srv_->createNameChangeRequests(lease2, lease1));
-    ASSERT_EQ(2, srv_->name_change_reqs_.size());
+    ASSERT_EQ(2, d2_mgr_.getQueueSize());
 
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
                             "192.0.2.3", "lease1.example.com.",
@@ -742,7 +757,7 @@ TEST_F(NameDhcpv4SrvTest, processDiscover) {
     ASSERT_NO_THROW(reply = srv_->processDiscover(req));
     checkResponse(reply, DHCPOFFER, 1234);
 
-    EXPECT_TRUE(srv_->name_change_reqs_.empty());
+    EXPECT_EQ(0, d2_mgr_.getQueueSize());
 }
 
 // Test that server generates client's hostname from the IP address assigned
@@ -761,7 +776,7 @@ TEST_F(NameDhcpv4SrvTest, processRequestFqdnEmptyDomainName) {
     checkResponse(reply, DHCPACK, 1234);
 
     // Verify that there is one NameChangeRequest generated.
-    ASSERT_EQ(1, srv_->name_change_reqs_.size());
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
 
     // The hostname is generated from the IP address acquired (yiaddr).
     std::string hostname = generatedNameFromAddress(reply->getYiaddr());
@@ -818,7 +833,7 @@ TEST_F(NameDhcpv4SrvTest, processRequestEmptyHostname) {
     checkResponse(reply, DHCPACK, 1234);
 
     // Verify that there is one NameChangeRequest generated.
-    ASSERT_EQ(1, srv_->name_change_reqs_.size());
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
 
     // The hostname is generated from the IP address acquired (yiaddr).
     std::string hostname = generatedNameFromAddress(reply->getYiaddr());
@@ -848,7 +863,7 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsFqdn) {
     checkResponse(reply, DHCPACK, 1234);
 
     // Verify that there is one NameChangeRequest generated.
-    ASSERT_EQ(1, srv_->name_change_reqs_.size());
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             reply->getYiaddr().toText(), "myhost.example.com.",
                             "00010132E91AA355CFBB753C0F0497A5A940436"
@@ -868,7 +883,7 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsFqdn) {
     checkResponse(reply, DHCPACK, 1234);
 
     // There should be two NameChangeRequests. Verify that they are valid.
-    ASSERT_EQ(2, srv_->name_change_reqs_.size());
+    ASSERT_EQ(2, d2_mgr_.getQueueSize());
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
                             reply->getYiaddr().toText(),
                             "myhost.example.com.",
@@ -905,7 +920,7 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsHostname) {
     checkResponse(reply, DHCPACK, 1234);
 
     // Verify that there is one NameChangeRequest generated.
-    ASSERT_EQ(1, srv_->name_change_reqs_.size());
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             reply->getYiaddr().toText(), "myhost.example.com.",
                             "00010132E91AA355CFBB753C0F0497A5A940436"
@@ -926,7 +941,7 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsHostname) {
     checkResponse(reply, DHCPACK, 1234);
 
     // There should be two NameChangeRequests. Verify that they are valid.
-    ASSERT_EQ(2, srv_->name_change_reqs_.size());
+    ASSERT_EQ(2, d2_mgr_.getQueueSize());
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
                             reply->getYiaddr().toText(),
                             "myhost.example.com.",
@@ -962,7 +977,7 @@ TEST_F(NameDhcpv4SrvTest, processRequestRelease) {
     checkResponse(reply, DHCPACK, 1234);
 
     // Verify that there is one NameChangeRequest generated for lease.
-    ASSERT_EQ(1, srv_->name_change_reqs_.size());
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             reply->getYiaddr().toText(), "myhost.example.com.",
                             "00010132E91AA355CFBB753C0F0497A5A940436"
@@ -979,7 +994,7 @@ TEST_F(NameDhcpv4SrvTest, processRequestRelease) {
 
     // The lease has been removed, so there should be a NameChangeRequest to
     // remove corresponding DNS entries.
-    ASSERT_EQ(1, srv_->name_change_reqs_.size());
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
                             reply->getYiaddr().toText(), "myhost.example.com.",
                             "00010132E91AA355CFBB753C0F0497A5A940436"
@@ -990,6 +1005,9 @@ TEST_F(NameDhcpv4SrvTest, processRequestRelease) {
 // Test that when the Release message is sent for a previously acquired lease
 // and DDNS updates are disabled that server does NOT generate a
 // NameChangeRequest to remove entries corresponding to the released lease.
+// Queue size is not available when updates are not enabled, however,
+// attempting to send a NCR when updates disabled will result in a throw.
+// If no throws are experienced then no attempt was made to send a NCR.
 TEST_F(NameDhcpv4SrvTest, processRequestReleaseUpdatesDisabled) {
     // Create fake interfaces and open fake sockets.
     IfaceMgrTestConfig test_config(true);
@@ -1008,10 +1026,6 @@ TEST_F(NameDhcpv4SrvTest, processRequestReleaseUpdatesDisabled) {
     ASSERT_NO_THROW(reply = srv_->processRequest(req));
     checkResponse(reply, DHCPACK, 1234);
 
-    // With DDNS updates disabled, there should be not be a NameChangeRequest
-    // for the add.
-    ASSERT_EQ(0, srv_->name_change_reqs_.size());
-
     // Create and process the Release message.
     Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
     rel->setCiaddr(reply->getYiaddr());
@@ -1019,10 +1033,6 @@ TEST_F(NameDhcpv4SrvTest, processRequestReleaseUpdatesDisabled) {
     rel->addOption(generateClientId());
     rel->addOption(srv_->getServerID());
     ASSERT_NO_THROW(srv_->processRelease(rel));
-
-    // With DDNS updates disabled, there should be not be a NameChangeRequest
-    // for the remove.
-    ASSERT_EQ(0, srv_->name_change_reqs_.size());
 }
 
 

src/lib/dhcp_ddns/dhcp_ddns_messages.mes

@@ -19,6 +19,12 @@ This is an error message that indicates that an invalid request to update
 a DNS entry was received by the application.  Either the format or the content
 of the request is incorrect. The request will be ignored.
 
+% DHCP_DDNS_NCR_FLUSH_IO_ERROR DHCP-DDNS Last send before stopping did not complete successfully: %1
+This is an error message that indicates the DHCP-DDNS client was unable to
+complete the last send prior to exiting send mode.  This is a programmatic
+error, highly unlikely to occur, and should not impair the application's ability
+to process requests.
+
 % DHCP_DDNS_NCR_LISTEN_CLOSE_ERROR application encountered an error while closing the listener used to receive NameChangeRequests : %1
 This is an error message that indicates the application was unable to close the
 listener connection used to receive NameChangeRequests.  Closure may occur

src/lib/dhcp_ddns/ncr_io.cc

@@ -15,6 +15,7 @@
 #include <dhcp_ddns/dhcp_ddns_log.h>
 #include <dhcp_ddns/ncr_io.h>
 
+#include <asio.hpp>
 #include <boost/algorithm/string/predicate.hpp>
 
 namespace isc {
@@ -159,7 +160,7 @@ NameChangeListener::invokeRecvHandler(const Result result,
 NameChangeSender::NameChangeSender(RequestSendHandler& send_handler,
                                    size_t send_queue_max)
     : sending_(false), send_handler_(send_handler),
-      send_queue_max_(send_queue_max) {
+      send_queue_max_(send_queue_max), io_service_(NULL) {
 
     // Queue size must be big enough to hold at least 1 entry.
     setQueueMaxSize(send_queue_max);
@@ -177,6 +178,8 @@ NameChangeSender::startSending(isc::asiolink::IOService& io_service) {
 
     // Call implementation dependent open.
     try {
+        // Remember io service we're given.
+        io_service_ = &io_service;
         open(io_service);
     } catch (const isc::Exception& ex) {
         stopSending();
@@ -185,10 +188,30 @@ NameChangeSender::startSending(isc::asiolink::IOService& io_service) {
 
     // Set our status to sending.
     setSending(true);
+
+    // If there's any queued already.. we'll start sending.
+    sendNext();
 }
 
 void
 NameChangeSender::stopSending() {
+    // Set it send indicator to false, no matter what. This allows us to at 
+    // least try to re-open via startSending(). Also, setting it false now, 
+    // allows us to break sendNext() chain in invokeSendHandler.
+    setSending(false);
+
+    // If there is an outstanding IO to complete, attempt to process it.
+    if (ioReady() && io_service_ != NULL) {
+        try {
+            runReadyIO();
+        } catch (const std::exception& ex) {
+            // Swallow exceptions. If we have some sort of error we'll log
+            // it but we won't propagate the throw.
+            LOG_ERROR(dhcp_ddns_logger,
+                  DHCP_DDNS_NCR_FLUSH_IO_ERROR).arg(ex.what());
+        }
+    }
+
     try {
         // Call implementation dependent close.
         close();
@@ -199,9 +222,7 @@ NameChangeSender::stopSending() {
                   DHCP_DDNS_NCR_SEND_CLOSE_ERROR).arg(ex.what());
     }
 
-    // Set it false, no matter what.  This allows us to at least try to
-    // re-open via startSending().
-    setSending(false);
+    io_service_ = NULL;
 }
 
 void
@@ -274,7 +295,9 @@ NameChangeSender::invokeSendHandler(const NameChangeSender::Result result) {
 
     // Set up the next send
     try {
-        sendNext();
+        if (amSending()) {
+            sendNext();
+        }
     } catch (const isc::Exception& ex) {
         // It is possible though unlikely, for sendNext to fail without
         // scheduling the send. While, unlikely, it does mean the callback
@@ -367,5 +390,20 @@ NameChangeSender::getSelectFd() {
     isc_throw(NotImplemented, "NameChangeSender::getSelectFd is not supported");
 }
 
+void
+NameChangeSender::runReadyIO() {
+    if (!io_service_) {
+        isc_throw(NcrSenderError, "NameChangeSender::runReadyIO"
+                  " sender io service is null");
+    }
+
+    // We shouldn't be here if IO isn't ready to execute.
+    // By running poll we're gauranteed not to hang.
+    /// @todo Trac# 3325 requests that asiolink::IOService provide a
+    /// wrapper for poll().
+    io_service_->get_io_service().poll_one();
+}
+
+
 } // namespace isc::dhcp_ddns
 } // namespace isc

src/lib/dhcp_ddns/ncr_io.h

@@ -161,7 +161,7 @@ public:
 /// Assuming the open is successful, startListener will call receiveNext, to
 /// initiate an asynchronous receive.  This method calls the virtual method,
 /// doReceive().  The listener derivation uses doReceive to instigate an IO
-/// layer asynchronous receieve passing in its IO layer callback to
+/// layer asynchronous receive passing in its IO layer callback to
 /// handle receive events from the IO source.
 ///
 /// As stated earlier, the derivation's NameChangeRequest completion handler
@@ -576,6 +576,11 @@ public:
     /// @throw NcrSenderError if the sender is not in send mode,
     virtual int getSelectFd() = 0;
 
+    /// @brief Returns whether or not the sender has IO ready to process.
+    ///
+    /// @return true if the sender has at IO ready, false otherwise.
+    virtual bool ioReady() = 0;
+
 protected:
     /// @brief Dequeues and sends the next request on the send queue.
     ///
@@ -688,7 +693,7 @@ public:
         return (send_queue_max_);
     }
 
-    /// @brief Sets the maxium queue size to the given value.
+    /// @brief Sets the maximum queue size to the given value.
     ///
     /// Sets the maximum number of entries allowed in the queue to the
     /// the given value.
@@ -715,6 +720,28 @@ public:
     /// end of the queue.
     const NameChangeRequestPtr& peekAt(const size_t index) const;
 
+    /// @brief Processes sender IO events
+    ///
+    /// Executes at most one ready handler on the sender's IO service. If
+    /// no handlers are ready it returns immediately.
+    ///
+    /// @warning - Running all ready handlers, in theory, could process all
+    /// messages currently queued.
+    ///
+    /// NameChangeSender daisy chains requests together in its completion
+    /// by one message completion's handler initiating the next message's send.
+    /// When using UDP, a send immediately marks its event handler as ready
+    /// to run.  If this occurs inside a call to ioservice::poll() or run(),
+    /// that event will also be run.  If that handler calls UDP send then
+    /// that send's handler will be marked ready and executed and so on.  If
+    /// there were 1000 messages in the queue then all them would be sent from
+    /// within the context of one call to runReadyIO().
+    /// By running only one handler at time, we ensure that NCR IO activity
+    /// doesn't starve other processing.  It is unclear how much of a real
+    /// threat this poses but for now it is best to err on the side of caution.
+    ///
+    virtual void runReadyIO();
+
 protected:
     /// @brief Returns a reference to the send queue.
     SendQueue& getSendQueue() {
@@ -746,6 +773,12 @@ private:
 
     /// @brief Pointer to the request which is in the process of being sent.
     NameChangeRequestPtr ncr_to_send_;
+
+    /// @brief Pointer to the IOService currently being used by the sender.
+    /// @note We need to remember the io_service but we receive it by
+    /// reference.  Use a raw pointer to store it.  This value should never be
+    /// exposed and is only valid while in send mode.
+    asiolink::IOService* io_service_;
 };
 
 /// @brief Defines a smart pointer to an instance of a sender.

src/lib/dhcp_ddns/ncr_udp.cc

@@ -359,6 +359,16 @@ NameChangeUDPSender::getSelectFd() {
     return(watch_socket_->getSelectFd());
 }
 
+bool
+NameChangeUDPSender::ioReady() {
+    if (watch_socket_) {
+        return (watch_socket_->isReady());
+    }
+
+    return (false);
+}
+
+
 
 }; // end of isc::dhcp_ddns namespace
 }; // end of isc namespace

src/lib/dhcp_ddns/ncr_udp.h

@@ -542,6 +542,11 @@ public:
     /// @throw NcrSenderError if the sender is not in send mode,
     virtual int getSelectFd();
 
+    /// @brief Returns whether or not the sender has IO ready to process.
+    ///
+    /// @return true if the sender has at IO ready, false otherwise.
+    virtual bool ioReady();
+
 private:
     /// @brief IP address from which to send.
     isc::asiolink::IOAddress ip_address_;

src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc

@@ -361,7 +361,11 @@ TEST(NameChangeUDPSenderBasicTest, basicSendTests) {
 
     // Verify select_fd is valid and currently shows no ready to read.
     ASSERT_NE(dhcp_ddns::WatchSocket::INVALID_SOCKET, select_fd);
+
+    // Make sure select_fd does evaluates to not ready via select and
+    // that ioReady() method agrees.
     ASSERT_EQ(0, selectCheck(select_fd));
+    ASSERT_FALSE(sender.ioReady());
 
     // Iterate over a series of messages, sending each one. Since we
     // do not invoke IOService::run, then the messages should accumulate
@@ -392,18 +396,22 @@ TEST(NameChangeUDPSenderBasicTest, basicSendTests) {
     // IOService::run_one. This should complete the send of exactly one
     // message and the queue count should decrement accordingly.
     for (int i = num_msgs; i > 0; i--) {
-        // Verify that sender shows IO ready.
+        // Make sure select_fd does evaluates to ready via select and
+        // that ioReady() method agrees.
         ASSERT_TRUE(selectCheck(select_fd) > 0);
+        ASSERT_TRUE(sender.ioReady());
 
         // Execute at one ready handler.
-        io_service.run_one();
+        ASSERT_NO_THROW(sender.runReadyIO());
 
         // Verify that the queue count decrements in step with each run.
         EXPECT_EQ(i-1, sender.getQueueSize());
     }
 
-    // Verify that sender shows no IO ready.
-    EXPECT_EQ(0, selectCheck(select_fd));
+    // Make sure select_fd does evaluates to not ready via select and
+    // that ioReady() method agrees.
+    ASSERT_EQ(0, selectCheck(select_fd));
+    ASSERT_FALSE(sender.ioReady());
 
     // Verify that the queue is empty.
     EXPECT_EQ(0, sender.getQueueSize());
@@ -419,22 +427,79 @@ TEST(NameChangeUDPSenderBasicTest, basicSendTests) {
     // Verify that flushing the queue is not allowed in sending state.
     EXPECT_THROW(sender.clearSendQueue(), NcrSenderError);
 
-    // Put a message on the queue.
-    EXPECT_NO_THROW(sender.sendRequest(ncr));
-    EXPECT_EQ(1, sender.getQueueSize());
+    // Put num_msgs messages on the queue.
+    for (int i = 0; i < num_msgs; i++) {
+        ASSERT_NO_THROW(ncr = NameChangeRequest::fromJSON(valid_msgs[i]));
+        EXPECT_NO_THROW(sender.sendRequest(ncr));
+    }
+
+    // Make sure we have number of messages expected.
+    EXPECT_EQ(num_msgs, sender.getQueueSize());
 
     // Verify that we can gracefully stop sending.
     EXPECT_NO_THROW(sender.stopSending());
     EXPECT_FALSE(sender.amSending());
 
     // Verify that the queue is preserved after leaving sending state.
-    EXPECT_EQ(1, sender.getQueueSize());
+    EXPECT_EQ(num_msgs - 1, sender.getQueueSize());
 
     // Verify that flushing the queue works when not sending.
     EXPECT_NO_THROW(sender.clearSendQueue());
     EXPECT_EQ(0, sender.getQueueSize());
 }
 
+/// @brief Tests that sending gets kick-started if the queue isn't empty
+/// when startSending is called.
+TEST(NameChangeUDPSenderBasicTest, autoStart) {
+    isc::asiolink::IOAddress ip_address(TEST_ADDRESS);
+    isc::asiolink::IOService io_service;
+    SimpleSendHandler ncr_handler;
+
+    // Tests are based on a list of messages, get the count now.
+    int num_msgs = sizeof(valid_msgs)/sizeof(char*);
+
+    // Create the sender, setting the queue max equal to the number of
+    // messages we will have in the list.
+    NameChangeUDPSender sender(ip_address, SENDER_PORT, ip_address,
+                               LISTENER_PORT, FMT_JSON, ncr_handler,
+                               num_msgs, true);
+
+    // Verify that we can start sending.
+    EXPECT_NO_THROW(sender.startSending(io_service));
+    EXPECT_TRUE(sender.amSending());
+
+    // Queue up messages.
+    NameChangeRequestPtr ncr;
+    for (int i = 0; i < num_msgs; i++) {
+        ASSERT_NO_THROW(ncr = NameChangeRequest::fromJSON(valid_msgs[i]));
+        EXPECT_NO_THROW(sender.sendRequest(ncr));
+    }
+    // Make sure queue count is what we expect.
+    EXPECT_EQ(num_msgs, sender.getQueueSize());
+
+    // Stop sending.
+    ASSERT_NO_THROW(sender.stopSending());
+    ASSERT_FALSE(sender.amSending());
+
+    // We should have completed the first message only.
+    EXPECT_EQ(--num_msgs, sender.getQueueSize());
+
+    // Restart sending.
+    EXPECT_NO_THROW(sender.startSending(io_service));
+
+    // We should be able to loop through remaining messages and send them.
+    for (int i = num_msgs; i > 0; i--) {
+        // ioReady() should evaluate to true.
+        ASSERT_TRUE(sender.ioReady());
+
+        // Execute at one ready handler.
+        ASSERT_NO_THROW(sender.runReadyIO());
+    }
+
+    // Verify that the queue is empty.
+    EXPECT_EQ(0, sender.getQueueSize());
+}
+
 /// @brief Tests NameChangeUDPSender basic send  with INADDR_ANY and port 0.
 TEST(NameChangeUDPSenderBasicTest, anyAddressSend) {
     isc::asiolink::IOAddress ip_address(TEST_ADDRESS);
@@ -454,23 +519,19 @@ TEST(NameChangeUDPSenderBasicTest, anyAddressSend) {
     ASSERT_NO_THROW(sender.startSending(io_service));
     EXPECT_TRUE(sender.amSending());
 
-    // Fetch the sender's select-fd.
-    int select_fd = sender.getSelectFd();
-
     // Create and queue up a message.
     NameChangeRequestPtr ncr;
     ASSERT_NO_THROW(ncr = NameChangeRequest::fromJSON(valid_msgs[0]));
     EXPECT_NO_THROW(sender.sendRequest(ncr));
     EXPECT_EQ(1, sender.getQueueSize());
 
-    // message and the queue count should decrement accordingly.
-    // Execute at one ready handler.
-    ASSERT_TRUE(selectCheck(select_fd) > 0);
-    ASSERT_NO_THROW(io_service.run_one());
+    // Verify we have a ready IO, then execute at one ready handler.
+    ASSERT_TRUE(sender.ioReady());
+    ASSERT_NO_THROW(sender.runReadyIO());
 
     // Verify that sender shows no IO ready.
     // and that the queue is empty.
-    EXPECT_EQ(0, selectCheck(select_fd));
+    ASSERT_FALSE(sender.ioReady());
     EXPECT_EQ(0, sender.getQueueSize());
 }
 
@@ -514,6 +575,9 @@ TEST(NameChangeSender, assumeQueue) {
     // Take sender1 out of send mode.
     ASSERT_NO_THROW(sender1.stopSending());
     ASSERT_FALSE(sender1.amSending());
+    // Stopping should have completed the first message.
+    --num_msgs;
+    EXPECT_EQ(num_msgs, sender1.getQueueSize());
 
     // Transfer should succeed. Verify sender1 has none,
     // and sender2 has num_msgs queued.
@@ -719,7 +783,7 @@ TEST(NameChangeUDPSenderBasicTest, watchClosedAfterSendRequest) {
     // Run one handler. This should execute the send completion handler
     // after sending the first message.  Duing completion handling, we will
     // attempt to queue the second message which should fail.
-    ASSERT_NO_THROW(io_service.run_one());
+    ASSERT_NO_THROW(sender.runReadyIO());
 
     // Verify handler got called twice. First request should have be sent
     // without error, second call should have failed to send due to watch
@@ -767,7 +831,7 @@ TEST(NameChangeUDPSenderBasicTest, watchSocketBadRead) {
     // after sending the message.  Duing completion handling clearing the
     // watch socket should fail, which will close the socket, but not
     // result in a throw.
-    ASSERT_NO_THROW(io_service.run_one());
+    ASSERT_NO_THROW(sender.runReadyIO());
 
     // Verify handler got called twice. First request should have be sent
     // without error, second call should have failed to send due to watch

src/lib/dhcpsrv/Makefile.am

@@ -39,7 +39,8 @@ libb10_dhcpsrv_la_SOURCES  =
 libb10_dhcpsrv_la_SOURCES += addr_utilities.cc addr_utilities.h
 libb10_dhcpsrv_la_SOURCES += alloc_engine.cc alloc_engine.h
 libb10_dhcpsrv_la_SOURCES += callout_handle_store.h
-libb10_dhcpsrv_la_SOURCES += d2_client.cc d2_client.h
+libb10_dhcpsrv_la_SOURCES += d2_client_cfg.cc d2_client_cfg.h
+libb10_dhcpsrv_la_SOURCES += d2_client_mgr.cc d2_client_mgr.h
 libb10_dhcpsrv_la_SOURCES += dbaccess_parser.cc dbaccess_parser.h
 libb10_dhcpsrv_la_SOURCES += dhcpsrv_log.cc dhcpsrv_log.h
 libb10_dhcpsrv_la_SOURCES += cfgmgr.cc cfgmgr.h

src/lib/dhcpsrv/cfgmgr.h

@@ -20,7 +20,7 @@
 #include <dhcp/option_definition.h>
 #include <dhcp/option_space.h>
 #include <dhcp/classify.h>
-#include <dhcpsrv/d2_client.h>
+#include <dhcpsrv/d2_client_mgr.h>
 #include <dhcpsrv/option_space_container.h>
 #include <dhcpsrv/pool.h>
 #include <dhcpsrv/subnet.h>

src/lib/dhcpsrv/d2_client_cfg.cc

@@ -0,0 +1,146 @@
+// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dhcp_ddns/ncr_udp.h>
+#include <dhcpsrv/d2_client_cfg.h>
+#include <dhcpsrv/dhcpsrv_log.h>
+
+#include <string>
+
+using namespace std;
+
+namespace isc {
+namespace dhcp {
+
+D2ClientConfig::D2ClientConfig(const  bool enable_updates,
+                               const isc::asiolink::IOAddress& server_ip,
+                               const size_t server_port,
+                               const dhcp_ddns::
+                                     NameChangeProtocol& ncr_protocol,
+                               const dhcp_ddns::
+                                     NameChangeFormat& ncr_format,
+                               const bool always_include_fqdn,
+                               const bool override_no_update,
+                               const bool override_client_update,
+                               const bool replace_client_name,
+                               const std::string& generated_prefix,
+                               const std::string& qualifying_suffix)
+    : enable_updates_(enable_updates),
+    server_ip_(server_ip),
+    server_port_(server_port),
+    ncr_protocol_(ncr_protocol),
+    ncr_format_(ncr_format),
+    always_include_fqdn_(always_include_fqdn),
+    override_no_update_(override_no_update),
+    override_client_update_(override_client_update),
+    replace_client_name_(replace_client_name),
+    generated_prefix_(generated_prefix),
+    qualifying_suffix_(qualifying_suffix) {
+    validateContents();
+}
+
+D2ClientConfig::D2ClientConfig()
+    : enable_updates_(false),
+      server_ip_(isc::asiolink::IOAddress("0.0.0.0")),
+      server_port_(0),
+      ncr_protocol_(dhcp_ddns::NCR_UDP),
+      ncr_format_(dhcp_ddns::FMT_JSON),
+      always_include_fqdn_(false),
+      override_no_update_(false),
+      override_client_update_(false),
+      replace_client_name_(false),
+      generated_prefix_("myhost"),
+      qualifying_suffix_("example.com") {
+    validateContents();
+}
+
+D2ClientConfig::~D2ClientConfig(){};
+
+void
+D2ClientConfig::enableUpdates(bool enable) {
+    enable_updates_ = enable;
+}
+
+void
+D2ClientConfig::validateContents() {
+    if (ncr_format_ != dhcp_ddns::FMT_JSON) {
+        isc_throw(D2ClientError, "D2ClientConfig: NCR Format:"
+                    << dhcp_ddns::ncrFormatToString(ncr_format_)
+                    << " is not yet supported");
+    }
+
+    if (ncr_protocol_ != dhcp_ddns::NCR_UDP) {
+        isc_throw(D2ClientError, "D2ClientConfig: NCR Protocol:"
+                    << dhcp_ddns::ncrProtocolToString(ncr_protocol_)
+                    << " is not yet supported");
+    }
+
+    /// @todo perhaps more validation we should do yet?
+    /// Are there any invalid combinations of options we need to test against?
+}
+
+bool
+D2ClientConfig::operator == (const D2ClientConfig& other) const {
+    return ((enable_updates_ == other.enable_updates_) &&
+            (server_ip_ == other.server_ip_) &&
+            (server_port_ == other.server_port_) &&
+            (ncr_protocol_ == other.ncr_protocol_) &&
+            (ncr_format_ == other.ncr_format_) &&
+            (always_include_fqdn_ == other.always_include_fqdn_) &&
+            (override_no_update_ == other.override_no_update_) &&
+            (override_client_update_ == other.override_client_update_) &&
+            (replace_client_name_ == other.replace_client_name_) &&
+            (generated_prefix_ == other.generated_prefix_) &&
+            (qualifying_suffix_ == other.qualifying_suffix_));
+}
+
+bool
+D2ClientConfig::operator != (const D2ClientConfig& other) const {
+    return (!(*this == other));
+}
+
+std::string
+D2ClientConfig::toText() const {
+    std::ostringstream stream;
+
+    stream << "enable_updates: " << (enable_updates_ ? "yes" : "no");
+    if (enable_updates_) {
+        stream << ", server_ip: " << server_ip_.toText()
+               << ", server_port: " << server_port_
+               << ", ncr_protocol: " << ncr_protocol_
+               << ", ncr_format: " << ncr_format_
+               << ", always_include_fqdn: " << (always_include_fqdn_ ?
+                                                "yes" : "no")
+               << ", override_no_update: " << (override_no_update_ ?
+                                               "yes" : "no")
+               << ", override_client_update: " << (override_client_update_ ?
+                                                   "yes" : "no")
+               << ", replace_client_name: " << (replace_client_name_ ?
+                                                "yes" : "no")
+               << ", generated_prefix: [" << generated_prefix_ << "]"
+               << ", qualifying_suffix: [" << qualifying_suffix_ << "]";
+    }
+
+    return (stream.str());
+}
+
+std::ostream&
+operator<<(std::ostream& os, const D2ClientConfig& config) {
+    os << config.toText();
+    return (os);
+}
+
+};  // namespace dhcp
+
+};  // namespace isc

src/lib/dhcpsrv/d2_client_cfg.h

@@ -0,0 +1,226 @@
+// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef D2_CLIENT_CFG_H
+#define D2_CLIENT_CFG_H
+
+/// @file d2_client_cfg.h Defines the D2ClientConfig class.
+/// This file defines the classes Kea uses to manage configuration needed to
+/// act as a client of the b10-dhcp-ddns module (aka D2).
+///
+#include <asiolink/io_address.h>
+#include <dhcp_ddns/ncr_io.h>
+#include <exceptions/exceptions.h>
+
+#include <boost/shared_ptr.hpp>
+
+#include <stdint.h>
+#include <string>
+#include <vector>
+
+namespace isc {
+namespace dhcp {
+
+
+/// An exception that is thrown if an error occurs while configuring
+/// the D2 DHCP DDNS client.
+class D2ClientError : public isc::Exception {
+public:
+
+    /// @brief constructor
+    ///
+    /// @param file name of the file, where exception occurred
+    /// @param line line of the file, where exception occurred
+    /// @param what text description of the issue that caused exception
+    D2ClientError(const char* file, size_t line, const char* what)
+        : isc::Exception(file, line, what) {}
+};
+
+/// @brief Acts as a storage vault for D2 client configuration
+///
+/// A simple container class for storing and retrieving the configuration
+/// parameters associated with DHCP-DDNS and acting as a client of D2.
+/// Instances of this class may be constructed through configuration parsing.
+///
+class D2ClientConfig {
+public:
+    /// @brief Constructor
+    ///
+    /// @param enable_updates Enables DHCP-DDNS updates
+    /// @param server_ip IP address of the b10-dhcp-ddns server (IPv4 or IPv6)
+    /// @param server_port IP port of the b10-dhcp-ddns server
+    /// @param ncr_protocol Socket protocol to use with b10-dhcp-ddns
+    /// Currently only UDP is supported.
+    /// @param ncr_format Format of the b10-dhcp-ddns requests.
+    /// Currently only JSON format is supported.
+    /// @param always_include_fqdn Enables always including the FQDN option in
+    /// DHCP responses.
+    /// @param override_no_update Enables updates, even if clients request no
+    /// updates.
+    /// @param override_client_update Perform updates, even if client requested
+    /// delegation.
+    /// @param replace_client_name enables replacement of the domain-name
+    /// supplied by the client with a generated name.
+    /// @param generated_prefix Prefix to use when generating domain-names.
+    /// @param  qualifying_suffix Suffix to use to qualify partial domain-names.
+    ///
+    /// @throw D2ClientError if given an invalid protocol or format.
+    D2ClientConfig(const bool enable_updates,
+                   const isc::asiolink::IOAddress& server_ip,
+                   const size_t server_port,
+                   const dhcp_ddns::NameChangeProtocol& ncr_protocol,
+                   const dhcp_ddns::NameChangeFormat& ncr_format,
+                   const bool always_include_fqdn,
+                   const bool override_no_update,
+                   const bool override_client_update,
+                   const bool replace_client_name,
+                   const std::string& generated_prefix,
+                   const std::string& qualifying_suffix);
+
+    /// @brief Default constructor
+    /// The default constructor creates an instance that has updates disabled.
+    D2ClientConfig();
+
+    /// @brief Destructor
+    virtual ~D2ClientConfig();
+
+    /// @brief Return whether or not DHCP-DDNS updating is enabled.
+    bool getEnableUpdates() const {
+        return(enable_updates_);
+    }
+
+    /// @brief Return the IP address of b10-dhcp-ddns (IPv4 or IPv6).
+    const isc::asiolink::IOAddress& getServerIp() const {
+        return(server_ip_);
+    }
+
+    /// @brief Return the IP port of b10-dhcp-ddns.
+    size_t getServerPort() const {
+        return(server_port_);
+    }
+
+    /// @brief Return the socket protocol to use with b10-dhcp-ddns.
+    const dhcp_ddns::NameChangeProtocol& getNcrProtocol() const {
+         return(ncr_protocol_);
+    }
+
+    /// @brief Return the b10-dhcp-ddns request format.
+    const dhcp_ddns::NameChangeFormat& getNcrFormat() const {
+        return(ncr_format_);
+    }
+
+    /// @brief Return whether or not FQDN is always included in DHCP responses.
+    bool getAlwaysIncludeFqdn() const {
+        return(always_include_fqdn_);
+    }
+
+    /// @brief Return if updates are done even if clients request no updates.
+    bool getOverrideNoUpdate() const {
+        return(override_no_update_);
+    }
+
+    /// @brief Return if updates are done even when clients request delegation.
+    bool getOverrideClientUpdate() const {
+        return(override_client_update_);
+    }
+
+    /// @brief Return whether or not client's domain-name is always replaced.
+    bool getReplaceClientName() const {
+        return(replace_client_name_);
+    }
+
+    /// @brief Return the prefix to use when generating domain-names.
+    const std::string& getGeneratedPrefix() const {
+        return(generated_prefix_);
+    }
+
+    /// @brief Return the suffix to use to qualify partial domain-names.
+    const std::string& getQualifyingSuffix() const {
+        return(qualifying_suffix_);
+    }
+
+    /// @brief Compares two D2ClientConfigs for equality
+    bool operator == (const D2ClientConfig& other) const;
+
+    /// @brief Compares two D2ClientConfigs for inequality
+    bool operator != (const D2ClientConfig& other) const;
+
+    /// @brief Generates a string representation of the class contents.
+    std::string toText() const;
+
+    /// @brief Sets enable-updates flag to the given value.
+    ///
+    /// This is the only value that may be altered outside the constructor
+    /// as it may be desirable to toggle it off and on when dealing with
+    /// D2 IO errors.
+    ///
+    /// @param enable boolean value to assign to the enable-updates flag
+    void enableUpdates(bool enable);
+
+protected:
+    /// @brief Validates member values.
+    ///
+    /// Method is used by the constructor to validate member contents.
+    ///
+    /// @throw D2ClientError if given an invalid protocol or format.
+    virtual void validateContents();
+
+private:
+    /// @brief Indicates whether or not DHCP DDNS updating is enabled.
+    bool enable_updates_;
+
+    /// @brief IP address of the b10-dhcp-ddns server (IPv4 or IPv6).
+    isc::asiolink::IOAddress server_ip_;
+
+    /// @brief IP port of the b10-dhcp-ddns server.
+    size_t server_port_;
+
+    /// @brief The socket protocol to use with b10-dhcp-ddns.
+    /// Currently only UDP is supported.
+    dhcp_ddns::NameChangeProtocol ncr_protocol_;
+
+    /// @brief Format of the b10-dhcp-ddns requests.
+    /// Currently only JSON format is supported.
+    dhcp_ddns::NameChangeFormat ncr_format_;
+
+    /// @brief Should Kea always include the FQDN option in its response.
+    bool always_include_fqdn_;
+
+    /// @brief Should Kea perform updates, even if client requested no updates.
+    /// Overrides the client request for no updates via the N flag.
+    bool override_no_update_;
+
+    /// @brief Should Kea perform updates, even if client requested delegation.
+    bool override_client_update_;
+
+    /// @brief Should Kea replace the domain-name supplied by the client.
+    bool replace_client_name_;
+
+    /// @brief Prefix Kea should use when generating domain-names.
+    std::string generated_prefix_;
+
+    /// @brief Suffix Kea should use when to qualify partial domain-names.
+    std::string qualifying_suffix_;
+};
+
+std::ostream&
+operator<<(std::ostream& os, const D2ClientConfig& config);
+
+/// @brief Defines a pointer for D2ClientConfig instances.
+typedef boost::shared_ptr<D2ClientConfig> D2ClientConfigPtr;
+
+} // namespace isc
+} // namespace dhcp
+
+#endif

src/lib/dhcpsrv/d2_client.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -12,10 +12,13 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <dhcp/iface_mgr.h>
 #include <dhcp_ddns/ncr_udp.h>
-#include <dhcpsrv/d2_client.h>
+#include <dhcpsrv/d2_client_mgr.h>
 #include <dhcpsrv/dhcpsrv_log.h>
 
+#include <boost/bind.hpp>
+
 #include <string>
 
 using namespace std;
@@ -23,133 +26,26 @@ using namespace std;
 namespace isc {
 namespace dhcp {
 
-//***************************** D2ClientConfig ********************************
-
-D2ClientConfig::D2ClientConfig(const  bool enable_updates,
-                               const isc::asiolink::IOAddress& server_ip,
-                               const size_t server_port,
-                               const dhcp_ddns::
-                                     NameChangeProtocol& ncr_protocol,
-                               const dhcp_ddns::
-                                     NameChangeFormat& ncr_format,
-                               const bool always_include_fqdn,
-                               const bool override_no_update,
-                               const bool override_client_update,
-                               const bool replace_client_name,
-                               const std::string& generated_prefix,
-                               const std::string& qualifying_suffix)
-    : enable_updates_(enable_updates),
-    server_ip_(server_ip),
-    server_port_(server_port),
-    ncr_protocol_(ncr_protocol),
-    ncr_format_(ncr_format),
-    always_include_fqdn_(always_include_fqdn),
-    override_no_update_(override_no_update),
-    override_client_update_(override_client_update),
-    replace_client_name_(replace_client_name),
-    generated_prefix_(generated_prefix),
-    qualifying_suffix_(qualifying_suffix) {
-    validateContents();
-}
-
-D2ClientConfig::D2ClientConfig()
-    : enable_updates_(false),
-      server_ip_(isc::asiolink::IOAddress("0.0.0.0")),
-      server_port_(0),
-      ncr_protocol_(dhcp_ddns::NCR_UDP),
-      ncr_format_(dhcp_ddns::FMT_JSON),
-      always_include_fqdn_(false),
-      override_no_update_(false),
-      override_client_update_(false),
-      replace_client_name_(false),
-      generated_prefix_("myhost"),
-      qualifying_suffix_("example.com") {
-    validateContents();
-}
-
-D2ClientConfig::~D2ClientConfig(){};
-
-void
-D2ClientConfig::validateContents() {
-    if (ncr_format_ != dhcp_ddns::FMT_JSON) {
-        isc_throw(D2ClientError, "D2ClientConfig: NCR Format:"
-                    << dhcp_ddns::ncrFormatToString(ncr_format_)
-                    << " is not yet supported");
-    }
-
-    if (ncr_protocol_ != dhcp_ddns::NCR_UDP) {
-        isc_throw(D2ClientError, "D2ClientConfig: NCR Protocol:"
-                    << dhcp_ddns::ncrProtocolToString(ncr_protocol_)
-                    << " is not yet supported");
-    }
-
-    /// @todo perhaps more validation we should do yet?
-    /// Are there any invalid combinations of options we need to test against?
-}
-
-bool
-D2ClientConfig::operator == (const D2ClientConfig& other) const {
-    return ((enable_updates_ == other.enable_updates_) &&
-            (server_ip_ == other.server_ip_) &&
-            (server_port_ == other.server_port_) &&
-            (ncr_protocol_ == other.ncr_protocol_) &&
-            (ncr_format_ == other.ncr_format_) &&
-            (always_include_fqdn_ == other.always_include_fqdn_) &&
-            (override_no_update_ == other.override_no_update_) &&
-            (override_client_update_ == other.override_client_update_) &&
-            (replace_client_name_ == other.replace_client_name_) &&
-            (generated_prefix_ == other.generated_prefix_) &&
-            (qualifying_suffix_ == other.qualifying_suffix_));
-}
-
-bool
-D2ClientConfig::operator != (const D2ClientConfig& other) const {
-    return (!(*this == other));
-}
-
-std::string
-D2ClientConfig::toText() const {
-    std::ostringstream stream;
-
-    stream << "enable_updates: " << (enable_updates_ ? "yes" : "no");
-    if (enable_updates_) {
-        stream << ", server_ip: " << server_ip_.toText()
-               << ", server_port: " << server_port_
-               << ", ncr_protocol: " << ncr_protocol_
-               << ", ncr_format: " << ncr_format_
-               << ", always_include_fqdn: " << (always_include_fqdn_ ?
-                                                "yes" : "no")
-               << ", override_no_update: " << (override_no_update_ ?
-                                               "yes" : "no")
-               << ", override_client_update: " << (override_client_update_ ?
-                                                   "yes" : "no")
-               << ", replace_client_name: " << (replace_client_name_ ?
-                                                "yes" : "no")
-               << ", generated_prefix: [" << generated_prefix_ << "]"
-               << ", qualifying_suffix: [" << qualifying_suffix_ << "]";
-    }
-
-    return (stream.str());
-}
-
-std::ostream&
-operator<<(std::ostream& os, const D2ClientConfig& config) {
-    os << config.toText();
-    return (os);
-}
-
-
-//******************************** D2ClientMgr ********************************
-
-
 D2ClientMgr::D2ClientMgr() : d2_client_config_(new D2ClientConfig()),
-    name_change_sender_(), private_io_service_(), sender_io_service_(NULL) {
+    name_change_sender_(), private_io_service_(),
+    registered_select_fd_(dhcp_ddns::WatchSocket::INVALID_SOCKET) {
     // Default constructor initializes with a disabled configuration.
 }
 
 D2ClientMgr::~D2ClientMgr(){
-    if (name_change_sender_) {
-        stopSender();
+    stopSender();
+}
+
+void
+D2ClientMgr::suspendUpdates() {
+    if (ddnsEnabled()) {
+        /// @todo For now we will disable updates and stop sending.
+        /// This at least provides a means to shut it off if there are errors.
+        LOG_WARN(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_SUSPEND_UPDATES);
+        d2_client_config_->enableUpdates(false);
+        if (name_change_sender_) {
+            stopSender();
+        }
     }
 }
 
@@ -162,9 +58,11 @@ D2ClientMgr::setD2ClientConfig(D2ClientConfigPtr& new_config) {
 
     // Don't do anything unless configuration values are actually different.
     if (*d2_client_config_ != *new_config) {
+        // Make sure we stop sending first.
+        stopSender();
         if (!new_config->getEnableUpdates()) {
-            // Updating has been turned off, destroy current sender.
-            // Any queued requests are tossed.
+            // Updating has been turned off.
+            // Destroy current sender (any queued requests are tossed).
             name_change_sender_.reset();
         } else {
             dhcp_ddns::NameChangeSenderPtr new_sender;
@@ -201,7 +99,6 @@ D2ClientMgr::setD2ClientConfig(D2ClientConfigPtr& new_config) {
             /// then the queued contents might now be invalid.  There is
             /// no way to regenerate them if they are wrong.
             if (name_change_sender_) {
-                name_change_sender_->stopSending();
                 new_sender->assumeQueue(*name_change_sender_);
             }
 
@@ -311,15 +208,25 @@ D2ClientMgr::qualifyName(const std::string& partial_name) const {
 
 void
 D2ClientMgr::startSender(D2ClientErrorHandler error_handler) {
+    if (amSending()) {
+        return;
+    }
+
     // Create a our own service instance when we are not being multiplexed
     // into an external service..
     private_io_service_.reset(new asiolink::IOService());
     startSender(error_handler, *private_io_service_);
+    LOG_INFO(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_SENDER_STARTED)
+             .arg(d2_client_config_->toText());
 }
 
 void
 D2ClientMgr::startSender(D2ClientErrorHandler error_handler,
                          isc::asiolink::IOService& io_service) {
+    if (amSending()) {
+        return;
+    }
+
     if (!name_change_sender_)  {
         isc_throw(D2ClientError, "D2ClientMgr::startSender sender is null");
     }
@@ -331,14 +238,16 @@ D2ClientMgr::startSender(D2ClientErrorHandler error_handler,
     // Set the error handler.
     client_error_handler_ = error_handler;
 
-    // Remember the io service being used.
-    sender_io_service_ = &io_service;
-
     // Start the sender on the given service.
-    name_change_sender_->startSending(*sender_io_service_);
-
-    /// @todo need to register sender's select-fd with IfaceMgr once 3315 is
-    /// done.
+    name_change_sender_->startSending(io_service);
+
+    // Register sender's select-fd with IfaceMgr.
+    // We need to remember the fd that is registered so we can unregister later.
+    // IO error handling in the sender may alter its select-fd.
+    registered_select_fd_ = name_change_sender_->getSelectFd();
+    IfaceMgr::instance().addExternalSocket(registered_select_fd_,
+                                           boost::bind(&D2ClientMgr::runReadyIO,
+                                                       this));
 }
 
 bool
@@ -348,23 +257,51 @@ D2ClientMgr::amSending() const {
 
 void
 D2ClientMgr::stopSender() {
-    if (!name_change_sender_)  {
-        isc_throw(D2ClientError, "D2ClientMgr::stopSender sender is null");
+    /// Unregister sender's select-fd.
+    if (registered_select_fd_ != dhcp_ddns::WatchSocket::INVALID_SOCKET) {
+        IfaceMgr::instance().deleteExternalSocket(registered_select_fd_);
+        registered_select_fd_ = dhcp_ddns::WatchSocket::INVALID_SOCKET;
     }
 
-    /// @todo need to unregister sender's select-fd with IfaceMgr once 3315 is
-    /// done.
-
-    name_change_sender_->stopSending();
+    // If its not null, call stop.
+    if (amSending()) {
+        name_change_sender_->stopSending();
+        LOG_INFO(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_SENDER_STOPPED);
+    }
 }
 
 void
 D2ClientMgr::sendRequest(dhcp_ddns::NameChangeRequestPtr& ncr) {
-    if (!name_change_sender_) {
-        isc_throw(D2ClientError, "D2ClientMgr::sendRequest sender is null");
+    if (!amSending()) {
+        // This is programmatic error so bust them for it.
+        isc_throw(D2ClientError, "D2ClientMgr::sendRequest not in send mode");
+    }
+
+    try {
+        name_change_sender_->sendRequest(ncr);
+    } catch (const std::exception& ex) {
+        LOG_ERROR(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_NCR_REJECTED)
+                  .arg(ex.what()).arg((ncr ? ncr->toText() : " NULL "));
+        invokeClientErrorHandler(dhcp_ddns::NameChangeSender::ERROR, ncr);
     }
+}
 
-    name_change_sender_->sendRequest(ncr);
+void
+D2ClientMgr::invokeClientErrorHandler(const dhcp_ddns::NameChangeSender::
+                                      Result result,
+                                      dhcp_ddns::NameChangeRequestPtr& ncr) {
+    // Handler is mandatory to enter send mode but test it just to be safe.
+    if (!client_error_handler_) {
+        LOG_ERROR(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_HANDLER_NULL);
+    } else {
+        // Handler is not supposed to throw, but catch just in case.
+        try {
+            (client_error_handler_)(result, ncr);
+        } catch (const std::exception& ex) {
+            LOG_ERROR(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_ERROR_EXCEPTION)
+                      .arg(ex.what());
+        }
+    }
 }
 
 size_t
@@ -376,6 +313,16 @@ D2ClientMgr::getQueueSize() const {
     return(name_change_sender_->getQueueSize());
 }
 
+size_t
+D2ClientMgr::getQueueMaxSize() const {
+    if (!name_change_sender_) {
+        isc_throw(D2ClientError, "D2ClientMgr::getQueueMaxSize sender is null");
+    }
+
+    return(name_change_sender_->getQueueMaxSize());
+}
+
+
 
 const dhcp_ddns::NameChangeRequestPtr&
 D2ClientMgr::peekAt(const size_t index) const {
@@ -402,21 +349,8 @@ D2ClientMgr::operator()(const dhcp_ddns::NameChangeSender::Result result,
         LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
                   DHCPSRV_DHCP_DDNS_NCR_SENT).arg(ncr->toText());
     } else {
-        // Handler is mandatory but test it just to be safe.
-        /// @todo Until we have a better feel for how errors need to be
-        /// handled we farm it out to the application layer.
-        if (client_error_handler_) {
-            // Handler is not supposed to throw, but catch just in case.
-            try {
-                (client_error_handler_)(result, ncr);
-            } catch (const std::exception& ex) {
-                LOG_ERROR(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_ERROR_EXCEPTION)
-                          .arg(ex.what());
-            }
-        } else {
-            LOG_ERROR(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_HANDLER_NULL);
-        }
-   }
+        invokeClientErrorHandler(result, ncr);
+    }
 }
 
 int
@@ -431,17 +365,13 @@ D2ClientMgr::getSelectFd() {
 
 void
 D2ClientMgr::runReadyIO() {
-    if (!sender_io_service_) {
+    if (!name_change_sender_) {
         // This should never happen.
         isc_throw(D2ClientError, "D2ClientMgr::runReadyIO"
-                  " sender io service is null");
+                  " name_change_sender is null");
     }
 
-    // We shouldn't be here if IO isn't ready to execute.
-    // By running poll we're gauranteed not to hang.
-    /// @todo Trac# 3325 requests that asiolink::IOService provide a
-    /// wrapper for poll().
-    sender_io_service_->get_io_service().poll();
+    name_change_sender_->runReadyIO();
 }
 
 };  // namespace dhcp

src/lib/dhcpsrv/d2_client.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -12,18 +12,20 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#ifndef D2_CLIENT_H
-#define D2_CLIENT_H
+#ifndef D2_CLIENT_MGR_H
+#define D2_CLIENT_MGR_H
 
-/// @file d2_client.h Defines the D2ClientConfig and D2ClientMgr classes.
-/// This file defines the classes Kea uses to act as a client of the b10-
-/// dhcp-ddns module (aka D2).
+/// @file d2_client_mgr.h Defines the D2ClientMgr class.
+/// This file defines the class Kea uses to act as a client of the
+/// b10-dhcp-ddns module (aka D2).
 ///
 #include <asiolink/io_address.h>
 #include <dhcp_ddns/ncr_io.h>
+#include <dhcpsrv/d2_client_cfg.h>
 #include <exceptions/exceptions.h>
 
 #include <boost/shared_ptr.hpp>
+#include <boost/noncopyable.hpp>
 
 #include <stdint.h>
 #include <string>
@@ -32,185 +34,6 @@
 namespace isc {
 namespace dhcp {
 
-
-/// An exception that is thrown if an error occurs while configuring
-/// the D2 DHCP DDNS client.
-class D2ClientError : public isc::Exception {
-public:
-
-    /// @brief constructor
-    ///
-    /// @param file name of the file, where exception occurred
-    /// @param line line of the file, where exception occurred
-    /// @param what text description of the issue that caused exception
-    D2ClientError(const char* file, size_t line, const char* what)
-        : isc::Exception(file, line, what) {}
-};
-
-/// @brief Acts as a storage vault for D2 client configuration
-///
-/// A simple container class for storing and retrieving the configuration
-/// parameters associated with DHCP-DDNS and acting as a client of D2.
-/// Instances of this class may be constructed through configuration parsing.
-///
-class D2ClientConfig {
-public:
-    /// @brief Constructor
-    ///
-    /// @param enable_updates Enables DHCP-DDNS updates
-    /// @param server_ip IP address of the b10-dhcp-ddns server (IPv4 or IPv6)
-    /// @param server_port IP port of the b10-dhcp-ddns server
-    /// @param ncr_protocol Socket protocol to use with b10-dhcp-ddns
-    /// Currently only UDP is supported.
-    /// @param ncr_format Format of the b10-dhcp-ddns requests.
-    /// Currently only JSON format is supported.
-    /// @param always_include_fqdn Enables always including the FQDN option in
-    /// DHCP responses.
-    /// @param override_no_update Enables updates, even if clients request no
-    /// updates.
-    /// @param override_client_update Perform updates, even if client requested
-    /// delegation.
-    /// @param replace_client_name enables replacement of the domain-name
-    /// supplied by the client with a generated name.
-    /// @param generated_prefix Prefix to use when generating domain-names.
-    /// @param  qualifying_suffix Suffix to use to qualify partial domain-names.
-    ///
-    /// @throw D2ClientError if given an invalid protocol or format.
-    D2ClientConfig(const bool enable_updates,
-                   const isc::asiolink::IOAddress& server_ip,
-                   const size_t server_port,
-                   const dhcp_ddns::NameChangeProtocol& ncr_protocol,
-                   const dhcp_ddns::NameChangeFormat& ncr_format,
-                   const bool always_include_fqdn,
-                   const bool override_no_update,
-                   const bool override_client_update,
-                   const bool replace_client_name,
-                   const std::string& generated_prefix,
-                   const std::string& qualifying_suffix);
-
-    /// @brief Default constructor
-    /// The default constructor creates an instance that has updates disabled.
-    D2ClientConfig();
-
-    /// @brief Destructor
-    virtual ~D2ClientConfig();
-
-    /// @brief Return whether or not DHCP-DDNS updating is enabled.
-    bool getEnableUpdates() const {
-        return(enable_updates_);
-    }
-
-    /// @brief Return the IP address of b10-dhcp-ddns (IPv4 or IPv6).
-    const isc::asiolink::IOAddress& getServerIp() const {
-        return(server_ip_);
-    }
-
-    /// @brief Return the IP port of b10-dhcp-ddns.
-    size_t getServerPort() const {
-        return(server_port_);
-    }
-
-    /// @brief Return the socket protocol to use with b10-dhcp-ddns.
-    const dhcp_ddns::NameChangeProtocol& getNcrProtocol() const {
-         return(ncr_protocol_);
-    }
-
-    /// @brief Return the b10-dhcp-ddns request format.
-    const dhcp_ddns::NameChangeFormat& getNcrFormat() const {
-        return(ncr_format_);
-    }
-
-    /// @brief Return whether or not FQDN is always included in DHCP responses.
-    bool getAlwaysIncludeFqdn() const {
-        return(always_include_fqdn_);
-    }
-
-    /// @brief Return if updates are done even if clients request no updates.
-    bool getOverrideNoUpdate() const {
-        return(override_no_update_);
-    }
-
-    /// @brief Return if updates are done even when clients request delegation.
-    bool getOverrideClientUpdate() const {
-        return(override_client_update_);
-    }
-
-    /// @brief Return whether or not client's domain-name is always replaced.
-    bool getReplaceClientName() const {
-        return(replace_client_name_);
-    }
-
-    /// @brief Return the prefix to use when generating domain-names.
-    const std::string& getGeneratedPrefix() const {
-        return(generated_prefix_);
-    }
-
-    /// @brief Return the suffix to use to qualify partial domain-names.
-    const std::string& getQualifyingSuffix() const {
-        return(qualifying_suffix_);
-    }
-
-    /// @brief Compares two D2ClientConfigs for equality
-    bool operator == (const D2ClientConfig& other) const;
-
-    /// @brief Compares two D2ClientConfigs for inequality
-    bool operator != (const D2ClientConfig& other) const;
-
-    /// @brief Generates a string representation of the class contents.
-    std::string toText() const;
-
-protected:
-    /// @brief Validates member values.
-    ///
-    /// Method is used by the constructor to validate member contents.
-    ///
-    /// @throw D2ClientError if given an invalid protocol or format.
-    virtual void validateContents();
-
-private:
-    /// @brief Indicates whether or not DHCP DDNS updating is enabled.
-    bool enable_updates_;
-
-    /// @brief IP address of the b10-dhcp-ddns server (IPv4 or IPv6).
-    isc::asiolink::IOAddress server_ip_;
-
-    /// @brief IP port of the b10-dhcp-ddns server.
-    size_t server_port_;
-
-    /// @brief The socket protocol to use with b10-dhcp-ddns.
-    /// Currently only UDP is supported.
-    dhcp_ddns::NameChangeProtocol ncr_protocol_;
-
-    /// @brief Format of the b10-dhcp-ddns requests.
-    /// Currently only JSON format is supported.
-    dhcp_ddns::NameChangeFormat ncr_format_;
-
-    /// @brief Should Kea always include the FQDN option in its response.
-    bool always_include_fqdn_;
-
-    /// @brief Should Kea perform updates, even if client requested no updates.
-    /// Overrides the client request for no updates via the N flag.
-    bool override_no_update_;
-
-    /// @brief Should Kea perform updates, even if client requested delegation.
-    bool override_client_update_;
-
-    /// @brief Should Kea replace the domain-name supplied by the client.
-    bool replace_client_name_;
-
-    /// @brief Prefix Kea should use when generating domain-names.
-    std::string generated_prefix_;
-
-    /// @brief Suffix Kea should use when to qualify partial domain-names.
-    std::string qualifying_suffix_;
-};
-
-std::ostream&
-operator<<(std::ostream& os, const D2ClientConfig& config);
-
-/// @brief Defines a pointer for D2ClientConfig instances.
-typedef boost::shared_ptr<D2ClientConfig> D2ClientConfigPtr;
-
 /// @brief Defines the type for D2 IO error handler.
 /// This callback is invoked when a send to b10-dhcp-ddns completes with a
 /// failed status.  This provides the application layer (Kea) with a means to
@@ -219,7 +42,7 @@ typedef boost::shared_ptr<D2ClientConfig> D2ClientConfigPtr;
 /// @param result Result code of the send operation.
 /// @param ncr NameChangeRequest which failed to send.
 ///
-/// @note Handlers are expected not to throw. In the event a hanlder does
+/// @note Handlers are expected not to throw. In the event a handler does
 /// throw invoking code logs the exception and then swallows it.
 typedef
 boost::function<void(const dhcp_ddns::NameChangeSender::Result result,
@@ -240,13 +63,13 @@ boost::function<void(const dhcp_ddns::NameChangeSender::Result result,
 /// used by the NCR IPC with the select-driven IO used by Kea.  Senders expose
 /// a file descriptor, the "select-fd" that can monitored for read-readiness
 /// with the select() function (or variants).  D2ClientMgr provides a method,
-/// runReadyIO(), that will process all ready events on a sender's
-/// IOservice.  Track# 3315 is extending Kea's IfaceMgr to support the
-/// registration of multiple external sockets with callbacks that are then
-/// monitored with IO readiness via select().
-/// @todo D2ClientMgr will be modified to register the sender's select-fd and
-/// runReadyIO() with IfaceMgr when entering the send mode and will
-/// unregister when exiting send mode.
+/// runReadyIO(), that will instructs the sender to process the next ready
+/// ready IO handler on the sender's IOservice.  Track# 3315 extended
+/// Kea's IfaceMgr to support the registration of multiple external sockets
+/// with callbacks that are then monitored with IO readiness via select().
+/// D2ClientMgr registers the sender's select-fd and runReadyIO() with
+/// IfaceMgr when entering the send mode and unregisters it when exiting send
+/// mode.
 ///
 /// To place the manager in send mode, the calling layer must supply an error
 /// handler and optionally an IOService instance.  The error handler is invoked
@@ -258,7 +81,8 @@ boost::function<void(const dhcp_ddns::NameChangeSender::Result result,
 /// into the sender.  Using a private service isolates the sender's IO from
 /// any other services.
 ///
-class D2ClientMgr : public dhcp_ddns::NameChangeSender::RequestSendHandler {
+class D2ClientMgr : public dhcp_ddns::NameChangeSender::RequestSendHandler,
+                    boost::noncopyable {
 public:
     /// @brief Constructor
     ///
@@ -339,7 +163,7 @@ public:
     /// Templated wrapper around the analyzeFqdn() allowing that method to
     /// be used for either IPv4 or IPv6 processing.  This methods resets all
     /// of the flags in the response to zero and then sets the S,N, and O
-    /// flags.  Any other flags are the responsiblity of the invoking layer.
+    /// flags.  Any other flags are the responsibility of the invoking layer.
     ///
     /// @param fqdn FQDN option from which to read client (inbound) flags
     /// @param fqdn_resp FQDN option to update with the server (outbound) flags
@@ -430,17 +254,36 @@ public:
     /// @brief Send the given NameChangeRequests to b10-dhcp-ddns
     ///
     /// Passes NameChangeRequests to the NCR sender for transmission to
-    /// b10-dhcp-ddns.
+    /// b10-dhcp-ddns. If the sender rejects the message, the client's error
+    /// handler will be invoked.  The most likely cause for rejection is
+    /// the senders' queue has reached maximum capacity.
     ///
     /// @param ncr NameChangeRequest to send
     ///
-    /// @throw D2ClientError if sender instance is null. Underlying layer
-    /// may throw NCRSenderExceptions exceptions.
+    /// @throw D2ClientError if sender instance is null or not in send
+    /// mode.  Either of these represents a programmatic error.
     void sendRequest(dhcp_ddns::NameChangeRequestPtr& ncr);
 
+    /// @brief Calls the client's error handler.
+    ///
+    /// Calls the error handler method set by startSender() when an
+    /// error occurs attempting to send a method.  If the error handler
+    /// throws an exception it will be caught and logged.
+    ///
+    /// @param result contains that send outcome status.
+    /// @param ncr is a pointer to the NameChangeRequest that was attempted.
+    ///
+    /// This method is exception safe.
+    void invokeClientErrorHandler(const dhcp_ddns::NameChangeSender::
+                                  Result result,
+                                  dhcp_ddns::NameChangeRequestPtr& ncr);
+
     /// @brief Returns the number of NCRs queued for transmission.
     size_t getQueueSize() const;
 
+    /// @brief Returns the maximum number of NCRs allowed in the queue.
+    size_t getQueueMaxSize() const;
+
     /// @brief Returns the nth NCR queued for transmission.
     ///
     /// Note that the entry is not removed from the queue.
@@ -462,14 +305,28 @@ public:
 
     /// @brief Processes sender IO events
     ///
-    /// Runs all handlers ready for execution on the sender's IO service.
+    /// Serves as callback registered for the sender's select-fd with IfaceMgr.
+    /// It instructs the sender to execute the next ready IO handler.
+    /// It provides an instance method that can be bound via boost::bind, as
+    /// NameChangeSender is abstract.
     void runReadyIO();
 
+    /// @brief Suspends sending requests.
+    ///
+    /// This method is intended to be used when IO errors occur.  It toggles
+    /// the enable-updates configuration flag to off, and takes the sender
+    /// out of send mode.  Messages in the sender's queue will remain in the
+    /// queue.
+    /// @todo This logic may change in NameChangeSender is altered allow
+    /// queuing while stopped.  Currently when a sender is not in send mode
+    /// it will not accept additional messages.
+    void suspendUpdates();
+
 protected:
     /// @brief Function operator implementing the NCR sender callback.
     ///
     /// This method is invoked each time the NameChangeSender completes
-    /// an asychronous send.
+    /// an asynchronous send.
     ///
     /// @param result contains that send outcome status.
     /// @param ncr is a pointer to the NameChangeRequest that was
@@ -491,6 +348,14 @@ protected:
     /// mode.
     int getSelectFd();
 
+    /// @brief Fetches the select-fd that is currently registered.
+    ///
+    /// @return The currently registered select-fd or
+    /// dhcp_ddns::WatchSocket::INVALID_SOCKET.
+    ///
+    /// @note This is only exposed for testing purposes.
+    int getRegisteredSelectFd();
+
 private:
     /// @brief Container class for DHCP-DDNS configuration parameters.
     D2ClientConfigPtr d2_client_config_;
@@ -506,12 +371,8 @@ private:
     /// completes with a failed status.
     D2ClientErrorHandler client_error_handler_;
 
-    /// @brief Pointer to the IOService currently being used by the sender.
-    /// @note We need to remember the io_service given to the sender however
-    /// we may have received only a referenece to it from the calling layer.
-    /// Use a raw pointer to store it.  This value should never be exposed
-    /// and is only valid while in send mode.
-    asiolink::IOService* sender_io_service_;
+    /// @brief Remembers the select-fd registered with IfaceMgr.
+    int registered_select_fd_;
 };
 
 template <class T>

src/lib/dhcpsrv/dhcp_parsers.h

@@ -18,7 +18,7 @@
 #include <asiolink/io_address.h>
 #include <cc/data.h>
 #include <dhcp/option_definition.h>
-#include <dhcpsrv/d2_client.h>
+#include <dhcpsrv/d2_client_cfg.h>
 #include <dhcpsrv/dhcp_config_parser.h>
 #include <dhcpsrv/option_space_container.h>
 #include <dhcpsrv/subnet.h>

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -138,11 +138,29 @@ the database access parameters are changed: in the latter case, the
 server closes the currently open database, and opens a database using
 the new parameters.
 
-% DHCPSRV_HOOK_LEASE4_SELECT_SKIP Lease4 creation was skipped, because of callout skip flag.
-This debug message is printed when a callout installed on lease4_select
-hook point sets the skip flag. It means that the server was told that
-no lease4 should be assigned. The server will not put that lease in its
-database and the client will get a NAK packet.
+% DHCPSRV_DHCP_DDNS_ERROR_EXCEPTION error handler for DHCP_DDNS IO generated an expected exception: %1
+This is an error message that occurs when an attempt to send a request to
+b10-dhcp-ddns fails there registered error handler threw an uncaught exception.
+This is a programmatic error which should not occur. By convention, the error
+handler should not propagate exceptions. Please report this error.
+
+% DHCPSRV_DHCP_DDNS_HANDLER_NULL error handler for DHCP_DDNS IO is not set.
+This is an error message that occurs when an attempt to send a request to
+b10-dhcp-ddns fails and there is no registered error handler.  This is a
+programmatic error which should never occur and should be reported.
+
+% DHCPSRV_DHCP_DDNS_NCR_SENT NameChangeRequest sent to b10-dhcp-ddns: %1
+A debug message issued when a NameChangeRequest has been successfully sent to
+b10-dhcp-ddns.
+
+% DHCPSRV_DHCP_DDNS_SENDER_STARTED NameChangeRequest sender has been started: %1
+A informational message issued when a communications with b10-dhcp-ddns has
+been successfully started.
+
+% DHCPSRV_DHCP_DDNS_SENDER_STOPPED NameChangeRequest sender has been stopped.
+A informational message issued when a communications with b10-dhcp-ddns has
+been stopped. This normally occurs during reconfiguration and as part of normal
+shutdown. It may occur if b10-dhcp-ddns communications breakdown.
 
 % DHCPSRV_HOOK_LEASE4_RENEW_SKIP DHCPv4 lease was not renewed because a callout set the skip flag.
 This debug message is printed when a callout installed on lease4_renew
@@ -150,6 +168,12 @@ hook point set the skip flag. For this particular hook point, the setting
 of the flag by a callout instructs the server to not renew a lease. The
 server will use existing lease as it is, without extending its lifetime.
 
+% DHCPSRV_HOOK_LEASE4_SELECT_SKIP Lease4 creation was skipped, because of callout skip flag.
+This debug message is printed when a callout installed on lease4_select
+hook point sets the skip flag. It means that the server was told that
+no lease4 should be assigned. The server will not put that lease in its
+database and the client will get a NAK packet.
+
 % DHCPSRV_HOOK_LEASE6_SELECT_SKIP Lease6 (non-temporary) creation was skipped, because of callout skip flag.
 This debug message is printed when a callout installed on lease6_select
 hook point sets the skip flag. It means that the server was told that
@@ -198,6 +222,11 @@ A debug message issued when the server is attempting to obtain a set of
 IPv4 leases from the memory file database for a client with the specified
 client identification.
 
+% DHCPSRV_MEMFILE_GET_CLIENTID_HWADDR_SUBID obtaining IPv4 lease for client ID %1, hardware address %2 and subnet ID %3
+A debug message issued when the server is attempting to obtain an IPv4
+lease from the memory file database for a client with the specified
+client ID, hardware address and subnet ID.
+
 % DHCPSRV_MEMFILE_GET_HWADDR obtaining IPv4 leases for hardware address %1
 A debug message issued when the server is attempting to obtain a set of
 IPv4 leases from the memory file database for a client with the specified
@@ -213,11 +242,6 @@ A debug message issued when the server is attempting to obtain an IPv6
 lease from the memory file database for a client with the specified IAID
 (Identity Association ID), Subnet ID and DUID (DHCP Unique Identifier).
 
-% DHCPSRV_MEMFILE_GET_CLIENTID_HWADDR_SUBID obtaining IPv4 lease for client ID %1, hardware address %2 and subnet ID %3
-A debug message issued when the server is attempting to obtain an IPv4
-lease from the memory file database for a client with the specified
-client ID, hardware address and subnet ID.
-
 % DHCPSRV_MEMFILE_GET_SUBID_CLIENTID obtaining IPv4 lease for subnet ID %1 and client ID %2
 A debug message issued when the server is attempting to obtain an IPv4
 lease from the memory file database for a client with the specified
@@ -345,17 +369,15 @@ indicate an error in the source code, please submit a bug report.
 The database access string specified a database type (given in the
 message) that is unknown to the software.  This is a configuration error.
 
-% DHCPSRV_DHCP_DDNS_HANDLER_NULL error handler for DHCP_DDNS IO is not set.
-This is an error message that occurs when an attempt to send a request to
-b10-dhcp-ddns fails and there is no registered error handler.  This is a
-programmatic error which should never occur and should be reported.
-
-% DHCPSRV_DHCP_DDNS_ERROR_EXCEPTION error handler for DHCP_DDNS IO generated an expected exception: %1
-This is an error message that occurs when an attempt to send a request to
-b10-dhcp-ddns fails there registered error handler threw an uncaught exception.
-This is a programmatic error which should not occur. By convention, the error
-handler should not propagate exceptions. Please report this error.
-
-% DHCPSRV_DHCP_DDNS_NCR_SENT NameChangeRequest sent to b10-dhcp-ddns: %1
-A debug message issued when a NameChangeRequest has been successfully sent to
-b10-dhcp-ddns.
+% DHCPSRV_DHCP_DDNS_SUSPEND_UPDATES DHCP_DDNS updates are being suspended.
+This is a warning message indicating the DHCP_DDNS updates have been turned
+off.  This should only occur if IO errors communicating with b10-dhcp-ddns
+have been experienced.  Any such errors should have preceding entries in the
+log with details.  No further attempts to communicate with b10-dhcp-ddns will
+be made without intervention.
+
+% DHCPSRV_DHCP_DDNS_NCR_REJECTED NameChangeRequest rejected by the sender: %1, ncr: %2
+This is an error message indicating that NameChangeSender used to deliver DDNS
+update requests to b10-dhcp-ddns rejected the request.  This most likely cause
+is the sender's queue has reached maximum capacity.  This would imply that
+requests are being generated faster than they can be delivered.

src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -903,7 +903,7 @@ TEST_F(CfgMgrTest, d2ClientConfig) {
     // After CfgMgr construction, D2ClientMgr member should be initialized
     // with a D2 configuration that is disabled.
     // Verify we can Fetch the mgr.
-    D2ClientMgr d2_mgr = CfgMgr::instance().getD2ClientMgr();
+    D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr();
     EXPECT_FALSE(d2_mgr.ddnsEnabled());
 
     // Make sure the convenience method fetches the config correctly.

src/lib/dhcpsrv/tests/d2_client_unittest.cc

@@ -15,7 +15,7 @@
 #include <config.h>
 #include <dhcp/option4_client_fqdn.h>
 #include <dhcp/option6_client_fqdn.h>
-#include <dhcpsrv/d2_client.h>
+#include <dhcpsrv/d2_client_mgr.h>
 #include <exceptions/exceptions.h>
 
 #include <gtest/gtest.h>
@@ -36,6 +36,12 @@ TEST(D2ClientConfigTest, constructorsAndAccessors) {
     ASSERT_NO_THROW(d2_client_config.reset(new D2ClientConfig()));
     EXPECT_FALSE(d2_client_config->getEnableUpdates());
 
+    // Verify the enable-updates can be toggled.
+    d2_client_config->enableUpdates(true);
+    EXPECT_TRUE(d2_client_config->getEnableUpdates());
+    d2_client_config->enableUpdates(false);
+    EXPECT_FALSE(d2_client_config->getEnableUpdates());
+
     d2_client_config.reset();
 
     bool enable_updates = true;

src/lib/dhcpsrv/tests/d2_udp_unittest.cc

@@ -19,7 +19,8 @@
 #include <asio.hpp>
 #include <asiolink/io_service.h>
 #include <config.h>
-#include <dhcpsrv/d2_client.h>
+#include <dhcp/iface_mgr.h>
+#include <dhcpsrv/d2_client_mgr.h>
 #include <exceptions/exceptions.h>
 
 #include <boost/function.hpp>
@@ -231,7 +232,7 @@ TEST_F(D2ClientMgrTest, udpSenderQueing) {
 
     // Trying to send a NCR when not in send mode should fail.
     dhcp_ddns::NameChangeRequestPtr ncr = buildTestNcr();
-    EXPECT_THROW(sendRequest(ncr), dhcp_ddns::NcrSenderError);
+    EXPECT_THROW(sendRequest(ncr), D2ClientError);
 
     // Place sender in send mode.
     ASSERT_NO_THROW(startSender(getErrorHandler()));
@@ -330,55 +331,147 @@ TEST_F(D2ClientMgrTest, udpSendExternalIOService) {
 /// when send errors occur.
 TEST_F(D2ClientMgrTest, udpSendErrorHandler) {
     // Enable DDNS with server at 127.0.0.1/prot 53001 via UDP.
-    enableDdns("127.0.0.1", 530001, dhcp_ddns::NCR_UDP);
-
-    // Trying to fetch the select-fd when not sending should fail.
-    ASSERT_THROW(getSelectFd(), D2ClientError);
-
     // Place sender in send mode.
+    enableDdns("127.0.0.1", 530001, dhcp_ddns::NCR_UDP);
     ASSERT_NO_THROW(startSender(getErrorHandler()));
 
-    // select_fd should evaluate to NOT ready to read.
-    selectCheck(false);
-
     // Simulate a failed response in the send call back. This should
     // cause the error handler to get invoked.
     simulate_send_failure_ = true;
 
+    // Verify error count is zero.
     ASSERT_EQ(0, error_handler_count_);
 
     // Send a test request.
     dhcp_ddns::NameChangeRequestPtr ncr = buildTestNcr();
     ASSERT_NO_THROW(sendRequest(ncr));
 
-    // select_fd should evaluate to ready to read.
-    selectCheck(true);
+    // Call the ready handler. This should complete the message with an error.
+    ASSERT_NO_THROW(runReadyIO());
 
-    // Call service handler.
-    runReadyIO();
+    // If we executed error handler properly, the error count should one.
+    ASSERT_EQ(1, error_handler_count_);
+}
 
-    // select_fd should evaluate to not ready to read.
-    selectCheck(false);
 
-    ASSERT_EQ(1, error_handler_count_);
+/// @brief Checks that client error handler exceptions are handled gracefully.
+TEST_F(D2ClientMgrTest, udpSendErrorHandlerThrow) {
+    // Enable DDNS with server at 127.0.0.1/prot 53001 via UDP.
+    // Place sender in send mode.
+    enableDdns("127.0.0.1", 530001, dhcp_ddns::NCR_UDP);
+    ASSERT_NO_THROW(startSender(getErrorHandler()));
 
-    // Simulate a failed response in the send call back. This should
-    // cause the error handler to get invoked.
+    // Simulate a failed response in the send call back and
+    // force a throw in the error handler.
     simulate_send_failure_ = true;
     error_handler_throw_ = true;
 
+    // Verify error count is zero.
+    ASSERT_EQ(0, error_handler_count_);
+
     // Send a test request.
-    ncr = buildTestNcr();
+    dhcp_ddns::NameChangeRequestPtr ncr = buildTestNcr();
     ASSERT_NO_THROW(sendRequest(ncr));
 
-    // Call the io service handler.
-    runReadyIO();
+    // Call the ready handler. This should complete the message with an error.
+    // The handler should throw but the exception should not escape.
+    ASSERT_NO_THROW(runReadyIO());
 
-    // Simulation flag should be false.
+    // If throw flag is false, then we were in the error handler should
+    // have thrown.
     ASSERT_FALSE(error_handler_throw_);
 
-    // Count should still be 1.
-    ASSERT_EQ(1, error_handler_count_);
+    // If error count is still zero, then we did throw.
+    ASSERT_EQ(0, error_handler_count_);
+}
+
+/// @brief Tests that D2ClientMgr registers and unregisters with IfaceMgr.
+TEST_F(D2ClientMgrTest, ifaceRegister) {
+    // Enable DDNS with server at 127.0.0.1/prot 53001 via UDP.
+    enableDdns("127.0.0.1", 530001, dhcp_ddns::NCR_UDP);
+
+    // Place sender in send mode.
+    ASSERT_NO_THROW(startSender(getErrorHandler()));
+
+    // Queue three messages.
+    for (int i = 0; i < 3; ++i) {
+        dhcp_ddns::NameChangeRequestPtr ncr = buildTestNcr();
+        ASSERT_NO_THROW(sendRequest(ncr));
+    }
+
+    // Make sure queue count is correct.
+    EXPECT_EQ(3, getQueueSize());
+
+    // select_fd should evaluate to ready to read.
+    selectCheck(true);
+
+    // Calling receive should complete the first message and start the second.
+    IfaceMgr::instance().receive4(0, 0);
+
+    // Verify the callback hander was invoked, no errors counted.
+    EXPECT_EQ(2, getQueueSize());
+    ASSERT_EQ(1, callback_count_);
+    ASSERT_EQ(0, error_handler_count_);
+
+    // Stop the sender.  This should complete the second message but leave
+    // the third in the queue.
+    ASSERT_NO_THROW(stopSender());
+    EXPECT_EQ(1, getQueueSize());
+    ASSERT_EQ(2, callback_count_);
+    ASSERT_EQ(0, error_handler_count_);
+
+    // Calling receive again should have no affect.
+    IfaceMgr::instance().receive4(0, 0);
+    EXPECT_EQ(1, getQueueSize());
+    ASSERT_EQ(2, callback_count_);
+    ASSERT_EQ(0, error_handler_count_);
+}
+
+/// @brief Checks that D2ClientMgr suspendUpdates works properly.
+TEST_F(D2ClientMgrTest, udpSuspendUpdates) {
+    // Enable DDNS with server at 127.0.0.1/prot 53001 via UDP.
+    // Place sender in send mode.
+    enableDdns("127.0.0.1", 530001, dhcp_ddns::NCR_UDP);
+    ASSERT_NO_THROW(startSender(getErrorHandler()));
+
+    // Send a test request.
+    for (int i = 0; i < 3; ++i) {
+        dhcp_ddns::NameChangeRequestPtr ncr = buildTestNcr();
+        ASSERT_NO_THROW(sendRequest(ncr));
+    }
+    ASSERT_EQ(3, getQueueSize());
+
+    // Call the ready handler. This should complete the first message
+    // and initiate sending the second message.
+    ASSERT_NO_THROW(runReadyIO());
+
+    // Queue count should have gone down by 1.
+    ASSERT_EQ(2, getQueueSize());
+
+    // Suspend updates. This should disable updates and stop the sender.
+    ASSERT_NO_THROW(suspendUpdates());
+
+    EXPECT_FALSE(ddnsEnabled());
+    EXPECT_FALSE(amSending());
+
+    // Stopping the sender should have completed the second message's
+    // in-progess send, so queue size should be 1.
+    ASSERT_EQ(1, getQueueSize());
+}
+
+/// @brief Tests that invokeErrorHandler does not fail if there is no handler.
+TEST_F(D2ClientMgrTest, missingErrorHandler) {
+    // Ensure we aren't in send mode.
+    ASSERT_FALSE(ddnsEnabled());
+    ASSERT_FALSE(amSending());
+
+    // There is no error handler at this point, so invoking should not throw.
+    dhcp_ddns::NameChangeRequestPtr ncr;
+    ASSERT_NO_THROW(invokeClientErrorHandler(dhcp_ddns::NameChangeSender::ERROR,
+                                             ncr));
+
+    // Verify we didn't invoke the error handler, error count is zero.
+    ASSERT_EQ(0, error_handler_count_);
 }
 
 } // end of anonymous namespace