[#1115] Addressed review comments

src/bin/agent/tests/ca_controller_unittests.cc
   TEST_F(CtrlAgentControllerTest, shutdown) - new test

src/lib/process/daemon.h
    Daemon::shutdown() restored to virtual

Other Minor cleanups

diff --git a/doc/sphinx/arm/ctrl-channel.rst b/doc/sphinx/arm/ctrl-channel.rst
index 78b0f0d7254ebd4f3664e53b4190e242e4c76232..1e8f4298fa64fdcf570cdcdb9454629eb5d0a036 100644 (file)
--- a/doc/sphinx/arm/ctrl-channel.rst
+++ b/doc/sphinx/arm/ctrl-channel.rst
@@ -503,8 +503,8 @@ may look like this:
 
 The server responds with a confirmation that the shutdown procedure has
 been initiated.  The optional parameter, "exit-value", specifies the
-numeric value with which the server process will exit to the OS.  The
-default value is zero.
+numeric value with which the server process will exit to the system.
+The default value is zero.
 
 .. _command-dhcp-disable:
 

diff --git a/src/bin/agent/tests/ca_controller_unittests.cc b/src/bin/agent/tests/ca_controller_unittests.cc
index 0c084daf9a8858da6e9b3ed17f2b737568bc49e1..984feab7fbe3b9e2f6336a14a137dbd5986daba4 100644 (file)
--- a/src/bin/agent/tests/ca_controller_unittests.cc
+++ b/src/bin/agent/tests/ca_controller_unittests.cc
@@ -697,6 +697,59 @@ TEST_F(CtrlAgentControllerTest, statusGet) {
     EXPECT_GE(found_reload->intValue(), 0);
 }
 
+TEST_F(CtrlAgentControllerTest, shutdown) {
+    ASSERT_NO_THROW(initProcess());
+    EXPECT_TRUE(checkProcess());
+
+    // The framework available makes it very difficult to test the actual
+    // code as CtrlAgentController is not initialized the same way it is
+    // in production code. In particular, the way CtrlAgentController
+    // is initialized in tests does not call registerCommands().
+    // This is a crude workaround for this problem. Proper solution should
+    // be developed sooner rather than later.
+    const DControllerBasePtr& base = getController();
+    const CtrlAgentControllerPtr& ctrl
+        = boost::dynamic_pointer_cast<CtrlAgentController>(base);
+    ASSERT_TRUE(ctrl);
+    // Now clean up after ourselves.
+    ctrl->registerCommands();
+
+    // This is normally set to whatever value is passed to -c when the server is
+    // started, but we're not starting it that way, so need to set it by hand.
+    getController()->setConfigFile("testvalid.json");
+
+    // Ok, enough fooling around. Let's create a valid config.
+    ofstream f("testvalid.json", ios::trunc);
+    f << "{ \"Control-agent\": "
+      << string(valid_agent_config)
+      << " }" << endl;
+    f.close();
+
+    // Build and execute the command.
+
+    ConstElementPtr cmd = Element::fromJSON("{ \"command\": \"shutdown\"}");
+    ConstElementPtr params;
+    ConstElementPtr answer;
+    answer = CtrlAgentCommandMgr::instance().handleCommand("shutdown",
+                                                           params, cmd);
+
+    // Verify the reload was successful.
+    string expected = "[ { \"result\": 0, \"text\": "
+                      "\"Control Agent is shutting down\" } ]";
+
+    EXPECT_EQ(expected, answer->str());
+
+    int exit_value = ctrl->getExitValue();
+    EXPECT_EQ(0, exit_value);
+
+    // Remove the file.
+    ::remove("testvalid.json");
+
+    // Now clean up after ourselves.
+    ctrl->deregisterCommands();
+}
+
+
 TEST_F(CtrlAgentControllerTest, shutdownExitValue) {
     ASSERT_NO_THROW(initProcess());
     EXPECT_TRUE(checkProcess());

diff --git a/src/bin/d2/tests/d2_command_unittest.cc b/src/bin/d2/tests/d2_command_unittest.cc
index e0eceafacbd3569f1cd9dd89ff5182905726f455..cfa1a8130030ae49b5468903f0b09ee039c2de6b 100644 (file)
--- a/src/bin/d2/tests/d2_command_unittest.cc
+++ b/src/bin/d2/tests/d2_command_unittest.cc
@@ -583,8 +583,8 @@ TEST_F(CtrlChannelD2Test, shutdownExitValue) {
     string response;
 
     sendUnixCommand("{ \"command\": \"shutdown\", "
-                    "\"arguments\": { \"exit-value\": 77 }}"
-                    , response);
+                    "\"arguments\": { \"exit-value\": 77 }}",
+                    response);
 
     EXPECT_EQ("{ \"result\": 0, \"text\": \"Shutdown initiated, type is: normal\" }",
               response);

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index 8e00344e7ae539666f032adf97b9681635639ff5..b3f1fa76ce599aa40cd10ca3dfe18c85c25e77ef 100644 (file)
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
@@ -227,7 +227,7 @@ ControlledDhcpv4Srv::commandShutdownHandler(const string&, ConstElementPtr args)
     }
 
     ControlledDhcpv4Srv::getInstance()->shutdown(exit_value);
-    return(createAnswer(CONTROL_RESULT_SUCCESS, "Shutting down."));
+    return (createAnswer(CONTROL_RESULT_SUCCESS, "Shutting down."));
 }
 
 ConstElementPtr

diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc
index bbd5a3c717edb372f89da2d8065a7d82cf6a95d7..4977409aa42e1af1baaa8c83595d13a327d061a0 100644 (file)
--- a/src/lib/process/d_controller.cc
+++ b/src/lib/process/d_controller.cc
@@ -69,7 +69,7 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) {
 
     if (isCheckOnly()) {
         checkConfigOnly();
-        return(EXIT_SUCCESS);
+        return (EXIT_SUCCESS);
     }
 
     // It is important that we set a default logger name because this name
@@ -155,7 +155,7 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) {
     LOG_INFO(dctl_logger, DCTL_SHUTDOWN)
         .arg(app_name_).arg(getpid()).arg(VERSION);
 
-    return(getExitValue());
+    return (getExitValue());
 }
 
 void

diff --git a/src/lib/process/d_controller.h b/src/lib/process/d_controller.h
index 072963890ae96b8d883499808fa774ff1aac6ff2..fa8de6f9220dbce6dc0c04d13e97e2dae78bc0a1 100644 (file)
--- a/src/lib/process/d_controller.h
+++ b/src/lib/process/d_controller.h
@@ -149,6 +149,7 @@ public:
     /// process object.
     /// ProcessRunError - A fatal error occurred while in the application
     /// process event loop.
+    /// @return The value from @c Daemon::getExitValue().
     virtual int launch(int argc, char* argv[], const bool test_mode);
 
     /// @brief Instance method invoked by the configuration event handler and

diff --git a/src/lib/process/daemon.h b/src/lib/process/daemon.h
index 0585c1b881b681d8a6718a28e734cec3edf6f2a6..9cb8e56265bbd201d9ddf3c8ac14f5a838c15716 100644 (file)
--- a/src/lib/process/daemon.h
+++ b/src/lib/process/daemon.h
@@ -73,7 +73,7 @@ public:
     virtual void cleanup();
 
     /// @brief Initiates shutdown procedure for the whole server.
-    void shutdown();
+    virtual void shutdown();
 
     /// @brief Initializes logger
     ///