glusterfs: handle new cli XML format

Change http://review.gluster.org/14931,
debuting in GlusterFS 3.7.14 has changed the
XML output emitted by the gluster command
line interface. Here we implement parsing
for the new variant as well.

Change-Id: Ia9f340f1d56c95d5ebf5577df6aae9d708a026c0
Closes-Bug: 1609858

manila/share/drivers/glusterfs/common.py

@@ -61,14 +61,18 @@ def _check_volume_presence(f):
     return wrapper
 
 
-def volxml_get(xmlout, path, *default):
+def volxml_get(xmlout, *paths, **kwargs):
-    """Extract a value by a path from XML."""
+    """Attempt to extract a value by a set of Xpaths from XML."""
-    value = xmlout.find(path)
+    for path in paths:
+        value = xmlout.find(path)
+        if value is not None:
+            break
     if value is None:
-        if default:
+        if 'default' in kwargs:
-            return default[0]
+            return kwargs['default']
         raise exception.InvalidShare(
-            _('Xpath %s not found in volume query response XML') % path)
+            _("Volume query response XML has no value for any of "
+              "the following Xpaths: %s") % ", ".join(paths))
     return value.text
 
 
@@ -225,7 +229,7 @@ class GlusterManager(object):
             ) % {'volume': self.volume, 'command': command})
         if list(six.itervalues(ret)) != [0, 0]:
             errdct = {'volume': self.volume, 'command': commandstr,
-                      'opErrstr': volxml_get(xmlout, 'opErrstr', None)}
+                      'opErrstr': volxml_get(xmlout, 'opErrstr', default=None)}
             errdct.update(ret)
             raise exception.InvalidShare(_(
                 'GlusterFS command %(command)s on volume %(volume)s got '
@@ -289,7 +293,10 @@ class GlusterManager(object):
             return self._get_vol_option_via_info(option)
 
         self.xml_response_check(optxml, args[1:], './volGetopts/count')
-        return volxml_get(optxml, './volGetopts/Value')
+        # the Xpath has changed from first to second as of GlusterFS
+        # 3.7.14 (see http://review.gluster.org/14931).
+        return volxml_get(optxml, './volGetopts/Value',
+                          './volGetopts/Opt/Value')
 
     def get_vol_option(self, option, boolean=False):
         """Get the value of an option set on a GlusterFS volume."""

manila/share/drivers/glusterfs/layout_volume.py

@@ -542,7 +542,7 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase):
             outxml = etree.fromstring(out)
             opret = int(common.volxml_get(outxml, 'opRet'))
             operrno = int(common.volxml_get(outxml, 'opErrno'))
-            operrstr = common.volxml_get(outxml, 'opErrstr', None)
+            operrstr = common.volxml_get(outxml, 'opErrstr', default=None)
 
         if opret == -1:
             vers = self.glusterfs_versions[gluster_mgr.host_access]

manila/tests/share/drivers/glusterfs/test_common.py

@@ -98,10 +98,20 @@ class GlusterManagerTestCase(test.TestCase):
         xmlout = mock.Mock()
         xmlout.find = mock.Mock(return_value=None)
 
-        ret = common.volxml_get(xmlout, 'some/path', default)
+        ret = common.volxml_get(xmlout, 'some/path', default=default)
 
         self.assertEqual(default, ret)
 
+    def test_volxml_get_multiple(self):
+        xmlout = mock.Mock()
+        value = mock.Mock()
+        value.text = 'foobar'
+        xmlout.find = mock.Mock(side_effect=(None, value))
+
+        ret = common.volxml_get(xmlout, 'some/path', 'better/path')
+
+        self.assertEqual('foobar', ret)
+
     def test_volxml_get_notfound(self):
         xmlout = mock.Mock()
         xmlout.find = mock.Mock(return_value=None)
@@ -385,11 +395,11 @@ class GlusterManagerTestCase(test.TestCase):
               {'opRet': '0', 'opErrno': '0', 'some/count': '2'})
     def test_xml_response_check_invalid(self, fdict):
 
-        def vxget(x, e, *a):
+        def vxget(x, *e, **kw):
-            if a:
+            if kw:
-                return fdict.get(e, a[0])
+                return fdict.get(e[0], kw['default'])
             else:
-                return fdict[e]
+                return fdict[e[0]]
 
         xtree = mock.Mock()
         command = ['volume', 'command', 'fake']
@@ -557,7 +567,8 @@ class GlusterManagerTestCase(test.TestCase):
         self._gluster_manager.gluster_call.assert_called_once_with(
             *args, check_exit_code=False)
 
-    def test_get_vol_regular_option(self):
+    @ddt.data({'start': "", 'end': ""}, {'start': "<Opt>", 'end': "</Opt>"})
+    def test_get_vol_regular_option(self, extratag):
 
         def xml_output(*ignore_args, **ignore_kwargs):
             return """<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
@@ -567,10 +578,12 @@ class GlusterManagerTestCase(test.TestCase):
   <opErrstr/>
   <volGetopts>
     <count>1</count>
+    %(start)s
     <Option>nfs.export-dir</Option>
     <Value>/foo(10.0.0.1|10.0.0.2),/bar(10.0.0.1)</Value>
+    %(end)s
   </volGetopts>
-</cliOutput>""", ''
+</cliOutput>""" % extratag, ''
 
         args = ('--xml', 'volume', 'get', self._gluster_manager.volume,
                 NFS_EXPORT_DIR)

releasenotes/notes/glusterfs-handle-new-volume-option-xml-schema-dad06253453c572c.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - GlusterFS drivers now  handle the volume
+    option XML schema of GlusterFS >= 3.7.14.