MXS-2271 Move script-related settings to the settings-container

Also moves related functions to class methods.

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<MXS_MONITORED_SERVER*> 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();
 

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)

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);
 

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);

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);
+    m_settings.script_timeout = config_get_integer(params, CN_SCRIPT_TIMEOUT);
-    script = config_get_string(params, CN_SCRIPT);
+    m_settings.script = config_get_string(params, CN_SCRIPT);
-    events = config_get_enum(params, CN_EVENTS, mxs_monitor_event_enum_values);
+    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<MXS_MONITORED_SERVER*>& 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()

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.