[master] Merged trac5070 (skip in lease4_select loops)

src/lib/dhcpsrv/alloc_engine.cc

@@ -710,6 +710,11 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
 
                 leases.push_back(lease);
                 return (leases);
+            } else if (ctx.callout_handle_ &&
+                       (ctx.callout_handle_->getStatus() !=
+                        CalloutHandle::NEXT_STEP_CONTINUE)) {
+                // Don't retry when the callout status is not continue.
+                break;
             }
 
             // Although the address was free just microseconds ago, it may have
@@ -2841,6 +2846,11 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) {
             new_lease = allocateOrReuseLease4(candidate, ctx);
             if (new_lease) {
                 return (new_lease);
+            } else if (ctx.callout_handle_ &&
+                       (ctx.callout_handle_->getStatus() !=
+                        CalloutHandle::NEXT_STEP_CONTINUE)) {
+                // Don't retry when the callout status is not continue.
+                break;
             }
         }
     }

src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2017 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
@@ -46,6 +46,7 @@ public:
         callback_addr_updated_ = IOAddress("::");
         callback_qry_pkt6_.reset();
         callback_qry_options_copy_ = false;
+        callback_skip_ = 0;
     }
 
     /// callback that stores received callout name and received values
@@ -91,6 +92,21 @@ public:
         return (0);
     }
 
+    /// callback that return next step skip status
+    static int
+    lease6_select_skip_callout(CalloutHandle& callout_handle) {
+
+        // Let's call the basic callout, so it can record all parameters
+        lease6_select_callout(callout_handle);
+
+        // Count the call
+        callback_skip_++;
+
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
+
+        return (0);
+    }
+
     // Values to be used in callout to override lease6 content
     static const IOAddress addr_override_;
     static const uint32_t t1_override_;
@@ -110,6 +126,9 @@ public:
     static vector<string> callback_argument_names_;
     static Pkt6Ptr callback_qry_pkt6_;
     static bool callback_qry_options_copy_;
+
+    // Counter for next step skip (should be not retried)
+    static unsigned callback_skip_;
 };
 
 // For some reason initialization within a class makes the linker confused.
@@ -132,6 +151,8 @@ vector<string> HookAllocEngine6Test::callback_argument_names_;
 Pkt6Ptr HookAllocEngine6Test::callback_qry_pkt6_;
 bool HookAllocEngine6Test::callback_qry_options_copy_;
 
+unsigned HookAllocEngine6Test::callback_skip_;
+
 // This test checks if the lease6_select callout is executed and expected
 // parameters as passed.
 TEST_F(HookAllocEngine6Test, lease6_select) {
@@ -153,7 +174,7 @@ TEST_F(HookAllocEngine6Test, lease6_select) {
     HookLibsCollection libraries; // no libraries at this time
     HooksManager::loadLibraries(libraries);
 
-    // Install pkt6_receive_callout
+    // Install lease6_select
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "lease6_select", lease6_select_callout));
 
@@ -263,6 +284,36 @@ TEST_F(HookAllocEngine6Test, change_lease6_select) {
     EXPECT_EQ(valid_override_, from_mgr->valid_lft_);
 }
 
+// This test checks if lease6_select callout can set the status to next
+// step skip without the engine to retry.
+TEST_F(HookAllocEngine6Test, skip_lease6_select) {
+
+    // Create allocation engine (hook names are registered in its ctor)
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100)));
+    ASSERT_TRUE(engine);
+
+    // Initialize Hooks Manager
+    HookLibsCollection libraries; // no libraries at this time
+    HooksManager::loadLibraries(libraries);
+
+    // Install a callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease6_select", lease6_select_skip_callout));
+
+    // Call allocateLeases6. Callouts should be triggered here.
+    Lease6Ptr lease;
+    AllocEngine::ClientContext6 ctx(subnet_, duid_, false, false, "", false,
+                                    Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234)),
+                                    HooksManager::createCalloutHandle());
+    ctx.currentIA().iaid_ = iaid_;
+    EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx)));
+    // Check that we got no lease
+    EXPECT_FALSE(lease);
+
+    // Check no retry was attempted
+    EXPECT_EQ(1, callback_skip_);
+}
 
 /// @brief helper class used in Hooks testing in AllocEngine4
 ///
@@ -294,6 +345,7 @@ public:
         callback_addr_updated_ = IOAddress("::");
         callback_qry_pkt4_.reset();
         callback_qry_options_copy_ = false;
+        callback_skip_ = 0;
     }
 
     /// callback that stores received callout name and received values
@@ -338,6 +390,21 @@ public:
         return (0);
     }
 
+    /// callback that return next step skip status
+    static int
+    lease4_select_skip_callout(CalloutHandle& callout_handle) {
+
+        // Let's call the basic callout, so it can record all parameters
+        lease4_select_callout(callout_handle);
+
+        // Count the call
+        callback_skip_++;
+
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
+
+        return (0);
+    }
+
     // Values to be used in callout to override lease4 content
     static const IOAddress addr_override_;
     static const uint32_t t1_override_;
@@ -356,6 +423,9 @@ public:
     static vector<string> callback_argument_names_;
     static Pkt4Ptr callback_qry_pkt4_;
     static bool callback_qry_options_copy_;
+
+    // Counter for next step skip (should be not retried)
+    static unsigned callback_skip_;
 };
 
 // For some reason initialization within a class makes the linker confused.
@@ -377,6 +447,8 @@ vector<string> HookAllocEngine4Test::callback_argument_names_;
 Pkt4Ptr HookAllocEngine4Test::callback_qry_pkt4_;
 bool HookAllocEngine4Test::callback_qry_options_copy_;
 
+unsigned HookAllocEngine4Test::callback_skip_;
+
 // This test checks if the lease4_select callout is executed and expected
 // parameters as passed.
 TEST_F(HookAllocEngine4Test, lease4_select) {
@@ -399,7 +471,7 @@ TEST_F(HookAllocEngine4Test, lease4_select) {
     HookLibsCollection libraries; // no libraries at this time
     HooksManager::loadLibraries(libraries);
 
-    // Install pkt4_receive_callout
+    // Install lease4_select
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "lease4_select", lease4_select_callout));
 
@@ -510,6 +582,41 @@ TEST_F(HookAllocEngine4Test, change_lease4_select) {
     EXPECT_EQ(valid_override_, from_mgr->valid_lft_);
 }
 
+// This test checks if lease4_select callout can set the status to next
+// step skip without the engine to retry.
+TEST_F(HookAllocEngine4Test, skip_lease4_select) {
+
+    // Create allocation engine (hook names are registered in its ctor)
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
+                                                 100, false)));
+    ASSERT_TRUE(engine);
+
+    // Initialize Hooks Manager
+    HookLibsCollection libraries; // no libraries at this time
+    HooksManager::loadLibraries(libraries);
+
+    // Install a callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease4_select", lease4_select_skip_callout));
+
+    CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
+
+    AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
+                                    IOAddress("0.0.0.0"),
+                                    false, false, "", false);
+    ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234));
+    ctx.callout_handle_ = callout_handle;
+
+    Lease4Ptr lease = engine->allocateLease4(ctx);
+
+    // Check that we got no lease
+    EXPECT_FALSE(lease);
+
+    // Check no retry was attempted
+    EXPECT_EQ(1, callback_skip_);
+}
+
 }; // namespace test
 }; // namespace dhcp
 }; // namespace isc