Revert "[5078] Allow forwarding precedence for the registered CA commands."

This reverts commit b3358ca54694a6908df8856a9fcf12527aef3d3d.

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");
                 }
             }
         }

src/bin/agent/ca_command_mgr.h

@@ -12,7 +12,6 @@
 #include <boost/noncopyable.hpp>
 #include <boost/shared_ptr.hpp>
 #include <array>
-#include <set>
 
 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<char, 8192> receive_buf_;
 
-    /// @brief A set of commands which are forwarded before CA handles them
-    /// if the 'service' parameter is specified.
-    std::set<std::string> forward_first_commands_;
-
 };
 
 } // end of namespace isc::agent

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));
 }
 

src/bin/agent/ca_process.cc

@@ -55,6 +55,8 @@ CtrlAgentProcess::run() {
                 CtrlAgentController::instance());
         controller->registerCommands();
 
+        CtrlAgentCfgContextPtr ctx =
+            boost::dynamic_pointer_cast<CtrlAgentCfgContext>(base_ctx);
         // Let's process incoming data or expiring timers in a loop until
         // shutdown condition is detected.
         while (!shouldShutdown()) {

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);
-}
-
 }

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: