From: Tomek Mrugalski <tomasz@isc.org>
Date: Mon, 30 Nov 2015 11:36:46 +0000 (+0100)
Subject: [master] Merge branch 'trac3988' (lease{4,6}_recover hooks)
X-Git-Tag: trac4231_base~33
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e1cd854c40786e528a24d4cc16e9874b95029c52;p=thirdparty%2Fkea.git

[master] Merge branch 'trac3988' (lease{4,6}_recover hooks)

# Conflicts:
#	src/lib/dhcpsrv/alloc_engine.cc
#	src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc
---

e1cd854c40786e528a24d4cc16e9874b95029c52
diff --cc src/lib/dhcpsrv/alloc_engine.cc
index 868e4a4f69,78eeb93e2b..02ead8dbc9
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@@ -24,9 -24,10 +24,10 @@@
  #include <dhcpsrv/host_mgr.h>
  #include <dhcpsrv/host.h>
  #include <dhcpsrv/lease_mgr_factory.h>
 -#include <dhcp/dhcp6.h>
 +#include <dhcpsrv/ncr_generator.h>
  #include <hooks/callout_handle.h>
  #include <hooks/hooks_manager.h>
+ #include <dhcpsrv/callout_handle_store.h>
  #include <stats/stats_mgr.h>
  #include <util/stopwatch.h>
  #include <hooks/server_hooks.h>
@@@ -1578,200 -1711,6 +1583,200 @@@ AllocEngine::reclaimExpiredLeases4(cons
      }
  }
  
 +template<typename LeasePtrType>
 +void
 +AllocEngine::reclaimExpiredLease(const LeasePtrType& lease, const bool remove_lease,
 +                                 const CalloutHandlePtr& callout_handle) {
 +    reclaimExpiredLease(lease, remove_lease ? DB_RECLAIM_REMOVE : DB_RECLAIM_UPDATE,
 +                        callout_handle);
 +}
 +
 +template<typename LeasePtrType>
 +void
 +AllocEngine::reclaimExpiredLease(const LeasePtrType& lease,
 +                                 const CalloutHandlePtr& callout_handle) {
 +    // This variant of the method is used by the code which allocates or
 +    // renews leases. It may be the case that the lease has already been
 +    // reclaimed, so there is nothing to do.
 +    if (!lease->stateExpiredReclaimed()) {
 +        reclaimExpiredLease(lease, DB_RECLAIM_LEAVE_UNCHANGED, callout_handle);
 +    }
 +}
 +
 +void
 +AllocEngine::reclaimExpiredLease(const Lease6Ptr& lease,
 +                                 const DbReclaimMode& reclaim_mode,
 +                                 const CalloutHandlePtr& callout_handle) {
 +
 +    LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
 +              ALLOC_ENGINE_V6_LEASE_RECLAIM)
 +        .arg(Pkt6::makeLabel(lease->duid_, lease->hwaddr_))
 +        .arg(lease->addr_.toText())
 +        .arg(static_cast<int>(lease->prefixlen_));
 +
 +    // The skip flag indicates if the callouts have taken responsibility
 +    // for reclaiming the lease. The callout will set this to true if
 +    // it reclaims the lease itself. In this case the reclamation routine
 +    // will not update DNS nor update the database.
 +    bool skipped = false;
 +    if (callout_handle) {
 +        callout_handle->deleteAllArguments();
 +        callout_handle->setArgument("lease6", lease);
 +        callout_handle->setArgument("remove_lease", reclaim_mode == DB_RECLAIM_REMOVE);
 +
 +        HooksManager::callCallouts(Hooks.hook_index_lease6_expire_,
 +                                   *callout_handle);
 +
 +        skipped = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
 +    }
 +
 +    /// @todo: Maybe add support for DROP status?
 +    /// Not sure if we need to support every possible status everywhere.
 +
 +    if (!skipped) {
 +
 +        // Generate removal name change request for D2, if required.
 +        // This will return immediatelly if the DNS wasn't updated
 +        // when the lease was created.
 +        queueNCR(CHG_REMOVE, lease);
 +
 +        // Let's check if the lease that just expired is in DECLINED state.
 +        // If it is, we need to conduct couple extra steps and also force
 +        // its removal.
 +        bool remove_tmp = (reclaim_mode == DB_RECLAIM_REMOVE);
 +        if (lease->state_ == Lease::STATE_DECLINED) {
 +            // There's no point in keeping declined lease after its
 +            // reclaimation. Declined lease doesn't have any client
 +            // identifying information anymore.
 +            if (reclaim_mode != DB_RECLAIM_LEAVE_UNCHANGED) {
 +                remove_tmp = true;
 +            }
 +
 +            // Do extra steps required for declined lease reclaimation:
 +            // - bump decline-related stats
 +            // - log separate message
-             reclaimDeclined(lease);
++            remove_tmp = reclaimDeclined(lease);
 +        }
 +
 +        if (reclaim_mode != DB_RECLAIM_LEAVE_UNCHANGED) {
 +            // Reclaim the lease - depending on the configuration, set the
 +            // expired-reclaimed state or simply remove it.
 +            LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
 +            reclaimLeaseInDatabase<Lease6Ptr>(lease, remove_tmp,
 +                                              boost::bind(&LeaseMgr::updateLease6,
 +                                                          &lease_mgr, _1));
 +        }
 +    }
 +
 +    // Update statistics.
 +
 +    // Decrease number of assigned leases.
 +    if (lease->type_ == Lease::TYPE_NA) {
 +        // IA_NA
 +        StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
 +                                                             lease->subnet_id_,
 +                                                             "assigned-nas"),
 +                                      int64_t(-1));
 +
 +    } else if (lease->type_ == Lease::TYPE_PD) {
 +        // IA_PD
 +        StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
 +                                                             lease->subnet_id_,
 +                                                             "assigned-pds"),
 +                                      int64_t(-1));
 +
 +    }
 +
 +    // Increase total number of reclaimed leases.
 +    StatsMgr::instance().addValue("reclaimed-leases", int64_t(1));
 +
 +    // Increase number of reclaimed leases for a subnet.
 +    StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
 +                                                         lease->subnet_id_,
 +                                                         "reclaimed-leases"),
 +                                  int64_t(1));
 +}
 +
 +void
 +AllocEngine::reclaimExpiredLease(const Lease4Ptr& lease,
 +                                 const DbReclaimMode& reclaim_mode,
 +                                 const CalloutHandlePtr& callout_handle) {
 +
 +    LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
 +              ALLOC_ENGINE_V4_LEASE_RECLAIM)
 +        .arg(Pkt4::makeLabel(lease->hwaddr_, lease->client_id_))
 +        .arg(lease->addr_.toText());
 +
 +    // The skip flag indicates if the callouts have taken responsibility
 +    // for reclaiming the lease. The callout will set this to true if
 +    // it reclaims the lease itself. In this case the reclamation routine
 +    // will not update DNS nor update the database.
 +    bool skipped = false;
 +    if (callout_handle) {
 +        callout_handle->deleteAllArguments();
 +        callout_handle->setArgument("lease4", lease);
 +        callout_handle->setArgument("remove_lease", reclaim_mode == DB_RECLAIM_REMOVE);
 +
 +        HooksManager::callCallouts(Hooks.hook_index_lease4_expire_,
 +                                   *callout_handle);
 +
 +        skipped = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
 +    }
 +
 +    /// @todo: Maybe add support for DROP status?
 +    /// Not sure if we need to support every possible status everywhere.
 +
 +    if (!skipped) {
 +
 +        // Generate removal name change request for D2, if required.
 +        // This will return immediatelly if the DNS wasn't updated
 +        // when the lease was created.
 +        queueNCR(CHG_REMOVE, lease);
 +
 +        // Let's check if the lease that just expired is in DECLINED state.
 +        // If it is, we need to conduct couple extra steps and also force
 +        // its removal.
 +        bool remove_tmp = (reclaim_mode == DB_RECLAIM_REMOVE);
 +        if (lease->state_ == Lease::STATE_DECLINED) {
 +            // There's no point in keeping declined lease after its
 +            // reclaimation. Declined lease doesn't have any client
 +            // identifying information anymore.
 +            if (reclaim_mode != DB_RECLAIM_LEAVE_UNCHANGED) {
 +                remove_tmp = true;
 +            }
 +
 +            // Do extra steps required for declined lease reclaimation:
 +            // - bump decline-related stats
 +            // - log separate message
-             reclaimDeclined(lease);
++            remove_tmp = reclaimDeclined(lease);
 +        }
 +
 +        if (reclaim_mode != DB_RECLAIM_LEAVE_UNCHANGED) {
 +            // Reclaim the lease - depending on the configuration, set the
 +            // expired-reclaimed state or simply remove it.
 +            LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
 +            reclaimLeaseInDatabase<Lease4Ptr>(lease, remove_tmp,
 +                                              boost::bind(&LeaseMgr::updateLease4,
 +                                                          &lease_mgr, _1));
 +        }
 +    }
 +
 +    // Decrease number of assigned addresses.
 +    StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
 +                                                         lease->subnet_id_,
 +                                                         "assigned-addresses"),
 +                                  int64_t(-1));
 +
 +    // Increase total number of reclaimed leases.
 +    StatsMgr::instance().addValue("reclaimed-leases", int64_t(1));
 +
 +    // Increase number of reclaimed leases for a subnet.
 +    StatsMgr::instance().addValue(StatsMgr::generateName("subnet",
 +                                                         lease->subnet_id_,
 +                                                         "reclaimed-leases"),
 +                                  int64_t(1));
 +}
 +
  void
  AllocEngine::deleteExpiredReclaimedLeases4(const uint32_t secs) {
      LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
@@@ -1853,9 -1847,41 +1913,9 @@@ AllocEngine::reclaimDeclined(const Leas
      // Note that we do not touch assigned-addresses counters. Those are
      // modified in whatever code calls this method.
  
-     /// @todo: call lease6_decline_recycle hook here.
+     return (true);
  }
  
 -template<typename LeasePtrType, typename IdentifierType>
 -void
 -AllocEngine::queueRemovalNameChangeRequest(const LeasePtrType& lease,
 -                                           const IdentifierType& identifier) const {
 -
 -    // Check if there is a need for update.
 -    if (!lease || lease->hostname_.empty() || (!lease->fqdn_fwd_ && !lease->fqdn_rev_)
 -        || !CfgMgr::instance().getD2ClientMgr().ddnsEnabled()) {
 -        return;
 -    }
 -
 -    try {
 -        // Create DHCID
 -        std::vector<uint8_t> hostname_wire;
 -        OptionDataTypeUtil::writeFqdn(lease->hostname_, hostname_wire, true);
 -        dhcp_ddns::D2Dhcid dhcid = D2Dhcid(identifier, hostname_wire);
 -
 -        // Create name change request.
 -        NameChangeRequestPtr ncr(new NameChangeRequest(isc::dhcp_ddns::CHG_REMOVE,
 -                                                       lease->fqdn_fwd_, lease->fqdn_rev_,
 -                                                       lease->hostname_,
 -                                                       lease->addr_.toText(),
 -                                                       dhcid, 0, lease->valid_lft_));
 -        // Send name change request.
 -        CfgMgr::instance().getD2ClientMgr().sendRequest(ncr);
 -
 -    } catch (const std::exception& ex) {
 -        LOG_ERROR(alloc_engine_logger, ALLOC_ENGINE_REMOVAL_NCR_FAILED)
 -            .arg(lease->addr_.toText())
 -            .arg(ex.what());
 -    }
 -}
  
  template<typename LeasePtrType>
  void AllocEngine::reclaimLeaseInDatabase(const LeasePtrType& lease,
diff --cc src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc
index aea83c1cfc,8514514384..cb26d3ff53
--- a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc
@@@ -1185,18 -1171,54 +1195,64 @@@ public
      /// @brief Test that statistics is updated when leases are reclaimed.
      void testReclaimExpiredLeasesStats();
  
 +    /// @brief Test that expired leases are reclaimed before they are allocated.
 +    ///
 +    /// @param msg_type DHCPv6 message type.
 +    /// @param use_reclaimed Boolean parameter indicating if the leases
 +    /// stored in the lease database should be marked as 'expired-reclaimed'
 +    /// or 'expired'. This allows to test whether the allocation engine can
 +    /// determine that the lease has been reclaimed already and not reclaim
 +    /// it the second time.
 +    void testReclaimReusedLeases(const uint16_t msg_type, const bool use_reclaimed);
 +
+     /// @brief Callout for lease6_recover
+     ///
+     /// This callout stores passed parameter into static fields.
+     ///
+     /// @param callout_handle will be provided by hooks framework
+     /// @return always 0
+     static int lease6RecoverCallout(CalloutHandle& callout_handle) {
+         callout_name_ = "lease6_recover";
+ 
+         callout_handle.getArgument("lease6", callout_lease_);
+ 
+         return (0);
+     }
+ 
+     /// @brief Callout for lease6_recover that sets status to SKIP
+     ///
+     /// This callout stores passed parameter into static fields.
+     ///
+     /// @param callout_handle will be provided by hooks framework
+     /// @return always 0
+     static int lease6RecoverSkipCallout(CalloutHandle& callout_handle) {
+         // Set the next step status to SKIP
+         callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
+ 
+         return (lease6RecoverCallout(callout_handle));
+     }
+ 
+     /// @brief Test install a hook callout, recovers declined leases
+     ///
+     /// This test: declines, then expires half of the leases, then
+     /// installs a callout on lease6_recover hook, then reclaims
+     /// expired leases and checks that:
+     /// - the callout was indeed called
+     /// - the parameter (lease6) was indeed passed as expected
+     /// - checks that the leases are removed (skip=false) or
+     /// - checks that the leases are still there (skip=true)
+     /// @param skip should the callout set the next step status to skip?
+     void
+     testReclaimDeclinedHook(bool skip);
+ 
+     /// The following parameters will be written by a callout
+     static std::string callout_name_; ///< Stores callout name
+     static Lease6Ptr callout_lease_;  ///< Stores callout parameter
  };
  
+ std::string ExpirationAllocEngine6Test::callout_name_;
+ Lease6Ptr ExpirationAllocEngine6Test::callout_lease_;
+ 
  ExpirationAllocEngine6Test::ExpirationAllocEngine6Test()
      : ExpirationAllocEngineTest<Lease6Ptr>("type=memfile universe=6 persist=false") {
      createLeases();
@@@ -1330,73 -1340,44 +1390,111 @@@ ExpirationAllocEngine6Test::testReclaim
      }
  }
  
 +void
 +ExpirationAllocEngine6Test::testReclaimReusedLeases(const uint16_t msg_type,
 +                                                    const bool use_reclaimed) {
 +    BOOST_STATIC_ASSERT(TEST_LEASES_NUM < 1000);
 +
 +    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
 +        // Depending on the parameter, mark leases 'expired-reclaimed' or
 +        // simply 'expired'.
 +        if (use_reclaimed) {
 +            reclaim(i, 1000 - i);
 +
 +        } else {
 +            // Mark all leases as expired.
 +            expire(i, 1000 - i);
 +        }
 +
 +        // For the Renew case, we don't change the ownership of leases. We
 +        // will let the lease owners renew them. For other cases, we modify
 +        // the DUIDs to simulate reuse of expired leases.
 +        if (msg_type != DHCPV6_RENEW) {
 +            transferOwnership(i);
 +        }
 +    }
 +
 +    // Create subnet and the pool. This is required by the allocation process.
 +    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 64, 10, 20, 50, 60,
 +                                  SubnetID(1)));
 +    ASSERT_NO_THROW(subnet->addPool(Pool6Ptr(new Pool6(Lease::TYPE_NA,
 +                                                       IOAddress("2001:db8:1::"),
 +                                                       IOAddress("2001:db8:1::FFFF")))));
 +
 +    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
 +        // Build the context.
 +        AllocEngine::ClientContext6 ctx(subnet, leases_[i]->duid_, 1,
 +                                        leases_[i]->addr_,
 +                                        Lease::TYPE_NA,
 +                                        false, false,
 +                                        leases_[i]->hostname_,
 +                                        msg_type == DHCPV6_SOLICIT);
 +        // Query is needed for logging purposes.
 +        ctx.query_.reset(new Pkt6(msg_type, 0x1234));
 +
 +        // Depending on the message type, we will call a different function.
 +        if (msg_type == DHCPV6_RENEW) {
 +            ASSERT_NO_THROW(engine_->renewLeases6(ctx));
 +
 +        } else {
 +            ASSERT_NO_THROW(engine_->allocateLeases6(ctx));
 +        }
 +    }
 +
 +    // The Solicit should not trigger leases reclamation. The Renew and
 +    // Request must trigger leases reclamation unless the lease is
 +    // initially reclaimed.
 +    if (use_reclaimed || (msg_type == DHCPV6_SOLICIT)) {
 +        EXPECT_TRUE(testStatistics("reclaimed-leases", 0));
 +
 +    } else {
 +        EXPECT_TRUE(testStatistics("reclaimed-leases", TEST_LEASES_NUM));
 +        // Leases should have been updated in the lease database and their
 +        // state should not be 'expired-reclaimed' anymore.
 +        EXPECT_TRUE(testLeases(&leaseNotReclaimed, &allLeaseIndexes));
 +
 +    }
 +
 +}
 +
+ void
+ ExpirationAllocEngine6Test::testReclaimDeclinedHook(bool skip) {
+     for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
+ 
+         // Mark leases with even indexes as expired.
+         if (evenLeaseIndex(i)) {
+ 
+             // Mark lease as declined with 100 seconds of probation-period
+             // (i.e. lease is supposed to be off limits for 100 seconds)
+             decline(i, 100);
+ 
+             // The higher the index, the more expired the lease.
+             expire(i, 10 + i);
+         }
+     }
+ 
+     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                         "lease6_recover",
+                         skip ? lease6RecoverSkipCallout : lease6RecoverCallout));
+ 
+     // Run leases reclamation routine on all leases.
+     ASSERT_NO_THROW(reclaimExpiredLeases(0, 0, true));
+ 
+     // Make sure that the callout really was called. It was supposed to modify
+     // the callout_name_ and store the lease in callout_lease_
+     EXPECT_EQ("lease6_recover", callout_name_);
+     EXPECT_TRUE(callout_lease_);
+ 
+     // Leases with even indexes should not exist in the DB
+     if (skip) {
+         // Skip status should have prevented removing the lease.
+         EXPECT_TRUE(testLeases(&leaseExists, &evenLeaseIndex));
+     } else {
+         // The hook hasn't modified next step status. The lease should be gone.
+         EXPECT_TRUE(testLeases(&leaseDoesntExist, &evenLeaseIndex));
+     }
+ };
+ 
  // This test verifies that the leases can be reclaimed without being removed
  // from the database. In such case, the leases' state is set to
  // "expired-reclaimed".
@@@ -1500,40 -1481,17 +1598,52 @@@ TEST_F(ExpirationAllocEngine6Test, recl
      testReclaimDeclinedStats("assigned-nas");
  }
  
 +// This test verifies that expired leases are reclaimed before they are
 +// allocated to another client sending a Request message.
 +TEST_F(ExpirationAllocEngine6Test, reclaimReusedLeases) {
 +    testReclaimReusedLeases(DHCPV6_REQUEST, false);
 +}
 +
 +// This test verifies that allocation engine detects that the expired
 +// lease has been reclaimed already when it reuses this lease.
 +TEST_F(ExpirationAllocEngine6Test, reclaimReusedLeasesAlreadyReclaimed) {
 +    testReclaimReusedLeases(DHCPV6_REQUEST, true);
 +}
 +
 +// This test verifies that expired leases are reclaimed before they
 +// are renewed.
 +TEST_F(ExpirationAllocEngine6Test, reclaimRenewedLeases) {
 +    testReclaimReusedLeases(DHCPV6_RENEW, false);
 +}
 +
 +// This test verifies that allocation engine detects that the expired
 +// lease has been reclaimed already when it renews the lease.
 +TEST_F(ExpirationAllocEngine6Test, reclaimRenewedLeasesAlreadyReclaimed) {
 +    testReclaimReusedLeases(DHCPV6_RENEW, true);
 +}
 +
 +// This test verifies that the expired leases are not reclaimed when the
 +// Solicit message is being processed.
 +TEST_F(ExpirationAllocEngine6Test, reclaimReusedLeasesSolicit) {
 +    testReclaimReusedLeases(DHCPV6_SOLICIT, false);
 +}
 +
 +// This test verifies that the 'expired-reclaimed' leases are not reclaimed
 +// again when the Solicit message is being processed.
 +TEST_F(ExpirationAllocEngine6Test, reclaimReusedLeasesSolicitAlreadyReclaimed) {
 +    testReclaimReusedLeases(DHCPV6_SOLICIT, true);
++
+ // This test verifies if the hooks installed on lease6_recover are called
+ // when the lease expires.
+ TEST_F(ExpirationAllocEngine6Test, reclaimDeclinedHook1) {
+     testReclaimDeclinedHook(false); // false = don't use skip callout
+ }
+ 
+ // This test verifies if the hooks installed on lease6_recover are called
+ // when the lease expires and that the next step status set to SKIP
+ // causes the recovery to not be conducted.
+ TEST_F(ExpirationAllocEngine6Test, reclaimDeclinedHook2) {
+     testReclaimDeclinedHook(true); // true = use skip callout
  }
  
  // *******************************************************
@@@ -1634,20 -1591,55 +1752,66 @@@ public
      /// @brief Test that statistics is updated when leases are reclaimed..
      void testReclaimExpiredLeasesStats();
  
 +    /// @brief Test that the lease is reclaimed before it is renewed or
 +    /// reused.
 +    ///
 +    /// @param msg_type DHCPv4 message type, i.e. DHCPDISCOVER or DHCPREQUEST.
 +    /// @param client_renews A boolean value which indicates if the test should
 +    /// simulate renewals of leases (if true) or reusing expired leases which
 +    /// belong to different clients (if false).
 +    /// @param use_reclaimed Boolean parameter indicating if the leases being
 +    /// reused should initially be reclaimed.
 +    void testReclaimReusedLeases(const uint8_t msg_type, const bool client_renews,
 +                                 const bool use_reclaimed);
 +
+     /// @brief Callout for lease4_recover
+     ///
+     /// This callout stores passed parameter into static fields.
+     ///
+     /// @param callout_handle will be provided by hooks framework
+     /// @return always 0
+     static int lease4RecoverCallout(CalloutHandle& callout_handle) {
+         callout_name_ = "lease4_recover";
+ 
+         callout_handle.getArgument("lease4", callout_lease_);
+ 
+         return (0);
+     }
+ 
+     /// @brief Callout for lease4_recover that sets status to SKIP
+     ///
+     /// This callout stores passed parameter into static fields.
+     ///
+     /// @param callout_handle will be provided by hooks framework
+     /// @return always 0
+     static int lease4RecoverSkipCallout(CalloutHandle& callout_handle) {
+         // Set the next step status to SKIP
+         callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
+ 
+         return (lease4RecoverCallout(callout_handle));
+     }
+ 
+     /// @brief Test install a hook callout, recovers declined leases
+     ///
+     /// This test: declines, then expires half of the leases, then
+     /// installs a callout on lease4_recover hook, then reclaims
+     /// expired leases and checks that:
+     /// - the callout was indeed called
+     /// - the parameter (lease4) was indeed passed as expected
+     /// - checks that the leases are removed (skip=false) or
+     /// - checks that the leases are still there (skip=true)
+     /// @param skip should the callout set the next step status to skip?
+     void
+     testReclaimDeclinedHook(bool skip);
+ 
 -
+     /// The following parameters will be written by a callout
+     static std::string callout_name_; ///< Stores callout name
+     static Lease4Ptr callout_lease_;  ///< Stores callout parameter
  };
  
+ std::string ExpirationAllocEngine4Test::callout_name_;
+ Lease4Ptr ExpirationAllocEngine4Test::callout_lease_;
+ 
  ExpirationAllocEngine4Test::ExpirationAllocEngine4Test()
      : ExpirationAllocEngineTest<Lease4Ptr>("type=memfile universe=4 persist=false") {
      createLeases();
@@@ -1874,77 -1844,43 +2042,114 @@@ ExpirationAllocEngine4Test::testReclaim
      }
  }
  
 +void
 +ExpirationAllocEngine4Test::testReclaimReusedLeases(const uint8_t msg_type,
 +                                                    const bool client_renews,
 +                                                    const bool use_reclaimed) {
 +    // Let's restrict the number of leases.
 +    BOOST_STATIC_ASSERT(TEST_LEASES_NUM < 1000);
 +
 +    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
 +        // Depending on the parameter, mark leases 'expired-reclaimed' or
 +        // simply 'expired'.
 +        if (use_reclaimed) {
 +            reclaim(i, 1000 - i);
 +
 +        } else {
 +            // Mark all leases as expired.
 +            expire(i, 1000 - i);
 +        }
 +
 +        // Check if we're simulating renewals or reusing leases. If this is
 +        // about reusing leases, we should be using different MAC addresses
 +        // or client identifiers for the leases than those stored presently
 +        // in the database.
 +        if (!client_renews) {
 +            // This function modifies the MAC address or the client identifier
 +            // of the test lease to make sure it doesn't match the one we
 +            // have in the database.
 +            transferOwnership(i);
 +        }
 +    }
 +
 +    // The call to AllocEngine::allocateLease4 requires the subnet selection.
 +    // The pool must be present within a subnet for the allocation engine to
 +    // hand out address from.
 +    Subnet4Ptr subnet(new Subnet4(IOAddress("10.0.0.0"), 16, 10, 20, 60, SubnetID(1)));
 +    ASSERT_NO_THROW(subnet->addPool(Pool4Ptr(new Pool4(IOAddress("10.0.0.0"),
 +                                                       IOAddress("10.0.255.255")))));
 +
 +    // Re-allocate leases (reuse or renew).
 +    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
 +        // Build the context.
 +        AllocEngine::ClientContext4 ctx(subnet, leases_[i]->client_id_,
 +                                        leases_[i]->hwaddr_,
 +                                        leases_[i]->addr_, false, false,
 +                                        leases_[i]->hostname_,
 +                                        msg_type == DHCPDISCOVER);
 +        // Query is needed for logging purposes.
 +        ctx.query_.reset(new Pkt4(msg_type, 0x1234));
 +
 +        // Re-allocate a lease. Note that the iterative will pick addresses
 +        // starting from the beginning of the pool. This matches exactly
 +        // the set of addresses we have allocated and stored in the database.
 +        // Since all leases are marked expired the allocation engine will
 +        // reuse them or renew them as appropriate.
 +        ASSERT_NO_THROW(engine_->allocateLease4(ctx));
 +    }
 +
 +    // If DHCPDISCOVER is being processed, the leases should not be reclaimed.
 +    // Also, the leases should not be reclaimed if they are already in the
 +    // 'expired-reclaimed' state.
 +    if (use_reclaimed || (msg_type == DHCPDISCOVER)) {
 +        EXPECT_TRUE(testStatistics("reclaimed-leases", 0));
 +
 +    } else if (msg_type == DHCPREQUEST) {
 +        // Re-allocation of expired leases should result in reclamations.
 +        EXPECT_TRUE(testStatistics("reclaimed-leases", TEST_LEASES_NUM));
 +        // Leases should have been updated in the lease database and their
 +        // state should not be 'expired-reclaimed' anymore.
 +        EXPECT_TRUE(testLeases(&leaseNotReclaimed, &allLeaseIndexes));
 +    }
 +}
 +
+ void
+ ExpirationAllocEngine4Test::testReclaimDeclinedHook(bool skip) {
+     for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
+ 
+         // Mark leases with even indexes as expired.
+         if (evenLeaseIndex(i)) {
+ 
+             // Mark lease as declined with 100 seconds of probation-period
+             // (i.e. lease is supposed to be off limits for 100 seconds)
+             decline(i, 100);
+ 
+             // The higher the index, the more expired the lease.
+             expire(i, 10 + i);
+         }
+     }
+ 
+     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                         "lease4_recover",
+                         skip ? lease4RecoverSkipCallout : lease4RecoverCallout));
+ 
+     // Run leases reclamation routine on all leases.
+     ASSERT_NO_THROW(reclaimExpiredLeases(0, 0, true));
+ 
+     // Make sure that the callout really was called. It was supposed to modify
+     // the callout_name_ and store the lease in callout_lease_
+     EXPECT_EQ("lease4_recover", callout_name_);
+     EXPECT_TRUE(callout_lease_);
+ 
+     // Leases with even indexes should not exist in the DB
+     if (skip) {
+         // Skip status should have prevented removing the lease.
+         EXPECT_TRUE(testLeases(&leaseExists, &evenLeaseIndex));
+     } else {
+         // The hook hasn't modified next step status. The lease should be gone.
+         EXPECT_TRUE(testLeases(&leaseDoesntExist, &evenLeaseIndex));
+     }
+ };
  
  // This test verifies that the leases can be reclaimed without being removed
  // from the database. In such case, the leases' state is set to
@@@ -2055,52 -1991,17 +2260,64 @@@ TEST_F(ExpirationAllocEngine4Test, recl
      testReclaimDeclinedStats("assigned-addresses");
  }
  
 +// This test verifies that the lease is reclaimed before it is reused.
 +TEST_F(ExpirationAllocEngine4Test, reclaimReusedLeases) {
 +    // First false value indicates that the leases will be reused.
 +    // Second false value indicates that the lease will not be
 +    // initially reclaimed.
 +    testReclaimReusedLeases(DHCPREQUEST, false, false);
 +}
 +
 +// This test verifies that the lease is not reclaimed when it is
 +// reused and  if its state indicates that it has been already reclaimed.
 +TEST_F(ExpirationAllocEngine4Test, reclaimReusedLeasesAlreadyReclaimed) {
 +    // false value indicates that the leases will be reused
 +    // true value indicates that the lease will be initially reclaimed.
 +    testReclaimReusedLeases(DHCPREQUEST, false, true);
 +}
 +
 +// This test verifies that the expired lease is reclaimed before it
 +// is renewed.
 +TEST_F(ExpirationAllocEngine4Test, reclaimRenewedLeases) {
 +    // true value indicates that the leases will be renewed.
 +    // false value indicates that the lease will not be initially
 +    // reclaimed.
 +    testReclaimReusedLeases(DHCPREQUEST, true, false);
 +}
 +
 +// This test verifies that the lease is not reclaimed upon renewal
 +// if its state indicates that it has been already reclaimed.
 +TEST_F(ExpirationAllocEngine4Test, reclaimRenewedLeasesAlreadyReclaimed) {
 +    // First true value indicates that the leases will be renewed.
 +    // Second true value indicates that the lease will be initially
 +    // reclaimed.
 +    testReclaimReusedLeases(DHCPREQUEST, true, true);
 +}
 +
 +// This test verifies that the reused lease is not reclaimed when the
 +// processed message is a DHCPDISCOVER.
 +TEST_F(ExpirationAllocEngine4Test, reclaimReusedLeasesDiscover) {
 +    testReclaimReusedLeases(DHCPDISCOVER, false, false);
 +}
 +
 +// This test verifies that the lease being in the 'expired-reclaimed'
 +// state is not reclaimed again when processing the DHCPDISCOVER
 +// message.
 +TEST_F(ExpirationAllocEngine4Test, reclaimRenewedLeasesDiscoverAlreadyReclaimed) {
 +    testReclaimReusedLeases(DHCPDISCOVER, false, true);
 +}
 +
+ // This test verifies if the hooks installed on lease4_recover are called
+ // when the lease expires.
+ TEST_F(ExpirationAllocEngine4Test, reclaimDeclinedHook1) {
+     testReclaimDeclinedHook(false); // false = don't use skip callout
+ }
+ 
+ // This test verifies if the hooks installed on lease4_recover are called
+ // when the lease expires and that the next step status set to SKIP
+ // causes the recovery to not be conducted.
+ TEST_F(ExpirationAllocEngine4Test, reclaimDeclinedHook2) {
+     testReclaimDeclinedHook(true); // true = use skip callout
+ }
  
  }; // end of anonymous namespace