openstack
/
deb-cinder
Code Issues Proposed changes

Delete volume when rescheduling 

If a reschedulable failure happens in the driver after the volume was
created on the backend, it will get orphaned and only way to remove it
will be through backend internals. This commit adds an additional
delete_volume call to the driver after the volume got rescheduled to
make sure no volumes will get orphaned.

Change-Id: Idd86a4842bdc6ecf0cabbeff0a9c9704e030302a
Closes-Bug: 1561579
This commit is contained in:
Michał Dulko 2017-01-09 15:28:20 +01:00
parent 7502e1fed3
commit 7f09229d6c
2 changed files with 23 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
cinder/tests/unit/test_volume.py
View File

@ -531,6 +531,7 @@ class VolumeTestCase(base.BaseVolumeTestCase):
def test_create_driver_not_initialized_rescheduling(self):
self.volume.driver._initialized = False
mock_delete = self.mock_object(self.volume.driver, 'delete_volume')
volume = tests_utils.create_volume(
self.context,
@ -548,6 +549,10 @@ class VolumeTestCase(base.BaseVolumeTestCase):
# allocated_capacity tracking.
self.assertEqual({}, self.volume.stats['pools'])
# NOTE(dulek): As we've rescheduled, make sure delete_volume was
# called.
self.assertTrue(mock_delete.called)
db.volume_destroy(context.get_admin_context(), volume_id)
def test_create_non_cinder_exception_rescheduling(self):
@ -3708,12 +3713,16 @@ class VolumeTestCase(base.BaseVolumeTestCase):
self.mock_object(self.volume.driver, 'copy_image_to_volume',
fake_copy_image_to_volume)
mock_delete = self.mock_object(self.volume.driver, 'delete_volume')
self.assertRaises(exception.ImageCopyFailure,
self._create_volume_from_image)
# NOTE(dulek): Rescheduling should not occur, so lets assert that
# allocated_capacity is incremented.
self.assertDictEqual(self.volume.stats['pools'],
{'_pool0': {'allocated_capacity_gb': 1}})
# NOTE(dulek): As we haven't rescheduled, make sure no delete_volume
# was called.
self.assertFalse(mock_delete.called)
@mock.patch('cinder.utils.brick_get_connector_properties')
@mock.patch('cinder.utils.brick_get_connector')

18
cinder/volume/flows/manager/create_volume.py
View File

@ -58,7 +58,7 @@ class OnFailureRescheduleTask(flow_utils.CinderTask):
this volume elsewhere.
"""
def __init__(self, reschedule_context, db, scheduler_rpcapi,
def __init__(self, reschedule_context, db, driver, scheduler_rpcapi,
do_reschedule):
requires = ['filter_properties', 'request_spec', 'volume',
'context']
@ -67,6 +67,7 @@ class OnFailureRescheduleTask(flow_utils.CinderTask):
self.do_reschedule = do_reschedule
self.scheduler_rpcapi = scheduler_rpcapi
self.db = db
self.driver = driver
self.reschedule_context = reschedule_context
# These exception types will trigger the volume to be set into error
# status rather than being rescheduled.
@ -151,6 +152,16 @@ class OnFailureRescheduleTask(flow_utils.CinderTask):
LOG.debug("Volume %s: re-scheduled", volume.id)
# NOTE(dulek): Here we should be sure that rescheduling occurred and
# host field will be erased. Just in case volume was already created at
# the backend, we attempt to delete it.
try:
self.driver.delete_volume(volume)
except Exception:
# Most likely the volume weren't created at the backend. We can
# safely ignore this.
pass
def revert(self, context, result, flow_failures, volume, **kwargs):
# NOTE(dulek): Revert is occurring and manager need to know if
# rescheduling happened. We're returning boolean flag that will
@ -943,9 +954,8 @@ def get_flow(context, manager, db, driver, scheduler_rpcapi, host, volume,
# status when reverting the flow. Meanwhile, no need to revert process of
# ExtractVolumeRefTask.
do_reschedule = allow_reschedule and request_spec and retry
volume_flow.add(OnFailureRescheduleTask(reschedule_context, db,
scheduler_rpcapi,
do_reschedule))
volume_flow.add(OnFailureRescheduleTask(reschedule_context, db, driver,
scheduler_rpcapi, do_reschedule))
LOG.debug("Volume reschedule parameters: %(allow)s "
"retry: %(retry)s", {'allow': allow_reschedule, 'retry': retry})