db: Remove unused build_requests columns

These fields were never used in the API database. They can be removed
now, some years after originally intended.

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

3 files changed, 81 insertions, 32 deletions

diff --git a/nova/db/api/migrations/versions/b30f573d3377_remove_unused_build_requests_columns.py b/nova/db/api/migrations/versions/b30f573d3377_remove_unused_build_requests_columns.py
new file mode 100644
index 0000000000..39c5f1306d
--- /dev/null
+++ b/nova/db/api/migrations/versions/b30f573d3377_remove_unused_build_requests_columns.py
@@ -0,0 +1,45 @@
+# 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.
+
+"""Remove unused build_requests columns
+
+Revision ID: b30f573d3377
+Revises: d67eeaabee36
+Create Date: 2021-09-27 14:46:05.986174
+"""
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = 'b30f573d3377'
+down_revision = 'd67eeaabee36'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    with op.batch_alter_table('build_requests', schema=None) as batch_op:
+        batch_op.drop_column('vm_state')
+        batch_op.drop_column('access_ip_v6')
+        batch_op.drop_column('config_drive')
+        batch_op.drop_column('locked_by')
+        batch_op.drop_column('security_groups')
+        batch_op.drop_column('progress')
+        batch_op.drop_column('info_cache')
+        batch_op.drop_column('display_name')
+        batch_op.drop_column('instance_metadata')
+        batch_op.drop_column('image_ref')
+        batch_op.drop_column('key_name')
+        batch_op.drop_column('user_id')
+        batch_op.drop_column('access_ip_v4')
+        batch_op.drop_column('task_state')
+        batch_op.drop_column('request_spec_id')
diff --git a/nova/db/api/models.py b/nova/db/api/models.py
index afb7709def..7e0308c2f9 100644
--- a/nova/db/api/models.py
+++ b/nova/db/api/models.py
@@ -34,36 +34,12 @@ LOG = logging.getLogger(__name__)
 # underlying tables and attempts to resolve it. Tell it instead to ignore these
 # until we're ready to remove them ourselves.
 REMOVED_COLUMNS = {
-    ('build_requests', 'request_spec_id'),
-    ('build_requests', 'user_id'),
-    ('build_requests', 'display_name'),
-    ('build_requests', 'instance_metadata'),
-    ('build_requests', 'progress'),
-    ('build_requests', 'vm_state'),
-    ('build_requests', 'task_state'),
-    ('build_requests', 'image_ref'),
-    ('build_requests', 'access_ip_v4'),
-    ('build_requests', 'access_ip_v6'),
-    ('build_requests', 'info_cache'),
-    ('build_requests', 'security_groups'),
-    ('build_requests', 'config_drive'),
-    ('build_requests', 'key_name'),
-    ('build_requests', 'locked_by'),
-    ('build_requests', 'reservation_id'),
-    ('build_requests', 'launch_index'),
-    ('build_requests', 'hostname'),
-    ('build_requests', 'kernel_id'),
-    ('build_requests', 'ramdisk_id'),
-    ('build_requests', 'root_device_name'),
-    ('build_requests', 'user_data'),
     ('resource_providers', 'can_host'),
 }
 
 # NOTE(stephenfin): A list of foreign key constraints that were removed when
 # the column they were covering was removed.
-REMOVED_FKEYS = [
-    ('build_requests', ['request_spec_id']),
-]
+REMOVED_FKEYS = []
 
 # NOTE(stephenfin): A list of entire models that have been removed.
 REMOVED_TABLES = []
@@ -302,13 +278,6 @@ class BuildRequest(BASE):
     instance = sa.Column(types.MediumText())
     block_device_mappings = sa.Column(types.MediumText())
     tags = sa.Column(sa.Text())
-    # TODO(alaski): Drop these from the db in Ocata
-    # columns_to_drop = ['request_spec_id', 'user_id', 'display_name',
-    #         'instance_metadata', 'progress', 'vm_state', 'task_state',
-    #         'image_ref', 'access_ip_v4', 'access_ip_v6', 'info_cache',
-    #         'security_groups', 'config_drive', 'key_name', 'locked_by',
-    #         'reservation_id', 'launch_index', 'hostname', 'kernel_id',
-    #         'ramdisk_id', 'root_device_name', 'user_data']
 
 
 class KeyPair(BASE):
diff --git a/nova/tests/unit/db/api/test_migrations.py b/nova/tests/unit/db/api/test_migrations.py
index 0f51ad12a7..534be0822d 100644
--- a/nova/tests/unit/db/api/test_migrations.py
+++ b/nova/tests/unit/db/api/test_migrations.py
@@ -29,6 +29,7 @@ from oslo_db.sqlalchemy import enginefacade
 from oslo_db.sqlalchemy import test_fixtures
 from oslo_db.sqlalchemy import test_migrations
 from oslo_log import log as logging
+import sqlalchemy
 import testtools
 
 from nova.db.api import models
@@ -194,6 +195,40 @@ class NovaMigrationsWalk(
         if post_upgrade:
             post_upgrade(connection)
 
+    _b30f573d3377_removed_columns = {
+        'access_ip_v4',
+        'access_ip_v6',
+        'config_drive',
+        'display_name',
+        'image_ref',
+        'info_cache',
+        'instance_metadata',
+        'key_name',
+        'locked_by',
+        'progress',
+        'request_spec_id',
+        'security_groups',
+        'task_state',
+        'user_id',
+        'vm_state',
+    }
+
+    def _pre_upgrade_b30f573d3377(self, connection):
+        # we use the inspector here rather than oslo_db.utils.column_exists,
+        # since the latter will create a new connection
+        inspector = sqlalchemy.inspect(connection)
+        columns = [x['name'] for x in inspector.get_columns('build_requests')]
+        for removed_column in self._b30f573d3377_removed_columns:
+            self.assertIn(removed_column, columns)
+
+    def _check_b30f573d3377(self, connection):
+        # we use the inspector here rather than oslo_db.utils.column_exists,
+        # since the latter will create a new connection
+        inspector = sqlalchemy.inspect(connection)
+        columns = [x['name'] for x in inspector.get_columns('build_requests')]
+        for removed_column in self._b30f573d3377_removed_columns:
+            self.assertNotIn(removed_column, columns)
+
     def test_single_base_revision(self):
         """Ensure we only have a single base revision.