Remove default values for update_access()

update_access method has None as default values for add_rules and
delete_rules. These parameters should always be iterables, so this
should be removed. This patch removes the default values for this
method on the base class and on all implemented classes.

Change-Id: I86f4ccc9d496ec6183bd0fa5be9a77c3451378d5
Closes-bug: #1555294
This commit is contained in:
tpsilva 2016-03-10 09:56:07 -03:00 committed by Rodrigo Barbieri
parent 3abd644ef2
commit 02a9a2c3e2
21 changed files with 113 additions and 82 deletions

View File

@ -486,13 +486,13 @@ class ShareDriver(object):
"""Deny access to the share."""
raise NotImplementedError()
def update_access(self, context, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
"""Update access rules for given share.
Drivers should support 2 different cases in this method:
1. Recovery after error - 'access_rules' contains all access_rules,
'add_rules' and 'delete_rules' shall be None. Driver should clear any
'add_rules' and 'delete_rules' shall be empty. Driver should clear any
existent access rules and apply all access rules for given share.
This recovery is made at driver start up.
@ -518,9 +518,9 @@ class ShareDriver(object):
:param context: Current context
:param share: Share model with share data.
:param access_rules: All access rules for given share
:param add_rules: None or List of access rules which should be added
access_rules already contains these rules.
:param delete_rules: None or List of access rules which should be
:param add_rules: Empty List or List of access rules which should be
added. access_rules already contains these rules.
:param delete_rules: Empty List or List of access rules which should be
removed. access_rules doesn't contain these rules.
:param share_server: None or Share server model
"""

View File

@ -229,8 +229,8 @@ class CephFSNativeDriver(driver.ShareDriver,):
access['access_to'])
self.volume_client.evict(access['access_to'])
def update_access(self, context, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
# The interface to Ceph just provides add/remove methods, since it
# was created at start of mitaka cycle when there was no requirement
# to be able to list access rules or set them en masse. Therefore

View File

@ -847,13 +847,13 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
share_server['backend_details'], share['name'], recreate=True)
@ensure_server
def update_access(self, context, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
"""Update access rules for given share.
This driver has two different behaviors according to parameters:
1. Recovery after error - 'access_rules' contains all access_rules,
'add_rules' and 'delete_rules' shall be None. Previously existing
'add_rules' and 'delete_rules' shall be empty. Previously existing
access rules are cleared and then added back according
to 'access_rules'.
@ -865,9 +865,9 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
:param context: Current context
:param share: Share model with share data.
:param access_rules: All access rules for given share
:param add_rules: None or List of access rules which should be added
access_rules already contains these rules.
:param delete_rules: None or List of access rules which should be
:param add_rules: Empty List or List of access rules which should be
added. access_rules already contains these rules.
:param delete_rules: Empty List or List of access rules which should be
removed. access_rules doesn't contain these rules.
:param share_server: None or Share server model
"""

View File

@ -50,13 +50,13 @@ class NASHelperBase(object):
"""Configure server before allowing access."""
pass
def update_access(self, server, share_name, access_rules, add_rules=None,
delete_rules=None):
def update_access(self, server, share_name, access_rules, add_rules,
delete_rules):
"""Update access rules for given share.
This driver has two different behaviors according to parameters:
1. Recovery after error - 'access_rules' contains all access_rules,
'add_rules' and 'delete_rules' shall be None. Previously existing
'add_rules' and 'delete_rules' shall be empty. Previously existing
access rules are cleared and then added back according
to 'access_rules'.
@ -68,9 +68,9 @@ class NASHelperBase(object):
:param server: None or Share server's backend details
:param share_name: Share's path according to id.
:param access_rules: All access rules for given share
:param add_rules: None or List of access rules which should be added
access_rules already contains these rules.
:param delete_rules: None or List of access rules which should be
:param add_rules: Empty List or List of access rules which should be
added. access_rules already contains these rules.
:param delete_rules: Empty List or List of access rules which should be
removed. access_rules doesn't contain these rules.
"""
raise NotImplementedError()
@ -158,8 +158,8 @@ class NFSHelper(NASHelperBase):
"""Remove export."""
@nfs_synchronized
def update_access(self, server, share_name, access_rules, add_rules=None,
delete_rules=None):
def update_access(self, server, share_name, access_rules, add_rules,
delete_rules):
"""Update access rules for given share.
Please refer to base class for a more in-depth description.
@ -196,7 +196,7 @@ class NFSHelper(NASHelperBase):
add_rules, ('ip',),
(const.ACCESS_LEVEL_RO, const.ACCESS_LEVEL_RW))
for access in (delete_rules or []):
for access in delete_rules:
try:
self.validate_access_rules(
[access], ('ip',),
@ -214,7 +214,7 @@ class NFSHelper(NASHelperBase):
':'.join((access['access_to'], local_path))])
if delete_rules:
self._sync_nfs_temp_and_perm_files(server)
for access in (add_rules or []):
for access in add_rules:
access_to, access_type = (access['access_to'],
access['access_type'])
found_item = re.search(
@ -379,8 +379,8 @@ class CIFSHelperIPAccess(NASHelperBase):
self._ssh_exec(server, ['sudo', 'smbcontrol', 'all', 'close-share',
share_name])
def update_access(self, server, share_name, access_rules, add_rules=None,
delete_rules=None):
def update_access(self, server, share_name, access_rules, add_rules,
delete_rules):
"""Update access rules for given share.
Please refer to base class for a more in-depth description. For this
@ -472,8 +472,8 @@ class CIFSHelperUserAccess(CIFSHelperIPAccess):
'read only': 'no',
}
def update_access(self, server, share_name, access_rules, add_rules=None,
delete_rules=None):
def update_access(self, server, share_name, access_rules, add_rules,
delete_rules):
"""Update access rules for given share.
Please refer to base class for a more in-depth description. For this

View File

@ -132,18 +132,18 @@ class HDSHNASDriver(driver.ShareDriver):
hnas_evs_id, self.hnas_evs_ip, self.fs_name,
job_timeout)
def update_access(self, context, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
"""Update access rules for given share.
:param context: The `context.RequestContext` object for the request
:param share: Share that will have its access rules updated.
:param access_rules: All access rules for given share. This list
is enough to update the access rules for given share.
:param add_rules: None or List of access rules which should be added.
access_rules already contains these rules. Not used by
this driver.
:param delete_rules: None or List of access rules which should be
:param add_rules: Empty List or List of access rules which should be
added. access_rules already contains these rules. Not used by this
driver.
:param delete_rules: Empty List or List of access rules which should be
removed. access_rules doesn't contain these rules. Not used by
this driver.
:param share_server: Data structure with share server information.

View File

@ -157,8 +157,8 @@ class HuaweiNasDriver(driver.ShareDriver):
LOG.debug("Deny access.")
self.plugin.deny_access(share, access, share_server)
def update_access(self, context, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
"""Update access rules list."""
LOG.debug("Update access.")
self.plugin.update_access(share, access_rules,

View File

@ -743,8 +743,8 @@ class V3StorageConnection(driver.HuaweiBase):
self.helper._remove_access_from_share(access_id,
share_proto)
def update_access(self, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, share, access_rules, add_rules,
delete_rules, share_server=None):
"""Update access rules list."""
if not (add_rules or delete_rules):
self.clear_access(share, share_server)

View File

@ -266,13 +266,13 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver):
except exception.InvalidShare as exc:
LOG.warning(exc.message)
def update_access(self, context, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
"""Update access rules for given share.
This driver has two different behaviors according to parameters:
1. Recovery after error - 'access_rules' contains all access_rules,
'add_rules' and 'delete_rules' shall be None. Previously existing
'add_rules' and 'delete_rules' shall be empty. Previously existing
access rules are cleared and then added back according
to 'access_rules'.
@ -284,9 +284,9 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver):
:param context: Current context
:param share: Share model with share data.
:param access_rules: All access rules for given share
:param add_rules: None or List of access rules which should be added
access_rules already contains these rules.
:param delete_rules: None or List of access rules which should be
:param add_rules: Empty List or List of access rules which should be
added. access_rules already contains these rules.
:param delete_rules: Empty List or List of access rules which should be
removed. access_rules doesn't contain these rules.
:param share_server: None or Share server model
"""

View File

@ -502,14 +502,14 @@ class LXDCIFSHelper(object):
)
def update_access(self, share_name, server_id, access_rules,
add_rules=None, delete_rules=None):
add_rules, delete_rules):
if not (add_rules or delete_rules):
# clean all hosts from allowed hosts list first.
self.lxd.execute_sync(
server_id,
["net", "conf", "setparm", share_name, "hosts allow", ""]
)
for rule in (access_rules or []):
for rule in access_rules:
host_to_allow = rule['access_to']
access_level = rule['access_level']
access_type = rule['access_type']

View File

@ -95,8 +95,10 @@ class NetAppCmodeMultiSvmShareDriver(driver.ShareDriver):
def unmanage(self, share):
raise NotImplementedError
def update_access(self, context, share, access_rules, **kwargs):
self.library.update_access(context, share, access_rules, **kwargs)
def update_access(self, context, share, access_rules, add_rules,
delete_rules, **kwargs):
self.library.update_access(context, share, access_rules, add_rules,
delete_rules, **kwargs)
def _update_share_stats(self, data=None):
data = self.library.get_share_stats()

View File

@ -95,8 +95,10 @@ class NetAppCmodeSingleSvmShareDriver(driver.ShareDriver):
def unmanage(self, share):
self.library.unmanage(share)
def update_access(self, context, share, access_rules, **kwargs):
self.library.update_access(context, share, access_rules, **kwargs)
def update_access(self, context, share, access_rules, add_rules,
delete_rules, **kwargs):
self.library.update_access(context, share, access_rules, add_rules,
delete_rules, **kwargs)
def _update_share_stats(self, data=None):
data = self.library.get_share_stats()

View File

@ -1037,8 +1037,8 @@ class NetAppCmodeFileStorageLibrary(object):
vserver_client.set_volume_size(share_name, new_size)
@na_utils.trace
def update_access(self, context, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
"""Updates access rules for a share."""
# NOTE(ameade): We do not need to add export rules to a non-active
# replica as it will fail.

View File

@ -328,13 +328,13 @@ class QuobyteShareDriver(driver.ExecuteMixin, driver.ShareDriver,):
"""
self._resize_share(share=shrink_share, new_size=shrink_size)
def update_access(self, context, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
"""Update access rules for given share.
Two different cases are supported in here:
1. Recovery after error - 'access_rules' contains all access_rules,
'add_rules' and 'delete_rules' are None. Driver should apply all
'add_rules' and 'delete_rules' are empty. Driver should apply all
access rules for given share.
2. Adding/Deleting of several access rules - 'access_rules' contains
@ -345,9 +345,9 @@ class QuobyteShareDriver(driver.ExecuteMixin, driver.ShareDriver,):
:param context: Current context
:param share: Share model with share data.
:param access_rules: All access rules for given share
:param add_rules: None or List of access rules which should be added
access_rules already contains these rules.
:param delete_rules: None or List of access rules which should be
:param add_rules: Empty List or List of access rules which should be
added. access_rules already contains these rules.
:param delete_rules: Empty List or List of access rules which should be
removed. access_rules doesn't contain these rules.
:param share_server: None or Share server model
:raises If all of the *_rules params are None the method raises an

View File

@ -398,8 +398,8 @@ class TegileShareDriver(driver.ShareDriver):
raise exception.InvalidShareAccess(reason=reason)
@debugger
def update_access(self, context, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
if not (add_rules or delete_rules):
# Recovery mode
pool, project, share_name = (

View File

@ -529,8 +529,8 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
self.zfs('set', 'quota=%sG' % new_size, dataset_name)
@ensure_share_server_not_provided
def update_access(self, context, share, access_rules, add_rules=None,
delete_rules=None, share_server=None):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
"""Updates access rules for given share."""
dataset_name = self._get_dataset_name(share)
return self._get_share_helper(share['share_proto']).update_access(

View File

@ -156,8 +156,8 @@ class NASHelperBase(object):
"""Removes share exports."""
@abc.abstractmethod
def update_access(self, dataset_name, access_rules, add_rules=None,
delete_rules=None):
def update_access(self, dataset_name, access_rules, add_rules,
delete_rules):
"""Update access rules for specified ZFS dataset."""
@ -228,8 +228,8 @@ class NFSviaZFSHelper(ExecuteMixin, NASHelperBase):
return access_to.split('/')[0] + '/' + netmask
@zfs_dataset_synchronized
def update_access(self, dataset_name, access_rules, add_rules=None,
delete_rules=None, make_all_ro=False):
def update_access(self, dataset_name, access_rules, add_rules,
delete_rules, make_all_ro=False):
"""Update access rules for given ZFS dataset exported as NFS share."""
rw_rules = []
ro_rules = []

View File

@ -155,7 +155,7 @@ class HDSHNASTestCase(test.TestCase):
self.mock_object(ssh.HNASSSHBackend, "update_access_rule",
mock.Mock())
self._driver.update_access('context', share, access_list)
self._driver.update_access('context', share, access_list, [], [])
ssh.HNASSSHBackend.update_access_rule.assert_called_once_with(
share['id'], [access1['access_to'] + '('
@ -179,7 +179,7 @@ class HDSHNASTestCase(test.TestCase):
self.assertRaises(exception.InvalidShareAccess,
self._driver.update_access, 'context', share,
access_list)
access_list, [], [])
def test_create_share(self):
self.mock_object(hds_hnas.HDSHNASDriver, "_check_fs_mounted",

View File

@ -1873,6 +1873,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
self.library.update_access(self.context,
fake.SHARE,
[fake.SHARE_ACCESS],
[],
[],
share_server=fake.SHARE_SERVER)
mock_get_vserver.assert_called_once_with(
@ -1901,6 +1903,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
self.library.update_access(self.context,
fake.SHARE,
[fake.SHARE_ACCESS],
[],
[],
share_server=fake.SHARE_SERVER)
mock_get_vserver.assert_called_once_with(
@ -1927,6 +1931,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
self.library.update_access(self.context,
fake.SHARE,
[fake.SHARE_ACCESS],
[],
[],
share_server=fake.SHARE_SERVER)
mock_get_vserver.assert_called_once_with(
@ -1955,6 +1961,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
self.library.update_access(self.context,
fake_share,
[fake.SHARE_ACCESS],
[],
[],
share_server=fake.SHARE_SERVER)
mock_get_vserver.assert_called_once_with(
@ -1971,6 +1979,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
self.library.update_access(self.context,
fake_share,
[fake.SHARE_ACCESS],
[],
[],
share_server=fake.SHARE_SERVER)
def test_setup_server(self):

View File

@ -500,7 +500,8 @@ class QuobyteShareDriverTestCase(test.TestCase):
@mock.patch.object(quobyte.LOG, "warning")
def test_update_access_no_rules(self, qb_log_mock):
self._driver.update_access(context=None, share=None, access_rules=[])
self._driver.update_access(context=None, share=None, access_rules=[],
add_rules=[], delete_rules=[])
qb_log_mock.assert_has_calls([mock.ANY])
@ -524,7 +525,8 @@ class QuobyteShareDriverTestCase(test.TestCase):
qb_subtr_mock.side_effect = [[new_access_1, new_access_2], []]
self._driver.update_access(self._context, self.share,
access_rules=add_access_rules)
access_rules=add_access_rules, add_rules=[],
delete_rules=[])
assert_calls = [mock.call(self._context, self.share, new_access_1),
mock.call(self._context, self.share, new_access_2)]
@ -550,7 +552,8 @@ class QuobyteShareDriverTestCase(test.TestCase):
old_access_rules = [old_access_1, old_access_2]
self._driver.update_access(self._context, self.share,
access_rules=old_access_rules)
access_rules=old_access_rules, add_rules=[],
delete_rules=[])
qb_deny_mock.assert_called_once_with(self._context,
self.share,
@ -587,7 +590,8 @@ class QuobyteShareDriverTestCase(test.TestCase):
[miss_access_1, old_access_2]]
self._driver.update_access(self._context, self.share,
new_access_rules)
new_access_rules, add_rules=[],
delete_rules=[])
a_calls = [mock.call(self._context, self.share, new_access_1),
mock.call(self._context, self.share, new_access_2)]

View File

@ -131,7 +131,9 @@ class NFSHelperTestCase(test.TestCase):
self._helper.update_access,
self.server,
self.share_name,
access_rules)
access_rules,
[],
[])
def test_update_access_invalid_level(self):
access_rules = [test_generic.get_fake_access_rule(
@ -141,7 +143,9 @@ class NFSHelperTestCase(test.TestCase):
self._helper.update_access,
self.server,
self.share_name,
access_rules)
access_rules,
[],
[])
def test_get_host_list(self):
fake_exportfs = ('/shares/share-1\n\t\t20.0.0.3\n'
@ -165,7 +169,8 @@ class NFSHelperTestCase(test.TestCase):
self.mock_object(self._helper, '_sync_nfs_temp_and_perm_files')
self.mock_object(self._helper, '_get_host_list',
mock.Mock(return_value=['1.1.1.1']))
self._helper.update_access(self.server, self.share_name, access_rules)
self._helper.update_access(self.server, self.share_name, access_rules,
[], [])
local_path = os.path.join(CONF.share_mount_path, self.share_name)
self._ssh_exec.assert_has_calls([
mock.call(self.server, ['sudo', 'exportfs']),
@ -410,7 +415,9 @@ class CIFSHelperIPAccessTestCase(test.TestCase):
self._helper.update_access,
self.server_details,
self.share_name,
access_rules)
access_rules,
[],
[])
def test_update_access_wrong_access_type(self):
access_rules = [test_generic.get_fake_access_rule(
@ -420,14 +427,16 @@ class CIFSHelperIPAccessTestCase(test.TestCase):
self._helper.update_access,
self.server_details,
self.share_name,
access_rules)
access_rules,
[],
[])
def test_update_access(self):
access_rules = [test_generic.get_fake_access_rule(
'1.1.1.1', const.ACCESS_LEVEL_RW), ]
self._helper.update_access(self.server_details, self.share_name,
access_rules)
access_rules, [], [])
self._helper._ssh_exec.assert_called_once_with(
self.server_details, ['sudo', 'net', 'conf', 'setparm',
self.share_name, '"hosts allow"',
@ -570,7 +579,7 @@ class CIFSHelperUserAccessTestCase(test.TestCase):
'user1', const.ACCESS_LEVEL_RW, access_type='ip')]
self.assertRaises(exception.InvalidShareAccess,
self._helper.update_access, self.server_details,
self.share_name, access_rules, None, None)
self.share_name, access_rules, [], [])
def test_update_access(self):
access_list = [test_generic.get_fake_access_rule(
@ -578,7 +587,7 @@ class CIFSHelperUserAccessTestCase(test.TestCase):
test_generic.get_fake_access_rule(
'user2', const.ACCESS_LEVEL_RO, access_type='user')]
self._helper.update_access(self.server_details, self.share_name,
access_list, None, None)
access_list, [], [])
self._helper._ssh_exec.assert_has_calls([
mock.call(self.server_details,
@ -597,7 +606,9 @@ class CIFSHelperUserAccessTestCase(test.TestCase):
self._helper.update_access,
self.server_details,
self.share_name,
access_rules)
access_rules,
[],
[])
@ddt.ddt

View File

@ -529,7 +529,9 @@ class ShareDriverTestCase(test.TestCase):
share_driver.update_access,
'ctx',
'fake_share',
'fake_access_rules'
'fake_access_rules',
'fake_add_rules',
'fake_delete_rules'
)
def test_create_replica(self):