openstack/deb-sahara
Code Issues Proposed changes

Check ACLs before adding access for Manila share

Browse Source 
On investigation, Manila access calls are non-idempotent.
For this reason, it is vital that we assess the presence
or absence of pre-existing ACLs for any one instance before
calling share.allow(..., ip, ...) Otherwise subsequent
attempts to idempotently verify or add shares to a cluster
will fail.

Change-Id: Ib471876f896523856141434b343a743a0f357f84
Closes-bug: 1481849
This commit is contained in:
Ethan Gafford 2015-08-05 14:23:45 -04:00
parent 18552b720b
commit dc4118c22c
2 changed files with 141 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
sahara/service/shares.py
View File

@ -153,7 +153,24 @@ class _ShareHandler(object):
def allow_access_to_instance(self, instance, share_config):
"""Mounts a specific share to a specific instance."""
access_level = self._get_access_level(share_config)
self.share.allow('ip', instance.internal_ip, access_level)
ip_filter = lambda x: (x.access_type == 'ip' and
x.access_to == instance.internal_ip)
accesses = list(filter(ip_filter, self.share.access_list()))
if accesses:
access = accesses[0]
if access.access_level not in ('ro', 'rw'):
LOG.warning(
_LW("Unknown permission level %(access_level)s on share "
"id %(share_id)s for ip %(ip)s. Leaving pre-existing "
"permissions.").format(
access_level=access.access_level,
share_id=self.share.id,
ip=instance.internal_ip))
elif access.access_level == 'ro' and access_level == 'rw':
self.share.deny(access.id)
self.share.allow('ip', instance.internal_ip, access_level)
else:
self.share.allow('ip', instance.internal_ip, access_level)
@abc.abstractmethod
def mount_to_instance(self, remote, share_info):

124
sahara/tests/unit/service/test_shares.py
View File

@ -31,11 +31,14 @@ class _FakeShare(object):
def __init__(self, id='12345678-1234-1234-1234-123456789012',
share_proto='NFS',
export_location='192.168.122.1:/path'):
export_location='192.168.122.1:/path',
access_list=None):
self.id = id
self.share_proto = share_proto
self.export_location = export_location
self.allow = mock.Mock()
self.deny = mock.Mock()
self.access_list = mock.Mock(return_value=access_list or [])
def _mock_node_group(ips, share_list):
@ -114,6 +117,7 @@ class TestShares(base.SaharaTestCase):
for executor in namenode_executors:
executor.assert_has_calls(
_setup_calls() +
_expected_calls('/mnt/localpath', '192.168.122.1:/path', '-w'))
for executor in datanode_executors:
self.assertEqual(0, executor.call_count)
@ -183,6 +187,7 @@ class TestShares(base.SaharaTestCase):
for executor in datanode_executors:
executor.assert_has_calls(
_setup_calls() +
_expected_calls('/mnt/somanylocalpaths',
'192.168.122.1:/path', '-r'))
self.assertEqual(4, executor.call_count)
@ -224,3 +229,120 @@ class TestShares(base.SaharaTestCase):
with testtools.ExpectedException(exceptions.NotFoundException):
shares.mount_shares(cluster)
@mock.patch('sahara.context.set_current_instance_id')
@mock.patch('sahara.utils.openstack.manila.client')
def test_acl_exists_unexpected_type(self, f_manilaclient, f_context):
share = _FakeShare(access_list=[mock.Mock(
access_level='wat', access_to=ip, access_type='ip')
for ip in _NAMENODE_IPS])
f_manilaclient.return_value = mock.Mock(
shares=mock.Mock(
get=mock.Mock(return_value=share)))
namenode_group, namenode_executors = _mock_node_group(
_NAMENODE_IPS,
[{
'id': '12345678-1234-1234-1234-123456789012',
'access_level': 'rw',
'path': '/mnt/localpath'
}])
datanode_group, datanode_executors = _mock_node_group(
_DATANODE_IPS, [])
cluster = mock.Mock(
node_groups=[namenode_group, datanode_group], shares=[])
shares.mount_shares(cluster)
self.assertEqual(0, share.allow.call_count)
for executor in namenode_executors:
executor.assert_has_calls(
_setup_calls() +
_expected_calls('/mnt/localpath', '192.168.122.1:/path', '-w'))
for executor in datanode_executors:
self.assertEqual(0, executor.call_count)
@mock.patch('sahara.context.set_current_instance_id')
@mock.patch('sahara.utils.openstack.manila.client')
def test_acl_exists_no_recreate(self, f_manilaclient, f_context):
share = _FakeShare(access_list=[mock.Mock(
access_level='rw', access_to=ip, access_type='ip')
for ip in _NAMENODE_IPS])
f_manilaclient.return_value = mock.Mock(
shares=mock.Mock(
get=mock.Mock(return_value=share)))
namenode_group, namenode_executors = _mock_node_group(
_NAMENODE_IPS,
[{
'id': '12345678-1234-1234-1234-123456789012',
'access_level': 'ro',
'path': '/mnt/localpath'
}])
datanode_group, datanode_executors = _mock_node_group(
_DATANODE_IPS, [])
cluster = mock.Mock(
node_groups=[namenode_group, datanode_group], shares=[])
shares.mount_shares(cluster)
self.assertEqual(0, share.allow.call_count)
for executor in namenode_executors:
executor.assert_has_calls(
_setup_calls() +
_expected_calls('/mnt/localpath', '192.168.122.1:/path', '-r'))
for executor in datanode_executors:
self.assertEqual(0, executor.call_count)
@mock.patch('sahara.context.set_current_instance_id')
@mock.patch('sahara.utils.openstack.manila.client')
def test_acl_exists_recreate(self, f_manilaclient, f_context):
share = _FakeShare(access_list=[mock.Mock(
access_level='ro', access_to=ip, access_type='ip', id="access_id")
for ip in _NAMENODE_IPS])
f_manilaclient.return_value = mock.Mock(
shares=mock.Mock(
get=mock.Mock(return_value=share)))
namenode_group, namenode_executors = _mock_node_group(
_NAMENODE_IPS,
[{
'id': '12345678-1234-1234-1234-123456789012',
'access_level': 'rw',
'path': '/mnt/localpath'
}])
datanode_group, datanode_executors = _mock_node_group(
_DATANODE_IPS, [])
cluster = mock.Mock(
node_groups=[namenode_group, datanode_group], shares=[])
shares.mount_shares(cluster)
namenode_denials = [mock.call('access_id')
for ip in _NAMENODE_IPS]
share.deny.assert_has_calls(namenode_denials)
namenode_permissions = [mock.call('ip', ip, 'rw')
for ip in _NAMENODE_IPS]
share.allow.assert_has_calls(namenode_permissions,
any_order=True)
for executor in namenode_executors:
executor.assert_has_calls(
_setup_calls() +
_expected_calls('/mnt/localpath', '192.168.122.1:/path', '-w'))
for executor in datanode_executors:
self.assertEqual(0, executor.call_count)