Browse Source

[3089] D2UpdateMgr now uses NameChangeTransaction derivations

Now that NameAddTransaction and NameRemoveTransction classes
have been implemented, D2UpdateMgr has been updated to use
them.  It now creates instances of NameAddTransaction and
NameRemoveTransaction based on the change type specified in
received NameChangeRequests as designed.
Thomas Markwalder 11 years ago
parent
commit
50c35b270c

+ 19 - 3
src/bin/d2/d2_config.cc

@@ -21,6 +21,8 @@
 #include <boost/foreach.hpp>
 #include <boost/lexical_cast.hpp>
 
+#include <string>
+
 namespace isc {
 namespace d2 {
 
@@ -49,6 +51,20 @@ DnsServerInfo::DnsServerInfo(const std::string& hostname,
 DnsServerInfo::~DnsServerInfo() {
 }
 
+std::string
+DnsServerInfo::toText() const {
+    std::ostringstream stream;
+    stream << (getIpAddress().toText()) << " port:" << getPort();
+    return (stream.str());
+}
+
+
+std::ostream&
+operator<<(std::ostream& os, const DnsServerInfo& server) {
+    os << server.toText();
+    return (os);
+}
+
 // *********************** DdnsDomain  *************************
 
 DdnsDomain::DdnsDomain(const std::string& name, const std::string& key_name,
@@ -253,7 +269,7 @@ TSIGKeyInfoParser::commit() {
 
 TSIGKeyInfoListParser::TSIGKeyInfoListParser(const std::string& list_name,
                                        TSIGKeyInfoMapPtr keys)
-    :list_name_(list_name), keys_(keys), local_keys_(new TSIGKeyInfoMap()), 
+    :list_name_(list_name), keys_(keys), local_keys_(new TSIGKeyInfoMap()),
      parsers_() {
     if (!keys_) {
         isc_throw(D2CfgError, "TSIGKeyInfoListParser ctor:"
@@ -291,9 +307,9 @@ TSIGKeyInfoListParser::commit() {
     BOOST_FOREACH(isc::dhcp::ParserPtr parser, parsers_) {
         parser->commit();
     }
-  
+
     // Now that we know we have a valid list, commit that list to the
-    // area given to us during construction (i.e. to the d2 context).   
+    // area given to us during construction (i.e. to the d2 context).
     *keys_ = *local_keys_;
 }
 

+ 6 - 0
src/bin/d2/d2_config.h

@@ -288,6 +288,9 @@ public:
         enabled_ = false;
     }
 
+    /// @brief Returns a text representation for the server.
+    std::string toText() const;
+
 
 private:
     /// @brief The resolvable name of the server. If not blank, then the
@@ -306,6 +309,9 @@ private:
     bool enabled_;
 };
 
+std::ostream&
+operator<<(std::ostream& os, const DnsServerInfo& server);
+
 /// @brief Defines a pointer for DnsServerInfo instances.
 typedef boost::shared_ptr<DnsServerInfo> DnsServerInfoPtr;
 

+ 7 - 0
src/bin/d2/d2_messages.mes

@@ -438,3 +438,10 @@ This is an error message issued after DHCP_DDNS attempts to submit DNS mapping
 entry removals have failed.  The precise reason for the failure should be
 documented in preceding log entries.
 
+% DHCP_DDNS_UPDATE_REQUEST_SENT for transaction key: %1  to server : %2
+This is a debug message issued when DHCP_DDNS sends a DNS request to a DNS
+server.
+
+% DHCP_DDNS_UPDATE_RESPONSE_RECEIVED for transaction key: %1  to server : %2 status: %3
+This is a debug message issued when DHCP_DDNS receives sends a DNS update
+response from a DNS server.

+ 27 - 12
src/bin/d2/d2_update_mgr.cc

@@ -13,6 +13,8 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <d2/d2_update_mgr.h>
+#include <d2/nc_add.h>
+#include <d2/nc_remove.h>
 
 #include <sstream>
 #include <iostream>
@@ -85,16 +87,12 @@ D2UpdateMgr::checkFinishedTransactions() {
     TransactionList::iterator it = transaction_list_.begin();
     while (it != transaction_list_.end()) {
         NameChangeTransactionPtr trans = (*it).second;
-        switch (trans->getNcrStatus())  {
-        case dhcp_ddns::ST_COMPLETED:
+        if (trans->isModelDone()) {
+            // @todo  Addtional actions based on NCR status could be
+            // performed here.
             transaction_list_.erase(it++);
-            break;
-        case dhcp_ddns::ST_FAILED:
-            transaction_list_.erase(it++);
-            break;
-        default:
+        } else {
             ++it;
-            break;
         }
     }
 }
@@ -163,12 +161,24 @@ D2UpdateMgr::makeTransaction(dhcp_ddns::NameChangeRequestPtr& next_ncr) {
     }
 
     // We matched to the required servers, so construct the transaction.
-    NameChangeTransactionPtr trans(new NameChangeTransaction(io_service_,
-                                                             next_ncr,
-                                                             forward_domain,
-                                                             reverse_domain));
+    // @todo If multi-threading is implemented, one would pass in an
+    // empty IOServicePtr, rather than our instance value.  This would cause
+    // the transaction to instantiate its own, separate IOService to handle
+    // the transaction's IO.
+    NameChangeTransactionPtr trans;
+    if (next_ncr->getChangeType() == dhcp_ddns::CHG_ADD) {
+        trans.reset(new NameAddTransaction(io_service_, next_ncr,
+                                           forward_domain, reverse_domain));
+    } else {
+        trans.reset(new NameRemoveTransaction(io_service_, next_ncr,
+                                           forward_domain, reverse_domain));
+    }
+
     // Add the new transaction to the list.
     transaction_list_[key] = trans;
+
+    // Start it.
+    trans->startTransaction();
 }
 
 TransactionList::iterator
@@ -190,6 +200,11 @@ D2UpdateMgr::removeTransaction(const TransactionKey& key) {
 }
 
 TransactionList::iterator
+D2UpdateMgr::transactionListBegin() {
+    return (transaction_list_.begin());
+}
+
+TransactionList::iterator
 D2UpdateMgr::transactionListEnd() {
     return (transaction_list_.end());
 }

+ 10 - 0
src/bin/d2/d2_update_mgr.h

@@ -162,6 +162,13 @@ protected:
     void makeTransaction(isc::dhcp_ddns::NameChangeRequestPtr& ncr);
 
 public:
+    /// @brief Gets the UpdateMgr's IOService.
+    ///
+    /// @return returns a reference to the IOService
+    const IOServicePtr& getIOService() {
+        return (io_service_);
+    }
+
     /// @brief Returns the maximum number of concurrent transactions.
     size_t getMaxTransactions() const {
         return (max_transactions_);
@@ -187,6 +194,9 @@ public:
     /// @brief Returns the transaction list end position.
     TransactionList::iterator transactionListEnd();
 
+    /// @brief Returns the transaction list beg position.
+    TransactionList::iterator transactionListBegin();
+
     /// @brief Convenience method that checks transaction list for the given key
     ///
     /// @param key the transaction key value for which to search.

+ 6 - 7
src/bin/d2/dns_client.cc

@@ -45,7 +45,7 @@ public:
     // A buffer holding response from a DNS.
     util::OutputBufferPtr in_buf_;
     // A caller-supplied object holding a parsed response from DNS.
-    D2UpdateMessagePtr response_;
+    D2UpdateMessagePtr& response_;
     // A caller-supplied external callback which is invoked when DNS message
     // exchange is complete or interrupted.
     DNSClient::Callback* callback_;
@@ -104,12 +104,6 @@ DNSClientImpl::DNSClientImpl(D2UpdateMessagePtr& response_placeholder,
                       << proto_ << "' specified for DNS Updates");
         }
     }
-
-    if (!response_) {
-        isc_throw(BadValue, "a pointer to an object to encapsulate the DNS"
-                  " server must be provided; found NULL value");
-
-    }
 }
 
 DNSClientImpl::~DNSClientImpl() {
@@ -122,6 +116,11 @@ DNSClientImpl::operator()(asiodns::IOFetch::Result result) {
     DNSClient::Status status = getStatus(result);
     if (status == DNSClient::SUCCESS) {
         InputBuffer response_buf(in_buf_->getData(), in_buf_->getLength());
+        // Allocate a new response message. (Note that Message::fromWire
+        // may only be run once per message, so we need to start fresh
+        // each time.)
+        response_.reset(new D2UpdateMessage(D2UpdateMessage::INBOUND));
+
         // Server's response may be corrupted. In such case, fromWire will
         // throw an exception. We want to catch this exception to return
         // appropriate status code to the caller and log this event.

+ 12 - 12
src/bin/d2/nc_add.cc

@@ -230,7 +230,7 @@ NameAddTransaction::addingFwdAddrsHandler() {
                 // If we get not authorized should we try the next server in
                 // the list? @todo  This needs some discussion perhaps.
                 LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_ADD_REJECTED)
-                          .arg(getCurrentServer()->getIpAddress())
+                          .arg(getCurrentServer()->toText())
                           .arg(getNcr()->getFqdn())
                           .arg(rcode.getCode());
                 transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
@@ -247,7 +247,7 @@ NameAddTransaction::addingFwdAddrsHandler() {
             // is not entirely clear if this is accurate.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_ADD_IO_ERROR)
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             retryTransition(SELECTING_FWD_SERVER_ST);
             break;
@@ -256,7 +256,7 @@ NameAddTransaction::addingFwdAddrsHandler() {
             // A response was received but was corrupt. Retry it like an IO
             // error.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_ADD_RESP_CORRUPT)
-                      .arg(getCurrentServer()->getIpAddress())
+                      .arg(getCurrentServer()->toText())
                       .arg(getNcr()->getFqdn());
 
             retryTransition(SELECTING_FWD_SERVER_ST);
@@ -268,7 +268,7 @@ NameAddTransaction::addingFwdAddrsHandler() {
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_ADD_BAD_DNSCLIENT_STATUS)
                       .arg(getDnsUpdateStatus())
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
             break;
@@ -342,7 +342,7 @@ NameAddTransaction::replacingFwdAddrsHandler() {
                 // If we get not authorized should try the next server in
                 // the list? @todo  This needs some discussion perhaps.
                 LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REPLACE_REJECTED)
-                          .arg(getCurrentServer()->getIpAddress())
+                          .arg(getCurrentServer()->toText())
                           .arg(getNcr()->getFqdn())
                           .arg(rcode.getCode());
                 transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
@@ -359,7 +359,7 @@ NameAddTransaction::replacingFwdAddrsHandler() {
             // is not entirely clear if this is accurate.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REPLACE_IO_ERROR)
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             // If we are out of retries on this server, we go back and start
             // all over on a new server.
@@ -370,7 +370,7 @@ NameAddTransaction::replacingFwdAddrsHandler() {
             // A response was received but was corrupt. Retry it like an IO
             // error.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REPLACE_RESP_CORRUPT)
-                      .arg(getCurrentServer()->getIpAddress())
+                      .arg(getCurrentServer()->toText())
                       .arg(getNcr()->getFqdn());
 
             // If we are out of retries on this server, we go back and start
@@ -385,7 +385,7 @@ NameAddTransaction::replacingFwdAddrsHandler() {
                       DHCP_DDNS_FORWARD_REPLACE_BAD_DNSCLIENT_STATUS)
                       .arg(getDnsUpdateStatus())
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
             break;
@@ -476,7 +476,7 @@ NameAddTransaction::replacingRevPtrsHandler() {
                 // If we get not authorized should try the next server in
                 // the list? @todo  This needs some discussion perhaps.
                 LOG_ERROR(dctl_logger, DHCP_DDNS_REVERSE_REPLACE_REJECTED)
-                          .arg(getCurrentServer()->getIpAddress())
+                          .arg(getCurrentServer()->toText())
                           .arg(getNcr()->getFqdn())
                           .arg(rcode.getCode());
                 transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
@@ -493,7 +493,7 @@ NameAddTransaction::replacingRevPtrsHandler() {
             // is not entirely clear if this is accurate.
             LOG_ERROR(dctl_logger, DHCP_DDNS_REVERSE_REPLACE_IO_ERROR)
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             // If we are out of retries on this server, we go back and start
             // all over on a new server.
@@ -504,7 +504,7 @@ NameAddTransaction::replacingRevPtrsHandler() {
             // A response was received but was corrupt. Retry it like an IO
             // error.
             LOG_ERROR(dctl_logger, DHCP_DDNS_REVERSE_REPLACE_RESP_CORRUPT)
-                      .arg(getCurrentServer()->getIpAddress())
+                      .arg(getCurrentServer()->toText())
                       .arg(getNcr()->getFqdn());
 
             // If we are out of retries on this server, we go back and start
@@ -519,7 +519,7 @@ NameAddTransaction::replacingRevPtrsHandler() {
                       DHCP_DDNS_REVERSE_REPLACE_BAD_DNSCLIENT_STATUS)
                       .arg(getDnsUpdateStatus())
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
             break;

+ 12 - 12
src/bin/d2/nc_remove.cc

@@ -225,7 +225,7 @@ NameRemoveTransaction::removingFwdAddrsHandler() {
                 // If we get not authorized should we try the next server in
                 // the list? @todo  This needs some discussion perhaps.
                 LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REMOVE_ADDRS_REJECTED)
-                          .arg(getCurrentServer()->getIpAddress())
+                          .arg(getCurrentServer()->toText())
                           .arg(getNcr()->getFqdn())
                           .arg(rcode.getCode());
                 transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
@@ -242,7 +242,7 @@ NameRemoveTransaction::removingFwdAddrsHandler() {
             // is not entirely clear if this is accurate.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REMOVE_ADDRS_IO_ERROR)
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             retryTransition(SELECTING_FWD_SERVER_ST);
             break;
@@ -251,7 +251,7 @@ NameRemoveTransaction::removingFwdAddrsHandler() {
             // A response was received but was corrupt. Retry it like an IO
             // error.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REMOVE_ADDRS_RESP_CORRUPT)
-                      .arg(getCurrentServer()->getIpAddress())
+                      .arg(getCurrentServer()->toText())
                       .arg(getNcr()->getFqdn());
 
             retryTransition(SELECTING_FWD_SERVER_ST);
@@ -264,7 +264,7 @@ NameRemoveTransaction::removingFwdAddrsHandler() {
                       DHCP_DDNS_FORWARD_REMOVE_ADDRS_BAD_DNSCLIENT_STATUS)
                       .arg(getDnsUpdateStatus())
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
             break;
@@ -340,7 +340,7 @@ NameRemoveTransaction::removingFwdRRsHandler() {
                 // If we get not authorized should try the next server in
                 // the list? @todo  This needs some discussion perhaps.
                 LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REMOVE_RRS_REJECTED)
-                          .arg(getCurrentServer()->getIpAddress())
+                          .arg(getCurrentServer()->toText())
                           .arg(getNcr()->getFqdn())
                           .arg(rcode.getCode());
                 transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
@@ -357,7 +357,7 @@ NameRemoveTransaction::removingFwdRRsHandler() {
             // is not entirely clear if this is accurate.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REMOVE_RRS_IO_ERROR)
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             // @note If we exhaust the IO retries for the current server
             // due to IO failures, we will abort the remaining updates.
@@ -374,7 +374,7 @@ NameRemoveTransaction::removingFwdRRsHandler() {
             // A response was received but was corrupt. Retry it like an IO
             // error.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REMOVE_RRS_RESP_CORRUPT)
-                      .arg(getCurrentServer()->getIpAddress())
+                      .arg(getCurrentServer()->toText())
                       .arg(getNcr()->getFqdn());
 
             // If we are out of retries on this server abandon the transaction.
@@ -389,7 +389,7 @@ NameRemoveTransaction::removingFwdRRsHandler() {
                       DHCP_DDNS_FORWARD_REMOVE_RRS_BAD_DNSCLIENT_STATUS)
                       .arg(getDnsUpdateStatus())
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
             break;
@@ -483,7 +483,7 @@ NameRemoveTransaction::removingRevPtrsHandler() {
                 // If we get not authorized should try the next server in
                 // the list? @todo  This needs some discussion perhaps.
                 LOG_ERROR(dctl_logger, DHCP_DDNS_REVERSE_REMOVE_REJECTED)
-                          .arg(getCurrentServer()->getIpAddress())
+                          .arg(getCurrentServer()->toText())
                           .arg(getNcr()->getFqdn())
                           .arg(rcode.getCode());
                 transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
@@ -500,7 +500,7 @@ NameRemoveTransaction::removingRevPtrsHandler() {
             // is not entirely clear if this is accurate.
             LOG_ERROR(dctl_logger, DHCP_DDNS_REVERSE_REMOVE_IO_ERROR)
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             // If we are out of retries on this server, we go back and start
             // all over on a new server.
@@ -511,7 +511,7 @@ NameRemoveTransaction::removingRevPtrsHandler() {
             // A response was received but was corrupt. Retry it like an IO
             // error.
             LOG_ERROR(dctl_logger, DHCP_DDNS_REVERSE_REMOVE_RESP_CORRUPT)
-                      .arg(getCurrentServer()->getIpAddress())
+                      .arg(getCurrentServer()->toText())
                       .arg(getNcr()->getFqdn());
 
             // If we are out of retries on this server, we go back and start
@@ -526,7 +526,7 @@ NameRemoveTransaction::removingRevPtrsHandler() {
                       DHCP_DDNS_REVERSE_REMOVE_BAD_DNSCLIENT_STATUS)
                       .arg(getDnsUpdateStatus())
                       .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->toText());
 
             transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
             break;

+ 12 - 0
src/bin/d2/nc_trans.cc

@@ -80,6 +80,7 @@ NameChangeTransaction::~NameChangeTransaction(){
 
 void
 NameChangeTransaction::startTransaction() {
+    setNcrStatus(dhcp_ddns::ST_PENDING);
     startModel(READY_ST);
 }
 
@@ -89,6 +90,12 @@ NameChangeTransaction::operator()(DNSClient::Status status) {
     // set to indicate IO completed.
     // runModel is exception safe so we are good to call it here.
     // It won't exit until we hit the next IO wait or the state model ends.
+    LOG_DEBUG(dctl_logger, DBGLVL_TRACE_DETAIL,
+              DHCP_DDNS_UPDATE_RESPONSE_RECEIVED)
+              .arg(getTransactionKey().toStr())
+              .arg(current_server_->toText())
+              .arg(status);
+
     setDnsUpdateStatus(status);
     runModel(IO_COMPLETED_EVT);
 }
@@ -109,6 +116,10 @@ NameChangeTransaction::sendUpdate(bool /* use_tsig_ */) {
 
         // Message is on its way, so the next event should be NOP_EVT.
         postNextEvent(NOP_EVT);
+        LOG_DEBUG(dctl_logger, DBGLVL_TRACE_DETAIL,
+                  DHCP_DDNS_UPDATE_REQUEST_SENT)
+                  .arg(getTransactionKey().toStr())
+                  .arg(current_server_->toText());
     } catch (const std::exception& ex) {
         // We were unable to initiate the send.
         // It is presumed that any throw from doUpdate is due to a programmatic
@@ -200,6 +211,7 @@ NameChangeTransaction::setDnsUpdateRequest(D2UpdateMessagePtr& request) {
 
 void
 NameChangeTransaction::clearDnsUpdateRequest() {
+    update_attempts_ = 0;
     dns_update_request_.reset();
 }
 

+ 21 - 12
src/bin/d2/nc_trans.h

@@ -152,7 +152,15 @@ public:
     //@}
 
     /// @brief Defualt time to assign to a single DNS udpate.
+#if 0
+    /// @todo  This value will be configurable in the near future, but
+    /// until it is there is no way to replace its value.  For now
+    /// we will define it to be relatively short, so unit tests will
+    /// run within reasonable amount of time.
     static const unsigned int DNS_UPDATE_DEFAULT_TIMEOUT = 5 * 1000;
+#else
+    static const unsigned int DNS_UPDATE_DEFAULT_TIMEOUT = 100;
+#endif
 
     /// @brief Maximum times to attempt a single update on a given server.
     static const unsigned int MAX_UPDATE_TRIES_PER_SERVER = 3;
@@ -287,7 +295,8 @@ protected:
     /// @param request is the new request packet to assign.
     void setDnsUpdateRequest(D2UpdateMessagePtr& request);
 
-    /// @brief Destroys the current update request packet.
+    /// @brief Destroys the current update request packet and resets
+    /// udpate attempts count..
     void clearDnsUpdateRequest();
 
     /// @brief Sets the update status to the given status value.
@@ -341,17 +350,6 @@ protected:
     /// servers from which to select.
     bool selectNextServer();
 
-    /// @brief Fetches the currently selected server.
-    ///
-    /// @return A const pointer reference to the DnsServerInfo of the current
-    /// server.
-    const DnsServerInfoPtr& getCurrentServer() const;
-
-    /// @brief Fetches the DNSClient instance
-    ///
-    /// @return A const pointer reference to the DNSClient
-    const DNSClientPtr& getDNSClient() const;
-
     /// @brief Sets the update attempt count to the given value.
     ///
     /// @param value is the new value to assign.
@@ -445,6 +443,17 @@ public:
     /// the request does not include a reverse change, the pointer will empty.
     DdnsDomainPtr& getReverseDomain();
 
+    /// @brief Fetches the currently selected server.
+    ///
+    /// @return A const pointer reference to the DnsServerInfo of the current
+    /// server.
+    const DnsServerInfoPtr& getCurrentServer() const;
+
+    /// @brief Fetches the DNSClient instance
+    ///
+    /// @return A const pointer reference to the DNSClient
+    const DNSClientPtr& getDNSClient() const;
+
     /// @brief Fetches the current DNS update request packet.
     ///
     /// @return A const pointer reference to the current D2UpdateMessage

+ 355 - 33
src/bin/d2/tests/d2_update_mgr_unittests.cc

@@ -16,6 +16,7 @@
 #include <d2/d2_update_mgr.h>
 #include <util/time_utilities.h>
 #include <d_test_stubs.h>
+#include <nc_test_utils.h>
 
 #include <boost/function.hpp>
 #include <boost/bind.hpp>
@@ -31,9 +32,9 @@ using namespace isc::d2;
 
 namespace {
 
-/// @brief Wrapper class for D2UpdateMgr to provide acces non-public methods.
+/// @brief Wrapper class for D2UpdateMgr to provide access non-public methods.
 ///
-/// This class faciliates testing by making non-public methods accessible so
+/// This class facilitates testing by making non-public methods accessible so
 /// they can be invoked directly in test routines.
 class D2UpdateMgrWrapper : public D2UpdateMgr {
 public:
@@ -66,18 +67,15 @@ typedef boost::shared_ptr<D2UpdateMgrWrapper> D2UpdateMgrWrapperPtr;
 /// D2CfgMgr.  This fixture provides an instance of each, plus a canned,
 /// valid DHCP_DDNS configuration sufficient to test D2UpdateMgr's basic
 /// functions.
-class D2UpdateMgrTest : public ConfigParseTest {
+class D2UpdateMgrTest : public TimedIO, public ConfigParseTest {
 public:
-    IOServicePtr io_service_;
     D2QueueMgrPtr queue_mgr_;
     D2CfgMgrPtr cfg_mgr_;
-    //D2UpdateMgrPtr update_mgr_;
     D2UpdateMgrWrapperPtr update_mgr_;
     std::vector<NameChangeRequestPtr> canned_ncrs_;
     size_t canned_count_;
 
     D2UpdateMgrTest() {
-        io_service_.reset(new isc::asiolink::IOService());
         queue_mgr_.reset(new D2QueueMgr(io_service_));
         cfg_mgr_.reset(new D2CfgMgr());
         update_mgr_.reset(new D2UpdateMgrWrapper(queue_mgr_, cfg_mgr_,
@@ -99,8 +97,8 @@ public:
         " \"change_type\" : 0 , "
         " \"forward_change\" : true , "
         " \"reverse_change\" : false , "
-        " \"fqdn\" : \"walah.walah.org.\" , "
-        " \"ip_address\" : \"192.168.2.1\" , "
+        " \"fqdn\" : \"my.example.com.\" , "
+        " \"ip_address\" : \"192.168.1.2\" , "
         " \"dhcid\" : \"0102030405060708\" , "
         " \"lease_expires_on\" : \"20130121132405\" , "
         " \"lease_length\" : 1300 "
@@ -112,6 +110,8 @@ public:
             dhcp_ddns::NameChangeRequestPtr ncr = NameChangeRequest::
                                                   fromJSON(msg_str);
             ncr->setDhcid(dhcids[i]);
+            ncr->setChangeType(i % 2 == 0 ?
+                               dhcp_ddns::CHG_ADD : dhcp_ddns::CHG_REMOVE);
             canned_ncrs_.push_back(ncr);
         }
     }
@@ -126,9 +126,9 @@ public:
                   "\"tsig_keys\": [] ,"
                   "\"forward_ddns\" : {"
                   "\"ddns_domains\": [ "
-                  "{ \"name\": \"two.three.org.\" , "
+                  "{ \"name\": \"example.com.\" , "
                   "  \"dns_servers\" : [ "
-                  "  { \"ip_address\": \"127.0.0.1\" } "
+                  "  { \"ip_address\": \"127.0.0.1\", \"port\" : 5301  } "
                   "  ] },"
                   "{ \"name\": \"org.\" , "
                   "  \"dns_servers\" : [ "
@@ -139,7 +139,7 @@ public:
                   "\"ddns_domains\": [ "
                   "{ \"name\": \"1.168.192.in-addr.arpa.\" , "
                   "  \"dns_servers\" : [ "
-                  "  { \"ip_address\": \"127.0.0.1\" } "
+                  "  { \"ip_address\": \"127.0.0.1\", \"port\" : 5301 } "
                   "  ] }, "
                   "{ \"name\": \"2.0.3.0.8.B.D.0.1.0.0.2.ip6.arpa.\" , "
                   "  \"dns_servers\" : [ "
@@ -153,6 +153,101 @@ public:
         ASSERT_TRUE(checkAnswer(0));
     }
 
+    /// @brief Fakes the completion of a given transaction.
+    ///
+    /// @param index index of the request from which the transaction was formed.
+    /// @param status completion status to assign to the request
+    void completeTransaction(const size_t index,
+                             const dhcp_ddns::NameChangeStatus& status) {
+        // add test on index
+        if (index >= canned_count_) {
+            ADD_FAILURE() << "request index is out of range: " << index;
+        }
+
+        const dhcp_ddns::D2Dhcid key = canned_ncrs_[index]->getDhcid();
+
+        // locate the transaction based on the request DHCID
+        TransactionList::iterator pos = update_mgr_->findTransaction(key);
+        if (pos == update_mgr_->transactionListEnd()) {
+            ADD_FAILURE() << "cannot find transaction for key: " << key.toStr();
+        }
+
+        NameChangeTransactionPtr trans = (*pos).second;
+        // Update the status of the request
+        trans->getNcr()->setStatus(status);
+        // End the model.
+        trans->endModel();
+    }
+
+    /// @brief Determines if any transactions are waiting for IO completion.
+    ///
+    /// @returns True if isModelWaiting() is true for at least one of the current
+    /// transactions.
+    bool anyoneWaiting() {
+        TransactionList::iterator it = update_mgr_->transactionListBegin();
+        while (it != update_mgr_->transactionListEnd()) {
+            if (((*it).second)->isModelWaiting()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /// @brief Process events until all requests have been completed.
+    ///
+    /// This method iteratively calls D2UpdateMgr::sweep and executes
+    /// IOService calls until both the request queue and transaction list
+    /// are empty or a timeout occurs.  Note that in addition to the safety
+    /// timer, the number of passes through the loop is also limited to
+    /// a given number.  This is a failsafe to guard against an infinite loop
+    /// in the test.
+    ///
+    /// @param timeout_millisec Maximum amount of time to allow IOService to
+    /// run before failing the test.  Note, If the intent of a test is to
+    /// verify proper handling of DNSClient timeouts, the value must be
+    /// slightly larger than that being used for DNSClient timeout value.
+    /// @param max_passes Maximum number of times through the loop before
+    /// failing the test.  The default value of twenty is likely large enough
+    /// for most tests.  The number of passes required for a given test can
+    /// vary.
+    void processAll(unsigned int timeout_millisec =
+                    NameChangeTransaction::DNS_UPDATE_DEFAULT_TIMEOUT + 100,
+                    size_t max_passes = 20) {
+        // Loop until all the transactions have been dequeued and run through to
+        // completion.
+        size_t passes = 0;
+        size_t handlers = 0;
+        while (update_mgr_->getQueueCount() ||
+            update_mgr_->getTransactionCount()) {
+            ++passes;
+            update_mgr_->sweep();
+            // If any transactions are waiting on IO, run the service.
+            if (anyoneWaiting()) {
+                int cnt = runTimedIO(timeout_millisec);
+
+                // If cnt is zero then the service stopped unexpectedly.
+                if (cnt == 0) {
+                    ADD_FAILURE()
+                        << "processALL: IO service stopped unexpectedly,"
+                        << " passes: " << passes << ", handlers executed: "
+                        << handlers;
+                }
+
+                handlers += cnt;
+            }
+
+            // This is a last resort fail safe to ensure we don't go around
+            // forever. We cut it off the number of passes at 20.  This is
+            // roughly twice the number for the longest test (currently,
+            // multiTransactionTimeout).
+            if (passes > max_passes) {
+                ADD_FAILURE() << "processALL failed, too many passes: "
+                    << passes <<  ", total handlers executed: " << handlers;
+            }
+        }
+    }
+
 };
 
 /// @brief Tests the D2UpdateMgr construction.
@@ -168,12 +263,12 @@ TEST(D2UpdateMgr, construction) {
     D2CfgMgrPtr cfg_mgr;
     D2UpdateMgrPtr update_mgr;
 
-    // Verify that constrctor fails if given an invalid queue manager.
+    // Verify that constructor fails if given an invalid queue manager.
     ASSERT_NO_THROW(cfg_mgr.reset(new D2CfgMgr()));
     EXPECT_THROW(D2UpdateMgr(queue_mgr, cfg_mgr, io_service),
                  D2UpdateMgrError);
 
-    // Verify that constrctor fails if given an invalid config manager.
+    // Verify that constructor fails if given an invalid config manager.
     ASSERT_NO_THROW(queue_mgr.reset(new D2QueueMgr(io_service)));
     ASSERT_NO_THROW(cfg_mgr.reset());
     EXPECT_THROW(D2UpdateMgr(queue_mgr, cfg_mgr, io_service),
@@ -212,10 +307,10 @@ TEST(D2UpdateMgr, construction) {
 /// This test verifies that:
 /// 1. A transaction can be added to the list.
 /// 2. Finding a transaction in the list by key works correctly.
-/// 3. Looking for a non-existant transaction works properly.
+/// 3. Looking for a non-existent transaction works properly.
 /// 4. Attempting to add a transaction for a DHCID already in the list fails.
 /// 5. Removing a transaction by key works properly.
-/// 6. Attempting to remove an non-existant transaction does no harm.
+/// 6. Attempting to remove an non-existent transaction does no harm.
 TEST_F(D2UpdateMgrTest, transactionList) {
     // Grab a canned request for test purposes.
     NameChangeRequestPtr& ncr = canned_ncrs_[0];
@@ -248,7 +343,7 @@ TEST_F(D2UpdateMgrTest, transactionList) {
     EXPECT_NO_THROW(update_mgr_->removeTransaction(ncr->getDhcid()));
     EXPECT_EQ(0, update_mgr_->getTransactionCount());
 
-    // Verify the we can try to remove a non-existant transaction without harm.
+    // Verify the we can try to remove a non-existent transaction without harm.
     EXPECT_NO_THROW(update_mgr_->removeTransaction(ncr->getDhcid()));
 }
 
@@ -266,12 +361,32 @@ TEST_F(D2UpdateMgrTest, checkFinishedTransaction) {
     for (int i = 0; i < canned_count_; i++) {
         EXPECT_NO_THROW(update_mgr_->makeTransaction(canned_ncrs_[i]));
     }
-    // Verfiy we have that the transaçtion count is correct.
+    // Verify we have that the transaction count is correct.
+    EXPECT_EQ(canned_count_, update_mgr_->getTransactionCount());
+
+    // Verify that all four transactions have been started.
+    TransactionList::iterator pos;
+    EXPECT_NO_THROW(pos = update_mgr_->transactionListBegin());
+    while (pos != update_mgr_->transactionListEnd()) {
+        NameChangeTransactionPtr trans = (*pos).second;
+        ASSERT_EQ(dhcp_ddns::ST_PENDING, trans->getNcrStatus());
+        ASSERT_TRUE(trans->isModelRunning());
+        ++pos;
+    }
+
+    // Verify that invoking checkFinishedTransactions does not throw.
+    EXPECT_NO_THROW(update_mgr_->checkFinishedTransactions());
+
+    // Since nothing is running IOService, the all four transactions should
+    // still be in the list.
     EXPECT_EQ(canned_count_, update_mgr_->getTransactionCount());
 
-    // Set two of the transactions to finished states.
-    (canned_ncrs_[1])->setStatus(dhcp_ddns::ST_COMPLETED);
-    (canned_ncrs_[3])->setStatus(dhcp_ddns::ST_FAILED);
+    // Now "complete" two of the four.
+    // Simulate a successful completion.
+    completeTransaction(1, dhcp_ddns::ST_COMPLETED);
+
+    // Simulate a failed completion.
+    completeTransaction(3, dhcp_ddns::ST_FAILED);
 
     // Verify that invoking checkFinishedTransactions does not throw.
     EXPECT_NO_THROW(update_mgr_->checkFinishedTransactions());
@@ -323,9 +438,9 @@ TEST_F(D2UpdateMgrTest, pickNextJob) {
     EXPECT_EQ(1, update_mgr_->getQueueCount());
 
     // Verify that invoking pickNextJob:
-    // 1. does not throw
-    // 2. does not make a new transaction
-    // 3. does not dequeu the entry
+    // 1. Does not throw
+    // 2. Does not make a new transaction
+    // 3. Does not dequeue the entry
     EXPECT_NO_THROW(update_mgr_->pickNextJob());
     EXPECT_EQ(canned_count_, update_mgr_->getTransactionCount());
     EXPECT_EQ(1, update_mgr_->getQueueCount());
@@ -345,9 +460,9 @@ TEST_F(D2UpdateMgrTest, pickNextJob) {
     ASSERT_NO_THROW(queue_mgr_->enqueue(bogus_ncr));
 
     // Verify that invoking pickNextJob:
-    // 1. does not throw
-    // 2. does not make a new transaction
-    // 3. does dequeue the entry
+    // 1. Does not throw
+    // 2. Does not make a new transaction
+    // 3. Does dequeue the entry
     EXPECT_NO_THROW(update_mgr_->pickNextJob());
     EXPECT_EQ(0, update_mgr_->getTransactionCount());
     EXPECT_EQ(0, update_mgr_->getQueueCount());
@@ -360,17 +475,17 @@ TEST_F(D2UpdateMgrTest, pickNextJob) {
 
     // Verify that invoking pickNextJob:
     // 1. does not throw
-    // 2. does not make a new transaction
-    // 3. does dequeue the entry
+    // 2. Does not make a new transaction
+    // 3. Does dequeue the entry
     EXPECT_NO_THROW(update_mgr_->pickNextJob());
     EXPECT_EQ(0, update_mgr_->getTransactionCount());
     EXPECT_EQ(0, update_mgr_->getQueueCount());
 }
 
 /// @brief Tests D2UpdateManager's sweep method.
-/// Since sweep is primarly a wrapper around chechFinishedTransactions and
+/// Since sweep is primarily a wrapper around checkFinishedTransactions and
 /// pickNextJob, along with checks on maximum transaction limits, it mostly
-/// verifies that these three pieces work togther to move process jobs.
+/// verifies that these three pieces work together to move process jobs.
 /// Most of what is tested here is tested above.
 TEST_F(D2UpdateMgrTest, sweep) {
     // Ensure we have at least 4 canned requests with which to work.
@@ -399,7 +514,7 @@ TEST_F(D2UpdateMgrTest, sweep) {
     // Verify max transactions can't be less than current transaction count.
     EXPECT_THROW(update_mgr_->setMaxTransactions(1), D2UpdateMgrError);
 
-    // Queue up a request for a DCHID which has a transaction in progress.
+    // Queue up a request for a DHCID which has a transaction in progress.
     dhcp_ddns::NameChangeRequestPtr
         subsequent_ncr(new dhcp_ddns::NameChangeRequest(*(canned_ncrs_[2])));
     EXPECT_NO_THROW(queue_mgr_->enqueue(subsequent_ncr));
@@ -412,7 +527,7 @@ TEST_F(D2UpdateMgrTest, sweep) {
     EXPECT_EQ(1, update_mgr_->getQueueCount());
 
     // Mark the transaction complete.
-    (canned_ncrs_[2])->setStatus(dhcp_ddns::ST_COMPLETED);
+    completeTransaction(2, dhcp_ddns::ST_COMPLETED);
 
     // Verify that invoking sweep, cleans up the completed transaction,
     // dequeues the queued job and adds its transaction to the list.
@@ -428,7 +543,7 @@ TEST_F(D2UpdateMgrTest, sweep) {
     EXPECT_EQ(1, update_mgr_->getQueueCount());
 
     // Verify that sweep does not dequeue the new request as we are at
-    // transaction count.
+    // maximum transaction count.
     EXPECT_NO_THROW(update_mgr_->sweep());
     EXPECT_EQ(canned_count_, update_mgr_->getTransactionCount());
     EXPECT_EQ(1, update_mgr_->getQueueCount());
@@ -447,4 +562,211 @@ TEST_F(D2UpdateMgrTest, sweep) {
     EXPECT_EQ(0, update_mgr_->getTransactionCount());
 }
 
+/// @brief Tests integration of NameAddTransaction
+/// This test verifies that update manager can create and manage a
+/// NameAddTransaction from start to finish.  It utilizes a fake server
+/// which responds to all requests sent with NOERROR, simulating a
+/// successful addition.  The transaction processes both forward and
+/// reverse changes.
+TEST_F(D2UpdateMgrTest, addTransaction) {
+    // Put each transaction on the queue.
+    canned_ncrs_[0]->setChangeType(dhcp_ddns::CHG_ADD);
+    canned_ncrs_[0]->setReverseChange(true);
+    ASSERT_NO_THROW(queue_mgr_->enqueue(canned_ncrs_[0]));
+
+    // Call sweep once, this should:
+    // 1. Dequeue the request
+    // 2. Create the transaction
+    // 3. Start the transaction
+    ASSERT_NO_THROW(update_mgr_->sweep());
+
+    // Get a copy of the transaction.
+    TransactionList::iterator pos = update_mgr_->transactionListBegin();
+    ASSERT_TRUE (pos != update_mgr_->transactionListEnd());
+    NameChangeTransactionPtr trans = (*pos).second;
+    ASSERT_TRUE(trans);
+
+    // At this point the transaction should have constructed
+    // and sent the DNS request.
+    ASSERT_TRUE(trans->getCurrentServer());
+    ASSERT_TRUE(trans->isModelRunning());
+    ASSERT_EQ(1, trans->getUpdateAttempts());
+    ASSERT_EQ(StateModel::NOP_EVT, trans->getNextEvent());
+
+    // Create a server based on the transaction's current server, and
+    // start it listening.
+    FauxServer server(*io_service_, *(trans->getCurrentServer()));
+    server.receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());
+
+    // Run sweep and IO until everything is done.
+    processAll();
+
+    // Verify that model succeeded.
+    EXPECT_FALSE(trans->didModelFail());
+
+    // Both completion flags should be true.
+    EXPECT_TRUE(trans->getForwardChangeCompleted());
+    EXPECT_TRUE(trans->getReverseChangeCompleted());
+
+    // Verify that we went through success state.
+    EXPECT_EQ(NameChangeTransaction::PROCESS_TRANS_OK_ST,
+              trans->getPrevState());
+    EXPECT_EQ(NameChangeTransaction::UPDATE_OK_EVT,
+              trans->getLastEvent());
+}
+
+/// @brief Tests integration of NameRemoveTransaction
+/// This test verifies that update manager can create and manage a
+/// NameRemoveTransaction from start to finish.  It utilizes a fake server
+/// which responds to all requests sent with NOERROR, simulating a
+/// successful addition.  The transaction processes both forward and
+/// reverse changes.
+TEST_F(D2UpdateMgrTest, removeTransaction) {
+    // Put each transaction on the queue.
+    canned_ncrs_[0]->setChangeType(dhcp_ddns::CHG_REMOVE);
+    canned_ncrs_[0]->setReverseChange(true);
+    ASSERT_NO_THROW(queue_mgr_->enqueue(canned_ncrs_[0]));
+
+    // Call sweep once, this should:
+    // 1. Dequeue the request
+    // 2. Create the transaction
+    // 3. Start the transaction
+    ASSERT_NO_THROW(update_mgr_->sweep());
+
+    // Get a copy of the transaction.
+    TransactionList::iterator pos = update_mgr_->transactionListBegin();
+    ASSERT_TRUE (pos != update_mgr_->transactionListEnd());
+    NameChangeTransactionPtr trans = (*pos).second;
+    ASSERT_TRUE(trans);
+
+    // At this point the transaction should have constructed
+    // and sent the DNS request.
+    ASSERT_TRUE(trans->getCurrentServer());
+    ASSERT_TRUE(trans->isModelRunning());
+    ASSERT_EQ(1, trans->getUpdateAttempts());
+    ASSERT_EQ(StateModel::NOP_EVT, trans->getNextEvent());
+
+    // Create a server based on the transaction's current server,
+    // and start it listening.
+    FauxServer server(*io_service_, *(trans->getCurrentServer()));
+    server.receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());
+
+    // Run sweep and IO until everything is done.
+    processAll();
+
+    // Verify that model succeeded.
+    EXPECT_FALSE(trans->didModelFail());
+
+    // Both completion flags should be true.
+    EXPECT_TRUE(trans->getForwardChangeCompleted());
+    EXPECT_TRUE(trans->getReverseChangeCompleted());
+
+    // Verify that we went through success state.
+    EXPECT_EQ(NameChangeTransaction::PROCESS_TRANS_OK_ST,
+              trans->getPrevState());
+    EXPECT_EQ(NameChangeTransaction::UPDATE_OK_EVT,
+              trans->getLastEvent());
+}
+
+
+/// @brief Tests handling of a transaction which fails.
+/// This test verifies that update manager correctly concludes a transaction
+/// which fails to complete successfully.  The failure simulated is repeated
+/// corrupt responses from the server, which causes an exhaustion of the
+/// available servers.
+TEST_F(D2UpdateMgrTest, errorTransaction) {
+    // Put each transaction on the queue.
+    ASSERT_NO_THROW(queue_mgr_->enqueue(canned_ncrs_[0]));
+
+    // Call sweep once, this should:
+    // 1. Dequeue the request
+    // 2. Create the transaction
+    // 3. Start the transaction
+    ASSERT_NO_THROW(update_mgr_->sweep());
+
+    // Get a copy of the transaction.
+    TransactionList::iterator pos = update_mgr_->transactionListBegin();
+    ASSERT_TRUE (pos != update_mgr_->transactionListEnd());
+    NameChangeTransactionPtr trans = (*pos).second;
+    ASSERT_TRUE(trans);
+
+    ASSERT_TRUE(trans->isModelRunning());
+    ASSERT_EQ(1, trans->getUpdateAttempts());
+    ASSERT_EQ(StateModel::NOP_EVT, trans->getNextEvent());
+    ASSERT_TRUE(trans->getCurrentServer());
+
+    // Create a server and start it listening.
+    FauxServer server(*io_service_, *(trans->getCurrentServer()));
+    server.receive(FauxServer::CORRUPT_RESP);
+
+    // Run sweep and IO until everything is done.
+    processAll();
+
+    // Verify that model succeeded.
+    EXPECT_FALSE(trans->didModelFail());
+
+    // Both completion flags should be false.
+    EXPECT_FALSE(trans->getForwardChangeCompleted());
+    EXPECT_FALSE(trans->getReverseChangeCompleted());
+
+    // Verify that we went through success state.
+    EXPECT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
+              trans->getPrevState());
+    EXPECT_EQ(NameChangeTransaction::NO_MORE_SERVERS_EVT,
+              trans->getLastEvent());
+
+
+}
+
+/// @brief Tests processing of multiple transactions.
+/// This test verifies that update manager can create and manage a multiple
+/// transactions, concurrently.  It uses a fake server that responds to all
+/// requests sent with NOERROR, simulating successful DNS updates. The
+/// transactions are a mix of both adds and removes.
+TEST_F(D2UpdateMgrTest, multiTransaction) {
+    // Queue up all the requests.
+    int test_count = canned_count_;
+    for (int i = test_count; i > 0; i--) {
+        canned_ncrs_[i-1]->setReverseChange(true);
+        ASSERT_NO_THROW(queue_mgr_->enqueue(canned_ncrs_[i-1]));
+    }
+
+    // Create a server and start it listening. Note this relies on the fact
+    // that all of configured servers have the same address.
+    // and start it listening.
+    asiolink::IOAddress server_ip("127.0.0.1");
+    FauxServer server(*io_service_, server_ip, 5301);
+    server.receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());
+
+    // Run sweep and IO until everything is done.
+    processAll();
+
+    for (int i = 0; i < test_count; i++) {
+        EXPECT_EQ(dhcp_ddns::ST_COMPLETED, canned_ncrs_[i]->getStatus());
+    }
+}
+
+/// @brief Tests processing of multiple transactions.
+/// This test verifies that update manager can create and manage a multiple
+/// transactions, concurrently.  It uses a fake server that responds to all
+/// requests sent with NOERROR, simulating successful DNS updates. The
+/// transactions are a mix of both adds and removes.
+TEST_F(D2UpdateMgrTest, multiTransactionTimeout) {
+    // Queue up all the requests.
+    int test_count = canned_count_;
+    for (int i = test_count; i > 0; i--) {
+        canned_ncrs_[i-1]->setReverseChange(true);
+        ASSERT_NO_THROW(queue_mgr_->enqueue(canned_ncrs_[i-1]));
+    }
+
+    // No server is running, so everything will time out.
+
+    // Run sweep and IO until everything is done.
+    processAll();
+
+    for (int i = 0; i < test_count; i++) {
+        EXPECT_EQ(dhcp_ddns::ST_FAILED, canned_ncrs_[i]->getStatus());
+    }
+}
+
 }

+ 25 - 8
src/bin/d2/tests/dns_client_unittests.cc

@@ -25,6 +25,7 @@
 #include <boost/bind.hpp>
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
+#include <nc_test_utils.h>
 
 using namespace std;
 using namespace isc;
@@ -69,6 +70,8 @@ public:
     bool corrupt_response_;
     bool expect_response_;
     asiolink::IntervalTimer test_timer_;
+    int received_;
+    int expected_;
 
     // @brief Constructor.
     //
@@ -83,7 +86,8 @@ public:
           status_(DNSClient::SUCCESS),
           corrupt_response_(false),
           expect_response_(true),
-          test_timer_(service_) {
+          test_timer_(service_),
+          received_(0), expected_(0) {
         asiodns::logger.setSeverity(isc::log::INFO);
         response_.reset(new D2UpdateMessage(D2UpdateMessage::INBOUND));
         dns_client_.reset(new DNSClient(response_, this));
@@ -108,7 +112,10 @@ public:
     // @param status A status code returned by DNSClient.
     virtual void operator()(DNSClient::Status status) {
         status_ = status;
-        service_.stop();
+        if (!expected_ || (expected_ == ++received_))
+        {
+            service_.stop();
+        }
 
         if (expect_response_) {
             if (!corrupt_response_) {
@@ -121,7 +128,7 @@ public:
                           response_->getRRCount(D2UpdateMessage::SECTION_ZONE));
                 D2ZonePtr zone = response_->getZone();
                 ASSERT_TRUE(zone);
-                EXPECT_EQ("example.com.", zone->getName().toText());
+                EXPECT_EQ("response.example.com.", zone->getName().toText());
                 EXPECT_EQ(RRClass::IN().getCode(), zone->getClass().getCode());
 
             } else {
@@ -187,9 +194,6 @@ public:
     // It also verifies that the constructor will not throw if the supplied
     // callback object is NULL.
     void runConstructorTest() {
-        D2UpdateMessagePtr null_response;
-        EXPECT_THROW(DNSClient(null_response, this, DNSClient::UDP),
-                     isc::BadValue);
         EXPECT_NO_THROW(DNSClient(response_, NULL, DNSClient::UDP));
 
         // The TCP Transport is not supported right now. So, we return exception
@@ -284,7 +288,7 @@ public:
         // Create a request DNS Update message.
         D2UpdateMessage message(D2UpdateMessage::OUTBOUND);
         ASSERT_NO_THROW(message.setRcode(Rcode(Rcode::NOERROR_CODE)));
-        ASSERT_NO_THROW(message.setZone(Name("example.com"), RRClass::IN()));
+        ASSERT_NO_THROW(message.setZone(Name("response.example.com"), RRClass::IN()));
 
         // In order to perform the full test, when the client sends the request
         // and receives a response from the server, we have to emulate the
@@ -322,11 +326,13 @@ public:
         // The socket is now ready to receive the data. Let's post some request
         // message then.
         const int timeout = 5;
+        expected_++;
         dns_client_->doUpdate(service_, IOAddress(TEST_ADDRESS), TEST_PORT,
                              message, timeout);
 
         // It is possible to request that two packets are sent concurrently.
         if (two_sends) {
+            expected_++;
             dns_client_->doUpdate(service_, IOAddress(TEST_ADDRESS), TEST_PORT,
                                   message, timeout);
 
@@ -338,6 +344,10 @@ public:
 
         udp_socket.close();
 
+        // Since the callback, operator(), calls stop() on the io_service,
+        // we must reset it in order for subsequent calls to run() or
+        // run_one() to work.
+        service_.get_io_service().reset();
     }
 };
 
@@ -393,6 +403,7 @@ TEST_F(DNSClientTest, sendReceiveCurrupted) {
 TEST_F(DNSClientTest, sendReceiveTwice) {
     runSendReceiveTest(false, false);
     runSendReceiveTest(false, false);
+    EXPECT_EQ(2, received_);
 }
 
 // Verify that it is possible to use the DNSClient instance to perform the
@@ -401,7 +412,13 @@ TEST_F(DNSClientTest, sendReceiveTwice) {
 // 2. send
 // 3. receive
 // 4. receive
-TEST_F(DNSClientTest, concurrentSendReceive) {
+// @todo  THIS Test does not function. The method runSendReceive only
+// schedules one "server" receive.  In other words only one request is
+// listened for and then received. Once it is received, the operator()
+// method calls stop() on the io_service, which causes the second receive
+// to be cancelled.  It is also unclear, what the asio layer does with a
+// second receive on the same socket.
+TEST_F(DNSClientTest, DISABLED_concurrentSendReceive) {
     runSendReceiveTest(false, true);
 }
 

+ 62 - 18
src/bin/d2/tests/nc_test_utils.cc

@@ -37,7 +37,8 @@ const bool NO_RDATA = false;
 FauxServer::FauxServer(asiolink::IOService& io_service,
                        asiolink::IOAddress& address, size_t port)
     :io_service_(io_service), address_(address), port_(port),
-     server_socket_() {
+     server_socket_(), receive_pending_(false), perpetual_receive_(true) {
+
     server_socket_.reset(new asio::ip::udp::socket(io_service_.get_io_service(),
                                                    asio::ip::udp::v4()));
     server_socket_->set_option(asio::socket_base::reuse_address(true));
@@ -47,7 +48,8 @@ FauxServer::FauxServer(asiolink::IOService& io_service,
 FauxServer::FauxServer(asiolink::IOService& io_service,
                        DnsServerInfo& server)
     :io_service_(io_service), address_(server.getIpAddress()),
-     port_(server.getPort()), server_socket_() {
+     port_(server.getPort()), server_socket_(), receive_pending_(false),
+     perpetual_receive_(true) {
     server_socket_.reset(new asio::ip::udp::socket(io_service_.get_io_service(),
                                                    asio::ip::udp::v4()));
     server_socket_->set_option(asio::socket_base::reuse_address(true));
@@ -61,6 +63,11 @@ FauxServer::~FauxServer() {
 void
 FauxServer::receive (const ResponseMode& response_mode,
                      const dns::Rcode& response_rcode) {
+    if (receive_pending_) {
+        return;
+    }
+
+    receive_pending_ = true;
     server_socket_->async_receive_from(asio::buffer(receive_buffer_,
                                                     sizeof(receive_buffer_)),
                                        remote_,
@@ -79,6 +86,7 @@ FauxServer::requestHandler(const asio::error_code& error,
     // We expect the client to send good requests.
     if (error.value() != 0 || bytes_recvd < 1) {
         ADD_FAILURE() << "FauxServer receive failed" << error.message();
+        receive_pending_ = false;
         return;
     }
 
@@ -95,10 +103,11 @@ FauxServer::requestHandler(const asio::error_code& error,
         // If the request cannot be parsed, then fail the test.
         // We expect the client to send good requests.
         ADD_FAILURE() << "FauxServer request is corrupt:" << ex.what();
+        receive_pending_ = false;
         return;
     }
 
-    // The request parsed ok, so let's build a response.
+    // The request parsed OK, so let's build a response.
     // We must use the QID we received in the response or IOFetch will
     // toss the response out, resulting in eventual timeout.
     // We fill in the zone with data we know is from the "server".
@@ -125,7 +134,7 @@ FauxServer::requestHandler(const asio::error_code& error,
         response_buf.writeUint16At(0xFFFF, 2);
     }
 
-    // Ship the reponse via synchronous send.
+    // Ship the response via synchronous send.
     try {
         int cnt = server_socket_->send_to(asio::
                                           buffer(response_buf.getData(),
@@ -139,35 +148,56 @@ FauxServer::requestHandler(const asio::error_code& error,
     } catch (const std::exception& ex) {
         ADD_FAILURE() << "FauxServer send failed: " << ex.what();
     }
+
+    receive_pending_ = false;
+    if (perpetual_receive_) {
+        // Schedule the next receive
+        receive (response_mode, response_rcode);
+    }
 }
 
-//********************** TransactionTest class ***********************
 
-const unsigned int TransactionTest::FORWARD_CHG = 0x01;
-const unsigned int TransactionTest::REVERSE_CHG = 0x02;
-const unsigned int TransactionTest::FWD_AND_REV_CHG = REVERSE_CHG | FORWARD_CHG;
+//********************** TimedIO class ***********************
 
-TransactionTest::TransactionTest()
-    : io_service_(new isc::asiolink::IOService()), ncr_(),
-    timer_(*io_service_), run_time_(0) {
+TimedIO::TimedIO()
+    : io_service_(new isc::asiolink::IOService()),
+     timer_(*io_service_), run_time_(0) {
 }
 
-TransactionTest::~TransactionTest() {
+TimedIO::~TimedIO() {
 }
 
-void
-TransactionTest::runTimedIO(int run_time) {
+int
+TimedIO::runTimedIO(int run_time) {
     run_time_ = run_time;
-    timer_.setup(boost::bind(&TransactionTest::timesUp, this), run_time_);
-    io_service_->run();
+    int cnt = io_service_->get_io_service().poll();
+    if (cnt == 0) {
+        timer_.setup(boost::bind(&TransactionTest::timesUp, this), run_time_);
+        cnt = io_service_->get_io_service().run_one();
+        timer_.cancel();
+    }
+
+    return (cnt);
 }
 
 void
-TransactionTest::timesUp() {
+TimedIO::timesUp() {
     io_service_->stop();
     FAIL() << "Test Time: " << run_time_ << " expired";
 }
 
+//********************** TransactionTest class ***********************
+
+const unsigned int TransactionTest::FORWARD_CHG = 0x01;
+const unsigned int TransactionTest::REVERSE_CHG = 0x02;
+const unsigned int TransactionTest::FWD_AND_REV_CHG = REVERSE_CHG | FORWARD_CHG;
+
+TransactionTest::TransactionTest() : ncr_() {
+}
+
+TransactionTest::~TransactionTest() {
+}
+
 void
 TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type,
                                          int change_mask) {
@@ -299,7 +329,7 @@ checkRR(dns::RRsetPtr rrset, const std::string& exp_name,
     EXPECT_EQ(exp_class.getCode(), rrset->getClass().getCode());
     EXPECT_EQ(exp_type.getCode(), rrset->getType().getCode());
     EXPECT_EQ(exp_ttl, rrset->getTTL().getValue());
-    if ((!has_rdata) || 
+    if ((!has_rdata) ||
        (exp_type == dns::RRType::ANY() || exp_class == dns::RRClass::ANY())) {
         // ANY types do not have RData
         ASSERT_EQ(0, rrset->getRdataCount());
@@ -653,6 +683,20 @@ void checkRemoveRevPtrsRequest(NameChangeTransaction& tran) {
     ASSERT_NO_THROW(request->toWire(renderer));
 }
 
+std::string toHexText(const uint8_t* data, size_t len) {
+    std::ostringstream stream;
+    stream << "Data length is: " << len << std::endl;
+    for (int i = 0; i < len; ++i) {
+        if (i > 0 && ((i % 16) == 0)) {
+            stream << std::endl;
+        }
+
+        stream << setfill('0') << setw(2) << setbase(16)
+               << static_cast<unsigned int>(data[i]) << " ";
+    }
+
+    return (stream.str());
+}
 
 }; // namespace isc::d2
 }; // namespace isc

+ 52 - 19
src/bin/d2/tests/nc_test_utils.h

@@ -50,6 +50,8 @@ public:
     SocketPtr server_socket_;
     asio::ip::udp::endpoint remote_;
     uint8_t receive_buffer_[TEST_MSG_MAX];
+    bool receive_pending_;
+    bool perpetual_receive_;
 
     /// @brief Constructor
     ///
@@ -93,17 +95,56 @@ public:
                         std::size_t bytes_recvd,
                         const ResponseMode& response_mode,
                         const dns::Rcode& response_rcode);
+
+    bool isReceivePending() {
+        return receive_pending_;
+    }
 };
 
-/// @brief Base class Test fixture for testing transactions.
-class TransactionTest : public ::testing::Test {
+class TimedIO {
 public:
     IOServicePtr io_service_;
+    asiolink::IntervalTimer timer_;
+    int run_time_;
+
+    // Constructor
+    TimedIO();
+
+    // Destructor
+    virtual ~TimedIO();
+
+    /// @brief IO Timer expiration handler
+    ///
+    /// Stops the IOService and fails the current test.
+    virtual void timesUp();
+
+    /// @brief Runs IOService till time expires or at least one handler executes.
+    ///
+    /// This method first polls IOService to run any ready handlers.  If no
+    /// handlers are ready, it starts the internal time to run for the given
+    /// amount of time and invokes service's run_one method.  This method
+    /// blocks until at least one handler executes or the IO Service is stopped.
+    /// Upon completion of this method the timer is cancelled.  Should the
+    /// timer expires prior to run_one returning, the timesUp handler will be
+    /// invoked which stops the IO service and fails the test.
+    ///
+    /// Note that this method closely mimics the runIO method in D2Process.
+    ///
+    /// @param run_time maximum length of time to run in milliseconds before
+    /// timing out.
+    ///
+    /// @return Returns the number of handlers executed or zero. A return of
+    /// zero indicates that the IOService has been stopped.
+    int runTimedIO(int run_time);
+
+};
+
+/// @brief Base class Test fixture for testing transactions.
+class TransactionTest : public TimedIO,  public ::testing::Test {
+public:
     dhcp_ddns::NameChangeRequestPtr ncr_;
     DdnsDomainPtr forward_domain_;
     DdnsDomainPtr reverse_domain_;
-    asiolink::IntervalTimer timer_;
-    int run_time_;
 
     /// #brief constants used to specify change directions for a transaction.
     static const unsigned int FORWARD_CHG;      // Only forward change.
@@ -113,20 +154,6 @@ public:
     TransactionTest();
     virtual ~TransactionTest();
 
-    /// @brief Run the IO service for no more than a given amount of time.
-    ///
-    /// Uses an IntervalTimer to interrupt the invocation of IOService run(),
-    /// after the given number of milliseconds elapse.  The timer executes
-    /// the timesUp() method if it expires.
-    ///
-    /// @param run_time amount of time in milliseconds to allow run to execute.
-    void runTimedIO(int run_time);
-
-    /// @brief IO Timer expiration handler
-    ///
-    /// Stops the IOSerivce and fails the current test.
-    virtual void timesUp();
-
     /// @brief Creates a transaction which requests an IPv4 DNS update.
     ///
     /// The transaction is constructed around a predefined (i.e. "canned")
@@ -190,7 +217,7 @@ extern void checkZone(const D2UpdateMessagePtr& request,
 /// @param has_rdata if true, RRset's rdata will be checked based on it's
 /// RRType.  Set this to false if the RRset's type supports Rdata but it does
 /// not contain it.  For instance, prerequisites of type NONE have no Rdata
-/// where udpates of type NONE may.
+/// where updates of type NONE may.
 extern void checkRR(dns::RRsetPtr rrset, const std::string& exp_name,
                     const dns::RRClass& exp_class, const dns::RRType& exp_type,
                     unsigned int exp_ttl, dhcp_ddns::NameChangeRequestPtr ncr,
@@ -308,6 +335,12 @@ extern void addDomainServer(DdnsDomainPtr& domain, const std::string& name,
                             const std::string& ip = TEST_DNS_SERVER_IP,
                             const size_t port = TEST_DNS_SERVER_PORT);
 
+/// @brief Creates a hex text dump of the given data buffer.
+///
+/// @param data pointer to the data to dump
+/// @param len size (in bytes) of data
+extern std::string toHexText(const uint8_t* data, size_t len);
+
 }; // namespace isc::d2
 }; // namespace isc
 

+ 2 - 0
src/bin/d2/tests/nc_trans_unittests.cc

@@ -686,6 +686,7 @@ TEST_F(NameChangeTransactionTest, modelFailure) {
 TEST_F(NameChangeTransactionTest, successfulUpdateTest) {
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    ASSERT_TRUE(name_change->selectFwdServer());
 
     EXPECT_TRUE(name_change->isModelNew());
     EXPECT_FALSE(name_change->getForwardChangeCompleted());
@@ -722,6 +723,7 @@ TEST_F(NameChangeTransactionTest, successfulUpdateTest) {
 TEST_F(NameChangeTransactionTest, failedUpdateTest) {
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    ASSERT_TRUE(name_change->selectFwdServer());
 
     // Launch the transaction by calling startTransaction.  The state model
     // should run up until the "IO" operation is initiated in DOING_UPDATE_ST.