From c217fba4937f29ea035ea6f9c2829490efc02381 Mon Sep 17 00:00:00 2001 From: Yuval Lifshitz Date: Tue, 4 Jul 2023 16:27:38 +0000 Subject: [PATCH] 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 --- src/common/options/rgw.yaml.in | 2 +- src/rgw/rgw_appmain.cc | 10 +--- src/rgw/rgw_lua.cc | 98 +++++++++++++++++++++++++++------- src/rgw/rgw_lua.h | 6 ++- src/vstart.sh | 2 +- 5 files changed, 87 insertions(+), 31 deletions(-) diff --git a/src/common/options/rgw.yaml.in b/src/common/options/rgw.yaml.in index dad1906a72cd2..3971929e412df 100644 --- 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 ebad686e354fe..5eaf03e5793e9 100644 --- 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("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 8e28b7d9081af..505c5fc25cd61 100644 --- 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(-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(-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 a6ebcc2d0e115..e8962aba6c44d 100644 --- 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 a33bcde3234bd..634fb74ac874d 100755 --- 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}" -- 2.39.5