]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Don't crash when adding duplicate gre allocation
authorJakub Libosvar <libosvar@redhat.com>
Tue, 3 Feb 2015 13:33:39 +0000 (14:33 +0100)
committerJakub Libosvar <libosvar@redhat.com>
Mon, 9 Feb 2015 23:10:25 +0000 (00:10 +0100)
This patch catches DBDuplicateError when initializing ML2 GRE type
driver and allocation already exists in DB. Because current allocations
are queried and then only those that doesn't exist in database are
added, DBDuplicateError should never occur.

But the race can happen when running multiple neutron-servers and one
of servers adds allocations between allocations are queried and added.

Change-Id: I427b7020d61b0d2c06292ff2804ba1f4483696c6
Closes-bug: 1417560

neutron/plugins/ml2/drivers/type_gre.py
neutron/tests/unit/ml2/test_type_gre.py

index 1d17b66f287e39327e7fc8f8dd6fdc8a9b422e9c..9a426284b414b617ccc00c3c12f8e4e916b5766c 100644 (file)
@@ -96,6 +96,16 @@ class GreTypeDriver(type_tunnel.TunnelTypeDriver):
                 gre_ids |= set(moves.xrange(tun_min, tun_max + 1))
 
         session = db_api.get_session()
+        try:
+            self._add_allocation(session, gre_ids)
+        except db_exc.DBDuplicateEntry:
+            # in case multiple neutron-servers start allocations could be
+            # already added by different neutron-server. because this function
+            # is called only when initializing this type driver, it's safe to
+            # assume allocations were added.
+            LOG.warning(_LW("Gre allocations were already created."))
+
+    def _add_allocation(self, session, gre_ids):
         with session.begin(subtransactions=True):
             # remove from table unallocated tunnels not currently allocatable
             allocs = (session.query(GreAllocation).all())
index 3bdb4e103af5de96c28b1bc82370685a31f78ba6..0ee4163ce7e1b6ab500c66eb09174b07515495f9 100644 (file)
 
 import mock
 
+from oslo_db import exception as db_exc
+from sqlalchemy.orm import exc as sa_exc
+import testtools
+
+from neutron.db import api as db_api
 from neutron.plugins.common import constants as p_const
 from neutron.plugins.ml2.drivers import type_gre
 from neutron.tests.unit.ml2 import test_type_tunnel
@@ -27,6 +32,16 @@ HOST_ONE = 'fake_host_one'
 HOST_TWO = 'fake_host_two'
 
 
+def _add_allocation(session, gre_id, allocated=False):
+    allocation = type_gre.GreAllocation(gre_id=gre_id, allocated=allocated)
+    allocation.save(session)
+
+
+def _get_allocation(session, gre_id):
+    return session.query(type_gre.GreAllocation).filter_by(
+        gre_id=gre_id).one()
+
+
 class GreTypeTest(test_type_tunnel.TunnelTypeTestMixin,
                   testlib_api.SqlTestCase):
     DRIVER_CLASS = type_gre.GreTypeDriver
@@ -83,6 +98,32 @@ class GreTypeTest(test_type_tunnel.TunnelTypeTestMixin,
         endpoints = self.driver.get_endpoints()
         self.assertNotIn(TUNNEL_IP_ONE, endpoints)
 
+    def test_sync_allocations_entry_added_during_session(self):
+        with mock.patch.object(self.driver, '_add_allocation',
+                               side_effect=db_exc.DBDuplicateEntry) as (
+                mock_add_allocation):
+            self.driver.sync_allocations()
+            self.assertTrue(mock_add_allocation.called)
+
+    def test__add_allocation_not_existing(self):
+        session = db_api.get_session()
+        _add_allocation(session, gre_id=1)
+        self.driver._add_allocation(session, {1, 2})
+        _get_allocation(session, 2)
+
+    def test__add_allocation_existing_allocated_is_kept(self):
+        session = db_api.get_session()
+        _add_allocation(session, gre_id=1, allocated=True)
+        self.driver._add_allocation(session, {2})
+        _get_allocation(session, 1)
+
+    def test__add_allocation_existing_not_allocated_is_removed(self):
+        session = db_api.get_session()
+        _add_allocation(session, gre_id=1)
+        self.driver._add_allocation(session, {2})
+        with testtools.ExpectedException(sa_exc.NoResultFound):
+            _get_allocation(session, 1)
+
 
 class GreTypeMultiRangeTest(test_type_tunnel.TunnelTypeMultiRangeTestMixin,
                            testlib_api.SqlTestCase):