openstack/manila
Code Issues Proposed changes

service_instance: Add lock to creation of security_group

Browse Source 
service_instance module, that is used by generic driver
does not have lock for creation of security_group.
This causes concurrent creation of security groups
in multibackend installation.

Change-Id: I00c1ab8a35bed3da3e46c20ffaf220cde26f7fcd
Closes-Bug: #1334598
This commit is contained in:
vponomaryov 2014-06-26 13:30:18 +03:00
parent 47cb0fa78c
commit 2368e23c04
2 changed files with 110 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
manila/share/drivers/service_instance.py
View File

@ -219,19 +219,42 @@ class ServiceInstanceManager(object):
LOG.error(msg)
raise exception.ServiceInstanceException(msg)
@lockutils.synchronized("_get_service_instance_security_group",
@lockutils.synchronized("_get_or_create_security_group",
external=True, lock_path="service_instance_locks")
def _get_service_instance_security_group(self, context, name=None):
"""Searches for security group by name."""
if not (name or self.get_config_option(
"service_instance_security_group")):
return None
def _get_or_create_security_group(self, context, name=None,
description=None):
"""Get or create security group for service_instance.
:param context: context, that should be used
:param name: this is used for selection/creation of sec.group
:param description: this is used on sec.group creation step only
:returns: SecurityGroup -- security group instance from Nova
:raises: exception.ServiceInstanceException.
"""
name = name or self.get_config_option(
"service_instance_security_group")
if not name:
name = self.get_config_option("service_instance_security_group")
LOG.warning(_("Name for service instance security group is not "
"provided. Skipping security group step."))
return None
s_groups = [s for s in self.compute_api.security_group_list(context)
if s.name == name]
if not s_groups:
return None
# Creating security group
if not description:
description = "This security group is intended "\
"to be used by share service."
LOG.debug("Creating security group with name '%s'." % name)
sg = self.compute_api.security_group_create(
context, name, description)
for protocol, ports in constants.SERVICE_INSTANCE_SECGROUP_DATA:
self.compute_api.security_group_rule_create(context,
parent_group_id=sg.id,
ip_protocol=protocol,
from_port=ports[0],
to_port=ports[1],
cidr="0.0.0.0/0",
)
elif len(s_groups) > 1:
msg = _("Ambiguous security_groups.")
raise exception.ServiceInstanceException(msg)
@ -239,26 +262,6 @@ class ServiceInstanceManager(object):
sg = s_groups[0]
return sg
def _create_service_instance_security_group(self, context,
name=None, description=None):
"""Creates and returns security_group for service vm."""
if not name:
name = self.get_config_option("service_instance_security_group")
if not description:
description = "This security group is intended to "\
"be used by share service."
LOG.debug("Creating security group with name '%s'." % name)
sg = self.compute_api.security_group_create(context, name, description)
for protocol, ports in constants.SERVICE_INSTANCE_SECGROUP_DATA:
self.compute_api.security_group_rule_create(context,
parent_group_id=sg.id,
ip_protocol=protocol,
from_port=ports[0],
to_port=ports[1],
cidr="0.0.0.0/0",
)
return sg
def ensure_service_instance(self, context, server):
"""Ensures that server exists and active."""
try:
@ -378,11 +381,7 @@ class ServiceInstanceManager(object):
raise exception.ServiceInstanceException(_('Neither service '
'instance password nor key are available.'))
security_group = self._get_service_instance_security_group(context)
if not security_group and \
self.get_config_option("service_instance_security_group"):
security_group = self._create_service_instance_security_group(
context)
security_group = self._get_or_create_security_group(context)
port = self._setup_network_for_instance(share_network_id)
try:
self._setup_connectivity_with_service_instances()

127
manila/tests/test_service_instance.py
View File

@ -123,62 +123,91 @@ class ServiceInstanceManagerTestCase(test.TestCase):
self.assertRaises(exception.ManilaException,
self._manager._get_server_ip, fake_server)
def test_get_security_group_name_is_unique(self):
fake_secgroup = fake_compute.FakeSecurityGroup()
def test_security_group_name_not_specified(self):
self.stubs.Set(self._manager, 'get_config_option',
mock.Mock(return_value=None))
result = self._manager._get_or_create_security_group(self._context)
self.assertEqual(result, None)
self._manager.get_config_option.assert_called_once_with(
'service_instance_security_group')
def test_security_group_name_from_config_and_sg_exist(self):
fake_secgroup = fake_compute.FakeSecurityGroup(name="fake_sg_name")
self.stubs.Set(self._manager, 'get_config_option',
mock.Mock(return_value="fake_sg_name"))
self.stubs.Set(self._manager.compute_api, 'security_group_list',
mock.Mock(return_value=[fake_secgroup, ]))
result = self._manager._get_service_instance_security_group(
self._context, fake_secgroup.name)
result = self._manager._get_or_create_security_group(self._context)
self.assertEqual(result, fake_secgroup)
self._manager.get_config_option.assert_has_calls([
mock.call('service_instance_security_group'),
])
self._manager.compute_api.security_group_list.assert_called_once_with(
self._context)
def test_get_security_group_name_is_absent(self):
def test_security_group_creation_with_name_from_config(self):
name = "fake_sg_name"
desc = "fake_sg_description"
fake_secgroup = fake_compute.FakeSecurityGroup(name=name,
description=desc)
self.stubs.Set(self._manager, 'get_config_option',
mock.Mock(return_value=name))
self.stubs.Set(self._manager.compute_api, 'security_group_list',
mock.Mock(return_value=[]))
result = self._manager._get_service_instance_security_group(
self._context, "fake_absent_name")
self.assertEqual(result, None)
self.stubs.Set(self._manager.compute_api, 'security_group_create',
mock.Mock(return_value=fake_secgroup))
self.stubs.Set(self._manager.compute_api, 'security_group_rule_create',
mock.Mock())
result = self._manager._get_or_create_security_group(
context=self._context,
name=None,
description=desc,
)
self.assertEqual(result, fake_secgroup)
self._manager.compute_api.security_group_list.assert_called_once_with(
self._context)
self._manager.compute_api.security_group_create.\
assert_called_once_with(self._context, name, desc)
self._manager.get_config_option.assert_has_calls([
mock.call('service_instance_security_group'),
])
def test_get_security_group_name_not_unique(self):
fake_secgroup1 = fake_compute.FakeSecurityGroup()
fake_secgroup2 = fake_compute.FakeSecurityGroup()
def test_security_group_creation_with_provided_name(self):
name = "fake_sg_name"
desc = "fake_sg_description"
fake_secgroup = fake_compute.FakeSecurityGroup(name=name,
description=desc)
self.stubs.Set(self._manager.compute_api, 'security_group_list',
mock.Mock(return_value=[fake_secgroup1,
fake_secgroup2, ]))
self.assertRaises(exception.ServiceInstanceException,
self._manager._get_service_instance_security_group,
self._context,
fake_secgroup1.name)
def test_get_security_group_name_not_provided_at_all(self):
fake_secgroup = fake_compute.FakeSecurityGroup(
name=CONF.service_instance_security_group)
self.stubs.Set(self._manager.compute_api, 'security_group_list',
mock.Mock(return_value=[fake_secgroup, ]))
default_option = CONF.service_instance_security_group
with mock.patch.object(self._manager, 'get_config_option',
mock.Mock(return_value="")):
result = self._manager._get_service_instance_security_group(
context=self._context, name="")
self.assertEqual(result, None)
def test_get_security_group_name_not_provided(self):
fake_secgroup = fake_compute.FakeSecurityGroup(
name=CONF.service_instance_security_group)
self.stubs.Set(self._manager.compute_api, 'security_group_list',
mock.Mock(return_value=[fake_secgroup, ]))
result = self._manager._get_service_instance_security_group(
context=self._context, name=None)
mock.Mock(return_value=[]))
self.stubs.Set(self._manager.compute_api, 'security_group_create',
mock.Mock(return_value=fake_secgroup))
self.stubs.Set(self._manager.compute_api, 'security_group_rule_create',
mock.Mock())
result = self._manager._get_or_create_security_group(
context=self._context,
name=name,
description=desc,
)
self._manager.compute_api.security_group_list.assert_called_once_with(
self._context)
self._manager.compute_api.security_group_create.\
assert_called_once_with(self._context, name, desc)
self.assertEqual(result, fake_secgroup)
def test_create_security_group(self):
self.stubs.Set(self._manager.compute_api, 'security_group_create',
mock.Mock(return_value=mock.Mock()))
self.stubs.Set(self._manager.compute_api, 'security_group_rule_create',
mock.Mock(return_value=mock.Mock()))
result = self._manager._create_service_instance_security_group(
context=self._context, name="fake_name",
description="fake_description")
self._manager.compute_api.security_group_create.assert_called_once()
def test_security_group_two_sg_in_list(self):
name = "fake_name"
fake_secgroup1 = fake_compute.FakeSecurityGroup(name=name)
fake_secgroup2 = fake_compute.FakeSecurityGroup(name=name)
self.stubs.Set(self._manager.compute_api, 'security_group_list',
mock.Mock(return_value=[fake_secgroup1,
fake_secgroup2]))
self.assertRaises(exception.ServiceInstanceException,
self._manager._get_or_create_security_group,
self._context,
name,
)
self._manager.compute_api.security_group_list.assert_called_once_with(
self._context)
def test_set_up_service_instance(self):
fake_server = {'id': 'fake',
@ -374,7 +403,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
mock.Mock(return_value=fake_server))
self.stubs.Set(self._manager, '_get_server_ip',
mock.Mock(return_value='fake_ip'))
self.stubs.Set(self._manager, '_get_service_instance_security_group',
self.stubs.Set(self._manager, '_get_or_create_security_group',
mock.Mock(return_value=fake_security_group))
self.stubs.Set(service_instance.socket, 'socket', mock.Mock())
self.stubs.Set(self._manager, '_get_service_instance_name',
@ -403,7 +432,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
self.stubs.Set(self._manager, '_get_key',
mock.Mock(
return_value=('fake_key_name', 'fake_key_path')))
self.stubs.Set(self._manager, '_get_service_instance_security_group',
self.stubs.Set(self._manager, '_get_or_create_security_group',
mock.Mock(return_value=fake_security_group))
self.stubs.Set(self._manager, '_setup_network_for_instance',
mock.Mock(return_value=fake_port))
@ -433,7 +462,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
self.stubs.Set(self._manager, '_get_key',
mock.Mock(
return_value=('fake_key_name', 'fake_key_path')))
self.stubs.Set(self._manager, '_get_service_instance_security_group',
self.stubs.Set(self._manager, '_get_or_create_security_group',
mock.Mock(return_value=fake_security_group))
self.stubs.Set(self._manager, '_setup_network_for_instance',
mock.Mock(return_value=fake_port))