rgw/lua: install lua packages in temp directory

and switch to it only once installation is done.
this is needed for cases where installation can happen
while RGW is running

Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>

diff --git a/src/common/options/rgw.yaml.in b/src/common/options/rgw.yaml.in
index dad1906a72cd219ce0dddfd7399b7669e62f0ae6..3971929e412dfc219c4f103228f38fb544b5a0ef 100644 (file)
--- a/src/common/options/rgw.yaml.in
+++ b/src/common/options/rgw.yaml.in
@@ -3755,7 +3755,7 @@ options:
   type: str
   level: advanced
   desc: Directory where luarocks install packages from allowlist
-  default: @rgw_luarocks_location@
+  default: /tmp/rgw_luarocks/$name
   services:
   - rgw
   flags:

diff --git a/src/rgw/rgw_appmain.cc b/src/rgw/rgw_appmain.cc
index ebad686e354fe89aff98582390947e188dc04cfd..5eaf03e5793e91dbd6c8e040a96b882ef4520bad 100644 (file)
--- a/src/rgw/rgw_appmain.cc
+++ b/src/rgw/rgw_appmain.cc
@@ -551,23 +551,15 @@ void rgw::AppMain::init_lua()
   rgw::sal::Driver* driver = env.driver;
   int r{0};
   std::string path = g_conf().get_val<std::string>("rgw_luarocks_location");
-  if (!path.empty()) {
-    path += "/" + g_conf()->name.to_str();
-  }
-  env.lua.luarocks_path = path;
 
 #ifdef WITH_RADOSGW_LUA_PACKAGES
   rgw::lua::packages_t failed_packages;
-  std::string output;
   r = rgw::lua::install_packages(dpp, driver, null_yield, path,
-                                 failed_packages, output);
+                                 failed_packages, env.lua.luarocks_path);
   if (r < 0) {
     dout(1) << "WARNING: failed to install lua packages from allowlist. error: " << r
             << dendl;
   }
-  if (!output.empty()) {
-    dout(10) << "INFO: lua packages installation output: \n" << output << dendl;
-  }
   for (const auto &p : failed_packages) {
     dout(5) << "WARNING: failed to install lua package: " << p
             << " from allowlist" << dendl;

diff --git a/src/rgw/rgw_lua.cc b/src/rgw/rgw_lua.cc
index 8e28b7d9081af92faddac902e4795384ee3eb579..505c5fc25cd61a58a6ceda263a52d0a50f715e2d 100644 (file)
--- a/src/rgw/rgw_lua.cc
+++ b/src/rgw/rgw_lua.cc
@@ -154,19 +154,34 @@ int list_packages(const DoutPrefixProvider *dpp, rgw::sal::Driver* driver, optio
   return lua_mgr->list_packages(dpp, y, packages);
 }
 
-int install_packages(const DoutPrefixProvider *dpp, rgw::sal::Driver* driver,
-                     optional_yield y, const std::string& luarocks_path,
-                     packages_t& failed_packages, std::string& output) {
-  // luarocks directory cleanup
+namespace fs = std::filesystem;
+
+// similar to to the "mkdir -p" command
+int create_directory_p(const DoutPrefixProvider *dpp, const fs::path& p) {
   std::error_code ec;
-  if (std::filesystem::remove_all(luarocks_path, ec)
-      == static_cast<std::uintmax_t>(-1) &&
-      ec != std::errc::no_such_file_or_directory) {
-    output.append("failed to clear luarocks directory: ");
-    output.append(ec.message());
-    output.append("\n");
-    return ec.value();
+  fs::path total_path;
+  for (const auto& pp : p) {
+    total_path /= pp;
+    auto should_create = !fs::exists(total_path, ec);
+    if (ec) {
+      ldpp_dout(dpp, 1) << "cannot check if " << total_path << 
+        " directory exists. error: " << ec.message() << dendl;
+      return -ec.value();
+    }
+    if (should_create) {
+      if (!create_directory(total_path, ec)) {
+        ldpp_dout(dpp, 1) << "failed to create  " << total_path << 
+          " directory. error: " << ec.message() << dendl;
+        return -ec.value();
+      }
+    }
   }
+  return 0;
+}
+
+int install_packages(const DoutPrefixProvider *dpp, rgw::sal::Driver* driver,
+                     optional_yield y, const std::string& luarocks_path,
+                     packages_t& failed_packages, std::string& install_dir) {
 
   packages_t packages;
   auto ret = list_packages(dpp, driver, y, packages);
@@ -175,29 +190,50 @@ int install_packages(const DoutPrefixProvider *dpp, rgw::sal::Driver* driver,
     return 0;
   }
   if (ret < 0) {
-    output.append("failed to get lua package list");
+    ldpp_dout(dpp, 1) << "Lua ERROR: failed to get package list. error: " << ret << dendl;
     return ret;
   }
   // verify that luarocks exists
   const auto p = bp::search_path("luarocks");
   if (p.empty()) {
-    output.append("failed to find luarocks");
+    ldpp_dout(dpp, 1) << "Lua ERROR: failed to find luarocks" << dendl;
     return -ECHILD;
   }
 
+  // create the luarocks parent directory
+  auto rc = create_directory_p(dpp, luarocks_path);
+  if (rc < 0) {
+    ldpp_dout(dpp, 1) << "Lua ERROR: failed to recreate luarocks directory: " <<
+      luarocks_path << ". error: " << rc << dendl; 
+    return rc;
+  }
+
+  // create a temporary sub-directory to install all luarocks packages
+  std::string tmp_path_template = luarocks_path;// fs::temp_directory_path();
+  tmp_path_template.append("/XXXXXX");
+  const auto tmp_luarocks_path = mkdtemp(tmp_path_template.data());
+  if (!tmp_luarocks_path) { 
+    const auto rc = -errno;
+    ldpp_dout(dpp, 1) << "Lua ERROR: failed to create temporary directory from template: " << 
+      tmp_path_template << ". error: " << rc << dendl;
+    return rc;
+  }
+  install_dir.assign(tmp_luarocks_path);
+
   // the lua rocks install dir will be created by luarocks the first time it is called
   for (const auto& package : packages) {
     bp::ipstream is;
-    const auto cmd = p.string() + " install --lua-version " + CEPH_LUA_VERSION + " --tree " + luarocks_path + " --deps-mode one " + package;
+    const auto cmd = p.string() + " install --lua-version " + CEPH_LUA_VERSION + " --tree " + install_dir + " --deps-mode one " + package;
     bp::child c(cmd, bp::std_in.close(), (bp::std_err & bp::std_out) > is);
 
     // once package reload is supported, code should yield when reading output
-    std::string line = std::string("CMD: ") + cmd;
+    std::string lines = std::string("Lua CMD: ") + cmd;
+    std::string line;
 
     do {
       if (!line.empty()) {
-        output.append(line);
-        output.append("\n");
+        lines.append("\n\t");
+        lines.append(line);
       }
     } while (c.running() && std::getline(is, line));
 
@@ -205,12 +241,38 @@ int install_packages(const DoutPrefixProvider *dpp, rgw::sal::Driver* driver,
     if (c.exit_code()) {
       failed_packages.insert(package);
     }
+    ldpp_dout(dpp, 20) << lines << dendl;
   }
+  
+  // luarocks directory cleanup
+  /*std::error_code ec;
+  if (fs::remove_all(luarocks_path, ec)
+      == static_cast<std::uintmax_t>(-1) &&
+      ec != std::errc::no_such_file_or_directory) {
+    ldpp_dout(dpp, 1) << "Lua ERROR: failed to clear luarocks directory: " <<
+      luarocks_path << ". error: " << ec.message() << dendl; 
+    return -ec.value();
+  }*/
+  /*auto rc = create_directory_p(dpp, luarocks_path);
+  if (rc < 0) {
+    ldpp_dout(dpp, 1) << "Lua ERROR: failed to recreate luarocks directory: " <<
+      luarocks_path << ". error: " << rc << dendl; 
+    return rc;
+  }*/
+
+  // switch temporary install directory to luarocks one
+  /*fs::rename(tmp_luarocks_path, luarocks_path, ec);
+  if (ec) {
+    ldpp_dout(dpp, 1) << "Lua ERROR: failed to switch between temp directory: " <<
+      tmp_luarocks_path << " and luarocks directory: " << luarocks_path << 
+      " . error: " << ec.message() << dendl; 
+    return -ec.value();
+  }*/
 
   return 0;
 }
 
-#endif
+#endif // WITH_RADOSGW_LUA_PACKAGES
 
 }
 

diff --git a/src/rgw/rgw_lua.h b/src/rgw/rgw_lua.h
index a6ebcc2d0e1153d586af5db01cdbc8274a671f55..e8962aba6c44d614a11316cac2559c5c7a99e64b 100644 (file)
--- a/src/rgw/rgw_lua.h
+++ b/src/rgw/rgw_lua.h
@@ -58,10 +58,12 @@ int remove_package(const DoutPrefixProvider *dpp, rgw::sal::Driver* driver, opti
 int list_packages(const DoutPrefixProvider *dpp, rgw::sal::Driver* driver, optional_yield y, packages_t& packages);
 
 // install all packages from the allowlist
-// return the list of packages that failed to install and the output of the install command
+// return (by reference) the list of packages that failed to install
+// and return (by reference) the temporary director in which the packages were installed
 int install_packages(const DoutPrefixProvider *dpp, rgw::sal::Driver* driver,
                      optional_yield y, const std::string& luarocks_path,
-                     packages_t& failed_packages, std::string& output);
+                     packages_t& failed_packages,
+                     std::string& install_dir);
 #endif
 }
 

diff --git a/src/vstart.sh b/src/vstart.sh
index a33bcde3234bd10aaaa105e6abdca26268d1e04e..634fb74ac874daaca1ff6d1131b72501476d96e8 100755 (executable)
--- a/src/vstart.sh
+++ b/src/vstart.sh
@@ -1796,7 +1796,7 @@ do_rgw()
             --log-file=${CEPH_OUT_DIR}/radosgw.${current_port}.log \
             --admin-socket=${CEPH_OUT_DIR}/radosgw.${current_port}.asok \
             --pid-file=${CEPH_OUT_DIR}/radosgw.${current_port}.pid \
-            --rgw_luarocks_location=${CEPH_OUT_DIR}/luarocks \
+            --rgw_luarocks_location=${CEPH_OUT_DIR}/radosgw.${current_port}.luarocks  \
             ${RGWDEBUG} \
             -n ${rgw_name} \
             "--rgw_frontends=${rgw_frontend} port=${current_port}${CEPH_RGW_HTTPS}${flight_conf:+,arrow_flight}"