[5638] Reset callout handle status prior to lease6_select.

src/lib/dhcpsrv/alloc_engine.cc

@@ -919,6 +919,13 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
         uint64_t max_attempts = (attempts_ > 0 ? attempts_  : possible_attempts);
         Network::HRMode hr_mode = subnet->getHostReservationMode();
 
+        // Set the default status code in case the lease6_select callouts
+        // do not exist and the callout handle has a status returned by
+        // any of the callouts already invoked for this packet.
+        if (ctx.callout_handle_) {
+            ctx.callout_handle_->setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
+        }
+
         for (uint64_t i = 0; i < max_attempts; ++i) {
 
             ++total_attempts;
@@ -967,6 +974,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
 
                     leases.push_back(lease);
                     return (leases);
+
                 } else if (ctx.callout_handle_ &&
                            (ctx.callout_handle_->getStatus() !=
                             CalloutHandle::NEXT_STEP_CONTINUE)) {

src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc

@@ -76,6 +76,19 @@ TEST_F(AllocEngine6Test, simpleAlloc6) {
 
     // We should have bumped the assigned counter by 1
     EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
+
+    // Reset last allocated address to check that the other client will
+    // be refused the already allocated address and will get the one
+    // available.
+    pool_->resetLastAllocated();
+
+    // Simulate another client. This client should be assigned a different
+    // address.
+    DuidPtr duid(new DUID(std::vector<uint8_t>(8, 0x84)));
+    simpleAlloc6Test(pool_, duid, IOAddress("::"), false);
+
+    // We should have bumped the assigned counter by 2
+    EXPECT_TRUE(testStatistics("assigned-nas", 2, subnet_->getID()));
 }
 
 // This test checks if the simple PD allocation (REQUEST) can succeed
@@ -749,7 +762,7 @@ TEST_F(AllocEngine6Test, smallPool6) {
     EXPECT_EQ("2001:db8:1::ad", lease->addr_.toText());
 
     // Do all checks on the lease
-    checkLease6(lease, Lease::TYPE_NA, 128);
+    checkLease6(duid_, lease, Lease::TYPE_NA, 128);
 
     // Check that the lease is indeed in LeaseMgr
     Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_,
@@ -847,7 +860,7 @@ TEST_F(AllocEngine6Test, solicitReuseExpiredLease6) {
     EXPECT_EQ(addr, lease->addr_);
 
     // Do all checks on the lease (if subnet-id, preferred/valid times are ok etc.)
-    checkLease6(lease, Lease::TYPE_NA, 128);
+    checkLease6(duid_, lease, Lease::TYPE_NA, 128);
 
     // CASE 2: Asking specifically for this address
     AllocEngine::ClientContext6 ctx2(subnet_, duid_, false, false, "", true,
@@ -2098,7 +2111,7 @@ TEST_F(AllocEngine6Test, solicitReuseDeclinedLease6) {
     EXPECT_EQ(addr, assigned->addr_);
 
     // Do all checks on the lease (if subnet-id, preferred/valid times are ok etc.)
-    checkLease6(assigned, Lease::TYPE_NA, 128);
+    checkLease6(duid_, assigned, Lease::TYPE_NA, 128);
 
     // CASE 2: Asking specifically for this address
     testReuseLease6(engine, declined, addr_txt, true, SHOULD_PASS, assigned);

src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc

-// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2018 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -189,7 +189,7 @@ TEST_F(HookAllocEngine6Test, lease6_select) {
     ASSERT_TRUE(lease);
 
     // Do all checks on the lease
-    checkLease6(lease, Lease::TYPE_NA, 128);
+    checkLease6(duid_, lease, Lease::TYPE_NA, 128);
 
     // Check that the lease is indeed in LeaseMgr
     Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_,

src/lib/dhcpsrv/tests/alloc_engine_utils.cc

-// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -14,6 +14,8 @@
 #include <dhcpsrv/host_mgr.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/memfile_lease_mgr.h>
+#include <hooks/hooks_manager.h>
+#include <hooks/callout_handle.h>
 #include <stats/stats_mgr.h>
 
 #include <dhcpsrv/tests/test_utils.h>
@@ -227,7 +229,7 @@ AllocEngine6Test::allocateTest(AllocEngine& engine, const Pool6Ptr& pool,
     for (Lease6Collection::iterator it = leases.begin(); it != leases.end(); ++it) {
 
         // Do all checks on the lease
-        checkLease6(*it, type, expected_len, in_pool, in_pool);
+        checkLease6(duid_, *it, type, expected_len, in_pool, in_pool);
 
         // Check that context has been updated with allocated addresses or
         // prefixes.
@@ -265,6 +267,12 @@ AllocEngine6Test::allocateTest(AllocEngine& engine, const Pool6Ptr& pool,
 Lease6Ptr
 AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const IOAddress& hint,
                                    bool fake, bool in_pool) {
+    return (simpleAlloc6Test(pool, duid_, hint, fake, in_pool));
+}
+
+Lease6Ptr
+AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const DuidPtr& duid,
+                                   const IOAddress& hint, bool fake, bool in_pool) {
     Lease::Type type = pool->getType();
     uint8_t expected_len = pool->getLength();
 
@@ -279,13 +287,18 @@ AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const IOAddress& hint,
 
     Pkt6Ptr query(new Pkt6(fake ? DHCPV6_SOLICIT : DHCPV6_REQUEST, 1234));
 
-    AllocEngine::ClientContext6 ctx(subnet_, duid_, false, false, "", fake, query);
+    AllocEngine::ClientContext6 ctx(subnet_, duid, false, false, "", fake, query);
     ctx.hwaddr_ = hwaddr_;
     ctx.addHostIdentifier(Host::IDENT_HWADDR, hwaddr_->hwaddr_);
     ctx.currentIA().iaid_ = iaid_;
     ctx.currentIA().type_ = type;
     ctx.currentIA().addHint(hint);
 
+    // Set some non-standard callout status to make sure it doesn't affect the
+    // allocation.
+    ctx.callout_handle_ = HooksManager::createCalloutHandle();
+    ctx.callout_handle_->setStatus(CalloutHandle::NEXT_STEP_SKIP);
+
     findReservation(*engine, ctx);
     Lease6Ptr lease;
     EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx)));
@@ -297,7 +310,7 @@ AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const IOAddress& hint,
     }
 
     // Do all checks on the lease
-    checkLease6(lease, type, expected_len, in_pool, in_pool);
+    checkLease6(duid, lease, type, expected_len, in_pool, in_pool);
 
     // Check that the lease is indeed in LeaseMgr
     Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(type, lease->addr_);
@@ -342,7 +355,7 @@ AllocEngine6Test::renewTest(AllocEngine& engine, const Pool6Ptr& pool,
     for (Lease6Collection::iterator it = leases.begin(); it != leases.end(); ++it) {
 
         // Do all checks on the lease
-        checkLease6(*it, type, expected_len, in_pool, in_pool);
+        checkLease6(duid_, *it, type, expected_len, in_pool, in_pool);
 
         // Check that context has been updated with allocated addresses or
         // prefixes.
@@ -406,7 +419,7 @@ AllocEngine6Test::allocWithUsedHintTest(Lease::Type type, IOAddress used_addr,
     EXPECT_NE(requested, lease->addr_);
 
     // Do all checks on the lease
-    checkLease6(lease, type, expected_pd_len);
+    checkLease6(duid_, lease, type, expected_pd_len);
 
     // Check that the lease is indeed in LeaseMgr
     Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_,
@@ -445,7 +458,7 @@ AllocEngine6Test::allocBogusHint6(Lease::Type type, asiolink::IOAddress hint,
     EXPECT_NE(hint, lease->addr_);
 
     // Do all checks on the lease
-    checkLease6(lease, type, expected_pd_len);
+    checkLease6(duid_, lease, type, expected_pd_len);
 
     // Check that the lease is indeed in LeaseMgr
     Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_,
@@ -493,7 +506,7 @@ AllocEngine6Test::testReuseLease6(const AllocEnginePtr& engine,
         ASSERT_EQ(1, leases.size());
         result = leases[0];
 
-        checkLease6(result, Lease::TYPE_NA, 128);
+        checkLease6(duid_, result, Lease::TYPE_NA, 128);
         break;
 
     case SHOULD_FAIL:

src/lib/dhcpsrv/tests/alloc_engine_utils.h

@@ -163,13 +163,15 @@ public:
 
     /// @brief checks if Lease6 matches expected configuration
     ///
+    /// @param duid pointer to the client's DUID.
     /// @param lease lease to be checked
     /// @param exp_type expected lease type
     /// @param exp_pd_len expected prefix length
     /// @param expected_in_subnet whether the lease is expected to be in subnet
     /// @param expected_in_pool whether the lease is expected to be in dynamic
-    void checkLease6(const Lease6Ptr& lease, Lease::Type exp_type,
-                     uint8_t exp_pd_len = 128, bool expected_in_subnet = true,
+    void checkLease6(const DuidPtr& duid, const Lease6Ptr& lease,
+                     Lease::Type exp_type, uint8_t exp_pd_len = 128,
+                     bool expected_in_subnet = true,
                      bool expected_in_pool = true) {
 
         // that is belongs to the right subnet
@@ -198,7 +200,7 @@ public:
         EXPECT_EQ(fqdn_fwd_, lease->fqdn_fwd_);
         EXPECT_EQ(fqdn_rev_, lease->fqdn_rev_);
         EXPECT_EQ(hostname_, lease->hostname_);
-        EXPECT_TRUE(*lease->duid_ == *duid_);
+        EXPECT_TRUE(*lease->duid_ == *duid);
         /// @todo: check cltt
     }
 
@@ -246,7 +248,7 @@ public:
 
     /// @brief Checks if the simple allocation can succeed
     ///
-    /// The type of lease is determined by pool type (pool->getType()
+    /// The type of lease is determined by pool type (pool->getType())
     ///
     /// @param pool pool from which the lease will be allocated from
     /// @param hint address to be used as a hint
@@ -257,6 +259,21 @@ public:
                                const asiolink::IOAddress& hint,
                                bool fake, bool in_pool = true);
 
+    /// @brief Checks if the simple allocation can succeed for custom DUID.
+    ///
+    /// The type of lease is determined by pool type (pool->getType())
+    ///
+    /// @param pool pool from which the lease will be allocated from
+    /// @param duid pointer to the DUID used for allocation.
+    /// @param hint address to be used as a hint
+    /// @param fake true - this is fake allocation (SOLICIT)
+    /// @param in_pool specifies whether the lease is expected to be in pool
+    /// @return allocated lease (or NULL)
+    Lease6Ptr simpleAlloc6Test(const Pool6Ptr& pool, const DuidPtr& duid,
+                               const asiolink::IOAddress& hint,
+                               bool fake, bool in_pool = true);
+
+
     /// @brief Checks if the allocation can succeed.
     ///
     /// The type of lease is determined by pool type (pool->getType()).