From: Jelte Jansen Date: Thu, 18 Oct 2012 14:40:31 +0000 (+0200) Subject: [2210] Add support for reconfigure in datasrc_clients_mgr X-Git-Tag: trac2402_base~4^2~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6319666e91e5f3f20d9e2fdb920ce90c2539d4ad;p=thirdparty%2Fkea.git [2210] Add support for reconfigure in datasrc_clients_mgr --- diff --git a/src/bin/auth/datasrc_clients_mgr.h b/src/bin/auth/datasrc_clients_mgr.h index 183ba3560a..3e9d76f376 100644 --- a/src/bin/auth/datasrc_clients_mgr.h +++ b/src/bin/auth/datasrc_clients_mgr.h @@ -21,12 +21,17 @@ #include #include +#include +#include #include +#include +#include #include #include #include +#include #include #include @@ -43,6 +48,7 @@ namespace datasrc_clientmgr_internal { /// \brief ID of commands from the DataSrcClientsMgr to DataSrcClientsBuilder. enum CommandID { NOOP, ///< Do nothing. Only useful for tests; no argument + RECONFIGURE, ///< Reconfigure the datasource client lists, configuration argument (TODO: describe here?) SHUTDOWN, ///< Shutdown the builder; no argument NUM_COMMANDS }; @@ -56,6 +62,10 @@ enum CommandID { typedef std::pair Command; } // namespace datasrc_clientmgr_internal +typedef std::map > + DataSrcClientListMap; + /// \brief Frontend to the manager object for data source clients. /// /// This class provides interfaces for configuring and updating a set of @@ -92,7 +102,7 @@ public: /// \throw std::bad_alloc internal memory allocation failure. /// \throw isc::Unexpected general unexpected system errors. DataSrcClientsMgrBase() : - builder_(&command_queue_, &cond_, &queue_mutex_), + builder_(&command_queue_, &cond_, &queue_mutex_, &clients_map_, &map_mutex_), builder_thread_(boost::bind(&BuilderType::run, &builder_)) {} @@ -161,10 +171,8 @@ private: std::list command_queue_; CondVarType cond_; // condition variable for queue operations MutexType queue_mutex_; // mutex to protect the queue -#ifdef notyet // until #2210 or #2212 - boost::shared_ptr clients_map_; + AuthSrv::DataSrcClientListsPtr clients_map_; MutexType map_mutex_; -#endif BuilderType builder_; ThreadType builder_thread_; // for safety this should be placed last @@ -199,12 +207,12 @@ public: /// /// \throw None DataSrcClientsBuilderBase(std::list* command_queue, - CondVarType* cond, MutexType* queue_mutex -#ifdef notyet - // In #2210 or #2212 we pass other data -#endif + CondVarType* cond, MutexType* queue_mutex, + AuthSrv::DataSrcClientListsPtr* clients_map, + MutexType* map_mutex ) : - command_queue_(command_queue), cond_(cond), queue_mutex_(queue_mutex) + command_queue_(command_queue), cond_(cond), queue_mutex_(queue_mutex), + clients_map_(clients_map), map_mutex_(map_mutex) {} /// \brief The main loop. @@ -225,10 +233,33 @@ private: // implementation really does nothing. void doNoop() {} + void doReconfigure(const isc::data::ConstElementPtr& config) { + if (config) { + try { + AuthSrv::DataSrcClientListsPtr new_clients_map = + configureDataSource(config); + typename MutexType::Locker locker(*map_mutex_); + std::swap(new_clients_map, *clients_map_); + } catch (isc::data::TypeError) { + // TODO: log + std::cout << "[XX] type error" << std::endl; + } catch (std::exception stde) { + // TODO: log + std::cout << "[XX] bug error" << std::endl; + } catch (...) { + // TODO: log + std::cout << "[XX] unknown error" << std::endl; + } + } + // lock + } + // The following are shared with the manager std::list* command_queue_; CondVarType* cond_; MutexType* queue_mutex_; + AuthSrv::DataSrcClientListsPtr* clients_map_; + MutexType* map_mutex_; }; // Shortcut typedef for normal use @@ -285,11 +316,14 @@ DataSrcClientsBuilderBase::handleCommand( } const boost::array command_desc = { - {"NOOP", "SHUTDOWN"} + {"NOOP", "RECONFIGURE", "SHUTDOWN"} }; LOG_DEBUG(auth_logger, DBGLVL_TRACE_BASIC, AUTH_DATASRC_CLIENTS_BUILDER_COMMAND).arg(command_desc.at(cid)); switch (command.first) { + case RECONFIGURE: + doReconfigure(command.second); + break; case SHUTDOWN: return (false); case NOOP: diff --git a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc index 4978d6efe2..13c5bd4fe4 100644 --- a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc +++ b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc @@ -28,7 +28,7 @@ namespace { class DataSrcClientsBuilderTest : public ::testing::Test { protected: DataSrcClientsBuilderTest() : - builder(&command_queue, &cond, &queue_mutex), + builder(&command_queue, &cond, &queue_mutex, &clients_map, &map_mutex), cond(command_queue, delayed_command_queue), shutdown_cmd(SHUTDOWN, ConstElementPtr()), noop_cmd(NOOP, ConstElementPtr()) @@ -37,8 +37,10 @@ protected: TestDataSrcClientsBuilder builder; std::list command_queue; // test command queue std::list delayed_command_queue; // commands available after wait + AuthSrv::DataSrcClientListsPtr clients_map; // 'configured' clients TestCondVar cond; TestMutex queue_mutex; + TestMutex map_mutex; const Command shutdown_cmd; const Command noop_cmd; }; @@ -92,10 +94,66 @@ TEST_F(DataSrcClientsBuilderTest, condWait) { EXPECT_EQ(2, queue_mutex.unlock_count); } +TEST_F(DataSrcClientsBuilderTest, reconfigure) { + // Full testing of different configurations is not here, but we + // do check a few cases of correct and erroneous input, to verify + // the error handling + + // A command structure we'll modify to send different commands + Command reconfig_cmd(RECONFIGURE, ConstElementPtr()); + + // Initially, no clients should be there + EXPECT_EQ(AuthSrv::DataSrcClientListsPtr(), clients_map); + + // A config that doesn't do much except be accepted + ConstElementPtr good_config = isc::data::Element::fromJSON( + "{" + "\"IN\": [{" + " \"type\": \"MasterFiles\"," + " \"params\": {}," + " \"cache-enable\": true" + "}]" + "}" + ); + + reconfig_cmd.second = good_config; + EXPECT_TRUE(builder.handleCommand(reconfig_cmd)); + EXPECT_EQ(1, clients_map->size()); + + // Store the nonempty clients map we now have + AuthSrv::DataSrcClientListsPtr working_config_clients(clients_map); + + // If a 'bad' command argument got here, the config validation should + // have failed already, but still, the handler should return true, + // and the clients_map should not be updated. + reconfig_cmd.second = isc::data::Element::create("{ \"foo\": \"bar\" }"); + EXPECT_TRUE(builder.handleCommand(reconfig_cmd)); + EXPECT_EQ(working_config_clients, clients_map); + + // The same goes for an empty parameter (it should at least be + // an empty map) + reconfig_cmd.second = ConstElementPtr(); + EXPECT_TRUE(builder.handleCommand(reconfig_cmd)); + EXPECT_EQ(working_config_clients, clients_map); + + // Reconfigure again with the same good clients, the result should + // be a different map than the original, but not an empty one. + reconfig_cmd.second = good_config; + EXPECT_TRUE(builder.handleCommand(reconfig_cmd)); + EXPECT_NE(working_config_clients, clients_map); + EXPECT_EQ(1, clients_map->size()); + + // And finally, try an empty config to disable all datasource clients + reconfig_cmd.second = isc::data::Element::createMap(); + EXPECT_TRUE(builder.handleCommand(reconfig_cmd)); + EXPECT_EQ(0, clients_map->size()); +} + TEST_F(DataSrcClientsBuilderTest, shutdown) { EXPECT_FALSE(builder.handleCommand(shutdown_cmd)); } + TEST_F(DataSrcClientsBuilderTest, badCommand) { // out-of-range command ID EXPECT_THROW(builder.handleCommand(Command(NUM_COMMANDS, diff --git a/src/bin/auth/tests/test_datasrc_clients_mgr.cc b/src/bin/auth/tests/test_datasrc_clients_mgr.cc index 72979b4844..1794b731ef 100644 --- a/src/bin/auth/tests/test_datasrc_clients_mgr.cc +++ b/src/bin/auth/tests/test_datasrc_clients_mgr.cc @@ -13,6 +13,7 @@ // PERFORMANCE OF THIS SOFTWARE. #include +#include #include "test_datasrc_clients_mgr.h" @@ -27,6 +28,8 @@ std::list FakeDataSrcClientsBuilder::command_queue_copy; TestCondVar* FakeDataSrcClientsBuilder::cond = NULL; TestCondVar FakeDataSrcClientsBuilder::cond_copy; TestMutex* FakeDataSrcClientsBuilder::queue_mutex = NULL; +AuthSrv::DataSrcClientListsPtr* FakeDataSrcClientsBuilder::clients_map = NULL; +TestMutex* FakeDataSrcClientsBuilder::map_mutex = NULL; TestMutex FakeDataSrcClientsBuilder::queue_mutex_copy; bool FakeDataSrcClientsBuilder::thread_waited = false; FakeDataSrcClientsBuilder::ExceptionFromWait diff --git a/src/bin/auth/tests/test_datasrc_clients_mgr.h b/src/bin/auth/tests/test_datasrc_clients_mgr.h index 7512e83715..31d66c7ff9 100644 --- a/src/bin/auth/tests/test_datasrc_clients_mgr.h +++ b/src/bin/auth/tests/test_datasrc_clients_mgr.h @@ -18,6 +18,7 @@ #include #include +#include #include @@ -130,6 +131,8 @@ public: static std::list* command_queue; static TestCondVar* cond; static TestMutex* queue_mutex; + static AuthSrv::DataSrcClientListsPtr* clients_map; + static TestMutex* map_mutex; static std::list command_queue_copy; static TestCondVar cond_copy; static TestMutex queue_mutex_copy; @@ -145,12 +148,16 @@ public: FakeDataSrcClientsBuilder( std::list* command_queue, TestCondVar* cond, - TestMutex* queue_mutex) + TestMutex* queue_mutex, + AuthSrv::DataSrcClientListsPtr* clients_map, + TestMutex* map_mutex) { FakeDataSrcClientsBuilder::started = false; FakeDataSrcClientsBuilder::command_queue = command_queue; FakeDataSrcClientsBuilder::cond = cond; FakeDataSrcClientsBuilder::queue_mutex = queue_mutex; + FakeDataSrcClientsBuilder::clients_map = clients_map; + FakeDataSrcClientsBuilder::map_mutex = map_mutex; FakeDataSrcClientsBuilder::thread_waited = false; FakeDataSrcClientsBuilder::thread_throw_on_wait = NOTHROW; }