[1596] Some review addressing

src/bin/auth/auth_log.h

@@ -29,6 +29,8 @@ namespace auth {
 
 // Debug messages indicating normal startup are logged at this debug level.
 const int DBG_AUTH_START = DBGLVL_START_SHUT;
+// Debug messages upon shutdown
+const int DBG_AUTH_SHUT = DBGLVL_START_SHUT;
 
 // Debug level used to log setting information (such as configuration changes).
 const int DBG_AUTH_OPS = DBGLVL_COMMAND;

src/bin/auth/auth_messages.mes

@@ -192,6 +192,10 @@ reason for the failure is included in the message.
 Initialization of the authoritative server has completed successfully
 and it is entering the main loop, waiting for queries to arrive.
 
+% AUTH_SHUTDOWN asked to stop, doing so
+This is a debug message indicating the server was asked to shut down and it is
+complying to the request.
+
 % AUTH_SQLITE3 nothing to do for loading sqlite3
 This is a debug message indicating that the authoritative server has
 found that the data source it is loading is an SQLite3 data source,

src/bin/auth/command.cc

@@ -16,15 +16,11 @@
 #include <auth/auth_log.h>
 #include <auth/auth_srv.h>
 
-#include <exceptions/exceptions.h>
-
-#include <dns/rrclass.h>
-
 #include <cc/data.h>
-
 #include <datasrc/memory_datasrc.h>
-
 #include <config/ccsession.h>
+#include <exceptions/exceptions.h>
+#include <dns/rrclass.h>
 
 #include <string>
 
@@ -113,16 +109,24 @@ class ShutdownCommand : public AuthCommand {
 public:
     virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) {
         // Is the pid argument provided?
-        if (args && args->getType() ==
+        if (args && args->getType() == isc::data::Element::map &&
-            isc::data::Element::map && args->contains("pid")) {
+            args->contains("pid")) {
             // If it is, we check it is the same as our PID
-            const int pid(args->get("pid")->intValue());
+            if (args->get("pid")->getType() == isc::data::Element::integer) {
-            const pid_t my_pid(getpid());
+                const int pid(args->get("pid")->intValue());
-            if (my_pid != pid) {
+                const pid_t my_pid(getpid());
-                // It is not for us
+                if (my_pid != pid) {
-                return;
+                    // It is not for us
+                    //
+                    // Note that this is completely expected situation, if
+                    // there are multiple instances of the server running and
+                    // another instance is being shut down, we get the message
+                    // too, due to the multicast nature of our message bus.
+                    return;
+                }
             }
         }
+        LOG_DEBUG(auth_logger, DBG_AUTH_SHUT, AUTH_SHUTDOWN);
         server.stop();
     }
 };

src/bin/auth/tests/command_unittest.cc

@@ -69,7 +69,7 @@ protected:
     AuthSrv server;
     ConstElementPtr result;
     // The shutdown command parameter
-    ConstElementPtr param;
+    ConstElementPtr param_;
     int rcode;
     isc::asiolink::IntervalTimer itimer_;
 public:
@@ -103,7 +103,7 @@ TEST_F(AuthCommandTest, sendStatistics) {
 
 void
 AuthCommandTest::stopServer() {
-    result = execAuthServerCommand(server, "shutdown", param);
+    result = execAuthServerCommand(server, "shutdown", param_);
     parseAnswer(rcode, result);
     assert(rcode == 0); // make sure the test stops when something is wrong
 }
@@ -120,7 +120,21 @@ TEST_F(AuthCommandTest, shutdownCorrectPID) {
     const pid_t pid(getpid());
     ElementPtr param(new isc::data::MapElement());
     param->set("pid", ConstElementPtr(new isc::data::IntElement(pid)));
-    this->param = param;
+    param_ = param;
+    // With the correct PID, it should act exactly the same as in case
+    // of no parameter
+    itimer_.setup(boost::bind(&AuthCommandTest::stopServer, this), 1);
+    server.getIOService().run();
+    EXPECT_EQ(0, rcode);
+}
+
+// If we provide something not an int, the PID is not really specified, so
+// act as if nothing came.
+TEST_F(AuthCommandTest, shutdownNotInt) {
+    // Put the pid parameter there
+    ElementPtr param(new isc::data::MapElement());
+    param->set("pid", ConstElementPtr(new isc::data::StringElement("pid")));
+    param_ = param;
     // With the correct PID, it should act exactly the same as in case
     // of no parameter
     itimer_.setup(boost::bind(&AuthCommandTest::stopServer, this), 1);
@@ -132,14 +146,14 @@ TEST_F(AuthCommandTest, shutdownCorrectPID) {
 // command, it should be running
 void
 AuthCommandTest::dontStopServer() {
-    result = execAuthServerCommand(server, "shutdown", param);
+    result = execAuthServerCommand(server, "shutdown", param_);
     parseAnswer(rcode, result);
     EXPECT_EQ(0, rcode);
     rcode = -1;
     // We run the stopServer now, to really stop the server.
     // If it had stopped already, it won't be run and the rcode -1 will
     // be left here.
-    param = ConstElementPtr();
+    param_ = ConstElementPtr();
     itimer_.cancel();
     itimer_.setup(boost::bind(&AuthCommandTest::stopServer, this), 1);
 }
@@ -147,7 +161,7 @@ AuthCommandTest::dontStopServer() {
 TEST_F(AuthCommandTest, shutdownIncorrectPID) {
     // The PID = 0 should be taken by init, so we are not init and the
     // PID should be different
-    param = Element::fromJSON("{\"pid\": 0}");
+    param_ = Element::fromJSON("{\"pid\": 0}");
     itimer_.setup(boost::bind(&AuthCommandTest::dontStopServer, this), 1);
     server.getIOService().run();
     EXPECT_EQ(0, rcode);

src/bin/resolver/main.cc

@@ -87,16 +87,24 @@ my_command_handler(const string& command, ConstElementPtr args) {
         answer = createAnswer(0, args);
     } else if (command == "shutdown") {
         // Is the pid argument provided?
-        if (args && args->getType() ==
+        if (args && args->getType() == isc::data::Element::map &&
-            isc::data::Element::map && args->contains("pid")) {
+            args->contains("pid")) {
             // If it is, we check it is the same as our PID
-            const int pid(args->get("pid")->intValue());
+            if (args->get("pid")->getType() == isc::data::Element::integer) {
-            const pid_t my_pid(getpid());
+                const int pid(args->get("pid")->intValue());
-            if (my_pid != pid) {
+                const pid_t my_pid(getpid());
-                // It is not for us
+                if (my_pid != pid) {
-                return (answer);
+                    // It is not for us
+                    //
+                    // Note that this is completely expected situation, if
+                    // there are multiple instances of the server running and
+                    // another instance is being shut down, we get the message
+                    // too, due to the multicast nature of our message bus.
+                    return answer;
+                }
             }
         }
+        LOG_DEBUG(resolver_logger, RESOLVER_DBG_INIT, RESOLVER_SHUTDOWN);
         io_service.stop();
     }
 

src/bin/resolver/resolver_messages.mes

@@ -246,3 +246,7 @@ RESOLVER_QUERY_REJECTED case, the server does not return any response.
 The log message shows the query in the form of <query name>/<query
 type>/<query class>, and the client that sends the query in the form of
 <Source IP address>#<source port>.
+
+% RESOLVER_SHUTDOWN asked to shut down, doing so
+A debug message noting that the server was asked to terminate and is
+complying to the request.