openstack
/
charm-mysql-innodb-cluster
Code Issues Proposed changes

Handle attempts to write to a RO node 

Though the charm took pains to avoid it there are states the charm can
get into in which it attempts to write to a read only node.

This change catches the OperationalError error code 1290: "The MySQL server
is running with the --super-read-only option so it cannot execute this
statement" gracefully.

Change-Id: If9188f5e72701f4bd7575b09217f355fa3d505a2
Closes-Bug: #1882205
This commit is contained in:
David Ames 2020-10-06 15:58:35 -07:00
parent a005409974
commit 6cc500974d
4 changed files with 203 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

106
src/lib/charm/openstack/mysql_innodb_cluster.py
View File

@ -194,6 +194,9 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
# For caching cluster status information
_cached_cluster_status = None
_read_only_error = 1290
_user_create_failed = 1396
@property
def mysqlsh_bin(self):
"""Determine binary path for MySQL Shell.
@ -488,8 +491,8 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
:param cluster_password: Cluster user's password
:type cluster_password: str
:side effect: Executes SQL to create DB user
:returns: This function is called for its side effect
:rtype: None
:returns: True if successful, False if there are failures
:rtype: Boolean
"""
SQL_CLUSTER_USER_CREATE = (
"CREATE USER '{user}'@'{host}' "
@ -517,22 +520,34 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
host=address,
password=cluster_password)
)
except mysql.MySQLdb._exceptions.OperationalError:
ch_core.hookenv.log("User {} already exists."
.format(cluster_user), "WARNING")
m_helper.execute(SQL_CLUSTER_USER_GRANT.format(
permissions="ALL PRIVILEGES",
user=cluster_user,
host=address)
)
m_helper.execute(SQL_CLUSTER_USER_GRANT.format(
permissions="GRANT OPTION",
user=cluster_user,
host=address)
)
m_helper.execute(SQL_CLUSTER_USER_GRANT.format(
permissions="ALL PRIVILEGES",
user=cluster_user,
host=address)
)
m_helper.execute(SQL_CLUSTER_USER_GRANT.format(
permissions="GRANT OPTION",
user=cluster_user,
host=address)
)
m_helper.execute("flush privileges")
m_helper.execute("flush privileges")
except mysql.MySQLdb._exceptions.OperationalError as e:
if e.args[0] == self._read_only_error:
ch_core.hookenv.log(
"Attempted to write to the RO node: {} in "
"configure_db_for_hosts. Skipping."
.format(m_helper.connection.get_host_info()),
"WARNING")
return False
if e.args[0] == self._user_create_failed:
ch_core.hookenv.log(
"User {} exists."
.format(cluster_user), "WARNING")
continue
else:
raise
return True
def configure_instance(self, address):
"""Configure MySQL instance for clustering.
@ -874,10 +889,12 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
"create cluster user for {}.".format(address))
# Make sure we have the user in the DB
for unit in cluster.all_joined_units:
self.create_cluster_user(
unit.received['cluster-address'],
unit.received['cluster-user'],
unit.received['cluster-password'])
if not self.create_cluster_user(
unit.received['cluster-address'],
unit.received['cluster-user'],
unit.received['cluster-password']):
raise Exception(
"Not all cluster users created.")
self.configure_instance(address)
self.add_instance_to_cluster(address)
@ -1119,17 +1136,20 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
allowed_units = " ".join(
[x.unit_name for x in unit.relation.joined_units])
interface.set_db_connection_info(
unit.relation.relation_id,
db_host,
password,
allowed_units=allowed_units,
prefix=prefix,
wait_timeout=self.options.wait_timeout,
ssl_ca=self.ssl_ca)
# Only set relation data if db/user create was successful
if password:
interface.set_db_connection_info(
unit.relation.relation_id,
db_host,
password,
allowed_units=allowed_units,
prefix=prefix,
wait_timeout=self.options.wait_timeout,
ssl_ca=self.ssl_ca)
# Validate that all attempts succeeded.
# i.e. We were not attempting writes during a topology change.
# i.e. We were not attempting writes during a topology change,
# we are not attempting to write to a read only node.
if all(completed):
return True
return False
@ -1179,7 +1199,18 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
return
for host in hosts:
password = rw_helper.configure_db(host, database, username)
try:
password = rw_helper.configure_db(host, database, username)
except mysql.MySQLdb._exceptions.OperationalError as e:
if e.args[0] == self._read_only_error:
password = None
ch_core.hookenv.log(
"Attempted to write to the RO node: {} in "
"configure_db_for_hosts. Skipping."
.format(rw_helper.connection.get_host_info()),
"WARNING")
else:
raise
return password
@ -1225,7 +1256,18 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
return
for host in hosts:
password = rw_helper.configure_router(host, username)
try:
password = rw_helper.configure_router(host, username)
except mysql.MySQLdb._exceptions.OperationalError as e:
if e.args[0] == self._read_only_error:
password = None
ch_core.hookenv.log(
"Attempted to write to the RO node: {} in "
"configure_db_router. Skipping."
.format(rw_helper.connection.get_host_info()),
"WARNING")
else:
raise
return password

21
src/reactive/mysql_innodb_cluster_handlers.py
View File

@ -55,10 +55,13 @@ def create_local_cluster_user():
"""
ch_core.hookenv.log("Creating local cluster user.", "DEBUG")
with charm.provide_charm_instance() as instance:
instance.create_cluster_user(
instance.cluster_address,
instance.cluster_user,
instance.cluster_password)
if not instance.create_cluster_user(
instance.cluster_address,
instance.cluster_user,
instance.cluster_password):
ch_core.hookenv.log("Local cluster user was not created.",
"WARNING")
return
reactive.set_flag("local.cluster.user-created")
instance.assess_status()
@ -100,10 +103,12 @@ def create_remote_cluster_user():
ch_core.hookenv.log("Creating remote users.", "DEBUG")
with charm.provide_charm_instance() as instance:
for unit in cluster.all_joined_units:
instance.create_cluster_user(
unit.received['cluster-address'],
unit.received['cluster-user'],
unit.received['cluster-password'])
if not instance.create_cluster_user(
unit.received['cluster-address'],
unit.received['cluster-user'],
unit.received['cluster-password']):
ch_core.hookenv.log("Not all remote users created.", "WARNING")
return
# Optimize clustering by causing a cluster relation changed
cluster.set_unit_configure_ready()

110
unit_tests/test_lib_charm_openstack_mysql_innodb_cluster.py
View File

@ -21,6 +21,13 @@ import charms_openstack.test_utils as test_utils
import charm.openstack.mysql_innodb_cluster as mysql_innodb_cluster
class FakeException(Exception):
def __init__(self, code, message):
self.code = code
self.message = message
class TestMySQLInnoDBClusterProperties(test_utils.PatchHelper):
def setUp(self):
@ -453,15 +460,8 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
_localhost = "localhost"
_helper.reset_mock()
self.get_relation_ip.return_value = _addr
midbc.create_cluster_user(_addr, _user, _pass)
midbc.create_cluster_user(_localhost, _user, _pass)
_calls = [
mock.call("CREATE USER '{}'@'{}' IDENTIFIED BY '{}'"
.format(_user, _addr, _pass)),
mock.call("GRANT ALL PRIVILEGES ON *.* TO '{}'@'{}'"
.format(_user, _addr)),
mock.call("GRANT GRANT OPTION ON *.* TO '{}'@'{}'"
.format(_user, _addr)),
mock.call('flush privileges'),
mock.call("CREATE USER '{}'@'{}' IDENTIFIED BY '{}'"
.format(_user, _localhost, _pass)),
mock.call("GRANT ALL PRIVILEGES ON *.* TO '{}'@'{}'"
@ -472,6 +472,30 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
_helper.execute.assert_has_calls(
_calls, any_order=True)
# Exception handling
self.patch_object(
mysql_innodb_cluster.mysql.MySQLdb, "_exceptions")
self._exceptions.OperationalError = FakeException
# User Exists
_helper.reset_mock()
_helper.execute.side_effect = (
self._exceptions.OperationalError(1396, "User exists"))
self.assertTrue(midbc.create_cluster_user(_localhost, _user, _pass))
# Read only node
_helper.reset_mock()
_helper.execute.side_effect = (
self._exceptions.OperationalError(1290, "Super read only"))
self.assertFalse(midbc.create_cluster_user(_localhost, _user, _pass))
# Unhandled Exception
_helper.reset_mock()
_helper.execute.side_effect = (
self._exceptions.OperationalError(99999, "BROKEN"))
with self.assertRaises(FakeException):
midbc.create_cluster_user(_localhost, _user, _pass)
def test_configure_instance(self):
_pass = "clusterpass"
_addr = "10.10.30.30"
@ -694,6 +718,18 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
self.interface.set_db_connection_info.assert_has_calls(
_set_calls, any_order=True)
# DB/User create is unsuccessful
midbc.configure_db_for_hosts.reset_mock()
midbc.configure_db_for_hosts.side_effect = None
midbc.configure_db_for_hosts.return_value = None
midbc.configure_db_router.side_effect = None
midbc.configure_db_router.return_value = None
# Execute the function under test expect incomplete
self.interface.set_db_connection_info.reset_mock()
self.assertFalse(midbc.create_databases_and_users(self.interface))
self.interface.set_db_connection_info.assert_not_called()
def test_create_databases_and_users_db_router(self):
# The test setup is a bit convoluted and requires mimicking reactive,
# however, this is the heart of the charm and therefore deserves to
@ -820,6 +856,18 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
self.interface.set_db_connection_info.assert_has_calls(
_set_calls, any_order=True)
# DB/User create is unsuccessful
midbc.configure_db_router.reset_mock()
midbc.configure_db_for_hosts.side_effect = None
midbc.configure_db_for_hosts.return_value = None
midbc.configure_db_router.side_effect = None
midbc.configure_db_router.return_value = None
# Execute the function under test expect incomplete
self.interface.set_db_connection_info.reset_mock()
self.assertFalse(midbc.create_databases_and_users(self.interface))
self.interface.set_db_connection_info.assert_not_called()
def test_configure_db_for_hosts(self):
_db = "db"
_user = "user"
@ -857,6 +905,26 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
_helper.configure_db.assert_has_calls(
_calls, any_order=True)
# Exception handling
self.patch_object(
mysql_innodb_cluster.mysql.MySQLdb, "_exceptions")
self._exceptions.OperationalError = FakeException
# Super read only
_helper.reset_mock()
_helper.configure_db.side_effect = (
self._exceptions.OperationalError(1290, "Super REad only"))
self.assertEqual(
None,
midbc.configure_db_for_hosts(_json_addrs, _db, _user))
# Unhandled Exception
_helper.reset_mock()
_helper.configure_db.side_effect = (
self._exceptions.OperationalError(999, "BROKEN"))
with self.assertRaises(FakeException):
midbc.configure_db_for_hosts(_json_addrs, _db, _user)
def test_configure_db_router(self):
_user = "user"
_addr = "10.10.90.90"
@ -893,6 +961,26 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
_helper.configure_router.assert_has_calls(
_calls, any_order=True)
# Exception handling
self.patch_object(
mysql_innodb_cluster.mysql.MySQLdb, "_exceptions")
self._exceptions.OperationalError = FakeException
# Super read only
_helper.reset_mock()
_helper.configure_router.side_effect = (
self._exceptions.OperationalError(1290, "Super REad only"))
self.assertEqual(
None,
midbc.configure_db_router(_json_addrs, _user))
# Unhandled Exception
_helper.reset_mock()
_helper.configure_router.side_effect = (
self._exceptions.OperationalError(999, "BROKEN"))
with self.assertRaises(FakeException):
midbc.configure_db_router(_json_addrs, _user)
def test_states_to_check(self):
self.patch_object(
mysql_innodb_cluster.charms_openstack.charm.OpenStackCharm,
@ -1445,6 +1533,7 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
"cluster-password": _pass,
}
_create_cluster_user = mock.MagicMock()
_create_cluster_user.return_value = True
midbc.create_cluster_user = _create_cluster_user
_configure_instance = mock.MagicMock()
midbc.configure_instance = _configure_instance
@ -1456,3 +1545,8 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
_remote_addr, _user, _pass)
_configure_instance.assert_called_once_with(_remote_addr)
_add_instance_to_cluster.assert_called_once_with(_remote_addr)
# Not all users created
_create_cluster_user.return_value = False
with self.assertRaises(Exception):
midbc.configure_and_add_instance(address=_remote_addr)

14
unit_tests/test_mysql_innodb_cluster_handlers.py
View File

@ -143,6 +143,7 @@ class TestMySQLInnoDBClusterHandlers(test_utils.PatchHelper):
self.set_flag.assert_called_once_with("charm.installed")
def test_create_local_cluster_user(self):
self.midbc.create_cluster_user.return_value = True
handlers.create_local_cluster_user()
self.midbc.create_cluster_user.assert_called_once_with(
self.midbc.cluster_address,
@ -150,6 +151,12 @@ class TestMySQLInnoDBClusterHandlers(test_utils.PatchHelper):
self.midbc.cluster_password)
self.set_flag.assert_called_once_with("local.cluster.user-created")
# Not successful
self.midbc.create_cluster_user.return_value = False
self.set_flag.reset_mock()
handlers.create_local_cluster_user()
self.set_flag.assert_not_called()
def test_send_cluster_connection_info(self):
self.endpoint_from_flag.return_value = self.cluster
handlers.send_cluster_connection_info()
@ -165,6 +172,7 @@ class TestMySQLInnoDBClusterHandlers(test_utils.PatchHelper):
self.data = {"cluster-address": _addr,
"cluster-user": _user,
"cluster-password": _pass}
self.midbc.create_cluster_user.return_value = True
self.endpoint_from_flag.return_value = self.cluster
handlers.create_remote_cluster_user()
self.midbc.create_cluster_user.assert_called_once_with(
@ -173,6 +181,12 @@ class TestMySQLInnoDBClusterHandlers(test_utils.PatchHelper):
self.set_flag.assert_called_once_with(
"local.cluster.all-users-created")
# Not successful
self.midbc.create_cluster_user.return_value = False
self.cluster.set_unit_configure_ready.reset_mock()
handlers.create_remote_cluster_user()
self.cluster.set_unit_configure_ready.assert_not_called()
def test_initialize_cluster(self):
handlers.initialize_cluster()
self.midbc.configure_instance.assert_called_once_with(