From 768b02bdb73b6a7ac99523f997d31b3f442413cb Mon Sep 17 00:00:00 2001
From: Csaba Henk <chenk@redhat.com>
Date: Sun, 7 Feb 2016 04:16:59 +0100
Subject: [PATCH] gluster*: clean up volume option querying
GlusterFS has two kind of options:
- regular ones, which are a hardcoded set, and option names
are verified in "gluster volume get"
- user ones, whose name matches user.* -- these are
arbitrarily named, are ignored by "gluster volume get" and
are listed in "gluster volume info" output
So far we used "gluster volume info" universally, but that,
apart from being cumbersome for regular options, is also
incorrect, as it can't distinguish an unset option name from
an undefined one (querying the former should be treated OK,
querying the second should be treated as error).
- implement querying of regular options with "gluster volume
get" (accepting empty response)
- implement querying of user options with searching "gluster vol
info" data
- verify operations on the XML tree, make tacit XML layout
assumptions explicit
- implement optional Boolean coercion of values
Partially implements bp gluster-code-cleanup
Change-Id: I9e0843b88cd1a1668fe48c6979029c012dcbaa13
---
manila/share/drivers/glusterfs/__init__.py | 6 +-
manila/share/drivers/glusterfs/common.py | 115 ++++++-
.../share/drivers/glusterfs/layout_volume.py | 19 +-
manila/share/drivers/glusterfs_native.py | 5 +-
.../share/drivers/glusterfs/test_common.py | 288 +++++++++++++++++-
.../drivers/glusterfs/test_layout_volume.py | 2 +-
manila/tests/share/drivers/test_glusterfs.py | 12 +-
.../share/drivers/test_glusterfs_native.py | 22 +-
8 files changed, 405 insertions(+), 64 deletions(-)
diff --git a/manila/share/drivers/glusterfs/__init__.py b/manila/share/drivers/glusterfs/__init__.py
index 36592e0916..1463e18d7d 100644
--- a/manila/share/drivers/glusterfs/__init__.py
+++ b/manila/share/drivers/glusterfs/__init__.py
@@ -96,10 +96,10 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.GaneshaMixin,
gluster_manager = share_manager['manager']
# TODO(csaba): This should be refactored into proper dispatch to helper
if self.nfs_helper == GlusterNFSHelper and not gluster_manager.path:
- # default is 'on'
+ # default behavior of NFS_EXPORT_VOL is as if it were 'on'
export_vol = gluster_manager.get_vol_option(
- NFS_EXPORT_VOL) or 'on'
- if export_vol.lower() not in ('on', '1', 'true', 'yes', 'enable'):
+ NFS_EXPORT_VOL, boolean=True)
+ if export_vol is False:
raise exception.GlusterfsException(
_("Gluster-NFS with volume layout should be used "
"with `nfs.export-volumes = on`"))
diff --git a/manila/share/drivers/glusterfs/common.py b/manila/share/drivers/glusterfs/common.py
index ff4803f837..8176030509 100644
--- a/manila/share/drivers/glusterfs/common.py
+++ b/manila/share/drivers/glusterfs/common.py
@@ -61,6 +61,17 @@ 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)
+ if value is None:
+ if default:
+ return default[0]
+ raise exception.InvalidShare(
+ _('Xpath %s not found in volume query response XML') % path)
+ return value.text
+
+
class GlusterManager(object):
"""Interface with a GlusterFS volume."""
@@ -68,6 +79,13 @@ class GlusterManager(object):
'(?P<host>[^:@/]+)'
'(?::/(?P<volume>[^/]+)(?P<path>/.*)?)?\Z')
+ # See this about GlusterFS' convention for Boolean interpretation
+ # of strings:
+ # https://github.com/gluster/glusterfs/blob/v3.7.8/
+ # libglusterfs/src/common-utils.c#L1680-L1708
+ GLUSTERFS_TRUE_VALUES = ('ON', 'YES', 'TRUE', 'ENABLE', '1')
+ GLUSTERFS_FALSE_VALUES = ('OFF', 'NO', 'FALSE', 'DISABLE', '0')
+
@classmethod
def parse(cls, address):
"""Parse address string into component dict."""
@@ -174,9 +192,38 @@ class GlusterManager(object):
return _gluster_call
- @_check_volume_presence
- def get_vol_option(self, option):
- """Get the value of an option set on a GlusterFS volume."""
+ def xml_response_check(self, xmlout, command, countpath=None):
+ """Sanity check for GlusterFS XML response."""
+ commandstr = ' '.join(command)
+ ret = {}
+ for e in 'opRet', 'opErrno':
+ ret[e] = int(volxml_get(xmlout, e))
+ if ret == {'opRet': -1, 'opErrno': 0}:
+ raise exception.GlusterfsException(_(
+ 'GlusterFS command %(command)s on volume %(volume)s failed'
+ ) % {'volume': self.volume, 'command': command})
+ if list(six.itervalues(ret)) != [0, 0]:
+ errdct = {'volume': self.volume, 'command': commandstr,
+ 'opErrstr': volxml_get(xmlout, 'opErrstr', None)}
+ errdct.update(ret)
+ raise exception.InvalidShare(_(
+ 'GlusterFS command %(command)s on volume %(volume)s got '
+ 'unexpected response: '
+ 'opRet=%(opRet)s, opErrno=%(opErrno)s, opErrstr=%(opErrstr)s'
+ ) % errdct)
+ if not countpath:
+ return
+ count = volxml_get(xmlout, countpath)
+ if count != '1':
+ raise exception.InvalidShare(
+ _('GlusterFS command %(command)s on volume %(volume)s got '
+ 'ambiguous response: '
+ '%(count)s records') % {
+ 'volume': self.volume, 'command': commandstr,
+ 'count': count})
+
+ def _get_vol_option_via_info(self, option):
+ """Get the value of an option set on a GlusterFS volume via volinfo."""
args = ('--xml', 'volume', 'info', self.volume)
out, err = self.gluster_call(*args, log=_LE("retrieving volume info"))
@@ -186,19 +233,65 @@ class GlusterManager(object):
self.volume
)
- vix = etree.fromstring(out)
- if int(vix.find('./volInfo/volumes/count').text) != 1:
- raise exception.InvalidShare('Volume name ambiguity')
- for e in vix.findall(".//option"):
- o, v = (e.find(a).text for a in ('name', 'value'))
+ volxml = etree.fromstring(out)
+ self.xml_response_check(volxml, args[1:], './volInfo/volumes/count')
+ for e in volxml.findall(".//option"):
+ o, v = (volxml_get(e, a) for a in ('name', 'value'))
if o == option:
return v
+ @_check_volume_presence
+ def _get_vol_user_option(self, useropt):
+ """Get the value of an user option set on a GlusterFS volume."""
+ option = '.'.join(('user', useropt))
+ return self._get_vol_option_via_info(option)
+
+ @_check_volume_presence
+ def _get_vol_regular_option(self, option):
+ """Get the value of a regular option set on a GlusterFS volume."""
+ args = ('--xml', 'volume', 'get', self.volume, option)
+
+ out, err = self.gluster_call(*args, check_exit_code=False)
+
+ if not out:
+ # all input is valid, but the option has not been set
+ # (nb. some options do come by a null value, but some
+ # don't even have that, see eg. cluster.nufa)
+ return
+
+ try:
+ optxml = etree.fromstring(out)
+ except Exception:
+ # non-xml output indicates that GlusterFS backend does not support
+ # 'vol get', we fall back to 'vol info' based retrieval (glusterfs
+ # < 3.7).
+ return self._get_vol_option_via_info(option)
+
+ self.xml_response_check(optxml, args[1:], './volGetopts/count')
+ return volxml_get(optxml, './volGetopts/Value')
+
+ def get_vol_option(self, option, boolean=False):
+ """Get the value of an option set on a GlusterFS volume."""
+ useropt = re.sub('\Auser\.', '', option)
+ if option == useropt:
+ value = self._get_vol_regular_option(option)
+ else:
+ value = self._get_vol_user_option(useropt)
+ if not boolean or value is None:
+ return value
+ if value.upper() in self.GLUSTERFS_TRUE_VALUES:
+ return True
+ if value.upper() in self.GLUSTERFS_FALSE_VALUES:
+ return False
+ raise exception.GlusterfsException(_(
+ "GlusterFS volume option on volume %(volume)s: "
+ "%(option)s=%(value)s cannot be interpreted as Boolean") % {
+ 'volume': self.volume, 'option': option, 'value': value})
+
@_check_volume_presence
def set_vol_option(self, option, value, ignore_failure=False):
- if value is True:
- value
- value = {True: 'on', False: 'off'}.get(value, value)
+ value = {True: self.GLUSTERFS_TRUE_VALUES[0],
+ False: self.GLUSTERFS_FALSE_VALUES[0]}.get(value, value)
if value is None:
args = ('reset', (option,))
else:
diff --git a/manila/share/drivers/glusterfs/layout_volume.py b/manila/share/drivers/glusterfs/layout_volume.py
index d31d2f7474..622b4ef087 100644
--- a/manila/share/drivers/glusterfs/layout_volume.py
+++ b/manila/share/drivers/glusterfs/layout_volume.py
@@ -531,9 +531,9 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase):
)
outxml = etree.fromstring(out)
- opret = int(outxml.find('opRet').text)
- operrno = int(outxml.find('opErrno').text)
- operrstr = outxml.find('opErrstr').text
+ opret = int(common.volxml_get(outxml, 'opRet'))
+ operrno = int(common.volxml_get(outxml, 'opErrno'))
+ operrstr = common.volxml_get(outxml, 'opErrstr', None)
if opret == -1:
vers = self.glusterfs_versions[gluster_mgr.host_access]
@@ -573,18 +573,7 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase):
)
outxml = etree.fromstring(out)
- opret = int(outxml.find('opRet').text)
- operrno = int(outxml.find('opErrno').text)
- operrstr = outxml.find('opErrstr').text
-
- if opret:
- raise exception.GlusterfsException(
- _("Deleting snapshot %(snap_id)s of share %(share_id)s failed "
- "with %(errno)d: %(errstr)s") % {
- 'snap_id': snapshot['id'],
- 'share_id': snapshot['share_id'],
- 'errno': operrno,
- 'errstr': operrstr})
+ gluster_mgr.xml_response_check(outxml, args[1:])
def ensure_share(self, context, share, share_server=None):
"""Invoked to ensure that share is exported."""
diff --git a/manila/share/drivers/glusterfs_native.py b/manila/share/drivers/glusterfs_native.py
index 8778e7f57f..239a661bfd 100644
--- a/manila/share/drivers/glusterfs_native.py
+++ b/manila/share/drivers/glusterfs_native.py
@@ -203,9 +203,8 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin,
ssl_allow_opt = ','.join(ssl_allow)
gluster_mgr.set_vol_option(AUTH_SSL_ALLOW, ssl_allow_opt)
- dynauth = gluster_mgr.get_vol_option(DYNAMIC_AUTH) or 'off'
- # TODO(csaba): boolean option processing shoud be done in common
- if dynauth.lower() not in ('on', '1', 'true', 'yes', 'enable'):
+ dynauth = gluster_mgr.get_vol_option(DYNAMIC_AUTH, boolean=True)
+ if not dynauth:
common._restart_gluster_vol(gluster_mgr)
def _update_share_stats(self):
diff --git a/manila/tests/share/drivers/glusterfs/test_common.py b/manila/tests/share/drivers/glusterfs/test_common.py
index bc4a7d6a32..eb7775bfee 100644
--- a/manila/tests/share/drivers/glusterfs/test_common.py
+++ b/manila/tests/share/drivers/glusterfs/test_common.py
@@ -73,6 +73,32 @@ class GlusterManagerTestCase(test.TestCase):
exception.GlusterfsException,
common._check_volume_presence(mock.Mock()), gmgr)
+ def test_volxml_get(self):
+ xmlout = mock.Mock()
+ value = mock.Mock()
+ value.text = 'foobar'
+ xmlout.find = mock.Mock(return_value=value)
+
+ ret = common.volxml_get(xmlout, 'some/path')
+
+ self.assertEqual('foobar', ret)
+
+ @ddt.data(None, 'some-value')
+ def test_volxml_get_notfound_fallback(self, default):
+ xmlout = mock.Mock()
+ xmlout.find = mock.Mock(return_value=None)
+
+ ret = common.volxml_get(xmlout, 'some/path', default)
+
+ self.assertEqual(default, ret)
+
+ def test_volxml_get_notfound(self):
+ xmlout = mock.Mock()
+ xmlout.find = mock.Mock(return_value=None)
+
+ self.assertRaises(exception.InvalidShare, common.volxml_get,
+ xmlout, 'some/path')
+
def test_gluster_manager_init(self):
self.assertEqual(fake_gluster_manager_attrs['user'],
self._gluster_manager.user)
@@ -222,21 +248,104 @@ class GlusterManagerTestCase(test.TestCase):
gluster_manager.make_gluster_call(fake_execute),
*fake_args, error_policy='foobar')
- def test_get_vol_option_empty_volinfo(self):
+ @ddt.data({}, {'opErrstr': None}, {'opErrstr': 'error'})
+ def test_xml_response_check(self, xdict):
+ fdict = {'opRet': '0', 'opErrno': '0', 'some/count': '1'}
+ fdict.update(xdict)
+
+ def vxget(x, e, *a):
+ if a:
+ return fdict.get(e, a[0])
+ else:
+ return fdict[e]
+
+ xtree = mock.Mock()
+ command = ['volume', 'command', 'fake']
+
+ with mock.patch.object(common, 'volxml_get', side_effect=vxget):
+ self._gluster_manager.xml_response_check(xtree, command,
+ 'some/count')
+
+ self.assertTrue(common.volxml_get.called)
+
+ @ddt.data('1', '2')
+ def test_xml_response_check_failure(self, count):
+ fdict = {'opRet': '-1', 'opErrno': '0', 'some/count': count}
+
+ def vxget(x, e, *a):
+ if a:
+ return fdict.get(e, a[0])
+ else:
+ return fdict[e]
+
+ xtree = mock.Mock()
+ command = ['volume', 'command', 'fake']
+
+ with mock.patch.object(common, 'volxml_get', side_effect=vxget):
+ self.assertRaises(exception.GlusterfsException,
+ self._gluster_manager.xml_response_check,
+ xtree, command, 'some/count')
+
+ self.assertTrue(common.volxml_get.called)
+
+ @ddt.data({'opRet': '-2', 'opErrno': '0', 'some/count': '1'},
+ {'opRet': '0', 'opErrno': '1', 'some/count': '1'},
+ {'opRet': '0', 'opErrno': '0', 'some/count': '0'},
+ {'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])
+ else:
+ return fdict[e]
+
+ xtree = mock.Mock()
+ command = ['volume', 'command', 'fake']
+
+ with mock.patch.object(common, 'volxml_get', side_effect=vxget):
+ self.assertRaises(exception.InvalidShare,
+ self._gluster_manager.xml_response_check,
+ xtree, command, 'some/count')
+
+ self.assertTrue(common.volxml_get.called)
+
+ @ddt.data({'opRet': '0', 'opErrno': '0'},
+ {'opRet': '0', 'opErrno': '0', 'some/count': '2'})
+ def test_xml_response_check_count_ignored(self, fdict):
+
+ def vxget(x, e, *a):
+ if a:
+ return fdict.get(e, a[0])
+ else:
+ return fdict[e]
+
+ xtree = mock.Mock()
+ command = ['volume', 'command', 'fake']
+
+ with mock.patch.object(common, 'volxml_get', side_effect=vxget):
+ self._gluster_manager.xml_response_check(xtree, command)
+
+ self.assertTrue(common.volxml_get.called)
+
+ def test_get_vol_option_via_info_empty_volinfo(self):
args = ('--xml', 'volume', 'info', self._gluster_manager.volume)
self.mock_object(self._gluster_manager, 'gluster_call',
mock.Mock(return_value=('', {})))
self.assertRaises(exception.GlusterfsException,
- self._gluster_manager.get_vol_option,
- NFS_EXPORT_DIR)
+ self._gluster_manager._get_vol_option_via_info,
+ 'foobar')
self._gluster_manager.gluster_call.assert_called_once_with(
*args, log=mock.ANY)
- def test_get_vol_option_ambiguous_volinfo(self):
+ def test_get_vol_option_via_info_ambiguous_volinfo(self):
def xml_output(*ignore_args, **ignore_kwargs):
return """<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<cliOutput>
+ <opRet>0</opRet>
+ <opErrno>0</opErrno>
+ <opErrstr/>
<volInfo>
<volumes>
<count>0</count>
@@ -248,16 +357,19 @@ class GlusterManagerTestCase(test.TestCase):
self.mock_object(self._gluster_manager, 'gluster_call',
mock.Mock(side_effect=xml_output))
self.assertRaises(exception.InvalidShare,
- self._gluster_manager.get_vol_option,
- NFS_EXPORT_DIR)
+ self._gluster_manager._get_vol_option_via_info,
+ 'foobar')
self._gluster_manager.gluster_call.assert_called_once_with(
*args, log=mock.ANY)
- def test_get_vol_option_trivial_volinfo(self):
+ def test_get_vol_option_via_info_trivial_volinfo(self):
def xml_output(*ignore_args, **ignore_kwargs):
return """<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<cliOutput>
+ <opRet>0</opRet>
+ <opErrno>0</opErrno>
+ <opErrstr/>
<volInfo>
<volumes>
<volume>
@@ -270,23 +382,26 @@ class GlusterManagerTestCase(test.TestCase):
args = ('--xml', 'volume', 'info', self._gluster_manager.volume)
self.mock_object(self._gluster_manager, 'gluster_call',
mock.Mock(side_effect=xml_output))
- ret = self._gluster_manager.get_vol_option(NFS_EXPORT_DIR)
+ ret = self._gluster_manager._get_vol_option_via_info('foobar')
self.assertIsNone(ret)
self._gluster_manager.gluster_call.assert_called_once_with(
*args, log=mock.ANY)
- def test_get_vol_option(self):
+ def test_get_vol_option_via_info(self):
def xml_output(*ignore_args, **ignore_kwargs):
return """<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<cliOutput>
+ <opRet>0</opRet>
+ <opErrno>0</opErrno>
+ <opErrstr/>
<volInfo>
<volumes>
<volume>
<options>
<option>
- <name>nfs.export-dir</name>
- <value>/foo(10.0.0.1|10.0.0.2),/bar(10.0.0.1)</value>
+ <name>foobar</name>
+ <value>FIRE MONKEY!</value>
</option>
</options>
</volume>
@@ -298,15 +413,158 @@ class GlusterManagerTestCase(test.TestCase):
args = ('--xml', 'volume', 'info', self._gluster_manager.volume)
self.mock_object(self._gluster_manager, 'gluster_call',
mock.Mock(side_effect=xml_output))
- ret = self._gluster_manager.get_vol_option(NFS_EXPORT_DIR)
- self.assertEqual('/foo(10.0.0.1|10.0.0.2),/bar(10.0.0.1)', ret)
+ ret = self._gluster_manager._get_vol_option_via_info('foobar')
+ self.assertEqual('FIRE MONKEY!', ret)
self._gluster_manager.gluster_call.assert_called_once_with(
*args, log=mock.ANY)
+ def test_get_vol_user_option(self):
+ self.mock_object(self._gluster_manager, '_get_vol_option_via_info',
+ mock.Mock(return_value='VALUE'))
+
+ ret = self._gluster_manager._get_vol_user_option('OPT')
+
+ self.assertEqual(ret, 'VALUE')
+ (self._gluster_manager._get_vol_option_via_info.
+ assert_called_once_with('user.OPT'))
+
+ def test_get_vol_regular_option_empty_reponse(self):
+ args = ('--xml', 'volume', 'get', self._gluster_manager.volume,
+ NFS_EXPORT_DIR)
+ self.mock_object(self._gluster_manager, 'gluster_call',
+ mock.Mock(return_value=('', {})))
+
+ ret = self._gluster_manager._get_vol_regular_option(NFS_EXPORT_DIR)
+
+ self.assertIsNone(ret)
+ self._gluster_manager.gluster_call.assert_called_once_with(
+ *args, check_exit_code=False)
+
+ @ddt.data(0, 2)
+ def test_get_vol_regular_option_ambiguous_volinfo(self, count):
+
+ def xml_output(*ignore_args, **ignore_kwargs):
+ return """<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<cliOutput>
+ <opRet>0</opRet>
+ <opErrno>0</opErrno>
+ <opErrstr/>
+ <volGetopts>
+ <count>%d</count>
+ </volGetopts>
+</cliOutput>""" % count, ''
+
+ args = ('--xml', 'volume', 'get', self._gluster_manager.volume,
+ NFS_EXPORT_DIR)
+ self.mock_object(self._gluster_manager, 'gluster_call',
+ mock.Mock(side_effect=xml_output))
+
+ self.assertRaises(exception.InvalidShare,
+ self._gluster_manager._get_vol_regular_option,
+ NFS_EXPORT_DIR)
+
+ self._gluster_manager.gluster_call.assert_called_once_with(
+ *args, check_exit_code=False)
+
+ def test_get_vol_regular_option(self):
+
+ def xml_output(*ignore_args, **ignore_kwargs):
+ return """<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<cliOutput>
+ <opRet>0</opRet>
+ <opErrno>0</opErrno>
+ <opErrstr/>
+ <volGetopts>
+ <count>1</count>
+ <Option>nfs.export-dir</Option>
+ <Value>/foo(10.0.0.1|10.0.0.2),/bar(10.0.0.1)</Value>
+ </volGetopts>
+</cliOutput>""", ''
+
+ args = ('--xml', 'volume', 'get', self._gluster_manager.volume,
+ NFS_EXPORT_DIR)
+ self.mock_object(self._gluster_manager, 'gluster_call',
+ mock.Mock(side_effect=xml_output))
+
+ ret = self._gluster_manager._get_vol_regular_option(NFS_EXPORT_DIR)
+
+ self.assertEqual('/foo(10.0.0.1|10.0.0.2),/bar(10.0.0.1)', ret)
+ self._gluster_manager.gluster_call.assert_called_once_with(
+ *args, check_exit_code=False)
+
+ def test_get_vol_regular_option_not_suppored(self):
+ args = ('--xml', 'volume', 'get', self._gluster_manager.volume,
+ NFS_EXPORT_DIR)
+ self.mock_object(self._gluster_manager, 'gluster_call',
+ mock.Mock(return_value=(
+ """Ceci n'est pas un XML.""", '')))
+ self.mock_object(self._gluster_manager, '_get_vol_option_via_info',
+ mock.Mock(return_value="VALUE"))
+
+ ret = self._gluster_manager._get_vol_regular_option(NFS_EXPORT_DIR)
+
+ self.assertEqual("VALUE", ret)
+ self._gluster_manager.gluster_call.assert_called_once_with(
+ *args, check_exit_code=False)
+ (self._gluster_manager._get_vol_option_via_info.
+ assert_called_once_with(NFS_EXPORT_DIR))
+
+ @ddt.data({'opt': 'some.option', 'opttype': 'regular',
+ 'lowopt': 'some.option'},
+ {'opt': 'user.param', 'opttype': 'user', 'lowopt': 'param'})
+ @ddt.unpack
+ def test_get_vol_option(self, opt, opttype, lowopt):
+ for t in ('user', 'regular'):
+ self.mock_object(self._gluster_manager, '_get_vol_%s_option' % t,
+ mock.Mock(return_value='value-%s' % t))
+
+ ret = self._gluster_manager.get_vol_option(opt)
+
+ self.assertEqual('value-%s' % opttype, ret)
+ for t in ('user', 'regular'):
+ func = getattr(self._gluster_manager, '_get_vol_%s_option' % t)
+ if opttype == t:
+ func.assert_called_once_with(lowopt)
+ else:
+ self.assertFalse(func.called)
+
+ def test_get_vol_option_unset(self):
+ self.mock_object(self._gluster_manager, '_get_vol_regular_option',
+ mock.Mock(return_value=None))
+
+ ret = self._gluster_manager.get_vol_option('some.option')
+
+ self.assertIsNone(ret)
+
+ @ddt.data({'value': '0', 'boolval': False},
+ {'value': 'Off', 'boolval': False},
+ {'value': 'no', 'boolval': False},
+ {'value': '1', 'boolval': True},
+ {'value': 'true', 'boolval': True},
+ {'value': 'enAble', 'boolval': True},
+ {'value': None, 'boolval': None})
+ @ddt.unpack
+ def test_get_vol_option_boolean(self, value, boolval):
+ self.mock_object(self._gluster_manager, '_get_vol_regular_option',
+ mock.Mock(return_value=value))
+
+ ret = self._gluster_manager.get_vol_option('some.option',
+ boolean=True)
+
+ self.assertEqual(boolval, ret)
+
+ def test_get_vol_option_boolean_bad(self):
+ self.mock_object(self._gluster_manager, '_get_vol_regular_option',
+ mock.Mock(return_value='jabberwocky'))
+
+ self.assertRaises(exception.GlusterfsException,
+ self._gluster_manager.get_vol_option,
+ 'some.option', boolean=True)
+
@ddt.data({'setting': 'some_value', 'args': ('set', 'some_value')},
{'setting': None, 'args': ('reset',)},
- {'setting': True, 'args': ('set', 'on')},
- {'setting': False, 'args': ('set', 'off')})
+ {'setting': True, 'args': ('set', 'ON')},
+ {'setting': False, 'args': ('set', 'OFF')})
@ddt.unpack
def test_set_vol_option(self, setting, args):
self.mock_object(self._gluster_manager, 'gluster_call', mock.Mock())
diff --git a/manila/tests/share/drivers/glusterfs/test_layout_volume.py b/manila/tests/share/drivers/glusterfs/test_layout_volume.py
index 5a4d326b6c..8c03b065cf 100644
--- a/manila/tests/share/drivers/glusterfs/test_layout_volume.py
+++ b/manila/tests/share/drivers/glusterfs/test_layout_volume.py
@@ -913,7 +913,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
(self._layout._find_actual_backend_snapshot_name.
assert_called_once_with(gmgr1, snapshot))
- @ddt.data({'side_effect': (glusterXMLOut(ret=-1, errno=2),),
+ @ddt.data({'side_effect': (glusterXMLOut(ret=-1, errno=0),),
'_exception': exception.GlusterfsException},
{'side_effect': (('', ''),),
'_exception': exception.GlusterfsException})
diff --git a/manila/tests/share/drivers/test_glusterfs.py b/manila/tests/share/drivers/test_glusterfs.py
index 39368ee5a5..f57fa213a0 100644
--- a/manila/tests/share/drivers/test_glusterfs.py
+++ b/manila/tests/share/drivers/test_glusterfs.py
@@ -116,31 +116,31 @@ class GlusterfsShareDriverTestCase(test.TestCase):
helpercls = mock.Mock(return_value=helper)
self._driver.nfs_helper = helpercls
if helpercls == glusterfs.GlusterNFSHelper and path is None:
- gmgr.get_vol_option = mock.Mock(return_value='on')
+ gmgr.get_vol_option = mock.Mock(return_value=True)
self._driver._setup_via_manager(
{'manager': gmgr, 'share': self.share})
if helpercls == glusterfs.GlusterNFSHelper and path is None:
gmgr.get_vol_option.assert_called_once_with(
- NFS_EXPORT_VOL)
+ NFS_EXPORT_VOL, boolean=True)
args = (NFS_RPC_AUTH_REJECT, '*')
else:
args = (NFS_EXPORT_VOL, False)
gmgr.set_vol_option.assert_called_once_with(*args)
- @ddt.data('off', 'no', '0', 'false', 'disable', 'foobarbaz')
- def test_setup_via_manager_export_volumes_on(self, export_vol):
+ def test_setup_via_manager_export_volumes_off(self):
gmgr = mock.Mock()
gmgr.path = None
- gmgr.get_vol_option = mock.Mock(return_value=export_vol)
+ gmgr.get_vol_option = mock.Mock(return_value=False)
self._driver.nfs_helper = glusterfs.GlusterNFSHelper
self.assertRaises(exception.GlusterfsException,
self._driver._setup_via_manager,
{'manager': gmgr, 'share': self.share})
- gmgr.get_vol_option.assert_called_once_with(NFS_EXPORT_VOL)
+ gmgr.get_vol_option.assert_called_once_with(NFS_EXPORT_VOL,
+ boolean=True)
def test_check_for_setup_error(self):
self._driver.check_for_setup_error()
diff --git a/manila/tests/share/drivers/test_glusterfs_native.py b/manila/tests/share/drivers/test_glusterfs_native.py
index 731f06ae92..13f11ea35a 100644
--- a/manila/tests/share/drivers/test_glusterfs_native.py
+++ b/manila/tests/share/drivers/test_glusterfs_native.py
@@ -227,18 +227,17 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
self.assertEqual(expected_exec, fake_utils.fake_execute_get_log())
- @ddt.data('on', '1', 'Yes', 'TRUE', 'enable')
- def test_deny_access_via_manager(self, trueish):
+ def test_deny_access_via_manager(self):
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute,
None, None)
- def _get_vol_option(opt):
+ def _get_vol_option(opt, **kw):
if opt == 'auth.ssl-allow':
return('some.common.name,' + access['access_to'])
elif opt == 'server.dynamic-auth':
- return trueish
+ return True
self.mock_object(
gmgr1, 'get_vol_option',
@@ -250,23 +249,24 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
self.share1, access)
gmgr1.get_vol_option.assert_has_calls(
- [mock.call(a) for a in ('auth.ssl-allow', 'server.dynamic-auth')])
+ [mock.call(a, **kw) for a, kw in (
+ ('auth.ssl-allow', {}),
+ ('server.dynamic-auth', {'boolean': True}))])
test_args = ('auth.ssl-allow', 'some.common.name')
gmgr1.set_vol_option.assert_called_once_with(*test_args)
self.assertFalse(common._restart_gluster_vol.called)
- @ddt.data('off', None, 'strangelove')
- def test_deny_access_via_manager_no_dyn_auth(self, falseish):
+ def test_deny_access_via_manager_no_dyn_auth(self):
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute,
None, None)
- def _get_vol_option(opt):
+ def _get_vol_option(opt, **kw):
if opt == 'auth.ssl-allow':
return('some.common.name,' + access['access_to'])
elif opt == 'server.dynamic-auth':
- return falseish
+ return False
self.mock_object(
gmgr1, 'get_vol_option',
@@ -278,7 +278,9 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
self.share1, access)
gmgr1.get_vol_option.assert_has_calls(
- [mock.call(a) for a in ('auth.ssl-allow', 'server.dynamic-auth')])
+ [mock.call(a, **kw) for a, kw in (
+ ('auth.ssl-allow', {}),
+ ('server.dynamic-auth', {'boolean': True}))])
test_args = ('auth.ssl-allow', 'some.common.name')
gmgr1.set_vol_option.assert_called_once_with(*test_args)
common._restart_gluster_vol.assert_called_once_with(gmgr1)