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

diff --git a/neutron/db/api.py b/neutron/db/api.py
index f1a9e33c4f7efed4c2249b21ba5d97f46c4d265b..3957de91a41b7bb6af002636c16ef65ee747022c 100644 (file)
--- 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 c12e60d1912db004f071e42ce7e1311b3201f9dc..92dcc7ac4141189081c6d6ac1064b748f67d4b90 100644 (file)
--- 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 d0b4f4bcabbd971a335fae2827e63fb3dd1f419c..4ae8c310723f122bf6388ca346c08db153d99959 100644 (file)
--- 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 be70bd6302f1db86f97206dbb0dba4eb937de893..58c049277bc0a96d0ee052ffb811ff83c02eddb7 100644 (file)
--- 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: