Revert "Merge pull request #55436 from tchaikov/mgr-python-3.12"

This reverts commit 8dffa8707dc665eb7ca6e2f48fa34e0fa8ac5b51, reversing
changes made to 80374da12bcc708d1fa306484842b3f97ac42acb.

This commit broke the import of the "mgr_module" module
within the python modules in the mgr at least on python 3.6.8
that we currently use in our centos 8 stream based containers

Failures would look like (removing beginning of log lines)

Loading python module 'alerts'
Module not found: 'mgr_module'
Class not found in module 'alerts'
Error loading module 'alerts': (22) Invalid argument

Signed-off-by: Adam King <adking@redhat.com>

diff --git a/src/mgr/PyModule.cc b/src/mgr/PyModule.cc
index 02a40526f7b8cfbff6cd238d09c245452b321a4e..084cf3ffc1eac44e784b122928c729091f216327 100644 (file)
--- a/src/mgr/PyModule.cc
+++ b/src/mgr/PyModule.cc
@@ -47,6 +47,7 @@ std::string PyModule::mgr_store_prefix = "mgr/";
 
 
 using std::string;
+using std::wstring;
 
 // decode a Python exception into a string
 std::string handle_pyerror(
@@ -230,6 +231,72 @@ std::pair<int, std::string> PyModuleConfig::set_config(
   }
 }
 
+std::string PyModule::get_site_packages()
+{
+  std::stringstream site_packages;
+
+  // CPython doesn't auto-add site-packages dirs to sys.path for us,
+  // but it does provide a module that we can ask for them.
+  auto site_module = PyImport_ImportModule("site");
+  ceph_assert(site_module);
+
+  auto site_packages_fn = PyObject_GetAttrString(site_module, "getsitepackages");
+  if (site_packages_fn != nullptr) {
+    auto site_packages_list = PyObject_CallObject(site_packages_fn, nullptr);
+    ceph_assert(site_packages_list);
+
+    auto n = PyList_Size(site_packages_list);
+    for (Py_ssize_t i = 0; i < n; ++i) {
+      if (i != 0) {
+        site_packages << ":";
+      }
+      site_packages << PyUnicode_AsUTF8(PyList_GetItem(site_packages_list, i));
+    }
+
+    Py_DECREF(site_packages_list);
+    Py_DECREF(site_packages_fn);
+  } else {
+    // Fall back to generating our own site-packages paths by imitating
+    // what the standard site.py does.  This is annoying but it lets us
+    // run inside virtualenvs :-/
+
+    auto site_packages_fn = PyObject_GetAttrString(site_module, "addsitepackages");
+    ceph_assert(site_packages_fn);
+
+    auto known_paths = PySet_New(nullptr);
+    auto pArgs = PyTuple_Pack(1, known_paths);
+    PyObject_CallObject(site_packages_fn, pArgs);
+    Py_DECREF(pArgs);
+    Py_DECREF(known_paths);
+    Py_DECREF(site_packages_fn);
+
+    auto sys_module = PyImport_ImportModule("sys");
+    ceph_assert(sys_module);
+    auto sys_path = PyObject_GetAttrString(sys_module, "path");
+    ceph_assert(sys_path);
+
+    dout(1) << "sys.path:" << dendl;
+    auto n = PyList_Size(sys_path);
+    bool first = true;
+    for (Py_ssize_t i = 0; i < n; ++i) {
+      dout(1) << "  " << PyUnicode_AsUTF8(PyList_GetItem(sys_path, i)) << dendl;
+      if (first) {
+        first = false;
+      } else {
+        site_packages << ":";
+      }
+      site_packages << PyUnicode_AsUTF8(PyList_GetItem(sys_path, i));
+    }
+
+    Py_DECREF(sys_path);
+    Py_DECREF(sys_module);
+  }
+
+  Py_DECREF(site_module);
+
+  return site_packages.str();
+}
+
 PyObject* PyModule::init_ceph_logger()
 {
   auto py_logger = PyModule_Create(&ceph_logger_module);
@@ -290,6 +357,17 @@ int PyModule::load(PyThreadState *pMainThreadState)
       return -EINVAL;
     } else {
       pMyThreadState.set(thread_state);
+      // Some python modules do not cope with an unpopulated argv, so lets
+      // fake one.  This step also picks up site-packages into sys.path.
+      const wchar_t *argv[] = {L"ceph-mgr"};
+      PySys_SetArgv(1, (wchar_t**)argv);
+      // Configure sys.path to include mgr_module_path
+      string paths = (g_conf().get_val<std::string>("mgr_module_path") + ':' +
+                      get_site_packages() + ':');
+      wstring sys_path(wstring(begin(paths), end(paths)) + Py_GetPath());
+      PySys_SetPath(const_cast<wchar_t*>(sys_path.c_str()));
+      dout(10) << "Computed sys.path '"
+              << string(begin(sys_path), end(sys_path)) << "'" << dendl;
     }
   }
   // Environment is all good, import the external module

diff --git a/src/mgr/PyModule.h b/src/mgr/PyModule.h
index 177447c2cb323cec7d04599dc22c3cd4b706cda9..8d88ff94c6271cd0c65a3ee04752f10b499daff8 100644 (file)
--- a/src/mgr/PyModule.h
+++ b/src/mgr/PyModule.h
@@ -51,6 +51,7 @@ class PyModule
   mutable ceph::mutex lock = ceph::make_mutex("PyModule::lock");
 private:
   const std::string module_name;
+  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?

diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc
index 5adce55bf04353cabd1ab17b8ab5007e52edbdbd..eb2d2babe75fa6da4ecb751c2146caa412530ae7 100644 (file)
--- a/src/mgr/PyModuleRegistry.cc
+++ b/src/mgr/PyModuleRegistry.cc
@@ -14,7 +14,6 @@
 #include "PyModuleRegistry.h"
 
 #include <filesystem>
-#include <boost/scope_exit.hpp>
 
 #include "include/stringify.h"
 #include "common/errno.h"
@@ -47,74 +46,14 @@ void PyModuleRegistry::init()
 
   // Set up global python interpreter
 #define WCHAR(s) L ## #s
-#if PY_VERSION_HEX >= 0x03080000
-  PyConfig py_config;
-  // do not enable isolated mode, otherwise we would not be able to have access
-  // to the site packages. since we cannot import any module before initializing
-  // the interpreter, we would not be able to use "site" module for retrieving
-  // the path to site packager. we import "site" module for retrieving
-  // sitepackages in Python < 3.8 though, this does not apply to the
-  // initialization with PyConfig.
-  PyConfig_InitPythonConfig(&py_config);
-  BOOST_SCOPE_EXIT_ALL(&py_config) {
-    PyConfig_Clear(&py_config);
-  };
-#if PY_VERSION_HEX >= 0x030b0000
-  py_config.safe_path = 0;
-#endif
-  py_config.parse_argv = 0;
-  py_config.configure_c_stdio = 0;
-  py_config.install_signal_handlers = 0;
-  py_config.pathconfig_warnings = 0;
-  // site_import is 1 by default, but set it explicitly as we do import
-  // site packages manually for Python < 3.8
-  py_config.site_import = 1;
-
-  PyStatus status;
-  status = PyConfig_SetString(&py_config, &py_config.program_name, WCHAR(MGR_PYTHON_EXECUTABLE));
-  ceph_assertf(!PyStatus_Exception(status), "PyConfig_SetString: %s:%s", status.func, status.err_msg);
-  // Some python modules do not cope with an unpopulated argv, so lets
-  // fake one.  This step also picks up site-packages into sys.path.
-  const wchar_t* argv[] = {L"ceph-mgr"};
-  status = PyConfig_SetArgv(&py_config, 1, (wchar_t *const *)argv);
-  ceph_assertf(!PyStatus_Exception(status), "PyConfig_SetArgv: %s:%s", status.func, status.err_msg);
-  // Add more modules
-  if (g_conf().get_val<bool>("daemonize")) {
-    PyImport_AppendInittab("ceph_logger", PyModule::init_ceph_logger);
-  }
-  PyImport_AppendInittab("ceph_module", PyModule::init_ceph_module);
-  // Configure sys.path to include mgr_module_path
-  auto pythonpath_env = g_conf().get_val<std::string>("mgr_module_path");
-  if (const char* pythonpath = getenv("PYTHONPATH")) {
-    pythonpath_env += ":";
-    pythonpath_env += pythonpath;
-  }
-  status = PyConfig_SetBytesString(&py_config, &py_config.pythonpath_env, pythonpath_env.data());
-  ceph_assertf(!PyStatus_Exception(status), "PyConfig_SetBytesString: %s:%s", status.func, status.err_msg);
-  dout(10) << "set PYTHONPATH to " << std::quoted(pythonpath_env) << dendl;
-  status = Py_InitializeFromConfig(&py_config);
-  ceph_assertf(!PyStatus_Exception(status), "Py_InitializeFromConfig: %s:%s", status.func, status.err_msg);
-#else
   Py_SetProgramName(const_cast<wchar_t*>(WCHAR(MGR_PYTHON_EXECUTABLE)));
+#undef WCHAR
   // Add more modules
   if (g_conf().get_val<bool>("daemonize")) {
     PyImport_AppendInittab("ceph_logger", PyModule::init_ceph_logger);
   }
   PyImport_AppendInittab("ceph_module", PyModule::init_ceph_module);
   Py_InitializeEx(0);
-  const wchar_t *argv[] = {L"ceph-mgr"};
-  PySys_SetArgv(1, (wchar_t**)argv);
-
-  std::string paths = (g_conf().get_val<std::string>("mgr_module_path") + ':' +
-                      get_site_packages() + ':');
-  std::wstring sys_path(begin(paths), end(paths));
-  sys_path += Py_GetPath();
-  PySys_SetPath(const_cast<wchar_t*>(sys_path.c_str()));
-  dout(10) << "Computed sys.path '"
-          << std::string(begin(sys_path), end(sys_path)) << "'" << dendl;
-#endif // PY_VERSION_HEX >= 0x03080000
-#undef WCHAR
-
 #if PY_VERSION_HEX < 0x03090000
   // Let CPython know that we will be calling it back from other
   // threads in future.
@@ -278,72 +217,6 @@ void PyModuleRegistry::active_start(
   }
 }
 
-std::string PyModuleRegistry::get_site_packages()
-{
-  std::stringstream site_packages;
-
-  // CPython doesn't auto-add site-packages dirs to sys.path for us,
-  // but it does provide a module that we can ask for them.
-  auto site_module = PyImport_ImportModule("site");
-  ceph_assert(site_module);
-
-  auto site_packages_fn = PyObject_GetAttrString(site_module, "getsitepackages");
-  if (site_packages_fn != nullptr) {
-    auto site_packages_list = PyObject_CallObject(site_packages_fn, nullptr);
-    ceph_assert(site_packages_list);
-
-    auto n = PyList_Size(site_packages_list);
-    for (Py_ssize_t i = 0; i < n; ++i) {
-      if (i != 0) {
-        site_packages << ":";
-      }
-      site_packages << PyUnicode_AsUTF8(PyList_GetItem(site_packages_list, i));
-    }
-
-    Py_DECREF(site_packages_list);
-    Py_DECREF(site_packages_fn);
-  } else {
-    // Fall back to generating our own site-packages paths by imitating
-    // what the standard site.py does.  This is annoying but it lets us
-    // run inside virtualenvs :-/
-
-    auto site_packages_fn = PyObject_GetAttrString(site_module, "addsitepackages");
-    ceph_assert(site_packages_fn);
-
-    auto known_paths = PySet_New(nullptr);
-    auto pArgs = PyTuple_Pack(1, known_paths);
-    PyObject_CallObject(site_packages_fn, pArgs);
-    Py_DECREF(pArgs);
-    Py_DECREF(known_paths);
-    Py_DECREF(site_packages_fn);
-
-    auto sys_module = PyImport_ImportModule("sys");
-    ceph_assert(sys_module);
-    auto sys_path = PyObject_GetAttrString(sys_module, "path");
-    ceph_assert(sys_path);
-
-    dout(1) << "sys.path:" << dendl;
-    auto n = PyList_Size(sys_path);
-    bool first = true;
-    for (Py_ssize_t i = 0; i < n; ++i) {
-      dout(1) << "  " << PyUnicode_AsUTF8(PyList_GetItem(sys_path, i)) << dendl;
-      if (first) {
-        first = false;
-      } else {
-        site_packages << ":";
-      }
-      site_packages << PyUnicode_AsUTF8(PyList_GetItem(sys_path, i));
-    }
-
-    Py_DECREF(sys_path);
-    Py_DECREF(sys_module);
-  }
-
-  Py_DECREF(site_module);
-
-  return site_packages.str();
-}
-
 std::vector<std::string> PyModuleRegistry::probe_modules(const std::string &path) const
 {
   const auto opt = g_conf().get_val<std::string>("mgr_disabled_modules");

diff --git a/src/mgr/PyModuleRegistry.h b/src/mgr/PyModuleRegistry.h
index da5bb596c938dfdee176d4ce1e3eaa4dc40e7594..9d6d9c2cdd0210b9b0077ffd4be3f996a7c65d29 100644 (file)
--- a/src/mgr/PyModuleRegistry.h
+++ b/src/mgr/PyModuleRegistry.h
@@ -55,7 +55,6 @@ private:
   // before ClusterState exists.
   MgrMap mgr_map;
 
-  static std::string get_site_packages();
   /**
    * Discover python modules from local disk
    */