Merge pull request #52883 from ajarr/wip-59737-pacific

pacific: mgr: store names of modules that register RADOS clients in the MgrMap

Reviewed-by: Ilya Dryomov <idryomov@redhat.com>

PendingReleaseNotes

@@ -38,6 +38,11 @@
 * CEPHFS: After recovering a Ceph File System post following the disaster recovery
   procedure, the recovered files under `lost+found` directory can now be deleted.
 
+* `ceph mgr dump` command now displays the name of the mgr module that
+  registered a RADOS client in the `name` field added to elements of the
+  `active_clients` array. Previously, only the address of a module's RADOS
+  client was shown in the `active_clients` array.
+
 >=16.2.12
 ---------
 
@@ -105,10 +110,6 @@
 * `ceph mgr dump` command now outputs `last_failure_osd_epoch` and
   `active_clients` fields at the top level.  Previously, these fields were
   output under `always_on_modules` field.
-* `ceph mgr dump` command now displays the name of the mgr module that
-  registered a RADOS client in the `name` field added to elements of the
-  `active_clients` array. Previously, only the address of a module's RADOS
-  client was shown in the `active_clients` array.
 * RBD: All rbd-mirror daemon perf counters became labeled and as such are now
   emitted only by the new `counter dump` and `counter schema` commands.  As part
   of the conversion, many also got renamed to better disambiguate journal-based

qa/workunits/cephtool/test.sh

@@ -758,6 +758,7 @@ function test_mon_misc()
 
   ceph mgr stat
   ceph mgr dump
+  ceph mgr dump | jq -e '.active_clients[0].name'
   ceph mgr module ls
   ceph mgr module enable restful
   expect_false ceph mgr module enable foodne

src/messages/MMgrBeacon.h

@@ -24,7 +24,7 @@
 
 class MMgrBeacon final : public PaxosServiceMessage {
 private:
-  static constexpr int HEAD_VERSION = 10;
+  static constexpr int HEAD_VERSION = 11;
   static constexpr int COMPAT_VERSION = 8;
 
 protected:
@@ -45,7 +45,7 @@ class MMgrBeacon final : public PaxosServiceMessage {
 
   std::map<std::string,std::string> metadata; ///< misc metadata about this osd
 
-  std::vector<entity_addrvec_t> clients;
+  std::multimap<std::string, entity_addrvec_t> clients;
 
   uint64_t mgr_features = 0;   ///< reporting mgr's features
 
@@ -59,12 +59,12 @@ class MMgrBeacon final : public PaxosServiceMessage {
              entity_addrvec_t server_addrs_, bool available_,
 	     std::vector<MgrMap::ModuleInfo>&& modules_,
 	     std::map<std::string,std::string>&& metadata_,
-             std::vector<entity_addrvec_t> clients,
+             std::multimap<std::string, entity_addrvec_t>&& clients_,
 	     uint64_t feat)
     : PaxosServiceMessage{MSG_MGR_BEACON, 0, HEAD_VERSION, COMPAT_VERSION},
       gid(gid_), server_addrs(server_addrs_), available(available_), name(name_),
       fsid(fsid_), modules(std::move(modules_)), metadata(std::move(metadata_)),
-      clients(std::move(clients)),
+      clients(std::move(clients_)),
       mgr_features(feat)
   {
   }
@@ -152,7 +152,17 @@ class MMgrBeacon final : public PaxosServiceMessage {
 
     encode(modules, payload);
     encode(mgr_features, payload);
-    encode(clients, payload, features);
+
+    std::vector<std::string> clients_names;
+    std::vector<entity_addrvec_t> clients_addrs;
+    for (const auto& i : clients) {
+      clients_names.push_back(i.first);
+      clients_addrs.push_back(i.second);
+    }
+    // The address vector needs to be encoded first to produce a
+    // backwards compatible messsage for older monitors.
+    encode(clients_addrs, payload, features);
+    encode(clients_names, payload, features);
   }
   void decode_payload() override {
     using ceph::decode;
@@ -194,7 +204,26 @@ class MMgrBeacon final : public PaxosServiceMessage {
       decode(mgr_features, p);
     }
     if (header.version >= 10) {
-      decode(clients, p);
+      std::vector<entity_addrvec_t> clients_addrs;
+      decode(clients_addrs, p);
+      clients.clear();
+      if (header.version >= 11) {
+	std::vector<std::string> clients_names;
+	decode(clients_names, p);
+	if (clients_names.size() != clients_addrs.size()) {
+	  throw ceph::buffer::malformed_input(
+	    "clients_names.size() != clients_addrs.size()");
+	}
+	auto cn = clients_names.begin();
+	auto ca = clients_addrs.begin();
+	for(; cn != clients_names.end(); ++cn, ++ca) {
+	  clients.emplace(*cn, *ca);
+	}
+      } else {
+	for (const auto& i : clients_addrs) {
+	  clients.emplace("", i);
+	}
+      }
     }
   }
 private:

src/mgr/ActivePyModules.cc

@@ -1492,8 +1492,6 @@ void ActivePyModules::cluster_log(const std::string &channel, clog_type prio,
 
 void ActivePyModules::register_client(std::string_view name, std::string addrs)
 {
-  std::lock_guard l(lock);
-
   entity_addrvec_t addrv;
   addrv.parse(addrs.data());
 
@@ -1503,8 +1501,6 @@ void ActivePyModules::register_client(std::string_view name, std::string addrs)
 
 void ActivePyModules::unregister_client(std::string_view name, std::string addrs)
 {
-  std::lock_guard l(lock);
-
   entity_addrvec_t addrv;
   addrv.parse(addrs.data());
 

src/mgr/PyModuleRegistry.h

@@ -204,10 +204,12 @@ class PyModuleRegistry
 
   void register_client(std::string_view name, entity_addrvec_t addrs)
   {
+    std::lock_guard l(lock);
     clients.emplace(std::string(name), std::move(addrs));
   }
   void unregister_client(std::string_view name, const entity_addrvec_t& addrs)
   {
+    std::lock_guard l(lock);
     auto itp = clients.equal_range(std::string(name));
     for (auto it = itp.first; it != itp.second; ++it) {
       if (it->second == addrs) {
@@ -219,12 +221,8 @@ class PyModuleRegistry
 
   auto get_clients() const
   {
-    std::scoped_lock l(lock);
-    std::vector<entity_addrvec_t> v;
-    for (const auto& p : clients) {
-      v.push_back(p.second);
-    }
-    return v;
+    std::lock_guard l(lock);
+    return clients;
   }
 
   // <<< (end of ActivePyModules cheeky call-throughs)

src/mon/MgrMap.h

@@ -238,7 +238,7 @@ class MgrMap
   /// features
   uint64_t active_mgr_features = 0;
 
-  std::vector<entity_addrvec_t> clients; // for blocklist
+  std::multimap<std::string, entity_addrvec_t> clients; // for blocklist
 
   std::map<uint64_t, StandbyInfo> standbys;
 
@@ -401,7 +401,7 @@ class MgrMap
       ENCODE_FINISH(bl);
       return;
     }
-    ENCODE_START(11, 6, bl);
+    ENCODE_START(12, 6, bl);
     encode(epoch, bl);
     encode(active_addrs, bl, features);
     encode(active_gid, bl);
@@ -415,14 +415,23 @@ class MgrMap
     encode(always_on_modules, bl);
     encode(active_mgr_features, bl);
     encode(last_failure_osd_epoch, bl);
-    encode(clients, bl, features);
+    std::vector<std::string> clients_names;
+    std::vector<entity_addrvec_t> clients_addrs;
+    for (const auto& i : clients) {
+      clients_names.push_back(i.first);
+      clients_addrs.push_back(i.second);
+    }
+    // The address vector needs to be encoded first to produce a
+    // backwards compatible messsage for older monitors.
+    encode(clients_addrs, bl, features);
+    encode(clients_names, bl, features);
     ENCODE_FINISH(bl);
     return;
   }
 
   void decode(ceph::buffer::list::const_iterator& p)
   {
-    DECODE_START(11, p);
+    DECODE_START(12, p);
     decode(epoch, p);
     decode(active_addrs, p);
     decode(active_gid, p);
@@ -468,7 +477,26 @@ class MgrMap
       decode(last_failure_osd_epoch, p);
     }
     if (struct_v >= 11) {
-      decode(clients, p);
+      std::vector<entity_addrvec_t> clients_addrs;
+      decode(clients_addrs, p);
+      clients.clear();
+      if (struct_v >= 12) {
+	std::vector<std::string> clients_names;
+	decode(clients_names, p);
+	if (clients_names.size() != clients_addrs.size()) {
+	  throw ceph::buffer::malformed_input(
+	    "clients_names.size() != clients_addrs.size()");
+	}
+	auto cn = clients_names.begin();
+	auto ca = clients_addrs.begin();
+	for(; cn != clients_names.end(); ++cn, ++ca) {
+	  clients.emplace(*cn, *ca);
+	}
+      } else {
+	for (const auto& i : clients_addrs) {
+	  clients.emplace("", i);
+	}
+      }
     }
     DECODE_FINISH(p);
   }
@@ -524,8 +552,11 @@ class MgrMap
     f->close_section(); // always_on_modules
     f->dump_int("last_failure_osd_epoch", last_failure_osd_epoch);
     f->open_array_section("active_clients");
-    for (const auto &c : clients) {
-      f->dump_object("client", c);
+    for (const auto& i : clients) {
+      f->open_object_section("client");
+      f->dump_string("name", i.first);
+      i.second.dump(f);
+      f->close_section();
     }
     f->close_section(); // active_clients
   }

src/mon/MgrMonitor.cc

@@ -919,7 +919,7 @@ bool MgrMonitor::drop_active()
 
   /* blocklist RADOS clients in use by the mgr */
   for (const auto& a : pending_map.clients) {
-    mon.osdmon()->blocklist(a, until);
+    mon.osdmon()->blocklist(a.second, until);
   }
   request_proposal(mon.osdmon());