From: Marcin Siodelski Date: Thu, 30 Mar 2017 07:53:45 +0000 (+0200) Subject: [5078] Allow forwarding precedence for the registered CA commands. X-Git-Tag: trac5196_base~14^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b3358ca54694a6908df8856a9fcf12527aef3d3d;p=thirdparty%2Fkea.git [5078] Allow forwarding precedence for the registered CA commands. --- diff --git a/src/bin/agent/ca_command_mgr.cc b/src/bin/agent/ca_command_mgr.cc index 449ec7fd50..e905ba8da2 100644 --- a/src/bin/agent/ca_command_mgr.cc +++ b/src/bin/agent/ca_command_mgr.cc @@ -34,6 +34,48 @@ CtrlAgentCommandMgr::instance() { CtrlAgentCommandMgr::CtrlAgentCommandMgr() : HookedCommandMgr() { + // The base class automatically registers 'list-commands'. This + // command should be forwarded if the 'service' is specified, so + // we have to indicate it by adding it to the set of commands + // for which forwarding takes precedence. + forward_first_commands_.insert("list-commands"); +} + +void +CtrlAgentCommandMgr::forwardOrHandle(const std::string& cmd, + const ForceForward& force_forward, + CommandHandler handler) { + BaseCommandMgr::registerCommand(cmd, handler); + if (force_forward.forward_) { + forward_first_commands_.insert(cmd); + } +} + +void +CtrlAgentCommandMgr::forwardOrHandleExtended(const std::string& cmd, + const ForceForward& force_forward, + ExtendedCommandHandler handler) { + BaseCommandMgr::registerExtendedCommand(cmd, handler); + if (force_forward.forward_) { + forward_first_commands_.insert(cmd); + } +} + +void +CtrlAgentCommandMgr::deregisterCommand(const std::string& cmd) { + BaseCommandMgr::deregisterCommand(cmd); + forward_first_commands_.erase(cmd); +} + +void +CtrlAgentCommandMgr::deregisterAll() { + BaseCommandMgr::deregisterAll(); + forward_first_commands_.clear(); + // The base class automatically registers 'list-commands'. This + // command should be forwarded if the 'service' is specified, so + // we have to indicate it by adding it to the set of commands + // for which forwarding takes precedence. + forward_first_commands_.insert("list-commands"); } ConstElementPtr @@ -43,11 +85,9 @@ CtrlAgentCommandMgr::handleCommand(const std::string& cmd_name, ConstElementPtr answer; try { - // list-commands is a special case. The Control Agent always supports this - // command but most of the time users don't want to list commands supported - // by the CA but by one of the Kea servers. The user would indicate that - // by specifying 'service' value. - if (cmd_name == "list-commands") { + // There are certain commands that have to be forwarded if the 'service' + // parameter is specified. + if (forward_first_commands_.count(cmd_name) > 0) { if (original_cmd && original_cmd->contains("service")) { ConstElementPtr services = original_cmd->get("service"); if (services && !services->empty()) { @@ -56,7 +96,7 @@ CtrlAgentCommandMgr::handleCommand(const std::string& cmd_name, // cheat that Control Agent doesn't support this command to // avoid it being handled by CA. answer = createAnswer(CONTROL_RESULT_COMMAND_UNSUPPORTED, - "forwarding list-commands command"); + "forwarding command"); } } } diff --git a/src/bin/agent/ca_command_mgr.h b/src/bin/agent/ca_command_mgr.h index 28c504d419..e3f6f5fab1 100644 --- a/src/bin/agent/ca_command_mgr.h +++ b/src/bin/agent/ca_command_mgr.h @@ -12,6 +12,7 @@ #include #include #include +#include namespace isc { namespace agent { @@ -31,6 +32,14 @@ public: isc::Exception(file, line, what) { }; }; +struct ForceForward { + bool forward_; + + explicit ForceForward(const bool forward) + : forward_(forward) { + } +}; + /// @brief Command Manager for Control Agent. /// /// This is an implementation of the Command Manager within Control Agent. @@ -51,6 +60,59 @@ public: /// @brief Returns sole instance of the Command Manager. static CtrlAgentCommandMgr& instance(); + /// @brief Registers a command handler. + /// + /// This method is similar to the @ref BaseCommandMgr::registerCommand + /// in that it registers a new command along with a handler. However, + /// it also allows specifying a @ref ForceForward flag which indicates + /// if the command should be forwarded to a specified Kea server before + /// this command manager attempts to handle it on its own. The command + /// to be forwarded must include 'service' parameter which holds the + /// name of the service to which the command should be forwarded. If + /// the 'service' parameter is not specified the control agent will + /// try to handle it on its own. + /// + /// @param cmd Command name. + /// @param force_forward Indicates if the command should be forwarded + /// to the specified Kea server before it is handled by the Control + /// Agent. + /// @param handler Handler associated with the command. + void forwardOrHandle(const std::string& cmd, + const ForceForward& force_forward, + CommandHandler handler); + + /// @brief Registers a command handler. + /// + /// This method is similar to the @ref BaseCommandMgr::registerExtendedCommand + /// in that it registers a new command along with a handler. However, + /// it also allows specifying a @ref ForceForward flag which indicates + /// if the command should be forwarded to a specified Kea server before + /// this command manager attempts to handle it on its own. The command + /// to be forwarded must include 'service' parameter which holds the + /// name of the service to which the command should be forwarded. If + /// the 'service' parameter is not specified the control agent will + /// try to handle it on its own. + /// + /// @param cmd Command name. + /// @param force_forward Indicates if the command should be forwarded + /// to the specified Kea server before it is handled by the Control + /// Agent. + /// @param handler Handler associated with the command. + void forwardOrHandleExtended(const std::string& cmd, + const ForceForward& force_forward, + ExtendedCommandHandler handler); + + /// @brief Deregisters specified command handler. + /// + /// @param cmd Name of the command that's no longer handled. + virtual void deregisterCommand(const std::string& cmd); + + /// @brief Auxiliary method that removes all installed commands. + /// + /// The only unwipeable method is list-commands, which is internally + /// handled at all times. + virtual void deregisterAll(); + /// @brief Handles the command having a given name and arguments. /// /// This method extends the base implementation with the ability to forward @@ -111,6 +173,10 @@ private: /// @brief Buffer into which responses to forwarded commands are stored. std::array receive_buf_; + /// @brief A set of commands which are forwarded before CA handles them + /// if the 'service' parameter is specified. + std::set forward_first_commands_; + }; } // end of namespace isc::agent diff --git a/src/bin/agent/ca_controller.cc b/src/bin/agent/ca_controller.cc index 02ad66ac9d..8a4c828410 100644 --- a/src/bin/agent/ca_controller.cc +++ b/src/bin/agent/ca_controller.cc @@ -50,22 +50,28 @@ CtrlAgentController::parseFile(const std::string& name) { void CtrlAgentController::registerCommands() { - CtrlAgentCommandMgr::instance().registerCommand(BUILD_REPORT_COMMAND, + CtrlAgentCommandMgr::instance().forwardOrHandle(BUILD_REPORT_COMMAND, + ForceForward(true), boost::bind(&DControllerBase::buildReportHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().registerCommand(CONFIG_GET_COMMAND, + CtrlAgentCommandMgr::instance().forwardOrHandle(CONFIG_GET_COMMAND, + ForceForward(true), boost::bind(&DControllerBase::configGetHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().registerCommand(CONFIG_TEST_COMMAND, + CtrlAgentCommandMgr::instance().forwardOrHandle(CONFIG_TEST_COMMAND, + ForceForward(true), boost::bind(&DControllerBase::configTestHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().registerCommand(CONFIG_WRITE_COMMAND, + CtrlAgentCommandMgr::instance().forwardOrHandle(CONFIG_WRITE_COMMAND, + ForceForward(true), boost::bind(&DControllerBase::configWriteHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().registerCommand(SHUT_DOWN_COMMAND, + CtrlAgentCommandMgr::instance().forwardOrHandle(SHUT_DOWN_COMMAND, + ForceForward(true), boost::bind(&DControllerBase::shutdownHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().registerCommand(VERSION_GET_COMMAND, + CtrlAgentCommandMgr::instance().forwardOrHandle(VERSION_GET_COMMAND, + ForceForward(true), boost::bind(&DControllerBase::versionGetHandler, this, _1, _2)); } diff --git a/src/bin/agent/ca_process.cc b/src/bin/agent/ca_process.cc index 65ac277dcc..3bf3e4a67b 100644 --- a/src/bin/agent/ca_process.cc +++ b/src/bin/agent/ca_process.cc @@ -55,8 +55,6 @@ CtrlAgentProcess::run() { CtrlAgentController::instance()); controller->registerCommands(); - CtrlAgentCfgContextPtr ctx = - boost::dynamic_pointer_cast(base_ctx); // Let's process incoming data or expiring timers in a loop until // shutdown condition is detected. while (!shouldShutdown()) { diff --git a/src/bin/agent/tests/ca_command_mgr_unittests.cc b/src/bin/agent/tests/ca_command_mgr_unittests.cc index 2de61cb3a9..2e95ff18c9 100644 --- a/src/bin/agent/tests/ca_command_mgr_unittests.cc +++ b/src/bin/agent/tests/ca_command_mgr_unittests.cc @@ -24,6 +24,7 @@ using namespace isc::agent; using namespace isc::asiolink; +using namespace isc::config; using namespace isc::data; using namespace isc::process; @@ -181,6 +182,25 @@ public: return (command); } + /// @brief Registers stub command handler. + /// + /// @param command Command name. + /// @param force_forward Indicates if the command should rather be forwarded + /// (if the 'service' is specified) or the local handler should be invoked. + void registerStubHandler(const std::string& command, + const ForceForward& force_forward) { + CtrlAgentCommandMgr::instance().forwardOrHandle(command, force_forward, + boost::bind(&CtrlAgentCommandMgrTest::stubHandler, this, _1, _2)); + } + + /// @brief Stub command handler. + /// + /// @return Answer with status code 1001. + ConstElementPtr + stubHandler(const std::string&, const ConstElementPtr&) { + return (createAnswer(1001, "stub handler invoked")); + } + /// @brief Test forwarding the command. /// /// @param server_type Server for which the client socket should be @@ -310,4 +330,67 @@ TEST_F(CtrlAgentCommandMgrTest, forwardListCommands) { checkAnswer(answer, 3); } +// Check that the command for which forwarding takes precedence over +// handling locally is forwarded if the 'service' value is specified. +// Check that the command is handled locally if the 'service' is +// not specified or if it is specified during the command registration +// that the command should be handled locally before it is forwarded. +TEST_F(CtrlAgentCommandMgrTest, forwardPrecedence) { + + // Register stub handler and configure the manager to forward the + // command if the 'service' parameter is not specified. + registerStubHandler("foo", ForceForward(true)); + + // Configure client side socket. + configureControlSocket(CtrlAgentCfgContext::TYPE_DHCP6); + // Create server side socket. + bindServerSocket("{ \"result\" : 3 }"); + + // The client side communication is synchronous. To be able to respond + // to this we need to run the server side socket at the same time. + // Running IO service in a thread guarantees that the server responds + // as soon as it receives the control command. + isc::util::thread::Thread(boost::bind(&IOService::run, + getIOService().get())); + + ConstElementPtr command = createCommand("foo", "dhcp6"); + ConstElementPtr answer = mgr_.handleCommand("foo", ConstElementPtr(), + command); + + // Answer of 3 is specific to the stub response we send when the + // command is forwarded. So having this value returned means that + // the command was forwarded as expected. + checkAnswer(answer, 3); + + // Recreate the command but this time do not specify the 'service'. + command = createCommand("foo", ""); + + // Reset IO service so as we can run again. + getIOService().reset(); + isc::util::thread::Thread(boost::bind(&IOService::run, + getIOService().get())); + + // This time the command should not be forwarded but handled by the stub + // handler. We check that by expecting the status code of 1001. + answer = mgr_.handleCommand("foo", ConstElementPtr(), command); + checkAnswer(answer, 1001); + + // Deregister the handler and register it again with the flag indicating + // that the command should rather be handled locally. + CtrlAgentCommandMgr::instance().deregisterAll(); + registerStubHandler("foo", ForceForward(false)); + + // Recreate the command and specify 'service' value again. + command = createCommand("foo", "dhcp6"); + + // Reset IO service so as we can run again. + getIOService().reset(); + isc::util::thread::Thread(boost::bind(&IOService::run, + getIOService().get())); + + // The local handler should be ran this time again. + answer = mgr_.handleCommand("foo", ConstElementPtr(), command); + checkAnswer(answer, 1001); +} + } diff --git a/src/lib/config/base_command_mgr.h b/src/lib/config/base_command_mgr.h index 19bbae3248..5336e8bf46 100644 --- a/src/lib/config/base_command_mgr.h +++ b/src/lib/config/base_command_mgr.h @@ -135,13 +135,13 @@ public: /// @brief Deregisters specified command handler. /// /// @param cmd Name of the command that's no longer handled. - void deregisterCommand(const std::string& cmd); + virtual void deregisterCommand(const std::string& cmd); /// @brief Auxiliary method that removes all installed commands. /// /// The only unwipeable method is list-commands, which is internally /// handled at all times. - void deregisterAll(); + virtual void deregisterAll(); protected: