From ce9b49d8d5a2d2648e78f0a53a1fc84062e5c830 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Mon, 28 Jan 2019 17:47:48 +0200 Subject: [PATCH] MXS-2271 Move script-related settings to the settings-container Also moves related functions to class methods. --- include/maxscale/monitor.hh | 54 +++++++++++++------ server/core/config_runtime.cc | 2 +- server/core/internal/externcmd.hh | 4 +- server/core/internal/monitor.hh | 26 --------- server/core/monitor.cc | 45 ++++++++-------- .../modules/monitor/mariadbmon/mariadbmon.hh | 2 +- 6 files changed, 64 insertions(+), 69 deletions(-) diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index f1f72909e..1516e4d53 100644 --- a/include/maxscale/monitor.hh +++ b/include/maxscale/monitor.hh @@ -31,6 +31,7 @@ class Monitor; struct DCB; struct json_t; +struct EXTERNCMD; /** * @verbatim @@ -285,6 +286,8 @@ public: */ void set_password(const std::string& passwd); + void set_script_timeout(int value); + /** * Create a list of running servers * @@ -315,9 +318,6 @@ public: std::vector m_servers; /**< Monitored servers */ time_t journal_max_age; /**< Maximum age of journal file */ - uint32_t script_timeout; /**< Timeout in seconds for the monitor scripts */ - const char* script; /**< Launchable script. */ - uint64_t events; /**< Enabled monitor events. */ protected: @@ -330,6 +330,13 @@ protected: */ bool test_permissions(const std::string& query); + /** + * Detect and handle state change events. This function should be called by all monitors at the end + * of each monitoring cycle. The function logs state changes and executes the monitor script on + * servers whose status changed. + */ + void detect_handle_state_changes(); + /** * Contains monitor base class settings. Since monitors are stopped before a setting change, * the items cannot be modified while a monitor is running. No locking required. @@ -337,7 +344,11 @@ protected: class Settings { public: - int64_t interval {0}; /**< Monitor interval in milliseconds */ + int64_t interval {0}; /**< Monitor interval in milliseconds */ + + std::string script; /**< Script triggered by events */ + int script_timeout {0}; /**< Timeout in seconds for the monitor scripts */ + uint64_t events {0}; /**< Bitfield of events which trigger the script */ SERVER::DiskSpaceLimits disk_space_limits; /**< Disk space thresholds */ /** @@ -360,6 +371,26 @@ private: * @return True on success */ bool configure_base(const MXS_CONFIG_PARAMETER* params); + + /** + * 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(MXS_MONITORED_SERVER* ptr); + + /** + * Launch a command + * + * @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(MXS_MONITORED_SERVER* ptr, EXTERNCMD* cmd); }; /** @@ -438,18 +469,6 @@ const char* mon_get_event_name(mxs_monitor_event_t event); */ void mon_alter_parameter(Monitor* monitor, const char* key, const char* value); -/** - * @brief Handle state change events - * - * This function should be called by all monitors at the end of each monitoring - * cycle. This will log state changes and execute any scripts that should be executed. - * - * @param monitor Monitor object - * @param script Script to execute or NULL for no script - * @param events Enabled events - */ -void mon_process_state_changes(Monitor* monitor, const char* script, uint64_t events); - /** * @brief Hangup connections to failed servers * @@ -728,7 +747,8 @@ protected: /** * @brief Called after tick returns * - * The default implementation will call @mon_process_state_changes. + * The default implementation will call @Monitor::detect_state_changes. Overriding functions + * should do the same before proceeding with their own processing. */ virtual void process_state_changes(); diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 63ecc58bc..dc019af5c 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -640,7 +640,7 @@ bool do_alter_monitor(Monitor* monitor, const char* key, const char* value) { if (auto ival = get_positive_int(value)) { - monitor_set_script_timeout(monitor, ival); + monitor->set_script_timeout(ival); } } else if (strcmp(key, CN_DISK_SPACE_THRESHOLD) == 0) diff --git a/server/core/internal/externcmd.hh b/server/core/internal/externcmd.hh index fba1195a3..414a5fbd0 100644 --- a/server/core/internal/externcmd.hh +++ b/server/core/internal/externcmd.hh @@ -19,14 +19,14 @@ MXS_BEGIN_DECLS -typedef struct extern_cmd_t +struct EXTERNCMD { char** argv; /**< Argument vector for the command, first being the * actual command being executed */ int n_exec; /**< Number of times executed */ pid_t child; /**< PID of the child process */ uint32_t timeout; /**< Command timeout in seconds */ -} EXTERNCMD; +}; char* externcmd_extract_command(const char* argstr); diff --git a/server/core/internal/monitor.hh b/server/core/internal/monitor.hh index 3a539d9fa..a917b0910 100644 --- a/server/core/internal/monitor.hh +++ b/server/core/internal/monitor.hh @@ -139,7 +139,6 @@ bool monitor_remove_parameter(Monitor* monitor, const char* key); void monitor_set_parameter(Monitor* monitor, const char* key, const char* value); void monitor_set_journal_max_age(Monitor* mon, time_t value); -void monitor_set_script_timeout(Monitor* mon, uint32_t value); /** * @brief Serialize a monitor to a file @@ -157,28 +156,3 @@ bool monitor_serialize(const Monitor* monitor); * @return The monitor watching this server, or NULL if not monitored */ Monitor* monitor_server_in_use(const SERVER* server); - -/** - * Launch a script - * - * @param mon Owning monitor - * @param ptr The server which has changed state - * @param script Script to execute - * @param timeout Timeout in seconds for the script - * - * @return Return value of the executed script or -1 on error - */ -int monitor_launch_script(Monitor* mon, MXS_MONITORED_SERVER* ptr, const char* script, uint32_t timeout); - -/** - * Launch a command - * - * @param mon Owning monitor - * @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 monitor_launch_command(Monitor* mon, MXS_MONITORED_SERVER* ptr, EXTERNCMD* cmd); diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 04493f820..1230daabf 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -194,9 +194,9 @@ bool Monitor::configure_base(const MXS_CONFIG_PARAMETER* params) m_settings.conn_settings.connect_attempts = config_get_integer(params, CN_BACKEND_CONNECT_ATTEMPTS); m_settings.interval = config_get_integer(params, CN_MONITOR_INTERVAL); journal_max_age = config_get_integer(params, CN_JOURNAL_MAX_AGE); - script_timeout = config_get_integer(params, CN_SCRIPT_TIMEOUT); - script = config_get_string(params, CN_SCRIPT); - events = config_get_enum(params, CN_EVENTS, mxs_monitor_event_enum_values); + m_settings.script_timeout = config_get_integer(params, CN_SCRIPT_TIMEOUT); + m_settings.script = config_get_string(params, CN_SCRIPT); + m_settings.events = config_get_enum(params, CN_EVENTS, mxs_monitor_event_enum_values); m_settings.disk_space_check_interval = config_get_integer(params, CN_DISK_SPACE_CHECK_INTERVAL); m_settings.conn_settings.username = config_get_string(params, CN_USER); m_settings.conn_settings.password = config_get_string(params, CN_PASSWORD); @@ -605,9 +605,9 @@ void monitor_set_journal_max_age(Monitor* mon, time_t value) mon->journal_max_age = value; } -void monitor_set_script_timeout(Monitor* mon, uint32_t value) +void Monitor::set_script_timeout(int value) { - mon->script_timeout = value; + m_settings.script_timeout = value; } bool Monitor::set_network_timeout(int type, int value, const char* key) @@ -1142,7 +1142,7 @@ static std::string child_nodes(const std::vector& servers return ss.str(); } -int monitor_launch_command(Monitor* mon, MXS_MONITORED_SERVER* ptr, EXTERNCMD* cmd) +int Monitor::launch_command(MXS_MONITORED_SERVER* ptr, EXTERNCMD* cmd) { if (externcmd_matches(cmd, "$INITIATOR")) { @@ -1154,7 +1154,7 @@ int monitor_launch_command(Monitor* mon, MXS_MONITORED_SERVER* ptr, EXTERNCMD* c if (externcmd_matches(cmd, "$PARENT")) { std::stringstream ss; - MXS_MONITORED_SERVER* parent = find_parent_node(mon->m_servers, ptr); + MXS_MONITORED_SERVER* parent = find_parent_node(m_servers, ptr); if (parent) { @@ -1165,7 +1165,7 @@ int monitor_launch_command(Monitor* mon, MXS_MONITORED_SERVER* ptr, EXTERNCMD* c if (externcmd_matches(cmd, "$CHILDREN")) { - externcmd_substitute_arg(cmd, "[$]CHILDREN", child_nodes(mon->m_servers, ptr).c_str()); + externcmd_substitute_arg(cmd, "[$]CHILDREN", child_nodes(m_servers, ptr).c_str()); } if (externcmd_matches(cmd, "$EVENT")) @@ -1178,37 +1178,37 @@ int monitor_launch_command(Monitor* mon, MXS_MONITORED_SERVER* ptr, EXTERNCMD* c if (externcmd_matches(cmd, "$CREDENTIALS")) { // We provide the credentials for _all_ servers. - mon->append_node_names(nodelist, sizeof(nodelist), 0, CREDENTIALS_INCLUDE); + append_node_names(nodelist, sizeof(nodelist), 0, CREDENTIALS_INCLUDE); externcmd_substitute_arg(cmd, "[$]CREDENTIALS", nodelist); } if (externcmd_matches(cmd, "$NODELIST")) { - mon->append_node_names(nodelist, sizeof(nodelist), SERVER_RUNNING); + append_node_names(nodelist, sizeof(nodelist), SERVER_RUNNING); externcmd_substitute_arg(cmd, "[$]NODELIST", nodelist); } if (externcmd_matches(cmd, "$LIST")) { - mon->append_node_names(nodelist, sizeof(nodelist), 0); + append_node_names(nodelist, sizeof(nodelist), 0); externcmd_substitute_arg(cmd, "[$]LIST", nodelist); } if (externcmd_matches(cmd, "$MASTERLIST")) { - mon->append_node_names(nodelist, sizeof(nodelist), SERVER_MASTER); + append_node_names(nodelist, sizeof(nodelist), SERVER_MASTER); externcmd_substitute_arg(cmd, "[$]MASTERLIST", nodelist); } if (externcmd_matches(cmd, "$SLAVELIST")) { - mon->append_node_names(nodelist, sizeof(nodelist), SERVER_SLAVE); + append_node_names(nodelist, sizeof(nodelist), SERVER_SLAVE); externcmd_substitute_arg(cmd, "[$]SLAVELIST", nodelist); } if (externcmd_matches(cmd, "$SYNCEDLIST")) { - mon->append_node_names(nodelist, sizeof(nodelist), SERVER_JOINED); + append_node_names(nodelist, sizeof(nodelist), SERVER_JOINED); externcmd_substitute_arg(cmd, "[$]SYNCEDLIST", nodelist); } @@ -1284,12 +1284,13 @@ int monitor_launch_command(Monitor* mon, MXS_MONITORED_SERVER* ptr, EXTERNCMD* c return rv; } -int monitor_launch_script(Monitor* mon, MXS_MONITORED_SERVER* ptr, const char* script, uint32_t timeout) +int Monitor::launch_script(MXS_MONITORED_SERVER* ptr) { + const char* script = m_settings.script.c_str(); char arg[strlen(script) + 1]; strcpy(arg, script); - EXTERNCMD* cmd = externcmd_allocate(arg, timeout); + EXTERNCMD* cmd = externcmd_allocate(arg, m_settings.script_timeout); if (cmd == NULL) { @@ -1299,7 +1300,7 @@ int monitor_launch_script(Monitor* mon, MXS_MONITORED_SERVER* ptr, const char* s return -1; } - int rv = monitor_launch_command(mon, ptr, cmd); + int rv = launch_command(ptr, cmd); externcmd_free(cmd); @@ -1590,12 +1591,12 @@ void monitor_check_maintenance_requests(Monitor* monitor) } } -void mon_process_state_changes(Monitor* monitor, const char* script, uint64_t events) +void Monitor::detect_handle_state_changes() { bool master_down = false; bool master_up = false; - for (MXS_MONITORED_SERVER* ptr : monitor->m_servers) + for (MXS_MONITORED_SERVER* ptr : m_servers) { if (mon_status_changed(ptr)) { @@ -1621,9 +1622,9 @@ void mon_process_state_changes(Monitor* monitor, const char* script, uint64_t ev master_up = true; } - if (script && *script && (events & event)) + if (!m_settings.script.empty() && (event & m_settings.events)) { - monitor_launch_script(monitor, ptr, script, monitor->script_timeout); + launch_script(ptr); } } } @@ -2855,7 +2856,7 @@ void MonitorWorker::post_loop() void MonitorWorker::process_state_changes() { - mon_process_state_changes(m_monitor, m_monitor->script, m_monitor->events); + detect_handle_state_changes(); } bool MonitorWorker::pre_run() diff --git a/server/modules/monitor/mariadbmon/mariadbmon.hh b/server/modules/monitor/mariadbmon/mariadbmon.hh index 7351c9a96..057dcbea3 100644 --- a/server/modules/monitor/mariadbmon/mariadbmon.hh +++ b/server/modules/monitor/mariadbmon/mariadbmon.hh @@ -109,7 +109,7 @@ public: protected: void pre_loop(); void tick(); - void process_state_changes(); + void process_state_changes() override; private: // Some methods need a log on/off setting.