mon: expose can_run flag in `module ls` + mgr dump

Enable people to see can_run failures and the explanatory
messages (telling them about a missing dependency)
before trying to enable a module.

Signed-off-by: John Spray <john.spray@redhat.com>

diff --git a/src/messages/MMgrBeacon.h b/src/messages/MMgrBeacon.h
index 660941d056d6f1470aa4ecead67ca29450919c34..efc8773d62123b63792257f245a1ca6005b0df68 100644 (file)
--- a/src/messages/MMgrBeacon.h
+++ b/src/messages/MMgrBeacon.h
@@ -17,6 +17,7 @@
 
 #include "messages/PaxosServiceMessage.h"
 #include "mon/MonCommand.h"
+#include "mon/MgrMap.h"
 
 #include "include/types.h"
 
@@ -27,34 +28,6 @@ class MMgrBeacon : public PaxosServiceMessage {
   static const int HEAD_VERSION = 7;
   static const int COMPAT_VERSION = 1;
 
-  public:
-
-  class ModuleInfo
-  {
-    public:
-    std::string name;
-    bool can_run = true;
-    std::string error_string;
-
-    // We do not include the module's `failed` field in the beacon,
-    // because it is exposed via health checks.
-    void encode(bufferlist &bl) const {
-      ENCODE_START(1, 1, bl);
-      ::encode(name, bl);
-      ::encode(can_run, bl);
-      ::encode(error_string, bl);
-      ENCODE_FINISH(bl);
-    }
-
-    void decode(bufferlist::iterator &bl) {
-      DECODE_START(1, bl);
-      ::decode(name, bl);
-      ::decode(can_run, bl);
-      ::decode(error_string, bl);
-      DECODE_FINISH(bl);
-    }
-  };
-
 protected:
   uint64_t gid;
   entity_addr_t server_addr;
@@ -69,7 +42,7 @@ protected:
   std::vector<MonCommand> command_descs;
 
   // Information about the modules found locally on this daemon
-  std::vector<ModuleInfo> modules;
+  std::vector<MgrMap::ModuleInfo> modules;
 
   map<string,string> metadata; ///< misc metadata about this osd
 
@@ -82,7 +55,7 @@ public:
 
   MMgrBeacon(const uuid_d& fsid_, uint64_t gid_, const std::string &name_,
              entity_addr_t server_addr_, bool available_,
-            std::vector<ModuleInfo>&& modules_,
+            std::vector<MgrMap::ModuleInfo>&& modules_,
             map<string,string>&& metadata_)
     : PaxosServiceMessage(MSG_MGR_BEACON, 0, HEAD_VERSION, COMPAT_VERSION),
       gid(gid_), server_addr(server_addr_), available(available_), name(name_),
@@ -95,7 +68,7 @@ public:
   bool get_available() const { return available; }
   const std::string& get_name() const { return name; }
   const uuid_d& get_fsid() const { return fsid; }
-  std::vector<ModuleInfo>& get_modules() { return modules; }
+  std::vector<MgrMap::ModuleInfo>& get_modules() { return modules; }
   const std::map<std::string,std::string>& get_metadata() const {
     return metadata;
   }
@@ -119,18 +92,11 @@ public:
     return command_descs;
   }
 
-  std::set<std::string> get_available_modules() const
+  const std::vector<MgrMap::ModuleInfo> &get_available_modules() const
   {
-    std::set<std::string> result;
-    for (const auto &i : modules) {
-      result.insert(i.name);
-    }
-
-    return result;
+    return modules;
   }
 
-
-
 private:
   ~MMgrBeacon() override {}
 
@@ -185,7 +151,7 @@ public:
       // ModuleInfo structures added in v7
       if (header.version < 7) {
         for (const auto &i : module_name_list) {
-          ModuleInfo info;
+          MgrMap::ModuleInfo info;
           info.name = i;
           modules.push_back(std::move(info));
         }
@@ -206,7 +172,5 @@ public:
   }
 };
 
-WRITE_CLASS_ENCODER(MMgrBeacon::ModuleInfo);
-
 
 #endif

diff --git a/src/mgr/MgrStandby.cc b/src/mgr/MgrStandby.cc
index cf491d8d72a99ad5d31c4cdeb901e361257df204..335e134141abd5914cbc8fbce82fd0e8376fe788 100644 (file)
--- a/src/mgr/MgrStandby.cc
+++ b/src/mgr/MgrStandby.cc
@@ -157,9 +157,9 @@ void MgrStandby::send_beacon()
 
   // Construct a list of the info about each loaded module
   // which we will transmit to the monitor.
-  std::vector<MMgrBeacon::ModuleInfo> module_info;
+  std::vector<MgrMap::ModuleInfo> module_info;
   for (const auto &module : modules) {
-    MMgrBeacon::ModuleInfo info;
+    MgrMap::ModuleInfo info;
     info.name = module->get_name();
     info.error_string = module->get_error_string();
     info.can_run = module->get_can_run();

diff --git a/src/mon/MgrMap.h b/src/mon/MgrMap.h
index 15d5c0cb29190dca5f010fa291132621f0910a6e..9415972904101f731b7ea10e4070b0b90af558c0 100644 (file)
--- a/src/mon/MgrMap.h
+++ b/src/mon/MgrMap.h
@@ -20,47 +20,113 @@
 #include "common/Formatter.h"
 #include "include/encoding.h"
 
-class StandbyInfo
+
+class MgrMap
 {
 public:
-  uint64_t gid;
-  std::string name;
-  std::set<std::string> available_modules;
+  class ModuleInfo
+  {
+    public:
+    std::string name;
+    bool can_run = true;
+    std::string error_string;
 
-  StandbyInfo(uint64_t gid_, const std::string &name_,
-             const std::set<std::string>& am)
-    : gid(gid_), name(name_), available_modules(am)
-  {}
+    // We do not include the module's `failed` field in the beacon,
+    // because it is exposed via health checks.
+    void encode(bufferlist &bl) const {
+      ENCODE_START(1, 1, bl);
+      encode(name, bl);
+      encode(can_run, bl);
+      encode(error_string, bl);
+      ENCODE_FINISH(bl);
+    }
 
-  StandbyInfo()
-    : gid(0)
-  {}
+    void decode(bufferlist::iterator &bl) {
+      DECODE_START(1, bl);
+      decode(name, bl);
+      decode(can_run, bl);
+      decode(error_string, bl);
+      DECODE_FINISH(bl);
+    }
 
-  void encode(bufferlist& bl) const
-  {
-    ENCODE_START(2, 1, bl);
-    encode(gid, bl);
-    encode(name, bl);
-    encode(available_modules, bl);
-    ENCODE_FINISH(bl);
-  }
+    bool operator==(const ModuleInfo &rhs) const
+    {
+      return (name == rhs.name) && (can_run == rhs.can_run);
+    }
 
-  void decode(bufferlist::iterator& p)
+    void dump(Formatter *f) const {
+      f->open_object_section("module");
+      f->dump_string("name", name);
+      f->dump_bool("can_run", can_run);
+      f->dump_string("error_string", error_string);
+      f->close_section();
+    }
+  };
+
+  class StandbyInfo
   {
-    DECODE_START(2, p);
-    decode(gid, p);
-    decode(name, p);
-    if (struct_v >= 2) {
-      decode(available_modules, p);
+  public:
+    uint64_t gid;
+    std::string name;
+    std::vector<ModuleInfo> available_modules;
+
+    StandbyInfo(uint64_t gid_, const std::string &name_,
+                const std::vector<ModuleInfo>& am)
+      : gid(gid_), name(name_), available_modules(am)
+    {}
+
+    StandbyInfo()
+      : gid(0)
+    {}
+
+    void encode(bufferlist& bl) const
+    {
+      ENCODE_START(3, 1, bl);
+      encode(gid, bl);
+      encode(name, bl);
+      std::set<std::string> old_available_modules;
+      for (const auto &i : available_modules) {
+        old_available_modules.insert(i.name);
+      }
+      encode(old_available_modules, bl);  // version 2
+      encode(available_modules, bl);  // version 3
+      ENCODE_FINISH(bl);
     }
-    DECODE_FINISH(p);
-  }
-};
-WRITE_CLASS_ENCODER(StandbyInfo)
 
-class MgrMap
-{
-public:
+    void decode(bufferlist::iterator& p)
+    {
+      DECODE_START(3, p);
+      decode(gid, p);
+      decode(name, p);
+      if (struct_v >= 2) {
+        std::set<std::string> old_available_modules;
+        decode(old_available_modules, p);
+        if (struct_v < 3) {
+          for (const auto &name : old_available_modules) {
+            MgrMap::ModuleInfo info;
+            info.name = name;
+            available_modules.push_back(std::move(info));
+          }
+        }
+      }
+      if (struct_v >= 3) {
+        decode(available_modules, p);
+      }
+      DECODE_FINISH(p);
+    }
+
+    bool have_module(const std::string &module_name) const
+    {
+      auto it = std::find_if(available_modules.begin(),
+          available_modules.end(),
+          [module_name](const ModuleInfo &m) -> bool {
+            return m.name == module_name;
+          });
+
+      return it != available_modules.end();
+    }
+  };
+
   epoch_t epoch = 0;
 
   /// global_id of the ceph-mgr instance selected as a leader
@@ -74,8 +140,11 @@ public:
 
   std::map<uint64_t, StandbyInfo> standbys;
 
+  // Modules which are enabled
   std::set<std::string> modules;
-  std::set<std::string> available_modules;
+
+  // Modules which are reported to exist
+  std::vector<ModuleInfo> available_modules;
 
   // Map of module name to URI, indicating services exposed by
   // running modules on the active mgr daemon.
@@ -88,17 +157,28 @@ public:
   const std::string &get_active_name() const { return active_name; }
 
   bool all_support_module(const std::string& module) {
-    if (!available_modules.count(module)) {
+    if (!have_module(module)) {
       return false;
     }
     for (auto& p : standbys) {
-      if (!p.second.available_modules.count(module)) {
+      if (!p.second.have_module(module)) {
        return false;
       }
     }
     return true;
   }
 
+  bool have_module(const std::string &module_name) const
+  {
+    for (const auto &i : available_modules) {
+      if (i.name == module_name) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
   bool have_name(const string& name) const {
     if (active_name == name) {
       return true;
@@ -124,7 +204,7 @@ public:
 
   void encode(bufferlist& bl, uint64_t features) const
   {
-    ENCODE_START(3, 1, bl);
+    ENCODE_START(4, 1, bl);
     encode(epoch, bl);
     encode(active_addr, bl, features);
     encode(active_gid, bl);
@@ -132,14 +212,23 @@ public:
     encode(active_name, bl);
     encode(standbys, bl);
     encode(modules, bl);
-    encode(available_modules, bl);
+
+    // Pre-version 4 string list of available modules
+    // (replaced by direct encode of ModuleInfo below)
+    std::set<std::string> old_available_modules;
+    for (const auto &i : available_modules) {
+      old_available_modules.insert(i.name);
+    }
+    encode(old_available_modules, bl);
+
     encode(services, bl);
+    encode(available_modules, bl);
     ENCODE_FINISH(bl);
   }
 
   void decode(bufferlist::iterator& p)
   {
-    DECODE_START(2, p);
+    DECODE_START(4, p);
     decode(epoch, p);
     decode(active_addr, p);
     decode(active_gid, p);
@@ -148,11 +237,26 @@ public:
     decode(standbys, p);
     if (struct_v >= 2) {
       decode(modules, p);
-      decode(available_modules, p);
+
+      // Reconstitute ModuleInfos from names
+      std::set<std::string> module_name_list;
+      decode(module_name_list, p);
+      // Only need to unpack this field if we won't have the full
+      // MgrMap::ModuleInfo structures added in v4
+      if (struct_v < 4) {
+        for (const auto &i : module_name_list) {
+          MgrMap::ModuleInfo info;
+          info.name = i;
+          available_modules.push_back(std::move(info));
+        }
+      }
     }
     if (struct_v >= 3) {
       decode(services, p);
     }
+    if (struct_v >= 4) {
+      decode(available_modules, p);
+    }
     DECODE_FINISH(p);
   }
 
@@ -168,8 +272,8 @@ public:
       f->dump_int("gid", i.second.gid);
       f->dump_string("name", i.second.name);
       f->open_array_section("available_modules");
-      for (auto& j : i.second.available_modules) {
-       f->dump_string("module", j);
+      for (const auto& j : i.second.available_modules) {
+        j.dump(f);
       }
       f->close_section();
       f->close_section();
@@ -181,8 +285,8 @@ public:
     }
     f->close_section();
     f->open_array_section("available_modules");
-    for (auto& j : available_modules) {
-      f->dump_string("module", j);
+    for (const auto& j : available_modules) {
+      j.dump(f);
     }
     f->close_section();
 
@@ -234,9 +338,18 @@ public:
     m.print_summary(nullptr, &ss);
     return out << ss.str();
   }
+
+  friend ostream& operator<<(ostream& out, const std::vector<ModuleInfo>& mi) {
+    for (const auto &i : mi) {
+      out << i.name << " ";
+    }
+    return out;
+  }
 };
 
 WRITE_CLASS_ENCODER_FEATURES(MgrMap)
+WRITE_CLASS_ENCODER(MgrMap::StandbyInfo)
+WRITE_CLASS_ENCODER(MgrMap::ModuleInfo);
 
 #endif
 

diff --git a/src/mon/MgrMonitor.cc b/src/mon/MgrMonitor.cc
index 302221ca968efb8bfa2fe09a7533f4ffeb8654b9..76985abc04332cc8070a2af1fc018713dbb59a69 100644 (file)
--- a/src/mon/MgrMonitor.cc
+++ b/src/mon/MgrMonitor.cc
@@ -295,7 +295,7 @@ bool MgrMonitor::prepare_beacon(MonOpRequestRef op)
 
   // See if we are seeing same name, new GID for any standbys
   for (const auto &i : pending_map.standbys) {
-    const StandbyInfo &s = i.second;
+    const MgrMap::StandbyInfo &s = i.second;
     if (s.name == m->get_name() && s.gid != m->get_gid()) {
       dout(4) << "Standby daemon restart (mgr." << m->get_name() << ")" << dendl;
       mon->clog->debug() << "Standby manager daemon " << m->get_name()
@@ -712,13 +712,17 @@ bool MgrMonitor::preprocess_command(MonOpRequestRef op)
     {
       f->open_array_section("enabled_modules");
       for (auto& p : map.modules) {
+        // We only show the name for enabled modules.  The any errors
+        // etc will show up as a health checks.
         f->dump_string("module", p);
       }
       f->close_section();
       f->open_array_section("disabled_modules");
       for (auto& p : map.available_modules) {
-        if (map.modules.count(p) == 0) {
-          f->dump_string("module", p);
+        if (map.modules.count(p.name) == 0) {
+          // For disabled modules, we show the full info, to
+          // give a hint about whether enabling it will work
+          p.dump(f.get());
         }
       }
       f->close_section();