Implement validated restart on change

Add new metadata cache parameters.
Use the charm-helpers' pausable_restart_on_change for config-changed
Validate connectivity on restart to resolve LP Bug #1915842

See "Metadata Cache Options"
https://dev.mysql.com/doc/mysql-router/8.0/en/mysql-router-conf-options.html

Closes-Bug: #1915842
Change-Id: Icd30f9ade2c7aadbadaa57300c22786cde63957f

src/config.yaml

@@ -20,3 +20,27 @@ options:
     description: |
         Base port number for RW interface. RO, xRW and xRO will
         increment from base_port.
+  ttl:
+    type: float
+    default: .5
+    description: |
+        Time to live (in seconds) of information in the metadata cache.
+        Accepts either an integer or a floating point value. The granularity is
+        limited to milliseconds, where 0.001 equates to one millisecond.
+        Precision is truncated to the supported range; for example ttl=0.0119
+        is treated as 11 milliseconds. The value 0 means that the metadata
+        cache module queries the metadata continuously in a tight loop.
+  auth_cache_refresh_interval:
+    type: int
+    default: 2
+    description: |
+        Time (in seconds) between the auth-cache refresh attempts. Defaults to
+        2. The value must be smaller than auth_cache_ttl and ttl else Router
+        won't start.
+  auth_cache_ttl:
+    type: int
+    default: -1
+    description: |
+        Time (in seconds) until the cache becomes invalid if not refreshed.
+        Defaults to -1 (infinite). The value must be larger than
+        auth_cache_refresh_interval else Router won't start.

src/lib/charm/openstack/mysql_router.py

@@ -16,6 +16,7 @@ import configparser
 import json
 import os
 import subprocess
+import tenacity
 
 import charms_openstack.charm
 import charms_openstack.adapters
@@ -28,6 +29,7 @@ import charmhelpers.contrib.network.ip as ch_net_ip
 import charmhelpers.contrib.database.mysql as mysql
 
 import charmhelpers.contrib.openstack.templating as os_templating
+import charmhelpers.contrib.openstack.utils as os_utils
 
 
 # Flag Strings
@@ -74,6 +76,10 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
     # For internal use with mysql.get_db_data
     _unprefixed = "MRUP"
 
+    # mysql.MySQLdb._exceptions.OperationalError error 2013
+    # LP Bug #1915842
+    _waiting_for_initial_communication_packet_error = 2013
+
     @property
     def mysqlrouter_bin(self):
         """Determine the path to the mysqlrouter binary.
@@ -218,6 +224,24 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
     def mysqlrouter_group(self):
         return "mysql"
 
+    @property
+    def ssl_ca(self):
+        """Return the SSL Certificate Authority
+
+        :param self: Self
+        :type self: MySQLInnoDBClusterCharm instance
+        :returns: Cluster username
+        :rtype: str
+        :rtype: Union[str, None]
+        """
+        if self.db_router_endpoint:
+            if self.db_router_endpoint.ssl_ca():
+                return json.loads(self.db_router_endpoint.ssl_ca())
+
+    @property
+    def restart_functions(self):
+        return {self.name: self.custom_restart_function}
+
     def install(self):
         """Custom install function.
 
@@ -313,12 +337,16 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
 
         return states_to_check
 
-    def check_mysql_connection(self):
+    def check_mysql_connection(self, reraise_on=None):
         """Check if an instance of MySQL is accessible.
 
         Attempt a connection to the given instance of mysql to determine if it
         is running and accessible.
 
+        :param reraise_on: List of integer error codes to reraise exceptions.
+                           For use with tenacity retries on specific
+                           exceptions.
+        :type reraise_on: List[int]
         :side effect: Uses get_db_helper to execute a connection to the DB.
         :returns: True if connection succeeds or False if not
         :rtype: boolean
@@ -331,9 +359,13 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
                              port=self.mysqlrouter_port,
                              connect_timeout=self.mysql_connect_timeout)
             return True
-        except mysql.MySQLdb._exceptions.OperationalError:
+        except mysql.MySQLdb._exceptions.OperationalError as e:
             ch_core.hookenv.log("Could not connect to db", "DEBUG")
-            return False
+            if not reraise_on:
+                return False
+            else:
+                if e.args[0] in reraise_on:
+                    raise e
 
     def custom_assess_status_check(self):
         """Custom assess status check.
@@ -524,19 +556,11 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
             _ssl_ca = receiving_interface.ssl_ca()
             if _ssl_ca:
                 _ssl_ca = json.loads(_ssl_ca)
-                # We are using CA signed certificates with validation for TLS
-                # Set client_ssl_mode = PASSTHROUGH
-                self.update_config_parameter(
-                    "DEFAULT", "client_ssl_mode", "PASSTHROUGH")
             else:
                 # Reset ssl_ca in case we previously had it set
                 ch_core.hookenv.log("Proactively resetting ssl_ca", "DEBUG")
                 sending_interface.relations[
                     unit.relation.relation_id].to_publish_raw["ssl_ca"] = None
-                # We are using self-signed certificates for TLS
-                # Set client_ssl_mode = PREFERRED
-                self.update_config_parameter(
-                    "DEFAULT", "client_ssl_mode", "PREFERRED")
 
             if ch_core.hookenv.local_unit() in (json.loads(
                     receiving_interface.allowed_units(prefix=prefix))):
@@ -556,29 +580,100 @@ class MySQLRouterCharm(charms_openstack.charm.OpenStackCharm):
                 db_port=self.mysqlrouter_port,
                 ssl_ca=_ssl_ca)
 
-    def update_config_parameter(self, heading, param, value):
-        """Update configuration parameter.
-
-        Update this instances mysqlrouter.conf with a key=value.
-        Such that under heading we get param = value.
-        Restart on changed based on the mysqlrouter.conf.
-
-        :param heading: The config file heading i.e. 'DEFAULT'
-        :type heading: str
-        :param param: The parameter key
-        :type param: str
-        :param value: The value getting set
-        :type value: str
-        :side effect: Writes the mysqlrouter.conf file and restarts services if
-                      there is a change.
+    def update_config_parameters(self, parameters):
+        """Update configuration parameters using ConfigParser.
+
+        Update this instances mysqlrouter.conf with a dictionary set of
+        configuration parameters and values of the form:
+
+        parameters = {
+            "HEADING": {
+                "param": "value"
+            }
+        }
+
+        :param parameters: Dictionary of parameters
+        :type parameters: dict
+        :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)
-        config[heading][param] = value
-        ch_core.hookenv.log("Updating {} with {} {} = {}".format(
-            self.mysqlrouter_conf, heading, param, value))
-        with self.restart_on_change():
-            with open(self.mysqlrouter_conf, 'w') as configfile:
-                config.write(configfile)
+        for heading in parameters.keys():
+            for param in parameters[heading].keys():
+                config[heading][param] = parameters[heading][param]
+        ch_core.hookenv.log("Writing {}".format(
+            self.mysqlrouter_conf))
+        with open(self.mysqlrouter_conf, 'w') as configfile:
+            config.write(configfile)
+
+    def config_changed(self):
+        """Config changed.
+
+        Custom config changed as we are not using templates we need to update
+        config via ConfigParser. We only update after the mysql-router service
+        has bootstrapped.
+
+        :side effect: Calls update_config_parameter 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):
+            ch_core.hookenv.log(
+                "mysqlrouter.conf does not yet exist. "
+                "Skipping config changed.", "DEBUG")
+            return
+
+        _parameters = {
+            "metadata_cache:jujuCluster": {
+                "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),
+            }
+        }
+
+        if self.ssl_ca:
+            ch_core.hookenv.log("TLS mode PASSTHROUGH", "DEBUG")
+            _parameters["DEFAULT"] = {"client_ssl_mode": "PASSTHROUGH"}
+        else:
+            ch_core.hookenv.log("TLS mode PREFERRED", "DEBUG")
+            _parameters["DEFAULT"] = {"client_ssl_mode": "PREFERRED"}
+
+        # NOTE: LP Bug #1917792
+        # Switch to context manager when work there is completed
+        @os_utils.pausable_restart_on_change(
+            self.restart_map, restart_functions=self.restart_functions)
+        def _config_changed(self, parameters):
+            ch_core.hookenv.log("Updating configuration parameters", "DEBUG")
+            self.update_config_parameters(parameters)
+
+        _config_changed(self, _parameters)
+
+    @tenacity.retry(wait=tenacity.wait_fixed(10),
+                    retry=tenacity.retry_if_exception_type(
+                        mysql.MySQLdb._exceptions.OperationalError),
+                    reraise=True,
+                    stop=tenacity.stop_after_attempt(5))
+    def custom_restart_function(self, service_name):
+        """Tenacity retry custom restart function for restart_on_change
+
+        Custom restart function for use in restart_on_change contexts. Tenacity
+        retry enabled based on verification of connectivity.
+
+        :side effect: Calls service_stop and service_start on the mysql-router
+                      service(s).
+        :returns: This function is called for its side effect
+        :rtype: None
+        """
+        ch_core.hookenv.log(
+            "Custom restart of {}".format(service_name), "DEBUG")
+        self.service_stop(service_name)
+        self.service_start(service_name)
+        # Only raise an exception if it matches
+        # mysql.MySQLdb._exceptions.OperationalError error 2013
+        # LP Bug #1915842
+        self.check_mysql_connection(
+            reraise_on=[self._waiting_for_initial_communication_packet_error])

src/reactive/mysql_router_handlers.py

@@ -99,5 +99,6 @@ def proxy_shared_db_responses(shared_db, db_router):
     :type db_router_interface: MySQLRouterRequires object
     """
     with charm.provide_charm_instance() as instance:
+        instance.config_changed()
         instance.proxy_db_and_user_responses(db_router, shared_db)
         instance.assess_status()

src/wheelhouse.txt

@@ -1,5 +1,6 @@
 psutil
 mysqlclient
+tenacity
 
 git+https://github.com/juju/charm-helpers.git#egg=charmhelpers
 

unit_tests/test_lib_charm_openstack_mysql_router.py

@@ -600,15 +600,75 @@ class TestMySQLRouterCharm(test_utils.PatchHelper):
             self.db_router, self.keystone_shared_db)
         self.keystone_shared_db.set_db_connection_info.assert_not_called()
 
-    def test_update_config_parameter(self):
+    def test_update_config_parameters(self):
         self.patch_object(mysql_router.configparser, "ConfigParser")
 
         self.service_name = "mysql-router"
         _mock_config_parser = mock.MagicMock()
         self.ConfigParser.return_value = _mock_config_parser
 
+        _params = {"DEFAULT": {"client_ssl_mode": "PREFERRED"}}
+
         mrc = mysql_router.MySQLRouterCharm()
-        mrc.update_config_parameter("DEFAULT", "client_ssl_mode", "PREFERRED")
+        mrc.update_config_parameters(_params)
         _mock_config_parser.read.assert_called_once()
+        _mock_config_parser.__getitem__.assert_called_once_with('DEFAULT')
+        _mock_config_parser.__getitem__().__setitem__.assert_called_once_with(
+            'client_ssl_mode', 'PREFERRED')
         _mock_config_parser.write.assert_called_once_with(
             self.mock_open()().__enter__())
+
+    def test_config_changed(self):
+        _config_data = {
+            "ttl": '5',
+            "auth_cache_ttl": '10',
+            "auth_cache_refresh_interval": '7',
+        }
+        _mock_decorator = mock.MagicMock()
+        _mock_decorated = mock.MagicMock()
+        _mock_decorator.return_value = _mock_decorated
+
+        def _fake_config(key=None):
+            return _config_data[key] if key else _config_data
+
+        self.patch_object(mysql_router.ch_core.hookenv, "config")
+        self.patch_object(mysql_router.os.path, "exists")
+        self.patch_object(mysql_router.os_utils, "pausable_restart_on_change")
+        self.config.side_effect = _fake_config
+        self.pausable_restart_on_change.return_value = _mock_decorator
+        self.service_name = "mysql-router"
+        self.endpoint_from_flag.return_value = self.db_router
+        self.db_router.ssl_ca.return_value = '"CACERT"'
+
+        _mock_update_config_parameters = mock.MagicMock()
+        mrc = mysql_router.MySQLRouterCharm()
+        mrc.update_config_parameters = _mock_update_config_parameters
+
+        _params = {
+            'metadata_cache:jujuCluster': _config_data,
+            'DEFAULT': {'client_ssl_mode': "PASSTHROUGH"},
+        }
+
+        # Not bootstrapped yet
+        self.exists.return_value = False
+        mrc.config_changed()
+        _mock_update_config_parameters.assert_not_called()
+
+        # Bootstrapped
+        self.exists.return_value = True
+        mrc.config_changed()
+        _mock_decorated.assert_has_calls([mock.call(mrc, _params)])
+
+    def test_custom_restart_function(self):
+        self.patch_object(mysql_router.ch_core.host, "service_stop")
+        self.patch_object(mysql_router.ch_core.host, "service_start")
+        self.service_name = "mysql-router"
+        _mock_check_mysql_connection = mock.MagicMock()
+
+        mrc = mysql_router.MySQLRouterCharm()
+        mrc.check_mysql_connection = _mock_check_mysql_connection
+
+        mrc.custom_restart_function(self.service_name)
+        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()