[master] Merge branch 'trac3977'

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>
@@ -806,51 +807,6 @@ Dhcpv4Srv::srvidToString(const OptionPtr& srvid) {
     return (addrs[0].toText());
 }
 
-isc::dhcp_ddns::D2Dhcid
-Dhcpv4Srv::computeDhcid(const Lease4Ptr& lease) {
-    if (!lease) {
-        isc_throw(DhcidComputeError, "a pointer to the lease must be not"
-                  " NULL to compute DHCID");
-
-    } else if (lease->hostname_.empty()) {
-        isc_throw(DhcidComputeError, "unable to compute the DHCID for the"
-                  " lease which has empty hostname set");
-
-    }
-
-    // In order to compute DHCID the client's hostname must be encoded in
-    // canonical wire format. It is unlikely that the FQDN is malformed
-    // because it is validated by the classes which encapsulate options
-    // carrying client FQDN. However, if the client name was carried in the
-    // Hostname option it is more likely as it carries the hostname as a
-    // regular string.
-    std::vector<uint8_t> fqdn_wire;
-    try {
-        OptionDataTypeUtil::writeFqdn(lease->hostname_, fqdn_wire, true);
-
-    } catch (const Exception& ex) {
-        isc_throw(DhcidComputeError, "unable to compute DHCID because the"
-                  " hostname: " << lease->hostname_ << " is invalid");
-
-    }
-
-    // Prefer client id to HW address to compute DHCID. If Client Id is
-    // NULL, use HW address.
-    try {
-        if (lease->client_id_) {
-            return (D2Dhcid(lease->client_id_->getClientId(), fqdn_wire));
-
-        } else {
-            return (D2Dhcid(lease->hwaddr_, fqdn_wire));
-        }
-    } catch (const Exception& ex) {
-        isc_throw(DhcidComputeError, "unable to compute DHCID: "
-                  << ex.what());
-
-    }
-
-}
-
 void
 Dhcpv4Srv::appendServerID(Dhcpv4Exchange& ex) {
     // The source address for the outbound message should have been set already.
@@ -1169,76 +1125,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
@@ -1863,10 +1756,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)
@@ -1985,13 +1877,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

@@ -44,13 +44,6 @@
 namespace isc {
 namespace dhcp {
 
-/// @brief Exception thrown when DHCID computation failed.
-class DhcidComputeError : public isc::Exception {
-public:
-    DhcidComputeError(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) { };
-};
-
 /// @brief DHCPv4 message exchange.
 ///
 /// This class represents the DHCPv4 message exchange. The message exchange
@@ -583,20 +576,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
@@ -691,28 +670,6 @@ protected:
     /// @return string representation
     static std::string srvidToString(const OptionPtr& opt);
 
-    /// @brief Computes DHCID from a lease.
-    ///
-    /// This method creates an object which represents DHCID. The DHCID value
-    /// is computed as described in RFC4701. The section 3.3. of RFC4701
-    /// specifies the DHCID RR Identifier Type codes:
-    /// - 0x0000 The 1 octet htype followed by glen octets of chaddr
-    /// - 0x0001 The data octets from the DHCPv4 client's Client Identifier
-    /// option.
-    /// - 0x0002 The client's DUID.
-    ///
-    /// Currently this function supports first two of these identifiers.
-    /// The 0x0001 is preferred over the 0x0000 - if the client identifier
-    /// option is present, the former is used. If the client identifier
-    /// is absent, the latter is used.
-    ///
-    /// @todo Implement support for 0x0002 DHCID identifier type.
-    ///
-    /// @param lease A pointer to the structure describing a lease.
-    /// @return An object encapsulating DHCID to be used for DNS updates.
-    /// @throw DhcidComputeError If the computation of the DHCID failed.
-    static isc::dhcp_ddns::D2Dhcid computeDhcid(const Lease4Ptr& lease);
-
     /// @brief Selects a subnet for a given client's packet.
     ///
     /// @param query client's message

src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -198,7 +198,6 @@ public:
     using Dhcpv4Srv::processDecline;
     using Dhcpv4Srv::processInform;
     using Dhcpv4Srv::processClientName;
-    using Dhcpv4Srv::computeDhcid;
     using Dhcpv4Srv::createNameChangeRequests;
     using Dhcpv4Srv::acceptServerId;
     using Dhcpv4Srv::sanityCheck;

src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -489,63 +489,6 @@ public:
     }
 };
 
-// Test that the exception is thrown if lease pointer specified as the argument
-// of computeDhcid function is NULL.
-TEST_F(NameDhcpv4SrvTest, dhcidNullLease) {
-    Lease4Ptr lease;
-    EXPECT_THROW(srv_->computeDhcid(lease), isc::dhcp::DhcidComputeError);
-
-}
-
-// Test that the appropriate exception is thrown if the lease object used
-// to compute DHCID comprises wrong hostname.
-TEST_F(NameDhcpv4SrvTest, dhcidWrongHostname) {
-    // First, make sure that the lease with the correct hostname is accepted.
-    Lease4Ptr lease = createLease(IOAddress("192.0.2.3"),
-                                  "myhost.example.com.", true, true);
-    ASSERT_NO_THROW(srv_->computeDhcid(lease));
-
-    // Now, use the wrong hostname. It should result in the exception.
-    lease->hostname_ = "myhost...example.com.";
-    EXPECT_THROW(srv_->computeDhcid(lease), isc::dhcp::DhcidComputeError);
-
-    // Also, empty hostname is wrong.
-    lease->hostname_ = "";
-    EXPECT_THROW(srv_->computeDhcid(lease), isc::dhcp::DhcidComputeError);
-}
-
-// Test that the DHCID is computed correctly, when the lease holds
-// correct hostname and non-NULL client id.
-TEST_F(NameDhcpv4SrvTest, dhcidComputeFromClientId) {
-    Lease4Ptr lease = createLease(IOAddress("192.0.2.3"),
-                                  "myhost.example.com.",
-                                  true, true);
-    isc::dhcp_ddns::D2Dhcid dhcid;
-    ASSERT_NO_THROW(dhcid = srv_->computeDhcid(lease));
-
-    // Make sure that the computed DHCID is valid.
-    std::string dhcid_ref = "00010132E91AA355CFBB753C0F0497A5A9404"
-        "36965B68B6D438D98E680BF10B09F3BCF";
-    EXPECT_EQ(dhcid_ref, dhcid.toStr());
-}
-
-// Test that the DHCID is computed correctly, when the lease holds correct
-// hostname and NULL client id.
-TEST_F(NameDhcpv4SrvTest, dhcidComputeFromHWAddr) {
-    Lease4Ptr lease = createLease(IOAddress("192.0.2.3"),
-                                  "myhost.example.com.",
-                                  true, true);
-    lease->client_id_.reset();
-
-    isc::dhcp_ddns::D2Dhcid dhcid;
-    ASSERT_NO_THROW(dhcid = srv_->computeDhcid(lease));
-
-    // Make sure that the computed DHCID is valid.
-    std::string dhcid_ref = "0000012247F6DC4423C3E8627434A9D6868609"
-        "D88948F78018B215EDCAA30C0C135035";
-    EXPECT_EQ(dhcid_ref, dhcid.toStr());
-}
-
 // Tests the following scenario:
 //  - Updates are enabled
 //  - All overrides are off
@@ -745,58 +688,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
@@ -157,14 +151,6 @@ from the acquired address. The first argument includes the client and the
 transaction identification information. The second argument is a leased
 address. The third argument includes the reason for the failure.
 
-% DHCP6_DDNS_LEASE_ASSIGN_FQDN_CHANGE FQDN %1: FQDN for the allocated lease: %2 has changed. New values: hostname = %3, reverse mapping = %4, forward mapping = %5
-This debug message is logged when FQDN mapping for a particular lease has
-been changed by the recent Request message. This mapping will be changed in DNS.
-The first argument includes the client and the transaction identification
-information. The second argument holds the details about the lease for which
-the FQDN information and/or mappings have changed. The remaining arguments
-hold the new FQDN information and flags for mappings.
-
 % DHCP6_DDNS_LEASE_RENEW_FQDN_CHANGE FQDN %1: FQDN for the renewed lease: %2 has changed. New values: hostname = %3, reverse mapping = %4, forward mapping = %5
 This debug message is logged when FQDN mapping for a particular lease has been
 changed by the recent Renew message. This mapping will be changed in DNS.
@@ -179,13 +165,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 +182,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>
@@ -1314,64 +1315,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()->
@@ -1503,27 +1446,6 @@ Dhcpv6Srv::assignIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
         // but this is considered waste of bandwidth as absence of status
         // code is considered a success.
 
-        if (!fake_allocation) {
-            Lease6Ptr old_lease;
-            if (!ctx.changed_leases_.empty()) {
-                old_lease = *ctx.changed_leases_.begin();
-
-                // Allocation engine has returned an existing lease. If so, we
-                // 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);
-            }
-
-            // We need to repeat that check for leases that used to be used, but
-            // are no longer valid.
-            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
         // cause of that failure. The only thing left is to insert
@@ -1542,23 +1464,6 @@ Dhcpv6Srv::assignIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
     return (ia_rsp);
 }
 
-void
-Dhcpv6Srv::conditionalNCRRemoval(const Pkt6Ptr& query, Lease6Ptr& old_lease,
-                                 Lease6Ptr& new_lease, const std::string& hostname,
-                                 bool do_fwd, bool do_rev) {
-    if (old_lease && !new_lease->hasIdenticalFqdn(*old_lease)) {
-        LOG_DEBUG(ddns6_logger, DBG_DHCP6_DETAIL, DHCP6_DDNS_LEASE_ASSIGN_FQDN_CHANGE)
-            .arg(query->getLabel())
-            .arg(old_lease->toText())
-            .arg(hostname)
-            .arg(do_rev ? "true" : "false")
-            .arg(do_fwd ? "true" : "false");
-
-        // Schedule removal of the existing lease.
-        createRemovalNameChangeRequest(query, old_lease);
-    }
-}
-
 OptionPtr
 Dhcpv6Srv::assignIA_PD(const Pkt6Ptr& query, const Pkt6Ptr& answer,
                        AllocEngine::ClientContext6& orig_ctx,
@@ -1792,7 +1697,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);
         }
     }
 
@@ -2207,7 +2112,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);
     }
@@ -2834,7 +2739,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

@@ -525,12 +525,11 @@ protected:
     /// The @c isc::dhcp_ddns::NameChangeRequest class encapsulates the request
     /// from the DHCPv6 server to the DHCP-DDNS module to perform DNS Update.
     /// The FQDN option carries response to the client about DNS updates that
-    /// server intents to perform for the DNS client. Based on this, the
+    /// server intends 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 for 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.
@@ -801,22 +781,6 @@ private:
     /// as a programmatic error.
     void generateFqdn(const Pkt6Ptr& answer);
 
-
-    /// @brief Triggers removal Name Change Request if FQDN data changes in leases
-    ///
-    /// If there are any differences (different fwd or rev flags, or different
-    /// hostname) a DNS update for removing entry will be generated.
-    ///
-    /// @param query a pointer to the client's message
-    /// @param old_lease old version of the lease
-    /// @param new_lease new version of the lease (may be NULL)
-    /// @param hostname specifies hostname (for printing purposes)
-    /// @param do_fwd specifies if reverse updates are enabled (for printing purposes)
-    /// @param do_rev specifies if reverse updates are enabled (for printing purposes)
-    void conditionalNCRRemoval(const Pkt6Ptr& query, Lease6Ptr& old_lease,
-                               Lease6Ptr& new_lease, const std::string& hostname,
-                               bool do_fwd, bool do_rev);
-
     /// @brief Updates statistics for received packets
     /// @param query packet received
     static void processStatsReceived(const Pkt6Ptr& query);

src/bin/dhcp6/tests/dhcp6_test_utils.h

@@ -106,7 +106,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;
@@ -653,7 +655,7 @@ TEST_F(FqdnDhcpv6SrvTest, createNameChangeRequestsNoAddr) {
 
     ASSERT_NO_THROW(srv_->createNameChangeRequests(answer));
 
-    // We didn't add any IAs, so there should be no NameChangeRequests in th
+    // We didn't add any IAs, so there should be no NameChangeRequests in the
     // queue.
     ASSERT_EQ(0, d2_mgr_.getQueueSize());
 }
@@ -725,20 +727,19 @@ 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);
 
 }
 
-// Checks that NameChangeRequests to remove entries are not created
-// when ddns updates are disabled.
+// Checks that calling queueNCR would not result in error if DDNS updates are
+// disabled. 
 TEST_F(FqdnDhcpv6SrvTest, noRemovalsWhenDisabled) {
     // Disable DDNS updates.
     disableD2();
@@ -750,9 +751,11 @@ TEST_F(FqdnDhcpv6SrvTest, noRemovalsWhenDisabled) {
     // as if we typed domain-name in lower case.
     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_));
+    // When DDNS is disabled an attempt to send a request should not throw, but
+    // nothing is generated. Unfortunately, we can't see if anything get
+    // generated because getting anything from the queue when DDNS is disabled
+    // would result in exception.
+    ASSERT_NO_THROW(queueNCR(CHG_REMOVE, lease_));
 }
 
 
@@ -763,8 +766,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 +774,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestRev) {
                             "2001:db8:1::1",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            0, 502);
+                            lease_->cltt_ + lease_->valid_lft_, 502);
 
 }
 
@@ -782,8 +784,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 +798,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 +812,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 +845,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 +950,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",
@@ -958,8 +964,8 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRelease) {
                             0, 4000);
 
     // Client may send Release message. In this case the lease should be
-    // removed and all existing DNS entries for this lease should be
-    // also removed. Therefore, we expect that single NameChangeRequest to
+    // removed and all existing DNS entries for this lease should also
+    // be removed. Therefore, we expect that single NameChangeRequest to
     // remove DNS entries is generated.
     testProcessMessage(DHCPV6_RELEASE, "otherhost.example.com",
                        "otherhost.example.com.");
@@ -968,7 +974,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRelease) {
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            0, 4000);
+                            lease_->cltt_ + lease_->valid_lft_, 4000);
 
 }
 
@@ -1028,15 +1034,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 +1073,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/dhcp/pkt4.cc

@@ -357,15 +357,26 @@ Pkt4::getLabel() const {
 std::string
 Pkt4::makeLabel(const HWAddrPtr& hwaddr, const ClientIdPtr& client_id,
                 const uint32_t transid) {
+    // Create label with HW address and client identifier.
+    stringstream label;
+    label << makeLabel(hwaddr, client_id);
+
+    // Append transaction id.
+    label << ", tid=0x" << hex << transid << dec;
+
+    return label.str();
+}
+
+std::string
+Pkt4::makeLabel(const HWAddrPtr& hwaddr, const ClientIdPtr& client_id) {
     stringstream label;
     label << "[" << (hwaddr ? hwaddr->toText() : "no hwaddr info")
           << "], cid=[" << (client_id ? client_id->toText() : "no info")
-          << "], tid=0x" << hex << transid << dec;
+          << "]";
 
     return label.str();
 }
 
-
 std::string
 Pkt4::toText() const {
     stringstream output;

src/lib/dhcp/pkt4.h

@@ -119,6 +119,15 @@ public:
                                  const ClientIdPtr& client_id,
                                  const uint32_t transid);
 
+    /// @brief Returns text representation of the given packet identifers.
+    ///
+    /// This variant of the method does not include transaction id.
+    ///
+    /// @param hwaddr hardware address to include in the string, it may be
+    /// NULL.
+    /// @param client_id client id to include in the string, it may be NULL.
+    static std::string makeLabel(const HWAddrPtr& hwaddr, const ClientIdPtr& client_id);
+
     /// @brief Returns text representation of the packet.
     ///
     /// This function is useful mainly for debugging.

src/lib/dhcp/pkt6.cc

@@ -498,22 +498,31 @@ Pkt6::getMACFromDUID() {
 std::string
 Pkt6::makeLabel(const DuidPtr duid, const uint32_t transid,
                 const HWAddrPtr& hwaddr) {
+    // Create label with DUID and HW address.
+    std::stringstream label;
+    label << makeLabel(duid, hwaddr);
+
+    // Append transaction id.
+    label << ", tid=0x" << std::hex << transid << std::dec;
+
+    return (label.str());
+}
+
+std::string
+Pkt6::makeLabel(const DuidPtr duid, const HWAddrPtr& hwaddr) {
     std::stringstream label;
     // DUID should be present at all times, so explicitly inform when
     // it is no present (no info).
     label << "duid=[" << (duid ? duid->toText() : "no info")
-          << "],";
+          << "]";
 
     // HW address is typically not carried in the DHCPv6 mmessages
     // and can be extracted using various, but not fully reliable,
     // techniques. If it is not present, don't print anything.
     if (hwaddr) {
-        label << " [" << hwaddr->toText() << "],";
+        label << ", [" << hwaddr->toText() << "]";
     }
 
-    // Transaction id is always there.
-    label << " tid=0x" << std::hex << transid << std::dec;
-
     return (label.str());
 }
 

src/lib/dhcp/pkt6.h

@@ -173,6 +173,16 @@ public:
     static std::string makeLabel(const DuidPtr duid, const uint32_t transid,
                                  const HWAddrPtr& hwaddr);
 
+    /// @brief Returns text representation of the given packet identifiers.
+    ///
+    /// This variant of the method does not include transaction id.
+    ///
+    /// @param duid Pointer to the client identifier or NULL.
+    /// @param hwaddr Hardware address to include in the string or NULL.
+    ///
+    /// @return String with text representation of the packet identifiers.
+    static std::string makeLabel(const DuidPtr duid, const HWAddrPtr& hwaddr);
+
     /// @brief Returns text representation of the primary packet identifiers
     ///
     /// This method is intended to be used to provide a consistent way to

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -1024,6 +1024,27 @@ TEST_F(Pkt4Test, getLabel) {
 
 }
 
+// Tests that the variant of makeLabel which doesn't include transaction
+// id produces expected output.
+TEST_F(Pkt4Test, makeLabelWithoutTransactionId) {
+    EXPECT_EQ("[no hwaddr info], cid=[no info]",
+              Pkt4::makeLabel(HWAddrPtr(), ClientIdPtr()));
+
+    // Test non-null hardware address.
+    HWAddrPtr hwaddr(new HWAddr(HWAddr::fromText("01:02:03:04:05:06", 123)));
+    EXPECT_EQ("[hwtype=123 01:02:03:04:05:06], cid=[no info]",
+              Pkt4::makeLabel(hwaddr, ClientIdPtr()));
+
+    // Test non-null client identifier and non-null hardware address.
+    ClientIdPtr cid = ClientId::fromText("01:02:03:04");
+    EXPECT_EQ("[hwtype=123 01:02:03:04:05:06], cid=[01:02:03:04]",
+              Pkt4::makeLabel(hwaddr, cid));
+
+    // Test non-nnull client identifier and null hardware address.
+    EXPECT_EQ("[no hwaddr info], cid=[01:02:03:04]",
+              Pkt4::makeLabel(HWAddrPtr(), cid));
+}
+
 // Tests that the correct DHCPv4 message name is returned for various
 // message types.
 TEST_F(Pkt4Test, getName) {

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -1475,6 +1475,30 @@ TEST_F(Pkt6Test, makeLabel) {
               Pkt6::makeLabel(DuidPtr(), 0x0, HWAddrPtr()));
 }
 
+// Tests that the variant of makeLabel which doesn't include transaction
+// id produces expected output.
+TEST_F(Pkt6Test, makeLabelWithoutTransactionId) {
+    DuidPtr duid(new DUID(DUID::fromText("0102020202030303030303")));
+    HWAddrPtr hwaddr(new HWAddr(HWAddr::fromText("01:02:03:04:05:06",
+                                                 HTYPE_ETHER)));
+
+    // Specify DUID and no HW Address.
+    EXPECT_EQ("duid=[01:02:02:02:02:03:03:03:03:03:03]",
+              Pkt6::makeLabel(duid, HWAddrPtr()));
+
+    // Specify HW Address and no DUID.
+    EXPECT_EQ("duid=[no info], [hwtype=1 01:02:03:04:05:06]",
+              Pkt6::makeLabel(DuidPtr(), hwaddr));
+
+    // Specify both DUID and HW Address.
+    EXPECT_EQ("duid=[01:02:02:02:02:03:03:03:03:03:03], "
+              "[hwtype=1 01:02:03:04:05:06]",
+              Pkt6::makeLabel(duid, hwaddr));
+
+    // Specify neither DUID nor HW Address.
+    EXPECT_EQ("duid=[no info]", Pkt6::makeLabel(DuidPtr(), HWAddrPtr()));
+}
+
 // This test verifies that it is possible to obtain the packet
 // identifiers in the textual format from the packet instance.
 TEST_F(Pkt6Test, getLabel) {

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,17 +14,17 @@
 
 #include <config.h>
 
-#include <dhcp/option_data_types.h>
+#include <dhcp/dhcp6.h>
+#include <dhcp/pkt4.h>
+#include <dhcp/pkt6.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 <dhcp/dhcp6.h>
+#include <dhcpsrv/ncr_generator.h>
 #include <hooks/callout_handle.h>
 #include <hooks/hooks_manager.h>
 #include <stats/stats_mgr.h>
@@ -831,6 +831,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 +897,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 +943,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 +963,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 +1197,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 +1217,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 +1227,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 +1277,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 +1376,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 status?
-            /// 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 +1516,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 status?
-            /// 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 +1578,200 @@ 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) {
+
+    LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
+              ALLOC_ENGINE_V6_LEASE_RECLAIM)
+        .arg(Pkt6::makeLabel(lease->duid_, lease->hwaddr_))
+        .arg(lease->addr_.toText())
+        .arg(static_cast<int>(lease->prefixlen_));
+
+    // 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 status?
+    /// 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) {
+
+    LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
+              ALLOC_ENGINE_V4_LEASE_RECLAIM)
+        .arg(Pkt4::makeLabel(lease->hwaddr_, lease->client_id_))
+        .arg(lease->addr_.toText());
+
+    // 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 status?
+    /// 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 +1856,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 +1870,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 +1878,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 +2468,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 +2545,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
+    /// removed 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/alloc_engine_messages.mes

@@ -75,6 +75,11 @@ client sending the DHCPDISCOVER has a reservation for the specified
 address. The allocation engine will try to offer this address to
 the client.
 
+% ALLOC_ENGINE_V4_LEASE_RECLAIM %1: reclaiming expired lease for address %2
+This debug message is issued when the server begins reclamation of the
+expired DHCPv4 lease. The first argument specifies the client identification
+information. The second argument holds the leased IPv4 address.
+
 % ALLOC_ENGINE_V4_LEASE_RECLAMATION_FAILED failed to reclaim the lease %1: %2
 This error message is logged when the allocation engine fails to
 reclaim an expired lease. The reason for the failure is included in the
@@ -340,6 +345,13 @@ reserved for it.
 This informational message signals that the specified client was assigned the prefix
 reserved for it.
 
+% ALLOC_ENGINE_V6_LEASE_RECLAIM %1: reclaiming expired lease for prefix %2/%3
+This debug message is issued when the server begins reclamation of the
+expired DHCPv6 lease. The reclaimed lease may either be an address lease
+or delegated prefix. The first argument provides the client identification
+information. The other arguments specify the prefix and the prefix length
+for the lease. The prefix length for address lease is equal to 128.
+
 % ALLOC_ENGINE_V6_LEASE_RECLAMATION_FAILED failed to reclaim the lease %1: %2
 This error message is logged when the allocation engine fails to
 reclaim an expired lease. The reason for the failure is included in the

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -609,6 +609,27 @@ 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 %1: name change request to %2 DNS entry queued: %3
+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 argument
+includes the client identification information. The second argument
+indicates whether the DNS entry is to be added or removed. The third
+argument carries the details of the NameChangeRequest.
+
+% DHCPSRV_QUEUE_NCR_FAILED %1: queueing %2 name change request failed for lease %3: %4
+This error message is logged when sending a name change request
+to DHCP DDNS failed. The first argument includes the client identification
+information. The second argument indicates whether the DNS entry is to be
+added or removed. The third argument specifies the leased address. The
+last argument provides the reason for failure.
+
+% DHCPSRV_QUEUE_NCR_SKIP %1: skip queueing name change request for lease: %2
+This debug message is issued when the server decides to not queue the name
+change request because the lease doesn't include the FQDN, the forward and
+reverse update is disabled for this lease or the DNS updates are disabled
+in the configuration. The first argument includes the client identification
+information. The second argument includes the leased address.
+
 % 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,117 @@
+// 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.
+/// @param label Client identification information in the textual format.
+/// This is used for logging purposes.
+///
+/// @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, const std::string& label) {
+
+    // Check if there is a need for update.
+    if (lease->hostname_.empty() || (!lease->fqdn_fwd_ && !lease->fqdn_rev_)
+        || !CfgMgr::instance().getD2ClientMgr().ddnsEnabled()) {
+        LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+                  DHCPSRV_QUEUE_NCR_SKIP)
+            .arg(label)
+            .arg(lease->addr_.toText());
+            
+        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(label)
+            .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(label)
+            .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(),
+                           Pkt4::makeLabel(lease->hwaddr_, lease->client_id_));
+
+        } else {
+            // Client id is not specified for the lease. Use HW address
+            // instead.
+            queueNCRCommon(chg_type, lease, lease->hwaddr_,
+                           Pkt4::makeLabel(lease->hwaddr_, lease->client_id_));
+        }
+    }
+}
+
+void queueNCR(const NameChangeType& chg_type, const Lease6Ptr& lease) {
+    // DUID is required to generate NCR.
+    if (lease && (lease->type_ != Lease::TYPE_PD) && lease->duid_) {
+        queueNCRCommon(chg_type, lease, *(lease->duid_),
+                       Pkt6::makeLabel(lease->duid_, lease->hwaddr_));
+    }
+}
+
+}
+}

src/lib/dhcpsrv/ncr_generator.h

@@ -0,0 +1,55 @@
+// 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 will skip sending the NCR if the lease type is a delegated
+/// prefix.
+///
+/// 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
@@ -1111,6 +1127,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.
@@ -1161,6 +1185,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()
@@ -1188,7 +1222,10 @@ ExpirationAllocEngine6Test::createLeases() {
                                    20, SubnetID(1), true, true,
                                    generateHostnameForLeaseIndex(i)));
         leases_.push_back(lease);
-        LeaseMgrFactory::instance().addLease(lease);
+        // Copy the lease before adding it to the lease manager. We want to
+        // make sure that modifications to the leases held in the leases_
+        // container doesn't affect the leases in the lease manager.
+        LeaseMgrFactory::instance().addLease(Lease6Ptr(new Lease6(*lease)));
 
         // Note in the statistics that this lease has been added.
         StatsMgr& stats_mgr = StatsMgr::instance();
@@ -1200,6 +1237,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) {
@@ -1280,6 +1330,73 @@ ExpirationAllocEngine6Test::testReclaimExpiredLeasesStats() {
     }
 }
 
+void
+ExpirationAllocEngine6Test::testReclaimReusedLeases(const uint16_t msg_type,
+                                                    const bool use_reclaimed) {
+    BOOST_STATIC_ASSERT(TEST_LEASES_NUM < 1000);
+
+    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_->allocateLeases6(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".
@@ -1383,6 +1500,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!
@@ -1417,6 +1570,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.
@@ -1472,6 +1634,18 @@ 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 being
+    /// reused should initially be reclaimed.
+    void testReclaimReusedLeases(const uint8_t msg_type, const bool client_renews,
+                                 const bool use_reclaimed);
+
 };
 
 ExpirationAllocEngine4Test::ExpirationAllocEngine4Test()
@@ -1502,7 +1676,10 @@ ExpirationAllocEngine4Test::createLeases() {
                                    time(NULL), SubnetID(1), true, true,
                                    generateHostnameForLeaseIndex(i)));
         leases_.push_back(lease);
-        LeaseMgrFactory::instance().addLease(lease);
+        // Copy the lease before adding it to the lease manager. We want to
+        // make sure that modifications to the leases held in the leases_
+        // container doesn't affect the leases in the lease manager.
+        LeaseMgrFactory::instance().addLease(Lease4Ptr(new Lease4(*lease)));
 
         // Note in the statistics that this lease has been added.
         StatsMgr& stats_mgr = StatsMgr::instance();
@@ -1539,6 +1716,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) {
@@ -1674,6 +1874,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.
+    BOOST_STATIC_ASSERT(TEST_LEASES_NUM < 1000);
+
+    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
@@ -1784,4 +2055,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,611 @@
+// 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 has 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 that calling queueNCR for NULL lease doesn't cause
+    /// an exception.
+    ///
+    /// @param chg_type Name change type.
+    void testNullLease(const NameChangeType chg_type) {
+        lease_.reset();
+        ASSERT_NO_FATAL_FAILURE(queueNCR(chg_type, lease_));
+        EXPECT_EQ(0, d2_mgr_.getQueueSize());
+    }
+};
+
+/// @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");
+    }
+
+    // Now try the same test with all 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");
+    }
+
+    {
+        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, revOnly) {
+    {
+        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 creation of the NameChangeRequest for forward mapping in the
+// given lease.
+TEST_F(NCRGenerator6Test, fwdOnly) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, true, false, "myhost.example.com.",
+                "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
+                "D0ECCEA5E0DD71730F392119A");
+    }
+
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, true, false, "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 an 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, "myhost...example.com.");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNoUpdate(CHG_ADD, false, false, "myhost...example.com.");
+    }
+}
+
+// Test that NameChangeRequest is not generated if the lease is not an
+// address lease, i.e. is a prefix.
+TEST_F(NCRGenerator6Test, wrongLeaseType) {
+    // Change lease type to delegated prefix.
+    lease_->type_ = Lease::TYPE_PD;
+
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNoUpdate(CHG_REMOVE, true, true, "myhost.example.org.");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNoUpdate(CHG_ADD, true, true, "myhost.example.org.");
+    }
+}
+
+// Test that NameChangeRequest is not generated if the lease is NULL,
+// and that the call to queueNCR doesn't cause an exception or
+// assertion.
+TEST_F(NCRGenerator6Test, nullLease) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNullLease(CHG_REMOVE);
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNullLease(CHG_ADD);
+    }
+}
+
+/// @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 DHCPv4 lease instance.
+    virtual void initLease() {
+        lease_.reset(new Lease4(IOAddress("192.0.2.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");
+    }
+
+    // Now try the same with all 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");
+    }
+
+    {
+        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, revOnly) {
+    {
+        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 creation of the NameChangeRequest for forward mapping in the
+// given lease.
+TEST_F(NCRGenerator4Test, fwdOnly) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, true, false, "myhost.example.com.",
+                "000001E356D43E5F0A496D65BCA24D982D646140813E3B"
+                "03AB370BFF46BFA309AE7BFD");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, true, false, "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, "myhost...example.org.");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNoUpdate(CHG_ADD, false, false, "myhost...example.org.");
+    }
+}
+
+// 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,
+                            "192.0.2.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");
+    }
+}
+
+// Test that NameChangeRequest is not generated if the lease is NULL,
+// and that the call to queueNCR doesn't cause an exception or
+// assertion.
+TEST_F(NCRGenerator4Test, nullLease) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNullLease(CHG_REMOVE);
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNullLease(CHG_ADD);
+    }
+}
+
+} // end of anonymous namespace

tools/.gitignore

@@ -1 +1,2 @@
 /path_replacer.sh
+/system_messages