Merge "gluster*: clean up volume option querying"

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`"))

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:

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."""

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):

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())

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})

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()

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)