From: Marcin Siodelski Date: Fri, 31 Mar 2017 13:04:07 +0000 (+0200) Subject: Revert "[5078] Allow forwarding precedence for the registered CA commands." X-Git-Tag: trac5196_base~14^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=bcb0eac2bf2966bf13c6bf42facacf52e8bacd4e;p=thirdparty%2Fkea.git Revert "[5078] Allow forwarding precedence for the registered CA commands." This reverts commit b3358ca54694a6908df8856a9fcf12527aef3d3d. --- diff --git a/src/bin/agent/ca_command_mgr.cc b/src/bin/agent/ca_command_mgr.cc index e905ba8da2..449ec7fd50 100644 --- a/src/bin/agent/ca_command_mgr.cc +++ b/src/bin/agent/ca_command_mgr.cc @@ -34,48 +34,6 @@ 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 @@ -85,9 +43,11 @@ CtrlAgentCommandMgr::handleCommand(const std::string& cmd_name, ConstElementPtr answer; try { - // There are certain commands that have to be forwarded if the 'service' - // parameter is specified. - if (forward_first_commands_.count(cmd_name) > 0) { + // 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") { if (original_cmd && original_cmd->contains("service")) { ConstElementPtr services = original_cmd->get("service"); if (services && !services->empty()) { @@ -96,7 +56,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 command"); + "forwarding list-commands command"); } } } diff --git a/src/bin/agent/ca_command_mgr.h b/src/bin/agent/ca_command_mgr.h index e3f6f5fab1..28c504d419 100644 --- a/src/bin/agent/ca_command_mgr.h +++ b/src/bin/agent/ca_command_mgr.h @@ -12,7 +12,6 @@ #include #include #include -#include namespace isc { namespace agent { @@ -32,14 +31,6 @@ 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. @@ -60,59 +51,6 @@ 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 @@ -173,10 +111,6 @@ 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 8a4c828410..02ad66ac9d 100644 --- a/src/bin/agent/ca_controller.cc +++ b/src/bin/agent/ca_controller.cc @@ -50,28 +50,22 @@ CtrlAgentController::parseFile(const std::string& name) { void CtrlAgentController::registerCommands() { - CtrlAgentCommandMgr::instance().forwardOrHandle(BUILD_REPORT_COMMAND, - ForceForward(true), + CtrlAgentCommandMgr::instance().registerCommand(BUILD_REPORT_COMMAND, boost::bind(&DControllerBase::buildReportHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().forwardOrHandle(CONFIG_GET_COMMAND, - ForceForward(true), + CtrlAgentCommandMgr::instance().registerCommand(CONFIG_GET_COMMAND, boost::bind(&DControllerBase::configGetHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().forwardOrHandle(CONFIG_TEST_COMMAND, - ForceForward(true), + CtrlAgentCommandMgr::instance().registerCommand(CONFIG_TEST_COMMAND, boost::bind(&DControllerBase::configTestHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().forwardOrHandle(CONFIG_WRITE_COMMAND, - ForceForward(true), + CtrlAgentCommandMgr::instance().registerCommand(CONFIG_WRITE_COMMAND, boost::bind(&DControllerBase::configWriteHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().forwardOrHandle(SHUT_DOWN_COMMAND, - ForceForward(true), + CtrlAgentCommandMgr::instance().registerCommand(SHUT_DOWN_COMMAND, boost::bind(&DControllerBase::shutdownHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().forwardOrHandle(VERSION_GET_COMMAND, - ForceForward(true), + CtrlAgentCommandMgr::instance().registerCommand(VERSION_GET_COMMAND, boost::bind(&DControllerBase::versionGetHandler, this, _1, _2)); } diff --git a/src/bin/agent/ca_process.cc b/src/bin/agent/ca_process.cc index 3bf3e4a67b..65ac277dcc 100644 --- a/src/bin/agent/ca_process.cc +++ b/src/bin/agent/ca_process.cc @@ -55,6 +55,8 @@ 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 2e95ff18c9..2de61cb3a9 100644 --- a/src/bin/agent/tests/ca_command_mgr_unittests.cc +++ b/src/bin/agent/tests/ca_command_mgr_unittests.cc @@ -24,7 +24,6 @@ using namespace isc::agent; using namespace isc::asiolink; -using namespace isc::config; using namespace isc::data; using namespace isc::process; @@ -182,25 +181,6 @@ 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 @@ -330,67 +310,4 @@ 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 5336e8bf46..19bbae3248 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. - virtual void deregisterCommand(const std::string& cmd); + 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(); + void deregisterAll(); protected: