diff --git a/src/lib/charm/openstack/mysql_router.py b/src/lib/charm/openstack/mysql_router.py index 19e0678..d451c86 100644 --- a/src/lib/charm/openstack/mysql_router.py +++ b/src/lib/charm/openstack/mysql_router.py @@ -15,6 +15,7 @@ import configparser import json import os +import re import subprocess import tenacity @@ -38,6 +39,15 @@ MYSQL_ROUTER_STARTED = "charm.mysqlrouter.started" DB_ROUTER_AVAILABLE = "db-router.available" DB_ROUTER_PROXY_AVAILABLE = "db-router.available.proxy" +# Section configuration search keys for setting configuration values +# Note, mysql object names can contain alphanumeric and $ characters. +DEFAULT_SECTION = 'DEFAULT' +METADATA_CACHE_SECTION = r'metadata_cache:[\w$]+$' +ROUTING_RW_SECTION = r'routing:[\w$]+_rw(? 1 and 'metadata_cache:jujuCluster' in sections: + ch_core.hookenv.log('Found multiple metadata_cache sections. ' + 'Removing hard-coded ' + 'metadata_cache:jujuCluster section', 'INFO') + config.remove_section('metadata_cache:jujuCluster') + ch_core.hookenv.log("Writing {}".format(self.mysqlrouter_conf)) + with open(self.mysqlrouter_conf, 'w') as configfile: + config.write(configfile) + + super(MySQLRouterCharm, self).upgrade_charm() + def get_db_helper(self): """Get an instance of the MySQLDB8Helper class. @@ -643,6 +675,13 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm): } } + mysqlrouter.conf file will have some headings which are targeted at + the mysql cluster name. Headings specified in the parameters may be a + regular expression for matching mysqlrouter configuration sections + which have variable information included in the name. Capturing + groups are not supported as the check is just used to see if the + section matches the regular expression. + :param parameters: Dictionary of parameters :type parameters: dict :side effect: Writes the mysqlrouter.conf file @@ -651,14 +690,23 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm): """ config = configparser.ConfigParser() config.read(self.mysqlrouter_conf) - for heading in parameters.keys(): - for param in parameters[heading].keys(): + + for heading, settings in parameters.items(): + for section in config.sections(): + if re.match(heading, section): + translated = section + break + else: + translated = heading + + for param, value in settings.items(): # BUG LP#1927981 - heading may not exist during a charm upgrade # Handle missing heading via direct assignment in except. try: - config[heading][param] = parameters[heading][param] + config[translated][param] = value except KeyError: - config[heading] = {param: parameters[heading][param]} + config[translated] = {param: value} + ch_core.hookenv.log("Writing {}".format( self.mysqlrouter_conf)) with open(self.mysqlrouter_conf, 'w') as configfile: @@ -683,13 +731,13 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm): return _parameters = { - "metadata_cache:jujuCluster": { + METADATA_CACHE_SECTION: { "ttl": str(self.options.ttl), "auth_cache_ttl": str(self.options.auth_cache_ttl), "auth_cache_refresh_interval": str(self.options.auth_cache_refresh_interval), }, - "DEFAULT": { + DEFAULT_SECTION: { "pid_file": self.mysqlrouter_pid_file, "max_connections": str(self.options.max_connections), } diff --git a/src/tests/bundles/focal-full.yaml b/src/tests/bundles/focal-full.yaml index 336d75f..32b97db 100644 --- a/src/tests/bundles/focal-full.yaml +++ b/src/tests/bundles/focal-full.yaml @@ -23,6 +23,7 @@ applications: num_units: 3 options: source: *openstack-origin + cluster-name: foo channel: latest/edge keystone: charm: ch:keystone diff --git a/unit_tests/test_lib_charm_openstack_mysql_router.py b/unit_tests/test_lib_charm_openstack_mysql_router.py index 2390f42..a51477f 100644 --- a/unit_tests/test_lib_charm_openstack_mysql_router.py +++ b/unit_tests/test_lib_charm_openstack_mysql_router.py @@ -57,6 +57,21 @@ class FakeException(Exception): return 1 +class FakeConfigParser(dict): + + def read(*args, **kwargs): + pass + + def write(*args, **kwargs): + pass + + def sections(self): + return self.keys() + + def remove_section(self, section): + self.pop(section, None) + + class TestMySQLRouterCharm(test_utils.PatchHelper): def setUp(self): @@ -663,13 +678,6 @@ class TestMySQLRouterCharm(test_utils.PatchHelper): def test_update_config_parameters_missing_heading(self): # test fix for Bug LP#1927981 - class FakeConfigParser(dict): - def read(*args, **kwargs): - pass - - def write(*args, **kwargs): - pass - current_config = {"DEFAULT": {"client_ssl_mode": "NONE"}} fake_config = FakeConfigParser(current_config) @@ -690,6 +698,54 @@ class TestMySQLRouterCharm(test_utils.PatchHelper): self.assertEqual(fake_config['metadata_cache:jujuCluster'], {"thing": "a-thing"}) + def test_update_config_parameters_regex(self): + # test fix for Bug LP#1927981 + current_config = { + "DEFAULT": {"client_ssl_mode": "NONE"}, + "metadata_cache:foo": { + "ttl": '5', + "auth_cache_ttl": '-1', + "auth_cache_refresh_interval": '2', + }, + "routing:foo_x_rw": { + "test": 'yes', + }, + "routing:foo_rw": { + "test": 'no', + } + } + fake_config = FakeConfigParser(current_config) + + self.patch_object(mysql_router.configparser, "ConfigParser", + return_value=fake_config) + + # metadata_cache:jujuCluster didn't exist in the previous config so the + # header needs to be created (c.f. BUG LP#1927981) + _params = { + "DEFAULT": {"client_ssl_mode": "PREFERRED"}, + mysql_router.METADATA_CACHE_SECTION: {"thing": "a-thing"}, + mysql_router.ROUTING_RW_SECTION: { + "test": True, + }, + mysql_router.ROUTING_X_RW_SECTION: { + "test": False, + } + } + + mrc = mysql_router.MySQLRouterCharm() + # should not throw a key error. + mrc.update_config_parameters(_params) + self.assertIn('metadata_cache:foo', fake_config) + self.assertNotIn(mysql_router.METADATA_CACHE_SECTION, fake_config) + self.assertEqual(fake_config['metadata_cache:foo'], + {"thing": "a-thing", "ttl": '5', + "auth_cache_ttl": '-1', + "auth_cache_refresh_interval": '2'}) + self.assertEqual(fake_config['routing:foo_x_rw'], + {"test": False}) + self.assertEqual(fake_config['routing:foo_rw'], + {"test": True}) + def test_config_changed(self): _config_data = { "ttl": '5', @@ -715,8 +771,8 @@ class TestMySQLRouterCharm(test_utils.PatchHelper): _metadata_config = _config_data.copy() _metadata_config.pop('max_connections') _params = { - 'metadata_cache:jujuCluster': _metadata_config, - 'DEFAULT': { + mysql_router.METADATA_CACHE_SECTION: _metadata_config, + mysql_router.DEFAULT_SECTION: { 'client_ssl_mode': "PASSTHROUGH", 'max_connections': _config_data['max_connections'], 'pid_file': '/run/mysql/mysqlrouter-foobar.pid' @@ -755,3 +811,29 @@ class TestMySQLRouterCharm(test_utils.PatchHelper): self.service_stop.assert_called_once_with(self.service_name) self.service_start.assert_called_once_with(self.service_name) _mock_check_mysql_connection.assert_called_once() + + def test_upgrade_charm_lp1927981(self): + # test fix for Bug LP#1927981 + current_config = { + "DEFAULT": {"client_ssl_mode": "NONE"}, + "metadata_cache:foo": { + "ttl": '5', + "auth_cache_ttl": '-1', + "auth_cache_refresh_interval": '2', + }, + "metadata_cache:jujuCluster": { + "ttl": '5', + }, + } + fake_config = FakeConfigParser(current_config) + + self.patch_object(mysql_router.charms_openstack.charm.OpenStackCharm, + 'upgrade_charm') + self.patch_object(mysql_router.configparser, "ConfigParser", + return_value=fake_config) + + mrc = mysql_router.MySQLRouterCharm() + # should not throw a key error. + mrc.upgrade_charm() + self.assertIn('metadata_cache:foo', fake_config) + self.assertNotIn('metadata_cache:.jujuCluster', fake_config)