[3977] Name change requests are now generated when lease is reused.

This change triggered a lot of code refactoring for generating the
NameChangeRequests. Long story short is that the functions responsible
for generating NCRs from the lease information have been moved to
the libdhcpsrv where they better fit and where they may be used
by both allocation engine and the servers.

src/bin/dhcp4/dhcp4_messages.mes

@@ -200,11 +200,6 @@ as declined (i.e. used by unknown entity). The server has information about
 a lease for that address, but the client's hardware address or client identifier
 does not match the server's stored information. The client's request will be ignored.
 
-% DHCP4_DHCID_COMPUTE_ERROR failed to compute the DHCID for lease: %1, reason: %2
-This error message is logged when the attempt to compute DHCID for a specified
-lease has failed. The lease details and reason for failure is logged in the
-message.
-
 % DHCP4_DISCOVER_CLASS_PROCESSING_FAILED %1: client class specific processing failed for DHCPDISCOVER
 This debug message means that the server processing that is unique for each
 client class has reported a failure. The response packet will not be sent.
@@ -528,11 +523,6 @@ A debug message printing the details of the received packet. The first
 argument includes the client and the transaction identification
 information.
 
-% DHCP4_QUEUE_NCR name change request to %1 DNS entry queued: %2
-A debug message which is logged when the NameChangeRequest to add or remove
-a DNS entries for a particular lease has been queued. The first parameter of
-this log message indicates whether the DNS entry is to be added or removed.
-The second parameter carries the details of the NameChangeRequest.
 
 % DHCP4_RELEASE %1: address %2 was released properly.
 This debug message indicates that an address was released properly. It

src/bin/dhcp4/dhcp4_srv.cc

@@ -33,6 +33,7 @@
 #include <dhcpsrv/cfg_subnets4.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/ncr_generator.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/subnet_selector.h>
 #include <dhcpsrv/utils.h>
@@ -1136,76 +1137,13 @@ Dhcpv4Srv::createNameChangeRequests(const Lease4Ptr& lease,
     if (!lease) {
         isc_throw(isc::Unexpected,
                   "NULL lease specified when creating NameChangeRequest");
-    }
-
-    // If old lease is not NULL, it is an indication that the lease has
-    // just been renewed. In such case we may need to generate the
-    // additional NameChangeRequest to remove an existing entry before
-    // we create a NameChangeRequest to add the entry for an updated lease.
-    // We may also decide not to generate any requests at all. This is when
-    // we discover that nothing has changed in the client's FQDN data.
-    if (old_lease) {
-        // There will be a NameChangeRequest generated to remove existing
-        // DNS entries if the following conditions are met:
-        // - The hostname is set for the existing lease, we can't generate
-        //   removal request for non-existent hostname.
-        // - A server has performed reverse, forward or both updates.
-        // - FQDN data between the new and old lease do not match.
-        if (!lease->hasIdenticalFqdn(*old_lease)) {
-            queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, old_lease);
-
-            // If FQDN data from both leases match, there is no need to update.
-        } else if (lease->hasIdenticalFqdn(*old_lease)) {
-            return;
-
-        }
-
-    }
-
-    // We may need to generate the NameChangeRequest for the new lease. It
-    // will be generated only if hostname is set and if forward or reverse
-    // update has been requested.
-    queueNameChangeRequest(isc::dhcp_ddns::CHG_ADD, lease);
-}
-
-void
-Dhcpv4Srv::
-queueNameChangeRequest(const isc::dhcp_ddns::NameChangeType chg_type,
-                       const Lease4Ptr& lease) {
-    // The hostname must not be empty, and at least one type of update
-    // should be requested.
-    if (!lease || lease->hostname_.empty() ||
-        (!lease->fqdn_rev_ && !lease->fqdn_fwd_)) {
-        return;
-    }
 
-    // Create the DHCID for the NameChangeRequest.
-    D2Dhcid dhcid;
-    try {
-        dhcid  = computeDhcid(lease);
-    } catch (const DhcidComputeError& ex) {
-        LOG_ERROR(ddns4_logger, DHCP4_DHCID_COMPUTE_ERROR)
-            .arg(lease->toText())
-            .arg(ex.what());
-        return;
+    } else if (!old_lease || (old_lease && !lease->hasIdenticalFqdn(*old_lease))) {
+        // We may need to generate the NameChangeRequest for the new lease. It
+        // will be generated only if hostname is set and if forward or reverse
+        // update has been requested.
+        queueNCR(CHG_ADD, lease);
     }
-
-    // Create NameChangeRequest
-    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(ddns4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_QUEUE_NCR)
-        .arg(chg_type == CHG_ADD ? "add" : "remove")
-        .arg(ncr->toText());
-
-    // And pass it to the the manager.
-    CfgMgr::instance().getD2ClientMgr().sendRequest(ncr);
 }
 
 void
@@ -1830,10 +1768,9 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
                     StatsMgr::generateName("subnet", lease->subnet_id_, "assigned-addresses"),
                     static_cast<int64_t>(-1));
 
-                if (CfgMgr::instance().ddnsEnabled()) {
-                    // Remove existing DNS entries for the lease, if any.
-                    queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, lease);
-                }
+                // Remove existing DNS entries for the lease, if any.
+                queueNCR(CHG_REMOVE, lease);
+
             } else {
                 // Release failed
                 LOG_ERROR(lease4_logger, DHCP4_RELEASE_FAIL)
@@ -1952,13 +1889,9 @@ Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const Pkt4Ptr& decline) {
         }
     }
 
-    // Clean up DDNS, if needed.
-    if (CfgMgr::instance().ddnsEnabled()) {
-        // Remove existing DNS entries for the lease, if any.
-        // queueNameChangeRequest will do the necessary checks and will
-        // skip the update, if not needed.
-        queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, lease);
-    }
+    // Remove existing DNS entries for the lease, if any.
+    // queueNCR will do the necessary checks and will skip the update, if not needed.
+    queueNCR(CHG_REMOVE, lease);
 
     // Bump up the statistics.
 

src/bin/dhcp4/dhcp4_srv.h

@@ -572,20 +572,6 @@ protected:
     void createNameChangeRequests(const Lease4Ptr& lease,
                                   const Lease4Ptr& old_lease);
 
-    /// @brief Creates the NameChangeRequest and adds to the queue for
-    /// processing.
-    ///
-    /// 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 kea-dhcp-ddns.
-    ///
-    /// @param chg_type A type of the NameChangeRequest (ADD or REMOVE).
-    /// @param lease A lease for which the NameChangeRequest is created and
-    /// queued.
-    void queueNameChangeRequest(const isc::dhcp_ddns::NameChangeType chg_type,
-                                const Lease4Ptr& lease);
-
     /// @brief Attempts to renew received addresses
     ///
     /// Attempts to renew existing lease. This typically includes finding a lease that

src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -745,58 +745,6 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsRenewNoChange) {
     ASSERT_EQ(0, d2_mgr_.getQueueSize());
 }
 
-// Test that no NameChangeRequest is generated when forward and reverse
-// DNS update flags are not set in the lease.
-TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsNoUpdate) {
-    Lease4Ptr lease1 = createLease(IOAddress("192.0.2.3"),
-                                   "lease1.example.com.",
-                                   true, true);
-    Lease4Ptr lease2 = createLease(IOAddress("192.0.2.3"),
-                                   "lease2.example.com.",
-                                   false, false);
-    ASSERT_NO_THROW(srv_->createNameChangeRequests(lease2, lease1));
-    EXPECT_EQ(1, d2_mgr_.getQueueSize());
-
-    verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
-                            "192.0.2.3", "lease1.example.com.",
-                            "0001013A5B311F5B9FB10DDF8E53689B874F25D"
-                            "62CC147C2FF237A64C90E5A597C9B7A",
-                            lease1->cltt_, 100);
-
-    lease2->hostname_ = "";
-    lease2->fqdn_rev_ = true;
-    lease2->fqdn_fwd_ = true;
-    ASSERT_NO_THROW(srv_->createNameChangeRequests(lease2, lease1));
-    EXPECT_EQ(1, d2_mgr_.getQueueSize());
-
-}
-
-// Test that two NameChangeRequests are generated when the lease is being
-// renewed and the new lease has updated FQDN data.
-TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsRenew) {
-    Lease4Ptr lease1 = createLease(IOAddress("192.0.2.3"),
-                                   "lease1.example.com.",
-                                   true, true);
-    Lease4Ptr lease2 = createLease(IOAddress("192.0.2.3"),
-                                   "lease2.example.com.",
-                                   true, true);
-    ASSERT_NO_THROW(srv_->createNameChangeRequests(lease2, lease1));
-    ASSERT_EQ(2, d2_mgr_.getQueueSize());
-
-    verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
-                            "192.0.2.3", "lease1.example.com.",
-                            "0001013A5B311F5B9FB10DDF8E53689B874F25D"
-                            "62CC147C2FF237A64C90E5A597C9B7A",
-                            lease1->cltt_, 100);
-
-    verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
-                            "192.0.2.3", "lease2.example.com.",
-                            "000101F906D2BB752E1B2EECC5FF2BF434C0B2D"
-                            "D6D7F7BD873F4F280165DB8C9DBA7CB",
-                            lease2->cltt_, 100);
-
-}
-
 // Test that the OFFER message generated as a result of the DISCOVER message
 // processing will not result in generation of the NameChangeRequests.
 TEST_F(NameDhcpv4SrvTest, processDiscover) {

src/bin/dhcp6/dhcp6_messages.mes

@@ -135,12 +135,6 @@ lease and other information.
 This debug message is logged when the new Name Change Request has been created
 to perform the DNS Update, which adds new RRs.
 
-% DHCP6_DDNS_CREATE_REMOVE_NAME_CHANGE_REQUEST %1: created name change request: %2
-This debug message is logged when the new Name Change Request has been created
-to perform the DNS Update, which removes RRs from the DNS. The first argument
-includes the client and transaction identification information. The second
-argument specifies the details of the generated name change request.
-
 % DHCP6_DDNS_FQDN_GENERATED %1: generated FQDN for the client: %2
 This debug message is logged when the server generated FQDN (name)
 for the client which message is processed. The names may be
@@ -179,13 +173,6 @@ sent by a client and started processing it. The first argument includes the
 client and transaction identification information. The second argument
 includes the received FQDN.
 
-% DHCP6_DDNS_REMOVE_INVALID_HOSTNAME %1: invalid FQDN %2 for the lease: %3 when removing DNS bindings
-This error message is issued when a lease being deleted contains an indication
-that the DNS Update has been performed for it, but the FQDN held in the lease
-database has invalid format and can't be transformed to the canonical on-wire
-format. The first argument includes the client and transaction identification
-information.
-
 % DHCP6_DDNS_REQUEST_SEND_FAILED failed sending a request to kea-dhcp-ddns, error: %1,  ncr: %2
 This error message indicates that IPv6 DHCP server failed to send a DDNS
 update request to the DHCP-DDNS server. This is most likely a configuration or
@@ -203,14 +190,6 @@ the lease is acquired for the client.
 This debug message is logged when server includes an DHCPv6 Client FQDN Option
 in its response to the client.
 
-% DHCP6_DDNS_SKIP_REMOVE_NAME_CHANGE_REQUEST %1: name change request creation skipped for lease: %2
-This debug message is logged when the server determines that removal
-name change request should not be sent to the DNS, because the DNS
-updates are disabled on the DHCP server, or no DNS update has been
-performed for the processed lease. The first argument includes the
-client and the transaction identification information. The second
-argument provides the details of the lease.
-
 % DHCP6_DEACTIVATE_INTERFACE deactivate interface %1
 This message is printed when DHCPv6 server disables an interface from being
 used to receive DHCPv6 traffic. Sockets on this interface will not be opened

src/bin/dhcp6/dhcp6_srv.cc

@@ -38,6 +38,7 @@
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/ncr_generator.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/subnet_selector.h>
 #include <dhcpsrv/utils.h>
@@ -1306,64 +1307,6 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer) {
     }
 }
 
-void
-Dhcpv6Srv::createRemovalNameChangeRequest(const Pkt6Ptr& query, const Lease6Ptr& lease) {
-    // Don't create NameChangeRequests if DNS updates are disabled
-    // or DNS update hasn't been performed.
-    if (!CfgMgr::instance().ddnsEnabled() || (!lease->fqdn_fwd_ && !lease->fqdn_rev_)) {
-        LOG_DEBUG(ddns6_logger, DBG_DHCP6_DETAIL_DATA,
-                  DHCP6_DDNS_SKIP_REMOVE_NAME_CHANGE_REQUEST)
-            .arg(query->getLabel())
-            .arg(lease->toText());
-        return;
-    }
-
-    // If hostname is non-empty, try to convert it to wire format so as
-    // DHCID can be computed from it. This may throw an exception if hostname
-    // has invalid format or is empty. Again, this should be only possible
-    // in case of manual intervention in the database. Note that the last
-    // parameter passed to the writeFqdn function forces conversion of the FQDN
-    // to lower case. This is required by the RFC4701, section 3.5.
-    // The DHCID computation is further in this function.
-    std::vector<uint8_t> hostname_wire;
-    try {
-        OptionDataTypeUtil::writeFqdn(lease->hostname_, hostname_wire, true);
-
-    } catch (const Exception& ex) {
-        LOG_ERROR(ddns6_logger, DHCP6_DDNS_REMOVE_INVALID_HOSTNAME)
-            .arg(query->getLabel())
-            .arg(lease->hostname_.empty() ? "(empty)" : lease->hostname_)
-            .arg(lease->addr_.toText());
-        return;
-    }
-
-    // DUID must have been checked already  by the caller of this function.
-    // Let's be on the safe side and make sure it is non-NULL and throw
-    // an exception if it is NULL.
-    if (!lease->duid_) {
-        isc_throw(isc::Unexpected, "DUID must be set when creating"
-                  << " NameChangeRequest for DNS records removal for "
-                  << lease->addr_);
-
-    }
-    isc::dhcp_ddns::D2Dhcid dhcid(*lease->duid_, hostname_wire);
-    // Create a NameChangeRequest to remove the entry.
-    NameChangeRequestPtr ncr;
-    ncr.reset(new NameChangeRequest(isc::dhcp_ddns::CHG_REMOVE,
-                                    lease->fqdn_fwd_, lease->fqdn_rev_,
-                                    lease->hostname_,
-                                    lease->addr_.toText(),
-                                    dhcid, 0, lease->valid_lft_));
-
-    LOG_DEBUG(ddns6_logger, DBG_DHCP6_DETAIL,
-              DHCP6_DDNS_CREATE_REMOVE_NAME_CHANGE_REQUEST)
-        .arg(query->getLabel())
-        .arg(ncr->toText());
-
-    // Post the NCR to the D2ClientMgr.
-    CfgMgr::instance().getD2ClientMgr().sendRequest(ncr);
-}
-
 HWAddrPtr
 Dhcpv6Srv::getMAC(const Pkt6Ptr& pkt) {
     CfgMACSources mac_sources = CfgMgr::instance().getCurrentCfg()->
@@ -1504,17 +1447,17 @@ Dhcpv6Srv::assignIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
                 // have to check that the FQDN settings we provided are the same
                 // that were set. If they aren't, we will have to remove existing
                 // DNS records and update the lease with the new settings.
-                conditionalNCRRemoval(query, old_lease, lease, ctx.hostname_,
-                                      do_fwd, do_rev);
+//                conditionalNCRRemoval(query, old_lease, lease, ctx.hostname_,
+ //                                     do_fwd, do_rev);
             }
 
             // We need to repeat that check for leases that used to be used, but
             // are no longer valid.
-            if (!ctx.old_leases_.empty()) {
+/*            if (!ctx.old_leases_.empty()) {
                 old_lease = *ctx.old_leases_.begin();
                 conditionalNCRRemoval(query, old_lease, lease, ctx.hostname_,
                                       do_fwd, do_rev);
-            }
+            } */
         }
     } else {
         // Allocation engine did not allocate a lease. The engine logged
@@ -1547,7 +1490,7 @@ Dhcpv6Srv::conditionalNCRRemoval(const Pkt6Ptr& query, Lease6Ptr& old_lease,
             .arg(do_fwd ? "true" : "false");
 
         // Schedule removal of the existing lease.
-        createRemovalNameChangeRequest(query, old_lease);
+        queueNCR(CHG_REMOVE, old_lease);
     }
 }
 
@@ -1784,7 +1727,7 @@ Dhcpv6Srv::extendIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
                 .arg(do_rev ? "true" : "false")
                 .arg(do_fwd ? "true" : "false");
 
-            createRemovalNameChangeRequest(query, *l);
+            queueNCR(CHG_REMOVE, *l);
         }
     }
 
@@ -2199,7 +2142,7 @@ Dhcpv6Srv::releaseIA_NA(const DuidPtr& duid, const Pkt6Ptr& query,
         // Check if a lease has flags indicating that the FQDN update has
         // been performed. If so, create NameChangeRequest which removes
         // the entries.
-        createRemovalNameChangeRequest(query, lease);
+        queueNCR(CHG_REMOVE, lease);
 
         return (ia_rsp);
     }
@@ -2770,7 +2713,7 @@ Dhcpv6Srv::declineLease(const Pkt6Ptr& decline, const Lease6Ptr lease,
     // Check if a lease has flags indicating that the FQDN update has
     // been performed. If so, create NameChangeRequest which removes
     // the entries. This method does all necessary checks.
-    createRemovalNameChangeRequest(decline, lease);
+    queueNCR(CHG_REMOVE, lease);
 
     // Bump up the subnet-specific statistic.
     StatsMgr::instance().addValue(

src/bin/dhcp6/dhcp6_srv.h

@@ -528,9 +528,8 @@ protected:
     /// server intents to perform for the DNS client. Based on this, the
     /// function will create zero or more @c isc::dhcp_ddns::NameChangeRequest
     /// objects and store them in the internal queue. Requests created by this
-    /// function are only adding or updating DNS records. In order to generate
-    /// requests for DNS records removal, use @c createRemovalNameChangeRequest.
-    /// If ddns updates are disabled, this method returns immediately.
+    /// function are only adding or updating DNS records. If DNS updates are
+    /// disabled, this method returns immediately.
     ///
     /// @todo Add support for multiple IAADDR options in the IA_NA.
     ///
@@ -538,25 +537,6 @@ protected:
     /// Client FQDN option, this option is used to create NameChangeRequests.
     void createNameChangeRequests(const Pkt6Ptr& answer);
 
-    /// @brief Creates a @c isc::dhcp_ddns::NameChangeRequest which requests
-    /// removal of DNS entries for a particular lease.
-    ///
-    /// This function should be called upon removal of the lease from the lease
-    /// database, i.e, when client sent Release or Decline message. It will
-    /// create a single @c isc::dhcp_ddns::NameChangeRequest which removes the
-    /// existing DNS records for the lease, which server is responsible for.
-    /// Note that this function will not remove the entries which server hadn't
-    /// added. This is the case, when client performs forward DNS update on its
-    /// own.
-    /// If ddns updates are disabled, this method returns immediately.
-    ///
-    /// @param query A pointer to the packet sent by the client for which the
-    /// name change request should be sent.
-    /// @param lease A lease for which the the removal of corresponding DNS
-    /// records will be performed.
-    void createRemovalNameChangeRequest(const Pkt6Ptr& query,
-                                        const Lease6Ptr& lease);
-
     /// @brief Attempts to extend the lifetime of IAs.
     ///
     /// This function is called when a client sends Renew or Rebind message.

src/bin/dhcp6/tests/dhcp6_test_utils.h

@@ -105,7 +105,6 @@ public:
     using Dhcpv6Srv::processInfRequest;
     using Dhcpv6Srv::processClientFqdn;
     using Dhcpv6Srv::createNameChangeRequests;
-    using Dhcpv6Srv::createRemovalNameChangeRequest;
     using Dhcpv6Srv::selectSubnet;
     using Dhcpv6Srv::testServerID;
     using Dhcpv6Srv::testUnicast;

src/bin/dhcp6/tests/fqdn_unittest.cc

@@ -24,8 +24,10 @@
 #include <dhcp/option6_iaaddr.h>
 #include <dhcp/option6_status_code.h>
 #include <dhcp/option_int_array.h>
-#include <dhcpsrv/lease.h>
 #include <dhcp/tests/iface_mgr_test_config.h>
+#include <dhcpsrv/lease.h>
+#include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/ncr_generator.h>
 
 #include <dhcp6/tests/dhcp6_test_utils.h>
 #include <boost/pointer_cast.hpp>
@@ -485,7 +487,7 @@ public:
                                  const bool reverse, const bool forward,
                                  const std::string& addr,
                                  const std::string& dhcid,
-                                 const uint16_t expires,
+                                 const uint64_t expires,
                                  const uint16_t len,
                                  const std::string& fqdn="") {
         NameChangeRequestPtr ncr;
@@ -725,15 +727,14 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestFwdRev) {
     // as if we typed domain-name in lower case.
     lease_->hostname_ = "MYHOST.example.com.";
 
-    Pkt6Ptr pkt(new Pkt6(DHCPREQUEST, 1234));
-    ASSERT_NO_THROW(srv_->createRemovalNameChangeRequest(pkt, lease_));
+    ASSERT_NO_THROW(queueNCR(CHG_REMOVE, lease_));
 
     ASSERT_EQ(1, d2_mgr_.getQueueSize());
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
                             "2001:db8:1::1",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            0, 502);
+                            lease_->cltt_ + lease_->valid_lft_, 502);
 
 }
 
@@ -751,8 +752,7 @@ TEST_F(FqdnDhcpv6SrvTest, noRemovalsWhenDisabled) {
     lease_->hostname_ = "MYHOST.example.com.";
 
     // When DDNS is disabled an attempt to send a request will throw.
-    Pkt6Ptr pkt(new Pkt6(DHCPREQUEST, 1234));
-    ASSERT_NO_THROW(srv_->createRemovalNameChangeRequest(pkt, lease_));
+    ASSERT_NO_THROW(queueNCR(CHG_REMOVE, lease_));
 }
 
 
@@ -763,8 +763,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestRev) {
     lease_->fqdn_rev_ = true;
     lease_->hostname_ = "myhost.example.com.";
 
-    Pkt6Ptr pkt(new Pkt6(DHCPREQUEST, 1234));
-    ASSERT_NO_THROW(srv_->createRemovalNameChangeRequest(pkt, lease_));
+    ASSERT_NO_THROW(queueNCR(CHG_REMOVE, lease_));
 
     ASSERT_EQ(1, d2_mgr_.getQueueSize());
 
@@ -772,7 +771,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestRev) {
                             "2001:db8:1::1",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            0, 502);
+                            lease_->cltt_ + lease_->valid_lft_, 502);
 
 }
 
@@ -782,8 +781,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestNoUpdate) {
     lease_->fqdn_fwd_ = false;
     lease_->fqdn_rev_ = false;
 
-    Pkt6Ptr pkt(new Pkt6(DHCPREQUEST, 1234));
-    ASSERT_NO_THROW(srv_->createRemovalNameChangeRequest(pkt, lease_));
+    ASSERT_NO_THROW(queueNCR(CHG_REMOVE, lease_));
 
     ASSERT_EQ(0, d2_mgr_.getQueueSize());
 
@@ -797,7 +795,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestNoHostname) {
     lease_->hostname_ = "";
 
     Pkt6Ptr pkt(new Pkt6(DHCPREQUEST, 1234));
-    ASSERT_NO_THROW(srv_->createRemovalNameChangeRequest(pkt, lease_));
+    ASSERT_NO_THROW(queueNCR(CHG_REMOVE, lease_));
 
     ASSERT_EQ(0, d2_mgr_.getQueueSize());
 
@@ -811,8 +809,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestWrongHostname) {
     lease_->fqdn_rev_ = true;
     lease_->hostname_ = "myhost..example.com.";
 
-    Pkt6Ptr pkt(new Pkt6(DHCPREQUEST, 1234));
-    ASSERT_NO_THROW(srv_->createRemovalNameChangeRequest(pkt, lease_));
+    ASSERT_NO_THROW(queueNCR(CHG_REMOVE, lease_));
 
     ASSERT_EQ(0, d2_mgr_.getQueueSize());
 
@@ -845,7 +842,7 @@ TEST_F(FqdnDhcpv6SrvTest, DISABLED_processTwoRequests) {
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            0, 4000);
+                            lease_->cltt_ + lease_->valid_lft_, 4000);
 
     // Client may send another request message with a new domain-name. In this
     // case the same lease will be returned. The existing DNS entry needs to
@@ -950,6 +947,12 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRelease) {
     // to add both reverse and forward mapping to DNS.
     testProcessMessage(DHCPV6_REQUEST, "myhost.example.com",
                        "myhost.example.com.");
+
+    // The lease should have been recorded in the database.
+    lease_ = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA,
+                                                   IOAddress("2001:db8:1:1::dead:beef"));
+    ASSERT_TRUE(lease_);
+
     ASSERT_EQ(1, d2_mgr_.getQueueSize());
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             "2001:db8:1:1::dead:beef",
@@ -968,7 +971,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRelease) {
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            0, 4000);
+                            lease_->cltt_ + lease_->valid_lft_, 4000);
 
 }
 
@@ -1028,15 +1031,16 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestReuseExpiredLease) {
                        "myhost.example.com.");
     // Test that the appropriate NameChangeRequest has been generated.
     ASSERT_EQ(1, d2_mgr_.getQueueSize());
-    verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
-                            "2001:db8:1:1::dead:beef",
-                            "000201415AA33D1187D148275136FA30300478"
-                            "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            0, 4);
-    // Get the lease acquired and modify it. In particular, expire it.
+
+    // Get the lease acquired.
     Lease6Ptr lease =
         LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, addr);
     ASSERT_TRUE(lease);
+
+    verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+                            "2001:db8:1:1::dead:beef",
+                            "000201415AA33D1187D148275136FA30300478"
+                            "FAAAA3EBD29826B5C907B2C9268A6F52", 0, 4);
     // One of the following: IAID, DUID or subnet identifier has to be changed
     // because otherwise the allocation engine will treat the lease as
     // being renewed by the same client. If we at least change subnet identifier
@@ -1066,14 +1070,13 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestReuseExpiredLease) {
                             "2001:db8:1:1::dead:beef",
                             "000201D422AA463306223D269B6CB7AFE7AAD2"
                             "65FCEA97F93623019B2E0D14E5323D5A",
-                            0, 5);
+                            lease->cltt_ + lease->valid_lft_, 5);
     // The second name change request should add a DNS mapping for
     // a new lease.
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
-                            "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            0, 4);
+                            "FAAAA3EBD29826B5C907B2C9268A6F52", 0, 4);
 
 }
 

src/lib/dhcpsrv/Makefile.am

@@ -114,6 +114,9 @@ if HAVE_MYSQL
 libkea_dhcpsrv_la_SOURCES += mysql_lease_mgr.cc mysql_lease_mgr.h
 libkea_dhcpsrv_la_SOURCES += mysql_connection.cc mysql_connection.h
 endif
+
+libkea_dhcpsrv_la_SOURCES += ncr_generator.cc ncr_generator.h
+
 if HAVE_PGSQL
 libkea_dhcpsrv_la_SOURCES += pgsql_lease_mgr.cc pgsql_lease_mgr.h
 endif

src/lib/dhcpsrv/alloc_engine.cc

@@ -14,16 +14,14 @@
 
 #include <config.h>
 
-#include <dhcp/option_data_types.h>
 #include <dhcp_ddns/ncr_msg.h>
 #include <dhcpsrv/alloc_engine.h>
 #include <dhcpsrv/alloc_engine_log.h>
-#include <dhcpsrv/cfgmgr.h>
-#include <dhcpsrv/d2_client_mgr.h>
 #include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/host_mgr.h>
 #include <dhcpsrv/host.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/ncr_generator.h>
 #include <dhcp/dhcp6.h>
 #include <hooks/callout_handle.h>
 #include <hooks/hooks_manager.h>
@@ -831,6 +829,9 @@ AllocEngine::removeNonmatchingReservedLeases6(ClientContext6& ctx,
         // Remove this lease from LeaseMgr
         LeaseMgrFactory::instance().deleteLease((*candidate)->addr_);
 
+        // Update DNS if needed.
+        queueNCR(CHG_REMOVE, *candidate);
+
         // Need to decrease statistic for assigned addresses.
         StatsMgr::instance().addValue(
             StatsMgr::generateName("subnet", ctx.subnet_->getID(),
@@ -894,6 +895,9 @@ AllocEngine::removeNonreservedLeases6(ClientContext6& ctx,
             // Remove this lease from LeaseMgr
             LeaseMgrFactory::instance().deleteLease((*lease)->addr_);
 
+            // Update DNS if required.
+            queueNCR(CHG_REMOVE, *lease);
+
             // Need to decrease statistic for assigned addresses.
             StatsMgr::instance().addValue(
                 StatsMgr::generateName("subnet", ctx.subnet_->getID(),
@@ -937,15 +941,11 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx,
         prefix_len = 128; // non-PD lease types must be always /128
     }
 
-    if ( (expired->state_ == Lease::STATE_DECLINED) &&
-         (ctx.fake_allocation_ == false)) {
-        // If this is a declined lease that expired, we need to conduct
-        // extra steps for it. However, we do want to conduct those steps
-        // only once. In particular, if we have an expired declined lease
-        // and client sent DHCPDISCOVER and will later send DHCPREQUEST,
-        // we only want to call this method once when responding to
-        // DHCPREQUEST (when the actual reclaimation takes place).
-        reclaimDeclined(expired);
+    if (!ctx.fake_allocation_) {
+        // The expired lease needs to be reclaimed before it can be reused.
+        // This includes declined leases for which probation period has
+        // elapsed.
+        reclaimExpiredLease(expired, ctx.callout_handle_);
     }
 
     // address, lease type and prefixlen (0) stay the same
@@ -961,6 +961,7 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx,
     expired->fqdn_fwd_ = ctx.fwd_dns_update_;
     expired->fqdn_rev_ = ctx.rev_dns_update_;
     expired->prefixlen_ = prefix_len;
+    expired->state_ = Lease::STATE_DEFAULT;
 
     LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE_DETAIL_DATA,
               ALLOC_ENGINE_V6_REUSE_EXPIRED_LEASE_DATA)
@@ -1194,6 +1195,9 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
         // Remove this lease from LeaseMgr
         LeaseMgrFactory::instance().deleteLease(lease->addr_);
 
+        // Updated DNS if required.
+        queueNCR(CHG_REMOVE, lease);
+
         // Need to decrease statistic for assigned addresses.
         StatsMgr::instance().addValue(
             StatsMgr::generateName("subnet", ctx.subnet_->getID(), "assigned-nas"),
@@ -1211,7 +1215,7 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
         .arg(lease->toText());
 
     // Keep the old data in case the callout tells us to skip update.
-    Lease6 old_data = *lease;
+    Lease6Ptr old_data(new Lease6(*lease));
 
     lease->preferred_lft_ = ctx.subnet_->getPreferred();
     lease->valid_lft_ = ctx.subnet_->getValid();
@@ -1221,6 +1225,7 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
     lease->fqdn_fwd_ = ctx.fwd_dns_update_;
     lease->fqdn_rev_ = ctx.rev_dns_update_;
     lease->hwaddr_ = ctx.hwaddr_;
+    lease->state_ = Lease::STATE_DEFAULT;
 
     // Extend lease lifetime if it is time to extend it.
     conditionalExtendLifetime(*lease);
@@ -1270,13 +1275,26 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
     }
 
     if (!skip) {
+        // If the lease we're renewing has expired, we need to reclaim this
+        // lease before we can renew it.
+        if (old_data->expired()) {
+            reclaimExpiredLease(old_data, ctx.callout_handle_);
+
+        } else  if (!lease->hasIdenticalFqdn(*old_data)) {
+            // We're not reclaiming the lease but since the FQDN has changed
+            // we have to at least send NCR.
+            queueNCR(CHG_REMOVE, old_data);
+        }
+        // Now that the lease has been reclaimed, we can go ahead and update it
+        // in the lease database.
         LeaseMgrFactory::instance().updateLease6(lease);
+
     } else {
         // Copy back the original date to the lease. For MySQL it doesn't make
         // much sense, but for memfile, the Lease6Ptr points to the actual lease
         // in memfile, so the actual update is performed when we manipulate
         // fields of returned Lease6Ptr, the actual updateLease6() is no-op.
-        *lease = old_data;
+        *lease = *old_data;
     }
 }
 
@@ -1356,87 +1374,10 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
     BOOST_FOREACH(Lease6Ptr lease, leases) {
 
         try {
-            // The skip flag indicates if the callouts have taken responsibility
-            // for reclaiming the lease. The callout will set this to true if
-            // it reclaims the lease itself. In this case the reclamation routine
-            // will not update DNS nor update the database.
-            bool skipped = false;
-            if (callout_handle) {
-                callout_handle->deleteAllArguments();
-                callout_handle->setArgument("lease6", lease);
-                callout_handle->setArgument("remove_lease", remove_lease);
-
-                HooksManager::callCallouts(Hooks.hook_index_lease6_expire_,
-                                           *callout_handle);
-
-                skipped = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
-            }
-
-            /// @todo: Maybe add support for DROP stauts?
-            /// Not sure if we need to support every possible status everywhere.
-
-            if (!skipped) {
-                // Generate removal name change request for D2, if required.
-                // This will return immediatelly if the DNS wasn't updated
-                // when the lease was created.
-                if (lease->duid_) {
-                    queueRemovalNameChangeRequest(lease, *(lease->duid_));
-                }
-
-                // Let's check if the lease that just expired is in DECLINED state.
-                // If it is, we need to conduct couple extra steps and also force
-                // its removal.
-                bool remove_tmp = remove_lease;
-                if (lease->state_ == Lease::STATE_DECLINED) {
-                    // There's no point in keeping declined lease after its
-                    // reclaimation. Declined lease doesn't have any client
-                    // identifying information anymore.
-                    remove_tmp = true;
-
-                    // Do extra steps required for declined lease reclaimation:
-                    // - bump decline-related stats
-                    // - log separate message
-                    reclaimDeclined(lease);
-                }
-
-                // Reclaim the lease - depending on the configuration, set the
-                // expired-reclaimed state or simply remove it.
-                reclaimLeaseInDatabase<Lease6Ptr>(lease, remove_tmp,
-                                                  boost::bind(&LeaseMgr::updateLease6,
-                                                              &lease_mgr, _1));
-            }
-
+            // Reclaim the lease.
+            reclaimExpiredLease(lease, remove_lease, callout_handle);
             ++leases_processed;
 
-            // Update statistics.
-
-            // Decrease number of assigned leases.
-            if (lease->type_ == Lease::TYPE_NA) {
-                // IA_NA
-                StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
-                                                                     lease->subnet_id_,
-                                                                     "assigned-nas"),
-                                              int64_t(-1));
-
-            } else if (lease->type_ == Lease::TYPE_PD) {
-                // IA_PD
-                StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
-                                                                     lease->subnet_id_,
-                                                                     "assigned-pds"),
-                                              int64_t(-1));
-
-            }
-
-            // Increase total number of reclaimed leases.
-            StatsMgr::instance().addValue("reclaimed-leases", int64_t(1));
-
-            // Increase number of reclaimed leases for a subnet.
-            StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
-                                                                 lease->subnet_id_,
-                                                                 "reclaimed-leases"),
-                                          int64_t(1));
-
-
         } catch (const std::exception& ex) {
             LOG_ERROR(alloc_engine_logger, ALLOC_ENGINE_V6_LEASE_RECLAMATION_FAILED)
                 .arg(lease->addr_.toText())
@@ -1573,81 +1514,10 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
     BOOST_FOREACH(Lease4Ptr lease, leases) {
 
         try {
-            // The skip flag indicates if the callouts have taken responsibility
-            // for reclaiming the lease. The callout will set this to true if
-            // it reclaims the lease itself. In this case the reclamation routine
-            // will not update DNS nor update the database.
-            bool skipped = false;
-            if (callout_handle) {
-                callout_handle->deleteAllArguments();
-                callout_handle->setArgument("lease4", lease);
-                callout_handle->setArgument("remove_lease", remove_lease);
-
-                HooksManager::callCallouts(Hooks.hook_index_lease4_expire_,
-                                           *callout_handle);
-
-                skipped = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
-            }
-
-            /// @todo: Maybe add support for DROP stauts?
-            /// Not sure if we need to support every possible status everywhere.
-
-            if (!skipped) {
-                // Generate removal name change request for D2, if required.
-                // This will return immediatelly if the DNS wasn't updated
-                // when the lease was created.
-                if (lease->client_id_) {
-                    // Client id takes precedence over HW address.
-                    queueRemovalNameChangeRequest(lease, lease->client_id_->getClientId());
-
-                } else {
-                    // Client id is not specified for the lease. Use HW address
-                    // instead.
-                    queueRemovalNameChangeRequest(lease, lease->hwaddr_);
-                }
-
-                // Let's check if the lease that just expired is in DECLINED state.
-                // If it is, we need to conduct couple extra steps and also force
-                // its removal.
-                bool remove_tmp = remove_lease;
-                if (lease->state_ == Lease::STATE_DECLINED) {
-                    // There's no point in keeping declined lease after its
-                    // reclaimation. Declined lease doesn't have any client
-                    // identifying information anymore.
-                    remove_tmp = true;
-
-                    // Do extra steps required for declined lease reclaimation:
-                    // - bump decline-related stats
-                    // - log separate message
-                    reclaimDeclined(lease);
-                }
-
-                // Reclaim the lease - depending on the configuration, set the
-                // expired-reclaimed state or simply remove it.
-                reclaimLeaseInDatabase<Lease4Ptr>(lease, remove_tmp,
-                                                  boost::bind(&LeaseMgr::updateLease4,
-                                                              &lease_mgr, _1));
-            }
-
+            // Reclaim the lease.
+            reclaimExpiredLease(lease, remove_lease, callout_handle);
             ++leases_processed;
 
-            // Update statistics.
-
-            // Decrease number of assigned addresses.
-            StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
-                                                                 lease->subnet_id_,
-                                                                 "assigned-addresses"),
-                                          int64_t(-1));
-
-            // Increase total number of reclaimed leases.
-            StatsMgr::instance().addValue("reclaimed-leases", int64_t(1));
-
-            // Increase number of reclaimed leases for a subnet.
-            StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
-                                                                 lease->subnet_id_,
-                                                                 "reclaimed-leases"),
-                                          int64_t(1));
-
         } catch (const std::exception& ex) {
             LOG_ERROR(alloc_engine_logger, ALLOC_ENGINE_V4_LEASE_RECLAMATION_FAILED)
                 .arg(lease->addr_.toText())
@@ -1706,6 +1576,188 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
     }
 }
 
+template<typename LeasePtrType>
+void
+AllocEngine::reclaimExpiredLease(const LeasePtrType& lease, const bool remove_lease,
+                                 const CalloutHandlePtr& callout_handle) {
+    reclaimExpiredLease(lease, remove_lease ? DB_RECLAIM_REMOVE : DB_RECLAIM_UPDATE,
+                        callout_handle);
+}
+
+template<typename LeasePtrType>
+void
+AllocEngine::reclaimExpiredLease(const LeasePtrType& lease,
+                                 const CalloutHandlePtr& callout_handle) {
+    // This variant of the method is used by the code which allocates or
+    // renews leases. It may be the case that the lease has already been
+    // reclaimed, so there is nothing to do.
+    if (!lease->stateExpiredReclaimed()) {
+        reclaimExpiredLease(lease, DB_RECLAIM_LEAVE_UNCHANGED, callout_handle);
+    }
+}
+
+void
+AllocEngine::reclaimExpiredLease(const Lease6Ptr& lease,
+                                 const DbReclaimMode& reclaim_mode,
+                                 const CalloutHandlePtr& callout_handle) {
+    // The skip flag indicates if the callouts have taken responsibility
+    // for reclaiming the lease. The callout will set this to true if
+    // it reclaims the lease itself. In this case the reclamation routine
+    // will not update DNS nor update the database.
+    bool skipped = false;
+    if (callout_handle) {
+        callout_handle->deleteAllArguments();
+        callout_handle->setArgument("lease6", lease);
+        callout_handle->setArgument("remove_lease", reclaim_mode == DB_RECLAIM_REMOVE);
+
+        HooksManager::callCallouts(Hooks.hook_index_lease6_expire_,
+                                   *callout_handle);
+
+        skipped = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
+    }
+
+    /// @todo: Maybe add support for DROP stauts?
+    /// Not sure if we need to support every possible status everywhere.
+
+    if (!skipped) {
+
+        // Generate removal name change request for D2, if required.
+        // This will return immediatelly if the DNS wasn't updated
+        // when the lease was created.
+        queueNCR(CHG_REMOVE, lease);
+
+        // Let's check if the lease that just expired is in DECLINED state.
+        // If it is, we need to conduct couple extra steps and also force
+        // its removal.
+        bool remove_tmp = (reclaim_mode == DB_RECLAIM_REMOVE);
+        if (lease->state_ == Lease::STATE_DECLINED) {
+            // There's no point in keeping declined lease after its
+            // reclaimation. Declined lease doesn't have any client
+            // identifying information anymore.
+            if (reclaim_mode != DB_RECLAIM_LEAVE_UNCHANGED) {
+                remove_tmp = true;
+            }
+
+            // Do extra steps required for declined lease reclaimation:
+            // - bump decline-related stats
+            // - log separate message
+            reclaimDeclined(lease);
+        }
+
+        if (reclaim_mode != DB_RECLAIM_LEAVE_UNCHANGED) {
+            // Reclaim the lease - depending on the configuration, set the
+            // expired-reclaimed state or simply remove it.
+            LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
+            reclaimLeaseInDatabase<Lease6Ptr>(lease, remove_tmp,
+                                              boost::bind(&LeaseMgr::updateLease6,
+                                                          &lease_mgr, _1));
+        }
+    }
+
+    // Update statistics.
+
+    // Decrease number of assigned leases.
+    if (lease->type_ == Lease::TYPE_NA) {
+        // IA_NA
+        StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
+                                                             lease->subnet_id_,
+                                                             "assigned-nas"),
+                                      int64_t(-1));
+
+    } else if (lease->type_ == Lease::TYPE_PD) {
+        // IA_PD
+        StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
+                                                             lease->subnet_id_,
+                                                             "assigned-pds"),
+                                      int64_t(-1));
+
+    }
+
+    // Increase total number of reclaimed leases.
+    StatsMgr::instance().addValue("reclaimed-leases", int64_t(1));
+
+    // Increase number of reclaimed leases for a subnet.
+    StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
+                                                         lease->subnet_id_,
+                                                         "reclaimed-leases"),
+                                  int64_t(1));
+}
+
+void
+AllocEngine::reclaimExpiredLease(const Lease4Ptr& lease,
+                                 const DbReclaimMode& reclaim_mode,
+                                 const CalloutHandlePtr& callout_handle) {
+
+    // The skip flag indicates if the callouts have taken responsibility
+    // for reclaiming the lease. The callout will set this to true if
+    // it reclaims the lease itself. In this case the reclamation routine
+    // will not update DNS nor update the database.
+    bool skipped = false;
+    if (callout_handle) {
+        callout_handle->deleteAllArguments();
+        callout_handle->setArgument("lease4", lease);
+        callout_handle->setArgument("remove_lease", reclaim_mode == DB_RECLAIM_REMOVE);
+
+        HooksManager::callCallouts(Hooks.hook_index_lease4_expire_,
+                                   *callout_handle);
+
+        skipped = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
+    }
+
+    /// @todo: Maybe add support for DROP stauts?
+    /// Not sure if we need to support every possible status everywhere.
+
+    if (!skipped) {
+
+        // Generate removal name change request for D2, if required.
+        // This will return immediatelly if the DNS wasn't updated
+        // when the lease was created.
+        queueNCR(CHG_REMOVE, lease);
+
+        // Let's check if the lease that just expired is in DECLINED state.
+        // If it is, we need to conduct couple extra steps and also force
+        // its removal.
+        bool remove_tmp = (reclaim_mode == DB_RECLAIM_REMOVE);
+        if (lease->state_ == Lease::STATE_DECLINED) {
+            // There's no point in keeping declined lease after its
+            // reclaimation. Declined lease doesn't have any client
+            // identifying information anymore.
+            if (reclaim_mode != DB_RECLAIM_LEAVE_UNCHANGED) {
+                remove_tmp = true;
+            }
+
+            // Do extra steps required for declined lease reclaimation:
+            // - bump decline-related stats
+            // - log separate message
+            reclaimDeclined(lease);
+        }
+
+        if (reclaim_mode != DB_RECLAIM_LEAVE_UNCHANGED) {
+            // Reclaim the lease - depending on the configuration, set the
+            // expired-reclaimed state or simply remove it.
+            LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
+            reclaimLeaseInDatabase<Lease4Ptr>(lease, remove_tmp,
+                                              boost::bind(&LeaseMgr::updateLease4,
+                                                          &lease_mgr, _1));
+        }
+    }
+
+    // Decrease number of assigned addresses.
+    StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
+                                                         lease->subnet_id_,
+                                                         "assigned-addresses"),
+                                  int64_t(-1));
+
+    // Increase total number of reclaimed leases.
+    StatsMgr::instance().addValue("reclaimed-leases", int64_t(1));
+
+    // Increase number of reclaimed leases for a subnet.
+    StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
+                                                         lease->subnet_id_,
+                                                         "reclaimed-leases"),
+                                  int64_t(1));
+}
+
 void
 AllocEngine::deleteExpiredReclaimedLeases4(const uint32_t secs) {
     LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
@@ -1790,44 +1842,13 @@ AllocEngine::reclaimDeclined(const Lease6Ptr& lease) {
     /// @todo: call lease6_decline_recycle hook here.
 }
 
-template<typename LeasePtrType, typename IdentifierType>
-void
-AllocEngine::queueRemovalNameChangeRequest(const LeasePtrType& lease,
-                                           const IdentifierType& identifier) const {
-
-    // Check if there is a need for update.
-    if (!lease || lease->hostname_.empty() || (!lease->fqdn_fwd_ && !lease->fqdn_rev_)
-        || !CfgMgr::instance().getD2ClientMgr().ddnsEnabled()) {
-        return;
-    }
-
-    try {
-        // Create DHCID
-        std::vector<uint8_t> hostname_wire;
-        OptionDataTypeUtil::writeFqdn(lease->hostname_, hostname_wire, true);
-        dhcp_ddns::D2Dhcid dhcid = D2Dhcid(identifier, hostname_wire);
-
-        // Create name change request.
-        NameChangeRequestPtr ncr(new NameChangeRequest(isc::dhcp_ddns::CHG_REMOVE,
-                                                       lease->fqdn_fwd_, lease->fqdn_rev_,
-                                                       lease->hostname_,
-                                                       lease->addr_.toText(),
-                                                       dhcid, 0, lease->valid_lft_));
-        // Send name change request.
-        CfgMgr::instance().getD2ClientMgr().sendRequest(ncr);
-
-    } catch (const std::exception& ex) {
-        LOG_ERROR(alloc_engine_logger, ALLOC_ENGINE_REMOVAL_NCR_FAILED)
-            .arg(lease->addr_.toText())
-            .arg(ex.what());
-    }
-}
 
 template<typename LeasePtrType>
 void AllocEngine::reclaimLeaseInDatabase(const LeasePtrType& lease,
                                          const bool remove_lease,
                                          const boost::function<void (const LeasePtrType&)>&
                                          lease_update_fun) const {
+
     LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
 
     // Reclaim the lease - depending on the configuration, set the
@@ -1835,7 +1856,7 @@ void AllocEngine::reclaimLeaseInDatabase(const LeasePtrType& lease,
     if (remove_lease) {
         lease_mgr.deleteLease(lease->addr_);
 
-    } else {
+    } else if (!lease_update_fun.empty()) {
         // Clear FQDN information as we have already sent the
         // name change request to remove the DNS record.
         lease->hostname_.clear();
@@ -1843,6 +1864,9 @@ void AllocEngine::reclaimLeaseInDatabase(const LeasePtrType& lease,
         lease->fqdn_rev_ = false;
         lease->state_ = Lease::STATE_EXPIRED_RECLAIMED;
         lease_update_fun(lease);
+
+    } else {
+        return;
     }
 
     // Lease has been reclaimed.
@@ -2430,6 +2454,21 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
     // Update the lease with the information from the context.
     updateLease4Information(lease, ctx);
 
+    if (!ctx.fake_allocation_) {
+        // If the lease is expired we have to reclaim it before
+        // re-assigning it to the client. The lease reclamation
+        // involves execution of hooks and DNS update.
+        if (ctx.old_lease_->expired()) {
+            reclaimExpiredLease(ctx.old_lease_, ctx.callout_handle_);
+            lease->state_ = Lease::STATE_DEFAULT;
+
+        } else if (!lease->hasIdenticalFqdn(*ctx.old_lease_)) {
+            // The lease is not expired but the FQDN information has
+            // changed. So, we have to remove the previous DNS entry.
+            queueNCR(CHG_REMOVE, ctx.old_lease_);
+        }
+    }
+
     bool skip = false;
     // Execute all callouts registered for lease4_renew.
     if (HooksManager::getHooksManager().
@@ -2492,15 +2531,12 @@ AllocEngine::reuseExpiredLease4(Lease4Ptr& expired,
         isc_throw(BadValue, "null subnet specified for the reuseExpiredLease");
     }
 
-    if ( (expired->state_ == Lease::STATE_DECLINED) &&
-         (ctx.fake_allocation_ == false)) {
-        // If this is a declined lease that expired, we need to conduct
-        // extra steps for it. However, we do want to conduct those steps
-        // only once. In particular, if we have an expired declined lease
-        // and client sent DHCPDISCOVER and will later send DHCPREQUEST,
-        // we only want to call this method once when responding to
-        // DHCPREQUEST (when the actual reclaimation takes place).
-        reclaimDeclined(expired);
+    if (!ctx.fake_allocation_) {
+        // The expired lease needs to be reclaimed before it can be reused.
+        // This includes declined leases for which probation period has
+        // elapsed.
+        reclaimExpiredLease(expired, ctx.callout_handle_);
+        expired->state_ = Lease::STATE_DEFAULT;
     }
 
     updateLease4Information(expired, ctx);

src/lib/dhcpsrv/alloc_engine.h

@@ -781,20 +781,76 @@ private:
     /// @param lease IPv6 lease to be extended.
     void extendLease6(ClientContext6& ctx, Lease6Ptr lease);
 
-    /// @brief Sends removal name change reuqest to D2.
+    /// @brief Reclamation mode used by the variants of @c reclaimExpiredLease
+    /// methods.
+    ///
+    /// The following operations are supported:
+    /// - remove lease upon reclamation,
+    /// - update lease's state upon reclamation to 'expired-reclaimed',
+    /// - leave the lease in the database unchanged.
+    enum DbReclaimMode {
+        DB_RECLAIM_REMOVE,
+        DB_RECLAIM_UPDATE,
+        DB_RECLAIM_LEAVE_UNCHANGED
+    };
+
+    /// @brief Reclaim DHCPv4 or DHCPv6 lease with updating lease database.
+    ///
+    /// This method is called by the lease reclamation routine to reclaim the
+    /// lease and update the lease database according to the value of the
+    /// @c remove_lease parameter.
     ///
-    /// This method is exception safe.
+    /// @param lease Pointer to the DHCPv4 or DHCPv6 lease.
+    /// @param remove_lease A boolean flag indicating if the lease should be
+    /// remove from the lease database (if true) upon reclamation.
+    /// @param callout_handle Pointer to the callout handle.
+    /// @tparam LeasePtrPtr Lease type, i.e. @c Lease4Ptr or @c Lease6Ptr.
+    template<typename LeasePtrType>
+    void reclaimExpiredLease(const LeasePtrType& lease,
+                             const bool remove_lease,
+                             const hooks::CalloutHandlePtr& callout_handle);
+
+    /// @brief Reclaim DHCPv4 or DHCPv6 lease without updating lease database.
     ///
-    /// @param lease Pointer to a lease for which NCR should be sent.
-    /// @param identifier Identifier to be used to generate DHCID for
-    /// the DNS update. For DHCPv4 it will be hardware address or client
-    /// identifier. For DHCPv6 it will be a DUID.
+    /// This method is called by the methods allocating leases, when the lease
+    /// being allocated needs to be first reclaimed. These methods update the
+    /// lease database on their own, so this reclamation method doesn't update
+    /// the database on reclamation.
     ///
-    /// @tparam LeasePtrType Pointer to a lease.
-    /// @tparam IdentifierType HW Address, Client Identifier or DUID.
-    template<typename LeasePtrType, typename IdentifierType>
-    void queueRemovalNameChangeRequest(const LeasePtrType& lease,
-                                       const IdentifierType& identifier) const;
+    /// @param lease Pointer to the DHCPv4 or DHCPv6 lease.
+    /// @param callout_handle Pointer to the callout handle.
+    /// @tparam LeasePtrType Lease type, i.e. @c Lease4Ptr or @c Lease6Ptr.
+    template<typename LeasePtrType>
+    void reclaimExpiredLease(const LeasePtrType& lease,
+                             const hooks::CalloutHandlePtr& callout_handle);
+
+    /// @brief Reclaim DHCPv6 lease.
+    ///
+    /// This method variant accepts the @c reclaim_mode parameter which
+    /// controls if the reclaimed lease should be left in the database with
+    /// no change or if it should be removed or updated.
+    ///
+    /// @param lease Pointer to the DHCPv6 lease.
+    /// @param reclaim_mode Indicates what the method should do with the reclaimed
+    /// lease in the lease database.
+    /// @param callout_handle Pointer to the callout handle.
+    void reclaimExpiredLease(const Lease6Ptr& lease,
+                             const DbReclaimMode& reclaim_mode,
+                             const hooks::CalloutHandlePtr& callout_handle);
+
+    /// @brief Reclaim DHCPv4 lease.
+    ///
+    /// This method variant accepts the @c reclaim_mode parameter which
+    /// controls if the reclaimed lease should be left in the database with
+    /// no change or if it should be removed or updated.
+    ///
+    /// @param lease Pointer to the DHCPv4 lease.
+    /// @param reclaim_mode Indicates what the method should do with the reclaimed
+    /// lease in the lease database.
+    /// @param callout_handle Pointer to the callout handle.
+    void reclaimExpiredLease(const Lease4Ptr& lease,
+                             const DbReclaimMode& reclaim_mode,
+                             const hooks::CalloutHandlePtr& callout_handle);
 
     /// @brief Marks lease as reclaimed in the database.
     ///

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -587,6 +587,18 @@ lease from the PostgreSQL database for the specified address.
 A debug message issued when the server is attempting to update IPv6
 lease from the PostgreSQL database for the specified address.
 
+% DHCPSRV_QUEUE_NCR name change request to %1 DNS entry queued: %2
+A debug message which is logged when the NameChangeRequest to add or remove
+a DNS entries for a particular lease has been queued. The first parameter of
+this log message indicates whether the DNS entry is to be added or removed.
+The second parameter carries the details of the NameChangeRequest.
+
+% DHCPSRV_QUEUE_NCR_FAILED queueing %1 name change request failed for lease %2: %3
+This error message is logged when sending a name change request
+to DHCP DDNS failed. The type of the name change request is specified
+as first argument. The remaining arguments provide the details of the
+lease and the reson for failure.
+
 % DHCPSRV_TIMERMGR_CALLBACK_FAILED running handler for timer %1 caused exception: %2
 This error message is emitted when the timer elapsed and the
 operation associated with this timer has thrown an exception.

src/lib/dhcpsrv/ncr_generator.cc

@@ -0,0 +1,105 @@
+// Copyright (C) 2015 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/option_data_types.h>
+#include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/dhcpsrv_log.h>
+#include <dhcpsrv/d2_client_mgr.h>
+#include <dhcpsrv/ncr_generator.h>
+#include <stdint.h>
+#include <vector>
+
+using namespace isc;
+using namespace isc::dhcp;
+using namespace isc::dhcp_ddns;
+
+namespace {
+
+/// @brief Sends name change request to D2 using lease information.
+///
+/// This method is exception safe.
+///
+/// @param lease Pointer to a lease for which NCR should be sent.
+/// @param identifier Identifier to be used to generate DHCID for
+/// the DNS update. For DHCPv4 it will be hardware address or client
+/// identifier. For DHCPv6 it will be a DUID.
+///
+/// @tparam LeasePtrType Pointer to a lease.
+/// @tparam IdentifierType HW Address, Client Identifier or DUID.
+template<typename LeasePtrType, typename IdentifierType>
+void queueNCRCommon(const NameChangeType& chg_type, const LeasePtrType& lease,
+                           const IdentifierType& identifier) {
+
+    // Check if there is a need for update.
+    if (!lease || lease->hostname_.empty() || (!lease->fqdn_fwd_ && !lease->fqdn_rev_)
+        || !CfgMgr::instance().getD2ClientMgr().ddnsEnabled()) {
+        return;
+    }
+
+    try {
+        // Create DHCID
+        std::vector<uint8_t> hostname_wire;
+        OptionDataTypeUtil::writeFqdn(lease->hostname_, hostname_wire, true);
+        D2Dhcid dhcid = D2Dhcid(identifier, hostname_wire);
+
+        // Create name change request.
+        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(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL_DATA, DHCPSRV_QUEUE_NCR)
+            .arg(chg_type == CHG_ADD ? "add" : "remove")
+            .arg(ncr->toText());
+
+        // Send name change request.
+        CfgMgr::instance().getD2ClientMgr().sendRequest(ncr);
+
+    } catch (const std::exception& ex) {
+        LOG_ERROR(dhcpsrv_logger, DHCPSRV_QUEUE_NCR_FAILED)
+            .arg(chg_type == CHG_ADD ? "add" : "remove")
+            .arg(lease->addr_.toText())
+            .arg(ex.what());
+    }
+}
+
+} // end of anonymous namespace
+
+namespace isc {
+namespace dhcp {
+
+void queueNCR(const NameChangeType& chg_type, const Lease4Ptr& lease) {
+    if (lease) {
+        // Client id takes precedence over HW address.
+        if (lease->client_id_) {
+            queueNCRCommon(chg_type, lease, lease->client_id_->getClientId());
+
+        } else {
+            // Client id is not specified for the lease. Use HW address
+            // instead.
+            queueNCRCommon(chg_type, lease, lease->hwaddr_);
+        }
+    }
+}
+
+void queueNCR(const NameChangeType& chg_type, const Lease6Ptr& lease) {
+    // DUID is required to generate NCR.
+    if (lease && lease->duid_) {
+        queueNCRCommon(chg_type, lease, *(lease->duid_));
+    }
+}
+
+}
+}

src/lib/dhcpsrv/ncr_generator.h

@@ -0,0 +1,52 @@
+// Copyright (C) 2015 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 NCR_GENERATOR_H
+#define NCR_GENERATOR_H
+
+#include <dhcp_ddns/ncr_msg.h>
+#include <dhcpsrv/lease.h>
+
+namespace isc {
+namespace dhcp {
+
+/// @brief Creates name change request from the DHCPv4 lease.
+///
+/// This function creates name change request from the information contained
+/// in the DHCPv4 lease. If the client identifier is present in the lease,
+/// this identifier is used to compute the DHCID, otherwise the HW address
+/// is used.
+///
+/// This function is exception safe. On failure, it logs an error.
+///
+/// @param chg_type Type of the name change request
+/// @param lease Pointer to the lease.
+void queueNCR(const dhcp_ddns::NameChangeType& chg_type, const Lease4Ptr& lease);
+
+/// @brief Creates name change request from the DHCPv6 lease.
+///
+/// This function creates name change request from the information contained
+/// in the DHCPv6 lease. The DUID is used to compute the DHCID for the name
+/// change request.
+///
+/// This function is exception safe. On failure, it logs an error.
+///
+/// @param chg_type Type of the name change request
+/// @param lease Pointer to the lease.
+void queueNCR(const dhcp_ddns::NameChangeType& chg_type, const Lease6Ptr& lease);
+
+} // end of isc::dhcp namespace
+} // end of isc namespace
+
+#endif // NCR_GENERATOR_H

src/lib/dhcpsrv/tests/Makefile.am

@@ -101,6 +101,9 @@ libdhcpsrv_unittests_SOURCES += dhcp_parsers_unittest.cc
 if HAVE_MYSQL
 libdhcpsrv_unittests_SOURCES += mysql_lease_mgr_unittest.cc
 endif
+
+libdhcpsrv_unittests_SOURCES += ncr_generator_unittest.cc
+
 if HAVE_PGSQL
 libdhcpsrv_unittests_SOURCES += pgsql_lease_mgr_unittest.cc
 endif

src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc

@@ -185,12 +185,18 @@ public:
         // Remove all statistics.
         StatsMgr::instance().resetAll();
 
+        // Set the 'reclaimed-leases' statistics to '0'. This statistics
+        // is used by some tests to verify that the leases reclamation
+        // routine has been called.
+        StatsMgr::instance().setValue("reclaimed-leases",
+                                      static_cast<int64_t>(0));
+
         // Create lease manager.
         LeaseMgrFactory::create(lease_mgr_params);
 
         // Create allocation engine instance.
         engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                      100, false));
+                                      100, true));
     }
 
     /// @brief Destructor
@@ -250,6 +256,16 @@ public:
         ASSERT_NO_THROW(updateLease(lease_index));
     }
 
+    /// @brief Changes the owner of a lease.
+    ///
+    /// This method changes the owner of the lease. It must be implemented in
+    /// the derived classes to update the unique identifier(s) in the lease to
+    /// point to a different client.
+    ///
+    /// @param lease_index Lease index. Must be between 0 and
+    /// @c TEST_LEASES_NUM.
+    virtual void transferOwnership(const uint16_t lease_index) = 0;
+
     /// @brief Marks lease as expired-reclaimed.
     ///
     /// @param lease_index Lease index. Must be between 0 and
@@ -1105,6 +1121,14 @@ public:
         LeaseMgrFactory::instance().updateLease6(leases_[lease_index]);
     }
 
+    /// @brief Changes the owner of a lease.
+    ///
+    /// This method changes the owner of the lease by modifying the DUID.
+    ///
+    /// @param lease_index Lease index. Must be between 0 and
+    /// @c TEST_LEASES_NUM.
+    virtual void transferOwnership(const uint16_t lease_index);
+
     /// @brief Sets subnet id for a lease.
     ///
     /// It also updates statistics of assigned leases in the stats manager.
@@ -1155,6 +1179,16 @@ public:
     /// @brief Test that statistics is updated when leases are reclaimed.
     void testReclaimExpiredLeasesStats();
 
+    /// @brief Test that expired leases are reclaimed before they are allocated.
+    ///
+    /// @param msg_type DHCPv6 message type.
+    /// @param use_reclaimed Boolean parameter indicating if the leases
+    /// stored in the lease database should be marked as 'expired-reclaimed'
+    /// or 'expired'. This allows to test whether the allocation engine can
+    /// determine that the lease has been reclaimed already and not reclaim
+    /// it the second time.
+    void testReclaimReusedLeases(const uint16_t msg_type, const bool use_reclaimed);
+
 };
 
 ExpirationAllocEngine6Test::ExpirationAllocEngine6Test()
@@ -1194,6 +1228,19 @@ ExpirationAllocEngine6Test::createLeases() {
 }
 
 void
+ExpirationAllocEngine6Test::transferOwnership(const uint16_t lease_index) {
+    ASSERT_GT(leases_.size(), lease_index);
+    std::vector<uint8_t> bytes = leases_[lease_index]->duid_->getDuid();
+    if (bytes.size() > 1) {
+        if (++bytes[0] == 0) {
+            ++bytes[1];
+        }
+    }
+
+    leases_[lease_index]->duid_.reset(new DUID(bytes));
+}
+
+void
 ExpirationAllocEngine6Test::setSubnetId(const uint16_t lease_index, const SubnetID& id) {
     ASSERT_GT(leases_.size(), lease_index);
     if (leases_[lease_index]->subnet_id_ != id) {
@@ -1274,6 +1321,73 @@ ExpirationAllocEngine6Test::testReclaimExpiredLeasesStats() {
     }
 }
 
+void
+ExpirationAllocEngine6Test::testReclaimReusedLeases(const uint16_t msg_type,
+                                                    const bool use_reclaimed) {
+    BOOST_STATIC_ASSERT(TEST_LEASES_NUM < 0xFFFF);
+
+    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
+        // Depending on the parameter, mark leases 'expired-reclaimed' or
+        // simply 'expired'.
+        if (use_reclaimed) {
+            reclaim(i, 1000 - i);
+
+        } else {
+            // Mark all leases as expired.
+            expire(i, 1000 - i);
+        }
+
+        // For the Renew case, we don't change the ownership of leases. We
+        // will let the lease owners renew them. For other cases, we modify
+        // the DUIDs to simulate reuse of expired leases.
+        if (msg_type != DHCPV6_RENEW) {
+            transferOwnership(i);
+        }
+    }
+
+    // Create subnet and the pool. This is required by the allocation process.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 64, 10, 20, 50, 60,
+                                  SubnetID(1)));
+    ASSERT_NO_THROW(subnet->addPool(Pool6Ptr(new Pool6(Lease::TYPE_NA,
+                                                       IOAddress("2001:db8:1::"),
+                                                       IOAddress("2001:db8:1::FFFF")))));
+
+    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
+        // Build the context.
+        AllocEngine::ClientContext6 ctx(subnet, leases_[i]->duid_, 1,
+                                        leases_[i]->addr_,
+                                        Lease::TYPE_NA,
+                                        false, false,
+                                        leases_[i]->hostname_,
+                                        msg_type == DHCPV6_SOLICIT);
+        // Query is needed for logging purposes.
+        ctx.query_.reset(new Pkt6(msg_type, 0x1234));
+
+        // Depending on the message type, we will call a different function.
+        if (msg_type == DHCPV6_RENEW) {
+            ASSERT_NO_THROW(engine_->renewLeases6(ctx));
+
+        } else {
+            ASSERT_NO_THROW(engine_->renewLeases6(ctx));
+        }
+    }
+
+    // The Solicit should not trigger leases reclamation. The Renew and
+    // Request must trigger leases reclamation unless the lease is
+    // initially reclaimed.
+    if (use_reclaimed || (msg_type == DHCPV6_SOLICIT)) {
+        EXPECT_TRUE(testStatistics("reclaimed-leases", 0));
+
+    } else {
+        EXPECT_TRUE(testStatistics("reclaimed-leases", TEST_LEASES_NUM));
+        // Leases should have been updated in the lease database and their
+        // state should not be 'expired-reclaimed' anymore.
+        EXPECT_TRUE(testLeases(&leaseNotReclaimed, &allLeaseIndexes));
+
+    }
+
+}
+
 // This test verifies that the leases can be reclaimed without being removed
 // from the database. In such case, the leases' state is set to
 // "expired-reclaimed".
@@ -1377,6 +1491,42 @@ TEST_F(ExpirationAllocEngine6Test, reclaimDeclinedStats) {
     testReclaimDeclinedStats("assigned-nas");
 }
 
+// This test verifies that expired leases are reclaimed before they are
+// allocated to another client sending a Request message.
+TEST_F(ExpirationAllocEngine6Test, reclaimReusedLeases) {
+    testReclaimReusedLeases(DHCPV6_REQUEST, false);
+}
+
+// This test verifies that allocation engine detects that the expired
+// lease has been reclaimed already when it reuses this lease.
+TEST_F(ExpirationAllocEngine6Test, reclaimReusedLeasesAlreadyReclaimed) {
+    testReclaimReusedLeases(DHCPV6_REQUEST, true);
+}
+
+// This test verifies that expired leases are reclaimed before they
+// are renewed.
+TEST_F(ExpirationAllocEngine6Test, reclaimRenewedLeases) {
+    testReclaimReusedLeases(DHCPV6_RENEW, false);
+}
+
+// This test verifies that allocation engine detects that the expired
+// lease has been reclaimed already when it renews the lease.
+TEST_F(ExpirationAllocEngine6Test, reclaimRenewedLeasesAlreadyReclaimed) {
+    testReclaimReusedLeases(DHCPV6_RENEW, true);
+}
+
+// This test verifies that the expired leases are not reclaimed when the
+// Solicit message is being processed.
+TEST_F(ExpirationAllocEngine6Test, reclaimReusedLeasesSolicit) {
+    testReclaimReusedLeases(DHCPV6_SOLICIT, false);
+}
+
+// This test verifies that the 'expired-reclaimed' leases are not reclaimed
+// again when the Solicit message is being processed.
+TEST_F(ExpirationAllocEngine6Test, reclaimReusedLeasesSolicitAlreadyReclaimed) {
+    testReclaimReusedLeases(DHCPV6_SOLICIT, true);
+}
+
 // *******************************************************
 //
 // DHCPv4 lease reclamation routine tests start here!
@@ -1411,6 +1561,15 @@ public:
         LeaseMgrFactory::instance().updateLease4(leases_[lease_index]);
     }
 
+    /// @brief Changes the owner of a lease.
+    ///
+    /// This method changes the owner of the lease by updating the client
+    /// identifier (if present) or HW address.
+    ///
+    /// @param lease_index Lease index. Must be between 0 and
+    /// @c TEST_LEASES_NUM.
+    virtual void transferOwnership(const uint16_t lease_index);
+
     /// @brief Retrieves lease from the database.
     ///
     /// @param lease_index Index of the lease.
@@ -1466,6 +1625,17 @@ public:
     /// @brief Test that statistics is updated when leases are reclaimed..
     void testReclaimExpiredLeasesStats();
 
+    /// @brief Test that the lease is reclaimed before it is renewed or
+    /// reused.
+    ///
+    /// @param msg_type DHCPv4 message type, i.e. DHCPDISCOVER or DHCPREQUEST.
+    /// @param client_renews A boolean value which indicates if the test should
+    /// simulate renewals of leases (if true) or reusing expired leases which
+    /// belong to different clients (if false).
+    /// @param use_reclaimed Boolean parameter indicating if the leases
+    void testReclaimReusedLeases(const uint8_t msg_type, const bool client_renews,
+                                 const bool use_reclaimed);
+
 };
 
 ExpirationAllocEngine4Test::ExpirationAllocEngine4Test()
@@ -1533,6 +1703,29 @@ ExpirationAllocEngine4Test::setSubnetId(const uint16_t lease_index, const Subnet
     }
 }
 
+void
+ExpirationAllocEngine4Test::transferOwnership(const uint16_t lease_index) {
+    ASSERT_GT(leases_.size(), lease_index);
+    std::vector<uint8_t> bytes;
+    if (leases_[lease_index]->client_id_) {
+        bytes = leases_[lease_index]->client_id_->getClientId();
+    } else {
+        bytes = leases_[lease_index]->hwaddr_->hwaddr_;
+    }
+
+    if (!bytes.empty()) {
+        if (++bytes[0] == 0) {
+            ++bytes[1];
+        }
+    }
+
+    if (leases_[lease_index]->client_id_) {
+        leases_[lease_index]->client_id_.reset(new ClientId(bytes));
+    } else {
+        leases_[lease_index]->hwaddr_.reset(new HWAddr(bytes, HTYPE_ETHER));
+    }
+}
+
 
 bool
 ExpirationAllocEngine4Test::dnsUpdateGeneratedFromClientId(const Lease4Ptr& lease) {
@@ -1668,6 +1861,77 @@ ExpirationAllocEngine4Test::testReclaimExpiredLeasesStats() {
     }
 }
 
+void
+ExpirationAllocEngine4Test::testReclaimReusedLeases(const uint8_t msg_type,
+                                                    const bool client_renews,
+                                                    const bool use_reclaimed) {
+    // Let's restrict the number of leases to /16.
+    BOOST_STATIC_ASSERT(TEST_LEASES_NUM < 0xFFFF);
+
+    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
+        // Depending on the parameter, mark leases 'expired-reclaimed' or
+        // simply 'expired'.
+        if (use_reclaimed) {
+            reclaim(i, 1000 - i);
+
+        } else {
+            // Mark all leases as expired.
+            expire(i, 1000 - i);
+        }
+
+        // Check if we're simulating renewals or reusing leases. If this is
+        // about reusing leases, we should be using different MAC addresses
+        // or client identifiers for the leases than those stored presently
+        // in the database.
+        if (!client_renews) {
+            // This function modifies the MAC address or the client identifier
+            // of the test lease to make sure it doesn't match the one we
+            // have in the database.
+            transferOwnership(i);
+        }
+    }
+
+    // The call to AllocEngine::allocateLease4 requires the subnet selection.
+    // The pool must be present within a subnet for the allocation engine to
+    // hand out address from.
+    Subnet4Ptr subnet(new Subnet4(IOAddress("10.0.0.0"), 16, 10, 20, 60, SubnetID(1)));
+    ASSERT_NO_THROW(subnet->addPool(Pool4Ptr(new Pool4(IOAddress("10.0.0.0"),
+                                                       IOAddress("10.0.255.255")))));
+
+    // Re-allocate leases (reuse or renew).
+    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
+        // Build the context.
+        AllocEngine::ClientContext4 ctx(subnet, leases_[i]->client_id_,
+                                        leases_[i]->hwaddr_,
+                                        leases_[i]->addr_, false, false,
+                                        leases_[i]->hostname_,
+                                        msg_type == DHCPDISCOVER);
+        // Query is needed for logging purposes.
+        ctx.query_.reset(new Pkt4(msg_type, 0x1234));
+
+        // Re-allocate a lease. Note that the iterative will pick addresses
+        // starting from the beginning of the pool. This matches exactly
+        // the set of addresses we have allocated and stored in the database.
+        // Since all leases are marked expired the allocation engine will
+        // reuse them or renew them as appropriate.
+        ASSERT_NO_THROW(engine_->allocateLease4(ctx));
+    }
+
+    // If DHCPDISCOVER is being processed, the leases should not be reclaimed.
+    // Also, the leases should not be reclaimed if they are already in the
+    // 'expired-reclaimed' state.
+    if (use_reclaimed || (msg_type == DHCPDISCOVER)) {
+        EXPECT_TRUE(testStatistics("reclaimed-leases", 0));
+
+    } else if (msg_type == DHCPREQUEST) {
+        // Re-allocation of expired leases should result in reclamations.
+        EXPECT_TRUE(testStatistics("reclaimed-leases", TEST_LEASES_NUM));
+        // Leases should have been updated in the lease database and their
+        // state should not be 'expired-reclaimed' anymore.
+        EXPECT_TRUE(testLeases(&leaseNotReclaimed, &allLeaseIndexes));
+    }
+}
+
 
 // This test verifies that the leases can be reclaimed without being removed
 // from the database. In such case, the leases' state is set to
@@ -1778,4 +2042,52 @@ TEST_F(ExpirationAllocEngine4Test, reclaimDeclinedStats) {
     testReclaimDeclinedStats("assigned-addresses");
 }
 
+// This test verifies that the lease is reclaimed before it is reused.
+TEST_F(ExpirationAllocEngine4Test, reclaimReusedLeases) {
+    // First false value indicates that the leases will be reused.
+    // Second false value indicates that the lease will not be
+    // initially reclaimed.
+    testReclaimReusedLeases(DHCPREQUEST, false, false);
+}
+
+// This test verifies that the lease is not reclaimed when it is
+// reused and  if its state indicates that it has been already reclaimed.
+TEST_F(ExpirationAllocEngine4Test, reclaimReusedLeasesAlreadyReclaimed) {
+    // false value indicates that the leases will be reused
+    // true value indicates that the lease will be initially reclaimed.
+    testReclaimReusedLeases(DHCPREQUEST, false, true);
+}
+
+// This test verifies that the expired lease is reclaimed before it
+// is renewed.
+TEST_F(ExpirationAllocEngine4Test, reclaimRenewedLeases) {
+    // true value indicates that the leases will be renewed.
+    // false value indicates that the lease will not be initially
+    // reclaimed.
+    testReclaimReusedLeases(DHCPREQUEST, true, false);
+}
+
+// This test verifies that the lease is not reclaimed upon renewal
+// if its state indicates that it has been already reclaimed.
+TEST_F(ExpirationAllocEngine4Test, reclaimRenewedLeasesAlreadyReclaimed) {
+    // First true value indicates that the leases will be renewed.
+    // Second true value indicates that the lease will be initially
+    // reclaimed.
+    testReclaimReusedLeases(DHCPREQUEST, true, true);
+}
+
+// This test verifies that the reused lease is not reclaimed when the
+// processed message is a DHCPDISCOVER.
+TEST_F(ExpirationAllocEngine4Test, reclaimReusedLeasesDiscover) {
+    testReclaimReusedLeases(DHCPDISCOVER, false, false);
+}
+
+// This test verifies that the lease being in the 'expired-reclaimed'
+// state is not reclaimed again when processing the DHCPDISCOVER
+// message.
+TEST_F(ExpirationAllocEngine4Test, reclaimRenewedLeasesDiscoverAlreadyReclaimed) {
+    testReclaimReusedLeases(DHCPDISCOVER, false, true);
+}
+
+
 }; // end of anonymous namespace

src/lib/dhcpsrv/tests/ncr_generator_unittest.cc

@@ -0,0 +1,489 @@
+// Copyright (C) 2015 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 <config.h>
+#include <asiolink/io_address.h>
+#include <dhcp/duid.h>
+#include <dhcp_ddns/ncr_msg.h>
+#include <dhcpsrv/ncr_generator.h>
+#include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/d2_client_mgr.h>
+#include <dhcpsrv/lease.h>
+
+#include <ctime>
+#include <boost/bind.hpp>
+#include <gtest/gtest.h>
+
+#include <stdint.h>
+#include <string>
+
+using namespace isc;
+using namespace isc::asiolink;
+using namespace isc::dhcp;
+using namespace isc::dhcp_ddns;
+
+namespace {
+
+/// @brief Base test fixture class for testing generation of the name
+/// change requests from leases.
+///
+/// @tparam LeasePtrType One of the @c Lease4Ptr or @c Lease6Ptr.
+template<typename LeasePtrType>
+class NCRGeneratorTest : public ::testing::Test {
+public:
+
+    /// @brief Reference to the D2 client manager.
+    D2ClientMgr& d2_mgr_;
+
+    /// @brief Pointer to the lease object used by the tests.
+    LeasePtrType lease_;
+
+    /// @brief Constructor.
+    NCRGeneratorTest()
+        : d2_mgr_(CfgMgr::instance().getD2ClientMgr()), lease_() {
+    }
+
+    /// @brief Initializes the lease pointer used by the tests and starts D2.
+    ///
+    /// This method initializes the pointer to the lease which will be used
+    /// throughout the tests. Because the lease may be either a v4 or v6 lease
+    /// it calls a virtual function @c initLease, which must be implemented
+    /// in the derived classes as appropriate. Note that lease object can't
+    /// be initialized in the constructor, because it is not allowed to
+    /// call virtual functions in the constructors. Hence, the @c SetUp
+    /// function is needed.
+    virtual void SetUp() {
+        // Base class SetUp.
+        ::testing::Test::SetUp();
+        // Initialize lease_ object.
+        initLease();
+        // Start D2 by default.
+        enableD2();
+    }
+
+    /// @brief Stops D2.
+    virtual void TearDown() {
+        // Stop D2 if running.
+        disableD2();
+        // Base class TearDown.
+        ::testing::Test::TearDown();
+    }
+
+    /// @brief Enables DHCP-DDNS updates.
+    ///
+    /// Replaces the current D2ClientConfiguration with a configuration
+    /// which as updates enabled and the control options set based upon
+    /// the bit mask of options.
+    void enableD2() {
+        D2ClientConfigPtr cfg(new D2ClientConfig());
+        ASSERT_NO_THROW(cfg->enableUpdates(true));
+        ASSERT_NO_THROW(CfgMgr::instance().setD2ClientConfig(cfg));
+        d2_mgr_.startSender(boost::bind(&NCRGeneratorTest::d2ErrorHandler, this,
+                                        _1, _2));
+    }
+
+    /// @brief Disables DHCP-DDNS updates.
+    void disableD2() {
+        d2_mgr_.stopSender();
+        // Default constructor creates a config with DHCP-DDNS updates
+        // disabled.
+        D2ClientConfigPtr cfg(new D2ClientConfig());
+        CfgMgr::instance().setD2ClientConfig(cfg);
+    }
+
+    /// @brief No-op error handler for D2.
+    void d2ErrorHandler(const NameChangeSender::Result, NameChangeRequestPtr&) {
+        // no-op
+    }
+
+    /// @brief Abstract method to initialize @c lease_ object.
+    virtual void initLease() = 0;
+
+    /// @brief Verify that NameChangeRequest holds valid values.
+    ///
+    /// This function picks first NameChangeRequest from the internal server's
+    /// queue and checks that it holds valid parameters. The NameChangeRequest
+    /// is removed from the queue.
+    ///
+    /// @param type An expected type of the NameChangeRequest (Add or Remove).
+    /// @param reverse An expected setting of the reverse update flag.
+    /// @param forward An expected setting of the forward udpate flag.
+    /// @param addr A string representation of the IPv6 address held in the
+    /// NameChangeRequest.
+    /// @param dhcid An expected DHCID value.
+    /// @note This value is the value that is produced by
+    /// dhcp_ddns::D2Dhcid::createDigest() with the appropriate arguments. This
+    /// method uses encryption tools to produce the value which cannot be
+    /// easily duplicated by hand.  It is more or less necessary to generate
+    /// these values programmatically and place them here. Should the
+    /// underlying implementation of createDigest() change these test values
+    /// will likely need to be updated as well.
+    /// @param expires A timestamp when the lease associated with the
+    /// NameChangeRequest expires.
+    /// @param len A valid lifetime of the lease associated with the
+    /// NameChangeRequest.
+    /// @param fqdn The expected string value of the FQDN, if blank the
+    /// check is skipped
+    void verifyNameChangeRequest(const isc::dhcp_ddns::NameChangeType type,
+                                 const bool reverse, const bool forward,
+                                 const std::string& addr,
+                                 const std::string& dhcid,
+                                 const uint64_t expires,
+                                 const uint16_t len,
+                                 const std::string& fqdn="") {
+        NameChangeRequestPtr ncr;
+        ASSERT_NO_THROW(ncr = CfgMgr::instance().getD2ClientMgr().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(dhcid, ncr->getDhcid().toStr());
+        EXPECT_EQ(expires, ncr->getLeaseExpiresOn());
+        EXPECT_EQ(len, ncr->getLeaseLength());
+        EXPECT_EQ(isc::dhcp_ddns::ST_NEW, ncr->getStatus());
+
+        if (!fqdn.empty()) {
+           EXPECT_EQ(fqdn, ncr->getFqdn());
+        }
+
+        // Process the message off the queue
+        ASSERT_NO_THROW(CfgMgr::instance().getD2ClientMgr().runReadyIO());
+    }
+
+    /// @brief Sets the FQDN information for a lease and queues an NCR.
+    ///
+    /// @param fwd Perform forward update.
+    /// @param rev Perform reverse update.
+    /// @param fqdn Hostname.
+    void queueRemovalNCR(const bool fwd, const bool rev, const std::string& fqdn) {
+        lease_->fqdn_fwd_ = fwd;
+        lease_->fqdn_rev_ = rev;
+        lease_->hostname_ = fqdn;
+
+        /// Send NCR to D2.
+        ASSERT_NO_THROW(queueNCR(CHG_REMOVE, lease_));
+    }
+
+    /// @brief Sets the FQDN information for a lease and queues an NCR.
+    ///
+    /// @param chg_type Name change type.
+    /// @param fwd Perform forward update.
+    /// @param rev Perform reverse update.
+    /// @param fqdn Hostname.
+    void sendNCR(const NameChangeType chg_type, const bool fwd, const bool rev,
+                 const std::string& fqdn) {
+        lease_->fqdn_fwd_ = fwd;
+        lease_->fqdn_rev_ = rev;
+        lease_->hostname_ = fqdn;
+
+        /// Send NCR to D2.
+        ASSERT_NO_THROW(queueNCR(chg_type, lease_));
+    }
+
+    /// @brief Test that for the given values the NCR is not generated.
+    ///
+    /// @param chg_type Name change type.
+    /// @param fwd Perform forward update.
+    /// @param rev Perform reverse update.
+    /// @param fqdn Hostname.
+    void testNoUpdate(const NameChangeType chg_type, const bool fwd, const bool rev,
+                      const std::string& fqdn) {
+        ASSERT_NO_FATAL_FAILURE(sendNCR(chg_type, fwd, rev, fqdn));
+        ASSERT_EQ(0, d2_mgr_.getQueueSize());
+    }
+
+    /// @brief Test that sending an NCR while DNS updates would not throw.
+    ///
+    /// @param chg_type Name change type.
+    void testD2Disabled(const NameChangeType chg_type) {
+        // Disable DDNS updates.
+        disableD2();
+        ASSERT_NO_FATAL_FAILURE(sendNCR(chg_type, true, true, "MYHOST.example.com."));
+    }
+
+    /// @brief Test that NCR is generated as expected.
+    ///
+    /// @param chg_type Name change type.
+    /// @param fwd Perform forward update.
+    /// @param rev Perform reverse update.
+    /// @param fqdn Hostname.
+    /// @param exp_dhcid Expected DHCID.
+    void testNCR(const NameChangeType chg_type, const bool fwd, const bool rev,
+                 const std::string& fqdn, const std::string exp_dhcid) {
+        // Queue NCR.
+        ASSERT_NO_FATAL_FAILURE(sendNCR(chg_type, fwd, rev, fqdn));
+        // Expecting one NCR be generated.
+        ASSERT_EQ(1, d2_mgr_.getQueueSize());
+        // Check the details of the NCR.
+        verifyNameChangeRequest(chg_type, rev, fwd, lease_->addr_.toText(), exp_dhcid,
+                                lease_->cltt_ + lease_->valid_lft_,
+                                lease_->valid_lft_);
+    }
+};
+
+/// @brief Test fixture class implementation for DHCPv6.
+class NCRGenerator6Test : public NCRGeneratorTest<Lease6Ptr> {
+public:
+
+    /// @brief Pointer to the DUID used in the tests.
+    DuidPtr duid_;
+
+    /// @brief Constructor.
+    ///
+    /// Initializes DUID.
+    NCRGenerator6Test()
+        : duid_() {
+        duid_.reset(new DUID(DUID::fromText("01:02:03:04:05:06:07:08:09")));
+    }
+
+    /// @brief Implementation of the method creating DHCPv6 lease instance.
+    virtual void initLease() {
+        lease_.reset(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:1::1"),
+                                duid_, 1234, 501, 502, 503,
+                                504, 1, HWAddrPtr(), 0));
+    }
+};
+
+
+// Test creation of the NameChangeRequest for both forward and reverse
+// mapping for the given lease.
+TEST_F(NCRGenerator6Test, fwdRev) {
+    // Part of the domain name is in upper case, to test that it gets converted
+    // to lower case before DHCID is computed. So, we should get the same DHCID
+    // as if we typed domain-name in lower case.
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, true, true, "MYHOST.example.com.",
+                "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
+                "D0ECCEA5E0DD71730F392119A");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, true, true, "MYHOST.example.com.",
+                "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
+                "D0ECCEA5E0DD71730F392119A");
+    }
+}
+
+// Checks that NameChangeRequests are not created when ddns updates are disabled.
+TEST_F(NCRGenerator6Test, d2Disabled) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testD2Disabled(CHG_REMOVE);
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testD2Disabled(CHG_ADD);
+    }
+}
+
+// Test creation of the NameChangeRequest for reverse mapping in the
+// given lease.
+TEST_F(NCRGenerator6Test, removeRev) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, false, true, "myhost.example.com.",
+                "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
+                "D0ECCEA5E0DD71730F392119A");
+    }
+
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, false, true, "myhost.example.com.",
+                "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
+                "D0ECCEA5E0DD71730F392119A");
+    }
+}
+
+// Test that NameChangeRequest is not generated when neither forward
+// nor reverse DNS update has been performed for a lease.
+TEST_F(NCRGenerator6Test, noFwdRevUpdate) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNoUpdate(CHG_REMOVE, false, false, "myhost.example.com.");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNoUpdate(CHG_ADD, false, false, "myhost.example.com.");
+    }
+}
+
+// Test that NameChangeRequest is not generated if the hostname hasn't been
+// specified for a lease for which forward and reverse mapping has been set.
+TEST_F(NCRGenerator6Test, noHostname) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNoUpdate(CHG_REMOVE, false, false, "");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNoUpdate(CHG_ADD, false, false, "");
+    }
+}
+
+// Test that NameChangeRequest is not generated if the invalid hostname has
+// been specified for a lease for which forward and reverse mapping has been
+// set.
+TEST_F(NCRGenerator6Test, wrongHostname) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNoUpdate(CHG_REMOVE, false, false, "");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNoUpdate(CHG_ADD, false, false, "");
+    }
+}
+
+/// @brief Test fixture class implementation for DHCPv4.
+class NCRGenerator4Test : public NCRGeneratorTest<Lease4Ptr> {
+public:
+
+    /// @brief Pointer to HW address used by the tests.
+    HWAddrPtr hwaddr_;
+
+    /// @brief Constructor.
+    ///
+    /// Initializes HW address.
+    NCRGenerator4Test()
+        : hwaddr_(new HWAddr(HWAddr::fromText("01:02:03:04:05:06"))) {
+    }
+
+    /// @brief Implementation of the method creating DHCPv6 lease instance.
+    virtual void initLease() {
+        lease_.reset(new Lease4(IOAddress("2001:db8:1::1"), hwaddr_, ClientIdPtr(),
+                                100, 30, 60, time(NULL), 1));
+    }
+};
+
+// Test creation of the NameChangeRequest for both forward and reverse
+// mapping for the given lease.
+TEST_F(NCRGenerator4Test, fwdRev) {
+    // Part of the domain name is in upper case, to test that it gets converted
+    // to lower case before DHCID is computed. So, we should get the same DHCID
+    // as if we typed domain-name in lower case.
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, true, true, "MYHOST.example.com.",
+                "000001E356D43E5F0A496D65BCA24D982D646140813E3"
+                "B03AB370BFF46BFA309AE7BFD");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, true, true, "MYHOST.example.com.",
+                "000001E356D43E5F0A496D65BCA24D982D646140813E3"
+                "B03AB370BFF46BFA309AE7BFD");
+    }
+}
+
+// Checks that NameChangeRequests are not created when ddns updates are disabled.
+TEST_F(NCRGenerator4Test, d2Disabled) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testD2Disabled(CHG_REMOVE);
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testD2Disabled(CHG_ADD);
+    }
+}
+
+// Test creation of the NameChangeRequest for reverse mapping in the
+// given lease.
+TEST_F(NCRGenerator4Test, removeRev) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, false, true, "myhost.example.com.",
+                "000001E356D43E5F0A496D65BCA24D982D646140813E3B"
+                "03AB370BFF46BFA309AE7BFD");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, false, true, "myhost.example.com.",
+                "000001E356D43E5F0A496D65BCA24D982D646140813E3B"
+                "03AB370BFF46BFA309AE7BFD");
+    }
+}
+
+// Test that NameChangeRequest is not generated when neither forward
+// nor reverse DNS update has been performed for a lease.
+TEST_F(NCRGenerator4Test, noFwdRevUpdate) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNoUpdate(CHG_REMOVE, false, false, "myhost.example.com.");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNoUpdate(CHG_ADD, false, false, "myhost.example.com.");
+    }
+}
+
+// Test that NameChangeRequest is not generated if the hostname hasn't been
+// specified for a lease for which forward and reverse mapping has been set.
+TEST_F(NCRGenerator4Test, noHostname) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNoUpdate(CHG_REMOVE, false, false, "");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNoUpdate(CHG_ADD, false, false, "");
+    }
+}
+
+// Test that NameChangeRequest is not generated if the invalid hostname has
+// been specified for a lease for which forward and reverse mapping has been
+// set.
+TEST_F(NCRGenerator4Test, wrongHostname) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNoUpdate(CHG_REMOVE, false, false, "");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNoUpdate(CHG_ADD, false, false, "");
+    }
+}
+
+// Test that the correct NameChangeRequest is generated when the lease
+// includes client identifier.
+TEST_F(NCRGenerator4Test, useClientId) {
+    lease_->client_id_ = ClientId::fromText("01:01:01:01");
+
+    ASSERT_NO_FATAL_FAILURE(queueRemovalNCR(true, true, "myhost.example.com."));
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
+
+    verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
+                            "2001:db8:1::1",
+                            "000101C7AA5420483BDA99C437636EA7DA2FE18"
+                            "31C9679FEB031C360CA571298F3D1FA",
+                            lease_->cltt_ + lease_->valid_lft_, 100);
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, true, true, "myhost.example.com.",
+                "000101C7AA5420483BDA99C437636EA7DA2FE1831C9679"
+                "FEB031C360CA571298F3D1FA");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, true, true, "myhost.example.com.",
+                "000101C7AA5420483BDA99C437636EA7DA2FE1831C9679"
+                "FEB031C360CA571298F3D1FA");
+    }
+
+}
+
+} // end of anonymous namespace