Merge "Stop using AUTOMATIC node whitelist"

This commit is contained in:
Zuul 2021-06-08 20:56:00 +00:00 committed by Gerrit Code Review
commit 8378c9dd89
8 changed files with 423 additions and 11 deletions

View File

@ -49,6 +49,15 @@ done once Vault has been initialised and has a root CA:
See the [vault][vault-charm-readme] charm README for more information.
## Adding a unit on a new subnet
If a new unit is added after the cluster has already formed and the new unit
is on different subnet to any of the existing units then the following actions
are needed to add the unit to the cluster:
juju run-action mysql-innodb-cluster/leader update-unit-acls
juju run-action mysql-innodb-cluster/leader add-instance address=<address of new unit>
## Actions
This section lists Juju [actions][juju-docs-actions] supported by the charm.
@ -65,6 +74,7 @@ If the charm is not deployed then see file `actions.yaml`.
* `remove-instance`
* `restore-mysqldump`
* `set-cluster-option`
* `update-unit-acls`
# Documentation

View File

@ -73,3 +73,11 @@ set-cluster-option:
description: Option value
description: |
Set an option on the cluster.
update-unit-acls:
description: |
This action should only be needed if a unit was added to the cluster on a
new subnet. In this case *group_replication_ip_allowlist* need updating to
include the subnet. Run *add-instance* to add in the new unit once this
action succesfully completes.
WARNING Group replication is stopped on each unit to allow the update to
happen.

View File

@ -341,6 +341,16 @@ def set_cluster_option(args):
ch_core.hookenv.action_fail("Set cluster option failed")
def update_unit_acls(args):
"""Update IP allow list on each node in cluster.
Update IP allow list on each node in cluster. At present this can only
be done when replication is stopped.
"""
with charm.provide_charm_instance() as instance:
instance.update_acls()
# A dictionary of all the defined actions to callables (which take
# parsed arguments).
ACTIONS = {"mysqldump": mysqldump, "cluster-status": cluster_status,
@ -351,10 +361,14 @@ ACTIONS = {"mysqldump": mysqldump, "cluster-status": cluster_status,
"rejoin-instance": rejoin_instance,
"add-instance": add_instance,
"remove-instance": remove_instance,
"cluster-rescan": cluster_rescan}
"cluster-rescan": cluster_rescan,
"update-unit-acls": update_unit_acls}
def main(args):
# Manually trigger any register atstart events to ensure all endpoints
# are correctly setup.
ch_core.hookenv._run_atstart()
action_name = os.path.basename(args[0])
try:
action = ACTIONS[action_name]
@ -370,6 +384,7 @@ def main(args):
"traceback": traceback.format_exc()})
ch_core.hookenv.action_fail(
"{} action failed.".format(action_name))
ch_core.hookenv._run_atexit()
if __name__ == "__main__":

View File

@ -0,0 +1 @@
actions.py

View File

@ -14,6 +14,7 @@
import base64
import datetime
import ipaddress
import json
import os
import subprocess
@ -569,6 +570,50 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
leadership.leader_set({
make_cluster_instance_configured_key(address): True})
def get_cluster_subnets(self):
"""Return a list of subnets covering all units.
:returns: List of subnets
:rtype: List
"""
ips = self.cluster_peer_addresses
ips.append(self.cluster_address)
return list(set([ch_net_ip.resolve_network_cidr(ip) for ip in ips]))
def generate_ip_allowlist_str(self):
"""Generate an ip allow list to permit all units to access each other.
Generate an ip allow list to permit all units to access each other
and to allow localhost connections.
:returns: Value for ip allow list for this cluster
:rtype: str
"""
return "127.0.0.1,::1,{}".format(
",".join(sorted(self.get_cluster_subnets())))
def reached_quorum(self):
"""Check if all peer units have joined.
Compare the number of units reported in goal state with the number of
units that have advertised their cluster address on the peer relation.
:returns: Whether all peer units have joined
:rtype: Boolean
"""
cluster = reactive.endpoint_from_flag("cluster.available")
peer_addresses = [u.received['cluster-address']
for u in cluster.all_joined_units
if u.received['cluster-address']]
ch_core.hookenv.log(
"Found peers: {}".format(",".join(peer_addresses)),
"DEBUG")
expected_unit_count = len(list(ch_core.hookenv.expected_peer_units()))
ch_core.hookenv.log(
"Expect {} peers".format(expected_unit_count),
"DEBUG")
return len(peer_addresses) >= expected_unit_count
def create_cluster(self):
"""Create the MySQL InnoDB cluster.
@ -595,11 +640,11 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
_script = (
"shell.connect('{}:{}@{}')\n"
"cluster = dba.create_cluster('{}', {{'autoRejoinTries': '{}', "
"'expelTimeout': '{}'}})"
"'expelTimeout': '{}', 'ipAllowlist': '{}'}})"
.format(
self.cluster_user, self.cluster_password, self.cluster_address,
self.options.cluster_name, self.options.auto_rejoin_tries,
self.options.expel_timeout
self.options.expel_timeout, self.generate_ip_allowlist_str()
))
ch_core.hookenv.log("Creating cluster: {}."
.format(self.options.cluster_name), "INFO")
@ -648,6 +693,70 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
# Reraise for action handling
raise e
def get_ip_allowlist_str_from_db(self, m_helper=None):
"""Helper for retrieving ip allow list
:param m_helper: Helper for connecting to DB.
:type m_helper: mysql.MySQL8Helper
:returns: Current setting of group_replication_ip_allowlist
:rtype: str
"""
if not m_helper:
m_helper = self.get_db_helper()
m_helper.connect(password=self.mysql_password)
query_out = m_helper.select(
"SHOW GLOBAL VARIABLES LIKE 'group_replication_ip_allowlist'")
assert len(query_out) == 1 and len(query_out[0]) == 2, \
"ip allowlist query returned unexpected result: {}".format(
query_out)
return query_out[0][1]
def get_ip_allowlist_list_from_db(self, m_helper=None):
"""Extract group_replication_ip_allowlist from db.
:param m_helper: Helper for connecting to DB.
:type m_helper: mysql.MySQL8Helper
:returns: Current setting of group_replication_ip_allowlist
:rtype: List[str]
"""
allow_list = self.get_ip_allowlist_str_from_db(m_helper=m_helper)
return allow_list.split(',')
def is_address_in_replication_ip_allowlist(self, address,
ip_allowlist=None):
"""Check if address is in ip allow list.
:param address: IP address to check against.
:type address: str
:param ip_allowlist: IP/CIDRs to check against.
:type ip_allowlist: List[str]
:returns: Whether address is allowed to connect to cluster.
:rtype: Boolean
"""
ip_allowlist = ip_allowlist or self.get_ip_allowlist_list_from_db()
allowed = False
for net in ip_allowlist:
if net == 'AUTOMATIC':
if ipaddress.ip_address(address).is_private:
allowed = True
elif ch_net_ip.is_address_in_network(net, address):
allowed = True
return allowed
def get_denied_peers(self):
"""List of peer units that are not in the IP allow list.
:returns: Units which are not permitted to connect to cluster.
:rtype: List[str]
"""
ip_allowlist = self.get_ip_allowlist_list_from_db()
denied_peers = []
for addr in self.cluster_peer_addresses:
if not self.is_address_in_replication_ip_allowlist(addr,
ip_allowlist):
denied_peers.append(addr)
return denied_peers
def add_instance_to_cluster(self, address):
"""Add MySQL instance to the cluster.
@ -674,11 +783,12 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
"{{'user': '{user}', 'host': '{addr}', 'password': '{pw}', "
"'port': '3306'}},"
"{{'recoveryMethod': 'clone', 'waitRecovery': '2', "
"'interactive': False}})"
"'interactive': False, 'ipAllowlist': '{allowlist}'}})"
.format(
user=self.cluster_user, pw=self.cluster_password,
caddr=_primary or self.cluster_address,
name=self.options.cluster_name, addr=address))
name=self.options.cluster_name, addr=address,
allowlist=self.generate_ip_allowlist_str()))
try:
output = self.run_mysqlsh_script(_script)
except subprocess.CalledProcessError as e:
@ -1392,7 +1502,6 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
# Check the state of the cluster. nocache=True will get live info
_cluster_status = self.get_cluster_status_summary(nocache=True)
# LP Bug #1917337
if _cluster_status is None:
ch_core.hookenv.status_set(
@ -1401,6 +1510,16 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
"Please check logs for details.")
return
# Check all peers are allowed to connect to this unit
denied_peers = self.get_denied_peers()
if denied_peers:
ch_core.hookenv.status_set(
"blocked",
"Units not allowed to replicate with this unit: {}. See "
"update-unit-acls action.".format(
",".join(denied_peers)))
return
if "OK" not in _cluster_status:
ch_core.hookenv.status_set(
"blocked",
@ -1797,3 +1916,86 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm):
_new_leader_settings[key] = _leader_settings[key]
leadership.leader_set(_new_leader_settings)
@tenacity.retry(wait=tenacity.wait_fixed(6),
reraise=True,
stop=tenacity.stop_after_attempt(5),
retry=tenacity.retry_if_exception_type(ValueError))
def wait_for_cluster_state(self, m_helper, node_address, target_state):
"""Wait for replication member to reach given state.
Wait for a given member of the cluster, indicated by node_address, to
be in the target_state.
:param m_helper: Helper for connecting to DB.
:type m_helper: mysql.MySQL8Helper
:param node_address: IP address of node to query.
:type node_address: str
:param target_state: State to wait for.
:type target_state: str
:raises: ValueError
"""
CLUSTER_QUERY = """
SELECT MEMBER_STATE
FROM performance_schema.replication_group_members
where MEMBER_HOST='{address}'
"""
query_out = m_helper.select(CLUSTER_QUERY.format(address=node_address))
assert len(query_out) == 1 and len(query_out[0]) == 1, \
"Cluster query returned unexpected result for {}: {}".format(
node_address,
query_out)
if query_out[0][0] != target_state:
raise ValueError
def get_clustered_addresses(self):
"""Get the cluster addresses of all units which have joined cluster.
:returns: List of IP addresses
:rtype: List[str]
"""
_leader_settings = ch_core.hookenv.leader_get()
all_addresses = self.cluster_peer_addresses
all_addresses.append(self.cluster_address)
clustered_addresses = []
for address in all_addresses:
leader_key = make_cluster_instance_clustered_key(address)
_value = _leader_settings.get(leader_key)
if _value and ch_core.strutils.bool_from_string(_value):
clustered_addresses.append(address)
return clustered_addresses
def update_acls(self):
"""Update IP allow list on each node in cluster.
Update IP allow list on each node in cluster. At present this can only
be done when replication is stopped.
"""
ip_allow_list = self.generate_ip_allowlist_str()
for address in self.get_clustered_addresses():
m_helper = mysql.MySQL8Helper(
'unused',
'unused',
host=address,
migrate_passwd_to_leader_storage=False,
delete_ondisk_passwd_file=False,
user=self.cluster_user,
password=self.cluster_password)
# Bug in helper causes it to use root user irrespective of the user
# setting when mysql.MySQL8Helper is instantiated.
m_helper.connect(user=self.cluster_user)
current_allow_list = self.get_ip_allowlist_str_from_db(
m_helper)
if current_allow_list == ip_allow_list:
ch_core.hookenv.log(
("group_replication_ip_allowlist does not need updating "
"on {}").format(address),
"DEBUG")
else:
m_helper.execute("STOP GROUP_REPLICATION")
self.wait_for_cluster_state(m_helper, address, 'OFFLINE')
m_helper.execute(
"SET GLOBAL group_replication_ip_allowlist = '{}'".format(
ip_allow_list))
m_helper.execute("START GROUP_REPLICATION")
self.wait_for_cluster_state(m_helper, address, 'ONLINE')

View File

@ -129,6 +129,25 @@ def create_remote_cluster_user():
instance.assess_status()
@reactive.when('cluster.available')
def check_quorum():
"""Check that all units have sent their cluster address.
When the cluster is created an ip allow list is set. This cannot be
updated while replication is running. To avoid the need to update
it after cluster creation wait for all cluster addresses to be present.
NOTE: The update-unit-acls action can be run if a unit on a new subnet
is added to an existing cluster.
"""
with charm.provide_charm_instance() as instance:
if instance.reached_quorum():
ch_core.hookenv.log("Reached quorum", "DEBUG")
reactive.set_flag('local.cluster.quorum-reached')
else:
ch_core.hookenv.log("Quorum not reached", "DEBUG")
@reactive.when('local.cluster.quorum-reached')
@reactive.when('leadership.is_leader')
@reactive.when('local.cluster.user-created')
@reactive.when_not('leadership.set.cluster-created')

View File

@ -549,6 +549,59 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
{"cluster-instance-configured-{}"
.format(_addr.replace(".", "-")): True})
@mock.patch(('charm.openstack.mysql_innodb_cluster.'
'MySQLInnoDBClusterCharm.cluster_peer_addresses'),
new_callable=mock.PropertyMock)
@mock.patch(('charm.openstack.mysql_innodb_cluster.'
'MySQLInnoDBClusterCharm.cluster_address'),
new_callable=mock.PropertyMock)
def test_get_cluster_subnets(self, cluster_address,
cluster_peer_addresses):
self.patch_object(
mysql_innodb_cluster.ch_net_ip,
"resolve_network_cidr",
side_effect=lambda x: '{}.{}.0.0/24'.format(
x.split('.')[0],
x.split('.')[1]))
cluster_peer_addresses.return_value = [
'10.0.0.13', '10.0.0.11', '10.10.0.10']
cluster_address.return_value = '10.0.0.12'
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
self.assertEqual(
midbc.get_cluster_subnets(),
['10.10.0.0/24', '10.0.0.0/24'])
def test_generate_ip_allowlist_str(self):
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
midbc.get_cluster_subnets = lambda: ['10.0.0.10', '10.0.0.11']
self.assertEqual(
midbc.generate_ip_allowlist_str(),
'127.0.0.1,::1,10.0.0.10,10.0.0.11')
def test_reached_quorum(self):
self.patch_object(
mysql_innodb_cluster.ch_core.hookenv, "expected_peer_units",
return_value=['u1'])
self.patch_object(
mysql_innodb_cluster.reactive, "endpoint_from_flag",
return_value=self.cluster)
self.data = {
"cluster-address": "10.0.0.11"}
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
self.assertTrue(midbc.reached_quorum())
def test_reached_quorum_fail(self):
self.patch_object(
mysql_innodb_cluster.ch_core.hookenv, "expected_peer_units",
return_value=['u1', 'u2'])
self.patch_object(
mysql_innodb_cluster.reactive, "endpoint_from_flag",
return_value=self.cluster)
self.data = {
"cluster-address": "10.0.0.11"}
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
self.assertFalse(midbc.reached_quorum())
def test_restart_instance(self):
_pass = "clusterpass"
_addr = "10.10.30.30"
@ -576,6 +629,7 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
_addr = "10.10.40.40"
_name = "jujuCluster"
_tries = 500
_allowlist = '10.0.0.0/24'
_expel_timeout = 5
self.get_relation_ip.return_value = _addr
self.data = {"cluster-password": _pass}
@ -589,14 +643,15 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
midbc.options.cluster_name = _name
midbc.options.auto_rejoin_tries = _tries
midbc.options.expel_timeout = _expel_timeout
midbc.generate_ip_allowlist_str = lambda: _allowlist
_script = (
"shell.connect('{}:{}@{}')\n"
"cluster = dba.create_cluster('{}', {{'autoRejoinTries': '{}', "
"'expelTimeout': '{}'}})"
"'expelTimeout': '{}', 'ipAllowlist': '{}'}})"
.format(
midbc.cluster_user, midbc.cluster_password,
midbc.cluster_address, midbc.cluster_name, _tries,
_expel_timeout))
_expel_timeout, _allowlist))
midbc.create_cluster()
_is_flag_set_calls = [
@ -616,6 +671,7 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
_local_addr = "10.10.50.50"
_remote_addr = "10.10.60.60"
_name = "theCluster"
_allowlist = '10.0.0.0/24'
self.get_relation_ip.return_value = _local_addr
self.get_relation_ip.return_value = _local_addr
self.data = {"cluster-password": _pass}
@ -629,17 +685,20 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
midbc.wait_until_connectable = mock.MagicMock()
midbc.run_mysqlsh_script = mock.MagicMock()
midbc.options.cluster_name = _name
midbc.is_address_in_replication_ip_allowlist = lambda x: True
midbc.generate_ip_allowlist_str = lambda: _allowlist
_script = (
"shell.connect('{}:{}@{}')\n"
"cluster = dba.get_cluster('{}')\n"
"cluster.add_instance("
"{{'user': '{}', 'host': '{}', 'password': '{}', 'port': '3306'}},"
"{{'recoveryMethod': 'clone', 'waitRecovery': '2', "
"'interactive': False}})"
"'interactive': False, 'ipAllowlist': '{}'}})"
.format(
midbc.cluster_user, midbc.cluster_password,
midbc.cluster_address, midbc.cluster_name,
midbc.cluster_user, _remote_addr, midbc.cluster_password))
midbc.cluster_user, _remote_addr, midbc.cluster_password,
_allowlist))
midbc.add_instance_to_cluster(_remote_addr)
self.is_flag_set.assert_called_once_with(
@ -1087,6 +1146,7 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
midbc.get_cluster_status_summary = _status
midbc.get_cluster_status_text = _status_text
midbc.get_cluster_instance_mode = _status_mode
midbc.get_denied_peers = lambda: []
midbc._assess_status()
self.assertEqual(4, len(_check.mock_calls))
@ -1480,6 +1540,49 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
self.assertEqual(_string, midbc.set_cluster_option(_key, _value))
midbc.run_mysqlsh_script.assert_called_once_with(_script)
def test_get_ip_allowlist_str_from_db(self):
mock_m_helper = mock.MagicMock()
mock_m_helper.select.return_value = [
['group_replication_ip_allowlist', '10.0.0.10/24']]
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
self.assertEqual(
midbc.get_ip_allowlist_str_from_db(mock_m_helper),
'10.0.0.10/24')
mock_m_helper.select.assert_called_once_with(
"SHOW GLOBAL VARIABLES LIKE 'group_replication_ip_allowlist'")
mock_m_helper.select.return_value = []
with self.assertRaises(AssertionError):
midbc.get_ip_allowlist_str_from_db(mock_m_helper)
def test_get_ip_allowlist_list_from_db(self):
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
midbc.get_cluster_rw_db_helper = mock.MagicMock(return_value=None)
midbc.get_ip_allowlist_str_from_db = \
lambda m_helper: '::1,10.0.0.20/24'
self.assertEqual(
midbc.get_ip_allowlist_list_from_db(),
['::1', '10.0.0.20/24'])
def test_is_address_in_replication_ip_allowlist(self):
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
self.assertTrue(midbc.is_address_in_replication_ip_allowlist(
'10.0.0.10',
['127.0.0.1', '10.0.0.10/24']))
@mock.patch(('charm.openstack.mysql_innodb_cluster.'
'MySQLInnoDBClusterCharm.cluster_peer_addresses'),
new_callable=mock.PropertyMock)
def test_get_denied_peers(self, cluster_peer_addresses):
def addr_in_list(addr, ip_allowlist):
return addr == '10.0.0.20'
cluster_peer_addresses.return_value = ['10.0.0.20', '10.20.0.20']
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
midbc.get_ip_allowlist_list_from_db = lambda: ['10.0.0.10/24']
midbc.is_address_in_replication_ip_allowlist = addr_in_list
self.assertEqual(
midbc.get_denied_peers(),
['10.20.0.20'])
def test_reboot_cluster_from_complete_outage(self):
_pass = "clusterpass"
_name = "theCluster"
@ -1668,3 +1771,54 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper):
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
midbc.update_dotted_flags()
self.leader_set.assert_called_once_with(_expected)
@mock.patch(('charm.openstack.mysql_innodb_cluster.'
'MySQLInnoDBClusterCharm.cluster_peer_addresses'),
new_callable=mock.PropertyMock)
@mock.patch(('charm.openstack.mysql_innodb_cluster.'
'MySQLInnoDBClusterCharm.cluster_address'),
new_callable=mock.PropertyMock)
def test_get_clustered_addresses(self, cluster_address,
cluster_peer_addresses):
_existing = {
"cluster-instance-clustered-10-5-0-10": True,
"cluster-instance-clustered-10-5-0-20": True,
"cluster-instance-clustered-10-5-0-30": False,
"key": "value",
"mysql.passwd": "must-not-change",
"cluster-instance-configured-10-5-0-40": True}
self.leader_get.side_effect = None
self.leader_get.return_value = _existing
cluster_address.return_value = '10.5.0.10'
cluster_peer_addresses.return_value = [
'10.5.0.20',
'10.5.0.30',
'10.5.0.40']
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
self.assertEqual(
midbc.get_clustered_addresses(),
['10.5.0.20', '10.5.0.10'])
def test_update_acls(self):
midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm()
midbc.generate_ip_allowlist_str = lambda: '10.0.0.0/24'
midbc.get_clustered_addresses = lambda: ['10.0.0.10']
midbc.get_ip_allowlist_str_from_db = lambda x: '10.0.0.0/24'
midbc.wait_for_cluster_state = lambda x, y, z: None
m_helper_mock = mock.MagicMock()
self.patch_object(
mysql_innodb_cluster.mysql, "MySQL8Helper",
return_value=m_helper_mock)
midbc.update_acls()
self.assertFalse(m_helper_mock.execute.called)
# Test update needed
m_helper_mock.reset_mock()
midbc.generate_ip_allowlist_str = lambda: '10.0.0.0/24,10.10.0.0/24'
midbc.update_acls()
m_helper_mock.execute.assert_has_calls([
mock.call('STOP GROUP_REPLICATION'),
mock.call(
("SET GLOBAL group_replication_ip_allowlist = "
"'10.0.0.0/24,10.10.0.0/24'")),
mock.call('START GROUP_REPLICATION')])

View File

@ -41,7 +41,9 @@ class TestRegisteredHooks(test_utils.TestRegisteredHooks):
"local.cluster.user-created", "cluster.connected",),
"create_remote_cluster_user": ("cluster.available",),
"initialize_cluster": (
"leadership.is_leader", "local.cluster.user-created",),
"local.cluster.quorum-reached",
"leadership.is_leader",
"local.cluster.user-created",),
"configure_instances_for_clustering": (
"leadership.is_leader", "local.cluster.all-users-created",
"leadership.set.cluster-created", "cluster.available"),
@ -83,6 +85,7 @@ class TestRegisteredHooks(test_utils.TestRegisteredHooks):
"certificates.ca.changed",
"certificates.certs.changed",),
"scale_in": ("endpoint.cluster.departed",),
"check_quorum": ("cluster.available",),
},
"when_not": {