Revert "[#1409] Correct DNS handling when reusing expired leases"

These changes overlook the fact that if a lease has expired, then
it's DNS entries have also expired and may or may not still be
in DNS. In any event, they need to be made anew.

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index ceb2a35a4aa75cf7500d4bb744e953822d45a11c..95629d6fb66f109c32c2ead1d370f80d09aaadd8 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -1847,15 +1847,7 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer,
     // FQDN option will be NULL. This is valid state, so we simply return.
     Option6ClientFqdnPtr opt_fqdn = boost::dynamic_pointer_cast<
         Option6ClientFqdn>(answer->getOption(D6O_CLIENT_FQDN));
-
     if (!opt_fqdn) {
-        // This lease has no FQDN, remove DNS entries for any changed leases
-        // that have it.
-        for (Lease6Collection::const_iterator l = ctx.currentIA().changed_leases_.begin();
-                l != ctx.currentIA().changed_leases_.end(); ++l) {
-            queueNCR(CHG_REMOVE, *l);
-        }
-
         return;
     }
 

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 0bf792b165f755e39a07879f96b629841d438099..84c025a028181a5dc499f91a61ae2a8a9379e855 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -941,10 +941,9 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
             if (!host) {
 
                 // Copy an existing, expired lease so as it can be returned
-                // to the caller as changed so caller can deal with DNS as
-                // needed.
+                // to the caller.
                 Lease6Ptr old_lease(new Lease6(*lease));
-                ctx.currentIA().changed_leases_.push_back(old_lease);
+                ctx.currentIA().old_leases_.push_back(old_lease);
 
                 /// We found a lease and it is expired, so we can reuse it
                 lease = reuseExpiredLease(lease, ctx, pool->getLength(),
@@ -2531,6 +2530,7 @@ 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_))
@@ -2564,6 +2564,12 @@ AllocEngine::reclaimExpiredLease(const Lease6Ptr& lease,
     /// 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 immediately 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 perform a couple extra steps.
         bool remove_lease = (reclaim_mode == DB_RECLAIM_REMOVE);
@@ -2580,15 +2586,6 @@ AllocEngine::reclaimExpiredLease(const Lease6Ptr& lease,
         }
 
         if (reclaim_mode != DB_RECLAIM_LEAVE_UNCHANGED) {
-            // Generate removal name change request for D2, if required.
-            // We're doing this here because the lease either being updated
-            // or removed.  If the lease is being reused, higher level logic
-            // will decide what to do for DNS and we won't be in this block.
-            queueNCR(CHG_REMOVE, lease);
-            lease->hostname_.clear();
-            lease->fqdn_fwd_ = false;
-            lease->fqdn_rev_ = false;
-
             // Reclaim the lease - depending on the configuration, set the
             // expired-reclaimed state or simply remove it.
             LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
@@ -2631,6 +2628,7 @@ 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_))
@@ -2662,6 +2660,12 @@ AllocEngine::reclaimExpiredLease(const Lease4Ptr& lease,
     /// 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 immediately 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 perform a couple extra steps.
         bool remove_lease = (reclaim_mode == DB_RECLAIM_REMOVE);
@@ -2678,15 +2682,6 @@ AllocEngine::reclaimExpiredLease(const Lease4Ptr& lease,
         }
 
         if (reclaim_mode != DB_RECLAIM_LEAVE_UNCHANGED) {
-            // Generate removal name change request for D2, if required.
-            // We're doing this here because the lease either being updated
-            // or removed.  If the lease is being reused, higher level logic
-            // will decide what to do for DNS and we won't be in this block.
-            queueNCR(CHG_REMOVE, lease);
-            lease->hostname_.clear();
-            lease->fqdn_fwd_ = false;
-            lease->fqdn_rev_ = false;
-
             // Reclaim the lease - depending on the configuration, set the
             // expired-reclaimed state or simply remove it.
             LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
@@ -2858,8 +2853,14 @@ void AllocEngine::reclaimLeaseInDatabase(const LeasePtrType& lease,
     if (remove_lease) {
         lease_mgr.deleteLease(lease);
     } else if (lease_update_fun) {
+        // Clear FQDN information as we have already sent the
+        // name change request to remove the DNS record.
+        lease->hostname_.clear();
+        lease->fqdn_fwd_ = false;
+        lease->fqdn_rev_ = false;
         lease->state_ = Lease::STATE_EXPIRED_RECLAIMED;
         lease_update_fun(lease);
+
     } else {
         return;
     }

diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
index cf1481915635d3c51289540dddfa8f7e8ec6f0ce..34c316a50fef44f39f4ee2e18fa36089887d0022 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
@@ -1203,15 +1203,14 @@ TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
     EXPECT_FALSE(lease->fqdn_fwd_);
     EXPECT_FALSE(lease->fqdn_rev_);
 
-    // Check that the changed lease has been returned.
-    Lease6Ptr changed_lease = expectOneLease(ctx.currentIA().changed_leases_);
-    ASSERT_TRUE(changed_lease);
+    // Check that the old lease has been returned.
+    Lease6Ptr old_lease = expectOneLease(ctx.currentIA().old_leases_);
     // It should at least have the same IPv6 address.
-    EXPECT_EQ(lease->addr_, changed_lease->addr_);
+    EXPECT_EQ(lease->addr_, old_lease->addr_);
     // Check that it carries not updated FQDN data.
-    EXPECT_EQ("myhost.example.com.", changed_lease->hostname_);
-    EXPECT_TRUE(changed_lease->fqdn_fwd_);
-    EXPECT_TRUE(changed_lease->fqdn_rev_);
+    EXPECT_EQ("myhost.example.com.", old_lease->hostname_);
+    EXPECT_TRUE(old_lease->fqdn_fwd_);
+    EXPECT_TRUE(old_lease->fqdn_rev_);
 
     // Check that the lease is indeed updated in LeaseMgr
     Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA,