From 58be1ef71a0197f4c41668a5c00fe18424344478 Mon Sep 17 00:00:00 2001 From: Csaba Henk Date: Mon, 8 Aug 2016 05:10:18 +0530 Subject: [PATCH] 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 | 23 ++++++++++------ .../share/drivers/glusterfs/layout_volume.py | 2 +- .../share/drivers/glusterfs/test_common.py | 27 ++++++++++++++----- ...me-option-xml-schema-dad06253453c572c.yaml | 4 +++ 4 files changed, 40 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/glusterfs-handle-new-volume-option-xml-schema-dad06253453c572c.yaml diff --git a/manila/share/drivers/glusterfs/common.py b/manila/share/drivers/glusterfs/common.py index fc4e1340c7..bb6fdb1e12 100644 --- a/manila/share/drivers/glusterfs/common.py +++ b/manila/share/drivers/glusterfs/common.py @@ -61,14 +61,18 @@ def _check_volume_presence(f): return wrapper -def volxml_get(xmlout, path, *default): - """Extract a value by a path from XML.""" - value = xmlout.find(path) +def volxml_get(xmlout, *paths, **kwargs): + """Attempt to extract a value by a set of Xpaths from XML.""" + for path in paths: + value = xmlout.find(path) + if value is not None: + break if value is None: - if default: - return default[0] + if 'default' in kwargs: + 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.""" diff --git a/manila/share/drivers/glusterfs/layout_volume.py b/manila/share/drivers/glusterfs/layout_volume.py index b56d87668e..1d8aebfa06 100644 --- a/manila/share/drivers/glusterfs/layout_volume.py +++ b/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] diff --git a/manila/tests/share/drivers/glusterfs/test_common.py b/manila/tests/share/drivers/glusterfs/test_common.py index 7a0f65c874..3f97331949 100644 --- a/manila/tests/share/drivers/glusterfs/test_common.py +++ b/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): - if a: - return fdict.get(e, a[0]) + def vxget(x, *e, **kw): + if kw: + 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': "", 'end': ""}) + def test_get_vol_regular_option(self, extratag): def xml_output(*ignore_args, **ignore_kwargs): return """ @@ -567,10 +578,12 @@ class GlusterManagerTestCase(test.TestCase): 1 + %(start)s /foo(10.0.0.1|10.0.0.2),/bar(10.0.0.1) + %(end)s -""", '' +""" % extratag, '' args = ('--xml', 'volume', 'get', self._gluster_manager.volume, NFS_EXPORT_DIR) diff --git a/releasenotes/notes/glusterfs-handle-new-volume-option-xml-schema-dad06253453c572c.yaml b/releasenotes/notes/glusterfs-handle-new-volume-option-xml-schema-dad06253453c572c.yaml new file mode 100644 index 0000000000..6e23a602ff --- /dev/null +++ b/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.