From: Eric Harney <eharney@redhat.com>
Date: Tue, 19 Nov 2013 21:25:47 +0000 (-0500)
Subject: GlusterFS: Complete snapshot_delete when info doesn't exist
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=d8a11168c908fe6c6a07fbb30a5bc88a6df6e939;p=openstack-build%2Fcinder-build.git

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 d2ac1fab9..ca27ea289 100644
--- 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 eac70e106..a9a86dbc4 100644
--- 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']),