Commit 8d52e11c authored by Tomek Mrugalski's avatar Tomek Mrugalski 🛰
Browse files

[3988] lease{4,6}_recover hook implemented (with unit-tests)

parent 814fabe9
......@@ -274,4 +274,22 @@ packet processing. Hook points that are not specific to packet processing
skip flag is only set when the lease has been actually reclaimed in the
database by the callout.
@subsection dhcpv4HooksLease4Recover lease4_recover
- @b Arguments:
- name: @b lease4, type: isc::dhcp::Lease4Ptr, direction: <b>in</b>
- @b Description: this callout is executed for each declined lease that
has expired and is being recovered. The lease has already been stripped
from any client identifying information when it was put into declined
state. In principle the callouts can modify the lease in this hook,
but it makes little sense. There's no useful data in the lease, except
the IPv4 address (which must not be modified).
- <b>Next step status</b>: if the callout sets the next step action to SKIP,
the server will skip the lease recovery. In other words, it will keep
the lease as is. This is not recommended in general, as the declined
expired leases will remain in the database and their recovery will
be attempted during the next reclaim cycle.
*/
......@@ -288,4 +288,22 @@ packet processing. Hook points that are not specific to packet processing
skip flag is only set when the lease has been actually reclaimed in the
database by the callout.
@subsection dhcpv6HooksLease4Recover lease6_recover
- @b Arguments:
- name: @b lease6, type: isc::dhcp::Lease6Ptr, direction: <b>in</b>
- @b Description: this callout is executed for each declined lease that
has expired and is being recovered. The lease has already been stripped
from any client identifying information when it was put into declined
state. In principle the callouts can modify the lease in this hook,
but it makes little sense. There's no useful data in the lease, except
the IPv4 address (which must not be modified).
- <b>Next step status</b>: if the callout sets the next step action to SKIP,
the server will skip the lease recovery. In other words, it will keep
the lease as is. This is not recommended in general, as the declined
expired leases will remain in the database and their recovery will
be attempted during the next reclaim cycle.
*/
......@@ -27,6 +27,7 @@
#include <dhcp/dhcp6.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>
......@@ -54,20 +55,24 @@ struct AllocEngineHooks {
int hook_index_lease4_select_; ///< index for "lease4_receive" hook point
int hook_index_lease4_renew_; ///< index for "lease4_renew" hook point
int hook_index_lease4_expire_; ///< index for "lease4_expire" hook point
int hook_index_lease4_recover_;///< index for "lease4_recover" hook point
int hook_index_lease6_select_; ///< index for "lease6_receive" hook point
int hook_index_lease6_renew_; ///< index for "lease6_renew" hook point
int hook_index_lease6_rebind_; ///< index for "lease6_rebind" hook point
int hook_index_lease6_expire_; ///< index for "lease6_expire" hook point
int hook_index_lease6_recover_;///< index for "lease6_recover" hook point
/// Constructor that registers hook points for AllocationEngine
AllocEngineHooks() {
hook_index_lease4_select_ = HooksManager::registerHook("lease4_select");
hook_index_lease4_renew_ = HooksManager::registerHook("lease4_renew");
hook_index_lease4_expire_ = HooksManager::registerHook("lease4_expire");
hook_index_lease4_recover_= HooksManager::registerHook("lease4_recover");
hook_index_lease6_select_ = HooksManager::registerHook("lease6_select");
hook_index_lease6_renew_ = HooksManager::registerHook("lease6_renew");
hook_index_lease6_rebind_ = HooksManager::registerHook("lease6_rebind");
hook_index_lease6_expire_ = HooksManager::registerHook("lease6_expire");
hook_index_lease6_recover_= HooksManager::registerHook("lease6_recover");
}
};
......@@ -1396,7 +1401,9 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
// Do extra steps required for declined lease reclaimation:
// - bump decline-related stats
// - log separate message
reclaimDeclined(lease);
// - call lease6_recover hooks
// (hooks can override the removal decision and keep the lease)
remove_tmp = reclaimDeclined(lease) && remove_lease;
}
// Reclaim the lease - depending on the configuration, set the
......@@ -1619,7 +1626,7 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
// Do extra steps required for declined lease reclaimation:
// - bump decline-related stats
// - log separate message
reclaimDeclined(lease);
remove_tmp = reclaimDeclined(lease) && remove_tmp;
}
// Reclaim the lease - depending on the configuration, set the
......@@ -1728,11 +1735,39 @@ AllocEngine::deleteExpiredReclaimedLeases4(const uint32_t secs) {
.arg(deleted_leases);
}
void
bool
AllocEngine::reclaimDeclined(const Lease4Ptr& lease) {
if (!lease || (lease->state_ != Lease::STATE_DECLINED) ) {
return;
return (true);
}
if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_lease4_recover_)) {
// Let's use a static callout handle. It will be initialized the first
// time lease6_recover is called and will keep to that value.
static CalloutHandlePtr callout_handle;
if (!callout_handle) {
callout_handle = HooksManager::createCalloutHandle();
}
// Delete all previous arguments
callout_handle->deleteAllArguments();
// Pass necessary arguments
callout_handle->setArgument("lease4", lease);
// Call the callouts
HooksManager::callCallouts(Hooks.hook_index_lease4_recover_, *callout_handle);
// Callouts decided to skip the action. This means that the lease is not
// assigned, so the client will get NoAddrAvail as a result. The lease
// won't be inserted into the database.
if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE4_RECOVER_SKIP)
.arg(lease->addr_.toText());
return (false);
}
}
LOG_INFO(alloc_engine_logger, ALLOC_ENGINE_V4_DECLINED_RECOVERED)
......@@ -1755,15 +1790,42 @@ AllocEngine::reclaimDeclined(const Lease4Ptr& lease) {
// Note that we do not touch assigned-addresses counters. Those are
// modified in whatever code calls this method.
/// @todo: call lease4_decline_recycle hook here.
return (true);
}
void
bool
AllocEngine::reclaimDeclined(const Lease6Ptr& lease) {
if (!lease || (lease->state_ != Lease::STATE_DECLINED) ) {
return;
return (true);
}
if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_lease6_recover_)) {
// Let's use a static callout handle. It will be initialized the first
// time lease6_recover is called and will keep to that value.
static CalloutHandlePtr callout_handle;
if (!callout_handle) {
callout_handle = HooksManager::createCalloutHandle();
}
// Delete all previous arguments
callout_handle->deleteAllArguments();
// Pass necessary arguments
callout_handle->setArgument("lease6", lease);
// Call the callouts
HooksManager::callCallouts(Hooks.hook_index_lease6_recover_, *callout_handle);
// Callouts decided to skip the action. This means that the lease is not
// assigned, so the client will get NoAddrAvail as a result. The lease
// won't be inserted into the database.
if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_RECOVER_SKIP)
.arg(lease->addr_.toText());
return (false);
}
}
LOG_INFO(alloc_engine_logger, ALLOC_ENGINE_V6_DECLINED_RECOVERED)
......@@ -1787,7 +1849,7 @@ AllocEngine::reclaimDeclined(const Lease6Ptr& lease) {
// 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>
......
......@@ -825,20 +825,24 @@ private:
/// These are the additional steps required when recoving a declined lease:
/// - bump decline recovered stat
/// - log lease recovery
/// - call hook (@todo)
/// - call lease4_recover hook
///
/// @param lease Lease to be reclaimed from Declined state
void reclaimDeclined(const Lease4Ptr& lease);
/// @return true if it's ok to remove the lease (false = hooks status says
/// to keep it)
bool reclaimDeclined(const Lease4Ptr& lease);
/// @brief Conducts steps necessary for reclaiming declined IPv6 lease.
///
/// These are the additional steps required when recoving a declined lease:
/// - bump decline recovered stat
/// - log lease recovery
/// - call hook (@todo)
/// - call lease6_recover hook
///
/// @param lease Lease to be reclaimed from Declined state
void reclaimDeclined(const Lease6Ptr& lease);
/// @return true if it's ok to remove the lease (false = hooks status says
/// to keep it)
bool reclaimDeclined(const Lease6Ptr& lease);
public:
......
......@@ -176,6 +176,14 @@ have been experienced. Any such errors should have preceding entries in the
log with details. No further attempts to communicate with kea-dhcp-ddns will
be made without intervention.
% DHCPSRV_HOOK_LEASE4_RECOVER_SKIP DHCPv4 lease %1 was not recoverd from the declined state because a callout set the skip status.
This debug message is printed when a callout installed on lease4_recover
hook point set the next step status to SKIP. For this particular hook point, this
indicates that the server should not recover the lease from declined state.
The server will leave the lease as it is, in the declined state. The
server will attempt to recover it the next time decline recovery procedure
takes place.
% DHCPSRV_HOOK_LEASE4_RENEW_SKIP DHCPv4 lease was not renewed because a callout set the skip flag.
This debug message is printed when a callout installed on lease4_renew
hook point set the skip flag. For this particular hook point, the setting
......@@ -196,6 +204,14 @@ extend the lifetime for a lease. If the client requested renewal of multiple
leases (by sending multiple IA options), the server will skip the renewal
of the one in question and will proceed with other renewals as usual.
% DHCPSRV_HOOK_LEASE6_RECOVER_SKIP DHCPv6 lease %1 was not recovered from declined state because a callout set the skip status.
This debug message is printed when a callout installed on lease6_recover
hook point set the next step status to SKIP. For this particular hook point, this
indicates that the server should not recover the lease from declined state.
The server will leave the lease as it is, in the declined state. The
server will attempt to recover it the next time decline recovery procedure
takes place.
% DHCPSRV_HOOK_LEASE6_SELECT_SKIP Lease6 (non-temporary) creation was skipped, because of callout skip flag.
This debug message is printed when a callout installed on lease6_select
hook point sets the skip flag. It means that the server was told that
......
......@@ -1088,6 +1088,8 @@ public:
AllocEnginePtr engine_;
};
/// @brief Specialization of the @c ExpirationAllocEngineTest class to test
/// reclamation of the IPv6 leases.
class ExpirationAllocEngine6Test : public ExpirationAllocEngineTest<Lease6Ptr> {
......@@ -1099,6 +1101,14 @@ public:
/// stores them in the lease manager.
ExpirationAllocEngine6Test();
/// @brief Virtual destructor.
///
/// Clears up static fields that may be modified by hooks.
virtual ~ExpirationAllocEngine6Test() {
callout_lease_.reset();
callout_name_ = string("");
}
/// @brief Creates collection of leases for a test.
///
/// It is called internally at the construction time.
......@@ -1161,12 +1171,62 @@ public:
/// @brief Test that statistics is updated when leases are reclaimed.
void testReclaimExpiredLeasesStats();
/// @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();
callout_argument_name = "lease6";
// Let's clear any garbage previous test may have left in static fields.
callout_name_ = string("");
callout_lease_.reset();
}
void
......@@ -1280,6 +1340,44 @@ ExpirationAllocEngine6Test::testReclaimExpiredLeasesStats() {
}
}
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".
......@@ -1383,6 +1481,19 @@ TEST_F(ExpirationAllocEngine6Test, reclaimDeclinedStats) {
testReclaimDeclinedStats("assigned-nas");
}
// 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
}
// *******************************************************
//
// DHCPv4 lease reclamation routine tests start here!
......@@ -1400,6 +1511,14 @@ public:
/// stores them in the lease manager.
ExpirationAllocEngine4Test();
/// @brief Virtual destructor.
///
/// Clears up static fields that may be modified by hooks.
virtual ~ExpirationAllocEngine4Test() {
callout_lease_.reset();
callout_name_ = string("");
}
/// @brief Creates collection of leases for a test.
///
/// It is called internally at the construction time.
......@@ -1472,8 +1591,55 @@ public:
/// @brief Test that statistics is updated when leases are reclaimed..
void testReclaimExpiredLeasesStats();
/// @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();
......@@ -1674,6 +1840,43 @@ ExpirationAllocEngine4Test::testReclaimExpiredLeasesStats() {
}
}
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
......@@ -1784,4 +1987,17 @@ TEST_F(ExpirationAllocEngine4Test, reclaimDeclinedStats) {
testReclaimDeclinedStats("assigned-addresses");
}
// This test verifies if the hooks installed on lease6_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 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(ExpirationAllocEngine4Test, reclaimDeclinedHook2) {
testReclaimDeclinedHook(true); // true = use skip callout
}
}; // end of anonymous namespace
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment