GlusterFS: Complete snapshot_delete when info doesn't exist

The snapshot_delete operation will fail if the snapshot info file
doesn't contain a record for the snapshot, or does not exist.
This happens in cases such as when snapshot_create fails to commit
anything to disk.

The driver should allow the manager to delete the snapshot
in this case, as there is no action required for the driver
to delete anything.

Closes-Bug: #1252864

Change-Id: I8686a1be09dbb7984072538bff6c026bb84eeb52

diff --git a/cinder/tests/test_glusterfs.py b/cinder/tests/test_glusterfs.py
index d2ac1fab982c91648ec33c990d05d456baba62f4..ca27ea289b02b98e4ff550822a5bb0c7d6b0a543 100644 (file)
--- a/cinder/tests/test_glusterfs.py
+++ b/cinder/tests/test_glusterfs.py
@@ -877,7 +877,8 @@ class GlusterFsDriverTestCase(test.TestCase):
         snap_path_chain = [{self.SNAP_UUID: snap_file},
                            {'active': snap_file}]
 
-        drv._read_info_file(info_path).AndReturn(info_file_dict)
+        drv._read_info_file(info_path, empty_if_missing=True).\
+            AndReturn(info_file_dict)
 
         drv._execute('qemu-img', 'commit', snap_path_2, run_as_root=True)
 
@@ -966,7 +967,8 @@ class GlusterFsDriverTestCase(test.TestCase):
         drv._ensure_share_writable(volume_dir)
 
         info_path = drv._local_path_volume(volume) + '.info'
-        drv._read_info_file(info_path).AndReturn(info_file_dict)
+        drv._read_info_file(info_path, empty_if_missing=True).\
+            AndReturn(info_file_dict)
 
         img_info = imageutils.QemuImgInfo(qemu_img_info_output_snap_1)
         image_utils.qemu_img_info(snap_path).AndReturn(img_info)
@@ -1001,6 +1003,44 @@ class GlusterFsDriverTestCase(test.TestCase):
 
         mox.VerifyAll()
 
+    def test_delete_snapshot_not_in_info(self):
+        """Snapshot not in info file / info file doesn't exist.
+
+        Snapshot creation failed so nothing is on-disk.  Driver
+        should allow operation to succeed so the manager can
+        remove the snapshot record.
+
+        (Scenario: Snapshot object created in Cinder db but not
+         on backing storage.)
+
+        """
+        (mox, drv) = self._mox, self._driver
+
+        hashed = drv._get_hash_str(self.TEST_EXPORT1)
+        volume_dir = os.path.join(self.TEST_MNT_POINT_BASE, hashed)
+        volume_filename = 'volume-%s' % self.VOLUME_UUID
+        volume_path = os.path.join(volume_dir, volume_filename)
+        info_path = '%s%s' % (volume_path, '.info')
+
+        mox.StubOutWithMock(drv, '_read_file')
+        mox.StubOutWithMock(drv, '_read_info_file')
+        mox.StubOutWithMock(drv, '_ensure_share_writable')
+
+        snap_ref = {'name': 'test snap',
+                    'volume_id': self.VOLUME_UUID,
+                    'volume': self._simple_volume(),
+                    'id': self.SNAP_UUID_2}
+
+        drv._ensure_share_writable(volume_dir)
+
+        drv._read_info_file(info_path, empty_if_missing=True).AndReturn({})
+
+        mox.ReplayAll()
+
+        drv.delete_snapshot(snap_ref)
+
+        mox.VerifyAll()
+
     def test_read_info_file(self):
         (mox, drv) = self._mox, self._driver
 
@@ -1222,7 +1262,8 @@ class GlusterFsDriverTestCase(test.TestCase):
 
         drv._ensure_share_writable(volume_dir)
 
-        drv._read_info_file(info_path).AndReturn(snap_info)
+        drv._read_info_file(info_path, empty_if_missing=True).\
+            AndReturn(snap_info)
 
         os.path.exists(snap_path).AndReturn(True)
 
@@ -1314,7 +1355,8 @@ class GlusterFsDriverTestCase(test.TestCase):
 
         drv._ensure_share_writable(volume_dir)
 
-        drv._read_info_file(info_path).AndReturn(snap_info)
+        drv._read_info_file(info_path, empty_if_missing=True).\
+            AndReturn(snap_info)
 
         os.path.exists(snap_path).AndReturn(True)
 
@@ -1397,7 +1439,8 @@ class GlusterFsDriverTestCase(test.TestCase):
         snap_info = {'active': snap_file,
                      self.SNAP_UUID: snap_file}
 
-        drv._read_info_file(info_path).AndReturn(snap_info)
+        drv._read_info_file(info_path, empty_if_missing=True).\
+            AndReturn(snap_info)
 
         os.path.exists(snap_path).AndReturn(True)
 

diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py
index eac70e106ebb0523e6e6c3546ec6df28775fbe02..a9a86dbc49da340bf3be0525ba3c34e1a61c873b 100644 (file)
--- a/cinder/volume/drivers/glusterfs.py
+++ b/cinder/volume/drivers/glusterfs.py
@@ -547,6 +547,10 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
         If volume status is 'in-use', calculate what qcow2 files need to
         merge, and call to Nova to perform this operation.
 
+        :raises: InvalidVolume if status not acceptable
+        :raises: GlusterfsException(msg) if operation fails
+        :returns: None
+
         """
 
         LOG.debug(_('deleting snapshot %s') % snapshot['id'])
@@ -562,9 +566,18 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
         # Determine the true snapshot file for this snapshot
         #  based on the .info file
         info_path = self._local_path_volume(snapshot['volume']) + '.info'
-        snap_info = self._read_info_file(info_path)
-        snapshot_file = snap_info[snapshot['id']]
+        snap_info = self._read_info_file(info_path, empty_if_missing=True)
+
+        if snapshot['id'] not in snap_info:
+            # If snapshot info file is present, but snapshot record does not
+            # exist, do not attempt to delete.
+            # (This happens, for example, if snapshot_create failed due to lack
+            # of permission to write to the share.)
+            LOG.info(_('Snapshot record for %s is not present, allowing '
+                       'snapshot_delete to proceed.') % snapshot['id'])
+            return
 
+        snapshot_file = snap_info[snapshot['id']]
         LOG.debug(_('snapshot_file for this snap is %s') % snapshot_file)
 
         snapshot_path = '%s/%s' % (self._local_volume_dir(snapshot['volume']),