Check monitor permissions when reconnecting to server

Previously, the permissions would only be checked at monitor start.
Now, the permissions are checked if [Auth Error] is on or server
was reconnected.

server/modules/monitor/mariadbmon/mariadbmon.cc

@@ -298,24 +298,38 @@ void MariaDBMonitor::update_server(MariaDBServer& server)
     bool in_maintenance = server.is_in_maintenance();
     if (!in_maintenance)
     {
-        mxs_connect_result_t rval = mon_ping_or_connect_to_db(m_monitor, mon_srv);
+        mxs_connect_result_t conn_status = mon_ping_or_connect_to_db(m_monitor, mon_srv);
-
         MYSQL* conn = mon_srv->con; // mon_ping_or_connect_to_db() may have reallocated the MYSQL struct.
-        if (mon_connection_is_ok(rval))
-        {
-            server.clear_status(SERVER_AUTH_ERROR);
-            server.set_status(SERVER_RUNNING);
 
-            if (rval == MONITOR_CONN_NEWCONN_OK)
+        if (mon_connection_is_ok(conn_status))
+        {
+            server.set_status(SERVER_RUNNING);
+            if (conn_status == MONITOR_CONN_NEWCONN_OK)
             {
-                server.update_server_info();
+                // Is a new connection or a reconnection. Check server version.
+                server.update_server_version();
             }
-            if (should_update_disk_space_status(mon_srv))
+
+            if (server.m_version != MariaDBServer::version::UNKNOWN)
             {
-                update_disk_space_status(mon_srv);
+                // Check permissions if permissions failed last time or if this is a new connection.
+                if (server.had_status(SERVER_AUTH_ERROR) || conn_status == MONITOR_CONN_NEWCONN_OK)
+                {
+                    server.check_permissions();
+                }
+
+                // If permissions are ok, continue.
+                if (!server.has_status(SERVER_AUTH_ERROR))
+                {
+                    if (should_update_disk_space_status(mon_srv))
+                    {
+                        update_disk_space_status(mon_srv);
+                    }
+
+                    // Query MariaDBServer specific data
+                    server.monitor_server();
+                }
             }
-            // Query MariaDBServer specific data
-            server.monitor_server();
         }
         else
         {
@@ -332,7 +346,7 @@ void MariaDBMonitor::update_server(MariaDBServer& server)
              * iteration. */
             if (mon_srv->mon_prev_status & (SERVER_RUNNING | SERVER_MAINT))
             {
-                mon_log_connect_error(mon_srv, rval);
+                mon_log_connect_error(mon_srv, conn_status);
             }
         }
     }
@@ -354,16 +368,6 @@ void MariaDBMonitor::pre_loop()
         check_maxscale_schema_replication();
     }
 
-    /* Check monitor permissions. Failure won't cause the monitor to stop. Afterwards, close connections so
-     * that update_server() reconnects and checks server version. TODO: check permissions when checking
-     * server version. */
-    check_monitor_permissions(m_monitor, "SHOW SLAVE STATUS");
-    for (auto iter = m_servers.begin(); iter != m_servers.end(); iter++)
-    {
-        mysql_close((*iter)->m_server_base->con);
-        (*iter)->m_server_base->con = NULL;
-    }
-
     m_log_no_master = true;
 }
 

server/modules/monitor/mariadbmon/mariadbserver.cc

@@ -449,6 +449,16 @@ bool MariaDBServer::is_relay_server() const
            (SERVER_RUNNING | SERVER_MASTER | SERVER_SLAVE);
 }
 
+bool MariaDBServer::has_status(uint64_t bits) const
+{
+    return (m_server_base->pending_status & bits) == bits;
+}
+
+bool MariaDBServer::had_status(uint64_t bits) const
+{
+    return (m_server_base->mon_prev_status & bits) == bits;
+}
+
 bool MariaDBServer::is_read_only() const
 {
     return m_read_only;
@@ -803,7 +813,7 @@ bool MariaDBServer::update_slave_status(string* errmsg_out)
  * Update information which changes rarely. This method should be called after (re)connecting to a backend.
  * Calling this every monitoring loop is overkill.
  */
-void MariaDBServer::update_server_info()
+void MariaDBServer::update_server_version()
 {
     m_version = version::UNKNOWN;
     auto conn = m_server_base->con;
@@ -842,6 +852,38 @@ void MariaDBServer::update_server_info()
     }
 }
 
+/**
+ * Checks monitor permissions on the server. Sets/clears the SERVER_AUTH_ERROR bit.
+ */
+void MariaDBServer::check_permissions()
+{
+    MYSQL* conn = m_server_base->con;
+    // Test with a typical query to make sure the monitor has sufficient permissions.
+    const char* query = "SHOW SLAVE STATUS;";
+    if (mxs_mysql_query(conn, query) != 0)
+    {
+        /* In theory, this could be due to other errors as well, but that is quite unlikely since the
+         * connection was just checked. The end result is in any case that the server is not updated,
+         * and that this test is retried next round. */
+        set_status(SERVER_AUTH_ERROR);
+        // Only print error if last round was ok.
+        if (!had_status(SERVER_AUTH_ERROR))
+        {
+            MXS_WARNING("Query '%s' to server '%s' failed when checking monitor permissions: '%s'. ",
+                        query, name(), mysql_error(conn));
+        }
+    }
+    else
+    {
+        clear_status(SERVER_AUTH_ERROR);
+        MYSQL_RES* result = mysql_store_result(conn);
+        if (result)
+        {
+            mysql_free_result(result);
+        }
+    }
+}
+
 void MariaDBServer::clear_status(uint64_t bits)
 {
     monitor_clear_pending_status(m_server_base, bits);

server/modules/monitor/mariadbmon/mariadbserver.hh

@@ -151,7 +151,8 @@ public:
     MariaDBServer(MXS_MONITORED_SERVER* monitored_server, int config_index);
 
     void monitor_server();
-    void update_server_info();
+    void update_server_version();
+    void check_permissions();
 
     /**
      * Calculate how many events are left in the relay log.
@@ -261,6 +262,22 @@ public:
      */
     bool is_relay_server() const;
 
+    /**
+     * Check if server has the given bits on in 'pending_status'.
+     *
+     * @param bits Bits to check
+     * @return True if all given bits are on
+     */
+    bool has_status(uint64_t bits) const;
+
+    /**
+     * Check if server has the given bits on in 'mon_prev_status'.
+     *
+     * @param bits Bits to check
+     * @return True if all given bits are on
+     */
+    bool had_status(uint64_t bits) const;
+
     /**
      * Getter for m_read_only.
      *