diff --git a/src/mgr/MgrStandby.cc b/src/mgr/MgrStandby.cc index d90b799f577..da358d868fb 100644 --- a/src/mgr/MgrStandby.cc +++ b/src/mgr/MgrStandby.cc @@ -188,10 +188,6 @@ void MgrStandby::send_beacon() // which we will transmit to the monitor. std::vector module_info; for (const auto &module : modules) { - // do not announce always_on modules to the monitor - if (module->is_always_on()) { - continue; - } MgrMap::ModuleInfo info; info.name = module->get_name(); info.error_string = module->get_error_string(); diff --git a/src/mgr/PyModule.h b/src/mgr/PyModule.h index 3ffdeb8f13a..db5c501d375 100644 --- a/src/mgr/PyModule.h +++ b/src/mgr/PyModule.h @@ -57,13 +57,15 @@ class PyModule mutable Mutex lock{"PyModule::lock"}; private: const std::string module_name; - const bool always_on; std::string get_site_packages(); int load_subclass_of(const char* class_name, PyObject** py_class); // Did the MgrMap identify this module as one that should run? bool enabled = false; + // Did the MgrMap flag this module as always on? + bool always_on = false; + // Did we successfully import this python module and look up symbols? // (i.e. is it possible to instantiate a MgrModule subclass instance?) bool loaded = false; @@ -97,8 +99,8 @@ public: PyObject *pClass = nullptr; PyObject *pStandbyClass = nullptr; - explicit PyModule(const std::string &module_name_, bool always_on) - : module_name(module_name_), always_on(always_on) + explicit PyModule(const std::string &module_name_) + : module_name(module_name_) { } @@ -120,6 +122,10 @@ public: enabled = enabled_; } + void set_always_on(const bool always_on_) { + always_on = always_on_; + } + /** * Extend `out` with the contents of `this->commands` */ diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc index 3e96639e838..7c0b1a4e426 100644 --- a/src/mgr/PyModuleRegistry.cc +++ b/src/mgr/PyModuleRegistry.cc @@ -71,12 +71,9 @@ void PyModuleRegistry::init() for (const auto& module_name : module_names) { dout(1) << "Loading python module '" << module_name << "'" << dendl; - const bool always_on = always_on_modules.find(module_name) != - always_on_modules.end(); - // Everything starts disabled, set enabled flag on module // when we see first MgrMap - auto mod = std::make_shared(module_name, always_on); + auto mod = std::make_shared(module_name); int r = mod->load(pMainThreadState); if (r != 0) { // Don't use handle_pyerror() here; we don't have the GIL @@ -111,11 +108,14 @@ bool PyModuleRegistry::handle_mgr_map(const MgrMap &mgr_map_) for (const auto &[module_name, module] : modules) { const bool enabled = (mgr_map.modules.count(module_name) > 0); module->set_enabled(enabled); + const bool always_on = (mgr_map.get_always_on_modules().count(module_name) > 0); + module->set_always_on(always_on); } return false; } else { - bool modules_changed = mgr_map_.modules != mgr_map.modules; + bool modules_changed = mgr_map_.modules != mgr_map.modules || + mgr_map_.always_on_modules != mgr_map.always_on_modules; mgr_map = mgr_map_; if (standby_modules != nullptr) { @@ -374,7 +374,7 @@ void PyModuleRegistry::get_health_checks(health_check_map_t *checks) } // report failed always_on modules as health errors - for (const auto& name : always_on_modules) { + for (const auto& name : mgr_map.get_always_on_modules()) { if (!active_modules->module_exists(name)) { if (failed_modules.find(name) == failed_modules.end() && dependency_modules.find(name) == dependency_modules.end()) { diff --git a/src/mgr/PyModuleRegistry.h b/src/mgr/PyModuleRegistry.h index f2b143e4288..090fd101fe2 100644 --- a/src/mgr/PyModuleRegistry.h +++ b/src/mgr/PyModuleRegistry.h @@ -19,6 +19,7 @@ #include #include +#include #include #include "common/LogClient.h" diff --git a/src/mgr/mgr_commands.cc b/src/mgr/mgr_commands.cc index 702b858eadc..aafee99140c 100644 --- a/src/mgr/mgr_commands.cc +++ b/src/mgr/mgr_commands.cc @@ -3,24 +3,6 @@ #include "mgr_commands.h" -// an always_on module behaves as if it provides built-in functionality. it's -// always enabled, and always_on modules do not appear in the list of modules -// that can be enabled or disabled by the user. a health warning error is -// reported if a module in this set is cannot be loaded and started. -// -// NOTE: if a module is added to this set AND that module _may_ have been -// enabled in some cluster (e.g. an existing module is being upgraded to -// always-on status), then additional logic is needed in the monitor to trim the -// enabled set (or upgrade script to run `disable always_on_modules = { - "crash", - "status", - "balancer", - "devicehealth", -}; - /* The set of statically defined (C++-handled) commands. This * does not include the Python-defined commands, which are loaded * in PyModules */ diff --git a/src/mgr/mgr_commands.h b/src/mgr/mgr_commands.h index e32aa54c939..c6ed6c68d72 100644 --- a/src/mgr/mgr_commands.h +++ b/src/mgr/mgr_commands.h @@ -4,9 +4,6 @@ #pragma once #include "mon/MonCommand.h" -#include -#include #include -extern const std::set always_on_modules; extern const std::vector mgr_commands; diff --git a/src/mon/MgrMap.h b/src/mon/MgrMap.h index bdfb27d37fc..48cb6862a8e 100644 --- a/src/mon/MgrMap.h +++ b/src/mon/MgrMap.h @@ -19,6 +19,7 @@ #include "msg/msg_types.h" #include "common/Formatter.h" #include "include/encoding.h" +#include "common/version.h" class MgrMap @@ -145,6 +146,11 @@ public: // Modules which are enabled std::set modules; + // Modules which should always be enabled. A manager daemon will enable + // modules from the union of this set and the `modules` set above, latest + // active version. + std::map> always_on_modules; + // Modules which are reported to exist std::vector available_modules; @@ -236,6 +242,13 @@ public: return ls; } + std::set get_always_on_modules() const { + auto it = always_on_modules.find(ceph_release()); + if (it == always_on_modules.end()) + return {}; + return it->second; + } + void encode(bufferlist& bl, uint64_t features) const { if (!HAVE_FEATURE(features, SERVER_NAUTILUS)) { @@ -261,7 +274,7 @@ public: ENCODE_FINISH(bl); return; } - ENCODE_START(7, 6, bl); + ENCODE_START(8, 6, bl); encode(epoch, bl); encode(active_addrs, bl, features); encode(active_gid, bl); @@ -272,6 +285,7 @@ public: encode(services, bl); encode(available_modules, bl); encode(active_change, bl); + encode(always_on_modules, bl); ENCODE_FINISH(bl); return; } @@ -314,6 +328,9 @@ public: } else { active_change = {}; } + if (struct_v >= 8) { + decode(always_on_modules, p); + } DECODE_FINISH(p); } @@ -353,6 +370,16 @@ public: f->dump_string(i.first.c_str(), i.second); } f->close_section(); + + f->open_object_section("always_on_modules"); + for (auto& v : always_on_modules) { + f->open_array_section(ceph_release_name(v.first)); + for (auto& m : v.second) { + f->dump_string("module", m); + } + f->close_section(); + } + f->close_section(); } static void generate_test_instances(list &l) { diff --git a/src/mon/MgrMonitor.cc b/src/mon/MgrMonitor.cc index c02088d4cc2..39442c05dc9 100644 --- a/src/mon/MgrMonitor.cc +++ b/src/mon/MgrMonitor.cc @@ -36,6 +36,19 @@ static ostream& _prefix(std::ostream *_dout, Monitor *mon, << ").mgr e" << mgrmap.get_epoch() << " "; } +// the system treats always_on_modules as if they provide built-in functionality +// by ensuring that they are always enabled. +const static std::map> always_on_modules = { + { + CEPH_RELEASE_NAUTILUS, { + "crash", + "status", + "balancer", + "devicehealth" + } + } +}; + // Prefix for mon store of active mgr's command descriptions const static std::string command_descs_prefix = "mgr_command_descs"; @@ -57,9 +70,11 @@ void MgrMonitor::create_initial() for (auto& m : tok) { pending_map.modules.insert(m); } + pending_map.always_on_modules = always_on_modules; pending_command_descs = mgr_commands; dout(10) << __func__ << " initial modules " << pending_map.modules - << ", " << pending_command_descs.size() << " commands" + << ", always on modules " << pending_map.get_always_on_modules() + << ", " << pending_command_descs.size() << " commands" << dendl; } @@ -546,6 +561,13 @@ void MgrMonitor::on_active() { if (mon->is_leader()) { mon->clog->debug() << "mgrmap e" << map.epoch << ": " << map; + + if (pending_map.always_on_modules != always_on_modules) { + pending_map.always_on_modules = always_on_modules; + dout(4) << "always on modules changed " + << pending_map.get_always_on_modules() << dendl; + propose_pending(); + } } } @@ -761,6 +783,8 @@ bool MgrMonitor::preprocess_command(MonOpRequestRef op) { f->open_array_section("enabled_modules"); for (auto& p : map.modules) { + if (map.get_always_on_modules().count(p) > 0) + continue; // We only show the name for enabled modules. The any errors // etc will show up as a health checks. f->dump_string("module", p); @@ -768,7 +792,8 @@ bool MgrMonitor::preprocess_command(MonOpRequestRef op) f->close_section(); f->open_array_section("disabled_modules"); for (auto& p : map.available_modules) { - if (map.modules.count(p.name) == 0) { + if (map.modules.count(p.name) == 0 && + map.get_always_on_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()); @@ -926,6 +951,10 @@ bool MgrMonitor::prepare_command(MonOpRequestRef op) r = -EINVAL; goto out; } + if (pending_map.get_always_on_modules().count(module) > 0) { + ss << "module '" << module << "' is already enabled (always-on)"; + goto out; + } string force; cmd_getval(g_ceph_context, cmdmap, "force", force); if (!pending_map.all_support_module(module) && @@ -958,6 +987,11 @@ bool MgrMonitor::prepare_command(MonOpRequestRef op) r = -EINVAL; goto out; } + if (pending_map.get_always_on_modules().count(module) > 0) { + ss << "module '" << module << "' cannot be disabled (always-on)"; + r = -EINVAL; + goto out; + } if (!pending_map.module_enabled(module)) { ss << "module '" << module << "' is already disabled"; r = 0;