This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 437f13632dbe204f67a04455246dd13ce80c029d Author: Chris McFarlen <[email protected]> AuthorDate: Thu Dec 12 10:34:47 2024 -0600 Updates to allow verify plugin tests to run without install (#11889) * Updates to allow verify plugin tests to run without install - Add option to skip verify tests - Add `preinit` flag option to `-C` command to run the command before TS fully initializes * Fix autest since diags.log isnt created now --------- Co-authored-by: Chris McFarlen <[email protected]> (cherry picked from commit 873f053910f0d8af7b9573d8d8b703ef957bdf15) --- CMakeLists.txt | 1 + CMakePresets.json | 3 +- cmake/add_atsplugin.cmake | 26 +++++++----- src/traffic_server/traffic_server.cc | 48 ++++++++++++++-------- .../command_argument/verify_global_plugin.test.py | 9 ++-- .../command_argument/verify_remap_plugin.test.py | 5 +-- 6 files changed, 57 insertions(+), 35 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7d95b3ad16..7c5de09b57 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -135,6 +135,7 @@ option(EXTERNAL_YAML_CPP "Use external yaml-cpp (default OFF)") option(EXTERNAL_LIBSWOC "Use external libswoc (default OFF)") option(LINK_PLUGINS "Link core libraries to plugins (default OFF)") option(ENABLE_PROBES "Enable ATS SystemTap probes (default OFF)") +option(ENABLE_VERIFY_PLUGINS "Enable plugin verification tests (default ON)" ON) # Setup user # NOTE: this is the user trafficserver runs as diff --git a/CMakePresets.json b/CMakePresets.json index aad0eccaa1..9995c90b65 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -46,7 +46,8 @@ "CMAKE_BUILD_TYPE": "Release", "CMAKE_COMPILE_WARNING_AS_ERROR": "OFF", "CMAKE_INSTALL_PREFIX": "/opt/ats", - "BUILD_EXPERIMENTAL_PLUGINS": "ON" + "BUILD_EXPERIMENTAL_PLUGINS": "ON", + "ENABLE_VERIFY_PLUGINS": "OFF" } }, { diff --git a/cmake/add_atsplugin.cmake b/cmake/add_atsplugin.cmake index cb0bc2301e..8130728190 100644 --- a/cmake/add_atsplugin.cmake +++ b/cmake/add_atsplugin.cmake @@ -37,19 +37,25 @@ function(add_atsplugin name) endfunction() function(verify_remap_plugin target) - add_test(NAME verify_remap_${target} COMMAND $<TARGET_FILE:traffic_server> -C - "verify_remap_plugin $<TARGET_FILE:${target}>" - ) + if(ENABLE_VERIFY_PLUGINS) + add_test(NAME verify_remap_${target} COMMAND $<TARGET_FILE:traffic_server> -C + "verify_remap_plugin $<TARGET_FILE:${target}>" + ) + endif() endfunction() function(verify_global_plugin target) - add_test(NAME verify_global_${target} COMMAND $<TARGET_FILE:traffic_server> -C - "verify_global_plugin $<TARGET_FILE:${target}>" - ) - # Process the optional suppression file parameter. - set(suppression_file ${ARGV1}) - if(suppression_file) - set_tests_properties(verify_global_${target} PROPERTIES ENVIRONMENT "LSAN_OPTIONS=suppressions=${suppression_file}") + if(ENABLE_VERIFY_PLUGINS) + add_test(NAME verify_global_${target} COMMAND $<TARGET_FILE:traffic_server> -C + "verify_global_plugin $<TARGET_FILE:${target}>" + ) + # Process the optional suppression file parameter. + set(suppression_file ${ARGV1}) + if(suppression_file) + set_tests_properties( + verify_global_${target} PROPERTIES ENVIRONMENT "LSAN_OPTIONS=suppressions=${suppression_file}" + ) + endif() endif() endfunction() diff --git a/src/traffic_server/traffic_server.cc b/src/traffic_server/traffic_server.cc index bdbc6692c2..3ea388a01f 100644 --- a/src/traffic_server/traffic_server.cc +++ b/src/traffic_server/traffic_server.cc @@ -30,6 +30,7 @@ ****************************************************************************/ +#include "tscore/TSSystemState.h" #include "tscore/Version.h" #include "swoc/swoc_file.h" @@ -1032,6 +1033,9 @@ enum class plugin_type_t { bool try_loading_plugin(plugin_type_t plugin_type, const fs::path &plugin_path, std::string &error) { + // At least one plugin checks this during plugin unload, so act like the event system is shutdown + TSSystemState::shut_down_event_system(); + switch (plugin_type) { case plugin_type_t::GLOBAL: { void *handle = nullptr; @@ -1138,15 +1142,16 @@ const struct CMD { const char *h; // help string (multi-line) int (*f)(char *); bool no_process_lock; /// If set this command doesn't need a process level lock. + bool preinit = false; } commands[] = { - {"list", "List cache configuration", + {"list", "List cache configuration", "LIST\n" "\n" "FORMAT: list\n" "\n" "List the sizes of the Host Database and Cache Index,\n" - "and the storage available to the cache.\n", cmd_list, false}, - {"check", "Check the cache (do not make any changes)", + "and the storage available to the cache.\n", cmd_list, false}, + {"check", "Check the cache (do not make any changes)", "CHECK\n" "\n" "FORMAT: check\n" @@ -1154,51 +1159,51 @@ const struct CMD { "Check the cache for inconsistencies or corruption.\n" "CHECK does not make any changes to the data stored in\n" "the cache. CHECK requires a scan of the contents of the\n" - "cache and may take a long time for large caches.\n", cmd_check, true }, - {"clear", "Clear the entire cache", + "cache and may take a long time for large caches.\n", cmd_check, true}, + {"clear", "Clear the entire cache", "CLEAR\n" "\n" "FORMAT: clear\n" "\n" "Clear the entire cache. All data in the cache is\n" "lost and the cache is reconfigured based on the current\n" - "description of database sizes and available storage.\n", cmd_clear, false}, - {"clear_cache", "Clear the document cache", + "description of database sizes and available storage.\n", cmd_clear, false}, + {"clear_cache", "Clear the document cache", "CLEAR_CACHE\n" "\n" "FORMAT: clear_cache\n" "\n" "Clear the document cache. All documents in the cache are\n" "lost and the cache is reconfigured based on the current\n" - "description of database sizes and available storage.\n", cmd_clear, false}, - {"clear_hostdb", "Clear the hostdb cache", + "description of database sizes and available storage.\n", cmd_clear, false}, + {"clear_hostdb", "Clear the hostdb cache", "CLEAR_HOSTDB\n" "\n" "FORMAT: clear_hostdb\n" "\n" "Clear the entire hostdb cache. All host name resolution\n" - "information is lost.\n", cmd_clear, false}, - {CMD_VERIFY_CONFIG, "Verify the config", + "information is lost.\n", cmd_clear, false}, + {CMD_VERIFY_CONFIG, "Verify the config", "\n" "\n" "FORMAT: verify_config\n" "\n" - "Load the config and verify traffic_server comes up correctly. \n", cmd_verify, true }, + "Load the config and verify traffic_server comes up correctly. \n", cmd_verify, true}, {"verify_global_plugin", "Verify a global plugin's shared object file", "VERIFY_GLOBAL_PLUGIN\n" "\n" "FORMAT: verify_global_plugin [global_plugin_so_file]\n" "\n" "Load a global plugin's shared object file and verify it meets\n" - "minimal plugin API requirements. \n", cmd_verify_global_plugin, true }, - {"verify_remap_plugin", "Verify a remap plugin's shared object file", + "minimal plugin API requirements. \n", cmd_verify_global_plugin, true, true}, + {"verify_remap_plugin", "Verify a remap plugin's shared object file", "VERIFY_REMAP_PLUGIN\n" "\n" "FORMAT: verify_remap_plugin [remap_plugin_so_file]\n" "\n" "Load a remap plugin's shared object file and verify it meets\n" - "minimal plugin API requirements. \n", cmd_verify_remap_plugin, true }, - {"help", "Obtain a short description of a command (e.g. 'help clear')", + "minimal plugin API requirements. \n", cmd_verify_remap_plugin, true, true}, + {"help", "Obtain a short description of a command (e.g. 'help clear')", "HELP\n" "\n" "FORMAT: help [command_name]\n" @@ -1206,7 +1211,7 @@ const struct CMD { "EXAMPLES: help help\n" " help commit\n" "\n" - "Provide a short description of a command (like this).\n", cmd_help, false}, + "Provide a short description of a command (like this).\n", cmd_help, false}, }; int @@ -1866,6 +1871,15 @@ main(int /* argc ATS_UNUSED */, const char **argv) // fprintf's before those calls bind_outputs(bind_stdout, bind_stderr); + if (command_valid && commands[command_index].preinit) { + int cmd_ret = cmd_mode(); + if (cmd_ret >= 0) { + ::exit(0); // everything is OK + } else { + ::exit(1); // in error + } + } + // Records init initialize_records(); diff --git a/tests/gold_tests/command_argument/verify_global_plugin.test.py b/tests/gold_tests/command_argument/verify_global_plugin.test.py index 8b736d6f1f..e9aa44ce52 100644 --- a/tests/gold_tests/command_argument/verify_global_plugin.test.py +++ b/tests/gold_tests/command_argument/verify_global_plugin.test.py @@ -24,6 +24,7 @@ Test that the TrafficServer verify_global_plugin command works as expected. ''' process_counter = 0 +no_log_data = {'diags': 'stderr', 'error': 'stderr', 'manager': 'stderr'} def create_ts_process(): @@ -33,7 +34,7 @@ def create_ts_process(): global process_counter process_counter += 1 - ts = Test.MakeATSProcess("ts{counter}".format(counter=process_counter)) + ts = Test.MakeATSProcess("ts{counter}".format(counter=process_counter), log_data=no_log_data) # Ideally we would set the test run's Processes.Default to ts, but deep # copy of processes is not currently implemented in autest. Therefore we @@ -88,7 +89,7 @@ tr.Processes.Default.ReturnCode = 1 tr.Processes.Default.StartBefore(ts) tr.Processes.Default.Streams.stderr = Testers.ContainsExpression( "ERROR: .*unable to find TSPluginInit function in", "Should warn about the need for the TSPluginInit symbol") -ts.Disk.diags_log.Content = Testers.ContainsExpression("ERROR", "ERROR: .*unable to find TSPluginInit function in") +ts.Disk.diags_log.Content = [] """ TEST: Verify that passing a remap plugin produces a warning because it doesn't have the global plugin symbols. @@ -104,7 +105,7 @@ tr.Processes.Default.ReturnCode = 1 tr.Processes.Default.StartBefore(ts) tr.Processes.Default.Streams.stderr = Testers.ContainsExpression( "ERROR: .*unable to find TSPluginInit function in", "Should warn about the need for the TSPluginInit symbol") -ts.Disk.diags_log.Content = Testers.ContainsExpression("ERROR", "ERROR: .*unable to find TSPluginInit function in") +ts.Disk.diags_log.Content = [] """ TEST: The happy case: a global plugin shared object file is passed as an argument that has the definition for the expected Plugin symbols. @@ -139,4 +140,4 @@ tr.Processes.Default.ReturnCode = 1 tr.Processes.Default.StartBefore(ts) tr.Processes.Default.Streams.stderr = Testers.ContainsExpression( "ERROR:.*unable to load", "Should log failure to load shared object") -ts.Disk.diags_log.Content = Testers.ContainsExpression("ERROR", "Should log failure to load shared object") +ts.Disk.diags_log.Content = [] diff --git a/tests/gold_tests/command_argument/verify_remap_plugin.test.py b/tests/gold_tests/command_argument/verify_remap_plugin.test.py index d291231604..3b68147962 100644 --- a/tests/gold_tests/command_argument/verify_remap_plugin.test.py +++ b/tests/gold_tests/command_argument/verify_remap_plugin.test.py @@ -24,6 +24,7 @@ Test that the TrafficServer verify_remap_plugin command works as expected. ''' process_counter = 0 +no_log_data = {'diags': 'stderr', 'error': 'stderr', 'manager': 'stderr'} def create_ts_process(): @@ -33,7 +34,7 @@ def create_ts_process(): global process_counter process_counter += 1 - ts = Test.MakeATSProcess("ts{counter}".format(counter=process_counter)) + ts = Test.MakeATSProcess("ts{counter}".format(counter=process_counter), log_data=no_log_data) # Ideally we would set the test run's Processes.Default to ts, but deep # copy of processes is not currently implemented in autest. Therefore we @@ -88,7 +89,6 @@ tr.Processes.Default.ReturnCode = 1 tr.Processes.Default.StartBefore(ts) tr.Processes.Default.Streams.stderr = Testers.ContainsExpression( "ERROR: .*missing required function TSRemapInit", "Should warn about the need for the TSRemapInit symbol") -ts.Disk.diags_log.Content = Testers.ContainsExpression("ERROR", "ERROR: .*missing required function TSRemapInit") """ TEST: verify_remap_plugin should complain if the plugin has the global plugin symbols but not the remap ones. @@ -104,7 +104,6 @@ tr.Processes.Default.ReturnCode = 1 tr.Processes.Default.StartBefore(ts) tr.Processes.Default.Streams.stderr = Testers.ContainsExpression( "ERROR: .*missing required function TSRemapInit", "Should warn about the need for the TSRemapInit symbol") -ts.Disk.diags_log.Content = Testers.ContainsExpression("ERROR", "ERROR: .*missing required function TSRemapInit") """ TEST: The happy case: a remap plugin shared object file is passed as an argument that has the definition for the expected Plugin symbols.
