From 42104bb5d631023e32e0c6c909b3d9e9368e9e39 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Thu, 4 Jun 2020 16:15:56 +0100 Subject: [PATCH] Add new API / TSPluginDSOReloadEnable that overrides the configuration variable `proxy.config.plugin.dynamic_reload_mode` for a particular plugin. --- .../functions/TSPluginDSOReloadEnable.en.rst | 51 ++++++ include/ts/ts.h | 14 ++ proxy/http/remap/PluginDso.cc | 49 +++++- proxy/http/remap/PluginDso.h | 31 ++++ proxy/http/remap/PluginFactory.cc | 7 + proxy/http/remap/PluginFactory.h | 9 + .../remap/unit-tests/test_PluginFactory.cc | 164 +++++++++++++++++- src/traffic_server/InkAPI.cc | 17 ++ 8 files changed, 336 insertions(+), 6 deletions(-) create mode 100644 doc/developer-guide/api/functions/TSPluginDSOReloadEnable.en.rst diff --git a/doc/developer-guide/api/functions/TSPluginDSOReloadEnable.en.rst b/doc/developer-guide/api/functions/TSPluginDSOReloadEnable.en.rst new file mode 100644 index 00000000000..2f10e813e99 --- /dev/null +++ b/doc/developer-guide/api/functions/TSPluginDSOReloadEnable.en.rst @@ -0,0 +1,51 @@ +.. Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +.. include:: ../../../common.defs + +.. default-domain:: c + +TSPluginDSOReloadEnable +************************* + +Control whether this plugin will take part in the remap dynamic reload preocess (remap.config) + +Synopsis +======== + +.. code-block:: cpp + + #include + +.. function:: TSReturnCode TSPluginDSOReloadEnable(int enabled) + +Description +=========== + +This function provides the ability to enable/disable programmatically the plugin +dynamic reloading when the same Dynamic Shared Object (DSO) is also used as a remap plugin. +This overrides :ts:cv:`proxy.config.plugin.dynamic_reload_mode`. + +.. warning:: This function should be called from within :func:`TSPluginInit` + +The function will return :type:`TS_ERROR` in any of the following cases: + - The function was not called from within :func:`TSPluginInit` + - TS is unable to get the canonical path from the plugin's path. + +See Also +======== + +:manpage:`TSAPI(3ts)` diff --git a/include/ts/ts.h b/include/ts/ts.h index 21c3ec6be30..535f3bada61 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -162,6 +162,20 @@ int TSTrafficServerVersionGetPatch(void); */ tsapi TSReturnCode TSPluginRegister(const TSPluginRegistrationInfo *plugin_info); +/** + This function provides the ability to enable/disable programmatically + the plugin dynamic reloading when the same Dynamic Shared Object (DSO) + is also used as a remap plugin. This overrides `proxy.config.plugin.dynamic_reload_mode` + configuration variable. + + @param enabled boolean flag. 0/false will disable the reload on the caller plugin. + @return TS_ERROR if the function is not called from within TSPluginInit or if TS is + unable to get the canonical path from the plugin's path. TS_SUCCESS otherwise. + + @note This function should be called from within TSPluginInit + */ +tsapi TSReturnCode TSPluginDSOReloadEnable(int enabled); + /* -------------------------------------------------------------------------- Files */ /** diff --git a/proxy/http/remap/PluginDso.cc b/proxy/http/remap/PluginDso.cc index 778adab2c2c..76e302f215a 100644 --- a/proxy/http/remap/PluginDso.cc +++ b/proxy/http/remap/PluginDso.cc @@ -308,7 +308,7 @@ PluginDso::LoadedPlugins::remove(PluginDso *plugin) } /* check if need to reload the plugin DSO - * if dynamic reload not enabled: check if plugin Dso with same effective path aready loaded + * if dynamic reload not enabled: check if plugin Dso with same effective path already loaded * if dynamic reload enabled: check if plugin Dso with same effective path and same time stamp already loaded * return pointer to already loaded plugin if found, else return null */ @@ -361,4 +361,51 @@ PluginDso::LoadedPlugins::indicatePostReload(bool reloadSuccessful, const std::u } } +bool +PluginDso::LoadedPlugins::addPluginPathToDsoOptOutTable(std::string_view pluginPath) +{ + std::error_code ec; + auto effectivePath = fs::canonical(fs::path{pluginPath}, ec); + + if (ec) { + PluginError("Error getting the canonical path: %s", ec.message().c_str()); + return false; + } + + { + SCOPED_MUTEX_LOCK(lock, _mutex, this_ethread()); + _optoutDsoReloadPlugins.push_front(DisableDSOReloadPluginInfo{effectivePath}); + } + + return true; +} + +void +PluginDso::LoadedPlugins::removePluginPathFromDsoOptOutTable(std::string_view pluginPath) +{ + std::error_code ec; + auto effectivePath = fs::canonical(fs::path{pluginPath}, ec); + + if (ec) { + PluginError("Error getting the canonical path: %s", ec.message().c_str()); + return; + } + + { + SCOPED_MUTEX_LOCK(lock, _mutex, this_ethread()); + _optoutDsoReloadPlugins.remove_if([&effectivePath](auto const &info) { return info.dsoEffectivePath == effectivePath; }); + } +} + +bool +PluginDso::LoadedPlugins::isPluginInDsoOptOutTable(const fs::path &effectivePath) +{ + SCOPED_MUTEX_LOCK(lock, _mutex, this_ethread()); + const auto found = std::find_if(std::begin(this->_optoutDsoReloadPlugins), std::end(this->_optoutDsoReloadPlugins), + [&effectivePath](const auto &info) { return info.dsoEffectivePath == effectivePath; }); + + // if is found, then the plugin opt out. + return (found != std::end(this->_optoutDsoReloadPlugins)); +} + Ptr PluginDso::_plugins; diff --git a/proxy/http/remap/PluginDso.h b/proxy/http/remap/PluginDso.h index be43803499d..9e6df2d1a74 100644 --- a/proxy/http/remap/PluginDso.h +++ b/proxy/http/remap/PluginDso.h @@ -32,6 +32,7 @@ #include #include #include +#include #include #include "ts/apidefs.h" @@ -104,9 +105,39 @@ class PluginDso : public PluginThreadContext void indicatePreReload(const char *factoryId); void indicatePostReload(bool reloadSuccessful, const std::unordered_map &pluginUsed, const char *factoryId); + /** + * @brief Check if the opt out table contains the passed plugin's effective path. + * @return true If the plugin was marked to not participate in the dynamic reload by + * TSPluginDSOReloadEnable() API. + * @param effectivePath canonical value of the plugin's path. + */ + bool isPluginInDsoOptOutTable(const fs::path &effectivePath); + /** + * @brief Add the plugin's path to the opt out table in order to let the Plugin Factory + * that this plugin is not interested in taking part of the dynamic reloading. + * This function will store the plugin's canonical path. + * @param effectivePath Plugin's path. + * @return false if any errors converting the plugin's path to a canonical path, true otherwise. + */ + bool addPluginPathToDsoOptOutTable(std::string_view pluginPath); + /** + * @brief Removes the passed plugin's effective path from the opt out list. + * @note This is mostly used by unit test than needs to remove the plugin's effectivePath + * from the out out list. + * @param pluginPath plugin's path. + * @note This function is mainly for unit tests purposes. + */ + void removePluginPathFromDsoOptOutTable(std::string_view pluginPath); + private: PluginList _list; /** @brief plugin list */ Ptr _mutex; /** @brief mutex used when updating the plugin list from multiple threads */ + + struct DisableDSOReloadPluginInfo { + fs::path dsoEffectivePath; + }; + + std::forward_list _optoutDsoReloadPlugins; }; static const Ptr & diff --git a/proxy/http/remap/PluginFactory.cc b/proxy/http/remap/PluginFactory.cc index 0cb5dfbc623..1837ddd7411 100644 --- a/proxy/http/remap/PluginFactory.cc +++ b/proxy/http/remap/PluginFactory.cc @@ -152,6 +152,13 @@ PluginFactory::getRemapPlugin(const fs::path &configPath, int argc, char **argv, return nullptr; } + // The plugin may have opt out by `TSPluginDSOReloadEnable`, let's check and overwrite + if (dynamicReloadEnabled && PluginDso::loadedPlugins()->isPluginInDsoOptOutTable(effectivePath)) { + // plugin not interested to be reload. + PluginDebug(_tag, "Plugin %s not interested in taking part of the reload.", effectivePath.c_str()); + dynamicReloadEnabled = false; + } + /* Only one plugin with this effective path can be loaded by a plugin factory */ RemapPluginInfo *plugin = dynamic_cast(findByEffectivePath(effectivePath, dynamicReloadEnabled)); RemapPluginInst *inst = nullptr; diff --git a/proxy/http/remap/PluginFactory.h b/proxy/http/remap/PluginFactory.h index 4f6ee6d3589..6b5a2f8bf27 100644 --- a/proxy/http/remap/PluginFactory.h +++ b/proxy/http/remap/PluginFactory.h @@ -82,6 +82,8 @@ class RemapPluginInst * filesystem links and different dl library implementations. * * @note This is meant to unify the way global and remap plugins are (re)loaded (global plugin support is not implemented yet). + * @note In the case of a mixed plugin, getRemapPlugin/dynamicReloadEnabled can be internally overwritten if the plugin opt out to + * take part in the dynamic reload process. */ class PluginFactory { @@ -116,5 +118,12 @@ class PluginFactory std::error_code _ec; bool _preventiveCleaning = true; + // Hold the full path for global plugins not taking part of the dynamic reloading. + struct DisableReloadPluginInfo { + fs::path fullPath; + }; + + std::forward_list optoutPlugins; + static constexpr const char *const _tag = "plugin_factory"; /** @brief log tag used by this class */ }; diff --git a/proxy/http/remap/unit-tests/test_PluginFactory.cc b/proxy/http/remap/unit-tests/test_PluginFactory.cc index 8623df0b5a6..c2ceca1f13f 100644 --- a/proxy/http/remap/unit-tests/test_PluginFactory.cc +++ b/proxy/http/remap/unit-tests/test_PluginFactory.cc @@ -298,7 +298,6 @@ SCENARIO("loading plugins", "[plugin][core]") { fs::path configPath = searchDir / "subdir" / pluginName; CHECK(configPath.is_absolute()); /* make sure this is absolute path - this is what we are testing */ - setupConfigPathTest(configPath, buildPath, tempComponent, effectivePath, runtimePath); PluginFactoryUnitTest *factory = getFactory(tempComponent); RemapPluginInst *plugin = factory->getRemapPlugin(configPath, 0, nullptr, error, isPluginDynamicReloadEnabled()); @@ -321,7 +320,6 @@ SCENARIO("loading plugins", "[plugin][core]") disablePluginDynamicReload(); fs::path configPath = searchDir / "subdir" / pluginName; CHECK(configPath.is_absolute()); /* make sure this is absolute path - this is what we are testing */ - setupConfigPathTest(configPath, buildPath, tempComponent, effectivePath, runtimePath); PluginFactoryUnitTest *factory = getFactory(tempComponent); RemapPluginInst *plugin = factory->getRemapPlugin(configPath, 0, nullptr, error, isPluginDynamicReloadEnabled()); @@ -340,6 +338,39 @@ SCENARIO("loading plugins", "[plugin][core]") enablePluginDynamicReload(); } + WHEN("config is using plugin absolute path - dynamic reload is ENABLED but overwritten by optout") + { + enablePluginDynamicReload(); + fs::path configPath = searchDir / "subdir" / pluginName; + CHECK(configPath.is_absolute()); /* make sure this is absolute path - this is what we are testing */ + + // We have to force the path to be the effective == runtime + disablePluginDynamicReload(); + setupConfigPathTest(configPath, buildPath, tempComponent, effectivePath, runtimePath); + enablePluginDynamicReload(); + + PluginFactoryUnitTest *factory = getFactory(tempComponent); + // make the factory take this plugin as it opt out. + PluginDso::loadedPlugins()->addPluginPathToDsoOptOutTable(effectivePath.string()); + + RemapPluginInst *plugin = factory->getRemapPlugin(configPath, 0, nullptr, error, isPluginDynamicReloadEnabled()); + + THEN("expect it to successfully load") + { + validateSuccessfulConfigPathTest(plugin, error, effectivePath, runtimePath); + static const bool disableDynamicReloadByOptOut{false}; + CHECK(nullptr != PluginDso::loadedPlugins()->findByEffectivePath(effectivePath, disableDynamicReloadByOptOut)); + + // check Dso still exists + CHECK(plugin->_plugin.effectivePath() == plugin->_plugin.runtimePath()); + CHECK(fs::exists(plugin->_plugin.effectivePath())); + } + // we need to remove this from the list so next tests can work as expected. + PluginDso::loadedPlugins()->removePluginPathFromDsoOptOutTable(effectivePath.string()); + teardownConfigPathTest(factory); + enablePluginDynamicReload(); + } + WHEN("config using nonexisting relative plugin file name") { fs::path relativeExistingPath = pluginName; @@ -587,7 +618,7 @@ checkTwoLoadedVersionsDifferent(const RemapPluginInst *plugin_v1, const RemapPlu } void -checkTwoLoadedVersionsSame(RemapPluginInst *plugin_v1, RemapPluginInst *plugin_v2) +checkTwoLoadedVersionsSame(RemapPluginInst *plugin_v1, RemapPluginInst *plugin_v2, bool pluginOptOut = false) { void *tsRemapInitSym_v1_t2 = nullptr; /* callback address from DSO v1 at moment t2 */ void *tsRemapInitSym_v2_t2 = nullptr; /* callback address from DSO v2 at moment t2 */ @@ -610,9 +641,9 @@ checkTwoLoadedVersionsSame(RemapPluginInst *plugin_v1, RemapPluginInst *plugin_v CHECK(tsRemapInitSym_v1_t2 == tsRemapInitSym_v2_t2); // 2 versions can be same even when dynamic reload is enabled - // 2 versions must be same when dynamic reload is disabled + // 2 versions must be same when dynamic reload is disabled or the plugin opt out // check presence/absence of Dso files - if (!isPluginDynamicReloadEnabled()) { + if (pluginOptOut || !isPluginDynamicReloadEnabled()) { CHECK(plugin_v1->_plugin.effectivePath() == plugin_v1->_plugin.runtimePath()); CHECK(fs::exists(plugin_v1->_plugin.effectivePath())); } else { @@ -634,6 +665,27 @@ testSetupLoadPlugin(const fs::path &configName, const fs::path &buildPath, const return {pluginInst, factory}; } +std::tuple +testSetupLoadPluginWithOptOut(const fs::path &configName, const fs::path &buildPath, const fs::path &uuid, const time_t mtime, + fs::path &effectivePath, fs::path &runtimePath) +{ + std::string error; + + // force paths to be as dynamic disabled which will be the case for a plugin that opt out. + disablePluginDynamicReload(); + setupConfigPathTest(configName, buildPath, uuid, effectivePath, runtimePath, mtime); + enablePluginDynamicReload(); + + auto factory = getFactory(uuid); + + // Set factory's opt out plugin information. + PluginDso::loadedPlugins()->addPluginPathToDsoOptOutTable(effectivePath.string()); + auto pluginInst = factory->getRemapPlugin(configName, 0, nullptr, error, isPluginDynamicReloadEnabled()); + // make sure we remove it so there is no evidence of this in the Plugin's list. + PluginDso::loadedPlugins()->removePluginPathFromDsoOptOutTable(effectivePath.string()); + return {pluginInst, factory}; +} + SCENARIO("loading multiple version of the same plugin at the same time", "[plugin][core]") { REQUIRE_FALSE(sandboxDir.empty()); @@ -778,6 +830,50 @@ SCENARIO("loading multiple version of the same plugin at the same time", "[plugi } } + GIVEN("two different versions v1 and v2 of same plugin with different time stamps - dynamic plugin reload ENABLED") + { + WHEN("(1) loading v1, (2) overwriting with v2 and then (3) reloading by using the same plugin name, " + "(*) v1 and v2 DSOs modification time are different (changed)") + { + enablePluginDynamicReload(); + + /* Simulate installing plugin plugin_v1.so (ver 1) as plugin.so and loading it at some point of time t1 */ + auto [plugin_v1, factory1] = + testSetupLoadPluginWithOptOut(configName, buildPath_v1, uuid_t1, 1556825556, effectivePath_v1, runtimePath_v1); + + plugin_v1->_plugin.getSymbol("TSRemapInit", tsRemapInitSym_v1_t1, error); + + /* Simulate installing plugin plugin_v2.so (v1) as plugin.so and loading it at some point of time t2 */ + /* Note that during the installation plugin_v2.so (v2) is "barberically" overriding the existing plugin.so which was v1 */ + auto [plugin_v2, factory2] = + testSetupLoadPluginWithOptOut(configName, buildPath_v2, uuid_t2, 1556825557, effectivePath_v2, runtimePath_v2); + plugin_v1->_plugin.getSymbol("TSRemapInit", tsRemapInitSym_v1_t2, error); + + /* Make sure plugin.so was overriden */ + CHECK(effectivePath_v1 == effectivePath_v2); + + /* since dynamic reload is disabled, runtimepaths should be same */ + CHECK(runtimePath_v1 == runtimePath_v2); + + THEN("expect v1 plugin to remain loaded since dynamic reload is disabled, even though the timestamp has changed") + { + /* Both getRemapPlugin() calls should succeed but only v1 plugin DSO should be used */ + validateSuccessfulConfigPathTest(plugin_v1, error1, effectivePath_v1, runtimePath_v1); + validateSuccessfulConfigPathTest(plugin_v2, error2, effectivePath_v2, runtimePath_v1); + + const bool pluginOptOut{true}; + checkTwoLoadedVersionsSame(plugin_v1, plugin_v2, pluginOptOut); + + /* Make sure v1 callback functions addresses did not change for v1 after v2 was loaded */ + CHECK(tsRemapInitSym_v1_t1 == tsRemapInitSym_v1_t2); + } + + teardownConfigPathTest(factory1); + teardownConfigPathTest(factory2); + enablePluginDynamicReload(); + } + } + GIVEN("two different versions v1 and v2 of same plugin with same time stamp - dynamic plugin reload DISABLED") { WHEN("(1) loading v1, (2) overwriting with v2 and then (3) reloading by using the same plugin name, " @@ -923,6 +1019,64 @@ SCENARIO("loading multiple version of the same plugin in mixed mode - global as } } + GIVEN("two different versions v1 and v2 of same plugin in mixed mode - dynamic plugin reload ENABLED but overwritten by opt out") + { + WHEN("(1) loading v1, (2) overwriting with v2 and then (3) reloading by using the same plugin name, " + "(*) v1 and v2 DSOs modification time are different (changed)") + { + // just to get the proper path's + disablePluginDynamicReload(); + /* Simulate installing plugin plugin_v1.so (ver 1) as plugin.so and loading it at some point of time t1 */ + setupConfigPathTest(configName, buildPath_v1, uuid_t1, effectivePath_v1, runtimePath_v1, 1556825556); + + GlobalPluginInfo global_plugin_v1; + CHECK(global_plugin_v1.loadDso(effectivePath_v1) == true); + global_plugin_v1.getSymbol("TSRemapInit", tsRemapInitSym_v1_t1, error); + + /* Simulate installing plugin plugin_v2.so (v1) as plugin.so and loading it at some point of time t2 */ + /* Note that during the installation plugin_v2.so (v2) is "barberically" overriding the existing plugin.so which was v1 */ + setupConfigPathTest(configName, buildPath_v2, uuid_t2, effectivePath_v2, runtimePath_v2, 1556825557); + enablePluginDynamicReload(); + PluginFactoryUnitTest *factory = getFactory(uuid_t2); + + // Set factory's opt out plugin information. + PluginDso::loadedPlugins()->addPluginPathToDsoOptOutTable(effectivePath_v1.string()); + + RemapPluginInst *plugin_v2 = factory->getRemapPlugin(configName, 0, nullptr, error2, isPluginDynamicReloadEnabled()); + + /* Make sure plugin.so was overriden */ + CHECK(effectivePath_v1 == effectivePath_v2); + + /* since dynamic reload is disabled, runtimepaths should be same */ + CHECK(runtimePath_v1 == runtimePath_v2); + + THEN("expect v1 plugin to remain loaded since dynamic reload is disabled, even though the timestamp has changed") + { + validateSuccessfulConfigPathTest(plugin_v2, error2, effectivePath_v2, runtimePath_v2); + + /* Make sure we ended up with the same DSO object and runtime paths should be same - no new plugin was loaded */ + CHECK(global_plugin_v1.dlOpenHandle() == plugin_v2->_plugin.dlOpenHandle()); + + /* Make sure v2 DSO was NOT really loaded - both instances should return same v1 version */ + CHECK(1 == global_plugin_v1.getPluginVersion()); + CHECK(1 == getPluginVersion(plugin_v2->_plugin)); + + /* Make sure the symbols we get from the 2 loaded plugins yield the same callback function pointer */ + global_plugin_v1.getSymbol("TSRemapInit", tsRemapInitSym_v1_t2, error); + plugin_v2->_plugin.getSymbol("TSRemapInit", tsRemapInitSym_v2_t2, error); + CHECK(nullptr != tsRemapInitSym_v1_t2); + CHECK(nullptr != tsRemapInitSym_v2_t2); + CHECK(tsRemapInitSym_v1_t2 == tsRemapInitSym_v2_t2); + + /* check that the symbol we got originally is still valid */ + CHECK(tsRemapInitSym_v1_t1 == tsRemapInitSym_v1_t2); + } + PluginDso::loadedPlugins()->removePluginPathFromDsoOptOutTable(effectivePath_v1.string()); + teardownConfigPathTest(factory); + enablePluginDynamicReload(); + } + } + GIVEN("two different versions v1 and v2 of same plugin in mixed mode - dynamic plugin reload DISABLED") { WHEN("(1) loading v1, (2) overwriting with v2 and then (3) reloading by using the same plugin name, " diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index 50a6742cdd8..891b7b6d1a6 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -1966,6 +1966,23 @@ TSPluginRegister(const TSPluginRegistrationInfo *plugin_info) return TS_SUCCESS; } +TSReturnCode +TSPluginDSOReloadEnable(int enabled) +{ + TSReturnCode ret = TS_SUCCESS; + if (!plugin_reg_current) { + return TS_ERROR; + } + + if (!enabled) { + if (!PluginDso::loadedPlugins()->addPluginPathToDsoOptOutTable(plugin_reg_current->plugin_path)) { + ret = TS_ERROR; + } + } + + return ret; +} + //////////////////////////////////////////////////////////////////// // // API file management