openstack/manila
Code Issues Proposed changes

EMC Isilon Driver Support For NFS Read-Only Share

Browse Source 
Added support for read-only shares to the NFS protocol in the Isilon
driver. Fixed parameters in the exceptions.

Change-Id: Idf8b8772e30d55667317de9f50117968d34ee877
Implements: blueprint emc-isilon-driver-readonly-share
This commit is contained in:
Shaun Edwards
2016-01-21 13:25:43 -08:00
parent e5f4e981f5
commit 20efd1046d
3 changed files with 157 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
doc/source/devref/share_back_ends_feature_support_mapping.rst
View File

@@ -82,7 +82,7 @@ Mapping of share drivers and share access rules support
+----------------------------------------+--------------+----------------+------------+--------------+----------------+------------+
| EMC VNX | NFS (J) | CIFS (J) | \- | NFS (L) | CIFS (L) | \- |
+----------------------------------------+--------------+----------------+------------+--------------+----------------+------------+
| EMC Isilon | NFS,CIFS (K) | \- | \- | \- | \- | \- |
| EMC Isilon | NFS,CIFS (K) | \- | \- | NFS (M) | \- | \- |
+----------------------------------------+--------------+----------------+------------+--------------+----------------+------------+
| Red Hat GlusterFS | NFS (J) | \- | \- | \- | \- | \- |
+----------------------------------------+--------------+----------------+------------+--------------+----------------+------------+

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

@@ -23,6 +23,7 @@ from oslo_log import log
from oslo_utils import units
import six
from manila.common import constants as const
from manila import exception
from manila.i18n import _, _LW
from manila.share.drivers.emc.plugins import base
@@ -69,7 +70,7 @@ class IsilonStorageConnection(base.StorageConnection):
message = (_('Unsupported share protocol: %(proto)s.') %
{'proto': share['share_proto']})
LOG.error(message)
raise exception.InvalidShare(message=message)
raise exception.InvalidShare(reason=message)
# apply directory quota based on share size
max_share_size = share['size'] * units.Gi
@@ -102,7 +103,7 @@ class IsilonStorageConnection(base.StorageConnection):
_('The requested NFS share "%(share)s" was not created.') %
{'share': share['name']})
LOG.error(message)
raise exception.ShareBackendException(message=message)
raise exception.ShareBackendException(msg=message)
location = '{0}:{1}'.format(self._server, container_path)
return location
@@ -131,7 +132,7 @@ class IsilonStorageConnection(base.StorageConnection):
message = (_('Unsupported share type: %(type)s.') %
{'type': share['share_proto']})
LOG.error(message)
raise exception.InvalidShare(message=message)
raise exception.InvalidShare(reason=message)
def _delete_nfs_share(self, share):
"""Is called to remove nfs share."""
@@ -148,7 +149,7 @@ class IsilonStorageConnection(base.StorageConnection):
if not export_deleted:
message = _('Error deleting NFS share: %s') % share['name']
LOG.error(message)
raise exception.ShareBackendException(message=message)
raise exception.ShareBackendException(msg=message)
def _delete_cifs_share(self, share):
"""Is called to remove CIFS share."""
@@ -162,7 +163,7 @@ class IsilonStorageConnection(base.StorageConnection):
if not share_deleted:
message = _('Error deleting CIFS share: %s') % share['name']
LOG.error(message)
raise exception.ShareBackendException(message=message)
raise exception.ShareBackendException(msg=message)
def delete_snapshot(self, context, snapshot, share_server):
"""Is called to remove snapshot."""
@@ -184,45 +185,57 @@ class IsilonStorageConnection(base.StorageConnection):
if access['access_type'] != 'ip':
message = _('Only ip access type allowed.')
LOG.error(message)
raise exception.ShareBackendException(message=message)
raise exception.ShareBackendException(msg=message)
access_ip = access['access_to']
if share['share_proto'] == 'NFS':
export_path = self._get_container_path(share)
self._nfs_allow_access(access_ip, export_path)
self._nfs_allow_access(
access_ip, export_path, access['access_level'])
elif share['share_proto'] == 'CIFS':
self._cifs_allow_access(access_ip, share)
self._cifs_allow_access(
access_ip, share, access['access_level'])
else:
message = _(
'Unsupported share protocol: %s. Only "NFS" and '
'"CIFS" are currently supported share protocols.') % share[
'share_proto']
LOG.error(message)
raise exception.InvalidShare(message=message)
raise exception.InvalidShare(reason=message)
def _nfs_allow_access(self, access_ip, export_path):
def _nfs_allow_access(self, access_ip, export_path, access_level):
"""Allow access to nfs share."""
share_id = self._isilon_api.lookup_nfs_export(export_path)
share_access_group = 'clients'
if access_level == const.ACCESS_LEVEL_RO:
share_access_group = 'read_only_clients'
# Get current allowed clients
export = self._isilon_api.get_nfs_export(share_id)
current_clients = export['clients']
export = self._get_existing_nfs_export(share_id)
current_clients = export[share_access_group]
# Format of ips could be '10.0.0.2', or '10.0.0.2, 10.0.0.0/24'
ips = list()
ips.append(access_ip)
ips.extend(current_clients)
export_params = {"clients": ips}
export_params = {share_access_group: ips}
url = '{0}/platform/1/protocols/nfs/exports/{1}'.format(
self._server_url, share_id)
resp = self._isilon_api.request('PUT', url, data=export_params)
resp.raise_for_status()
def _cifs_allow_access(self, ip, share):
def _cifs_allow_access(self, ip, share, access_level):
"""Allow access to cifs share."""
if access_level == const.ACCESS_LEVEL_RO:
message = _('Only RW Access allowed for CIFS Protocol when using '
'the "ip" access type.')
LOG.error(message)
raise exception.InvalidShareAccess(reason=message)
allowed_ip = 'allow:' + ip
smb_share = self._isilon_api.lookup_smb_share(share['name'])
host_acl = smb_share['host_acl']
@@ -241,14 +254,19 @@ class IsilonStorageConnection(base.StorageConnection):
return
denied_ip = access['access_to']
access_level = access['access_level']
if share['share_proto'] == 'NFS':
self._nfs_deny_access(denied_ip, share)
self._nfs_deny_access(denied_ip, share, access_level)
elif share['share_proto'] == 'CIFS':
self._cifs_deny_access(denied_ip, share)
def _nfs_deny_access(self, denied_ip, share):
def _nfs_deny_access(self, denied_ip, share, access_level):
"""Deny access to nfs share."""
share_access_group = 'clients'
if access_level == const.ACCESS_LEVEL_RO:
share_access_group = 'read_only_clients'
# Get list of currently allowed client ips
export_id = self._isilon_api.lookup_nfs_export(
self._get_container_path(share))
@@ -256,27 +274,36 @@ class IsilonStorageConnection(base.StorageConnection):
message = _('Share %s should have been created, but was not '
'found.') % share['name']
LOG.error(message)
raise exception.ShareBackendException(message=message)
clients = self._get_nfs_ip_access_list(export_id)
raise exception.ShareBackendException(msg=message)
export = self._get_existing_nfs_export(export_id)
try:
clients = export[share_access_group]
except KeyError:
message = (_('Export %(export_name)s should have contained the '
'JSON key %(json_key)s, but this key was not found.')
% {'export_name': share['name'],
'json_key': share_access_group})
LOG.error(message)
raise exception.ShareBackendException(msg=message)
allowed_ips = set(clients)
if allowed_ips.__contains__(denied_ip):
allowed_ips.remove(denied_ip)
data = {"clients": list(allowed_ips)}
data = {share_access_group: list(allowed_ips)}
url = ('{0}/platform/1/protocols/nfs/exports/{1}'
.format(self._server_url, six.text_type(export_id)))
r = self._isilon_api.request('PUT', url, data=data)
r.raise_for_status()
def _get_nfs_ip_access_list(self, export_id):
def _get_existing_nfs_export(self, export_id):
export = self._isilon_api.get_nfs_export(export_id)
if export is None:
message = _('NFS share with export id %d should have been '
'created, but was not found.') % export_id
LOG.error(message)
raise exception.ShareBackendException(message=message)
raise exception.ShareBackendException(msg=message)
return export["clients"]
return export
def _cifs_deny_access(self, denied_ip, share):
"""Deny access to cifs share."""

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

@@ -16,7 +16,9 @@
import mock
from oslo_log import log
from oslo_utils import units
import six
from manila.common import constants as const
from manila import exception
from manila.share.drivers.emc.plugins.isilon import isilon
from manila import test
@@ -72,11 +74,13 @@ class IsilonTest(test.TestCase):
def test_allow_access_single_ip_nfs(self):
# setup
share = {'name': self.SHARE_NAME, 'share_proto': 'NFS'}
access = {'access_type': 'ip', 'access_to': '10.1.1.10'}
access = {'access_type': 'ip', 'access_to': '10.1.1.10',
'access_level': const.ACCESS_LEVEL_RW}
share_server = None
fake_export_id = 1
self._mock_isilon_api.lookup_nfs_export.return_value = fake_export_id
self._mock_isilon_api.get_nfs_export.return_value = {'clients': []}
self._mock_isilon_api.get_nfs_export.return_value = {
'clients': []}
self.assertFalse(self._mock_isilon_api.request.called)
# call method under test
@@ -90,7 +94,59 @@ class IsilonTest(test.TestCase):
self._mock_isilon_api.request.assert_called_once_with(
'PUT', expected_url, data=expected_data)
def test_deny_access_ip_nfs(self):
def test_allow_access_with_nfs_readonly(self):
# setup
share = {'name': self.SHARE_NAME, 'share_proto': 'NFS'}
access = {'access_type': 'ip', 'access_to': '10.1.1.10',
'access_level': const.ACCESS_LEVEL_RO}
fake_export_id = 70
self._mock_isilon_api.lookup_nfs_export.return_value = fake_export_id
self._mock_isilon_api.get_nfs_export.return_value = {
'read_only_clients': []}
self.assertFalse(self._mock_isilon_api.request.called)
self.storage_connection.allow_access(
self.mock_context, share, access, None)
# verify expected REST API call is executed
expected_url = (self.API_URL + '/platform/1/protocols/nfs/exports/' +
six.text_type(fake_export_id))
expected_data = {'read_only_clients': ['10.1.1.10']}
self._mock_isilon_api.request.assert_called_once_with(
'PUT', expected_url, data=expected_data)
def test_allow_access_with_nfs_readwrite(self):
# setup
share = {'name': self.SHARE_NAME, 'share_proto': 'NFS'}
access = {'access_type': 'ip', 'access_to': '10.1.1.10',
'access_level': const.ACCESS_LEVEL_RW}
fake_export_id = 70
self._mock_isilon_api.lookup_nfs_export.return_value = fake_export_id
self._mock_isilon_api.get_nfs_export.return_value = {
'clients': []}
self.assertFalse(self._mock_isilon_api.request.called)
self.storage_connection.allow_access(
self.mock_context, share, access, None)
# verify expected REST API call is executed
expected_url = (self.API_URL + '/platform/1/protocols/nfs/exports/' +
six.text_type(fake_export_id))
expected_data = {'clients': ['10.1.1.10']}
self._mock_isilon_api.request.assert_called_once_with(
'PUT', expected_url, data=expected_data)
def test_allow_access_with_cifs_ip_readonly(self):
# Note: Driver does not currently support readonly access for "ip" type
share = {'name': self.SHARE_NAME, 'share_proto': 'CIFS'}
access = {'access_type': 'ip', 'access_to': '10.1.1.10',
'access_level': const.ACCESS_LEVEL_RO}
self.assertRaises(
exception.InvalidShareAccess, self.storage_connection.allow_access,
self.mock_context, share, access, None)
def test_deny_access__ip_nfs_readwrite(self):
"""Verifies that an IP will be remove from a whitelist."""
fake_export_id = 1
self._mock_isilon_api.lookup_nfs_export.return_value = fake_export_id
@@ -100,7 +156,8 @@ class IsilonTest(test.TestCase):
'clients': [ip_addr]}
share = {'name': self.SHARE_NAME, 'share_proto': 'NFS'}
access = {'access_type': 'ip', 'access_to': ip_addr}
access = {'access_type': 'ip', 'access_to': ip_addr,
'access_level': const.ACCESS_LEVEL_RW}
share_server = None
# call method under test
@@ -116,6 +173,32 @@ class IsilonTest(test.TestCase):
'PUT', expected_url, data=expected_data
)
def test_deny_access__nfs_ip_readonly(self):
fake_export_id = 1
self._mock_isilon_api.lookup_nfs_export.return_value = fake_export_id
# simulate an IP added to the whitelist
ip_addr = '10.0.0.4'
self._mock_isilon_api.get_nfs_export.return_value = {
'read_only_clients': [ip_addr]}
share = {'name': self.SHARE_NAME, 'share_proto': 'NFS'}
access = {'access_type': 'ip', 'access_to': ip_addr,
'access_level': const.ACCESS_LEVEL_RO}
share_server = None
# call method under test
self.assertFalse(self._mock_isilon_api.request.called)
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
expected_url = (self.API_URL + '/platform/1/protocols/nfs/exports/' +
six.text_type(fake_export_id))
expected_data = {'read_only_clients': []}
self._mock_isilon_api.request.assert_called_once_with(
'PUT', expected_url, data=expected_data
)
def test_deny_access_ip_cifs(self):
"""Verifies that an IP will be remove from a whitelist.
@@ -131,7 +214,8 @@ class IsilonTest(test.TestCase):
self.assertFalse(self._mock_isilon_api.request.called)
# call method under test
access = {'access_type': 'ip', 'access_to': ip_addr}
access = {'access_type': 'ip', 'access_to': ip_addr,
'access_level': const.ACCESS_LEVEL_RW}
share_server = None
self.storage_connection.deny_access(self.mock_context, share, access,
share_server)
@@ -153,7 +237,8 @@ class IsilonTest(test.TestCase):
def test_deny_access_invalid_share_protocol(self):
share = {'name': self.SHARE_NAME, 'share_proto': 'FOO'}
access = {'access_type': 'ip', 'access_to': '10.0.0.1'}
access = {'access_type': 'ip', 'access_to': '10.0.0.1',
'access_level': const.ACCESS_LEVEL_RW}
# This operation should return silently
self.storage_connection.deny_access(
@@ -161,7 +246,8 @@ class IsilonTest(test.TestCase):
def test_deny_access_nfs_export_does_not_exist(self):
share = {'name': self.SHARE_NAME, 'share_proto': 'NFS'}
access = {'access_type': 'ip', 'access_to': '10.0.0.1'}
access = {'access_type': 'ip', 'access_to': '10.0.0.1',
'access_level': const.ACCESS_LEVEL_RW}
self._mock_isilon_api.lookup_nfs_export.return_value = 1
self._mock_isilon_api.get_nfs_export.return_value = None
@@ -173,7 +259,8 @@ class IsilonTest(test.TestCase):
def test_deny_access_nfs_share_does_not_exist(self):
share = {'name': self.SHARE_NAME, 'share_proto': 'NFS'}
access = {'access_type': 'ip', 'access_to': '10.0.0.1'}
access = {'access_type': 'ip', 'access_to': '10.0.0.1',
'access_level': const.ACCESS_LEVEL_RW}
self._mock_isilon_api.lookup_nfs_export.return_value = None
self.assertRaises(
@@ -194,17 +281,20 @@ class IsilonTest(test.TestCase):
new_allowed_ip = '10.7.7.8'
self._mock_isilon_api.lookup_nfs_export.return_value = fake_export_id
existing_ips = ['10.0.0.1', '10.1.1.1', '10.0.0.2']
export_json = {'clients': existing_ips}
export_json = {
'clients': existing_ips,
'access_level': const.ACCESS_LEVEL_RW,
}
self._mock_isilon_api.get_nfs_export.return_value = export_json
self.assertFalse(self._mock_isilon_api.request.called)
# call method under test
share = {'name': self.SHARE_NAME, 'share_proto': 'NFS'}
access = {'access_type': 'ip', 'access_to': new_allowed_ip}
access = {'access_type': 'ip', 'access_to': new_allowed_ip,
'access_level': const.ACCESS_LEVEL_RW}
share_server = None
self.storage_connection.allow_access(self.mock_context, share,
access,
share_server)
self.storage_connection.allow_access(
self.mock_context, share, access, share_server)
# verify access rule is applied
expected_url = (self.API_URL + '/platform/1/protocols/nfs/exports/' +
@@ -239,7 +329,8 @@ class IsilonTest(test.TestCase):
# call method under test
share = {'name': share_name, 'share_proto': 'CIFS'}
access = {'access_type': 'ip', 'access_to': new_allowed_ip}
access = {'access_type': 'ip', 'access_to': new_allowed_ip,
'access_level': const.ACCESS_LEVEL_RW}
share_server = None
self.storage_connection.allow_access(self.mock_context, share,
access,
@@ -265,7 +356,8 @@ class IsilonTest(test.TestCase):
share_name = self.SHARE_NAME
share = {'name': share_name, 'share_proto': 'CIFS'}
allow_ip = '10.1.1.10'
access = {'access_type': 'ip', 'access_to': allow_ip}
access = {'access_type': 'ip', 'access_to': allow_ip,
'access_level': const.ACCESS_LEVEL_RW}
share_server = None
self._mock_isilon_api.lookup_smb_share.return_value = {
'name': share_name, 'host_acl': []}