mgr,mgr/MgrClient: use fsid to signal mon-mgr vs cli MCommands

We can't use the feature bit for the MCommand connection to tell whether
it is a tell or CLI command because new clients may have to send CLI
commands via MCommand for old clusters, and they don't always know whether
this mgr is new or old yet.

Prior to octopus, MCommand contained a mon/mgr CLI command, and did not
have the fsid field set.  Start populating the fsid field, and use this
to signal whether a client is a new MgrClient that knows MCommand vs
MMgrCommand.  If we get an MCommand with the fsid set, that means it is
a tell command; otherwise, it's an old client sending a CLI command.

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

diff --git a/src/librados/RadosClient.cc b/src/librados/RadosClient.cc
index 2a441a9f1a1da31324b0e372a2da62152905eed5..372419816d8b34166d79185922aeac8ed79d6895 100644 (file)
--- a/src/librados/RadosClient.cc
+++ b/src/librados/RadosClient.cc
@@ -62,7 +62,7 @@ librados::RadosClient::RadosClient(CephContext *cct_)
     conf(cct_->_conf),
     state(DISCONNECTED),
     monclient(cct_),
-    mgrclient(cct_, nullptr),
+    mgrclient(cct_, nullptr, &monclient.monmap),
     messenger(NULL),
     instance_id(0),
     objecter(NULL),

diff --git a/src/mds/MDSDaemon.cc b/src/mds/MDSDaemon.cc
index 75d80d54e6bf1a135e678c837d9bfc10bcfd7535..5595a01a42eb52d965bbf55555512b59b105b058 100644 (file)
--- a/src/mds/MDSDaemon.cc
+++ b/src/mds/MDSDaemon.cc
@@ -71,7 +71,7 @@ MDSDaemon::MDSDaemon(std::string_view n, Messenger *m, MonClient *mc) :
   name(n),
   messenger(m),
   monc(mc),
-  mgrc(m->cct, m),
+  mgrc(m->cct, m, &mc->monmap),
   log_client(m->cct, messenger, &mc->monmap, LogClient::NO_FLAGS),
   mds_rank(NULL),
   asok_hook(NULL),

diff --git a/src/messages/MMonCommand.h b/src/messages/MMonCommand.h
index 8b4c1af9e43cd957241bac45bf30eeebde8d2bf8..583b9a89c93c2df65f957903cdf9384ccfd98164 100644 (file)
--- a/src/messages/MMonCommand.h
+++ b/src/messages/MMonCommand.h
@@ -22,6 +22,10 @@
 
 class MMonCommand : public PaxosServiceMessage {
 public:
+  // weird note: prior to octopus, MgrClient would leave fsid blank when
+  // sending commands to the mgr.  Starting with octopus, this is either
+  // populated with a valid fsid (tell command) or an MMgrCommand is sent
+  // instead.
   uuid_d fsid;
   std::vector<std::string> cmd;
 

diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc
index de894ad0b85f7c3b58786c6ce8ba9342e746f0ce..b5ad5e9323a0eee06183821be8808e60acc03632 100644 (file)
--- a/src/mgr/DaemonServer.cc
+++ b/src/mgr/DaemonServer.cc
@@ -791,7 +791,9 @@ public:
 bool DaemonServer::handle_command(const ref_t<MCommand>& m)
 {
   std::lock_guard l(lock);
-  if (HAVE_FEATURE(m->get_connection()->get_features(), SERVER_OCTOPUS)) {
+  // a blank fsid in MCommand signals a legacy client sending a "mon-mgr" CLI
+  // command.
+  if (m->fsid != uuid_d()) {
     cct->get_admin_socket()->queue_tell_command(m);
     return true;
   } else {

diff --git a/src/mgr/MgrClient.cc b/src/mgr/MgrClient.cc
index 87a301c72c39ca79fe604ae19033ae7fd202a268..94ff99c390fd9bc1d5b9bed9883872dc4bad83f4 100644 (file)
--- a/src/mgr/MgrClient.cc
+++ b/src/mgr/MgrClient.cc
@@ -15,6 +15,7 @@
 #include "MgrClient.h"
 
 #include "mgr/MgrContext.h"
+#include "mon/MonMap.h"
 
 #include "msg/Messenger.h"
 #include "messages/MMgrMap.h"
@@ -37,9 +38,12 @@ using ceph::bufferlist;
 #undef dout_prefix
 #define dout_prefix *_dout << "mgrc " << __func__ << " "
 
-MgrClient::MgrClient(CephContext *cct_, Messenger *msgr_)
-    : Dispatcher(cct_), cct(cct_), msgr(msgr_),
-      timer(cct_, lock)
+MgrClient::MgrClient(CephContext *cct_, Messenger *msgr_, MonMap *monmap_)
+  : Dispatcher(cct_),
+    cct(cct_),
+    msgr(msgr_),
+    monmap(monmap_),
+    timer(cct_, lock)
 {
   ceph_assert(cct != nullptr);
 }
@@ -472,7 +476,8 @@ int MgrClient::start_command(const vector<string>& cmd, const bufferlist& inbl,
   op.on_finish = onfinish;
 
   if (session && session->con) {
-    // Leaving fsid argument null because it isn't used.
+    // Leaving fsid argument null because it isn't used historically, and
+    // we can use it as a signal that we are sending a non-tell command.
     auto m = op.get_message(
       {},
       HAVE_FEATURE(map.active_mgr_features, SERVER_OCTOPUS));
@@ -508,10 +513,9 @@ int MgrClient::start_tell_command(
   op.on_finish = onfinish;
 
   if (session && session->con && (name.size() == 0 || map.active_name == name)) {
-    // Leaving fsid argument null because it isn't used.
-    // Note: this simply won't work we pre-octopus mgrs because they route
-    // MCommand to the cluster command handler.
-    auto m = op.get_message({}, false);
+    // Set fsid argument to signal that this is really a tell message (and
+    // we are not a legacy client sending a non-tell command via MCommand).
+    auto m = op.get_message(monmap->fsid, false);
     session->con->send_message2(std::move(m));
   } else {
     ldout(cct, 5) << "no mgr session (no running mgr daemon?), or "

diff --git a/src/mgr/MgrClient.h b/src/mgr/MgrClient.h
index 9d5251d9d206bd8dfcd20e771aa1fa13a6699b26..7e5272dbedbeebacbeb148fa53ebc6bea998ccd1 100644 (file)
--- a/src/mgr/MgrClient.h
+++ b/src/mgr/MgrClient.h
@@ -32,6 +32,7 @@ class MMgrClose;
 class Messenger;
 class MCommandReply;
 class MPGStats;
+class MonMap;
 
 class MgrSessionState
 {
@@ -59,6 +60,7 @@ protected:
   CephContext *cct;
   MgrMap map;
   Messenger *msgr;
+  MonMap *monmap;
 
   std::unique_ptr<MgrSessionState> session;
 
@@ -105,7 +107,7 @@ protected:
   bool mgr_optional = false;
 
 public:
-  MgrClient(CephContext *cct_, Messenger *msgr_);
+  MgrClient(CephContext *cct_, Messenger *msgr_, MonMap *monmap);
 
   void set_messenger(Messenger *msgr_) { msgr = msgr_; }
 

diff --git a/src/mgr/MgrStandby.cc b/src/mgr/MgrStandby.cc
index 465af9299ec825832f15ae8c22af289e55cad80f..7d193b6575b928aea7e201b0ac011786c17f55c0 100644 (file)
--- a/src/mgr/MgrStandby.cc
+++ b/src/mgr/MgrStandby.cc
@@ -48,7 +48,7 @@ MgrStandby::MgrStandby(int argc, const char **argv) :
                     0)),
   objecter{g_ceph_context, client_messenger.get(), &monc, NULL, 0, 0},
   client{client_messenger.get(), &monc, &objecter},
-  mgrc(g_ceph_context, client_messenger.get()),
+  mgrc(g_ceph_context, client_messenger.get(), &monc.monmap),
   log_client(g_ceph_context, client_messenger.get(), &monc.monmap, LogClient::NO_FLAGS),
   clog(log_client.create_channel(CLOG_CHANNEL_CLUSTER)),
   audit_clog(log_client.create_channel(CLOG_CHANNEL_AUDIT)),

diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index 796dcfba1b4534f8e39c78bb1e63afa952aff20f..42c977891b886e764d6120cc23f964f040a426ae 100644 (file)
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -146,7 +146,7 @@ Monitor::Monitor(CephContext* cct_, string nm, MonitorDBStore *s,
                        cct->_conf->auth_supported.empty() ?
                        cct->_conf->auth_service_required : cct->_conf->auth_supported),
   mgr_messenger(mgr_m),
-  mgr_client(cct_, mgr_m),
+  mgr_client(cct_, mgr_m, monmap),
   gss_ktfile_client(cct->_conf.get_val<std::string>("gss_ktab_client_file")),
   store(s),
   

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index ab63822f0729c9f993df07bbbaa82ef14022edc9..3d7fb104177bca415ba83e4dc4eac7d08d80f0ff 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -2117,7 +2117,7 @@ OSD::OSD(CephContext *cct_, ObjectStore *store_,
   client_messenger(external_messenger),
   objecter_messenger(osdc_messenger),
   monc(mc),
-  mgrc(cct_, client_messenger),
+  mgrc(cct_, client_messenger, &mc->monmap),
   logger(NULL),
   recoverystate_perf(NULL),
   store(store_),