mgr/MgrClient: close mgr session on daemon MgrClient shutdown

If we are a service daemon, send a MMgrClose on shutdown to clear ourselves
out of the ServiceMap.

Note that this is a best-effort attempt; we don't block shutdown if the
mgr isn't currently available.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/common/options.cc b/src/common/options.cc
index fb6914d0dda679f3ede491af561ccdb15c1d0ac4..17a03f228ee78efc4f3c52165f513233ca6b2fa1 100644 (file)
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -4803,6 +4803,10 @@ std::vector<Option> get_global_options() {
     .set_description("Period in seconds from last beacon to manager dropping "
                      "state about a monitored service (RGW, rbd-mirror etc)"),
 
+    Option("mgr_client_service_daemon_unregister_timeout", Option::TYPE_FLOAT, Option::LEVEL_DEV)
+    .set_default(1.0)
+    .set_description("Time to wait during shutdown to deregister service with mgr"),
+
     Option("mon_mgr_digest_period", Option::TYPE_INT, Option::LEVEL_DEV)
     .set_default(5)
     .add_service("mon")

diff --git a/src/mgr/MgrClient.cc b/src/mgr/MgrClient.cc
index 5fadf1fe72295b88f6130383f83bf09dafa8d247..65617a521ffa27594f84445d70ed6a60eaa9be15 100644 (file)
--- a/src/mgr/MgrClient.cc
+++ b/src/mgr/MgrClient.cc
@@ -20,6 +20,7 @@
 #include "messages/MMgrMap.h"
 #include "messages/MMgrReport.h"
 #include "messages/MMgrOpen.h"
+#include "messages/MMgrClose.h"
 #include "messages/MMgrConfigure.h"
 #include "messages/MCommand.h"
 #include "messages/MCommandReply.h"
@@ -48,6 +49,7 @@ void MgrClient::init()
 void MgrClient::shutdown()
 {
   Mutex::Locker l(lock);
+  ldout(cct, 10) << dendl;
 
   if (connect_retry_callback) {
     timer.cancel_event(connect_retry_callback);
@@ -57,6 +59,20 @@ void MgrClient::shutdown()
   // forget about in-flight commands if we are prematurely shut down
   // (e.g., by control-C)
   command_table.clear();
+  if (service_daemon &&
+      session &&
+      session->con &&
+      HAVE_FEATURE(session->con->get_features(), SERVER_MIMIC)) {
+    ldout(cct, 10) << "closing mgr session" << dendl;
+    MMgrClose *m = new MMgrClose();
+    m->daemon_name = daemon_name;
+    m->service_name = service_name;
+    session->con->send_message(m);
+    utime_t timeout;
+    timeout.set_from_double(cct->_conf->get_val<double>(
+                             "mgr_client_service_daemon_unregister_timeout"));
+    shutdown_cond.WaitInterval(lock, timeout);
+  }
 
   timer.shutdown();
   if (session) {
@@ -74,6 +90,8 @@ bool MgrClient::ms_dispatch(Message *m)
     return handle_mgr_map(static_cast<MMgrMap*>(m));
   case MSG_MGR_CONFIGURE:
     return handle_mgr_configure(static_cast<MMgrConfigure*>(m));
+  case MSG_MGR_CLOSE:
+    return handle_mgr_close(static_cast<MMgrClose*>(m));
   case MSG_COMMAND_REPLY:
     if (m->get_source().type() == CEPH_ENTITY_TYPE_MGR) {
       handle_command_reply(static_cast<MCommandReply*>(m));
@@ -379,6 +397,13 @@ bool MgrClient::handle_mgr_configure(MMgrConfigure *m)
   return true;
 }
 
+bool MgrClient::handle_mgr_close(MMgrClose *m)
+{
+  service_daemon = false;
+  shutdown_cond.Signal();
+  return true;
+}
+
 int MgrClient::start_command(const vector<string>& cmd, const bufferlist& inbl,
                   bufferlist *outbl, string *outs,
                   Context *onfinish)

diff --git a/src/mgr/MgrClient.h b/src/mgr/MgrClient.h
index 2a502ecc57e6c2db730eebcb3f4f86a9ed6782c3..4a044b5d5e3c98b9f5482c3a8f1b8cbbe2aa8e87 100644 (file)
--- a/src/mgr/MgrClient.h
+++ b/src/mgr/MgrClient.h
@@ -25,6 +25,7 @@
 
 class MMgrMap;
 class MMgrConfigure;
+class MMgrClose;
 class Messenger;
 class MCommandReply;
 class MPGStats;
@@ -57,6 +58,7 @@ protected:
   unique_ptr<MgrSessionState> session;
 
   Mutex lock = {"MgrClient::lock"};
+  Cond shutdown_cond;
 
   uint32_t stats_period = 0;
   uint32_t stats_threshold = 0;
@@ -101,6 +103,7 @@ public:
 
   bool handle_mgr_map(MMgrMap *m);
   bool handle_mgr_configure(MMgrConfigure *m);
+  bool handle_mgr_close(MMgrClose *m);
   bool handle_command_reply(MCommandReply *m);
 
   void send_pgstats();

diff --git a/src/test/librados/service.cc b/src/test/librados/service.cc
index 7c3e56aeb75b2b49e66d82c41dd5ee1328c286e2..afb4ee23d8cea58a5750635d28754de07c9b0ddf 100644 (file)
--- a/src/test/librados/service.cc
+++ b/src/test/librados/service.cc
@@ -5,6 +5,7 @@
 #include "include/stringify.h"
 
 #include <algorithm>
+#include <thread>
 #include <errno.h>
 #include "gtest/gtest.h"
 #include "test/unit.cc"
@@ -114,3 +115,47 @@ TEST(LibRadosServicePP, Status) {
   }
   cluster.shutdown();
 }
+
+TEST(LibRadosServicePP, Close) {
+  int tries = 20;
+  string name = string("close-test-pid") + stringify(getpid());
+  int i;
+  for (i = 0; i < tries; ++i) {
+    cout << "attempt " << i << " of " << tries << std::endl;
+    {
+      Rados cluster;
+      cluster.init("admin");
+      ASSERT_EQ(0, cluster.conf_read_file(NULL));
+      cluster.conf_parse_env(NULL);
+      ASSERT_EQ(0, cluster.connect());
+      ASSERT_EQ(0, cluster.service_daemon_register(
+                 "laundry", name, {{"foo", "bar"}, {"this", "that"}}));
+      sleep(3); // let it register
+      cluster.shutdown();
+    }
+    // mgr updates servicemap every tick
+    //sleep(g_conf->get_val<int64_t>("mgr_tick_period"));
+    std::this_thread::sleep_for(g_conf->get_val<std::chrono::seconds>(
+                                 "mgr_tick_period"));
+    // make sure we are deregistered
+    {
+      Rados cluster;
+      cluster.init("admin");
+      ASSERT_EQ(0, cluster.conf_read_file(NULL));
+      cluster.conf_parse_env(NULL);
+      ASSERT_EQ(0, cluster.connect());
+      bufferlist inbl, outbl;
+      ASSERT_EQ(0, cluster.mon_command("{\"prefix\": \"service dump\"}",
+                                      inbl, &outbl, NULL));
+      string s = outbl.to_str();
+      cluster.shutdown();
+
+      if (s.find(name) != string::npos) {
+       cout << " failed to deregister:\n" << s << std::endl;
+      } else {
+       break;
+      }
+    }
+  }
+  ASSERT_LT(i, tries);
+}