From aaee893a5e830ce166b592fe7fa414c714b46f92 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 8 Apr 2019 17:44:18 -0400 Subject: [PATCH] Drop delete_build_requests_with_no_instance_uuid online migration This migration was added in Ocata and backported to Newton in change I8a05ee01ec7f6a6f88b896f78414fb5487e0071e to deal with Mitaka-era build_requests records that would not have an instance_uuid value and thus raise a ValueError in BuildRequest._from_db_object (because BuildRequest.instance_uuid is not nullable). This is essentially a revert of that change now since operators have had long enough to run the migration. If anyone were to skip level upgrade from Mitaka to Train (which we don't support, we require you to roll through), and hit an issue with this they could simply execute this on their nova_api DB: DELETE FROM build_requests WHERE instance_uuid IS NULL; Change-Id: Ie9593657544b7aef1fd7a5c8f01e30e09e3fcce6 --- doc/source/cli/nova-manage.rst | 1 - nova/cmd/manage.py | 5 -- nova/db/sqlalchemy/api_models.py | 1 + nova/objects/build_request.py | 50 ----------- .../tests/functional/db/test_build_request.py | 82 ------------------- 5 files changed, 1 insertion(+), 138 deletions(-) diff --git a/doc/source/cli/nova-manage.rst b/doc/source/cli/nova-manage.rst index b12aef2cca..017aa5834b 100644 --- a/doc/source/cli/nova-manage.rst +++ b/doc/source/cli/nova-manage.rst @@ -117,7 +117,6 @@ Nova Database | Migration | Total Needed | Completed | +---------------------------------------------+--------------+-----------+ | create_incomplete_consumers | 0 | 0 | - | delete_build_requests_with_no_instance_uuid | 0 | 0 | | migrate_instances_add_request_spec | 2 | 0 | | migrate_quota_classes_to_api_db | 0 | 0 | | migrate_quota_limits_to_api_db | 0 | 0 | diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 440905ca2a..0b37e8af3c 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -59,7 +59,6 @@ from nova import exception from nova.i18n import _ from nova import objects from nova.objects import block_device as block_device_obj -from nova.objects import build_request as build_request_obj from nova.objects import compute_node as compute_node_obj from nova.objects import host_mapping as host_mapping_obj from nova.objects import instance as instance_obj @@ -380,10 +379,6 @@ class DbCommands(object): # not migratable (or don't need migrating), but all migrations that can # complete have finished. online_migrations = ( - # Added in Ocata - # NOTE(mriedem): This online migration is going to be backported to - # Newton also since it's an upgrade issue when upgrading from Mitaka. - build_request_obj.delete_build_requests_with_no_instance_uuid, # Added in Pike db.service_uuids_online_data_migration, # Added in Pike diff --git a/nova/db/sqlalchemy/api_models.py b/nova/db/sqlalchemy/api_models.py index 609a75e2f4..c709f047aa 100644 --- a/nova/db/sqlalchemy/api_models.py +++ b/nova/db/sqlalchemy/api_models.py @@ -250,6 +250,7 @@ class BuildRequest(API_BASE): ) id = Column(Integer, primary_key=True) + # TODO(mriedem): instance_uuid should be nullable=False instance_uuid = Column(String(36)) project_id = Column(String(255), nullable=False) instance = Column(MediumText()) diff --git a/nova/objects/build_request.py b/nova/objects/build_request.py index 86303c62af..9615ab8c80 100644 --- a/nova/objects/build_request.py +++ b/nova/objects/build_request.py @@ -18,7 +18,6 @@ from oslo_serialization import jsonutils from oslo_utils import versionutils from oslo_versionedobjects import exception as ovoo_exc import six -from sqlalchemy.sql import null from nova.db.sqlalchemy import api as db from nova.db.sqlalchemy import api_models @@ -462,52 +461,3 @@ class BuildRequestList(base.ObjectListBase, base.NovaObject): return cls(context, objects=sorted_build_reqs[marker_index:limit_index]) - - -@db.api_context_manager.reader -def _get_build_requests_with_no_instance_uuid(context, limit): - """Returns up to $limit build_requests where instance_uuid is null""" - # build_requests don't use the SoftDeleteMixin so we don't have to filter - # on the deleted column. - return context.session.query(api_models.BuildRequest).\ - filter_by(instance_uuid=null()).\ - limit(limit).\ - all() - - -@db.api_context_manager.writer -def _destroy_in_db(context, id): - return context.session.query(api_models.BuildRequest).filter_by( - id=id).delete() - - -def delete_build_requests_with_no_instance_uuid(context, count): - """Online data migration which cleans up failed build requests from Mitaka - - build_requests were initially a mirror of instances and had similar fields - to satisfy listing/showing instances while they were building. In Mitaka - if an instance failed to build we'd delete the instance but didn't delete - the associated BuildRequest. In the Newton release we changed the schema - on the build_requests table to just store a serialized Instance object and - added an instance_uuid field which is expected to not be None as seen how - it's used in _from_db_object. However, failed build requests created before - that schema migration won't have the instance_uuid set and fail to load - as an object when calling BuildRequestList.get_all(). So we need to perform - a cleanup routine here where we search for build requests which do not have - the instance_uuid field set and delete them. - - :param context: The auth context used to query the database. - :type context: nova.context.RequestContext - :param count: The max number of build requests to delete. - :type count: int - :returns: 2-item tuple of - (number of orphaned build requests read from DB, number deleted) - """ - orphaned_build_requests = ( - _get_build_requests_with_no_instance_uuid(context, count)) - done = 0 - for orphan_buildreq in orphaned_build_requests: - result = _destroy_in_db(context, orphan_buildreq.id) - if result: - done += 1 - return len(orphaned_build_requests), done diff --git a/nova/tests/functional/db/test_build_request.py b/nova/tests/functional/db/test_build_request.py index 71e9091fd2..eb5bfd3add 100644 --- a/nova/tests/functional/db/test_build_request.py +++ b/nova/tests/functional/db/test_build_request.py @@ -10,11 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. -import datetime - from oslo_serialization import jsonutils from oslo_utils import uuidutils -import six from nova import context from nova import exception @@ -106,85 +103,6 @@ class BuildRequestTestCase(test.NoDBTestCase): db_req.destroy() self.assertRaises(exception.BuildRequestNotFound, db_req.save) - def _get_mitaka_db_build_request_no_instance_uuid(self): - fake_info_cache = objects.InstanceInfoCache(network_model=[]) - fake_secgroups = objects.SecurityGroupList() - # This is more or less taken straight from bug 1633734. - db_req = { - 'created_at': datetime.datetime(2016, 8, 2, 20, 26, 20), - 'updated_at': None, - 'project_id': self.context.project_id, - 'user_id': self.context.user_id, - 'display_name': 'admin-auth', - 'instance_metadata': None, - 'progress': 0, - 'vm_state': 'building', - 'task_state': 'scheduling', - 'image_ref': None, - 'access_ip_v4': None, - 'access_ip_v6': None, - 'info_cache': jsonutils.dumps(fake_info_cache.obj_to_primitive()), - 'security_groups': - jsonutils.dumps(fake_secgroups.obj_to_primitive()), - 'config_drive': 1, - 'key_name': 'Turbo_Fredriksson', - 'locked_by': None, - 'instance_uuid': None, - 'instance': None, - 'block_device_mappings': None, - } - return db_req - - def test_load_from_broken_mitaka_build_request_with_no_instance(self): - db_req = self._get_mitaka_db_build_request_no_instance_uuid() - db_req = self.build_req_obj._create_in_db(self.context, db_req) - # This should fail because the build request in the database does not - # have instance_uuid set, and BuildRequest.instance_uuid is not - # nullable so trying to set build_request.instance_uuid = None is going - # to raise the ValueError. - ex = self.assertRaises(ValueError, - build_request.BuildRequest._from_db_object, - self.context, self.build_req_obj, db_req) - self.assertIn('instance_uuid', six.text_type(ex)) - - def test_delete_build_requests_with_no_instance_uuid(self): - """Tests the online data migration used to cleanup failed Mitaka-era - build requests that have no instance_uuid set. - """ - # First let's create 2 of these busted build requests so we can test - # paging. - for x in range(2): - db_req = self._get_mitaka_db_build_request_no_instance_uuid() - self.build_req_obj._create_in_db(self.context, db_req) - # nova-manage uses an admin contet - ctxt = context.get_admin_context() - - # Make sure we can get 0 back without deleting any. - total, deleted = ( - build_request.delete_build_requests_with_no_instance_uuid(ctxt, 0)) - self.assertEqual(0, total) - self.assertEqual(0, deleted) - - # Delete only 1. - total, deleted = ( - build_request.delete_build_requests_with_no_instance_uuid(ctxt, 1)) - self.assertEqual(1, total) - self.assertEqual(1, deleted) - - # Delete 50 (default in nova-manage online_data_migrations). - total, deleted = ( - build_request.delete_build_requests_with_no_instance_uuid( - ctxt, 50)) - self.assertEqual(1, total) - self.assertEqual(1, deleted) - - # Do it again, nothing should come back. - total, deleted = ( - build_request.delete_build_requests_with_no_instance_uuid( - ctxt, 50)) - self.assertEqual(0, total) - self.assertEqual(0, deleted) - class BuildRequestListTestCase(test.NoDBTestCase): USES_DB_SELF = True