From 3bbcaa0b37b48503e61f68e5b3d247d624bc7e3d Mon Sep 17 00:00:00 2001 From: Billy Olsen Date: Fri, 18 Mar 2022 15:11:42 -0700 Subject: [PATCH] Configure mysqlrouter.conf sections based on wildcards Upon bootstrap, the mysqlrouter will create a mysqlrouter.conf file for the cluster it is connecting to. This creates sections such as metadata_cache:, routing:_rw, routing:_ro, etc. The cluster name is not provided on the mysql-router interface so this information is not available in determining the correct section name. Since the mysql-router is designed to work with a single cluster, the need to update the interface which in turn requires the user to update a number of deployed charms in the environment, an approach is taken to allow regular expressions to be used when matching the section name. There is some risk to this in that it requires that future edits carefully consider the possible section names when future sections are added. However, this developer cost is traded off in order to ease the burden of operators. For the upgrade scenario, this patch also checks to see if the file rendered on disk contains multiple 'metadata_cache' sections, and if so rewrites the mysqlrouter.conf file with the hardcoded metadata_cache:jujuCluster section removed. Closes-Bug: #1927981 Change-Id: Iad44744ad01c0b6429fbafb041e6fc11887dbfb9 (cherry picked from commit 5a2da1800272af2875064a295b8ba7044cf7bcea) --- src/lib/charm/openstack/mysql_router.py | 60 +++++++++-- src/tests/bundles/focal-full.yaml | 1 + .../test_lib_charm_openstack_mysql_router.py | 100 ++++++++++++++++-- 3 files changed, 146 insertions(+), 15 deletions(-) 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)