[#754,!558] Update v6 lease data for dynamic leases with HRs

src/lib/dhcpsrv/alloc_engine.cc
    AllocEngine::allocateLeases6(ClientContext6& ctx) - add call to
    updateLeaseData() to case 3.

src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
    TEST_F(AllocEngine6Test, hostDynamicAddress)
    TEST_F(AllocEngine6Test, globalHostDynamicAddress) - revamped to
    verify assignment and update for REQUESTs

diff --git a/ChangeLog b/ChangeLog
index ff1a07d439f15fc5a7bbd097971b00a6859fde0b..ca92c02b8604614e42e5a16aa2b4dbf4f75e0661 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+16xx.  [bug]           tmark
+       Corrected an issue in kea-dhcp6 where the server would fail
+       to extend the expiry of a existing dynamic lease assigned to
+       client with host reservation.
+       (Gitlab #754)
+
 1670.  [func]          fdupont
        Added searches for host reservations by hostname and by hostname
        and subnet.

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 4f79f5cd95e5137c22ee1514e81b7169148fec73..ab40373f40c12ee3722100684728f18a1950c816 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -790,6 +790,9 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) {
 
             // All checks are done. Let's hope we have some leases left.
 
+            // Update any leases we have left.
+            leases = updateLeaseData(ctx, leases);
+
             // If we don't have any leases at this stage, it means that we hit
             // one of the following cases:
             // - we have a reservation, but it's not for this IAID/ia-type and

diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
index d35985a73653d51f0030bec08cea1b8a492958d0..a762b970f55f69de3b8c3620dc11e9db52daf156 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
@@ -3132,7 +3132,10 @@ TEST_F(SharedNetworkAlloc6Test, requestRunningOut) {
     EXPECT_TRUE(leases.empty());
 }
 
-// Verifies that client with a hostname reservation can get and
+// Verifies that client with a hostname reservation can
+// 1. Get a dynamic lease
+// 2. Renew the same lease via REQUEST (calls allocateLease6)
+// 3. Renew the same lease via RENEW/REBIND (calls renewLeases6)
 // renew a dynamic lease from their selected subnet.
 TEST_F(AllocEngine6Test, hostDynamicAddress) {
     boost::scoped_ptr<AllocEngine> engine;
@@ -3157,6 +3160,7 @@ TEST_F(AllocEngine6Test, hostDynamicAddress) {
 
     // Look up the reservation.
     findReservation(*engine, ctx);
+
     // Make sure we found our host.
     ConstHostPtr current = ctx.currentHost();
     ASSERT_TRUE(current);
@@ -3169,13 +3173,7 @@ TEST_F(AllocEngine6Test, hostDynamicAddress) {
     EXPECT_EQ("2001:db8:1::10", lease->addr_.toText());
 
     // We're going to rollback the clock a little so we can verify a renewal.
-    // We verify the "time" change in case the lease returned to us
-    // by expectOneLease ever becomes a copy and not what is in the lease mgr.
-    --lease->cltt_;
-    Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_,
-                                                               lease->addr_);
-    ASSERT_TRUE(from_mgr);
-    EXPECT_EQ(from_mgr->cltt_, lease->cltt_);
+    ASSERT_NO_FATAL_FAILURE(rollbackPersistedCltt(lease));
 
     // This is what the client will send in his renew message.
     AllocEngine::HintContainer hints;
@@ -3189,11 +3187,20 @@ TEST_F(AllocEngine6Test, hostDynamicAddress) {
     Lease6Collection renewed = renewTest(*engine, pool_, hints, true);
     ASSERT_EQ(1, renewed.size());
 
+    Lease6Ptr renewed_lease = renewed[0];
+    EXPECT_EQ("2001:db8:1::10", renewed_lease->addr_.toText());
+
     // And the lease lifetime should be extended.
-    EXPECT_GT(renewed[0]->cltt_, lease->cltt_)
+    EXPECT_GT(renewed_lease->cltt_, lease->cltt_)
         << "Lease lifetime was not extended, but it should";
 
-    // Now let's verify that if the client returns via SARR, they get the same lease.
+    // Now let's verify that if the client returns via SARR, they get the same
+    // lease and the cltt gets extended.
+
+    // First, we're going to rollback the clock again so we can verify the
+    // allocation updates the expiry.
+    ASSERT_NO_FATAL_FAILURE(rollbackPersistedCltt(renewed_lease));
+
     // Create a new context which will be used to try to allocate leases
     query.reset(new Pkt6(DHCPV6_REQUEST, 1234));
     AllocEngine::ClientContext6 ctx2(subnet_, duid_, false, false, "", false, query);
@@ -3208,13 +3215,20 @@ TEST_F(AllocEngine6Test, hostDynamicAddress) {
     ASSERT_EQ("host1", current->getHostname());
 
     // Check that we have been allocated the original dynamic address.
-    ASSERT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx2)));
-    ASSERT_TRUE(lease);
-    EXPECT_EQ("2001:db8:1::10", lease->addr_.toText());
+    Lease6Ptr lease2;
+    ASSERT_NO_THROW(lease2 = expectOneLease(engine->allocateLeases6(ctx2)));
+    ASSERT_TRUE(lease2);
+    EXPECT_EQ("2001:db8:1::10", lease2->addr_.toText());
+
+    // And the lease lifetime should be extended.
+    EXPECT_GT(lease2->cltt_, renewed_lease->cltt_)
+        << "Lease lifetime was not extended, but it should";
 }
 
-// Verifies that client with a global hostname reservation can get and
-// renew a dynamic lease from their selected subnet.
+// Verifies that client with a global hostname reservation can:
+// 1. Get a dynamic lease
+// 2. Renew the same lease via REQUEST (calls allocateLease6)
+// 3. Renew the same lease via RENEW/REBIND (calls renewLeases6)
 TEST_F(AllocEngine6Test, globalHostDynamicAddress) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100)));
@@ -3249,13 +3263,7 @@ TEST_F(AllocEngine6Test, globalHostDynamicAddress) {
     EXPECT_EQ("2001:db8:1::10", lease->addr_.toText());
 
     // We're going to rollback the clock a little so we can verify a renewal.
-    // We verify the "time" change in case the lease returned to us
-    // by expectOneLease ever becomes a copy and not what is in the lease mgr.
-    --lease->cltt_;
-    Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_,
-                                                               lease->addr_);
-    ASSERT_TRUE(from_mgr);
-    EXPECT_EQ(from_mgr->cltt_, lease->cltt_);
+    ASSERT_NO_FATAL_FAILURE(rollbackPersistedCltt(lease));
 
     // This is what the client will send in his renew message.
     AllocEngine::HintContainer hints;
@@ -3270,11 +3278,17 @@ TEST_F(AllocEngine6Test, globalHostDynamicAddress) {
     ASSERT_EQ(1, renewed.size());
 
     // And the lease lifetime should be extended.
-    EXPECT_GT(renewed[0]->cltt_, lease->cltt_)
+    ASSERT_GT(renewed[0]->cltt_, lease->cltt_)
         << "Lease lifetime was not extended, but it should";
 
-    // Now let's verify that if the client returns via SARR, they get the same lease.
-    // Create a new context which will be used to try to allocate leases
+    // Now let's verify that if the client returns via SARR, they get the same
+    // lease. Create a new context which will be used to try to allocate leases
+
+    // First, we're going to rollback the clock again so we can verify the
+    // allocation updates the expiry.
+    Lease6Ptr renewed_lease = renewed[0];
+    ASSERT_NO_FATAL_FAILURE(rollbackPersistedCltt(renewed_lease));
+
     query.reset(new Pkt6(DHCPV6_REQUEST, 1234));
     AllocEngine::ClientContext6 ctx2(subnet_, duid_, false, false, "", false, query);
     ctx2.currentIA().iaid_ = iaid_;
@@ -3288,9 +3302,14 @@ TEST_F(AllocEngine6Test, globalHostDynamicAddress) {
     ASSERT_EQ("ghost1", current->getHostname());
 
     // Check that we have been allocated a dynamic address.
-    ASSERT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx2)));
-    ASSERT_TRUE(lease);
-    EXPECT_EQ("2001:db8:1::10", lease->addr_.toText());
+    Lease6Ptr lease2;
+    ASSERT_NO_THROW(lease2 = expectOneLease(engine->allocateLeases6(ctx2)));
+    ASSERT_TRUE(lease2);
+    EXPECT_EQ("2001:db8:1::10", lease2->addr_.toText());
+
+    // And the lease lifetime should be extended.
+    EXPECT_GT(lease2->cltt_, renewed_lease->cltt_)
+        << "Lease lifetime was not extended, but it should";
 }
 
 // Verifies that client with a global address reservation can get and

diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h
index ad148b0611ba86a1bd505422845ab6954f32ee54..2b547a4f55beb10338c3711447c49cafdde46ac4 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine_utils.h
+++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.h
@@ -439,6 +439,35 @@ public:
                       HWAddrPtr& hwaddr, const asiolink::IOAddress& addr,
                       uint8_t prefix_len);
 
+    /// @brief Utility function that decrements cltt of a persisted lease
+    /// 
+    /// This function is used to simulate the passage of time by decrementing
+    /// the lease's cltt, currently by 1.  It fetches the desired lease from the
+    /// lease manager, decrements the cltt, then updates the lease in the lease
+    /// manager.  Next, it refetches the lease and verifies the update took place.
+    ///
+    /// @param[in][out] lease pointer reference to the lease to modify.  Upon
+    /// return it will point to the newly updated lease.
+    void 
+    rollbackPersistedCltt(Lease6Ptr& lease) {
+        ASSERT_TRUE(lease) << "rollbackCltt lease is empty";
+       
+        // Fetch it, so we can update it. 
+        Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_,
+                                                                   lease->addr_);
+        ASSERT_TRUE(from_mgr) << "rollbackCltt: lease not found?";
+
+        // Decrement cltt then update it in the manager.
+        --from_mgr->cltt_;
+        ASSERT_NO_THROW(LeaseMgrFactory::instance().updateLease6(from_mgr));
+
+        // Fetch it fresh.
+        lease = LeaseMgrFactory::instance().getLease6(lease->type_, lease->addr_);
+
+        // Make sure it stuck.
+        ASSERT_EQ(lease->cltt_, from_mgr->cltt_);
+    }
+
     virtual ~AllocEngine6Test() {
         factory_.destroy();
     }