From 47d70deb358bcc304cf096b49d9eb7ab7a896b4f Mon Sep 17 00:00:00 2001 From: Andrew Laski Date: Fri, 27 May 2016 16:26:25 -0400 Subject: [PATCH] Raise exception if BuildRequest deleted twice After an instance has been scheduled to a cell and mapped to the instance table there the BuildRequest for it will be destroyed. An instance delete that occurs before that remapping has happened will also destroy the BuildRequest. In order to know that a delete has happened, and stop the build, the destroy method should raise an exception if it is already deleted. Change-Id: Ib86ac14e2bd933bb9e76ea86b2a1ec4f40d204a3 --- nova/objects/build_request.py | 10 ++++++---- nova/tests/functional/db/test_build_request.py | 16 ++++++++++++++++ nova/tests/unit/objects/test_build_request.py | 6 ++++-- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/nova/objects/build_request.py b/nova/objects/build_request.py index 0289fe957445..19ee1da55cad 100644 --- a/nova/objects/build_request.py +++ b/nova/objects/build_request.py @@ -132,10 +132,12 @@ class BuildRequest(base.NovaObject): @staticmethod @db.api_context_manager.writer - def _destroy_in_db(context, id): - context.session.query(api_models.BuildRequest).filter_by( - id=id).delete() + def _destroy_in_db(context, instance_uuid): + result = context.session.query(api_models.BuildRequest).filter_by( + instance_uuid=instance_uuid).delete() + if not result: + raise exception.BuildRequestNotFound(uuid=instance_uuid) @base.remotable def destroy(self): - self._destroy_in_db(self._context, self.id) + self._destroy_in_db(self._context, self.instance_uuid) diff --git a/nova/tests/functional/db/test_build_request.py b/nova/tests/functional/db/test_build_request.py index c9a424191f05..4aa04bf13ab7 100644 --- a/nova/tests/functional/db/test_build_request.py +++ b/nova/tests/functional/db/test_build_request.py @@ -90,3 +90,19 @@ class BuildRequestTestCase(test.NoDBTestCase): 'keypairs': obj_comp}) continue self.assertEqual(expected, db_value) + + def test_destroy(self): + self._create_req() + db_req = self.build_req_obj.get_by_instance_uuid(self.context, + self.instance_uuid) + db_req.destroy() + self.assertRaises(exception.BuildRequestNotFound, + self.build_req_obj._get_by_instance_uuid_from_db, self.context, + self.instance_uuid) + + def test_destroy_twice_raises(self): + self._create_req() + db_req = self.build_req_obj.get_by_instance_uuid(self.context, + self.instance_uuid) + db_req.destroy() + self.assertRaises(exception.BuildRequestNotFound, db_req.destroy) diff --git a/nova/tests/unit/objects/test_build_request.py b/nova/tests/unit/objects/test_build_request.py index 41462924ca40..3aac48bafeb9 100644 --- a/nova/tests/unit/objects/test_build_request.py +++ b/nova/tests/unit/objects/test_build_request.py @@ -20,6 +20,7 @@ from nova.objects import build_request from nova.tests.unit import fake_build_request from nova.tests.unit import fake_instance from nova.tests.unit.objects import test_objects +from nova.tests import uuidsentinel as uuids class _TestBuildRequestObject(object): @@ -94,10 +95,11 @@ class _TestBuildRequestObject(object): @mock.patch.object(build_request.BuildRequest, '_destroy_in_db') def test_destroy(self, destroy_in_db): req_obj = build_request.BuildRequest(self.context) - req_obj.id = 1 + req_obj.instance_uuid = uuids.instance req_obj.destroy() - destroy_in_db.assert_called_once_with(self.context, req_obj.id) + destroy_in_db.assert_called_once_with(self.context, + req_obj.instance_uuid) class TestBuildRequestObject(test_objects._LocalTest,