ceph/trunk/backport_mgr_disabled_modules_workaround_PR34846.patch

   1 From 40d06ce623f19172721a2f1d5b4f4f8b642a3077 Mon Sep 17 00:00:00 2001
   2 From: Kefu Chai <kchai@redhat.com>
   3 Date: Thu, 30 Apr 2020 10:43:01 +0800
   4 Subject: [PATCH] mgr/PyModuleRegistry: probe modules using std::filesystem
   5
   6 for better readability
   7
   8 Signed-off-by: Kefu Chai <kchai@redhat.com>
   9 ---
  10  src/mgr/PyModuleRegistry.cc | 43 +++++++++++++++++--------------------
  11  1 file changed, 20 insertions(+), 23 deletions(-)
  12
  13 diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc
  14 index 440f7c8bafc1e..466da5d404964 100644
  15 --- a/src/mgr/PyModuleRegistry.cc
  16 +++ b/src/mgr/PyModuleRegistry.cc
  17 @@ -11,6 +11,17 @@
  18   * Foundation.  See file COPYING.
  19   */
  20
  21 +#include "PyModuleRegistry.h"
  22 +
  23 +#if __has_include(<filesystem>)
  24 +#include <filesystem>
  25 +namespace fs = std::filesystem;
  26 +#elif __has_include(<experimental/filesystem>)
  27 +#include <experimental/filesystem>
  28 +namespace fs = std::experimental::filesystem;
  29 +#else
  30 +#error std::filesystem not available!
  31 +#endif
  32
  33  #include "include/stringify.h"
  34  #include "common/errno.h"
  35 @@ -24,8 +35,6 @@
  36
  37  #include "ActivePyModules.h"
  38
  39 -#include "PyModuleRegistry.h"
  40 -
  41  #define dout_context g_ceph_context
  42  #define dout_subsys ceph_subsys_mgr
  43
  44 @@ -258,29 +267,17 @@ void PyModuleRegistry::shutdown()
  45
  46  std::set<std::string> PyModuleRegistry::probe_modules(const std::string &path) const
  47  {
  48 -  DIR *dir = opendir(path.c_str());
  49 -  if (!dir) {
  50 -    return {};
  51 -  }
  52 -
  53 -  std::set<std::string> modules_out;
  54 -  struct dirent *entry = NULL;
  55 -  while ((entry = readdir(dir)) != NULL) {
  56 -    string n(entry->d_name);
  57 -    string fn = path + "/" + n;
  58 -    struct stat st;
  59 -    int r = ::stat(fn.c_str(), &st);
  60 -    if (r == 0 && S_ISDIR(st.st_mode)) {
  61 -      string initfn = fn + "/module.py";
  62 -      r = ::stat(initfn.c_str(), &st);
  63 -      if (r == 0) {
  64 -       modules_out.insert(n);
  65 -      }
  66 +  std::set<std::string> modules;
  67 +  for (const auto& entry: fs::directory_iterator(path)) {
  68 +    if (!fs::is_directory(entry)) {
  69 +      continue;
  70 +    }
  71 +    auto module_path = entry.path() / "module.py";
  72 +    if (fs::exists(module_path)) {
  73 +      modules.emplace(entry.path().filename());
  74      }
  75    }
  76 -  closedir(dir);
  77 -
  78 -  return modules_out;
  79 +  return modules;
  80  }
  81
  82  int PyModuleRegistry::handle_command(
  83 From 067adbf9a032b5de793fd0b41b071f24f075270a Mon Sep 17 00:00:00 2001
  84 From: Kefu Chai <kchai@redhat.com>
  85 Date: Thu, 30 Apr 2020 11:34:07 +0800
  86 Subject: [PATCH] mgr: do not load disabled modules
  87
  88 an option named "mgr_disabled_modules" is added in this change to
  89 prevent mgr from loading modules listed in this option. because mgr
  90 loads *all* modules found in the configured path, and per
  91 https://tracker.ceph.com/issues/45147, python subinterpreter could hang
  92 when loading numpy, so this behavior practically creates a deadlock
  93 in mgr.
  94
  95 this issue is found when mgr uses python3.8 runtime. in development
  96 environment, it'd be inconvenient to disable the offending mgr module
  97 without changing the source code, even if we can choose to not install
  98 them, for instance, the enduser can workaround this issue by
  99 uninstalling `ceph-mgr-diskprediction-local`.
 100
 101 an option would be useful in this case, so we can add the module to the
 102 list before mgr tries to load it.
 103
 104 as this issue is found with python3.8 + diskprediction_local (numpy), so
 105 this mgr module is disabled by default if mgr is compiled with python3.8
 106 runtime.
 107
 108 Fixes: https://tracker.ceph.com/issues/45147
 109 Signed-off-by: Kefu Chai <kchai@redhat.com>
 110 ---
 111  CMakeLists.txt                |  5 +++++
 112  src/common/options.cc         | 14 ++++++++++++++
 113  src/include/config-h.in.cmake |  3 +++
 114  src/mgr/PyModuleRegistry.cc   | 11 ++++++++++-
 115  4 files changed, 32 insertions(+), 1 deletion(-)
 116
 117 diff --git a/CMakeLists.txt b/CMakeLists.txt
 118 index 0f7e86414c2d2..fa00d1316bcc0 100644
 119 --- a/CMakeLists.txt
 120 +++ b/CMakeLists.txt
 121 @@ -442,6 +442,11 @@ if(WITH_MGR)
 122    set(MGR_PYTHON_LIBRARIES ${Python3_LIBRARIES})
 123    set(MGR_PYTHON_VERSION_MAJOR ${Python3_VERSION_MAJOR})
 124    set(MGR_PYTHON_VERSION_MINOR ${Python3_VERSION_MINOR})
 125 +  # https://tracker.ceph.com/issues/45147
 126 +  if(Python3_VERSION VERSION_GREATER_EQUAL 3.8)
 127 +    set(MGR_DISABLED_MODULES "diskprediction_local")
 128 +    message(STATUS "mgr module disabled for ${Python3_VERSION}: ${MGR_DISABLED_MODULES}")
 129 +  endif()
 130    # Boost dependency check deferred to Boost section
 131  endif(WITH_MGR)
 132
 133 diff --git a/src/common/options.cc b/src/common/options.cc
 134 index be1e955ab51ea..c78d9b69d7591 100644
 135 --- a/src/common/options.cc
 136 +++ b/src/common/options.cc
 137 @@ -5169,6 +5169,20 @@ std::vector<Option> get_global_options() {
 138      .add_service("mgr")
 139      .set_description("Filesystem path to manager modules."),
 140
 141 +    Option("mgr_disabled_modules", Option::TYPE_STR, Option::LEVEL_ADVANCED)
 142 +#ifdef MGR_DISABLED_MODULES
 143 +    .set_default(MGR_DISABLED_MODULES)
 144 +#endif
 145 +    .set_flag(Option::FLAG_STARTUP)
 146 +    .add_service("mgr")
 147 +    .set_description("List of manager modules never get loaded")
 148 +    .set_long_description("A comma delimited list of module names. This list "
 149 +        "is read by manager when it starts. By default, manager loads all "
 150 +        "modules found in specified 'mgr_module_path', and it starts the "
 151 +        "enabled ones as instructed. The modules in this list will not be "
 152 +        "loaded at all.")
 153 +    .add_see_also("mgr_module_path"),
 154 +
 155      Option("mgr_initial_modules", Option::TYPE_STR, Option::LEVEL_BASIC)
 156      .set_default("restful iostat")
 157      .set_flag(Option::FLAG_NO_MON_UPDATE)
 158 diff --git a/src/include/config-h.in.cmake b/src/include/config-h.in.cmake
 159 index dc213938f5c9c..ea550c81e3a0e 100644
 160 --- a/src/include/config-h.in.cmake
 161 +++ b/src/include/config-h.in.cmake
 162 @@ -309,6 +309,9 @@
 163
 164  #cmakedefine MGR_PYTHON_EXECUTABLE "@MGR_PYTHON_EXECUTABLE@"
 165
 166 +/* the default value of "mgr_disabled_module" option */
 167 +#cmakedefine MGR_DISABLED_MODULES "@MGR_DISABLED_MODULES@"
 168 +
 169  /* Define to 1 if you have the `getprogname' function. */
 170  #cmakedefine HAVE_GETPROGNAME 1
 171
 172 diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc
 173 index 466da5d404964..2e2e080aa76c0 100644
 174 --- a/src/mgr/PyModuleRegistry.cc
 175 +++ b/src/mgr/PyModuleRegistry.cc
 176 @@ -25,6 +25,7 @@ namespace fs = std::experimental::filesystem;
 177
 178  #include "include/stringify.h"
 179  #include "common/errno.h"
 180 +#include "common/split.h"
 181
 182  #include "BaseMgrModule.h"
 183  #include "PyOSDMap.h"
 184 @@ -267,14 +268,22 @@ void PyModuleRegistry::shutdown()
 185
 186  std::set<std::string> PyModuleRegistry::probe_modules(const std::string &path) const
 187  {
 188 +  const auto opt = g_conf().get_val<std::string>("mgr_disabled_modules");
 189 +  const auto disabled_modules = ceph::split(opt);
 190 +
 191    std::set<std::string> modules;
 192    for (const auto& entry: fs::directory_iterator(path)) {
 193      if (!fs::is_directory(entry)) {
 194        continue;
 195      }
 196 +    const std::string name = entry.path().filename();
 197 +    if (std::count(disabled_modules.begin(), disabled_modules.end(), name)) {
 198 +      dout(10) << "ignoring disabled module " << name << dendl;
 199 +      continue;
 200 +    }
 201      auto module_path = entry.path() / "module.py";
 202      if (fs::exists(module_path)) {
 203 -      modules.emplace(entry.path().filename());
 204 +      modules.emplace(name);
 205      }
 206    }
 207    return modules;