openstack
/
neutron
Code Issues Proposed changes

Don't create default SG in transaction 

Creating default security groups should not be done inside
another transaction because the creation code emits AFTER events.
Handlers for the push notification code expect to be able to read
from the DB during an after event using a new transaction, which will
not work if they are inside of exising transactions.

This patch adjusts the default security group creation logic to avoid
using layered transactions for the default security group creation and
adjusts ML2 to stop calling ensure_default_security_group inside of a
port/network transaction.

Partially-Implements: blueprint push-notifications
Change-Id: Iaa83c8664d5bfde31fdcdd694f6f18d9ef9bf14a
This commit is contained in:
Kevin Benton 2017-01-06 14:06:11 -08:00
parent 499f3c6d56
commit ffc4489a58
3 changed files with 48 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
neutron/db/securitygroups_db.py
View File

@ -83,6 +83,11 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
if not default_sg:
self._ensure_default_security_group(context, tenant_id)
else:
existing_def_sg_id = self._get_default_sg_id(context, tenant_id)
if existing_def_sg_id is not None:
# default already exists, return it
return self.get_security_group(context, existing_def_sg_id)
with db_api.autonested_transaction(context.session):
security_group_db = sg_models.SecurityGroup(id=s.get('id') or (
@ -661,27 +666,30 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
port[ext_sg.SECURITYGROUPS] = (security_group_ids and
list(security_group_ids) or [])
def _ensure_default_security_group(self, context, tenant_id):
"""Create a default security group if one doesn't exist.
:returns: the default security group id for given tenant.
"""
def _get_default_sg_id(self, context, tenant_id):
try:
query = self._model_query(context, sg_models.DefaultSecurityGroup)
default_group = query.filter_by(tenant_id=tenant_id).one()
return default_group['security_group_id']
except exc.NoResultFound:
security_group = {
'security_group':
{'name': 'default',
'tenant_id': tenant_id,
'description': _('Default security group')}
}
# starting a transaction before create to avoid db retries
with context.session.begin(subtransactions=True):
sg_id = self.create_security_group(
context, security_group, default_sg=True)['id']
return sg_id
pass
def _ensure_default_security_group(self, context, tenant_id):
"""Create a default security group if one doesn't exist.
:returns: the default security group id for given tenant.
"""
existing = self._get_default_sg_id(context, tenant_id)
if existing is not None:
return existing
security_group = {
'security_group':
{'name': 'default',
'tenant_id': tenant_id,
'description': _('Default security group')}
}
return self.create_security_group(context, security_group,
default_sg=True)['id']
def _get_security_groups_on_port(self, context, port):
"""Check that all security groups on port belong to tenant.

8
neutron/plugins/ml2/plugin.py
View File

@ -740,7 +740,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
tenant_id = net_data['tenant_id']
session = context.session
with session.begin(subtransactions=True):
self._ensure_default_security_group(context, tenant_id)
net_db = self.create_network_db(context, network)
result = self._make_network_dict(net_db, process_extensions=False,
context=context)
@ -776,6 +775,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
@utils.transaction_guard
@db_api.retry_if_session_inactive()
def create_network(self, context, network):
self._ensure_default_security_group(context,
network['network']['tenant_id'])
result, mech_context = self._create_network_db(context, network)
kwargs = {'context': context, 'network': result}
registry.notify(resources.NETWORK, events.AFTER_CREATE, self, **kwargs)
@ -792,6 +793,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
@utils.transaction_guard
@db_api.retry_if_session_inactive()
def create_network_bulk(self, context, networks):
tenants = {n['network']['tenant_id'] for n in networks['networks']}
map(lambda t: self._ensure_default_security_group(context, t), tenants)
objects = self._create_bulk_ml2(attributes.NETWORK, context, networks)
return [obj['result'] for obj in objects]
@ -1245,6 +1248,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
if not attrs.get('status'):
attrs['status'] = const.PORT_STATUS_DOWN
# NOTE(kevinbenton): triggered outside of transaction since it
# emits 'AFTER' events if it creates.
self._ensure_default_security_group(context, attrs['tenant_id'])
session = context.session
with session.begin(subtransactions=True):
dhcp_opts = attrs.get(edo_ext.EXTRADHCPOPTS, [])

17
neutron/tests/unit/plugins/ml2/test_security_group.py
View File

@ -20,6 +20,9 @@ import mock
from neutron_lib import constants as const
from neutron_lib.plugins import directory
from neutron.callbacks import events
from neutron.callbacks import registry
from neutron.callbacks import resources
from neutron import context
from neutron.extensions import securitygroup as ext_sg
from neutron.tests import tools
@ -148,6 +151,20 @@ class TestMl2SecurityGroups(Ml2SecurityGroupsTestCase,
# the or_ function should only have one argument
or_mock.assert_called_once_with(mock.ANY)
def test_security_groups_created_outside_transaction(self):
def record_after_state(r, e, t, context, *args, **kwargs):
self.was_active = context.session.is_active
registry.subscribe(record_after_state, resources.SECURITY_GROUP,
events.AFTER_CREATE)
with self.subnet() as s:
self.assertFalse(self.was_active)
self._delete(
'security-groups',
self._list('security-groups')['security_groups'][0]['id'])
with self.port(subnet=s):
self.assertFalse(self.was_active)
class TestMl2SGServerRpcCallBack(
Ml2SecurityGroupsTestCase,