Fix restarts during upgrade-charm hook
Upgrade-charm code needs to make various adjustments to the config files and restart the mysql-router service, but the code is currently running config-changed and getting stuck in a restart loop before upgrade-charm is able to run to make the config adjustments and invoke a successful restart. This change adds a condition to skip the execution of config_changed function when upgrade-charm hook runs, so upgrade_charm code takes care of the adjustments. Also, now upgrade-charm hook is guaranteed to restart the mysql-router service whenever any change to the config file is made as part of its execution, whereas before it wasn't doing so when making the adjustments of the custom upgrade_charm function, resulting in the possibility of no restarts after a charm upgrade. This change includes the now-abandoned change: https://review.opendev.org/848748 Co-Authored-By: Felipe Reyes <felipe.reyes@canonical.com> Closes-bug: #1980693 Closes-bug: #1979263 Related-bug: #1927981 Change-Id: If9d71bdb839c9c0ee3f4b33e4d44a5c93bdd13de
This commit is contained in:
Rodrigo Barbieri
parent
e3fcc5c96c
commit
6df798a939
|
|
@ -339,33 +339,37 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
|
|||
config = configparser.ConfigParser()
|
||||
config.read(self.mysqlrouter_conf)
|
||||
|
||||
# On upgrade set the unknown_config_option to warning (LP: #1971565)
|
||||
if 'unknown_config_option' not in config[DEFAULT_SECTION]:
|
||||
ch_core.hookenv.log(f'[{DEFAULT_SECTION}].unknown_config_option '
|
||||
f'is not present in the configuration file, '
|
||||
f'so setting it to "warning"')
|
||||
config[DEFAULT_SECTION]['unknown_config_option'] = 'warning'
|
||||
ch_core.hookenv.log("Writing {}".format(self.mysqlrouter_conf))
|
||||
with open(self.mysqlrouter_conf, 'w') as configfile:
|
||||
config.write(configfile)
|
||||
with ch_core.host.restart_on_change(
|
||||
self.restart_map,
|
||||
restart_functions=self.restart_functions):
|
||||
|
||||
# Bug 1927981 - For mysql-innodb-clusters which were deployed with a
|
||||
# cluster name which was not 'jujuCluster', an extra section to the
|
||||
# mysqrouter.conf file was written which causes the mysql router
|
||||
# service to fail to start. Remove the extraneous section at charm
|
||||
# upgrade time.
|
||||
sections = list(filter(lambda x: x.startswith('metadata_cache'),
|
||||
config.sections()))
|
||||
if len(sections) > 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()
|
||||
|
||||
super(MySQLRouterCharm, self).upgrade_charm()
|
||||
# On upgrade set the unknown_config_option to warning
|
||||
# (LP: #1971565)
|
||||
if 'unknown_config_option' not in config[DEFAULT_SECTION]:
|
||||
msg = (f'[{DEFAULT_SECTION}].unknown_config_option '
|
||||
f'is not present in the configuration file, '
|
||||
f'so setting it to "warning"')
|
||||
ch_core.hookenv.log(msg)
|
||||
config[DEFAULT_SECTION]['unknown_config_option'] = 'warning'
|
||||
|
||||
# Bug 1927981 - For mysql-innodb-clusters which were deployed with
|
||||
# a cluster name which was not 'jujuCluster', an extra section to
|
||||
# the mysqlrouter.conf file was written which causes the mysql
|
||||
# router service to fail to start. Remove the extraneous section
|
||||
# at charm upgrade time.
|
||||
sections = list(filter(lambda x: x.startswith('metadata_cache'),
|
||||
config.sections()))
|
||||
if len(sections) > 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')
|
||||
|
||||
parameters = self._get_config_parameters()
|
||||
self.update_config_parameters(parameters, config=config)
|
||||
|
||||
def get_db_helper(self):
|
||||
"""Get an instance of the MySQLDB8Helper class.
|
||||
|
|
@ -679,7 +683,7 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
|
|||
db_port=self.mysqlrouter_port,
|
||||
ssl_ca=_ssl_ca)
|
||||
|
||||
def update_config_parameters(self, parameters):
|
||||
def update_config_parameters(self, parameters, config=None):
|
||||
"""Update configuration parameters using ConfigParser.
|
||||
|
||||
Update this instances mysqlrouter.conf with a dictionary set of
|
||||
|
|
@ -700,12 +704,24 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
|
|||
|
||||
:param parameters: Dictionary of parameters
|
||||
:type parameters: dict
|
||||
:param config: an optional existing ConfigParser object
|
||||
:type config: configparser.ConfigParser
|
||||
:side effect: Writes the mysqlrouter.conf file
|
||||
:returns: This function is called for its side effect
|
||||
:rtype: None
|
||||
"""
|
||||
config = configparser.ConfigParser()
|
||||
config.read(self.mysqlrouter_conf)
|
||||
# No reason to write the file if it does not exist due to
|
||||
# mysql-router not having been bootstrapped yet
|
||||
if not os.path.exists(self.mysqlrouter_conf):
|
||||
ch_core.hookenv.log(
|
||||
"mysqlrouter.conf does not yet exist. "
|
||||
"Skipping config-changed.", "DEBUG")
|
||||
return
|
||||
|
||||
ch_core.hookenv.log("Updating configuration parameters", "DEBUG")
|
||||
if not config:
|
||||
config = configparser.ConfigParser()
|
||||
config.read(self.mysqlrouter_conf)
|
||||
|
||||
for heading, settings in parameters.items():
|
||||
for section in config.sections():
|
||||
|
|
@ -723,8 +739,7 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
|
|||
except KeyError:
|
||||
config[translated] = {param: value}
|
||||
|
||||
ch_core.hookenv.log("Writing {}".format(
|
||||
self.mysqlrouter_conf))
|
||||
ch_core.hookenv.log("Writing {}".format(self.mysqlrouter_conf))
|
||||
with open(self.mysqlrouter_conf, 'w') as configfile:
|
||||
config.write(configfile)
|
||||
|
||||
|
|
@ -735,17 +750,27 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
|
|||
config via ConfigParser. We only update after the mysql-router service
|
||||
has bootstrapped.
|
||||
|
||||
:side effect: Calls update_config_parameter and restarts mysql-router
|
||||
:side effect: Calls update_config_parameters and restarts mysql-router
|
||||
if the config file has changed.
|
||||
:returns: This function is called for its side effect
|
||||
:rtype: None
|
||||
"""
|
||||
if not os.path.exists(self.mysqlrouter_conf):
|
||||
# LP: #1980693 - config_changed may be invoked during upgrade-charm
|
||||
# hook before the PID has been changed.
|
||||
if 'upgrade-charm' in ch_core.hookenv.hook_name().lower():
|
||||
ch_core.hookenv.log(
|
||||
"mysqlrouter.conf does not yet exist. "
|
||||
"Skipping config changed.", "DEBUG")
|
||||
"Skipping config-changed function as it is being invoked "
|
||||
"within the upgrade-charm hook.", "DEBUG")
|
||||
return
|
||||
|
||||
parameters = self._get_config_parameters()
|
||||
with ch_core.host.restart_on_change(
|
||||
self.restart_map,
|
||||
restart_functions=self.restart_functions):
|
||||
self.update_config_parameters(parameters)
|
||||
|
||||
def _get_config_parameters(self):
|
||||
|
||||
_parameters = {
|
||||
METADATA_CACHE_SECTION: {
|
||||
"ttl": str(self.options.ttl),
|
||||
|
|
@ -788,10 +813,7 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
|
|||
self.options.max_connections
|
||||
)
|
||||
|
||||
with ch_core.host.restart_on_change(
|
||||
self.restart_map, restart_functions=self.restart_functions):
|
||||
ch_core.hookenv.log("Updating configuration parameters", "DEBUG")
|
||||
self.update_config_parameters(_parameters)
|
||||
return _parameters
|
||||
|
||||
@tenacity.retry(
|
||||
wait=tenacity.wait_fixed(10),
|
||||
|
|
|
|||
|
|
@ -746,6 +746,16 @@ class TestMySQLRouterCharm(test_utils.PatchHelper):
|
|||
self.assertEqual(fake_config['routing:foo_rw'],
|
||||
{"test": True})
|
||||
|
||||
def test_update_config_parameters_not_bootstrapped(self):
|
||||
self.patch_object(mysql_router.os.path, "exists",
|
||||
return_value=False)
|
||||
mock_config = mock.MagicMock()
|
||||
self.patch_object(mysql_router.configparser, "ConfigParser",
|
||||
return_value=mock_config)
|
||||
mrc = mysql_router.MySQLRouterCharm()
|
||||
mrc.update_config_parameters({})
|
||||
mock_config.read.assert_not_called()
|
||||
|
||||
def test_config_changed(self):
|
||||
_config_data = {
|
||||
"ttl": '5',
|
||||
|
|
@ -807,12 +817,6 @@ class TestMySQLRouterCharm(test_utils.PatchHelper):
|
|||
_params["DEFAULT"]["max_total_connections"] = \
|
||||
_config_data['max_connections']
|
||||
|
||||
# Not bootstrapped yet
|
||||
self.exists.return_value = False
|
||||
_mock_update_config_parameters.reset_mock()
|
||||
mrc.config_changed()
|
||||
_mock_update_config_parameters.assert_not_called()
|
||||
|
||||
# mysql-router pkg >= 8.0.23, no client_ssl_cert
|
||||
self.cmp_pkgrevno.return_value = 1
|
||||
self.db_router.ssl_ca.return_value = None
|
||||
|
|
@ -881,17 +885,25 @@ class TestMySQLRouterCharm(test_utils.PatchHelper):
|
|||
},
|
||||
}
|
||||
fake_config = FakeConfigParser(current_config)
|
||||
fake_params = {}
|
||||
|
||||
self.patch_object(mysql_router.charms_openstack.charm.OpenStackCharm,
|
||||
'upgrade_charm')
|
||||
self.patch_object(
|
||||
mysql_router.MySQLRouterCharm, '_get_config_parameters',
|
||||
return_value=fake_params)
|
||||
mock_update_config_params = mock.MagicMock()
|
||||
self.patch_object(mysql_router.configparser, "ConfigParser",
|
||||
return_value=fake_config)
|
||||
|
||||
mrc = mysql_router.MySQLRouterCharm()
|
||||
mrc.update_config_parameters = mock_update_config_params
|
||||
# should not throw a key error.
|
||||
mrc.upgrade_charm()
|
||||
self.assertIn('metadata_cache:foo', fake_config)
|
||||
self.assertNotIn('metadata_cache:.jujuCluster', fake_config)
|
||||
mock_update_config_params.assert_called_once_with(
|
||||
fake_params, config=fake_config)
|
||||
|
||||
def test_upgrade_charm_lp1971565(self):
|
||||
# test fix for Bug LP#1971565
|
||||
|
|
@ -907,15 +919,23 @@ class TestMySQLRouterCharm(test_utils.PatchHelper):
|
|||
},
|
||||
}
|
||||
fake_config = FakeConfigParser(current_config)
|
||||
fake_params = {}
|
||||
|
||||
self.patch_object(mysql_router.charms_openstack.charm.OpenStackCharm,
|
||||
'upgrade_charm')
|
||||
self.patch_object(
|
||||
mysql_router.MySQLRouterCharm, '_get_config_parameters',
|
||||
return_value=fake_params)
|
||||
mock_update_config_params = mock.MagicMock()
|
||||
self.patch_object(mysql_router.configparser, "ConfigParser",
|
||||
return_value=fake_config)
|
||||
|
||||
mrc = mysql_router.MySQLRouterCharm()
|
||||
mrc.update_config_parameters = mock_update_config_params
|
||||
mrc.upgrade_charm()
|
||||
self.assertIn('metadata_cache:foo', fake_config)
|
||||
self.assertIn('unknown_config_option', fake_config['DEFAULT'])
|
||||
self.assertEqual(fake_config['DEFAULT']['unknown_config_option'],
|
||||
'warning')
|
||||
mock_update_config_params.assert_called_once_with(
|
||||
fake_params, config=fake_config)
|
||||
|
|
|
|||
Loading…
Reference in New Issue