From 3db17a8c11f7a49e9380a3db947eb145864c6d24 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Fri, 28 Dec 2018 21:59:11 +0100 Subject: [PATCH] [30-implement-control-socket-for-ddns-2] Finished unit tests --- src/bin/d2/d2_controller.cc | 3 --- src/bin/d2/d2_process.cc | 36 +++++++++++-------------- src/bin/d2/d2_process.h | 18 +++++-------- src/bin/d2/tests/d2_command_unittest.cc | 6 +++-- 4 files changed, 25 insertions(+), 38 deletions(-) diff --git a/src/bin/d2/d2_controller.cc b/src/bin/d2/d2_controller.cc index 6d6a788f21..7e3bb084ae 100644 --- a/src/bin/d2/d2_controller.cc +++ b/src/bin/d2/d2_controller.cc @@ -50,9 +50,6 @@ D2Controller::D2Controller() void D2Controller::registerCommands() { - // CommandMgr uses IO service to run asynchronous socket operations. - CommandMgr::instance().setIOService(getIOService()); - // These are the commands always supported by the D2 server. // Please keep the list in alphabetic order. CommandMgr::instance().registerCommand(BUILD_REPORT_COMMAND, diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc index c911c0c372..7baaffe8f4 100644 --- a/src/bin/d2/d2_process.cc +++ b/src/bin/d2/d2_process.cc @@ -24,8 +24,7 @@ const unsigned int D2Process::QUEUE_RESTART_PERCENT = 80; D2Process::D2Process(const char* name, const asiolink::IOServicePtr& io_service) : DProcessBase(name, io_service, DCfgMgrBasePtr(new D2CfgMgr())), - reconf_queue_flag_(false), reconf_control_socket_flag_(false), - shutdown_type_(SD_NORMAL) { + reconf_queue_flag_(false), shutdown_type_(SD_NORMAL) { // Instantiate queue manager. Note that queue manager does not start // listening at this point. That can only occur after configuration has @@ -43,6 +42,8 @@ D2Process::D2Process(const char* name, const asiolink::IOServicePtr& io_service) void D2Process::init() { + // CommandMgr uses IO service to run asynchronous socket operations. + isc::config::CommandMgr::instance().setIOService(getIoService()); }; void @@ -56,11 +57,6 @@ D2Process::run() { // Loop forever until we are allowed to shutdown. while (!canShutdown()) { - // Check if the command channel should be (re-)configured. - if (getReconfControlSocketFlag()) { - reconfigureCommandChannel(); - } - // Check on the state of the request queue. Take any // actions necessary regarding it. checkQueueStatus(); @@ -215,7 +211,8 @@ D2Process::configure(isc::data::ConstElementPtr config_set, bool check_only) { .arg(config_set->str()); isc::data::ConstElementPtr answer; - answer = getCfgMgr()->simpleParseConfig(config_set, check_only); + answer = getCfgMgr()->simpleParseConfig(config_set, check_only, + boost::bind(&D2Process::reconfigureCommandChannel, this)); if (check_only) { return (answer); } @@ -229,7 +226,6 @@ D2Process::configure(isc::data::ConstElementPtr config_set, bool check_only) { // action. In integrated mode, this will send a failed response back // to the configuration backend. reconf_queue_flag_ = false; - reconf_control_socket_flag_ = false; return (answer); } @@ -245,7 +241,6 @@ D2Process::configure(isc::data::ConstElementPtr config_set, bool check_only) { // things that need reconfiguration. It might also be useful if we // did some analysis to decide what if anything we need to do.) reconf_queue_flag_ = true; - reconf_control_socket_flag_ = true; // If we are here, configuration was valid, at least it parsed correctly // and therefore contained no invalid values. @@ -413,36 +408,35 @@ const char* D2Process::getShutdownTypeStr(const ShutdownType& type) { void D2Process::reconfigureCommandChannel() { - reconf_control_socket_flag_ = false; - - // Current socket configuration. - static isc::data::ConstElementPtr current_sock_cfg; - // Get new socket configuration. isc::data::ConstElementPtr sock_cfg = getD2CfgMgr()->getControlSocketInfo(); // Determine if the socket configuration has changed. It has if // both old and new configuration is specified but respective // data elements aren't equal. - bool sock_changed = (sock_cfg && current_sock_cfg && - !sock_cfg->equals(*current_sock_cfg)); + bool sock_changed = (sock_cfg && current_control_socket_ && + !sock_cfg->equals(*current_control_socket_)); // If the previous or new socket configuration doesn't exist or // the new configuration differs from the old configuration we // close the existing socket and open a new socket as appropriate. // Note that closing an existing socket means the client will not // receive the configuration result. - if (!sock_cfg || !current_sock_cfg || sock_changed) { - // Close the existing socket (if any). - isc::config::CommandMgr::instance().closeCommandSocket(); + if (!sock_cfg || !current_control_socket_ || sock_changed) { + // Close the existing socket. + if (current_control_socket_) { + isc::config::CommandMgr::instance().closeCommandSocket(); + current_control_socket_.reset(); + } + // Open the new socket. if (sock_cfg) { isc::config::CommandMgr::instance().openCommandSocket(sock_cfg); } } // Commit the new socket configuration. - current_sock_cfg = sock_cfg; + current_control_socket_ = sock_cfg; } }; // namespace isc::d2 diff --git a/src/bin/d2/d2_process.h b/src/bin/d2/d2_process.h index b20ff11ded..d453665195 100644 --- a/src/bin/d2/d2_process.h +++ b/src/bin/d2/d2_process.h @@ -67,10 +67,8 @@ public: /// PRIOR to configuration reception. The base class provides this method /// as a place to perform any derivation-specific initialization steps /// that are inappropriate for the constructor but necessary prior to - /// launch. So far, no such steps have been identified for D2, so its - /// implementation is empty but required. - /// - /// @throw DProcessBaseError if the initialization fails. + /// configure. + /// For D2 it is used to initialize the command manager. virtual void init(); /// @brief Implements the process's event loop. @@ -284,11 +282,6 @@ public: return (reconf_queue_flag_); } - /// @brief Returns true if the control socket should be reconfigured. - bool getReconfControlSocketFlag() const { - return (reconf_control_socket_flag_); - } - /// @brief Returns the type of shutdown requested. /// /// Note, this value is meaningless unless shouldShutdown() returns true. @@ -314,11 +307,12 @@ private: /// @brief Indicates if the queue manager should be reconfigured. bool reconf_queue_flag_; - /// @brief Indicates if the control socket should be reconfigured. - bool reconf_control_socket_flag_; - /// @brief Indicates the type of shutdown requested. ShutdownType shutdown_type_; + + /// @brief Current socket control configuration. + isc::data::ConstElementPtr current_control_socket_; + }; /// @brief Defines a shared pointer to D2Process. diff --git a/src/bin/d2/tests/d2_command_unittest.cc b/src/bin/d2/tests/d2_command_unittest.cc index 38aa669503..4c7675516b 100644 --- a/src/bin/d2/tests/d2_command_unittest.cc +++ b/src/bin/d2/tests/d2_command_unittest.cc @@ -51,7 +51,7 @@ public: return (getController()); } - virtual ~NakedD2Controller() { } + virtual ~NakedD2Controller() { deregisterCommands(); } using DControllerBase::getIOService; using DControllerBase::initProcess; @@ -129,7 +129,7 @@ public: /// @brief Destructor. ~CtrlChannelD2Test() { - // Include deregister & co. + // Deregister & co. server_.reset(); // Remove files. @@ -202,6 +202,7 @@ public: ASSERT_TRUE(proc); ConstElementPtr answer = proc->configure(config, false); ASSERT_TRUE(answer); + ASSERT_NO_THROW(d2Controller()->registerCommands()); int status = 0; ConstElementPtr txt = parseAnswer(status, answer); @@ -686,6 +687,7 @@ TEST_F(CtrlChannelD2Test, configTest) { ASSERT_TRUE(answer); EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration applied successfully.\" }", answer->str()); + ASSERT_NO_THROW(d2Controller()->registerCommands()); // Check that the config was indeed applied. D2CfgMgrPtr cfg_mgr = proc->getD2CfgMgr(); -- 2.47.2