Removes unecessary call to rbd.Image

Removed duplicate and unprotected call to rbd.Image
in rbd driver which can result in an uncaught exception
if the rbd image no longer exists.

Also added unit test for _delete_backup_snaps()

Closes-Bug: 1323203
Change-Id: I89eedb0d0276a985b4d496093df2eee2ce067ec1

diff --git a/cinder/tests/test_rbd.py b/cinder/tests/test_rbd.py
index c2c572c3b5be9346b4d51218afc42b32df31b959..43b43c224cb07457ddfc703a8f3c6d9de0426f2c 100644 (file)
--- a/cinder/tests/test_rbd.py
+++ b/cinder/tests/test_rbd.py
@@ -201,6 +201,17 @@ class RBDTestCase(test.TestCase):
             client.__exit__.assert_called_once()
             mock_supports_layering.assert_called_once()
 
+    @common_mocks
+    def test_delete_backup_snaps(self):
+        self.driver.rbd.Image.remove_snap = mock.Mock()
+        with mock.patch.object(self.driver, '_get_backup_snaps') as \
+                mock_get_backup_snaps:
+            mock_get_backup_snaps.return_value = [{'name': 'snap1'}]
+            rbd_image = self.driver.rbd.Image()
+            self.driver._delete_backup_snaps(rbd_image)
+            mock_get_backup_snaps.assert_called_once_with(rbd_image)
+            self.assertTrue(self.driver.rbd.Image.remove_snap.called)
+
     @common_mocks
     def test_delete_volume(self):
         client = self.mock_client.return_value

diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py
index ee7ddce95119855a993271be1a3ef45157726a02..11bf608873dd1afa8c3e9859ebc9023a978c7050 100644 (file)
--- a/cinder/volume/drivers/rbd.py
+++ b/cinder/volume/drivers/rbd.py
@@ -518,17 +518,13 @@ class RBDDriver(driver.VolumeDriver):
         if int(volume['size']):
             self._resize(volume)
 
-    def _delete_backup_snaps(self, client, volume_name):
-        rbd_image = self.rbd.Image(client.ioctx, volume_name)
-        try:
-            backup_snaps = self._get_backup_snaps(rbd_image)
-            if backup_snaps:
-                for snap in backup_snaps:
-                    rbd_image.remove_snap(snap['name'])
-            else:
-                LOG.debug(_("volume has no backup snaps"))
-        finally:
-            rbd_image.close()
+    def _delete_backup_snaps(self, rbd_image):
+        backup_snaps = self._get_backup_snaps(rbd_image)
+        if backup_snaps:
+            for snap in backup_snaps:
+                rbd_image.remove_snap(snap['name'])
+        else:
+            LOG.debug(_("volume has no backup snaps"))
 
     def _get_clone_info(self, volume, volume_name, snap=None):
         """If volume is a clone, return its parent info.
@@ -602,7 +598,7 @@ class RBDDriver(driver.VolumeDriver):
             parent = None
 
             # Ensure any backup snapshots are deleted
-            self._delete_backup_snaps(client, volume_name)
+            self._delete_backup_snaps(rbd_image)
 
             # If the volume has non-clone snapshots this delete is expected to
             # raise VolumeIsBusy so do so straight away.