Ensure that nagios user gets created with a password

The associated bug is due to a change introduced in commit d55dcde
which was to ensure that the correct password update is used for
different versions of mysql (pre and post 5.7.5).  However, this change
has broken the nagios user creation due to not setting the password.

This patch creates the nagios user and passord at the same time.  The
updating of the password is only done if the account already exists.

The change also corrects the nagios password store in leader-settings to
the 'mysql-nagios.passwd' key instead of 'nagios-password'.  This was an
unfortuante error when the nagios change password was introduced.  The
charm detects if the 'nagios-password' key is used on charm-upgrade and
moves it to 'mysql-nagios.passwd'.  This enables the key to work with
the standard MySQLHelper functions.

Finally, the ALTER command (on percona) doesn't update non-InnoDB tables
and thus needs to be run for each unit when the nagios password is
changed via the action.  The changes in percona_utils.py enable this to
happen.

Whilst the change looks large it ONLY affects the nagios password parts
of the charm.

The related bug is a tracking bug to serve as a reminder to fix this in
charm-helpers and this charm (i.e. make the charm-helpers code work to
change a password for any user other than root, and then enable this
charm to use that code).

Change-Id: Ibc751bef7b4654ebffdf843c556b193373e6e80c
Related-Bug: #1925377
Closes-Bug: #1925042

actions/actions.py

@@ -227,7 +227,7 @@ def mysqldump(args):
 def generate_nagios_password(args):
     """Regenerate nagios password."""
     if is_leader():
-        leader_set({"nagios-password": pwgen()})
+        leader_set({"mysql-nagios.passwd": pwgen()})
         percona_utils.set_nagios_user()
         action_set({"output": "New password for nagios created successfully."})
     else:

hooks/percona_hooks.py

@@ -530,6 +530,14 @@ def upgrade():
         if not leader_get('root-password') and leader_get('mysql.passwd'):
             leader_set(**{'root-password': leader_get('mysql.passwd')})
 
+        # move the nagios password out of nagios-password and into
+        # mysql-nagios.passwd
+        # BUG: #1925042
+        nagios_password = leader_get('nagios-password')
+        if nagios_password:
+            leader_set(**{"mysql-nagios.passwd": nagios_password,
+                          "nagios-password": None})
+
         # On upgrade-charm we assume the cluster was complete at some point
         kvstore = kv()
         initial_clustered = kvstore.get(INITIAL_CLUSTERED_KEY, False)

hooks/percona_utils.py

@@ -830,8 +830,8 @@ def assess_status(configs):
             base = determine_packages()[0]
             yield base
             if '.' not in base:
-                for i in range(5, 7+1):
-                    yield base+'-5.'+str(i)
+                for i in range(5, 7 + 1):
+                    yield base + '-5.' + str(i)
         version = None
         for pkg in _possible_packages():
             version = get_upstream_version(pkg)
@@ -1054,7 +1054,11 @@ root_password = partial(_get_password, 'root-password')
 
 sst_password = partial(_get_password, 'sst-password')
 
-nagios_password = partial(_get_password, 'nagios-password')
+
+def nagios_password():
+    """Get the nagios password using the MySQLdb helper in charmhelpers"""
+    m_helper = get_db_helper()
+    return m_helper.get_mysql_password('nagios')
 
 
 def pxc_installed():
@@ -1802,38 +1806,93 @@ def create_nagios_user():
 
     :raises: OperationalError
     """
-    m_helper = get_db_helper()
-    try:
-        m_helper.connect(password=m_helper.get_mysql_root_password())
-    except OperationalError:
-        log("Could not connect to db", level=ERROR)
-        raise
-    # NOTE (rgildein): This user has access to the SHOW GLOBAL STATUS
-    # statement, which does not require any privilege, only the ability to
-    # connect to the server.
-    nagios_exists = False
-    try:
-        # NOTE (rgildein): This method of creation nagios user should be
-        # replaced by the `MySQLHelper.create_user` method after
-        # bug #1905586 has been fixed and merged.
-        nagios_exists = m_helper.select(
-            "SELECT EXISTS(SELECT 1 FROM mysql.user WHERE user = 'nagios')"
-        )
-        m_helper.execute("CREATE USER 'nagios'@'localhost';")
-    except OperationalError as error:
-        if not nagios_exists:
-            log("Creating user nagios failed due: {}".format(error),
-                level="ERROR")
-            raise error
-        else:
-            log("User 'nagios'@'localhost' already exists.", level="WARNING")
+    m_helper = None
+    if is_leader():
+        # Only run the CREATE USER on the leader; it does replicate to other
+        # instances.
+        m_helper = get_db_helper()
+        try:
+            m_helper.connect(password=m_helper.get_mysql_root_password())
+        except OperationalError:
+            log("Could not connect to db", level=ERROR)
+            raise
+        # NOTE (rgildein): This user has access to the SHOW GLOBAL STATUS
+        # statement, which does not require any privilege, only the ability to
+        # connect to the server.
+        nagios_exists = False
+        try:
+            # NOTE (rgildein): This method of creation nagios user should be
+            # replaced by the `MySQLHelper.create_user` method after
+            # bug #1905586 has been fixed and merged.
+            nagios_exists = m_helper.select(
+                "SELECT EXISTS(SELECT 1 FROM mysql.user WHERE user = 'nagios')"
+            )
+            m_helper.execute(
+                "CREATE USER 'nagios'@'localhost' IDENTIFIED BY '{}';"
+                .format(nagios_password()))
+            m_helper.flush_priviledges()
+            log("Created user nagios in database.", level="INFO")
+            return
+        except OperationalError as error:
+            if not nagios_exists:
+                log("Creating user nagios failed due: {}".format(error),
+                    level="ERROR")
+                raise error
+            else:
+                log("User 'nagios'@'localhost' already exists.", level="INFO")
+
     # NOTE (rgildein): Update the user's password if it has changed.
-    m_helper.set_mysql_password('nagios', nagios_password())
+    # Update the password for username with new_password.
+
+    # BUG: #1925042
+    # This is a strategic hotfix to fix nagios password updates.  The
+    # charm-helpers library function set_mysql_password() tries to use the
+    # username that was passed, rather than root, to update the mysql.user
+    # table and this fails.  Instead, this function uses root to update the
+    # password and flush privileges.  Note this needs to be run on each node as
+    # it uses UPDATE and flush privileges.
+
+    # See https://galeracluster.com/library/kb/user-changes.html
+
+    # copied and adapted from MySQLHelper.set_mysql_password()
+    cursor = None
+    try:
+        # NOTE(freyes): Due to skip-name-resolve root@$HOSTNAME account
+        # fails when using SET PASSWORD so using UPDATE against the
+        # mysql.user table is needed, but changes to this table are not
+        # replicated across the cluster, so this update needs to run in
+        # all the nodes. More info at
+        # http://galeracluster.com/documentation-webpages/userchanges.html
+        release = CompareHostReleases(lsb_release()['DISTRIB_CODENAME'])
+        if release < 'bionic':
+            SQL_UPDATE_PASSWD = ("UPDATE mysql.user SET password = "
+                                 "PASSWORD( %s ) WHERE user = %s;")
+        else:
+            # PXC 5.7 (introduced in Bionic) uses authentication_string
+            SQL_UPDATE_PASSWD = ("UPDATE mysql.user SET "
+                                 "authentication_string = "
+                                 "PASSWORD( %s ) WHERE user = %s;")
+
+        if m_helper is None:
+            m_helper = get_db_helper()
+        try:
+            m_helper.connect(password=m_helper.get_mysql_root_password())
+        except OperationalError:
+            log("Could not connect to db", level=WARNING)
+            return
+        cursor = m_helper.connection.cursor()
+        cursor.execute(SQL_UPDATE_PASSWD, (nagios_password(), 'nagios'))
+        cursor.execute('FLUSH PRIVILEGES;')
+        m_helper.connection.commit()
+    except OperationalError as ex:
+        log("Could not connect to db: {}".format(str(ex)), level=WARNING)
+    finally:
+        if cursor:
+            cursor.close()
 
 
 def set_nagios_user():
     """Create MySQL user and configuration file with user credentials."""
-    if is_leader():
-        create_nagios_user()
+    create_nagios_user()
 
     write_nagios_my_cnf()

tests/bundles/bionic-nagios-ha.yaml

@@ -0,0 +1 @@
+bionic-nagios.yaml

tests/bundles/bionic-nagios.yaml

@@ -0,0 +1,28 @@
+series: bionic
+
+applications:
+  percona-cluster:
+    series: bionic
+    charm: ../../../percona-cluster
+    num_units: 1
+  keystone:
+    charm: cs:~openstack-charmers-next/keystone
+    num_units: 1
+    options:
+      token-expiration: 60
+  nagios:
+    charm: cs:nagios
+    series: bionic
+    num_units: 1
+  nrpe:
+    charm: cs:nrpe
+
+relations:
+- - keystone:shared-db
+  - percona-cluster:shared-db
+
+- - nrpe:monitors
+  - nagios:monitors
+
+- - nrpe:nrpe-external-master
+  - percona-cluster:nrpe-external-master

tests/bundles/overlays/bionic-nagios-ha.yaml.j2

@@ -0,0 +1 @@
+ha.yaml

tests/tests.yaml

@@ -19,3 +19,5 @@ smoke_bundles:
   - bionic_model: bionic-ha
 dev_bundles:
   - xenial_series_upgrade_model: xenial-queens-ha-series-upgrade
+  - bionic_model: bionic-nagios
+  - bionic_model: bionic-nagios-ha

unit_tests/test_actions.py

@@ -124,7 +124,8 @@ class NagiosTestCase(CharmTestCase):
         self.is_leader.return_value = True
         self.pwgen.return_value = "1234"
         actions.generate_nagios_password([])
-        self.leader_set.assert_called_once_with({"nagios-password": "1234"})
+        self.leader_set.assert_called_once_with(
+            {"mysql-nagios.passwd": "1234"})
         mock_set_nagios_user.assert_called_once_with()
         self.action_set.assert_called_once_with(
             {"output": "New password for nagios created successfully."}

unit_tests/test_percona_hooks.py

@@ -816,7 +816,8 @@ class TestUpgradeCharm(CharmTestCase):
         self.is_leader.return_value = True
         self.is_unit_paused_set.return_value = False
         self.get_relation_ip.return_value = '10.10.10.10'
-        self.leader_get.side_effect = [None, 'mypasswd', 'mypasswd']
+        self.leader_get.side_effect = [
+            None, 'mypasswd', 'mypasswd', 'nagios-password']
 
         def c(k):
             values = {'wsrep_ready': 'on',
@@ -833,7 +834,9 @@ class TestUpgradeCharm(CharmTestCase):
 
         self.leader_set.assert_has_calls(
             [mock.call(**{'leader-ip': '10.10.10.10'}),
-             mock.call(**{'root-password': 'mypasswd'})])
+             mock.call(**{'root-password': 'mypasswd'}),
+             mock.call(**{'mysql-nagios.passwd': 'nagios-password',
+                          'nagios-password': None})])
 
 
 class TestConfigs(CharmTestCase):

unit_tests/test_percona_utils.py

@@ -539,27 +539,32 @@ class UtilsTests(CharmTestCase):
 
         mock_apt_update.assert_not_called()
 
+    @mock.patch.object(percona_utils, "nagios_password")
     @mock.patch.object(percona_utils, "lsb_release")
     @mock.patch.object(percona_utils, "get_db_helper")
     @mock.patch.object(percona_utils, "write_nagios_my_cnf")
     def test_create_nagios_user(self,
                                 mock_create_nagios_mysql_credential,
                                 mock_get_db_helper,
-                                mock_lsb_release):
+                                mock_lsb_release,
+                                mock_nagios_password):
         mock_lsb_release.return_value = {'DISTRIB_CODENAME': 'bionic'}
         my_mock = mock.Mock()
-        self.leader_get.return_value = "1234"
+        mock_nagios_password.return_value = "1234"
         self.is_leader.return_value = True
         mock_get_db_helper.return_value = my_mock
+        mock_cursor = mock.Mock()
+        my_mock.connection.cursor.return_value = mock_cursor
 
         percona_utils.create_nagios_user()
         my_mock.select.assert_called_once_with(
             "SELECT EXISTS(SELECT 1 FROM mysql.user WHERE user = 'nagios')"
         )
         my_mock.execute.assert_has_calls([
-            mock.call("CREATE USER 'nagios'@'localhost';"),
+            mock.call(
+                "CREATE USER 'nagios'@'localhost' IDENTIFIED BY '1234';"),
         ])
-        my_mock.set_mysql_password.assert_called_once_with('nagios', '1234')
+        mock_cursor.execute.assert_not_called()
 
         class OperationalError(Exception):
             pass
@@ -573,6 +578,16 @@ class UtilsTests(CharmTestCase):
         my_mock.execute.side_effect = mysql_create_user
         with self.assertRaises(OperationalError):
             percona_utils.create_nagios_user()
+        mock_cursor.execute.assert_not_called()
+
+        my_mock.select.return_value = True
+        percona_utils.create_nagios_user()
+        mock_cursor.execute.assert_has_calls([
+            mock.call('UPDATE mysql.user SET authentication_string = '
+                      'PASSWORD( %s ) WHERE user = %s;', ('1234', 'nagios')),
+            mock.call('FLUSH PRIVILEGES;'),
+        ])
+        my_mock.connection.commit.assert_called_once_with()
 
     def test_get_nrpe_threads_connected_thresholds(self):
         """Test function for getting and verifying threshold values."""