diff --git a/doc/source/cli/nova-manage.rst b/doc/source/cli/nova-manage.rst index 6b633e2818..bb17990fe4 100644 --- a/doc/source/cli/nova-manage.rst +++ b/doc/source/cli/nova-manage.rst @@ -122,7 +122,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 5b7652e01b..9c0082259e 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -60,7 +60,6 @@ from nova.network.neutronv2 import api as neutron_api from nova.network.neutronv2 import constants 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 @@ -381,10 +380,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 469fec47f9..750b9e3ca6 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