diff --git a/glance_store/_drivers/rbd.py b/glance_store/_drivers/rbd.py index d3847059..d78c6623 100644 --- a/glance_store/_drivers/rbd.py +++ b/glance_store/_drivers/rbd.py @@ -388,7 +388,7 @@ class Store(driver.Store): if snapshot_name is not None: with rbd.Image(ioctx, image_name) as image: try: - image.unprotect_snap(snapshot_name) + self._unprotect_snapshot(image, snapshot_name) image.remove_snap(snapshot_name) except rbd.ImageNotFound as exc: msg = (_("Snap Operating Exception " @@ -422,6 +422,15 @@ class Store(driver.Store): msg = _("RBD image %s does not exist") % image_name raise exceptions.NotFound(message=msg) + def _unprotect_snapshot(self, image, snap_name): + try: + image.unprotect_snap(snap_name) + except rbd.InvalidArgument: + # NOTE(slaweq): if snapshot was unprotected already, rbd library + # raises InvalidArgument exception without any "clear" message. + # Such exception is not dangerous for us so it will be just logged + LOG.debug("Snapshot %s is unprotected already" % snap_name) + @capabilities.check def add(self, image_id, image_file, image_size, context=None, verifier=None): diff --git a/glance_store/tests/unit/test_rbd_store.py b/glance_store/tests/unit/test_rbd_store.py index b4294aae..4d1f0779 100644 --- a/glance_store/tests/unit/test_rbd_store.py +++ b/glance_store/tests/unit/test_rbd_store.py @@ -24,6 +24,10 @@ from glance_store.tests import base from glance_store.tests.unit import test_store_capabilities +class TestException(Exception): + pass + + class MockRados(object): class Error(Exception): @@ -80,6 +84,9 @@ class MockRBD(object): class ImageNotFound(Exception): pass + class InvalidArgument(Exception): + pass + class Image(object): def __init__(self, *args, **kwargs): @@ -298,6 +305,54 @@ class TestStore(base.StoreBaseTest, self.called_commands_expected = ['unprotect_snap', 'remove_snap', 'remove'] + @mock.patch.object(MockRBD.RBD, 'remove') + @mock.patch.object(MockRBD.Image, 'remove_snap') + @mock.patch.object(MockRBD.Image, 'unprotect_snap') + def test_delete_image_w_unprotected_snap(self, unprotect, remove_snap, + remove): + def _fake_unprotect_snap(*args, **kwargs): + self.called_commands_actual.append('unprotect_snap') + raise MockRBD.InvalidArgument() + + def _fake_remove_snap(*args, **kwargs): + self.called_commands_actual.append('remove_snap') + + def _fake_remove(*args, **kwargs): + self.called_commands_actual.append('remove') + + remove.side_effect = _fake_remove + unprotect.side_effect = _fake_unprotect_snap + remove_snap.side_effect = _fake_remove_snap + self.store._delete_image('fake_pool', self.location.image, + snapshot_name='snap') + + self.called_commands_expected = ['unprotect_snap', 'remove_snap', + 'remove'] + + @mock.patch.object(MockRBD.RBD, 'remove') + @mock.patch.object(MockRBD.Image, 'remove_snap') + @mock.patch.object(MockRBD.Image, 'unprotect_snap') + def test_delete_image_w_snap_with_error(self, unprotect, remove_snap, + remove): + def _fake_unprotect_snap(*args, **kwargs): + self.called_commands_actual.append('unprotect_snap') + raise TestException() + + def _fake_remove_snap(*args, **kwargs): + self.called_commands_actual.append('remove_snap') + + def _fake_remove(*args, **kwargs): + self.called_commands_actual.append('remove') + + remove.side_effect = _fake_remove + unprotect.side_effect = _fake_unprotect_snap + remove_snap.side_effect = _fake_remove_snap + self.assertRaises(TestException, self.store._delete_image, + 'fake_pool', self.location.image, + snapshot_name='snap') + + self.called_commands_expected = ['unprotect_snap'] + def test_delete_image_w_snap_exc_image_busy(self): def _fake_unprotect_snap(*args, **kwargs): self.called_commands_actual.append('unprotect_snap')