Deprecate MySQL NDB Cluster Support

Traditionally, the MySQL support in oslo.db has assumed use of the
InnoDB storage engine. However, this isn't the only storage engine
available and a few years ago an effort was made to add support for
another storage engine, MySQL Cluster (NDB). The oslo.db aspects of this
effort were tracked via bug 1564110 [1] and from reading this bug and
looking at other patches related to this effort [2], it becomes obvious
that this was never seen through to the completion and the
OpenStack-wide effort never took off [3]. As a result, much of what is
here is in-effect dead code now.

Given no one is using this engine, there's no reason to keep it around.
Deprecate it with an eye on removing it sooner rather than later.

[1] https://bugs.launchpad.net/oslo.db/+bug/1564110
[2] https://review.opendev.org/q/owner:octave.orgeron%2540oracle.com
[3] https://review.opendev.org/c/openstack/openstack-specs/+/429940

Change-Id: Id5ddf1d6f47b8a572001f58ad8b9b8a7dbe4e8ac
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

9 files changed, 143 insertions, 29 deletions

diff --git a/oslo_db/options.py b/oslo_db/options.py
index df07c33..d827970 100644
--- a/oslo_db/options.py
+++ b/oslo_db/options.py
@@ -62,6 +62,12 @@ database_opts = [
     cfg.BoolOpt(
         'mysql_enable_ndb',
         default=False,
+        deprecated_for_removal=True,
+        deprecated_since='12.1.0',
+        deprecated_reason=(
+            'Support for the MySQL NDB Cluster storage engine has been '
+            'deprecated and will be removed in a future release.'
+        ),
         help=(
             'If True, transparently enables support for handling '
             'MySQL Cluster (NDB).'
diff --git a/oslo_db/sqlalchemy/enginefacade.py b/oslo_db/sqlalchemy/enginefacade.py
index f69811b..81341b7 100644
--- a/oslo_db/sqlalchemy/enginefacade.py
+++ b/oslo_db/sqlalchemy/enginefacade.py
@@ -316,6 +316,14 @@ class _TransactionFactory(object):
         self._configure(False, kw)
 
     def _configure(self, as_defaults, kw):
+        if 'mysql_enable_ndb' in kw:
+            debtcollector.deprecate(
+                (
+                    'Support for the MySQL NDB Cluster storage engine has '
+                    'been deprecated and will be removed in a future release.'
+                ),
+                version='12.1.0',
+            )
 
         if self._started:
             raise AlreadyStartedError(
@@ -1239,6 +1247,7 @@ class LegacyEngineFacade(object):
     :keyword mysql_enable_ndb: If True, transparently enables support for
                                handling MySQL Cluster (NDB).
                                (defaults to False)
+                               **DEPRECATED**
     :keyword connection_recycle_time: Time period for connections to be
                             recycled upon checkout (defaults to 3600)
     :keyword connection_debug: verbosity of SQL debugging information.
diff --git a/oslo_db/sqlalchemy/engines.py b/oslo_db/sqlalchemy/engines.py
index 32c4def..529db19 100644
--- a/oslo_db/sqlalchemy/engines.py
+++ b/oslo_db/sqlalchemy/engines.py
@@ -23,6 +23,7 @@ import re
 import time
 from urllib import parse
 
+import debtcollector.removals
 import debtcollector.renames
 import sqlalchemy
 from sqlalchemy import event
@@ -147,8 +148,19 @@ def _vet_url(url):
             )
 
 
+@debtcollector.removals.removed_kwarg(
+    'mysql_enable_ndb',
+    message=(
+        'Support for the MySQL NDB Cluster storage engine has been deprecated '
+        'and will be removed in a future release.'
+    ),
+    version='12.1.0',
+)
 @debtcollector.renames.renamed_kwarg(
-    "idle_timeout", "connection_recycle_time", replace=True)
+    'idle_timeout',
+    'connection_recycle_time',
+    replace=True,
+)
 def create_engine(sql_connection, sqlite_fk=False, mysql_sql_mode=None,
                   mysql_enable_ndb=False,
                   connection_recycle_time=3600,
diff --git a/oslo_db/sqlalchemy/ndb.py b/oslo_db/sqlalchemy/ndb.py
index 270b899..42b3189 100644
--- a/oslo_db/sqlalchemy/ndb.py
+++ b/oslo_db/sqlalchemy/ndb.py
@@ -15,18 +15,26 @@
 
 import re
 
-from oslo_db.sqlalchemy.types import String
-
+import debtcollector.removals
 from sqlalchemy import event, schema
 from sqlalchemy.ext.compiler import compiles
 from sqlalchemy.types import String as _String
 from sqlalchemy.types import to_instance
 
+from oslo_db.sqlalchemy.types import String
+
 
 engine_regex = re.compile("engine=innodb", re.IGNORECASE)
 trans_regex = re.compile("savepoint|rollback|release savepoint", re.IGNORECASE)
 
 
+@debtcollector.removals.remove(
+    message=(
+        'Support for the MySQL NDB Cluster storage engine has been deprecated '
+        'and will be removed in a future release.'
+    ),
+    version='12.1.0',
+)
 def enable_ndb_support(engine):
     """Enable NDB Support.
 
@@ -36,16 +44,45 @@ def enable_ndb_support(engine):
     engine.dialect._oslodb_enable_ndb_support = True
 
 
+def _ndb_status(engine_or_compiler):
+    """Test if NDB Support is enabled.
+
+    Function to test if NDB support is enabled or not.
+
+    .. note::
+
+        This is for internal use only while we deprecate and remove ndb
+        support. **Do not use this outside of oslo.db!**
+    """
+    return getattr(
+        engine_or_compiler.dialect,
+        '_oslodb_enable_ndb_support',
+        False,
+    )
+
+
+@debtcollector.removals.remove(
+    message=(
+        'Support for the MySQL NDB Cluster storage engine has been deprecated '
+        'and will be removed in a future release.'
+    ),
+    version='12.1.0',
+)
 def ndb_status(engine_or_compiler):
     """Test if NDB Support is enabled.
 
     Function to test if NDB support is enabled or not.
     """
-    return getattr(engine_or_compiler.dialect,
-                   '_oslodb_enable_ndb_support',
-                   False)
+    return _ndb_status(engine_or_compiler)
 
 
+@debtcollector.removals.remove(
+    message=(
+        'Support for the MySQL NDB Cluster storage engine has been deprecated '
+        'and will be removed in a future release.'
+    ),
+    version='12.1.0',
+)
 def init_ndb_events(engine):
     """Initialize NDB Events.
 
@@ -60,7 +97,7 @@ def init_ndb_events(engine):
         convert InnoDB to NDBCLUSTER, drop SAVEPOINT requests, drop
         ROLLBACK requests, and drop RELEASE SAVEPOINT requests.
         """
-        if ndb_status(engine):
+        if _ndb_status(engine):
             statement = engine_regex.sub("ENGINE=NDBCLUSTER", statement)
             if re.match(trans_regex, statement):
                 statement = "SET @oslo_db_ndb_savepoint_rollback_disabled = 0;"
@@ -68,6 +105,8 @@ def init_ndb_events(engine):
         return statement, parameters
 
 
+# TODO(stephenfin): This is effectively deprecated and should be removed when
+# we remove the rest of this module since it'll be a no-op then.
 @compiles(schema.CreateTable, "mysql")
 def prefix_inserts(create_table, compiler, **kw):
     """Replace InnoDB with NDBCLUSTER automatically.
@@ -76,12 +115,14 @@ def prefix_inserts(create_table, compiler, **kw):
     convert InnoDB to NDBCLUSTER. Targets compiler events.
     """
     existing = compiler.visit_create_table(create_table, **kw)
-    if ndb_status(compiler):
+    if _ndb_status(compiler):
         existing = engine_regex.sub("ENGINE=NDBCLUSTER", existing)
 
     return existing
 
 
+# TODO(stephenfin): This is effectively deprecated and should be removed when
+# we remove the rest of this module since it'll be a no-op then.
 @compiles(String, "mysql")
 def _compile_ndb_string(element, compiler, **kw):
     """Process ndb specific overrides for String.
@@ -95,7 +136,7 @@ def _compile_ndb_string(element, compiler, **kw):
     mysql_ndb_type will change the column type to the requested
     data type.
     """
-    if not ndb_status(compiler):
+    if not _ndb_status(compiler):
         return compiler.visit_string(element, **kw)
 
     if element.mysql_ndb_length:
diff --git a/oslo_db/sqlalchemy/types.py b/oslo_db/sqlalchemy/types.py
index a86f5ac..b30afce 100644
--- a/oslo_db/sqlalchemy/types.py
+++ b/oslo_db/sqlalchemy/types.py
@@ -12,6 +12,7 @@
 
 import json
 
+import debtcollector.removals
 from sqlalchemy.dialects import mysql
 from sqlalchemy.types import Integer, Text, TypeDecorator, String as _String
 
@@ -113,6 +114,18 @@ class SoftDeleteInteger(TypeDecorator):
         return int(value)
 
 
+# NOTE(stephenfin): We deprecate the whole class rather than just the
+# ndb-related arguments, since without these arguments this is identical to the
+# upstream SQLAlchemy type
+@debtcollector.removals.removed_class(
+    'String',
+    message=(
+        'Support for the MySQL NDB Cluster storage engine has been '
+        'deprecated and will be removed in a future release. Use the '
+        'standard String type from sqlalchemy.types'
+    ),
+    version='12.1.0',
+)
 class String(_String):
     """String subclass that implements oslo_db specific options.
 
@@ -124,7 +137,12 @@ class String(_String):
     """
 
     def __init__(
-            self, length, mysql_ndb_length=None, mysql_ndb_type=None, **kw):
+        self,
+        length,
+        mysql_ndb_length=None,
+        mysql_ndb_type=None,
+        **kw,
+    ):
         """Initialize options."""
         super(String, self).__init__(length, **kw)
         self.mysql_ndb_type = mysql_ndb_type
diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py
index 3a6a993..09156f5 100644
--- a/oslo_db/sqlalchemy/utils.py
+++ b/oslo_db/sqlalchemy/utils.py
@@ -1136,6 +1136,13 @@ def get_non_innodb_tables(connectable, skip_tables=('migrate_version',
     return [i[0] for i in noninnodb]
 
 
+@debtcollector.removals.remove(
+    message=(
+        'Support for the MySQL NDB Cluster storage engine has been deprecated '
+        'and will be removed in a future release.'
+    ),
+    version='12.1.0',
+)
 def get_non_ndbcluster_tables(connectable, skip_tables=None):
     """Get a list of tables which don't use MySQL Cluster (NDB) storage engine.
 
@@ -1187,7 +1194,8 @@ def get_foreign_key_constraint_name(engine, table_name, column_name):
 
 @contextlib.contextmanager
 def suspend_fk_constraints_for_col_alter(
-        engine, table_name, column_name, referents=[]):
+    engine, table_name, column_name, referents=[],
+):
     """Detect foreign key constraints, drop, and recreate.
 
     This is used to guard against a column ALTER that on some backends
@@ -1218,11 +1226,16 @@ def suspend_fk_constraints_for_col_alter(
     :param referents: sequence of string table names to search for foreign
      key constraints.   A future version of this function may no longer
      require this argument, however for the moment it is required.
-
     """
-    if (
-        not ndb.ndb_status(engine)
-    ):
+    debtcollector.deprecate(
+        (
+            'Support for the MySQL NDB Cluster storage engine has been '
+            'deprecated and will be removed in a future release.'
+        ),
+        version='12.1.0',
+    )
+
+    if not ndb._ndb_status(engine):
         yield
     else:
         with engine.connect() as conn:
diff --git a/oslo_db/tests/sqlalchemy/test_ndb.py b/oslo_db/tests/sqlalchemy/test_ndb.py
index 06c4d82..c516b1b 100644
--- a/oslo_db/tests/sqlalchemy/test_ndb.py
+++ b/oslo_db/tests/sqlalchemy/test_ndb.py
@@ -15,6 +15,7 @@
 
 import logging
 from unittest import mock
+import warnings
 
 from oslo_db import exception
 from oslo_db.sqlalchemy import enginefacade
@@ -38,12 +39,16 @@ from sqlalchemy import Text
 LOG = logging.getLogger(__name__)
 
 _MOCK_CONNECTION = 'mysql+pymysql://'
-_TEST_TABLE = Table("test_ndb", MetaData(),
-                    Column('id', Integer, primary_key=True),
-                    Column('test1', String(255, mysql_ndb_type=TEXT)),
-                    Column('test2', String(4096, mysql_ndb_type=TEXT)),
-                    Column('test3', String(255, mysql_ndb_length=64)),
-                    mysql_engine='InnoDB')
+with warnings.catch_warnings():  # hide deprecation warning
+    _TEST_TABLE = Table(
+        "test_ndb",
+        MetaData(),
+        Column('id', Integer, primary_key=True),
+        Column('test1', String(255, mysql_ndb_type=TEXT)),
+        Column('test2', String(4096, mysql_ndb_type=TEXT)),
+        Column('test3', String(255, mysql_ndb_length=64)),
+        mysql_engine='InnoDB',
+    )
 
 
 class NDBMockTestBase(test_base.BaseTestCase):
@@ -57,7 +62,9 @@ class NDBMockTestBase(test_base.BaseTestCase):
         self.addCleanup(
             setattr, test_engine.dialect, "_oslodb_enable_ndb_support", False
         )
-        ndb.init_ndb_events(test_engine)
+        # hide deprecation warnings
+        with warnings.catch_warnings():
+            ndb.init_ndb_events(test_engine)
 
 
 class NDBEventTestCase(NDBMockTestBase):
@@ -164,9 +171,10 @@ class NDBOpportunisticTestCase(
             # if we want NDB, make a new local engine that uses the
             # URL / database / schema etc. of the provisioned engine,
             # since NDB-ness is a per-table thing
-            self.engine = engines.create_engine(
-                self.engine.url, mysql_enable_ndb=True
-            )
+            with warnings.catch_warnings():  # hide deprecation warnings
+                self.engine = engines.create_engine(
+                    self.engine.url, mysql_enable_ndb=True
+                )
             self.addCleanup(self.engine.dispose)
         self.test_table = _TEST_TABLE
         try:
@@ -176,7 +184,8 @@ class NDBOpportunisticTestCase(
 
     def test_ndb_enabled(self):
         self.init_db(True)
-        self.assertTrue(ndb.ndb_status(self.engine))
+        with warnings.catch_warnings():  # hide deprecation warnings
+            self.assertTrue(ndb.ndb_status(self.engine))
         self.assertIsInstance(self.test_table.c.test1.type, TINYTEXT)
         self.assertIsInstance(self.test_table.c.test2.type, Text)
         self.assertIsInstance(self.test_table.c.test3.type, String)
@@ -185,7 +194,8 @@ class NDBOpportunisticTestCase(
 
     def test_ndb_disabled(self):
         self.init_db(False)
-        self.assertFalse(ndb.ndb_status(self.engine))
+        with warnings.catch_warnings():  # hide deprecation warnings
+            self.assertFalse(ndb.ndb_status(self.engine))
         self.assertIsInstance(self.test_table.c.test1.type, String)
         self.assertEqual(255, self.test_table.c.test1.type.length)
         self.assertIsInstance(self.test_table.c.test2.type, String)
diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py
index 27ed640..50973fa 100644
--- a/oslo_db/tests/sqlalchemy/test_utils.py
+++ b/oslo_db/tests/sqlalchemy/test_utils.py
@@ -1020,7 +1020,7 @@ class TestMigrationUtils(db_test_base._DbTestCase):
             normalize_fk_entries(existing_foreign_keys)
         )
 
-        with mock.patch("oslo_db.sqlalchemy.ndb.ndb_status",
+        with mock.patch("oslo_db.sqlalchemy.ndb._ndb_status",
                         mock.Mock(return_value=True)):
             with utils.suspend_fk_constraints_for_col_alter(
                     self.engine, 'a', 'id', referents=['b', 'c']):
@@ -1034,7 +1034,7 @@ class TestMigrationUtils(db_test_base._DbTestCase):
 
         self.assertEqual(existing_foreign_keys, get_fk_entries())
 
-        with mock.patch("oslo_db.sqlalchemy.ndb.ndb_status",
+        with mock.patch("oslo_db.sqlalchemy.ndb._ndb_status",
                         mock.Mock(return_value=True)):
             with utils.suspend_fk_constraints_for_col_alter(
                     self.engine, 'b', 'archive_id', referents=['c']):
diff --git a/releasenotes/notes/deprecate-mysql-ndb-cluster-support-cdcaa177b6a6773c.yaml b/releasenotes/notes/deprecate-mysql-ndb-cluster-support-cdcaa177b6a6773c.yaml
new file mode 100644
index 0000000..0fdf75a
--- /dev/null
+++ b/releasenotes/notes/deprecate-mysql-ndb-cluster-support-cdcaa177b6a6773c.yaml
@@ -0,0 +1,5 @@
+---
+deprecations:
+  - |
+    MySQL NDB Cluster support has been deprecated for removal. It appears no
+    one is using this functionality and it's poorly understood.