[#3246] Do not delete soft released leases

8 files changed, 262 insertions, 28 deletions

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 6019012b1f..5e84357079 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -2445,7 +2445,7 @@ AllocEngine::updateLeaseData(ClientContext6& ctx, const Lease6Collection& leases
         if (!ctx.fake_allocation_) {
             bool update_stats = false;
 
-            if (lease->state_ == Lease::STATE_EXPIRED_RECLAIMED) {
+            if (lease->state_ == Lease::STATE_EXPIRED_RECLAIMED || lease->state_ == Lease::STATE_RELEASED) {
                 // Transition lease state to default (aka assigned)
                 lease->state_ = Lease::STATE_DEFAULT;
 
@@ -2908,6 +2908,20 @@ AllocEngine::reclaimExpiredLease(const Lease6Ptr& lease,
 
     // Update statistics.
 
+    // Increase number of reclaimed leases for a subnet.
+    StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
+                                                         lease->subnet_id_,
+                                                         "reclaimed-leases"),
+        static_cast<int64_t>(1));
+
+    // Increase total number of reclaimed leases.
+    StatsMgr::instance().addValue("reclaimed-leases", static_cast<int64_t>(1));
+
+    // Statistics must have been updated during the release.
+    if (lease->state_ == Lease::STATE_RELEASED) {
+        return;
+    }
+
     // Decrease number of assigned leases.
     if (lease->type_ == Lease::TYPE_NA) {
         // IA_NA
@@ -2959,15 +2973,6 @@ AllocEngine::reclaimExpiredLease(const Lease6Ptr& lease,
             }
         }
     }
-
-    // Increase number of reclaimed leases for a subnet.
-    StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
-                                                         lease->subnet_id_,
-                                                         "reclaimed-leases"),
-        static_cast<int64_t>(1));
-
-    // Increase total number of reclaimed leases.
-    StatsMgr::instance().addValue("reclaimed-leases", static_cast<int64_t>(1));
 }
 
 void
@@ -3043,11 +3048,8 @@ AllocEngine::reclaimExpiredLease(const Lease4Ptr& lease,
 
     // Update statistics.
 
-    // Decrease number of assigned addresses.
-    StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
-                                                         lease->subnet_id_,
-                                                         "assigned-addresses"),
-        static_cast<int64_t>(-1));
+    // Increase total number of reclaimed leases.
+    StatsMgr::instance().addValue("reclaimed-leases", static_cast<int64_t>(1));
 
     // Increase number of reclaimed leases for a subnet.
     StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
@@ -3055,6 +3057,17 @@ AllocEngine::reclaimExpiredLease(const Lease4Ptr& lease,
                                                          "reclaimed-leases"),
         static_cast<int64_t>(1));
 
+    // Statistics must have been updated during the release.
+    if (lease->state_ == Lease4::STATE_RELEASED) {
+        return;
+    }
+
+    // Decrease number of assigned addresses.
+    StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
+                                                         lease->subnet_id_,
+                                                         "assigned-addresses"),
+        static_cast<int64_t>(-1));
+
     auto const& subnet = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getBySubnetId(lease->subnet_id_);
     if (subnet) {
         auto const& pool = subnet->getPool(Lease::TYPE_V4, lease->addr_, false);
@@ -3072,9 +3085,6 @@ AllocEngine::reclaimExpiredLease(const Lease4Ptr& lease,
                 static_cast<int64_t>(1));
         }
     }
-
-    // Increase total number of reclaimed leases.
-    StatsMgr::instance().addValue("reclaimed-leases", static_cast<int64_t>(1));
 }
 
 void
@@ -4134,7 +4144,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
             .arg(ctx.query_->getLabel())
             .arg(client_lease->addr_.toText());
 
-        if (LeaseMgrFactory::instance().deleteLease(client_lease)) {
+        if (LeaseMgrFactory::instance().deleteLease(client_lease) && (client_lease->state_ != Lease4::STATE_RELEASED)) {
             // Need to decrease statistic for assigned addresses.
             StatsMgr::instance().addValue(
                 StatsMgr::generateName("subnet", client_lease->subnet_id_,
diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc
index 2ea916794d..373bd03504 100644
--- a/src/lib/dhcpsrv/lease.cc
+++ b/src/lib/dhcpsrv/lease.cc
@@ -24,9 +24,10 @@ using namespace std;
 namespace isc {
 namespace dhcp {
 
-const uint32_t Lease::STATE_DEFAULT = 0x0;
-const uint32_t Lease::STATE_DECLINED = 0x1;
-const uint32_t Lease::STATE_EXPIRED_RECLAIMED = 0x2;
+const uint32_t Lease::STATE_DEFAULT = 0;
+const uint32_t Lease::STATE_DECLINED = 1;
+const uint32_t Lease::STATE_EXPIRED_RECLAIMED = 2;
+const uint32_t Lease::STATE_RELEASED = 3;
 
 std::string
 Lease::lifetimeToText(uint32_t lifetime) {
@@ -97,6 +98,8 @@ Lease::basicStatesToText(const uint32_t state) {
         return ("declined");
     case STATE_EXPIRED_RECLAIMED:
         return ("expired-reclaimed");
+    case STATE_RELEASED:
+        return ("released");
     default:
         // The default case will be handled further on
         ;
diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h
index f84302d276..8e4d297d76 100644
--- a/src/lib/dhcpsrv/lease.h
+++ b/src/lib/dhcpsrv/lease.h
@@ -74,6 +74,9 @@ struct Lease : public isc::data::UserContext, public isc::data::CfgToElement {
     /// @brief Expired and reclaimed lease.
     static const uint32_t STATE_EXPIRED_RECLAIMED;
 
+    /// @brief Released lease held in the database for lease affinity.
+    static const uint32_t STATE_RELEASED;
+
     /// @brief Returns name(s) of the basic lease state(s).
     ///
     /// @param state A numeric value holding a state information.
diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
index 8a4286ad59..906c3a9952 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
@@ -1950,6 +1950,72 @@ TEST_F(AllocEngine4Test, requestReuseDeclinedLease4Stats) {
     EXPECT_TRUE(testStatistics("reclaimed-declined-addresses", 1, subnet_->getID()));
 }
 
+// This test checks if a released lease can be reused in REQUEST (actual allocation)
+TEST_F(AllocEngine4Test, requestReuseReleasedLease4) {
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(0)));
+    ASSERT_TRUE(engine);
+
+    IOAddress addr("192.0.2.105");
+
+    EXPECT_TRUE(testStatistics("assigned-addresses", 0, subnet_->getID()));
+    int64_t cumulative = getStatistics("cumulative-assigned-addresses",
+                                       subnet_->getID());
+    int64_t glbl_cumulative = getStatistics("cumulative-assigned-addresses");
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0, subnet_->getID()));
+
+    // Just a different hw/client-id for the second client
+    uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe };
+    HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
+    uint8_t clientid2[] = { 8, 7, 6, 5, 4, 3, 2, 1 };
+    time_t now = time(NULL) - 500; // Allocated 500 seconds ago
+
+    Lease4Ptr lease(new Lease4(addr, hwaddr2, clientid2, sizeof(clientid2),
+                               495, now, subnet_->getID()));
+    lease->state_ = Lease4::STATE_RELEASED;
+    // Make a copy of the lease, so as we can compare that with the old lease
+    // instance returned by the allocation engine.
+    Lease4 original_lease(*lease);
+
+    // Lease was assigned 500 seconds ago, but its valid lifetime is 495, so it
+    // is expired already
+    ASSERT_TRUE(lease->expired());
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+
+    // A client comes along, asking specifically for this address
+    AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
+                                    IOAddress(addr), false, false,
+                                    "host.example.com.", false);
+    ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234));
+    lease = engine->allocateLease4(ctx);
+
+    // Check that he got that single lease
+    ASSERT_TRUE(lease);
+    EXPECT_EQ(addr, lease->addr_);
+
+    // Check that the lease is indeed updated in LeaseMgr
+    Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(addr);
+    ASSERT_TRUE(from_mgr);
+
+    // Now check that the lease in LeaseMgr has the same parameters
+    detailCompareLease(lease, from_mgr);
+
+    // The allocation engine should return a copy of the old lease. This
+    // lease should be equal to the original lease.
+    ASSERT_TRUE(ctx.old_lease_);
+    EXPECT_TRUE(*ctx.old_lease_ == original_lease);
+
+    EXPECT_TRUE(testStatistics("assigned-addresses", 1, subnet_->getID()));
+    cumulative += 1;
+    EXPECT_TRUE(testStatistics("cumulative-assigned-addresses",
+                               cumulative, subnet_->getID()));
+    glbl_cumulative += 1;
+    EXPECT_TRUE(testStatistics("cumulative-assigned-addresses", glbl_cumulative));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 1));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 1, subnet_->getID()));
+}
+
 // This test checks that the Allocation Engine correctly identifies the
 // existing client's lease in the lease database, using the client
 // identifier and HW address.
diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
index 6f3e88cc2e..1e6469658e 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
@@ -776,6 +776,95 @@ TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
     EXPECT_TRUE(testStatistics("reclaimed-leases", 1, other_subnetid));
 }
 
+// This test checks if a released lease can be reused in REQUEST (actual allocation)
+TEST_F(AllocEngine6Test, requestReuseReleasedLease6) {
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(100)));
+    ASSERT_TRUE(engine);
+
+    IOAddress addr("2001:db8:1::ad");
+    CfgMgr& cfg_mgr = CfgMgr::instance();
+    cfg_mgr.clear(); // Get rid of the default test configuration
+
+    // Create configuration similar to other tests, but with a single address pool
+    subnet_ = Subnet6::create(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4, SubnetID(10));
+    pool_ = Pool6Ptr(new Pool6(Lease::TYPE_NA, addr, addr)); // just a single address
+    subnet_->addPool(pool_);
+    cfg_mgr.getStagingCfg()->getCfgSubnets6()->add(subnet_);
+    cfg_mgr.commit();
+    int64_t cumulative = getStatistics("cumulative-assigned-nas",
+                                       subnet_->getID());
+    int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas");
+
+    // Let's create an expired lease
+    DuidPtr other_duid = DuidPtr(new DUID(vector<uint8_t>(12, 0xff)));
+    const uint32_t other_iaid = 3568;
+
+    const SubnetID other_subnetid = 999;
+    Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, other_duid, other_iaid,
+                               501, 502, other_subnetid, HWAddrPtr()));
+    lease->state_ = Lease6::STATE_RELEASED;
+    int64_t other_cumulative =
+        getStatistics("cumulative-assigned-nas", other_subnetid);
+
+    lease->cltt_ = time(NULL) - 500; // Allocated 500 seconds ago
+    lease->valid_lft_ = 495; // Lease was valid for 495 seconds
+    lease->fqdn_fwd_ = true;
+    lease->fqdn_rev_ = true;
+    lease->hostname_ = "myhost.example.com.";
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+
+    // A client comes along, asking specifically for this address
+    AllocEngine::ClientContext6 ctx(subnet_, duid_, false, false, "", false,
+                                    Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234)));
+    ctx.currentIA().iaid_ = iaid_;
+    ctx.currentIA().addHint(addr);
+
+    EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx)));
+
+    // Check that he got that single lease
+    ASSERT_TRUE(lease);
+    EXPECT_EQ(addr, lease->addr_);
+    // This reactivated lease should have updated FQDN data.
+    EXPECT_TRUE(lease->hostname_.empty());
+    EXPECT_FALSE(lease->fqdn_fwd_);
+    EXPECT_FALSE(lease->fqdn_rev_);
+    EXPECT_EQ(Lease6::STATE_DEFAULT, lease->state_);
+
+    // Check that the old lease has been returned.
+    Lease6Ptr old_lease = expectOneLease(ctx.currentIA().old_leases_);
+    ASSERT_TRUE(old_lease);
+
+    // It should at least have the same IPv6 address.
+    EXPECT_EQ(lease->addr_, old_lease->addr_);
+    // Check that it carries not updated FQDN data.
+    EXPECT_EQ("myhost.example.com.", old_lease->hostname_);
+    EXPECT_TRUE(old_lease->fqdn_fwd_);
+    EXPECT_TRUE(old_lease->fqdn_rev_);
+    EXPECT_EQ(Lease6::STATE_RELEASED, old_lease->state_);
+
+    // Check that the lease is indeed updated in LeaseMgr
+    Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA,
+                                                               addr);
+    ASSERT_TRUE(from_mgr);
+
+    EXPECT_EQ(Lease6::STATE_DEFAULT, from_mgr->state_);
+
+    // Now check that the lease in LeaseMgr has the same parameters
+    detailCompareLease(lease, from_mgr);
+
+    // Verify the stats got adjusted correctly
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
+    cumulative += 1;
+    EXPECT_TRUE(testStatistics("cumulative-assigned-nas",
+                               cumulative, subnet_->getID()));
+    glbl_cumulative += 1;
+    EXPECT_TRUE(testStatistics("cumulative-assigned-nas", glbl_cumulative));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 1));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 1, other_subnetid));
+}
+
 // Checks if the lease lifetime is extended when the client sends the
 // Request.
 TEST_F(AllocEngine6Test, requestExtendLeaseLifetime) {
@@ -1046,6 +1135,70 @@ TEST_F(AllocEngine6Test, renewClassLeaseLifetime) {
     EXPECT_EQ(renewed[0]->valid_lft_, 700);
 }
 
+// Checks if a released lease is renewed and that its state is set to default.
+TEST_F(AllocEngine6Test, renewReleasedLease) {
+    // Create a released lease for the client.
+    Lease6Ptr lease(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:1::15"),
+                               duid_, iaid_, 300, 400,
+                               subnet_->getID(), HWAddrPtr()));
+    lease->state_ = Lease6::STATE_RELEASED;
+
+    // Allocated 200 seconds ago - half of the lifetime.
+    time_t lease_cltt = time(NULL) - 200;
+    lease->cltt_ = lease_cltt;
+
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+
+    AllocEngine engine(100);
+
+    // This is what the client will send in his renew message.
+    AllocEngine::HintContainer hints;
+    hints.push_back(AllocEngine::Resource(IOAddress("2001:db8:1::15"), 128));
+
+    // Client should renew the lease.
+    Lease6Collection renewed = renewTest(engine, pool_, hints, IN_SUBNET, IN_POOL);
+    ASSERT_EQ(1, renewed.size());
+
+    // And the lease lifetime should be extended.
+    EXPECT_GT(renewed[0]->cltt_, lease_cltt)
+        << "Lease lifetime was not extended, but it should";
+
+    // The lease should have the default state.
+    EXPECT_EQ(Lease6::STATE_DEFAULT, renewed[0]->state_);
+}
+
+// Checks if a released lease is re-allocated and that its state set to default.
+TEST_F(AllocEngine6Test, reallocReleasedLease) {
+    // Create a released lease for the client.
+    Lease6Ptr lease(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:1::15"),
+                               duid_, iaid_, 300, 400,
+                               subnet_->getID(), HWAddrPtr()));
+    lease->state_ = Lease6::STATE_RELEASED;
+
+    // Allocated 200 seconds ago - half of the lifetime.
+    time_t lease_cltt = time(NULL) - 200;
+    lease->cltt_ = lease_cltt;
+
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+
+    AllocEngine engine(100);
+
+    // This is what the client will send in his renew message.
+    AllocEngine::HintContainer hints;
+    hints.push_back(AllocEngine::Resource(IOAddress("2001:db8:1::15"), 128));
+
+    // Reallocate the released lease.
+    Lease6Ptr renewed = simpleAlloc6Test(pool_, IOAddress("::"), false);
+    ASSERT_TRUE(renewed);
+
+    // And the lease lifetime should be extended.
+    EXPECT_GT(renewed->cltt_, lease_cltt)
+        << "Lease lifetime was not extended, but it should";
+
+    // The lease should have the default state.
+    EXPECT_EQ(Lease6::STATE_DEFAULT, renewed->state_);
+}
+
 // Renew and the client has a reservation for the lease.
 TEST_F(AllocEngine6Test, renewExtendLeaseLifetimeForReservation) {
     // Create reservation for the client. This is in-pool reservation,
@@ -1097,10 +1250,7 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolSolicitNoHint) {
 
     AllocEngine engine(100);
 
-    Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("::"), true);
-    ASSERT_TRUE(lease);
-    EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
-}
+                                                                                                                                                     }
 
 // Checks that a client gets the address reserved (in-pool case)
 // This test checks the behavior of the allocation engine in the following
diff --git a/src/lib/dhcpsrv/tests/lease_unittest.cc b/src/lib/dhcpsrv/tests/lease_unittest.cc
index e50db582bb..8d3b1b6b56 100644
--- a/src/lib/dhcpsrv/tests/lease_unittest.cc
+++ b/src/lib/dhcpsrv/tests/lease_unittest.cc
@@ -589,6 +589,7 @@ TEST_F(Lease4Test, stateToText) {
     EXPECT_EQ("default", Lease4::statesToText(Lease::STATE_DEFAULT));
     EXPECT_EQ("declined", Lease4::statesToText(Lease::STATE_DECLINED));
     EXPECT_EQ("expired-reclaimed", Lease4::statesToText(Lease::STATE_EXPIRED_RECLAIMED));
+    EXPECT_EQ("released", Lease4::statesToText(Lease::STATE_RELEASED));
 }
 
 /// @brief Creates an instance of the lease with certain FQDN data.
@@ -1363,6 +1364,7 @@ TEST(Lease6Test, stateToText) {
     EXPECT_EQ("default", Lease6::statesToText(Lease::STATE_DEFAULT));
     EXPECT_EQ("declined", Lease6::statesToText(Lease::STATE_DECLINED));
     EXPECT_EQ("expired-reclaimed", Lease6::statesToText(Lease::STATE_EXPIRED_RECLAIMED));
+    EXPECT_EQ("released", Lease6::statesToText(Lease::STATE_RELEASED));
 }
 
 } // end of anonymous namespace
diff --git a/src/lib/mysql/mysql_constants.h b/src/lib/mysql/mysql_constants.h
index a0d79f97fc..443faa4490 100644
--- a/src/lib/mysql/mysql_constants.h
+++ b/src/lib/mysql/mysql_constants.h
@@ -52,7 +52,7 @@ const int MLM_MYSQL_FETCH_FAILURE = 0;
 
 /// @name Current database schema version values.
 //@{
-const uint32_t MYSQL_SCHEMA_VERSION_MAJOR = 22;
+const uint32_t MYSQL_SCHEMA_VERSION_MAJOR = 23;
 const uint32_t MYSQL_SCHEMA_VERSION_MINOR = 0;
 
 //@}
diff --git a/src/lib/pgsql/pgsql_connection.h b/src/lib/pgsql/pgsql_connection.h
index e61d0763c6..e9f9fb2be3 100644
--- a/src/lib/pgsql/pgsql_connection.h
+++ b/src/lib/pgsql/pgsql_connection.h
@@ -18,7 +18,7 @@ namespace isc {
 namespace db {
 
 /// @brief Define the PostgreSQL backend version.
-const uint32_t PGSQL_SCHEMA_VERSION_MAJOR = 22;
+const uint32_t PGSQL_SCHEMA_VERSION_MAJOR = 23;
 const uint32_t PGSQL_SCHEMA_VERSION_MINOR = 0;
 
 // Maximum number of parameters that can be used a statement