openstack
/
manila
Code Issues Proposed changes

glusterfs: Fix issues in backend instrumentation 

- allow share or sub-directory access to more than one guest/ip address
  by modifying the GlusterFS volume configuration options that change
  the Gluster-NFS server settings.

  `nfs.export-dir` option does not interpret a spec of
  '/dir(host1),/dir(host2)'as we expected, ie. two independent access
  specifiers -- instead, it just discards the latter. The required
  format looks as follows: '/dir(host1|host2)'. Update the export-dir
  generating routines accordingly.

- properly remove the last share or sub-directory access setting in a
  GlusterFS volume when needed by an API request to changes access
  rules of shares.

  `volume set` option in GlusterFS volume settings cannot take empty
  value, use instead `volume reset` to go back to defaults

- GlusterFS volume contains all the shares. It has a default setting
  that allows NFS access to its entire contents. So turn off this
  setting as a part of the initial driver setup.

  turn off `nfs.export-volumes` setting.

Change-Id: I652a13d4d34d1b4c5ee3d31af40143a098c38074
This commit is contained in:
Csaba Henk 2014-06-05 10:58:01 +05:30 committed by Ramana Raja
parent 67aa5bcf2d
commit 99857a4983
2 changed files with 167 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

99
manila/share/drivers/glusterfs.py
View File

@ -47,7 +47,8 @@ GlusterfsManilaShare_opts = [
CONF = cfg.CONF
CONF.register_opts(GlusterfsManilaShare_opts)
_nfs_export_dir = 'nfs.export-dir'
NFS_EXPORT_DIR = 'nfs.export-dir'
NFS_EXPORT_VOL = 'nfs.export-volumes'
class GlusterAddress(object):
@ -107,6 +108,17 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.ShareDriver):
self._ensure_gluster_vol_mounted()
# exporting the whole volume must be prohibited
# to not to defeat access control
args, kw = self.gluster_address.make_gluster_args(
'volume', 'set', self.gluster_address.volume,
NFS_EXPORT_VOL, 'off')
try:
self._execute(*args, **kw)
except exception.ProcessExecutionError as exc:
LOG.error(_("Error in gluster volume set: %s") % exc.stderr)
raise
def check_for_setup_error(self):
"""Is called after do_setup method. Nothing to do."""
pass
@ -152,7 +164,7 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.ShareDriver):
with open(config_file) as f:
return f.readline().strip()
def _get_export_dir_list(self):
def _get_export_dir_dict(self):
try:
args, kw = self.gluster_address.make_gluster_args(
'--xml',
@ -177,13 +189,23 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.ShareDriver):
for o, v in \
((e.find(a).text for a in ('name', 'value'))
for e in vix.findall(".//option")):
if o == _nfs_export_dir:
if o == NFS_EXPORT_DIR:
export_dir = v
break
edh = {}
if export_dir:
return export_dir.split(',')
else:
return []
# see
# https://github.com/gluster/glusterfs
# /blob/aa19909/xlators/nfs/server/src/nfs.c#L1582
# regarding the format of nfs.export-dir
edl = export_dir.split(',')
# parsing export_dir into a dict of {dir: [hostpec,..]..}
# format
r = re.compile('\A/(.*)\((.*)\)\Z')
for ed in edl:
d, e = r.match(ed).groups()
edh[d] = e.split('|')
return edh
def _ensure_gluster_vol_mounted(self):
"""Ensure that a Gluster volume is native-mounted on Manila host.
@ -247,42 +269,63 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.ShareDriver):
pass
def _manage_access(self, context, share, access, cbk):
"""Manage share access by adjusting the export list with cbk
"""Manage share access with cbk.
cbk is a callable of args (dl, acc), where dl is a list of strings
and acc is a string. It should return True (or a value of Boolean
reduct True) if it leaves dl intact, and False (or a value of Boolean
reduct False) if it makes a change on dl
Adjust the exports of the Gluster-NFS server using cbk.
cbk will be called with dl being list of currently exported dirs and
acc being a textual specification derived from access.
:param share: share object
:param access: access object
:param cbk: callback to adjust the exports of NFS server
Following is the description of cbk(ddict, edir, host).
:param ddict: association of shares with ips that have access to them
:type ddict: dict
:param edir: name of share i.e. export directory
:type edir: string
:param host: ip address derived from the access object
:type host: string
:returns: bool (cbk leaves ddict intact) or None (cbk modifies ddict)
"""
if access['access_type'] != 'ip':
raise exception.InvalidShareAccess('only ip access type allowed')
export_dir_list = self._get_export_dir_list()
access_spec = "/%s(%s)" % (share['name'], access['access_to'])
if cbk(export_dir_list, access_spec):
export_dir_dict = self._get_export_dir_dict()
if cbk(export_dir_dict, share['name'], access['access_to']):
return
export_dir_new = ",".join(export_dir_list)
try:
if export_dir_dict:
export_dir_new = ",".join("/%s(%s)" % (d, "|".join(v))
for d, v in export_dir_dict.items())
args, kw = self.gluster_address.make_gluster_args(
'volume', 'set', self.gluster_address.volume,
_nfs_export_dir, export_dir_new)
'volume', 'set', self.gluster_address.volume,
NFS_EXPORT_DIR, export_dir_new)
else:
args, kw = self.gluster_address.make_gluster_args(
'volume', 'reset', self.gluster_address.volume,
NFS_EXPORT_DIR)
try:
self._execute(*args, **kw)
except exception.ProcessExecutionError as exc:
LOG.error(_("Error in gluster volume set: %s") % exc.stderr)
raise
def allow_access(self, context, share, access):
"""NFS export a dir to a volume"""
self._manage_access(context, share, access,
lambda dl, acc:
True if acc in dl else dl.append(acc))
"""Allow access to a share."""
def cbk(ddict, edir, host):
if edir not in ddict:
ddict[edir] = []
if host in ddict[edir]:
return True
ddict[edir].append(host)
self._manage_access(context, share, access, cbk)
def deny_access(self, context, share, access):
"""Deny access to the share."""
self._manage_access(context, share, access,
lambda dl, acc:
True if acc not in dl else dl.remove(acc))
"""Deny access to a share."""
def cbk(ddict, edir, host):
if edir not in ddict or host not in ddict[edir]:
return True
ddict[edir].remove(host)
if not ddict[edir]:
ddict.pop(edir)
self._manage_access(context, share, access, cbk)

140
manila/tests/test_share_glusterfs.py
View File

@ -133,25 +133,49 @@ class GlusterfsShareDriverTestCase(test.TestCase):
def test_do_setup(self):
self._driver._read_gluster_vol_from_config =\
Mock(return_value='testuser@127.0.0.1:/testvol/fakename')
Mock(return_value='127.0.0.1:/testvol')
self._driver._ensure_gluster_vol_mounted = Mock()
expected_exec = ['mount.glusterfs']
exec_cmd1 = 'mount.glusterfs'
exec_cmd2 = 'gluster volume set testvol nfs.export-volumes off'
expected_exec = [exec_cmd1, exec_cmd2]
self._driver.do_setup(self._context)
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
self._driver._ensure_gluster_vol_mounted.assert_called_once_with()
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
def test_do_setup_mount_glusterfs_not_installed(self):
self._driver._read_gluster_vol_from_config =\
Mock(return_value='testuser@127.0.0.1:/testvol/fakename')
Mock(return_value='127.0.0.1:/testvol')
self._driver._ensure_gluster_vol_mounted = Mock()
def exec_runner(*ignore_args, **ignore_kw):
def exec_runner(*ignore_args, **ignore_kwargs):
raise OSError(errno.ENOENT, os.strerror(errno.ENOENT))
expected_exec = ['mount.glusterfs']
fake_utils.fake_execute_set_repliers([(expected_exec[0], exec_runner)])
self.assertRaises(exception.GlusterfsException, self._driver.do_setup,
self._context)
def test_do_setup_mount_glusterfs_error_gluster_vol_set(self):
self._driver._read_gluster_vol_from_config =\
Mock(return_value='127.0.0.1:/testvol')
self._driver._ensure_gluster_vol_mounted = Mock()
glusterfs.LOG.error = Mock()
def exec_runner(*ignore_args, **ignore_kwargs):
raise exception.ProcessExecutionError(stderr='testvol')
expected_exec = ['gluster volume set testvol nfs.export-volumes off']
fake_utils.fake_execute_set_repliers([(expected_exec[0], exec_runner)])
try:
self._driver.do_setup(self._context)
except exception.ProcessExecutionError:
pass
else:
self.fail('Expecting exception.ProcessExecutionError')
glusterfs.LOG.error.assert_called_with("Error in gluster volume set: "
"testvol")
def test_do_mount(self):
expected_exec = ['true']
ret = self._driver._do_mount(expected_exec, False)
@ -220,15 +244,15 @@ class GlusterfsShareDriverTestCase(test.TestCase):
self.assertRaises(exception.GlusterfsException,
self._driver._ensure_gluster_vol_mounted)
def test_get_export_dir_list_empty_volinfo(self):
def test_get_export_dir_dict_empty_volinfo(self):
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
expected_exec = ['true']
self.assertRaises(exception.GlusterfsException,
self._driver._get_export_dir_list)
self._driver._get_export_dir_dict)
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
def test_get_export_dir_list_failing_volinfo(self):
def test_get_export_dir_dict_failing_volinfo(self):
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
@ -236,10 +260,10 @@ class GlusterfsShareDriverTestCase(test.TestCase):
raise RuntimeError('fake error')
expected_exec = ['true']
fake_utils.fake_execute_set_repliers([(expected_exec[0], exec_runner)])
self.assertRaises(RuntimeError, self._driver._get_export_dir_list)
self.assertRaises(RuntimeError, self._driver._get_export_dir_dict)
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
def test_get_export_dir_list_ambiguous_volinfo(self):
def test_get_export_dir_dict_ambiguous_volinfo(self):
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
@ -257,10 +281,10 @@ class GlusterfsShareDriverTestCase(test.TestCase):
expected_exec = ['true']
fake_utils.fake_execute_set_repliers([(expected_exec[0], exec_runner)])
self.assertRaises(exception.InvalidShare,
self._driver._get_export_dir_list)
self._driver._get_export_dir_dict)
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
def test_get_export_dir_list_trivial_volinfo(self):
def test_get_export_dir_dict_trivial_volinfo(self):
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
@ -279,11 +303,11 @@ class GlusterfsShareDriverTestCase(test.TestCase):
""", ''
expected_exec = ['true']
fake_utils.fake_execute_set_repliers([(expected_exec[0], exec_runner)])
ret = self._driver._get_export_dir_list()
ret = self._driver._get_export_dir_dict()
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
self.assertEqual(ret, [])
self.assertEqual(ret, {})
def test_get_export_dir_list(self):
def test_get_export_dir_dict(self):
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
@ -297,7 +321,7 @@ class GlusterfsShareDriverTestCase(test.TestCase):
<options>
<option>
<name>nfs.export-dir</name>
<value>foo,bar</value>
<value>/foo(10.0.0.1|10.0.0.2),/bar(10.0.0.1)</value>
</option>
</options>
</volume>
@ -308,9 +332,11 @@ class GlusterfsShareDriverTestCase(test.TestCase):
""", ''
expected_exec = ['true']
fake_utils.fake_execute_set_repliers([(expected_exec[0], exec_runner)])
ret = self._driver._get_export_dir_list()
ret = self._driver._get_export_dir_dict()
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
self.assertEqual(ret, ['foo', 'bar'])
self.assertEqual(ret,
{'foo': ['10.0.0.1', '10.0.0.2'], 'bar': ['10.0.0.1']}
)
def test_get_local_share_path(self):
with patch.object(os, 'access', return_value=True):
@ -377,8 +403,8 @@ class GlusterfsShareDriverTestCase(test.TestCase):
def test_manage_access_noop(self):
cbk = Mock(return_value=True)
access = {'access_type': 'ip', 'access_to': '0.0.0.0'}
self._driver._get_export_dir_list = Mock()
access = {'access_type': 'ip', 'access_to': '10.0.0.1'}
self._driver._get_export_dir_dict = Mock()
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
expected_exec = []
@ -388,10 +414,12 @@ class GlusterfsShareDriverTestCase(test.TestCase):
self.assertEqual(ret, None)
def test_manage_access_adding_entry(self):
cbk = list.append
access = {'access_type': 'ip', 'access_to': '0.0.0.0'}
self._driver._get_export_dir_list =\
Mock(return_value=['/example.com(0.0.0.0)'])
def cbk(d, key, value):
d[key].append(value)
access = {'access_type': 'ip', 'access_to': '10.0.0.2'}
self._driver._get_export_dir_dict =\
Mock(return_value={'example.com': ['10.0.0.1'],
'fakename': ['10.0.0.1']})
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
expected_exec = ['true']
@ -402,13 +430,15 @@ class GlusterfsShareDriverTestCase(test.TestCase):
self.assertTrue(self._driver.gluster_address.make_gluster_args.called)
self.assertEqual(
self._driver.gluster_address.make_gluster_args.call_args[0][-1],
'/example.com(0.0.0.0),/fakename(0.0.0.0)')
'/example.com(10.0.0.1),/fakename(10.0.0.1|10.0.0.2)')
def test_manage_access_adding_entry_cmd_fail(self):
cbk = list.append
access = {'access_type': 'ip', 'access_to': '0.0.0.0'}
self._driver._get_export_dir_list =\
Mock(return_value=['/example.com(0.0.0.0)'])
def cbk(d, key, value):
d[key].append(value)
access = {'access_type': 'ip', 'access_to': '10.0.0.2'}
self._driver._get_export_dir_dict =\
Mock(return_value={'example.com': ['10.0.0.1'],
'fakename': ['10.0.0.1']})
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
expected_exec = ['true']
@ -424,45 +454,67 @@ class GlusterfsShareDriverTestCase(test.TestCase):
self.assertTrue(self._driver.gluster_address.make_gluster_args.called)
self.assertEqual(
self._driver.gluster_address.make_gluster_args.call_args[0][-1],
'/example.com(0.0.0.0),/fakename(0.0.0.0)')
'/example.com(10.0.0.1),/fakename(10.0.0.1|10.0.0.2)')
def test_manage_access_removing_last_entry(self):
def cbk(d, key, value):
d.pop(key)
access = {'access_type': 'ip', 'access_to': '10.0.0.1'}
self._driver._get_export_dir_dict =\
Mock(return_value={'fakename': ['10.0.0.1']})
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
expected_exec = ['true']
ret = self._driver._manage_access(self._context, self.share, access,
cbk)
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
self.assertEqual(ret, None)
self.assertTrue(self._driver.gluster_address.make_gluster_args.called)
self.assertEqual(
self._driver.gluster_address.make_gluster_args.call_args[0][1],
'reset')
self.assertEqual(
self._driver.gluster_address.make_gluster_args.call_args[0][-1],
'nfs.export-dir')
def test_allow_access_with_share_having_noaccess(self):
access = {'access_type': 'ip', 'access_to': '0.0.0.0'}
self._driver._get_export_dir_list =\
Mock(return_value=['/example.com(0.0.0.0)'])
access = {'access_type': 'ip', 'access_to': '10.0.0.1'}
self._driver._get_export_dir_dict =\
Mock(return_value={'example.com': ['10.0.0.1']})
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
self._driver.allow_access(self._context, self.share, access)
self.assertTrue(self._driver.gluster_address.make_gluster_args.called)
self.assertEqual(
self._driver.gluster_address.make_gluster_args.call_args[0][-1],
'/example.com(0.0.0.0),/fakename(0.0.0.0)')
'/example.com(10.0.0.1),/fakename(10.0.0.1)')
def test_allow_access_with_share_having_access(self):
access = {'access_type': 'ip', 'access_to': '0.0.0.0'}
self._driver._get_export_dir_list = \
Mock(return_value=['/fakename(0.0.0.0)'])
access = {'access_type': 'ip', 'access_to': '10.0.0.1'}
self._driver._get_export_dir_dict = \
Mock(return_value={'fakename': ['10.0.0.1']})
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
self._driver.allow_access(self._context, self.share, access)
self.assertFalse(self._driver.gluster_address.make_gluster_args.called)
def test_deny_access_with_share_having_noaccess(self):
access = {'access_type': 'ip', 'access_to': '0.0.0.0'}
self._driver._get_export_dir_list = Mock(return_value=[])
access = {'access_type': 'ip', 'access_to': '10.0.0.1'}
self._driver._get_export_dir_dict = Mock(return_value={})
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
self._driver.deny_access(self._context, self.share, access)
self.assertFalse(self._driver.gluster_address.make_gluster_args.called)
def test_deny_access_with_share_having_access(self):
access = {'access_type': 'ip', 'access_to': '0.0.0.0'}
self._driver._get_export_dir_list = \
Mock(return_value=['/fakename(0.0.0.0)', '/example.com(0.0.0.0)'])
access = {'access_type': 'ip', 'access_to': '10.0.0.1'}
self._driver._get_export_dir_dict = \
Mock(return_value={'fakename': ['10.0.0.1'],
'example.com': ['10.0.0.1']})
self._driver.gluster_address = Mock(make_gluster_args=
Mock(return_value=(('true',), {})))
self._driver.deny_access(self._context, self.share, access)
self.assertTrue(self._driver.gluster_address.make_gluster_args.called)
self.assertEqual(
self._driver.gluster_address.make_gluster_args.call_args[0][-1],
'/example.com(0.0.0.0)')
'/example.com(10.0.0.1)')