Browse Source

[3747] Removed spurious check when generating NCRs.

Before creating NCRs the code used to check if there is a match
between a new and old lease and refused to send NCRs if there was
no match. This was wrong behavior because the allocation engine
could be reusing other client's lease in which case there could
be no match whatsoever and the NCRs should still be generated.
Marcin Siodelski 10 years ago
parent
commit
a829b4c3a6
2 changed files with 11 additions and 33 deletions
  1. 11 19
      src/bin/dhcp4/dhcp4_srv.cc
  2. 0 14
      src/bin/dhcp4/tests/fqdn_unittest.cc

+ 11 - 19
src/bin/dhcp4/dhcp4_srv.cc

@@ -969,29 +969,21 @@ Dhcpv4Srv::createNameChangeRequests(const Lease4Ptr& lease,
     // We may also decide not to generate any requests at all. This is when
     // 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.
     // we discover that nothing has changed in the client's FQDN data.
     if (old_lease) {
     if (old_lease) {
-        if (!lease->matches(*old_lease)) {
-            isc_throw(isc::Unexpected,
-                      "there is no match between the current instance of the"
-                      " lease: " << lease->toText() << ", and the previous"
-                      " instance: " << lease->toText());
-        } else {
-            // 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);
+        // 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.
             // If FQDN data from both leases match, there is no need to update.
-            } else if (lease->hasIdenticalFqdn(*old_lease)) {
-                return;
-
-            }
+        } else if (lease->hasIdenticalFqdn(*old_lease)) {
+            return;
 
 
         }
         }
+
     }
     }
 
 
     // We may need to generate the NameChangeRequest for the new lease. It
     // We may need to generate the NameChangeRequest for the new lease. It

+ 0 - 14
src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -765,20 +765,6 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsRenew) {
 
 
 }
 }
 
 
-// This test verifies that exception is thrown when leases passed to the
-// createNameChangeRequests function do not match, i.e. they comprise
-// different IP addresses, client ids etc.
-TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsLeaseMismatch) {
-    Lease4Ptr lease1 = createLease(IOAddress("192.0.2.3"),
-                                   "lease1.example.com.",
-                                   true, true);
-    Lease4Ptr lease2 = createLease(IOAddress("192.0.2.4"),
-                                   "lease2.example.com.",
-                                   true, true);
-    EXPECT_THROW(srv_->createNameChangeRequests(lease2, lease1),
-                 isc::Unexpected);
-}
-
 // Test that the OFFER message generated as a result of the DISCOVER message
 // Test that the OFFER message generated as a result of the DISCOVER message
 // processing will not result in generation of the NameChangeRequests.
 // processing will not result in generation of the NameChangeRequests.
 TEST_F(NameDhcpv4SrvTest, processDiscover) {
 TEST_F(NameDhcpv4SrvTest, processDiscover) {