From: bhavishyagopesh <bhavishyagopesh@gmail.com>
Date: Mon, 9 Oct 2017 05:40:41 +0000 (+0530)
Subject: mgr: In plugins 'module' classes need not to be called "Module" anymore.
X-Git-Tag: v13.0.1~303^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=345c14438e015c5e447d362489592a46821caa17;p=ceph.git

mgr: In plugins 'module' classes need not to be called "Module" anymore.

Fixes: http://tracker.ceph.com/issues/17454
Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: bhavishyagopesh <bhavishyagopesh@gmail.com>
---

diff --git a/doc/mgr/plugins.rst b/doc/mgr/plugins.rst
index 654c50537bc..7570b470d05 100644
--- a/doc/mgr/plugins.rst
+++ b/doc/mgr/plugins.rst
@@ -6,7 +6,7 @@ Creating a plugin
 -----------------
 
 In pybind/mgr/, create a python module.  Within your module, create a class
-named ``Module`` that inherits from ``MgrModule``.
+that inherits from ``MgrModule``.
 
 The most important methods to override are:
 
@@ -33,7 +33,7 @@ or older versions of Ceph.
 Logging
 -------
 
-MgrModule instances have a ``log`` property which is a logger instance that
+``MgrModule`` instances have a ``log`` property which is a logger instance that
 sends log messages into the Ceph logging layer where they will be recorded
 in the mgr daemon's log file.
 
@@ -169,7 +169,7 @@ serve HTTP redirect responses from the standby managers so that
 the user can point his browser at any of the manager daemons without
 having to worry about which one is active.
 
-Standby manager daemons look for a class called ``StandbyModule``
+Standby manager daemons look for a subclass of ``StandbyModule``
 in each module.  If the class is not found then the module is not
 used at all on standby daemons.  If the class is found, then
 its ``serve`` method is called.  Implementations of ``StandbyModule``
diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc
index bab057c6c28..9b9de353594 100644
--- a/src/mgr/PyModuleRegistry.cc
+++ b/src/mgr/PyModuleRegistry.cc
@@ -254,43 +254,80 @@ int PyModule::load(PyThreadState *pMainThreadState)
   // Environment is all good, import the external module
   {
     Gil gil(pMyThreadState);
-
-    // Load the module
-    PyObject *pName = PyString_FromString(module_name.c_str());
-    auto pModule = PyImport_Import(pName);
-    Py_DECREF(pName);
-    if (pModule == nullptr) {
-      derr << "Module not found: '" << module_name << "'" << dendl;
-      derr << handle_pyerror() << dendl;
-      return -ENOENT;
-    }
-
-    // Find the class
-    // TODO: let them call it what they want instead of just 'Module'
-    pClass = PyObject_GetAttrString(pModule, (const char*)"Module");
-    if (pClass == nullptr) {
+    int r;
+    r = load_subclass_of("MgrModule", &pClass);
+    if (r) {
       derr << "Class not found in module '" << module_name << "'" << dendl;
-      derr << handle_pyerror() << dendl;
-      return -EINVAL;
+      return r;
     }
-
-    pStandbyClass = PyObject_GetAttrString(pModule,
-                                           (const char*)"StandbyModule");
-    if (pStandbyClass) {
+    r = load_subclass_of("MgrStandbyModule", &pStandbyClass);
+    if (!r) {
       dout(4) << "Standby mode available in module '" << module_name
               << "'" << dendl;
     } else {
       dout(4) << "Standby mode not provided by module '" << module_name
               << "'" << dendl;
-      PyErr_Clear();
     }
-
-    Py_DECREF(pModule);
   }
-
   return 0;
 } 
 
+int PyModule::load_subclass_of(const char* base_class, PyObject** py_class)
+{
+  // load the base class
+  PyObject *mgr_module = PyImport_ImportModule("mgr_module");
+  if (!mgr_module) {
+    derr << "Module not found: 'mgr_module'" << dendl;
+    derr << handle_pyerror() << dendl;
+    return -EINVAL;
+  }
+  auto mgr_module_type = PyObject_GetAttrString(mgr_module, base_class);
+  Py_DECREF(mgr_module);
+  if (!mgr_module_type) {
+    derr << "Unable to import MgrModule from mgr_module" << dendl;
+    derr << handle_pyerror() << dendl;
+    return -EINVAL;
+  }
+
+  // find the sub class
+  PyObject *plugin_module = PyImport_ImportModule(module_name.c_str());
+  if (!plugin_module) {
+    derr << "Module not found: '" << module_name << "'" << dendl;
+    derr << handle_pyerror() << dendl;
+    return -ENOENT;
+  }
+  auto locals = PyModule_GetDict(plugin_module);
+  Py_DECREF(plugin_module);
+  PyObject *key, *value;
+  Py_ssize_t pos = 0;
+  *py_class = nullptr;
+  while (PyDict_Next(locals, &pos, &key, &value)) {
+    if (!PyType_Check(value)) {
+      continue;
+    }
+    if (!PyObject_IsSubclass(value, mgr_module_type)) {
+      continue;
+    }
+    if (PyObject_RichCompareBool(value, mgr_module_type, Py_EQ)) {
+      continue;
+    }
+    auto class_name = PyString_AsString(key);
+    if (*py_class) {
+      derr << __func__ << ": ignoring '"
+	   << module_name << "." << class_name << "'"
+	   << ": only one '" << base_class
+	   << "' class is loaded from each plugin" << dendl;
+      continue;
+    }
+    *py_class = value;
+    dout(4) << __func__ << ": found class: '"
+	    << module_name << "." << class_name << "'" << dendl;
+  }
+  Py_DECREF(mgr_module_type);
+
+  return *py_class ? 0 : -EINVAL;
+}
+
 PyModule::~PyModule()
 {
   if (pMyThreadState.ts != nullptr) {
diff --git a/src/mgr/PyModuleRegistry.h b/src/mgr/PyModuleRegistry.h
index 5564e7f1fa5..4dec1008116 100644
--- a/src/mgr/PyModuleRegistry.h
+++ b/src/mgr/PyModuleRegistry.h
@@ -31,7 +31,7 @@ class PyModule
 private:
   const std::string module_name;
   std::string get_site_packages();
-
+  int load_subclass_of(const char* class_name, PyObject** py_class);
 public:
   SafeThreadState pMyThreadState;
   PyObject *pClass = nullptr;