Merge "RBD: Trash image when snapshots prevent deletion"
This commit is contained in:
commit
c401d3e371
|
@ -471,14 +471,6 @@ class Store(driver.Store):
|
|||
if snapshot_name is not None:
|
||||
with rbd.Image(ioctx, image_name) as image:
|
||||
try:
|
||||
# NOTE(abhishekk): Check whether snapshot
|
||||
# has any external references
|
||||
if self._snapshot_has_external_reference(
|
||||
image, snapshot_name):
|
||||
raise rbd.ImageBusy(
|
||||
"Image snapshot has external "
|
||||
"references.")
|
||||
|
||||
self._unprotect_snapshot(image, snapshot_name)
|
||||
image.remove_snap(snapshot_name)
|
||||
except rbd.ImageNotFound as exc:
|
||||
|
@ -498,10 +490,19 @@ class Store(driver.Store):
|
|||
# Then delete image.
|
||||
rbd.RBD().remove(ioctx, image_name)
|
||||
except rbd.ImageHasSnapshots:
|
||||
log_msg = (_LW("Remove image %(img_name)s failed. "
|
||||
"It has snapshot(s) left.") %
|
||||
log_msg = (_LW("Unable to remove image %(img_name)s: it "
|
||||
"has snapshot(s) left; trashing instead") %
|
||||
{'img_name': image_name})
|
||||
LOG.warning(log_msg)
|
||||
with rbd.Image(ioctx, image_name) as image:
|
||||
try:
|
||||
rbd.RBD().trash_move(ioctx, image_name)
|
||||
LOG.debug('Moved %s to trash', image_name)
|
||||
except rbd.ImageBusy:
|
||||
LOG.warning(_('Unable to move in-use image to '
|
||||
'trash'))
|
||||
raise exceptions.InUseByStore()
|
||||
return
|
||||
raise exceptions.HasSnapshot()
|
||||
except rbd.ImageBusy:
|
||||
log_msg = (_LW("Remove image %(img_name)s failed. "
|
||||
|
|
|
@ -172,6 +172,9 @@ class MockRBD(object):
|
|||
def clone(self, *args, **kwargs):
|
||||
raise NotImplementedError()
|
||||
|
||||
def trash_move(self, *args, **kwargs):
|
||||
pass
|
||||
|
||||
RBD_FEATURE_LAYERING = 1
|
||||
|
||||
|
||||
|
@ -427,10 +430,9 @@ class TestMultiStore(base.MultiStoreBaseTest,
|
|||
with mock.patch.object(MockRBD.Image, 'list_children') as mocked:
|
||||
mocked.return_value = True
|
||||
|
||||
self.assertRaises(exceptions.InUseByStore,
|
||||
self.store._delete_image,
|
||||
'fake_pool', self.location.image,
|
||||
snapshot_name='snap')
|
||||
self.store._delete_image('fake_pool',
|
||||
self.location.image,
|
||||
snapshot_name='snap')
|
||||
|
||||
def test_delete_image_w_snap_exc_image_has_snap(self):
|
||||
def _fake_remove(*args, **kwargs):
|
||||
|
@ -439,8 +441,8 @@ class TestMultiStore(base.MultiStoreBaseTest,
|
|||
|
||||
with mock.patch.object(MockRBD.RBD, 'remove') as remove:
|
||||
remove.side_effect = _fake_remove
|
||||
self.assertRaises(exceptions.HasSnapshot, self.store._delete_image,
|
||||
'fake_pool', self.location.image)
|
||||
self.store._delete_image('fake_pool',
|
||||
self.location.image)
|
||||
|
||||
self.called_commands_expected = ['remove']
|
||||
|
||||
|
|
|
@ -173,6 +173,9 @@ class MockRBD(object):
|
|||
def clone(self, *args, **kwargs):
|
||||
raise NotImplementedError()
|
||||
|
||||
def trash_move(self, *args, **kwargs):
|
||||
pass
|
||||
|
||||
RBD_FEATURE_LAYERING = 1
|
||||
|
||||
|
||||
|
@ -633,23 +636,45 @@ class TestStore(base.StoreBaseTest,
|
|||
with mock.patch.object(MockRBD.Image, 'list_children') as mocked:
|
||||
mocked.return_value = True
|
||||
|
||||
self.assertRaises(exceptions.InUseByStore,
|
||||
self.store._delete_image,
|
||||
'fake_pool', self.location.image,
|
||||
snapshot_name='snap')
|
||||
self.store._delete_image('fake_pool',
|
||||
self.location.image,
|
||||
snapshot_name='snap')
|
||||
|
||||
def test_delete_image_w_snap_exc_image_has_snap(self):
|
||||
def _fake_remove(*args, **kwargs):
|
||||
self.called_commands_actual.append('remove')
|
||||
raise MockRBD.ImageHasSnapshots()
|
||||
|
||||
mock.patch.object(MockRBD.RBD, 'trash_move').start()
|
||||
|
||||
with mock.patch.object(MockRBD.RBD, 'remove') as remove:
|
||||
remove.side_effect = _fake_remove
|
||||
self.assertRaises(exceptions.HasSnapshot, self.store._delete_image,
|
||||
'fake_pool', self.location.image)
|
||||
self.store._delete_image('fake_pool',
|
||||
self.location.image)
|
||||
|
||||
self.called_commands_expected = ['remove']
|
||||
|
||||
MockRBD.RBD.trash_move.assert_called_once_with(mock.ANY, 'fake_image')
|
||||
|
||||
def test_delete_image_w_snap_exc_image_has_snap_2(self):
|
||||
def _fake_remove(*args, **kwargs):
|
||||
self.called_commands_actual.append('remove')
|
||||
raise MockRBD.ImageHasSnapshots()
|
||||
|
||||
mock.patch.object(MockRBD.RBD, 'trash_move',
|
||||
side_effect=MockRBD.ImageBusy).start()
|
||||
|
||||
with mock.patch.object(MockRBD.RBD, 'remove') as remove:
|
||||
remove.side_effect = _fake_remove
|
||||
self.assertRaises(exceptions.InUseByStore,
|
||||
self.store._delete_image,
|
||||
'fake_pool',
|
||||
self.location.image)
|
||||
|
||||
self.called_commands_expected = ['remove']
|
||||
|
||||
MockRBD.RBD.trash_move.assert_called_once_with(mock.ANY, 'fake_image')
|
||||
|
||||
def test_get_partial_image(self):
|
||||
loc = g_location.Location('test_rbd_store', rbd_store.StoreLocation,
|
||||
self.conf, store_specs=self.store_specs)
|
||||
|
|
|
@ -0,0 +1,10 @@
|
|||
---
|
||||
features:
|
||||
- |
|
||||
The RBD driver now moves images to the trash if they cannot be deleted
|
||||
immediately due to having snapshots. This fixes the long-standing issue
|
||||
where base images are unable to be deleted until/unless all snapshots of
|
||||
it are also deleted. Moving the image to the trash allows Glance to
|
||||
proceed with the deletion of the image (as far as it is concerned), mark
|
||||
the RBD image for deletion, which will happen once the last snapshot that
|
||||
uses it has been deleted.
|
Loading…
Reference in New Issue