Create ExternalCmd during monitor configuration

The command object need not be recreated every time it's ran.

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<ExternalCmd> 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 */

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> ExternalCmd::create(const string& argstr, int timeout)
 {
-    auto cmd = new ExternalCmd(argstr, timeout);
     bool success = false;
-    if (argstr && cmd)
+    std::unique_ptr<ExternalCmd> 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();

server/core/internal/externcmd.hh

@@ -14,6 +14,7 @@
 
 #include <maxscale/ccdefs.hh>
 #include <unistd.h>
+#include <memory>
 
 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<ExternalCmd> 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);
 

server/core/monitor.cc

@@ -462,8 +462,6 @@ bool Monitor::configure(const MXS_CONFIG_PARAMETER* params)
 {
     m_settings.interval = params->get_duration<milliseconds>(CN_MONITOR_INTERVAL).count();
     m_settings.journal_max_age = params->get_duration<seconds>(CN_JOURNAL_MAX_AGE).count();
-    m_settings.script_timeout = params->get_duration<seconds>(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<seconds>(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);
             }
         }
     }