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

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

src/bin/agent/ca_command_mgr.h

@@ -12,6 +12,7 @@
 #include <boost/noncopyable.hpp>
 #include <boost/shared_ptr.hpp>
 #include <array>
+#include <set>
 
 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<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,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));
 }
 

src/bin/agent/ca_process.cc

@@ -55,8 +55,6 @@ 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,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);
+}
+
 }

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: