Merge "Make DB migration creating indexes in RBACs conditional"

doc/source/admin/config-rbac.rst

@@ -804,3 +804,37 @@ IDs.
 If an operator wants to prevent normal users from doing this, the
 ``"create_rbac_policy":`` entry in ``policy.yaml`` can be adjusted
 from ``""`` to ``"rule:admin_only"``.
+
+
+Improve database RBAC query operations
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Since [1]_, present in Yoga version, Neutron has indexes for
+"target_tenant" (now "target_project") and "action" columns in all
+RBAC related tables. That improves the SQL queries involving the
+RBAC tables [2]_. Any system before Yoga won't have these indexes
+but the system administrator can manually add them to the Neutron
+database following the next steps:
+
+* Find the RBAC tables:
+
+.. code-block:: console
+
+    $ tables=`mysql -e "use ovs_neutron; show tables;" | grep rbac`
+
+
+* Insert the indexes for the "target_tenant" and "action" columns:
+
+    $ for table in $tables do; mysql -e \
+        "alter table $table add key (action); alter table $table add key (target_tenant);"; done
+
+
+In order to prevent errors during a system upgrade, [3]_ was
+implemented and backported up to Yoga. This patch checks if any index
+is already present in the Neutron tables and avoids executing the
+index creation command again.
+
+
+.. [1] https://review.opendev.org/c/openstack/neutron/+/810072
+.. [2] https://github.com/openstack/neutron-lib/blob/890d62a3df3f35bb18bf1a11e79a9e97e7dd2d2c/neutron_lib/db/model_query.py#L123-L131
+.. [3] https://review.opendev.org/c/openstack/neutron/+/884617

neutron/db/migration/alembic_migrations/versions/yoga/expand/ba859d649675_add_indexes_to_rbacs.py

@@ -14,6 +14,7 @@
 #
 
 from alembic import op
+import sqlalchemy as sa
 
 
 """Add indexes to RBACs
@@ -31,6 +32,27 @@ down_revision = 'c181bb1d89e4'
 OBJECTS = ('network', 'qospolicy', 'securitygroup', 'addressscope',
            'subnetpool', 'addressgroup')
 COLUMNS = ('target_tenant', 'action')
+_INSPECTOR = None
+
+
+def get_inspector():
+    global _INSPECTOR
+    if _INSPECTOR:
+        return _INSPECTOR
+    else:
+        _INSPECTOR = sa.inspect(op.get_bind())
+
+    return _INSPECTOR
+
+
+def has_index(table, column):
+    """Check if the table has an index *using only* the column name provided"""
+    inspector = get_inspector()
+    table_indexes = inspector.get_indexes(table)
+    for index in table_indexes:
+        if [column] == index['column_names']:
+            return True
+    return False
 
 
 def upgrade():
@@ -38,4 +60,6 @@ def upgrade():
         table = object + 'rbacs'
         ix = 'ix_' + table + '_'
         for column in COLUMNS:
-            op.create_index(op.f(ix + column), table, [column], unique=False)
+            if not has_index(table, column):
+                op.create_index(op.f(ix + column), table, [column],
+                                unique=False)

neutron/tests/functional/db/migrations/test_ba859d649675_add_indexes_to_rbacs.py

@@ -0,0 +1,55 @@
+# Copyright 2017 OpenStack Foundation
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+from oslo_db.sqlalchemy import utils as db_utils
+
+from neutron.db.migration.alembic_migrations.versions.yoga.expand import \
+    ba859d649675_add_indexes_to_rbacs as _migration
+from neutron.tests.functional.db import test_migrations
+
+
+class TestAddIndexesToRbacsMixin(object):
+    """Validates binding_index for NetworkDhcpAgentBinding migration."""
+
+    @staticmethod
+    def get_index(table_indexes, column):
+        for index in table_indexes:
+            if [column] == index['column_names']:
+                return True
+        return False
+
+    def _pre_upgrade_ba859d649675(self, engine):
+        for table in _migration.OBJECTS:
+            table_indexes = db_utils.get_indexes(engine, table + 'rbacs')
+            for column in _migration.COLUMNS:
+                self.assertFalse(self.get_index(table_indexes, column))
+
+    def _check_ba859d649675(self, engine, data):
+        for table in _migration.OBJECTS:
+            table_indexes = db_utils.get_indexes(engine, table + 'rbacs')
+            for column in _migration.COLUMNS:
+                self.assertTrue(self.get_index(table_indexes, column))
+
+
+class TestAddIndexesToRbacsMySQL(
+        TestAddIndexesToRbacsMixin,
+        test_migrations.TestWalkMigrationsMySQL):
+    pass
+
+
+class TestAddIndexesToRbacsPostgreSQL(
+        TestAddIndexesToRbacsMixin,
+        test_migrations.TestWalkMigrationsPostgreSQL):
+    pass