GPFS CES: Fix bugs related to access rules not found

Several bugs are caused by an error code and
exception raised when a path has no NFS exports.
Use the machine-readable mmnfs export list command
(the -Y option) which does not cause an exception
when checking for exports.

Co-Authored-By: digvijay2016 <digvijay.ukirde@in.ibm.com>

Change-Id: I770756e0a36c5b61386878164b651fadf9730b7f
Closes-Bug: #1650043
Closes-Bug: #1650044
Closes-Bug: #1650045
(cherry picked from commit 5e7323cd1f)
This commit is contained in:
Mark Sturdevant 2016-12-14 14:49:55 -08:00 committed by Digvijay Ukirde
parent 13b40b1463
commit c5545e861c
3 changed files with 91 additions and 63 deletions

View File

@ -769,14 +769,19 @@ class CESHelper(NASHelperBase):
raise exception.GPFSException(msg)
return out
def _has_client_access(self, local_path, access_to=None):
"""Check path for any export or for one with a specific IP address."""
err_msg = 'Failed to check exports on the system.'
out = self._execute_mmnfs_command(('list', '-n', local_path, '-Y'),
err_msg)
if access_to:
return ':' + access_to + ':' in out
else:
return ':' + local_path + ':' in out
def remove_export(self, local_path, share):
"""Remove export."""
err_msg = 'Failed to check exports on the system.'
out = self._execute_mmnfs_command(('list', '-n', local_path), err_msg)
out = re.search(re.escape(local_path), out)
if out is not None:
if self._has_client_access(local_path):
err_msg = ('Failed to remove export for share %s.'
% share['name'])
self._execute_mmnfs_command(('remove', local_path), err_msg)
@ -787,16 +792,13 @@ class CESHelper(NASHelperBase):
if access['access_type'] != 'ip':
raise exception.InvalidShareAccess(reason='Only ip access type '
'supported.')
err_msg = 'Failed to check exports on the system.'
out = self._execute_mmnfs_command(('list', '-n', local_path), err_msg)
has_exports = self._has_client_access(local_path)
options_not_allowed = ['access_type=ro', 'access_type=rw']
export_opts = self.get_export_options(share, access, 'CES',
options_not_allowed)
out = re.search(re.escape(local_path), out)
if out is None:
if not has_exports:
cmd = ['add', local_path, '-c',
access['access_to'] +
'(' + export_opts + ')']
@ -811,12 +813,9 @@ class CESHelper(NASHelperBase):
def deny_access(self, local_path, share, access, force=False):
"""Deny access to the host."""
err_msg = 'Failed to check exports on the system.'
out = self._execute_mmnfs_command(('list', '-n', local_path), err_msg)
has_export = self._has_client_access(local_path, access['access_to'])
out = re.search(re.escape(access['access_to']), out)
if out is not None:
if has_export:
err_msg = ('Failed to remove access for share %s.'
% share['name'])
self._execute_mmnfs_command(('change', local_path,

View File

@ -56,6 +56,20 @@ class GPFSShareDriverTestCase(test.TestCase):
self.fakefspath = "/gpfs0"
self.fakesharepath = "/gpfs0/share-fakeid"
self.fakesnapshotpath = "/gpfs0/.snapshots/snapshot-fakesnapshotid"
self.fake_ces_exports = """
mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Clients:Access_Type:Protocols:Transports:Squash:Anonymous_uid:Anonymous_gid:SecType:PrivilegedPort:DefaultDelegations:Manage_Gids:NFS_Commit:
mmcesnfslsexport:nfsexports:0:1:::/gpfs0/share-fakeid:none:44.3.2.11:RW:3,4:TCP:ROOT_SQUASH:-2:-2:SYS:FALSE:none:FALSE:FALSE:
mmcesnfslsexport:nfsexports:0:1:::/gpfs0/share-fakeid:none:1:2:3:4:5:6:7:8:RW:3,4:TCP:ROOT_SQUASH:-2:-2:SYS:FALSE:none:FALSE:FALSE:
mmcesnfslsexport:nfsexports:0:1:::/gpfs0/share-fakeid:none:10.0.0.1:RW:3,4:TCP:ROOT_SQUASH:-2:-2:SYS:FALSE:none:FALSE:FALSE:
"""
self.fake_ces_exports_not_found = """
mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Clients:Access_Type:Protocols:Transports:Squash:Anonymous_uid:Anonymous_gid:SecType:PrivilegedPort:DefaultDelegations:Manage_Gids:NFS_Commit:
"""
self.mock_object(gpfs.os.path, 'exists', mock.Mock(return_value=True))
self._driver._helpers = {
'KNFS': self._helper_fake
@ -860,24 +874,49 @@ class GPFSShareDriverTestCase(test.TestCase):
self._ces_helper.get_export_options,
share, access, 'CES', options_not_allowed)
def test_ces_remove_export(self):
mock_out = "Path Delegations Clients\n\
------------------------\n\
/gpfs0/share-fakeid none *"
@ddt.data((None, True),
('44.3.2.11', True),
('44.3.2.1', False),
('4.3.2.1', False),
('4.3.2.11', False),
('1.2.3.4', False),
('1:2:3:4:5:6:7:8', True))
@ddt.unpack
def test_ces__has_client_access(self, ip, has_access):
mock_out = self.fake_ces_exports
self._ces_helper._execute = mock.Mock(
return_value=[mock_out, 0])
mock_search_out = "/gpfs0/share-fakeid"
self.mock_object(re, 'search', mock.Mock(return_value=mock_search_out))
return_value=(mock_out, ''))
local_path = self.fakesharepath
self.assertEqual(has_access,
self._ces_helper._has_client_access(local_path, ip))
self._ces_helper._execute.assert_called_once_with(
'mmnfs', 'export', 'list', '-n', local_path, '-Y')
def test_ces_remove_export_no_exports(self):
mock_out = self.fake_ces_exports_not_found
self._ces_helper._execute = mock.Mock(
return_value=(mock_out, ''))
local_path = self.fakesharepath
self._ces_helper.remove_export(local_path, self.share)
self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'list',
'-n', local_path)
self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'remove',
local_path)
self._ces_helper._execute.assert_called_once_with(
'mmnfs', 'export', 'list', '-n', local_path, '-Y')
def test_ces_remove_export_existing_exports(self):
mock_out = self.fake_ces_exports
self._ces_helper._execute = mock.Mock(
return_value=(mock_out, ''))
local_path = self.fakesharepath
self._ces_helper.remove_export(local_path, self.share)
self._ces_helper._execute.assert_has_calls([
mock.call('mmnfs', 'export', 'list', '-n', local_path, '-Y'),
mock.call('mmnfs', 'export', 'remove', local_path),
])
def test_ces_remove_export_exception(self):
local_path = self.fakesharepath
@ -888,52 +927,42 @@ class GPFSShareDriverTestCase(test.TestCase):
local_path, self.share)
def test_ces_allow_access(self):
mock_out = "Path Delegations Clients\n\
------------------------"
mock_out = self.fake_ces_exports_not_found
self._ces_helper._execute = mock.Mock(
return_value=[mock_out, 0])
return_value=(mock_out, ''))
export_opts = "access_type=rw"
self._ces_helper.get_export_options = mock.Mock(
return_value=export_opts)
self.mock_object(re, 'search', mock.Mock(return_value=None))
access = self.access
local_path = self.fakesharepath
self._ces_helper.allow_access(local_path, self.share, access)
self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'list',
'-n', local_path)
self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'add',
local_path, '-c',
access['access_to']
+ '(' + export_opts + ')')
self._ces_helper._execute.assert_has_calls([
mock.call('mmnfs', 'export', 'list', '-n', local_path, '-Y'),
mock.call('mmnfs', 'export', 'add', local_path, '-c',
access['access_to'] + '(' + export_opts + ')')])
def test_ces_allow_access_existing_export(self):
mock_out = "Path Delegations Clients\n\
------------------------\n\
/gpfs0/share-fakeid none *"
def test_ces_allow_access_existing_exports(self):
mock_out = self.fake_ces_exports
self._ces_helper._execute = mock.Mock(
return_value=[mock_out, 0])
return_value=(mock_out, ''))
export_opts = "access_type=rw"
self._ces_helper.get_export_options = mock.Mock(
return_value=export_opts)
mock_search_out = "/gpfs0/share-fakeid"
self.mock_object(re, 'search', mock.Mock(return_value=mock_search_out))
access = self.access
local_path = self.fakesharepath
self._ces_helper.allow_access(local_path, self.share, access)
self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'list',
'-n', local_path)
self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'change',
local_path, '--nfsadd',
access['access_to']
+ '(' + export_opts + ')')
self._ces_helper._execute.assert_has_calls([
mock.call('mmnfs', 'export', 'list', '-n', local_path, '-Y'),
mock.call('mmnfs', 'export', 'change', local_path, '--nfsadd',
access['access_to'] + '(' + export_opts + ')')])
def test_ces_allow_access_invalid_access_type(self):
access = fake_share.fake_access(access_type='test')
@ -952,25 +981,19 @@ class GPFSShareDriverTestCase(test.TestCase):
self.share, access)
def test_ces_deny_access(self):
mock_out = "Path Delegations Clients\n\
------------------------\n\
/gpfs0/share-fakeid none *"
mock_out = self.fake_ces_exports
self._ces_helper._execute = mock.Mock(
return_value=[mock_out, 0])
mock_search_out = "/gpfs0/share-fakeid"
self.mock_object(re, 'search', mock.Mock(return_value=mock_search_out))
return_value=(mock_out, ''))
access = self.access
local_path = self.fakesharepath
self._ces_helper.deny_access(local_path, self.share, access)
self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'list',
'-n', local_path)
self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'change',
local_path, '--nfsremove',
access['access_to'])
self._ces_helper._execute.assert_has_calls([
mock.call('mmnfs', 'export', 'list', '-n', local_path, '-Y'),
mock.call('mmnfs', 'export', 'change', local_path, '--nfsremove',
access['access_to'])])
def test_ces_deny_access_exception(self):
access = self.access

View File

@ -0,0 +1,6 @@
---
fixes:
- Fixed GPFS CES to allow adding a first access rule to a share.
- Fixed GPFS CES to allow deleting a share with no access rules.
- Fixed GPFS CES to allow deletion of a failed access rule
when there are no successful access rules.