Don't fail when trying to unprotect unprotected snapshot on RBD

If rbd driver tries to unprotect snapshot which is already
unprotected, rbd lib raise InvalidArgument exception.
Rbd driver in glance_store will now handle such exception
and will continue deleting snapshot and image in such case.

Change-Id: I612b4421d5065dde002e3c0e0cb22537d2467adb
Closes-Bug: #1686063

2 files changed, 65 insertions, 1 deletions

diff --git a/glance_store/_drivers/rbd.py b/glance_store/_drivers/rbd.py
index d384705..d78c662 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 b4294aa..4d1f077 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')