glusterfs_native: delete_snapshot(): find out real GlusterFS snap name

So far the delete_snapshot() logic assumed that a GlusterFS snapshot
exists with exactly the same name as the Manila snapshot id.

This was a fragile assumption -- both GlusterFS and Manila can
potentially change how the GlusterFS snapshots are named.
(And currently we have a problem as GlusterFS has actually changed
this -- see referred bug about details.)

Therefore from now on we'll use the weaker assumption that the
Manila snapshot id is a substring of the backing GlusterFS snapshot
name. (That's a robust assumption, unlikely to break in the future.)
The actual GlusterFS snapshot name is found by doing a
`gluster snapshot list` and grepping for the Manila snapshot id in
it.

Change-Id: I40873208c7431e42885bee4db06d6229a202bad6
Closes-Bug: #1473044

manila/share/drivers/glusterfs_native.py

@@ -656,17 +656,33 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
 
         vol = snapshot['share']['export_location']
         gluster_mgr = self.gluster_used_vols_dict[vol]
-        args = ('--xml', 'snapshot', 'delete', snapshot['id'], '--mode=script')
+        args = ('snapshot', 'list', gluster_mgr.volume, '--mode=script')
         try:
             out, err = gluster_mgr.gluster_call(*args)
         except exception.ProcessExecutionError as exc:
-            LOG.error(_LE("Error retrieving volume info: %s"), exc.stderr)
-            raise exception.GlusterfsException("gluster %s failed" %
+            LOG.error(_LE("Error retrieving snapshot list: %s"), exc.stderr)
+            raise exception.GlusterfsException(_("gluster %s failed") %
+                                               ' '.join(args))
+        snapgrep = filter(lambda x: snapshot['id'] in x, out.split("\n"))
+        if len(snapgrep) != 1:
+            msg = (_("Failed to identify backing GlusterFS object "
+                     "for snapshot %(snap_id)s of share %(share_id)s: "
+                     "a single candidate was expected, %(found)d was found.") %
+                   {'snap_id': snapshot['id'],
+                    'share_id': snapshot['share_id'],
+                    'found': len(snapgrep)})
+            raise exception.GlusterfsException(msg)
+        args = ('--xml', 'snapshot', 'delete', snapgrep[0], '--mode=script')
+        try:
+            out, err = gluster_mgr.gluster_call(*args)
+        except exception.ProcessExecutionError as exc:
+            LOG.error(_LE("Error deleting snapshot: %s"), exc.stderr)
+            raise exception.GlusterfsException(_("gluster %s failed") %
                                                ' '.join(args))
 
         if not out:
             raise exception.GlusterfsException(
-                'gluster volume info %s: no data received' %
+                _('gluster snapshot delete %s: no data received') %
                 gluster_mgr.volume
             )
 

manila/tests/share/drivers/test_glusterfs_native.py

@@ -869,15 +869,18 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
             'share': self.share1
         }
 
-        args = ('--xml', 'snapshot', 'delete', 'fake_snap_id',
-                '--mode=script')
+        args = (('snapshot', 'list', gmgr1.volume, '--mode=script'),
+                ('--xml', 'snapshot', 'delete', 'fake_snap_id_xyz',
+                '--mode=script'))
         self.mock_object(gmgr1, 'gluster_call',
-                         mock.Mock(side_effect=GlusterXMLOut(ret=0, errno=0)))
+                         mock.Mock(side_effect=(('fake_snap_id_xyz', ''),
+                                   GlusterXMLOut(ret=0, errno=0)())))
         ret = self._driver.delete_snapshot(self._context, snapshot)
         self.assertEqual(None, ret)
-        gmgr1.gluster_call.assert_called_once_with(*args)
+        gmgr1.gluster_call.assert_has_calls([mock.call(*a) for a in args])
 
-    def test_delete_snapshot_error(self):
+    @ddt.data(GlusterXMLOut(ret=-1, errno=2)(), ('', ''))
+    def test_delete_snapshot_error(self, badxmlout):
         self._driver.gluster_nosnap_vols_dict = {}
 
         gmgr = glusterfs.GlusterManager
@@ -890,9 +893,34 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
             'share': self.share1
         }
 
-        args = ('--xml', 'snapshot', 'delete', 'fake_snap_id', '--mode=script')
+        args = (('snapshot', 'list', gmgr1.volume, '--mode=script'),
+                ('--xml', 'snapshot', 'delete', 'fake_snap_id_xyz',
+                '--mode=script'))
         self.mock_object(gmgr1, 'gluster_call',
-                         mock.Mock(side_effect=GlusterXMLOut(ret=-1, errno=2)))
+                         mock.Mock(side_effect=(('fake_snap_id_xyz', ''),
+                                   badxmlout)))
+        self.assertRaises(exception.GlusterfsException,
+                          self._driver.delete_snapshot, self._context,
+                          snapshot)
+        gmgr1.gluster_call.assert_has_calls([mock.call(*a) for a in args])
+
+    @ddt.data('this is too bad', 'fake_snap_id_xyx\nfake_snap_id_pqr')
+    def test_delete_snapshot_bad_snap_list(self, snaplist):
+        self._driver.gluster_nosnap_vols_dict = {}
+
+        gmgr = glusterfs.GlusterManager
+        gmgr1 = gmgr(self.share1['export_location'], self._execute, None, None)
+
+        self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
+        snapshot = {
+            'id': 'fake_snap_id',
+            'share_id': self.share1['id'],
+            'share': self.share1
+        }
+
+        args = ('snapshot', 'list', gmgr1.volume, '--mode=script')
+        self.mock_object(gmgr1, 'gluster_call',
+                         mock.Mock(side_effect=((snaplist, ''),)))
         self.assertRaises(exception.GlusterfsException,
                           self._driver.delete_snapshot, self._context,
                           snapshot)