openstack/manila
Code Issues Proposed changes

EMC: Remove unnecessary parameter emc_share_driver

Browse Source 
The parameter emc_share_driver is designed to transfer Manila backend
configuration to EMC share plugins, and it is only used by the function
connect() in EMC Share driver framework. This submit is to remove the
unnecessary parameter emc_share_driver from other EMC driver framework APIs
and update VNX and isilon driver properly.

Change-Id: I66f1e30f98169b8fd94bafe0fb0ea0f29181cfd5
Closes-Bug: #1502375
This commit is contained in:
Jay Xu 2015-04-27 19:51:28 -04:00
parent 4ad103f5af
commit bd0825f8ba
6 changed files with 79 additions and 128 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
manila/share/drivers/emc/driver.py
View File

@ -81,8 +81,7 @@ class EMCShareDriver(driver.ShareDriver):
def create_share(self, context, share, share_server=None):
"""Is called to create share."""
location = self.plugin.create_share(self, context, share,
share_server)
location = self.plugin.create_share(context, share, share_server)
return location
@ -90,37 +89,33 @@ class EMCShareDriver(driver.ShareDriver):
share_server=None):
"""Is called to create share from snapshot."""
location = self.plugin.create_share_from_snapshot(
self, context, share, snapshot, share_server)
context, share, snapshot, share_server)
return location
def create_snapshot(self, context, snapshot, share_server=None):
"""Is called to create snapshot."""
self.plugin.create_snapshot(self, context, snapshot,
share_server)
self.plugin.create_snapshot(context, snapshot, share_server)
def delete_share(self, context, share, share_server=None):
"""Is called to remove share."""
self.plugin.delete_share(self, context, share, share_server)
self.plugin.delete_share(context, share, share_server)
def delete_snapshot(self, context, snapshot, share_server=None):
"""Is called to remove snapshot."""
self.plugin.delete_snapshot(self, context, snapshot,
share_server)
self.plugin.delete_snapshot(context, snapshot, share_server)
def ensure_share(self, context, share, share_server=None):
"""Invoked to sure that share is exported."""
self.plugin.ensure_share(self, context, share, share_server)
self.plugin.ensure_share(context, share, share_server)
def allow_access(self, context, share, access, share_server=None):
"""Allow access to the share."""
self.plugin.allow_access(self, context, share, access,
share_server)
self.plugin.allow_access(context, share, access, share_server)
def deny_access(self, context, share, access, share_server=None):
"""Deny access to the share."""
self.plugin.deny_access(self, context, share, access,
share_server)
self.plugin.deny_access(context, share, access, share_server)
def check_for_setup_error(self):
"""Check for setup error."""
@ -144,14 +139,12 @@ class EMCShareDriver(driver.ShareDriver):
def get_network_allocations_number(self):
"""Returns number of network allocations for creating VIFs."""
return self.plugin.get_network_allocations_number(self)
return self.plugin.get_network_allocations_number()
def _setup_server(self, network_info, metadata=None):
"""Set up and configures share server with given network parameters."""
return self.plugin.setup_server(self, network_info, metadata)
return self.plugin.setup_server(network_info, metadata)
def _teardown_server(self, server_details, security_services=None):
"""Teardown share server."""
return self.plugin.teardown_server(self,
server_details,
security_services)
return self.plugin.teardown_server(server_details, security_services)

20
manila/share/drivers/emc/plugins/base.py
View File

@ -30,38 +30,34 @@ class StorageConnection(object):
self.driver_handles_share_servers = None
@abc.abstractmethod
def create_share(self, emc_share_driver, context, share, share_server):
def create_share(self, context, share, share_server):
"""Is called to create share."""
@abc.abstractmethod
def create_snapshot(self, emc_share_driver, context,
snapshot, share_server):
def create_snapshot(self, context, snapshot, share_server):
"""Is called to create snapshot."""
@abc.abstractmethod
def delete_share(self, emc_share_driver, context, share, share_server):
def delete_share(self, context, share, share_server):
"""Is called to remove share."""
@abc.abstractmethod
def delete_snapshot(self, emc_share_driver, context,
snapshot, share_server):
def delete_snapshot(self, context, snapshot, share_server):
"""Is called to remove snapshot."""
@abc.abstractmethod
def ensure_share(self, emc_share_driver, context, share, share_server):
def ensure_share(self, context, share, share_server):
"""Invoked to ensure that share is exported."""
@abc.abstractmethod
def allow_access(self, emc_share_driver, context, share,
access, share_server):
def allow_access(self, context, share, access, share_server):
"""Allow access to the share."""
@abc.abstractmethod
def deny_access(self, emc_share_driver, context, share,
access, share_server):
def deny_access(self, context, share, access, share_server):
"""Deny access to the share."""
def raise_connect_error(self, emc_share_driver):
def raise_connect_error(self):
"""Check for setup error."""
pass

28
manila/share/drivers/emc/plugins/isilon/isilon.py
View File

@ -58,7 +58,7 @@ class IsilonStorageConnection(base.StorageConnection):
"""Return path to a container."""
return os.path.join(self._root_dir, share['name'])
def create_share(self, emc_share_driver, context, share, share_server):
def create_share(self, context, share, share_server):
"""Is called to create share."""
if share['share_proto'] == 'NFS':
location = self._create_nfs_share(share)
@ -71,13 +71,12 @@ class IsilonStorageConnection(base.StorageConnection):
raise exception.InvalidShare(message=message)
return location
def create_share_from_snapshot(self, emc_share_driver, context, share,
snapshot, share_server):
def create_share_from_snapshot(self, context, share, snapshot,
share_server):
"""Creates a share from the snapshot."""
# Create share at new location
location = self.create_share(
emc_share_driver, context, share, share_server)
location = self.create_share(context, share, share_server)
# Clone snapshot to new location
fq_target_dir = self._get_container_path(share)
@ -110,13 +109,12 @@ class IsilonStorageConnection(base.StorageConnection):
share_path = '\\\\{0}\\{1}'.format(self._server, share['name'])
return share_path
def create_snapshot(self, emc_share_driver, context,
snapshot, share_server):
def create_snapshot(self, context, snapshot, share_server):
"""Is called to create snapshot."""
snapshot_path = os.path.join(self._root_dir, snapshot['share_name'])
self._isilon_api.create_snapshot(snapshot['name'], snapshot_path)
def delete_share(self, emc_share_driver, context, share, share_server):
def delete_share(self, context, share, share_server):
"""Is called to remove share."""
if share['share_proto'] == 'NFS':
self._delete_nfs_share(share)
@ -159,16 +157,14 @@ class IsilonStorageConnection(base.StorageConnection):
LOG.error(message)
raise exception.ShareBackendException(message=message)
def delete_snapshot(self, emc_share_driver, context,
snapshot, share_server):
def delete_snapshot(self, context, snapshot, share_server):
"""Is called to remove snapshot."""
self._isilon_api.delete_snapshot(snapshot['name'])
def ensure_share(self, emc_share_driver, context, share, share_server):
def ensure_share(self, context, share, share_server):
"""Invoked to ensure that share is exported."""
def allow_access(self, emc_share_driver, context, share,
access, share_server):
def allow_access(self, context, share, access, share_server):
"""Allow access to the share."""
# TODO(sedwards): Look into supporting ro/rw access to shares
@ -225,8 +221,7 @@ class IsilonStorageConnection(base.StorageConnection):
r = self._isilon_api.request('PUT', url, data=data)
r.raise_for_status()
def deny_access(self, emc_share_driver, context, share,
access, share_server):
def deny_access(self, context, share, access, share_server):
"""Deny access to the share."""
if access['access_type'] != 'ip':
@ -322,7 +317,6 @@ class IsilonStorageConnection(base.StorageConnection):
"""Set up and configures share server with given network parameters."""
# TODO(Shaun Edwards): Look into supporting share servers
def teardown_server(self, server_details,
security_services=None):
def teardown_server(self, server_details, security_services=None):
"""Teardown share server."""
# TODO(Shaun Edwards): Look into supporting share servers

35
manila/share/drivers/emc/plugins/vnx/connection.py
View File

@ -50,8 +50,7 @@ class VNXStorageConnection(driver.StorageConnection):
self._filesystems = {}
self.driver_handles_share_servers = True
def create_share(self, emc_share_driver, context, share,
share_server=None):
def create_share(self, context, share, share_server=None):
"""Is called to create share."""
share_name = share['name']
size = share['size'] * units.Ki
@ -150,8 +149,8 @@ class VNXStorageConnection(driver.StorageConnection):
% {'nfs_if': share_server['backend_details']['nfs_if'],
'share_name': share_name})
def create_share_from_snapshot(self, emc_share_driver, context,
share, snapshot, share_server=None):
def create_share_from_snapshot(self, context, share, snapshot,
share_server=None):
"""Is called to create share from snapshot."""
share_name = share['name']
vdm_ref = self.share_server_validation(share_server)
@ -171,8 +170,7 @@ class VNXStorageConnection(driver.StorageConnection):
return location
def create_snapshot(self, emc_share_driver, context, snapshot,
share_server=None):
def create_snapshot(self, context, snapshot, share_server=None):
"""Create snapshot from share."""
ckpt_name = snapshot['name']
@ -189,8 +187,7 @@ class VNXStorageConnection(driver.StorageConnection):
LOG.error(message)
raise exception.EMCVnxXMLAPIError(err=message)
def delete_share(self, emc_share_driver, context, share,
share_server=None):
def delete_share(self, context, share, share_server=None):
"""Is called to remove share."""
if share_server is None:
LOG.warn(_LW("Driver does not support share deletion without "
@ -314,8 +311,7 @@ class VNXStorageConnection(driver.StorageConnection):
LOG.error(message)
raise exception.EMCVnxXMLAPIError(err=message)
def delete_snapshot(self, emc_share_driver, context, snapshot,
share_server=None):
def delete_snapshot(self, context, snapshot, share_server=None):
"""Remove share's snapshot."""
ckpt_name = snapshot['name']
@ -337,14 +333,11 @@ class VNXStorageConnection(driver.StorageConnection):
LOG.error(message)
raise exception.EMCVnxXMLAPIError(err=message)
def ensure_share(self, emc_share_driver,
context, share,
share_server=None):
def ensure_share(self, context, share, share_server=None):
"""Invoked to ensure that share is exported."""
pass
def allow_access(self, emc_share_driver, context, share, access,
share_server=None):
def allow_access(self, context, share, access, share_server=None):
"""Allow access to the share."""
access_level = access['access_level']
if access_level not in const.ACCESS_LEVELS:
@ -407,8 +400,7 @@ class VNXStorageConnection(driver.StorageConnection):
LOG.error(message)
raise exception.EMCVnxXMLAPIError(err=message)
def deny_access(self, emc_share_driver, context, share, access,
share_server=None):
def deny_access(self, context, share, access, share_server=None):
"""Deny access to the share."""
if share['share_proto'] == 'NFS':
self._nfs_deny_access(share, access, share_server)
@ -468,7 +460,7 @@ class VNXStorageConnection(driver.StorageConnection):
LOG.error(message)
raise exception.EMCVnxXMLAPIError(err=message)
def check_for_setup_error(self, emc_share_driver):
def check_for_setup_error(self):
"""Check for setup error."""
pass
@ -500,11 +492,11 @@ class VNXStorageConnection(driver.StorageConnection):
stats_dict['free_capacity_gb'] = (
int(pool['total_size']) - int(pool['used_size']))
def get_network_allocations_number(self, emc_share_driver):
def get_network_allocations_number(self):
"""Returns number of network allocations for creating VIFs."""
return constants.IP_ALLOCATIONS
def setup_server(self, emc_share_driver, network_info, metadata=None):
def setup_server(self, network_info, metadata=None):
"""Set up and configures share server with given network parameters."""
# Only support single security service with type 'active_directory'
interface_info = []
@ -675,8 +667,7 @@ class VNXStorageConnection(driver.StorageConnection):
self._NASCmd_helper.enable_nfs_service(vdmRef['name'],
interface['name'])
def teardown_server(self, emc_share_driver, server_details,
security_services=None):
def teardown_server(self, server_details, security_services=None):
"""Teardown share server."""
if not server_details:
LOG.debug('Server details are empty.')

75
manila/tests/share/drivers/emc/plugins/isilon/test_isilon.py
View File

@ -79,8 +79,7 @@ class IsilonTest(test.TestCase):
self.assertFalse(self._mock_isilon_api.request.called)
# call method under test
self.storage_connection.allow_access(self.mock_emc_driver,
self.mock_context, share, access,
self.storage_connection.allow_access(self.mock_context, share, access,
share_server)
# verify expected REST API call is executed
@ -105,8 +104,7 @@ class IsilonTest(test.TestCase):
# call method under test
self.assertFalse(self._mock_isilon_api.request.called)
self.storage_connection.deny_access(self.mock_emc_driver,
self.mock_context, share, access,
self.storage_connection.deny_access(self.mock_context, share, access,
share_server)
# verify that a call is made to remove an existing IP from the list
@ -134,8 +132,7 @@ class IsilonTest(test.TestCase):
# call method under test
access = {'access_type': 'ip', 'access_to': ip_addr}
share_server = None
self.storage_connection.deny_access(self.mock_emc_driver,
self.mock_context, share, access,
self.storage_connection.deny_access(self.mock_context, share, access,
share_server)
# verify API call is made to remove IP is removed from whitelist
@ -151,7 +148,7 @@ class IsilonTest(test.TestCase):
# This operation should return silently
self.storage_connection.deny_access(
self.mock_emc_driver, self.mock_context, share, access, None)
self.mock_context, share, access, None)
def test_deny_access_invalid_share_protocol(self):
share = {'name': self.SHARE_NAME, 'share_proto': 'FOO'}
@ -159,7 +156,7 @@ class IsilonTest(test.TestCase):
# This operation should return silently
self.storage_connection.deny_access(
self.mock_emc_driver, self.mock_context, share, access, None)
self.mock_context, share, access, None)
def test_deny_access_nfs_export_does_not_exist(self):
share = {'name': self.SHARE_NAME, 'share_proto': 'NFS'}
@ -169,7 +166,7 @@ class IsilonTest(test.TestCase):
self.assertRaises(
exception.ShareBackendException,
self.storage_connection.deny_access, self.mock_emc_driver,
self.storage_connection.deny_access,
self.mock_context, share, access, None
)
@ -180,7 +177,7 @@ class IsilonTest(test.TestCase):
self.assertRaises(
exception.ShareBackendException,
self.storage_connection.deny_access, self.mock_emc_driver,
self.storage_connection.deny_access,
self.mock_context, share, access, None)
def test_allow_access_multiple_ip_nfs(self):
@ -204,8 +201,7 @@ class IsilonTest(test.TestCase):
share = {'name': self.SHARE_NAME, 'share_proto': 'NFS'}
access = {'access_type': 'ip', 'access_to': new_allowed_ip}
share_server = None
self.storage_connection.allow_access(self.mock_emc_driver,
self.mock_context, share,
self.storage_connection.allow_access(self.mock_context, share,
access,
share_server)
@ -244,8 +240,7 @@ class IsilonTest(test.TestCase):
share = {'name': share_name, 'share_proto': 'CIFS'}
access = {'access_type': 'ip', 'access_to': new_allowed_ip}
share_server = None
self.storage_connection.allow_access(self.mock_emc_driver,
self.mock_context, share,
self.storage_connection.allow_access(self.mock_context, share,
access,
share_server)
@ -276,8 +271,7 @@ class IsilonTest(test.TestCase):
self.assertFalse(self._mock_isilon_api.request.called)
# call method under test
self.storage_connection.allow_access(self.mock_emc_driver,
self.mock_context, share, access,
self.storage_connection.allow_access(self.mock_context, share, access,
share_server)
# verify access rule is applied
@ -297,7 +291,7 @@ class IsilonTest(test.TestCase):
# verify method under test throws the expected exception
self.assertRaises(
exception.ShareBackendException,
self.storage_connection.allow_access, self.mock_emc_driver,
self.storage_connection.allow_access,
self.mock_context, share, access, None)
def test_allow_access_invalid_share_protocol(self):
@ -310,7 +304,7 @@ class IsilonTest(test.TestCase):
# verify method under test throws the expected exception
self.assertRaises(
exception.InvalidShare, self.storage_connection.allow_access,
self.mock_emc_driver, self.mock_context, share, access, None)
self.mock_context, share, access, None)
def test_create_share_nfs(self):
share_path = self.SHARE_DIR
@ -319,8 +313,7 @@ class IsilonTest(test.TestCase):
# create the share
share = {"name": self.SHARE_NAME, "share_proto": 'NFS'}
location = self.storage_connection.create_share(self.mock_emc_driver,
self.mock_context,
location = self.storage_connection.create_share(self.mock_context,
share, None)
# verify location and API call made
@ -335,8 +328,7 @@ class IsilonTest(test.TestCase):
# create the share
share = {"name": self.SHARE_NAME, "share_proto": 'CIFS'}
location = self.storage_connection.create_share(self.mock_emc_driver,
self.mock_context,
location = self.storage_connection.create_share(self.mock_context,
share, None)
expected_location = '\\\\{0}\\{1}'.format(
@ -352,7 +344,7 @@ class IsilonTest(test.TestCase):
self.assertRaises(
exception.InvalidShare, self.storage_connection.create_share,
self.mock_emc_driver, self.mock_context, share, share_server=None)
self.mock_context, share, share_server=None)
def test_create_share_nfs_backend_failure(self):
share = {"name": self.SHARE_NAME, "share_proto": 'NFS'}
@ -360,7 +352,7 @@ class IsilonTest(test.TestCase):
self.assertRaises(
exception.ShareBackendException,
self.storage_connection.create_share, self.mock_emc_driver,
self.storage_connection.create_share,
self.mock_context, share, share_server=None)
def test_create_snapshot(self):
@ -369,8 +361,7 @@ class IsilonTest(test.TestCase):
snapshot_name = "snapshot01"
snapshot_path = '/ifs/home/admin'
snapshot = {'name': snapshot_name, 'share_name': snapshot_path}
self.storage_connection.create_snapshot(self.mock_emc_driver,
self.mock_context, snapshot,
self.storage_connection.create_snapshot(self.mock_context, snapshot,
None)
# verify the create snapshot API call is executed
@ -389,10 +380,7 @@ class IsilonTest(test.TestCase):
snapshot = {'name': snapshot_name, 'share_name': snapshot_path}
share = {"name": self.SHARE_NAME, "share_proto": 'NFS'}
location = self.storage_connection.create_share_from_snapshot(
self.mock_emc_driver,
self.mock_context,
share, snapshot,
None)
self.mock_context, share, snapshot, None)
# verify NFS export created at expected location
self._mock_isilon_api.create_nfs_export.assert_called_with(
@ -418,8 +406,7 @@ class IsilonTest(test.TestCase):
snapshot = {'name': snapshot_name, 'share_name': snapshot_path}
share = {"name": new_share_name, "share_proto": 'CIFS'}
location = self.storage_connection.create_share_from_snapshot(
self.mock_emc_driver, self.mock_context, share, snapshot,
None)
self.mock_context, share, snapshot, None)
# verify call made to create new CIFS share
self._mock_isilon_api.create_smb_share.assert_called_once_with(
@ -437,8 +424,7 @@ class IsilonTest(test.TestCase):
self.assertFalse(self._mock_isilon_api.delete_nfs_share.called)
# delete the share
self.storage_connection.delete_share(self.mock_emc_driver,
self.mock_context, share, None)
self.storage_connection.delete_share(self.mock_context, share, None)
# verify share delete
self._mock_isilon_api.delete_nfs_share.assert_called_with(
@ -449,8 +435,7 @@ class IsilonTest(test.TestCase):
# delete the share
share = {"name": self.SHARE_NAME, "share_proto": 'CIFS'}
self.storage_connection.delete_share(self.mock_emc_driver,
self.mock_context, share, None)
self.storage_connection.delete_share(self.mock_context, share, None)
# verify share deleted
self._mock_isilon_api.delete_smb_share.assert_called_with(
@ -460,7 +445,7 @@ class IsilonTest(test.TestCase):
share = {"name": self.SHARE_NAME, "share_proto": 'FOO_PROTOCOL'}
self.assertRaises(
exception.InvalidShare, self.storage_connection.delete_share,
self.mock_emc_driver, self.mock_context, share, None
self.mock_context, share, None
)
def test_delete_nfs_share_backend_failure(self):
@ -470,7 +455,7 @@ class IsilonTest(test.TestCase):
self.assertRaises(
exception.ShareBackendException,
self.storage_connection.delete_share,
self.mock_emc_driver, self.mock_context, share, None
self.mock_context, share, None
)
def test_delete_nfs_share_share_does_not_exist(self):
@ -479,8 +464,7 @@ class IsilonTest(test.TestCase):
# verify the calling delete on a non-existent share returns and does
# not throw exception
self.storage_connection.delete_share(
self.mock_emc_driver, self.mock_context, share, None)
self.storage_connection.delete_share(self.mock_context, share, None)
def test_delete_cifs_share_backend_failure(self):
share = {"name": self.SHARE_NAME, "share_proto": 'CIFS'}
@ -489,7 +473,7 @@ class IsilonTest(test.TestCase):
self.assertRaises(
exception.ShareBackendException,
self.storage_connection.delete_share,
self.mock_emc_driver, self.mock_context, share, None
self.mock_context, share, None
)
def test_delete_cifs_share_share_does_not_exist(self):
@ -498,8 +482,7 @@ class IsilonTest(test.TestCase):
# verify the calling delete on a non-existent share returns and does
# not throw exception
self.storage_connection.delete_share(
self.mock_emc_driver, self.mock_context, share, None)
self.storage_connection.delete_share(self.mock_context, share, None)
def test_delete_snapshot(self):
# create a snapshot
@ -509,8 +492,7 @@ class IsilonTest(test.TestCase):
self.assertFalse(self._mock_isilon_api.delete_snapshot.called)
# delete the created snapshot
self.storage_connection.delete_snapshot(self.mock_emc_driver,
self.mock_context, snapshot,
self.storage_connection.delete_snapshot(self.mock_context, snapshot,
None)
# verify the API call was made to delete the snapshot
@ -519,8 +501,7 @@ class IsilonTest(test.TestCase):
def test_ensure_share(self):
share = {"name": self.SHARE_NAME, "share_proto": 'CIFS'}
self.storage_connection.ensure_share(self.mock_emc_driver,
self.mock_context, share, None)
self.storage_connection.ensure_share(self.mock_context, share, None)
@mock.patch(
'manila.share.drivers.emc.plugins.isilon.isilon.isilon_api.IsilonApi',

20
manila/tests/share/drivers/emc/test_driver.py
View File

@ -34,39 +34,35 @@ class FakeConnection(base.StorageConnection):
def driver_handles_share_servers(self):
return True
def create_share(self, emc_share_driver, context, share, share_server):
def create_share(self, context, share, share_server):
"""Is called to create share."""
pass
def create_snapshot(self, emc_share_driver, context,
snapshot, share_server):
def create_snapshot(self, context, snapshot, share_server):
"""Is called to create snapshot."""
pass
def delete_share(self, emc_share_driver, context, share, share_server):
def delete_share(self, context, share, share_server):
"""Is called to remove share."""
pass
def delete_snapshot(self, emc_share_driver, context,
snapshot, share_server):
def delete_snapshot(self, context, snapshot, share_server):
"""Is called to remove snapshot."""
pass
def ensure_share(self, emc_share_driver, context, share, share_server):
def ensure_share(self, context, share, share_server):
"""Invoked to sure that share is exported."""
pass
def allow_access(self, emc_share_driver, context, share,
access, share_server):
def allow_access(self, context, share, access, share_server):
"""Allow access to the share."""
pass
def deny_access(self, emc_share_driver, context, share,
access, share_server):
def deny_access(self, context, share, access, share_server):
"""Deny access to the share."""
pass
def raise_connect_error(self, emc_share_driver):
def raise_connect_error(self):
"""Check for setup error."""
pass