From 4ed154d07f09ca8f184539d4997af82a5942ee9e Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Wed, 29 May 2019 14:43:05 +0300 Subject: [PATCH] Create ExternalCmd during monitor configuration The command object need not be recreated every time it's ran. --- include/maxscale/monitor.hh | 18 +++--------- server/core/externcmd.cc | 41 +++++++++++++------------- server/core/internal/externcmd.hh | 10 ++++--- server/core/monitor.cc | 49 +++++++++++++++---------------- 4 files changed, 54 insertions(+), 64 deletions(-) diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index bf7d0e1d1..b5e02a0f9 100644 --- a/include/maxscale/monitor.hh +++ b/include/maxscale/monitor.hh @@ -540,24 +540,12 @@ private: void remove_all_servers(); /** - * Launch a script - * - * @param ptr The server which has changed state - * @return Return value of the executed script or -1 on error - */ - int launch_script(MonitorServer* ptr); - - /** - * Launch a command + * Launch a command. All default script variables will be replaced. * * @param ptr The server which has changed state - * @param cmd The command to execute. - * - * @note All default script variables will be replaced. - * * @return Return value of the executed script or -1 on error. */ - int launch_command(MonitorServer* ptr, ExternalCmd* cmd); + int launch_command(MonitorServer* ptr); enum class CredentialsApproach { @@ -583,6 +571,8 @@ private: std::atomic_bool m_status_change_pending {false}; /**< Set when admin requests a status change. */ uint8_t m_journal_hash[SHA_DIGEST_LENGTH]; /**< SHA1 hash of the latest written journal */ + std::unique_ptr m_scriptcmd; /**< External command representing the monitor script */ + ServerVector m_servers; /**< Monitored servers */ MXS_CONFIG_PARAMETER m_parameters; /**< Configuration parameters in text form */ Settings m_settings; /**< Base class settings */ diff --git a/server/core/externcmd.cc b/server/core/externcmd.cc index 274c21e20..034daec4c 100644 --- a/server/core/externcmd.cc +++ b/server/core/externcmd.cc @@ -92,43 +92,39 @@ int ExternalCmd::tokenize_args(char* dest[], int dest_size) return i; } -ExternalCmd* ExternalCmd::create(const char* argstr, int timeout) +std::unique_ptr ExternalCmd::create(const string& argstr, int timeout) { - auto cmd = new ExternalCmd(argstr, timeout); bool success = false; - if (argstr && cmd) + std::unique_ptr cmd(new ExternalCmd(argstr, timeout)); + char* argvec[1] {}; // Parse just one argument for testing file existence and permissions. + if (cmd->tokenize_args(argvec, 1) > 0) { - char* argvec[1] {}; // Parse just one argument for testing. - if (cmd->tokenize_args(argvec, 1) > 0) + const char* cmdname = argvec[0]; + if (access(cmdname, X_OK) != 0) { - const char* cmdname = argvec[0]; - if (access(cmdname, X_OK) != 0) + if (access(cmdname, F_OK) != 0) { - if (access(cmdname, F_OK) != 0) - { - MXS_ERROR("Cannot find file: %s", cmdname); - } - else - { - MXS_ERROR("Cannot execute file '%s'. Missing execution permissions.", cmdname); - } + MXS_ERROR("Cannot find file '%s'.", cmdname); } else { - success = true; + MXS_ERROR("Cannot execute file '%s'. Missing execution permissions.", cmdname); } - MXS_FREE(argvec[0]); } else { - MXS_ERROR("Failed to parse argument string for external command: %s", argstr); + success = true; } + MXS_FREE(argvec[0]); + } + else + { + MXS_ERROR("Failed to parse argument string '%s' for external command.", argstr.c_str()); } if (!success) { - delete cmd; - cmd = nullptr; + cmd.reset(); } return cmd; } @@ -421,6 +417,11 @@ bool ExternalCmd::externcmd_matches(const char* match) return m_command.find(match) != string::npos; } +void ExternalCmd::reset_substituted() +{ + m_command_substituted = m_command; +} + const char* ExternalCmd::substituted() const { return m_command_substituted.c_str(); diff --git a/server/core/internal/externcmd.hh b/server/core/internal/externcmd.hh index df6a9c041..2083bd288 100644 --- a/server/core/internal/externcmd.hh +++ b/server/core/internal/externcmd.hh @@ -14,6 +14,7 @@ #include #include +#include class ExternalCmd { @@ -26,7 +27,7 @@ public: * @param timeout Command timeout in seconds * @return Pointer to new external command struct or NULL if an error occurred */ - static ExternalCmd* create(const char* argstr, int timeout); + static std::unique_ptr create(const std::string& argstr, int timeout); /** * Execute a command @@ -54,15 +55,16 @@ public: */ bool externcmd_matches(const char* match); + void reset_substituted(); + const char* substituted() const; +private: static const int MAX_ARGS {256}; -private: std::string m_command; /**< Original command */ std::string m_command_substituted; /**< Command with substitutions */ - - int m_timeout; /**< Command timeout in seconds */ + int m_timeout; /**< Command timeout in seconds */ ExternalCmd(const std::string& script, int timeout); diff --git a/server/core/monitor.cc b/server/core/monitor.cc index fadd1c127..b5c4c6669 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -462,8 +462,6 @@ bool Monitor::configure(const MXS_CONFIG_PARAMETER* params) { m_settings.interval = params->get_duration(CN_MONITOR_INTERVAL).count(); m_settings.journal_max_age = params->get_duration(CN_JOURNAL_MAX_AGE).count(); - m_settings.script_timeout = params->get_duration(CN_SCRIPT_TIMEOUT).count(); - m_settings.script = params->get_string(CN_SCRIPT); m_settings.events = params->get_enum(CN_EVENTS, mxs_monitor_event_enum_values); MonitorServer::ConnectionSettings& conn_settings = m_settings.conn_settings; @@ -504,6 +502,23 @@ bool Monitor::configure(const MXS_CONFIG_PARAMETER* params) error = true; } + m_settings.script_timeout = params->get_duration(CN_SCRIPT_TIMEOUT).count(); + m_settings.script = params->get_string(CN_SCRIPT); + if (m_settings.script.empty()) + { + // Reset current external cmd if any. + m_scriptcmd.reset(); + } + else + { + m_scriptcmd = ExternalCmd::create(m_settings.script, m_settings.script_timeout); + if (!m_scriptcmd) + { + MXS_ERROR("Failed to initialize script '%s'.", m_settings.script.c_str()); + error = true; + } + } + if (!error) { // Store module name into parameter storage. @@ -1054,8 +1069,11 @@ std::string Monitor::child_nodes(MonitorServer* parent) return ss.str(); } -int Monitor::launch_command(MonitorServer* ptr, ExternalCmd* cmd) +int Monitor::launch_command(MonitorServer* ptr) { + auto cmd = m_scriptcmd.get(); + cmd->reset_substituted(); + if (cmd->externcmd_matches("$INITIATOR")) { char initiator[strlen(ptr->server->address) + 24]; // Extra space for port @@ -1150,27 +1168,6 @@ int Monitor::launch_command(MonitorServer* ptr, ExternalCmd* cmd) return rv; } -int Monitor::launch_script(MonitorServer* ptr) -{ - const char* script = m_settings.script.c_str(); - char arg[strlen(script) + 1]; - strcpy(arg, script); - - ExternalCmd* cmd = ExternalCmd::create(arg, m_settings.script_timeout); - - if (cmd == NULL) - { - MXS_ERROR("Failed to initialize script '%s'. See previous errors for the " - "cause of this failure.", - script); - return -1; - } - - int rv = launch_command(ptr, cmd); - delete cmd; - return rv; -} - MonitorServer::ConnectResult Monitor::ping_or_connect_to_db(const MonitorServer::ConnectionSettings& sett, SERVER& server, MYSQL** ppConn) { @@ -1381,9 +1378,9 @@ void Monitor::detect_handle_state_changes() master_up = true; } - if (!m_settings.script.empty() && (event & m_settings.events)) + if (m_scriptcmd && (event & m_settings.events)) { - launch_script(ptr); + launch_command(ptr); } } }