From 0b8957d49caebe023c4a897985635006d137cc90 Mon Sep 17 00:00:00 2001 From: Kory Draughn Date: Mon, 21 Oct 2024 12:36:56 -0400 Subject: [PATCH] [?] Use setup and teardown hooks for REP static initialization. --- .../src/irods_rule_language.cpp | 24 ++++---- .../rule_engines/src/cpp_default_policy.cpp | 8 +++ plugins/rule_engines/src/passthrough.cpp | 60 +++++++------------ server/main_server/src/agent_main.cpp | 10 +++- server/main_server/src/main.cpp | 2 +- server/re/include/irods/irods_re_plugin.hpp | 48 ++++++++++++++- 6 files changed, 95 insertions(+), 57 deletions(-) diff --git a/plugins/rule_engines/irods_rule_language/src/irods_rule_language.cpp b/plugins/rule_engines/irods_rule_language/src/irods_rule_language.cpp index e7896c2598..9ee27f94e7 100644 --- a/plugins/rule_engines/irods_rule_language/src/irods_rule_language.cpp +++ b/plugins/rule_engines/irods_rule_language/src/irods_rule_language.cpp @@ -119,7 +119,7 @@ void initialize_microservice_table() extern Cache ruleEngineConfig; -irods::error start(irods::default_re_ctx&, const std::string& _instance_name) +irods::error setup(irods::default_re_ctx&, const std::string& _instance_name) { local_instance_name = _instance_name; @@ -178,11 +178,7 @@ irods::error start(irods::default_re_ctx&, const std::string& _instance_name) logger::rule_engine::error(msg); return ERROR(SYS_INVALID_INPUT_PARAM, msg); -} // start - -irods::error stop(irods::default_re_ctx& _u, const std::string& _instance_name) { - return SUCCESS(); -} +} // setup irods::error rule_exists(irods::default_re_ctx&, const std::string& _rn, bool& _ret) { if(ruleEngineConfig.ruleEngineStatus == UNINITIALIZED) { @@ -419,13 +415,17 @@ irods::error exec_rule_expression( extern "C" irods::pluggable_rule_engine* plugin_factory( const std::string& _inst_name, - const std::string& _context ) { - irods::pluggable_rule_engine* re = new irods::pluggable_rule_engine( _inst_name , _context); - re->add_operation( "start", - std::function( start ) ); + const std::string& _context ) +{ + const auto no_op = [](irods::default_re_ctx&, const std::string&) { return SUCCESS(); }; + + auto* re = new irods::pluggable_rule_engine( _inst_name , _context); + + re->add_operation("setup", std::function{setup}); + re->add_operation("teardown", std::function{no_op}); - re->add_operation( "stop", - std::function( stop ) ); + re->add_operation("start", std::function{no_op}); + re->add_operation("stop", std::function{no_op}); re->add_operation( "rule_exists", std::function( rule_exists ) ); diff --git a/plugins/rule_engines/src/cpp_default_policy.cpp b/plugins/rule_engines/src/cpp_default_policy.cpp index ba004efa22..b63f4dfc26 100644 --- a/plugins/rule_engines/src/cpp_default_policy.cpp +++ b/plugins/rule_engines/src/cpp_default_policy.cpp @@ -1171,6 +1171,14 @@ extern "C" irods::pluggable_rule_engine* plugin_factory( const std::string& _inst_name, const std::string& _context ) { irods::pluggable_rule_engine* re = new irods::pluggable_rule_engine( _inst_name , _context); + + const auto no_op = [](irods::default_re_ctx&, const std::string&) -> irods::error { + return SUCCESS(); + }; + + re->add_operation("setup", std::function{no_op}); + re->add_operation("teardown", std::function{no_op}); + re->add_operation( "start", std::function( start ) ); diff --git a/plugins/rule_engines/src/passthrough.cpp b/plugins/rule_engines/src/passthrough.cpp index cb9838c63d..d4cfa5a175 100644 --- a/plugins/rule_engines/src/passthrough.cpp +++ b/plugins/rule_engines/src/passthrough.cpp @@ -33,49 +33,24 @@ namespace int code; }; + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) std::unordered_map> pep_configs; + + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) int matched_code = 0; template using operation = std::function; - irods::error start(irods::default_re_ctx&, const std::string& _instance_name) + irods::error setup(irods::default_re_ctx&, const std::string& _instance_name) { - std::string config_path; - - if (auto error = irods::get_full_path_for_config_file("server_config.json", config_path); - !error.ok()) - { - std::string msg = "Server configuration not found [path => "; - msg += config_path; - msg += ']'; - - // clang-format off - log::rule_engine::error({{"rule_engine_plugin", "passthrough"}, - {"rule_engine_plugin_function", __func__}, - {"log_message", msg}}); - // clang-format on - - return ERROR(SYS_CONFIG_FILE_ERR, msg.c_str()); - } - - // clang-format off - log::rule_engine::trace({{"rule_engine_plugin", "passthrough"}, - {"rule_engine_plugin_function", __func__}, - {"log_message", "Reading plugin configuration ..."}}); - // clang-format on - - nlohmann::json config; - - { - std::ifstream config_file{config_path}; - config_file >> config; - } - try { + const auto config_handle = irods::server_properties::server_properties::instance().map(); + const auto& config = config_handle.get_json(); + // Iterate over the list of rule engine plugins until the Passthrough REP is found. for (const auto& re : config.at(irods::KW_CFG_PLUGIN_CONFIGURATION).at(irods::KW_CFG_PLUGIN_TYPE_RULE_ENGINE)) { - if (_instance_name == re.at(irods::KW_CFG_INSTANCE_NAME).get()) { + if (_instance_name == re.at(irods::KW_CFG_INSTANCE_NAME).get_ref()) { // Fill the "pep_configs" plugin variable with objects containing the values // defined in the "return_codes_for_peps" configuration. Each object in the list // will contain a regular expression and a code. @@ -98,10 +73,10 @@ namespace } return ERROR(SYS_CONFIG_FILE_ERR, "[passthrough] Bad rule engine plugin configuration"); - } + } // setup irods::error rule_exists(const std::string& _instance_name, - irods::default_re_ctx&, + [[maybe_unused]] irods::default_re_ctx& _ctx, const std::string& _rule_name, bool& _exists) { @@ -123,17 +98,18 @@ namespace return SUCCESS(); } - irods::error list_rules(irods::default_re_ctx&, std::vector& _rules) + irods::error list_rules(irods::default_re_ctx&, [[maybe_unused]] std::vector& _rules) { log::rule_engine::trace({{"rule_engine_plugin", "passthrough"}, {"rule_engine_plugin_function", __func__}}); return SUCCESS(); } irods::error exec_rule(const std::string& _instance_name, - irods::default_re_ctx&, + [[maybe_unused]] irods::default_re_ctx& _ctx, const std::string& _rule_name, - std::list& _rule_arguments, - irods::callback _effect_handler) + [[maybe_unused]] std::list& _rule_arguments, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [[maybe_unused]] irods::callback _effect_handler) { std::string msg = "Returned '"; msg += std::to_string(matched_code); @@ -185,12 +161,16 @@ pluggable_rule_engine* plugin_factory(const std::string& _instance_name, std::list& _rule_arguments, irods::callback _effect_handler) { + // NOLINTNEXTLINE(performance-unnecessary-value-param) return exec_rule(_instance_name, _ctx, _rule_name, _rule_arguments, _effect_handler); }; + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) auto* re = new pluggable_rule_engine{_instance_name, _context}; - re->add_operation("start", operation{start}); + re->add_operation("setup", operation{setup}); + re->add_operation("teardown", operation{no_op}); + re->add_operation("start", operation{no_op}); re->add_operation("stop", operation{no_op}); re->add_operation("rule_exists", operation{rule_exists_wrapper}); re->add_operation("list_rules", operation&>{list_rules}); diff --git a/server/main_server/src/agent_main.cpp b/server/main_server/src/agent_main.cpp index f827cbf441..303a6e29e6 100644 --- a/server/main_server/src/agent_main.cpp +++ b/server/main_server/src/agent_main.cpp @@ -293,7 +293,8 @@ int main(int _argc, char* _argv[]) } }}; - // TODO Why is this necessary? + // Initialize the global rule engine plugin facilities. + // This is where the rule engine plugins are loaded (i.e. plugin_factory is invoked here). irods::re_plugin_globals = std::make_unique(); // Initialize zone information for request processing. @@ -320,6 +321,10 @@ int main(int _argc, char* _argv[]) return 1; } + // Run setup() for each rule engine plugin. + // This allows rule engine plugins to setup state which doesn't change frequently. + irods::re_plugin_globals->global_re_mgr.call_setup_operations(); + // Enter parent process main loop. // // This process should never introduce threads. Everything it cares about must be handled @@ -426,6 +431,9 @@ int main(int _argc, char* _argv[]) // Do not accept new client requests (i.e. close the listening socket). close(svrComm.sock); + // Give the rule engine plugins a chance to free any resources established during setup(). + irods::re_plugin_globals->global_re_mgr.call_teardown_operations(); + if (0 == g_terminate_graceful) { // Instruct all agents to shutdown gracefully. // To avoid unnecessary complexity, we use SIGUSR1 as the termination signal for agents. diff --git a/server/main_server/src/main.cpp b/server/main_server/src/main.cpp index ac18da12c0..a993f99511 100644 --- a/server/main_server/src/main.cpp +++ b/server/main_server/src/main.cpp @@ -171,7 +171,7 @@ auto main(int _argc, char* _argv[]) -> int } if (create_pid_file(pid_file) != 0) { - fmt::print(stderr, "Error: could not create PID file [{}].", pid_file); + fmt::print(stderr, "Error: could not create PID file [{}].\n", pid_file); return 1; } } diff --git a/server/re/include/irods/irods_re_plugin.hpp b/server/re/include/irods/irods_re_plugin.hpp index d31b579eba..33994e3ff3 100644 --- a/server/re/include/irods/irods_re_plugin.hpp +++ b/server/re/include/irods/irods_re_plugin.hpp @@ -7,6 +7,11 @@ #include "irods/irods_re_structs.hpp" #include "irods/irods_state_table.h" +#include +#include + +#include + #include #include #include @@ -17,9 +22,6 @@ #include #include -#include -#include - #ifdef IRODS_ENABLE_SYSLOG # define IRODS_SERVER_ONLY(x) x # include "irods/irods_logger.hpp" @@ -193,6 +195,34 @@ namespace irods { } + error setup_operation(T& _in) + { + try { + auto fcn = boost::any_cast>(operations_["setup"]); + return fcn(_in, instance_name_); + } + catch (const boost::bad_any_cast& e) { + return ERROR(INVALID_ANY_CAST, fmt::format("failed to extract setup operation from instance [{}]: {}", instance_name_, e.what())); + } + catch (const std::exception& e) { + return ERROR(PLUGIN_ERROR, fmt::format("failed to extract setup operation from instance [{}]: {}", instance_name_, e.what())); + } + } // setup_operation + + error teardown_operation(T& _in) + { + try { + auto fcn = boost::any_cast>(operations_["teardown"]); + return fcn(_in, instance_name_); + } + catch (const boost::bad_any_cast& e) { + return ERROR(INVALID_ANY_CAST, fmt::format("failed to extract teardown operation from instance [{}]: {}", instance_name_, e.what())); + } + catch (const std::exception& e) { + return ERROR(PLUGIN_ERROR, fmt::format("failed to extract teardown operation from instance [{}]: {}", instance_name_, e.what())); + } + } // teardown_operation + error start_operation(T& _in) { try { auto fcn = boost::any_cast>( operations_["start"] ); @@ -418,6 +448,18 @@ namespace irods { return SUCCESS(); } + void call_setup_operations() { + std::for_each(begin(re_packs_), end(re_packs_), [](re_pack_inp &_inp) { + _inp.re_->setup_operation(_inp.re_ctx_); + }); + } + + void call_teardown_operations() { + std::for_each(begin(re_packs_), end(re_packs_), [](re_pack_inp &_inp) { + _inp.re_->teardown_operation(_inp.re_ctx_); + }); + } + void call_start_operations() { std::for_each(begin(re_packs_), end(re_packs_), [](re_pack_inp &_inp) { _inp.re_->start_operation(_inp.re_ctx_);