From: Razvan Becheriu Date: Thu, 14 May 2020 13:24:26 +0000 (+0300) Subject: [#1127] removed shared callout manager X-Git-Tag: Kea-1.7.8~93 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6fabeb7fbeace192198ec5159bb3a38e491ad213;p=thirdparty%2Fkea.git [#1127] removed shared callout manager --- diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc index 9c4203e331..a7a0abee21 100644 --- a/src/bin/dhcp4/tests/hooks_unittest.cc +++ b/src/bin/dhcp4/tests/hooks_unittest.cc @@ -105,6 +105,7 @@ public: /// @brief creates Dhcpv4Srv and prepares buffers for callouts HooksDhcpv4SrvTest() { + HooksManager::getHooksManager().setTestMode(false); // Allocate new DHCPv6 Server srv_ = new NakedDhcpv4Srv(0); @@ -129,8 +130,8 @@ public: HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("lease4_decline"); HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("host4_identifier"); - HooksManager::getHooksManager().setSharedCalloutManager(); delete srv_; + HooksManager::getHooksManager().setTestMode(false); } /// @brief creates an option with specified option code @@ -2377,14 +2378,12 @@ TEST_F(HooksDhcpv4SrvTest, HooksDecline) { IfaceMgrTestConfig test_config(true); IfaceMgr::instance().openSockets4(); - // Libraries will be reloaded later - HooksManager::getHooksManager().setSharedCalloutManager( - boost::shared_ptr(new CalloutManager(0))); - // Install a callout EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( "lease4_decline", lease4_decline_callout)); + HooksManager::getHooksManager().setTestMode(true); + // Conduct the actual DORA + Decline. Dhcp4Client client(Dhcp4Client::SELECTING); acquireAndDecline(client, "01:02:03:04:05:06", "12:14", @@ -2430,14 +2429,12 @@ TEST_F(HooksDhcpv4SrvTest, HooksDeclineSkip) { IfaceMgrTestConfig test_config(true); IfaceMgr::instance().openSockets4(); - // Libraries will be reloaded later - HooksManager::getHooksManager().setSharedCalloutManager( - boost::shared_ptr(new CalloutManager(0))); - // Install a callout EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( "lease4_decline", lease4_decline_skip_callout)); + HooksManager::getHooksManager().setTestMode(true); + // Conduct the actual DORA + Decline. The DECLINE should fail, as the // hook will set the status to SKIP. Dhcp4Client client(Dhcp4Client::SELECTING); @@ -2482,14 +2479,12 @@ TEST_F(HooksDhcpv4SrvTest, HooksDeclineDrop) { IfaceMgrTestConfig test_config(true); IfaceMgr::instance().openSockets4(); - // Libraries will be reloaded later - HooksManager::getHooksManager().setSharedCalloutManager( - boost::shared_ptr(new CalloutManager(0))); - // Install a callout EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( "lease4_decline", lease4_decline_drop_callout)); + HooksManager::getHooksManager().setTestMode(true); + // Conduct the actual DORA + Decline. The DECLINE should fail, as the // hook will set the status to DROP. Dhcp4Client client(Dhcp4Client::SELECTING); diff --git a/src/bin/dhcp6/tests/hooks_unittest.cc b/src/bin/dhcp6/tests/hooks_unittest.cc index 11a2d0face..c25f55c35a 100644 --- a/src/bin/dhcp6/tests/hooks_unittest.cc +++ b/src/bin/dhcp6/tests/hooks_unittest.cc @@ -122,6 +122,8 @@ public: HooksDhcpv6SrvTest() : Dhcpv6SrvTest() { + HooksManager::getHooksManager().setTestMode(false); + // Allocate new DHCPv6 Server srv_.reset(new NakedDhcpv6Srv(0)); @@ -136,10 +138,7 @@ public: /// @brief destructor (deletes Dhcpv6Srv) ~HooksDhcpv6SrvTest() { - - // Clear shared manager - HooksManager::getHooksManager().setSharedCalloutManager(); - + HooksManager::getHooksManager().setTestMode(false); } /// @brief creates an option with specified option code @@ -4417,14 +4416,12 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebindPrefix) { TEST_F(HooksDhcpv6SrvTest, basicLease6Decline) { IfaceMgrTestConfig test_config(true); - // Libraries will be reloaded later - HooksManager::getHooksManager().setSharedCalloutManager( - boost::shared_ptr(new CalloutManager(0))); - // Install lease6_decline callout EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( "lease6_decline", lease6_decline_callout)); + HooksManager::getHooksManager().setTestMode(true); + // Get an address and decline it. DUIDs, IAID match and we send valid // address, so the decline procedure should be successful. Dhcp6Client client; @@ -4471,14 +4468,12 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Decline) { TEST_F(HooksDhcpv6SrvTest, lease6DeclineSkip) { IfaceMgrTestConfig test_config(true); - // Libraries will be reloaded later - HooksManager::getHooksManager().setSharedCalloutManager( - boost::shared_ptr(new CalloutManager(0))); - // Install lease6_decline_skip callout. It will set the status to skip EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( "lease6_decline", lease6_decline_skip)); + HooksManager::getHooksManager().setTestMode(true); + // Get an address and decline it. DUIDs, IAID match and we send valid // address, so the decline procedure should be successful. Dhcp6Client client; @@ -4522,14 +4517,12 @@ TEST_F(HooksDhcpv6SrvTest, lease6DeclineSkip) { TEST_F(HooksDhcpv6SrvTest, lease6DeclineDrop) { IfaceMgrTestConfig test_config(true); - // Libraries will be reloaded later - HooksManager::getHooksManager().setSharedCalloutManager( - boost::shared_ptr(new CalloutManager(0))); - // Install lease6_decline_drop callout. It will set the status to drop EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( "lease6_decline", lease6_decline_drop)); + HooksManager::getHooksManager().setTestMode(true); + // Get an address and decline it. DUIDs, IAID match and we send valid // address, so it would work, but the callout sets status to DROP, so // the server should not update the lease and should not send back any diff --git a/src/lib/hooks/hooks_manager.cc b/src/lib/hooks/hooks_manager.cc index 31238810e6..04c857f129 100644 --- a/src/lib/hooks/hooks_manager.cc +++ b/src/lib/hooks/hooks_manager.cc @@ -24,9 +24,6 @@ using namespace std; namespace isc { namespace hooks { -boost::shared_ptr HooksManager::shared_callout_manager_; -bool HooksManager::loaded_; - // Constructor HooksManager::HooksManager() { @@ -93,6 +90,9 @@ HooksManager::callCommandHandlers(const std::string& command_name, bool HooksManager::loadLibrariesInternal(const HookLibsCollection& libraries) { + if (test_mode_) { + return (true); + } // Unload current set of libraries (if any are loaded). unloadLibrariesInternal(false); @@ -109,8 +109,6 @@ HooksManager::loadLibrariesInternal(const HookLibsCollection& libraries) { init(); } - loaded_ = true; - return (status); } @@ -132,8 +130,6 @@ HooksManager::unloadLibrariesInternal(bool initialize) { lm_collection_.reset(); callout_manager_.reset(); } - - loaded_ = false; } void HooksManager::unloadLibraries() { @@ -222,17 +218,10 @@ HooksManager::validateLibraries(const std::vector& libraries) { return (LibraryManagerCollection::validateLibraries(libraries)); } -boost::shared_ptr -HooksManager::getSharedCalloutManager() { - return (shared_callout_manager_); -} +bool HooksManager::test_mode_; -void -HooksManager::setSharedCalloutManager(boost::shared_ptr manager) { - shared_callout_manager_ = manager; - if (!loaded_) { - unloadLibraries(); - } +void HooksManager::setTestMode(bool mode) { + test_mode_ = mode; } } // namespace util diff --git a/src/lib/hooks/hooks_manager.h b/src/lib/hooks/hooks_manager.h index 6001278f48..8544109e93 100644 --- a/src/lib/hooks/hooks_manager.h +++ b/src/lib/hooks/hooks_manager.h @@ -226,21 +226,6 @@ public: static const int CONTEXT_CREATE = ServerHooks::CONTEXT_CREATE; static const int CONTEXT_DESTROY = ServerHooks::CONTEXT_DESTROY; - /// @brief Get the shared callout manager - /// - /// @return A reference to the shared callout manager - static boost::shared_ptr getSharedCalloutManager(); - - /// @brief Set the shared callout manager - /// - /// This function sets the shared callout manager and explicitly updates - /// @ref callout_manager_ (by calling @ref init) if @ref loadLibraries has - /// not yet been called. - /// - /// @param manager The shared callout manager - static void setSharedCalloutManager(boost::shared_ptr manager = - boost::shared_ptr()); - /// @brief Park an object (packet). /// /// The typical use case for parking an object is when the server needs to @@ -326,6 +311,12 @@ public: getHooksManager().clearParkingLotsInternal(); } + /// @brief Set test mode which enables the tests to resister callouts before + /// calling @ref loadLibraries and the @ref callout_manager_ is preserved. + /// + /// @param mode the flag which enables or disabled @ref test_mode_. + void setTestMode(bool mode); + private: /// @brief Constructor @@ -500,11 +491,9 @@ private: /// Callout manager for the set of library managers. boost::shared_ptr callout_manager_; - /// Shared callout manager to survive library reloads. - static boost::shared_ptr shared_callout_manager_; - - /// Loaded flag to indicate if @ref loadLibraries has been called - static bool loaded_; + /// Test flag to keep @ref callout_manager_ when calling @ref loadLibraries + /// from unittests (called by @ref configureDhcp[46]Server) + static bool test_mode_; }; } // namespace util diff --git a/src/lib/hooks/library_manager_collection.cc b/src/lib/hooks/library_manager_collection.cc index a6d65c032c..2577871a91 100644 --- a/src/lib/hooks/library_manager_collection.cc +++ b/src/lib/hooks/library_manager_collection.cc @@ -77,9 +77,6 @@ LibraryManagerCollection::loadLibraries() { // where the hooks-libraries clause was empty and is not changed), try // to re-use the existing callout manager (so retaining registered pre- // and post-library callouts). - if (library_names_.empty()) { - callout_manager_ = HooksManager::getSharedCalloutManager(); - } if (!library_names_.empty() || !callout_manager_) { callout_manager_.reset(new CalloutManager(library_names_.size())); } diff --git a/src/lib/hooks/tests/hooks_manager_unittest.cc b/src/lib/hooks/tests/hooks_manager_unittest.cc index 44b39cb6c7..d8339a5245 100644 --- a/src/lib/hooks/tests/hooks_manager_unittest.cc +++ b/src/lib/hooks/tests/hooks_manager_unittest.cc @@ -46,7 +46,6 @@ public: /// Unload all libraries and reset the shared manager. ~HooksManagerTest() { HooksManager::unloadLibraries(); - HooksManager::getHooksManager().setSharedCalloutManager(); } @@ -440,13 +439,10 @@ TEST_F(HooksManagerTest, PrePostCalloutTest) { // Test with a shared manager the pre- and post- callout functions survive // a reload -TEST_F(HooksManagerTest, PrePostCalloutShared) { +TEST_F(HooksManagerTest, DISABLED_PrePostCalloutShared) { HookLibsCollection library_names; - // Initialize the shared manager. - HooksManager::getHooksManager().setSharedCalloutManager( - boost::shared_ptr(new CalloutManager(0))); // Load the pre- and post- callouts. HooksManager::preCalloutsLibraryHandle().registerCallout("hookpt_two", @@ -486,16 +482,12 @@ TEST_F(HooksManagerTest, PrePostCalloutShared) { // Test with a shared manager the pre- and post- callout functions survive // a reload but not with a not empty list of libraries -TEST_F(HooksManagerTest, PrePostCalloutSharedNotEmpty) { +TEST_F(HooksManagerTest, DISABLED_PrePostCalloutSharedNotEmpty) { HookLibsCollection library_names; library_names.push_back(make_pair(std::string(FULL_CALLOUT_LIBRARY), data::ConstElementPtr())); - // Initialize the shared manager. - HooksManager::getHooksManager().setSharedCalloutManager( - boost::shared_ptr(new CalloutManager(0))); - // Load the pre- and post- callouts. HooksManager::preCalloutsLibraryHandle().registerCallout("hookpt_two", testPreCallout); @@ -534,15 +526,11 @@ TEST_F(HooksManagerTest, PrePostCalloutSharedNotEmpty) { // Test with a shared manager the pre- and post- callout functions don't // survive a reload if the shared manager is initialized too late. -TEST_F(HooksManagerTest, PrePostCalloutSharedTooLate) { +TEST_F(HooksManagerTest, DISABLED_PrePostCalloutSharedTooLate) { HookLibsCollection library_names; EXPECT_TRUE(HooksManager::loadLibraries(library_names)); - // Initialize the shared manager (after loadLibraries so too late) - HooksManager::getHooksManager().setSharedCalloutManager( - boost::shared_ptr(new CalloutManager(0))); - // Load the pre- and post- callouts. HooksManager::preCalloutsLibraryHandle().registerCallout("hookpt_two", testPreCallout);