From 8dee8df12b402943b3f7f2e34d3ccc2c00068619 Mon Sep 17 00:00:00 2001 From: Eugene Nikanorov Date: Sat, 14 Mar 2015 23:35:08 +0300 Subject: [PATCH] Handle DBDuplicateError exception properly when creating default sg Previously, an exception was not caught in one of invocations (create_network) of _ensure_default_security_group. Move exception handling inside that method so it never fails with such exception. Change-Id: Ibb0597d4db187c856f9ac1d9700701e0165c3c73 Closes-Bug: #1419723 --- neutron/db/api.py | 16 +++++++ neutron/db/securitygroups_db.py | 44 +++++++++++-------- neutron/tests/unit/test_db_plugin.py | 1 + .../unit/test_extension_security_group.py | 13 ++++-- 4 files changed, 52 insertions(+), 22 deletions(-) diff --git a/neutron/db/api.py b/neutron/db/api.py index f1a9e33c4..3957de91a 100644 --- a/neutron/db/api.py +++ b/neutron/db/api.py @@ -13,8 +13,12 @@ # License for the specific language governing permissions and limitations # under the License. +import contextlib + from oslo_config import cfg from oslo_db.sqlalchemy import session +from sqlalchemy import exc + _FACADE = None @@ -41,3 +45,15 @@ def get_session(autocommit=True, expire_on_commit=False): facade = _create_facade_lazily() return facade.get_session(autocommit=autocommit, expire_on_commit=expire_on_commit) + + +@contextlib.contextmanager +def autonested_transaction(sess): + """This is a convenience method to not bother with 'nested' parameter.""" + try: + session_context = sess.begin_nested() + except exc.InvalidRequestError: + session_context = sess.begin(subtransactions=True) + finally: + with session_context as tx: + yield tx diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index c12e60d19..92dcc7ac4 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -22,6 +22,7 @@ from sqlalchemy.orm import scoped_session from neutron.api.v2 import attributes from neutron.common import constants +from neutron.db import api as db_api from neutron.db import db_base_plugin_v2 from neutron.db import model_base from neutron.db import models_v2 @@ -127,11 +128,7 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): tenant_id = self._get_tenant_id_for_create(context, s) if not default_sg: - try: - self._ensure_default_security_group(context, tenant_id) - except exception.DBDuplicateEntry as ex: - LOG.debug("Duplicate default security group %s was not" - " created", ex.value) + self._ensure_default_security_group(context, tenant_id) with context.session.begin(subtransactions=True): security_group_db = SecurityGroup(id=s.get('id') or ( @@ -530,20 +527,29 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): :returns: the default security group id. """ query = self._model_query(context, DefaultSecurityGroup) - try: - default_group = query.filter( - DefaultSecurityGroup.tenant_id == tenant_id).one() - except exc.NoResultFound: - security_group = { - 'security_group': {'name': 'default', - 'tenant_id': tenant_id, - 'description': _('Default security group')} - } - ret = self.create_security_group(context, security_group, - default_sg=True) - return ret['id'] - else: - return default_group['security_group_id'] + # the next loop should do 2 iterations at max + while True: + with db_api.autonested_transaction(context.session): + try: + default_group = query.filter_by(tenant_id=tenant_id).one() + except exc.NoResultFound: + security_group = { + 'security_group': + {'name': 'default', + 'tenant_id': tenant_id, + 'description': _('Default security group')} + } + try: + ret = self.create_security_group( + context, security_group, default_sg=True) + except exception.DBDuplicateEntry as ex: + LOG.debug("Duplicate default security group %s was " + "not created", ex.value) + continue + else: + return ret['id'] + else: + return default_group['security_group_id'] def _get_security_groups_on_port(self, context, port): """Check that all security groups on port belong to tenant. diff --git a/neutron/tests/unit/test_db_plugin.py b/neutron/tests/unit/test_db_plugin.py index d0b4f4bca..4ae8c3107 100644 --- a/neutron/tests/unit/test_db_plugin.py +++ b/neutron/tests/unit/test_db_plugin.py @@ -153,6 +153,7 @@ class NeutronDbPluginV2TestCase(testlib_api.WebTestCase, getattr(manager.NeutronManager.get_plugin(), native_sorting_attr_name, False)) + self.plugin = manager.NeutronManager.get_plugin() self._skip_native_sorting = not _is_native_sorting_support() if ext_mgr: self.ext_api = test_extensions.setup_extensions_middleware(ext_mgr) diff --git a/neutron/tests/unit/test_extension_security_group.py b/neutron/tests/unit/test_extension_security_group.py index be70bd630..58c049277 100644 --- a/neutron/tests/unit/test_extension_security_group.py +++ b/neutron/tests/unit/test_extension_security_group.py @@ -266,10 +266,17 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self._assert_sg_rule_has_kvs(v6_rule, expected) def test_skip_duplicate_default_sg_error(self): + # can't always raise, or create_security_group will hang with mock.patch.object(SecurityGroupTestPlugin, - '_ensure_default_security_group', - side_effect=exc.DBDuplicateEntry()): - self._make_security_group(self.fmt, 'test_sg', 'test_desc') + 'create_security_group', + side_effect=[exc.DBDuplicateEntry(), + {'id': 'foo'}]): + self.plugin.create_network( + context.get_admin_context(), + {'network': {'name': 'foo', + 'admin_state_up': True, + 'shared': False, + 'tenant_id': 'bar'}}) def test_update_security_group(self): with self.security_group() as sg: -- 2.45.2