Permalink
Browse files

Merge "Defer raising an exception when deleting volumes"

  • Loading branch information...
2 parents 3955d18 + 70feb10 commit 0f1694089e064d466df17f1f3e22b390a2b4e9a5 Jenkins committed with openstack-gerrit Jul 17, 2014
Showing with 72 additions and 20 deletions.
  1. +15 −8 nova/compute/manager.py
  2. +0 −12 nova/tests/compute/test_compute.py
  3. +57 −0 nova/tests/compute/test_compute_mgr.py
View
@@ -39,6 +39,7 @@
import eventlet.timeout
from oslo.config import cfg
from oslo import messaging
+import six
from nova import block_device
from nova.cells import rpcapi as cells_rpcapi
@@ -2251,13 +2252,22 @@ def _shutdown_instance(self, context, instance,
self._notify_about_instance_usage(context, instance,
"shutdown.end")
- def _cleanup_volumes(self, context, instance_uuid, bdms):
+ def _cleanup_volumes(self, context, instance_uuid, bdms, raise_exc=True):
+ exc_info = None
+
for bdm in bdms:
LOG.debug("terminating bdm %s", bdm,
instance_uuid=instance_uuid)
if bdm.volume_id and bdm.delete_on_termination:
- self.volume_api.delete(context, bdm.volume_id)
- # NOTE(vish): bdms will be deleted on instance destroy
+ try:
+ self.volume_api.delete(context, bdm.volume_id)
+ except Exception as exc:
+ exc_info = sys.exc_info()
+ LOG.warn(_LW('Failed to delete volume: %(volume_id)s due '
+ 'to %(exc)s'), {'volume_id': bdm.volume_id,
+ 'exc': unicode(exc)})
+ if exc_info is not None and raise_exc:
+ six.reraise(exc_info[0], exc_info[1], exc_info[2])
@hooks.add_hook("delete_instance")
def _delete_instance(self, context, instance, bdms, quotas):
@@ -2294,11 +2304,8 @@ def _delete_instance(self, context, instance, bdms, quotas):
# future to set an instance fault the first time
# and to only ignore the failure if the instance
# is already in ERROR.
- try:
- self._cleanup_volumes(context, instance_uuid, bdms)
- except Exception as exc:
- err_str = _("Ignoring volume cleanup failure due to %s")
- LOG.warn(err_str % exc, instance=instance)
+ self._cleanup_volumes(context, instance_uuid, bdms,
+ raise_exc=False)
# if a delete task succeed, always update vm state and task
# state without expecting task state to be DELETING
instance.vm_state = vm_states.DELETED
@@ -3717,18 +3717,6 @@ def test_instance_set_to_error_on_uncaught_exception(self):
self.compute.terminate_instance(self.context,
self._objectify(instance), [], [])
- def test_delete_instance_succedes_on_volume_fail(self):
- instance = self._create_fake_instance_obj()
-
- def fake_cleanup_volumes(context, instance):
- raise test.TestingException()
-
- self.stubs.Set(self.compute, '_cleanup_volumes',
- fake_cleanup_volumes)
-
- self.compute._delete_instance(self.context, instance, [],
- self.none_quotas)
-
def test_delete_instance_keeps_net_on_power_off_fail(self):
self.mox.StubOutWithMock(self.compute.driver, 'destroy')
self.mox.StubOutWithMock(self.compute, '_deallocate_network')
@@ -30,6 +30,7 @@
from nova import exception
from nova.network import model as network_model
from nova import objects
+from nova.objects import block_device as block_device_obj
from nova.openstack.common import importutils
from nova.openstack.common import uuidutils
from nova import test
@@ -1585,6 +1586,62 @@ def do_test():
self.assertEqual(mock.call.rollback(), quotas.method_calls[0])
set_error.assert_called_once_with(self.context, instance)
+ def test_cleanup_volumes(self):
+ instance = fake_instance.fake_instance_obj(self.context)
+ bdm_do_not_delete_dict = fake_block_device.FakeDbBlockDeviceDict(
+ {'volume_id': 'fake-id1', 'source_type': 'image',
+ 'delete_on_termination': False})
+ bdm_delete_dict = fake_block_device.FakeDbBlockDeviceDict(
+ {'volume_id': 'fake-id2', 'source_type': 'image',
+ 'delete_on_termination': True})
+ bdms = block_device_obj.block_device_make_list(self.context,
+ [bdm_do_not_delete_dict, bdm_delete_dict])
+
+ with mock.patch.object(self.compute.volume_api,
+ 'delete') as volume_delete:
+ self.compute._cleanup_volumes(self.context, instance.uuid, bdms)
+ volume_delete.assert_called_once_with(self.context,
+ bdms[1].volume_id)
+
+ def test_cleanup_volumes_exception_do_not_raise(self):
+ instance = fake_instance.fake_instance_obj(self.context)
+ bdm_dict1 = fake_block_device.FakeDbBlockDeviceDict(
+ {'volume_id': 'fake-id1', 'source_type': 'image',
+ 'delete_on_termination': True})
+ bdm_dict2 = fake_block_device.FakeDbBlockDeviceDict(
+ {'volume_id': 'fake-id2', 'source_type': 'image',
+ 'delete_on_termination': True})
+ bdms = block_device_obj.block_device_make_list(self.context,
+ [bdm_dict1, bdm_dict2])
+
+ with mock.patch.object(self.compute.volume_api,
+ 'delete',
+ side_effect=[test.TestingException(), None]) as volume_delete:
+ self.compute._cleanup_volumes(self.context, instance.uuid, bdms,
+ raise_exc=False)
+ calls = [mock.call(self.context, bdm.volume_id) for bdm in bdms]
+ self.assertEqual(calls, volume_delete.call_args_list)
+
+ def test_cleanup_volumes_exception_raise(self):
+ instance = fake_instance.fake_instance_obj(self.context)
+ bdm_dict1 = fake_block_device.FakeDbBlockDeviceDict(
+ {'volume_id': 'fake-id1', 'source_type': 'image',
+ 'delete_on_termination': True})
+ bdm_dict2 = fake_block_device.FakeDbBlockDeviceDict(
+ {'volume_id': 'fake-id2', 'source_type': 'image',
+ 'delete_on_termination': True})
+ bdms = block_device_obj.block_device_make_list(self.context,
+ [bdm_dict1, bdm_dict2])
+
+ with mock.patch.object(self.compute.volume_api,
+ 'delete',
+ side_effect=[test.TestingException(), None]) as volume_delete:
+ self.assertRaises(test.TestingException,
+ self.compute._cleanup_volumes, self.context, instance.uuid,
+ bdms)
+ calls = [mock.call(self.context, bdm.volume_id) for bdm in bdms]
+ self.assertEqual(calls, volume_delete.call_args_list)
+
class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
def setUp(self):

0 comments on commit 0f16940

Please sign in to comment.