[3222] Augmented DNS message IO in b10-dhcp-ddns

Added descriptive message comment log statement for
to outbound DNS update requests, and conversion of status
and RCODEs for DNS inbound responses to b10-dhcp-ddns log.

src/bin/d2/d2_messages.mes

@@ -440,7 +440,7 @@ documented in preceding log entries.
 
 % DHCP_DDNS_STARTING_TRANSACTION Transaction Key: %1
 
-% DHCP_DDNS_UPDATE_REQUEST_SENT for transaction key: %1 to server: %2
+% DHCP_DDNS_UPDATE_REQUEST_SENT %1 for transaction key: %2 to server: %3
 This is a debug message issued when DHCP_DDNS sends a DNS request to a DNS
 server.
 

src/bin/d2/nc_add.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// 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
@@ -202,7 +202,7 @@ NameAddTransaction::addingFwdAddrsHandler() {
 
         // Call sendUpdate() to initiate the async send. Note it also sets
         // next event to NOP_EVT.
-        sendUpdate();
+        sendUpdate("Foward Add");
         break;
 
     case IO_COMPLETED_EVT: {
@@ -313,7 +313,7 @@ NameAddTransaction::replacingFwdAddrsHandler() {
 
         // Call sendUpdate() to initiate the async send. Note it also sets
         // next event to NOP_EVT.
-        sendUpdate();
+        sendUpdate("Forward Replace");
         break;
 
     case IO_COMPLETED_EVT: {
@@ -459,7 +459,7 @@ NameAddTransaction::replacingRevPtrsHandler() {
 
         // Call sendUpdate() to initiate the async send. Note it also sets
         // next event to NOP_EVT.
-        sendUpdate();
+        sendUpdate("Reverse Replace");
         break;
 
     case IO_COMPLETED_EVT: {

src/bin/d2/nc_remove.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// 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
@@ -207,7 +207,7 @@ NameRemoveTransaction::removingFwdAddrsHandler() {
 
         // Call sendUpdate() to initiate the async send. Note it also sets
         // next event to NOP_EVT.
-        sendUpdate();
+        sendUpdate("Forward A/AAAA Remove");
         break;
 
     case IO_COMPLETED_EVT: {
@@ -311,7 +311,7 @@ NameRemoveTransaction::removingFwdRRsHandler() {
 
         // Call sendUpdate() to initiate the async send. Note it also sets
         // next event to NOP_EVT.
-        sendUpdate();
+        sendUpdate("Forward RR Remove");
         break;
 
     case IO_COMPLETED_EVT: {
@@ -464,7 +464,7 @@ NameRemoveTransaction::removingRevPtrsHandler() {
 
         // Call sendUpdate() to initiate the async send. Note it also sets
         // next event to NOP_EVT.
-        sendUpdate();
+        sendUpdate("Reverse Remove");
         break;
 
     case IO_COMPLETED_EVT: {

src/bin/d2/nc_trans.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// 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
@@ -16,6 +16,8 @@
 #include <d2/nc_trans.h>
 #include <dns/rdata.h>
 
+#include <sstream>
+
 namespace isc {
 namespace d2 {
 
@@ -94,18 +96,53 @@ 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.
+    setDnsUpdateStatus(status);
     LOG_DEBUG(dctl_logger, DBGLVL_TRACE_DETAIL,
               DHCP_DDNS_UPDATE_RESPONSE_RECEIVED)
               .arg(getTransactionKey().toStr())
               .arg(current_server_->toText())
-              .arg(status);
+              .arg(responseString());
 
-    setDnsUpdateStatus(status);
     runModel(IO_COMPLETED_EVT);
 }
 
+std::string
+NameChangeTransaction::responseString() {
+    std::ostringstream stream;
+    switch (getDnsUpdateStatus()) {
+        case DNSClient::SUCCESS:
+            stream << "SUCCESS, rcode: ";
+            if (getDnsUpdateResponse()) {
+                 stream << getDnsUpdateResponse()->getRcode().toText();
+            } else {
+                stream << " update response is NULL";
+            }
+            break;
+        case DNSClient::TIMEOUT:
+            stream << "TIMEOUT";
+            break;
+        case DNSClient::IO_STOPPED:
+            stream << "IO_STOPPED";
+            break;
+        case DNSClient::INVALID_RESPONSE:
+            stream << "INVALID_RESPONSE";
+            break;
+        case DNSClient::OTHER:
+            stream << "OTHER";
+            break;
+        default:
+            stream << "UKNOWNN("
+                   << static_cast<int>(getDnsUpdateStatus()) << ")";
+            break;
+
+    }
+
+    return (stream.str());
+}
+
 void
-NameChangeTransaction::sendUpdate(bool /* use_tsig_ */) {
+NameChangeTransaction::sendUpdate(const std::string& comment,
+                                  bool /* use_tsig_ */) {
     try {
         ++update_attempts_;
         // @todo add logic to add/replace TSIG key info in request if
@@ -122,6 +159,7 @@ NameChangeTransaction::sendUpdate(bool /* use_tsig_ */) {
         postNextEvent(NOP_EVT);
         LOG_DEBUG(dctl_logger, DBGLVL_TRACE_DETAIL,
                   DHCP_DDNS_UPDATE_REQUEST_SENT)
+                  .arg(comment)
                   .arg(getTransactionKey().toStr())
                   .arg(current_server_->toText());
     } catch (const std::exception& ex) {

src/bin/d2/nc_trans.h

@@ -207,12 +207,14 @@ protected:
     /// currently selected server.  Since the send is asynchronous, the method
     /// posts NOP_EVT as the next event and then returns.
     ///
+    /// @param comment text to include in log detail
     /// @param use_tsig True if the update should be include a TSIG key. This
     /// is not yet implemented.
     ///
     /// If an exception occurs it will be logged and and the transaction will
     /// be failed.
-    virtual void sendUpdate(bool use_tsig = false);
+    virtual void sendUpdate(const std::string& comment = "",
+                            bool use_tsig = false);
 
     /// @brief Adds events defined by NameChangeTransaction to the event set.
     ///
@@ -401,6 +403,14 @@ protected:
     /// the RData cannot be added to the given RRset.
     void addPtrRdata(dns::RRsetPtr& rrset);
 
+    /// @brief Returns a string version of the current response status and rcode
+    ///
+    /// Renders a string containing the a text label current DNS update status
+    /// and RCODE (if status is DNSClient::SUCCESS)
+    ///
+    /// @return std::string containing constructed text
+    std::string responseString();
+
 public:
     /// @brief Fetches the NameChangeRequest for this transaction.
     ///

src/bin/d2/tests/nc_add_unittests.cc

@@ -54,10 +54,11 @@ public:
     /// It will also simulate an exception-based failure of sendUpdate, if
     /// the simulate_send_exception_ flag is true.
     ///
-    /// @param use_tsig_ Parameter is unused, but present in the base class
-    /// method.
+    /// @param comment Parameter is unused, but present in base class method.
+    /// @param use_tsig_ Parameter is unused, but present in base class method.
     ///
-    virtual void sendUpdate(bool /* use_tsig_ = false */) {
+    virtual void sendUpdate(const std::string& /*comment*/,
+                            bool /* use_tsig_ = false */) {
         if (simulate_send_exception_) {
             // Make the flag a one-shot by resetting it.
             simulate_send_exception_ = false;

src/bin/d2/tests/nc_remove_unittests.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// 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
@@ -55,10 +55,11 @@ public:
     /// It will also simulate an exception-based failure of sendUpdate, if
     /// the simulate_send_exception_ flag is true.
     ///
-    /// @param use_tsig_ Parameter is unused, but present in the base class
-    /// method.
+    /// @param comment Parameter is unused, but present in base class method
+    /// @param use_tsig Parameter is unused, but present in base class method.
     ///
-    virtual void sendUpdate(bool /* use_tsig_ = false */) {
+    virtual void sendUpdate(const std::string& /* comment */,
+                            bool /* use_tsig = false */) {
         if (simulate_send_exception_) {
             // Make the flag a one-shot by resetting it.
             simulate_send_exception_ = false;