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)