From d8e695cb900ad71996e6cf970894bc2e3f2df8b4 Mon Sep 17 00:00:00 2001 From: Prateek Arora Date: Fri, 18 Mar 2016 06:46:43 -0400 Subject: [PATCH] Delete traces of in-progress snapshot on VM being deleted When user tries to create snapshot of instance and at the same time if another request tries to delete the instance, at that time image goes in saving status for forever because of race between instance delete and snapshot requests. Caught exceptions(InstanceNotFound and UnexpectedDeletingTaskStateError) in except block and deleting the image which got stuck in saving state. Closes-Bug: #1555065 Change-Id: If0b918dc951030e6b6ffba147443225e0e4a370a --- nova/compute/api.py | 31 ++++++++++- nova/tests/unit/compute/test_compute_api.py | 60 +++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 9ecc9b0bf67b..ec63ef416856 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2318,7 +2318,36 @@ class API(base.Base): # NOTE(comstud): Any changes to this method should also be made # to the snapshot_instance() method in nova/cells/messaging.py instance.task_state = task_states.IMAGE_SNAPSHOT_PENDING - instance.save(expected_task_state=[None]) + try: + instance.save(expected_task_state=[None]) + except (exception.InstanceNotFound, + exception.UnexpectedDeletingTaskStateError) as ex: + # Changing the instance task state to use in raising the + # InstanceInvalidException below + LOG.debug('Instance disappeared during snapshot.', + instance=instance) + try: + image_id = image_meta['id'] + self.image_api.delete(context, image_id) + LOG.info(_LI('Image %s deleted because instance ' + 'deleted before snapshot started.'), + image_id, instance=instance) + except exception.ImageNotFound: + pass + except Exception as exc: + msg = _LW("Error while trying to clean up image %(img_id)s: " + "%(error_msg)s") + LOG.warning(msg, {"img_id": image_meta['id'], + "error_msg": six.text_type(exc)}) + attr = 'task_state' + state = task_states.DELETING + if type(ex) == exception.InstanceNotFound: + attr = 'vm_state' + state = vm_states.DELETED + raise exception.InstanceInvalidState(attr=attr, + instance_uuid=instance.uuid, + state=state, + method='snapshot') self.compute_rpcapi.snapshot_instance(context, instance, image_meta['id']) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 7cddc7abe04f..21865b5c243c 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -2488,6 +2488,66 @@ class _ComputeAPIUnitTestMixIn(object): self.compute_api.snapshot, self.context, instance, 'fake-name') + @mock.patch.object(objects.Instance, 'save') + @mock.patch.object(compute_api.API, '_create_image') + @mock.patch.object(compute_rpcapi.ComputeAPI, + 'snapshot_instance') + def test_vm_deleting_while_creating_snapshot(self, + snapshot_instance, + _create_image, save): + instance = self._create_instance_obj() + save.side_effect = exception.UnexpectedDeletingTaskStateError( + "Exception") + _create_image.return_value = dict(id='fake-image-id') + with mock.patch.object(self.compute_api.image_api, + 'delete') as image_delete: + self.assertRaises(exception.InstanceInvalidState, + self.compute_api.snapshot, + self.context, + instance, 'fake-image') + image_delete.assert_called_once_with(self.context, + 'fake-image-id') + + @mock.patch.object(objects.Instance, 'save') + @mock.patch.object(compute_api.API, '_create_image') + @mock.patch.object(compute_rpcapi.ComputeAPI, + 'snapshot_instance') + def test_vm_deleted_while_creating_snapshot(self, + snapshot_instance, + _create_image, save): + instance = self._create_instance_obj() + save.side_effect = exception.InstanceNotFound( + "Exception") + _create_image.return_value = dict(id='fake-image-id') + with mock.patch.object(self.compute_api.image_api, + 'delete') as image_delete: + self.assertRaises(exception.InstanceInvalidState, + self.compute_api.snapshot, + self.context, + instance, 'fake-image') + image_delete.assert_called_once_with(self.context, + 'fake-image-id') + + @mock.patch.object(objects.Instance, 'save') + @mock.patch.object(compute_api.API, '_create_image') + @mock.patch.object(compute_rpcapi.ComputeAPI, + 'snapshot_instance') + def test_vm_deleted_while_snapshot_and_snapshot_delete_failed(self, + snapshot_instance, + _create_image, save): + instance = self._create_instance_obj() + save.side_effect = exception.InstanceNotFound(instance_id='fake') + _create_image.return_value = dict(id='fake-image-id') + with mock.patch.object(self.compute_api.image_api, + 'delete') as image_delete: + image_delete.side_effect = test.TestingException() + self.assertRaises(exception.InstanceInvalidState, + self.compute_api.snapshot, + self.context, + instance, 'fake-image') + image_delete.assert_called_once_with(self.context, + 'fake-image-id') + def test_snapshot_with_base_image_ref(self): self._test_snapshot_and_backup(with_base_ref=True)