Allow connection query string to be passed separately.

The Nova project has made the decision to store the entire
contents of the "sql_connection" field in the database, with
alterations to the host value, such that various "cells" database
URLs can be located using Nova's database itself as a central
registry of database URLs.

This architecture has produced several problems.   The first
is that it is impossible to apply parameters to the URL that
are local to the client machine; the leading example of this
is the MySQL "bind_host" variable, which must match the hostname
of the connecting host.  Because cells puts these URLs into the
database and shares them with all controllers, we have to use a workaround
with the read_default_file parameter to specify a controller-local
file of options; this is not a standard thing
for other database drivers, and these parameters only apply to
the MySQL driver and not the SQLAlchemy engine.

The next issue is that it is inconvenient to add parameters
to the URL at all, once Nova has already been running, as one
must manually use Nova's command line tools
to alter all the URLs that have already been copied into the database
and alter the query parameters with each of those individually,
and then restart *all* services who will all receive the parameter
(no way to add params to just one controller).

Nova's "store the URL in the database" feature only needs to be able
to locate the host / database name of the alternate database,
and not change the URL tuning parameters.   This patch adds a new
oslo.db parameter connection_parameters which allows the params to
be applied separately from the sql_connection parameter, so that
Nova can continue persisting sql_connection but the parameters remain
local to the nova.conf file.   A URL parameter that truly had to remain
persisted in Nova's database (there aren't any) could still be applied
at the sql_connection level.

This feature is essential not just so that we can again place
simple parameters into controller-local files like "bind_host", but
also to allow for configuration of SQLAlchemy features such as
plugins that do connection pool monitoring.

Change-Id: Id4de4b09ec4719cbf8b372629fcf58cf368a33d4

oslo_db/options.py

@@ -146,6 +146,11 @@ database_opts = [
                     'error before error is '
                     'raised. Set to -1 to specify an infinite retry '
                     'count.'),
+    cfg.StrOpt('connection_parameters',
+               default='',
+               help='Optional URL parameters to append onto the connection '
+               'URL at connect time; specify as '
+               'param1=value1&param2=value2&...'),
 ]
 
 

oslo_db/sqlalchemy/enginefacade.py

@@ -149,7 +149,8 @@ class _TransactionFactory(object):
             'thread_checkin': _Default(True),
             'json_serializer': _Default(None),
             'json_deserializer': _Default(None),
-            'logging_name': _Default(None)
+            'logging_name': _Default(None),
+            'connection_parameters': _Default(None)
         }
         self._maker_cfg = {
             'expire_on_commit': _Default(False),
@@ -219,6 +220,9 @@ class _TransactionFactory(object):
         :param connection_debug: engine logging level, defaults to 0. set to
          50 for INFO, 100 for DEBUG.
 
+        :param connection_parameters: additional parameters to append onto the
+         database URL query string, pass as "param1=value1&param2=value2&..."
+
         :param max_pool_size: max size of connection pool, uses CONF for
          default
 

oslo_db/sqlalchemy/engines.py

@@ -104,6 +104,21 @@ def _setup_logging(connection_debug=0):
             logger.setLevel(logging.WARNING)
 
 
+def _extend_url_parameters(url, connection_parameters):
+    for key, value in six.moves.urllib.parse.parse_qs(
+            connection_parameters).items():
+        if key in url.query:
+            existing = url.query[key]
+            if not isinstance(existing, list):
+                url.query[key] = existing = utils.to_list(existing)
+            existing.extend(value)
+            value = existing
+        else:
+            url.query[key] = value
+        if len(value) == 1:
+            url.query[key] = value[0]
+
+
 def _vet_url(url):
     if "+" not in url.drivername and not url.drivername.startswith("sqlite"):
         if url.drivername.startswith("mysql"):
@@ -132,11 +147,14 @@ def create_engine(sql_connection, sqlite_fk=False, mysql_sql_mode=None,
                   connection_trace=False, max_retries=10, retry_interval=10,
                   thread_checkin=True, logging_name=None,
                   json_serializer=None,
-                  json_deserializer=None):
+                  json_deserializer=None, connection_parameters=None):
     """Return a new SQLAlchemy engine."""
 
     url = sqlalchemy.engine.url.make_url(sql_connection)
 
+    if connection_parameters:
+        _extend_url_parameters(url, connection_parameters)
+
     _vet_url(url)
 
     engine_args = {

oslo_db/tests/sqlalchemy/test_sqlalchemy.py

@@ -213,6 +213,79 @@ class FakeDB2Engine(object):
         pass
 
 
+class QueryParamTest(test_base.DbTestCase):
+    def _fixture(self):
+        from sqlalchemy import create_engine
+
+        def _mock_create_engine(*arg, **kw):
+            return create_engine("sqlite://")
+
+        return mock.patch(
+            "oslo_db.sqlalchemy.engines.sqlalchemy.create_engine",
+            side_effect=_mock_create_engine)
+
+    def test_add_assorted_params(self):
+        with self._fixture() as ce:
+            engines.create_engine(
+                "mysql+pymysql://foo:bar@bat",
+                connection_parameters="foo=bar&bat=hoho&bat=param2")
+
+        self.assertEqual(
+            ce.mock_calls[0][1][0].query,
+            {'bat': ['hoho', 'param2'], 'foo': 'bar'}
+        )
+
+    def test_add_no_params(self):
+        with self._fixture() as ce:
+            engines.create_engine(
+                "mysql+pymysql://foo:bar@bat")
+
+        self.assertEqual(
+            ce.mock_calls[0][1][0].query,
+            {}
+        )
+
+    def test_combine_params(self):
+        with self._fixture() as ce:
+            engines.create_engine(
+                "mysql+pymysql://foo:bar@bat/"
+                "?charset=utf8&param_file=tripleo.cnf",
+                connection_parameters="plugin=sqlalchemy_collectd&"
+                                      "collectd_host=127.0.0.1&"
+                                      "bind_host=192.168.1.5")
+
+        self.assertEqual(
+            ce.mock_calls[0][1][0].query,
+            {
+                'bind_host': '192.168.1.5',
+                'charset': 'utf8',
+                'collectd_host': '127.0.0.1',
+                'param_file': 'tripleo.cnf',
+                'plugin': 'sqlalchemy_collectd'
+            }
+        )
+
+    def test_combine_multi_params(self):
+        with self._fixture() as ce:
+            engines.create_engine(
+                "mysql+pymysql://foo:bar@bat/"
+                "?charset=utf8&param_file=tripleo.cnf&plugin=connmon",
+                connection_parameters="plugin=sqlalchemy_collectd&"
+                                      "collectd_host=127.0.0.1&"
+                                      "bind_host=192.168.1.5")
+
+        self.assertEqual(
+            ce.mock_calls[0][1][0].query,
+            {
+                'bind_host': '192.168.1.5',
+                'charset': 'utf8',
+                'collectd_host': '127.0.0.1',
+                'param_file': 'tripleo.cnf',
+                'plugin': ['connmon', 'sqlalchemy_collectd']
+            }
+        )
+
+
 class MySQLDefaultModeTestCase(test_base.MySQLOpportunisticTestCase):
     def test_default_is_traditional(self):
         with self.engine.connect() as conn:
@@ -357,6 +430,7 @@ class EngineFacadeTestCase(oslo_test.BaseTestCase):
             thread_checkin=mock.ANY,
             json_serializer=None,
             json_deserializer=None,
+            connection_parameters='',
             logging_name=mock.ANY,
         )
         get_maker.assert_called_once_with(engine=create_engine(),

releasenotes/notes/add_connection_parameters-231aa7d8b7d2d416.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Added new option connection_parameters which allows SQLAlchemy query
+    parameters to be stated separately from the URL itself, to allow
+    URL-persistence schemes like Nova cells to use controller-local
+    query parameters that aren't broadcast to all other servers.