From 6f35d2835268eade059535b62378d6d407ef9e87 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 27 Sep 2019 16:01:44 -0500 Subject: [PATCH] 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 --- src/librados/RadosClient.cc | 2 +- src/mds/MDSDaemon.cc | 2 +- src/messages/MMonCommand.h | 4 ++++ src/mgr/DaemonServer.cc | 4 +++- src/mgr/MgrClient.cc | 20 ++++++++++++-------- src/mgr/MgrClient.h | 4 +++- src/mgr/MgrStandby.cc | 2 +- src/mon/Monitor.cc | 2 +- src/osd/OSD.cc | 2 +- 9 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/librados/RadosClient.cc b/src/librados/RadosClient.cc index 2a441a9f1a1d..372419816d8b 100644 --- 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 75d80d54e6bf..5595a01a42eb 100644 --- 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 8b4c1af9e43c..583b9a89c93c 100644 --- 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 cmd; diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc index de894ad0b85f..b5ad5e9323a0 100644 --- a/src/mgr/DaemonServer.cc +++ b/src/mgr/DaemonServer.cc @@ -791,7 +791,9 @@ public: bool DaemonServer::handle_command(const ref_t& 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 87a301c72c39..94ff99c390fd 100644 --- 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& 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 9d5251d9d206..7e5272dbedbe 100644 --- 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 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 465af9299ec8..7d193b6575b9 100644 --- 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 796dcfba1b45..42c977891b88 100644 --- 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("gss_ktab_client_file")), store(s), diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index ab63822f0729..3d7fb104177b 100644 --- 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_), -- 2.47.3