[#3555] Cleanups and Changelog

Added Changelog entry
/src/lib/dhcpsrv/alloc_engine.cc
/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
    Minor cleanups

diff --git a/changelog_unreleased/3555-host-reservation-and-offer-lifetime-option-conflicts-causing-persistent-naks b/changelog_unreleased/3555-host-reservation-and-offer-lifetime-option-conflicts-causing-persistent-naks
new file mode 100644 (file)
index 0000000..bdf8bd7
--- /dev/null
+++ b/changelog_unreleased/3555-host-reservation-and-offer-lifetime-option-conflicts-causing-persistent-naks
@@ -0,0 +1,9 @@
+[bug]          tmark
+       Corrected an issue which caused kea-dhcp4 to
+       NAK requests for an offered lease when a
+       conflicting host reservation exists and 
+       offer-lifetime is greater than zero. The 
+       server will now remove the unusable lease,
+       renew the temporary lease, and send it back
+       in a DHCPACK.
+       (Gitlab #3555)

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 4d4acb2deb650f8c55f292689a740f9acc9b10e3..e4a09fd4c7906b3881578a231bea704e8173df95 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -4140,7 +4140,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
         // client's current lease (client_lease) should no longer be used and
         // so offered a different lease (existing) that was temporarily
         // allocated because offer-lifetime is greater than zero. We need to
-        // delete to unusable lease and renew the temporary lease.
+        // delete the unusable lease and renew the temporary lease.
         if (((client_lease && existing) && (client_lease->addr_ != existing->addr_) &&
              (existing->addr_ == ctx.requested_address_) &&
              (existing->belongsToClient(ctx.hwaddr_, ctx.subnet_->getMatchClientId() ?
@@ -4172,7 +4172,6 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
                       ALLOC_ENGINE_V4_REQUEST_EXTEND_LEASE)
                 .arg(ctx.query_->getLabel())
                 .arg(ctx.requested_address_);
-
             return (renewLease4(client_lease, ctx));
         }
     }

diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
index c63f13ca3f21e537cf15ecb37f25ab7fc1001971..09db913900831c980e76721b49717e9fc4aafabb 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
@@ -2149,9 +2149,8 @@ TEST_F(AllocEngine4Test, existingLeasePlusTemporary) {
     LeaseMgrFactory::instance().addLease(lease);
     LeaseMgrFactory::instance().addLease(lease2);
 
-    // First when temporary allocations are off.
-
-    // Client requests the second lease.
+    // When temporary allocations are off and client requests the
+    // second lease, the engine should not return a lease.
     AllocEngine engine(0);
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, IOAddress("192.0.2.102"),
                                     false, false, "", false);
@@ -2160,7 +2159,7 @@ TEST_F(AllocEngine4Test, existingLeasePlusTemporary) {
 
     ASSERT_FALSE(new_lease);
 
-    // Both leases should still be there.
+    // Both leases should still be in the lease back end.
     ASSERT_TRUE(LeaseMgrFactory::instance().getLease4(lease->addr_));
     ASSERT_TRUE(LeaseMgrFactory::instance().getLease4(lease2->addr_));
 
@@ -2173,11 +2172,11 @@ TEST_F(AllocEngine4Test, existingLeasePlusTemporary) {
     ctx2.query_.reset(new Pkt4(DHCPREQUEST, 1234));
     new_lease = engine.allocateLease4(ctx2);
 
-    // Allocation should renew it and remove the original lease.
+    // Allocation should return the second lease and remove the original lease.
     ASSERT_TRUE(new_lease);
     EXPECT_EQ("192.0.2.102", new_lease->addr_.toText());
 
-    // Orignal lease should be gone.
+    // Orignal lease should have been deleted from lease back.
     ASSERT_FALSE(LeaseMgrFactory::instance().getLease4(lease->addr_));
 }