[3965] Delete expired-reclaimed leases from the lease file.

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -231,6 +231,23 @@ A debug message issued when the server is attempting to delete a lease
 for the specified address from the memory file database for the specified
 address.
 
+% DHCPSRV_MEMFILE_DELETE_EXPIRED_RECLAIMED4 deleting reclaimed leases expired more than %1 s ago
+A debug message issued when the server is removing reclaimed DHCPv4
+leases which have expired longer than a specified period of time.
+The argument specified the number of seconds since leases' expiration
+before they can be removed.
+
+% DHCPSRV_MEMFILE_DELETE_EXPIRED_RECLAIMED6 deleting reclaimed leases expired more than %1 s ago
+A debug message issued when the server is removing reclaimed DHCPv6
+leases which have expired longer than a specified period of time.
+The argument specified the number of seconds since leases' expiration
+before they can be removed.
+
+% DHCPSRV_MEMFILE_DELETE_EXPIRED_RECLAIMED_START starting deletion of %1 expired-reclaimed leases
+A debug message issued wheb the server has found expired-reclaimed
+leases to be removed. The number of leases to be removed is logged
+in the message.
+
 % DHCPSRV_MEMFILE_GET_ADDR4 obtaining IPv4 lease for address %1
 A debug message issued when the server is attempting to obtain an IPv4
 lease from the memory file database for the specified address.

src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -648,18 +648,29 @@ Memfile_LeaseMgr::deleteLease(const isc::asiolink::IOAddress& addr) {
 
 void
 Memfile_LeaseMgr::deleteExpiredReclaimedLeases4(const uint32_t secs) {
-    deleteExpiredReclaimedLeases<Lease4StorageExpirationIndex>(secs, storage4_);
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MEMFILE_DELETE_EXPIRED_RECLAIMED4)
+        .arg(secs);
+    deleteExpiredReclaimedLeases<Lease4StorageExpirationIndex, Lease4>(secs, V4, storage4_,
+                                                                       lease_file4_);
 }
 
 void
 Memfile_LeaseMgr::deleteExpiredReclaimedLeases6(const uint32_t secs) {
-    deleteExpiredReclaimedLeases<Lease6StorageExpirationIndex>(secs, storage6_);
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MEMFILE_DELETE_EXPIRED_RECLAIMED6)
+        .arg(secs);
+    deleteExpiredReclaimedLeases<Lease6StorageExpirationIndex, Lease6>(secs, V6, storage6_,
+                                                                       lease_file6_);
 }
 
-template<typename IndexType, typename StorageType>
+template<typename IndexType, typename LeaseType, typename StorageType,
+         typename LeaseFileType>
 void
 Memfile_LeaseMgr::deleteExpiredReclaimedLeases(const uint32_t secs,
-                                               StorageType& storage) const {
+                                               const Universe& universe,
+                                               StorageType& storage,
+                                               LeaseFileType& lease_file) const {
     // Obtain the index which segragates leases by state and time.
     IndexType& index = storage.template get<ExpirationIndexTag>();
 
@@ -686,7 +697,29 @@ Memfile_LeaseMgr::deleteExpiredReclaimedLeases(const uint32_t secs,
         index.upper_bound(boost::make_tuple(true, std::numeric_limits<int64_t>::min()));
 
     // If there are some elements in this range, delete them.
-    if (std::distance(lower_limit, upper_limit) > 0) {
+    uint64_t num_leases = static_cast<uint64_t>(std::distance(lower_limit, upper_limit));
+    if (num_leases > 0) {
+
+        LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+                  DHCPSRV_MEMFILE_DELETE_EXPIRED_RECLAIMED_START)
+            .arg(num_leases);
+
+        // If lease persistence is enabled, we also have to mark leases
+        // as deleted in the lease file. We do this by setting the
+        // lifetime to 0.
+        if (persistLeases(universe)) {
+            for (typename IndexType::const_iterator lease = lower_limit;
+                 lease != upper_limit; ++lease) {
+                // Copy lease to not affect the lease in the container.
+                LeaseType lease_copy(**lease);
+                // Set the valid lifetime to 0 to indicate the removal
+                // of the lease.
+                lease_copy.valid_lft_ = 0;
+                lease_file->append(lease_copy);
+            }
+        }
+
+        // Erase leases from memory.
         index.erase(lower_limit, upper_limit);
     }
 }

src/lib/dhcpsrv/memfile_lease_mgr.h

@@ -340,17 +340,26 @@ private:
     /// @param secs Number of seconds since expiration of leases before
     /// they can be removed. Leases which have expired later than this
     /// time will not be deleted.
+    /// @param universe V4 or V6.
     /// @param storage Reference to the container where leases are held.
     /// Some expired-reclaimed leases will be removed from this container.
+    /// @param lease_file Reference to a DHCPv4 or DHCPv6 lease file
+    /// instance where leases should be marked as deleted.
     ///
     /// @tparam IndexType Index type to be used to search for the
     /// expired-reclaimed leases, i.e.
     /// @c Lease4StorageExpirationIndex or @c Lease6StorageExpirationIndex.
+    /// @tparam LeaseType Lease type, i.e. @c Lease4 or @c Lease6.
     /// @tparam StorageType Type of storage where leases are held, i.e.
     /// @c Lease4Storage or @c Lease6Storage.
-    template<typename IndexType, typename StorageType>
+    /// @tparam LeaseFileType Type of the lease file, i.e. DHCPv4 or
+    /// DHCPv6 lease file type.
+    template<typename IndexType, typename LeaseType, typename StorageType,
+             typename LeaseFileType>
     void deleteExpiredReclaimedLeases(const uint32_t secs,
-                                      StorageType& storage) const;
+                                      const Universe& universe,
+                                      StorageType& storage,
+                                      LeaseFileType& lease_file) const;
 
 public:
 

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc

@@ -1941,14 +1941,14 @@ GenericLeaseMgrTest::testDeleteExpiredReclaimedLeases4() {
         // 15 seconds ago, the lease should have been deleted.
         if (leases[i]->stateExpiredReclaimed() &&
             ((leases[i]->getExpirationTime() + lease_affinity_time) < current_time)) {
-            EXPECT_FALSE(lease) << "Lease with index " << i
-                << " should have been deleted, but it was not";
+            EXPECT_FALSE(lease) << "The following lease should have been"
+                " deleted: " << leases[i]->toText();
 
         } else {
             // If the lease is not reclaimed or it has expired less than
             // 15 seconds ago, the lease should still be there.
-            EXPECT_TRUE(lease) << "Lease with index " << i
-                << " shouldn't have been deleted, but it was";
+            EXPECT_TRUE(lease) << "The following lease shouldn't have been"
+                " deleted: " << leases[i]->toText();
         }
     }
 
@@ -1956,7 +1956,17 @@ GenericLeaseMgrTest::testDeleteExpiredReclaimedLeases4() {
     // to be deleted.
     ASSERT_NO_THROW(lmptr_->deleteExpiredReclaimedLeases4(lease_affinity_time));
 
+    // Reopen the database. This to ensure that the leases have been deleted
+    // from the persistent storage.
+    reopen(V4);
+
     for (size_t i = 0; i < leases.size(); ++i) {
+        /// @todo Leases with empty HW address are dropped by the memfile
+        /// backend. We will have to reevaluate if this is right behavior
+        /// of the backend when client identifier is present.
+        if (leases[i]->hwaddr_ && leases[i]->hwaddr_->hwaddr_.empty()) {
+            continue;
+        }
         // Obtain lease from the server.
         Lease4Ptr lease = lmptr_->getLease4(leases[i]->addr_);
 
@@ -1964,14 +1974,14 @@ GenericLeaseMgrTest::testDeleteExpiredReclaimedLeases4() {
         // 15 seconds ago, the lease should have been deleted.
         if (leases[i]->stateExpiredReclaimed() &&
             ((leases[i]->getExpirationTime() + lease_affinity_time) < current_time)) {
-            EXPECT_FALSE(lease) << "Lease with index " << i
-                << " should have been deleted, but it was not";
+            EXPECT_FALSE(lease) << "The following lease should have been"
+                " deleted: " << leases[i]->toText();
 
         } else {
             // If the lease is not reclaimed or it has expired less than
             // 15 seconds ago, the lease should still be there.
-            EXPECT_TRUE(lease) << "Lease with index " << i
-                << " shouldn't have been deleted, but it was";
+            EXPECT_TRUE(lease) << "The following lease shouldn't have been"
+                " deleted: " << leases[i]->toText();
         }
     }
 }
@@ -2020,14 +2030,14 @@ GenericLeaseMgrTest::testDeleteExpiredReclaimedLeases6() {
         // 15 seconds ago, the lease should have been deleted.
         if (leases[i]->stateExpiredReclaimed() &&
             ((leases[i]->getExpirationTime() + lease_affinity_time) < current_time)) {
-            EXPECT_FALSE(lease) << "Lease with index " << i
-                << " should have been deleted, but it was not";
+            EXPECT_FALSE(lease) << "The following lease should have been"
+                " deleted: " << leases[i]->toText();
 
         } else {
             // If the lease is not reclaimed or it has expired less than
             // 15 seconds ago, the lease should still be there.
-            EXPECT_TRUE(lease) << "Lease with index " << i
-                << " shouldn't have been deleted, but it was";
+            EXPECT_TRUE(lease) << "The following lease shouldn't have been"
+                " deleted: " << leases[i]->toText();
         }
     }
 
@@ -2035,6 +2045,10 @@ GenericLeaseMgrTest::testDeleteExpiredReclaimedLeases6() {
     // to be deleted.
     ASSERT_NO_THROW(lmptr_->deleteExpiredReclaimedLeases6(lease_affinity_time));
 
+    // Reopen the database. This to ensure that the leases have been deleted
+    // from the persistent storage.
+    reopen(V6);
+
     for (size_t i = 0; i < leases.size(); ++i) {
         // Obtain lease from the server.
         Lease6Ptr lease = lmptr_->getLease6(leases[i]->type_, leases[i]->addr_);
@@ -2043,14 +2057,14 @@ GenericLeaseMgrTest::testDeleteExpiredReclaimedLeases6() {
         // 15 seconds ago, the lease should have been deleted.
         if (leases[i]->stateExpiredReclaimed() &&
             ((leases[i]->getExpirationTime() + lease_affinity_time) < current_time)) {
-            EXPECT_FALSE(lease) << "Lease with index " << i
-                << " should have been deleted, but it was not";
+            EXPECT_FALSE(lease) << "The following lease should have been"
+                " deleted: " << leases[i]->toText();
 
         } else {
             // If the lease is not reclaimed or it has expired less than
             // 15 seconds ago, the lease should still be there.
-            EXPECT_TRUE(lease) << "Lease with index " << i
-                << " shouldn't have been deleted, but it was";
+            EXPECT_TRUE(lease) << "The following lease shouldn't have been"
+                " deleted: " << leases[i]->toText();
         }
     }
 }