From 50b469fae2dbd1517d856509f37328394e0697e4 Mon Sep 17 00:00:00 2001
From: Jakub Libosvar <libosvar@redhat.com>
Date: Tue, 3 Feb 2015 14:33:39 +0100
Subject: [PATCH] Don't crash when adding duplicate gre allocation

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 | 10 ++++++
 neutron/tests/unit/ml2/test_type_gre.py | 41 +++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/neutron/plugins/ml2/drivers/type_gre.py b/neutron/plugins/ml2/drivers/type_gre.py
index 1d17b66f287..9a426284b41 100644
--- a/neutron/plugins/ml2/drivers/type_gre.py
+++ b/neutron/plugins/ml2/drivers/type_gre.py
@@ -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())
diff --git a/neutron/tests/unit/ml2/test_type_gre.py b/neutron/tests/unit/ml2/test_type_gre.py
index 3bdb4e103af..0ee4163ce7e 100644
--- a/neutron/tests/unit/ml2/test_type_gre.py
+++ b/neutron/tests/unit/ml2/test_type_gre.py
@@ -15,6 +15,11 @@
 
 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):