Merge "Make ML2 ensure_dvr_port_binding more robust"

neutron/plugins/ml2/db.py

@@ -15,6 +15,8 @@
 
 from sqlalchemy.orm import exc
 
+from oslo.db import exception as db_exc
+
 from neutron.common import constants as n_const
 from neutron.db import api as db_api
 from neutron.db import models_v2
@@ -90,16 +92,13 @@ def get_locked_port_and_binding(session, port_id):
 
 
 def ensure_dvr_port_binding(session, port_id, host, router_id=None):
-    # FIXME(armando-migliaccio): take care of LP #1335226
-    # DVR ports are slightly different from the others in
-    # that binding happens at a later stage via L3 agent
-    # therefore we need to keep this logic of creation on
-    # missing binding.
-    with session.begin(subtransactions=True):
-        try:
-            record = (session.query(models.DVRPortBinding).
-                      filter_by(port_id=port_id, host=host).one())
-        except exc.NoResultFound:
+    record = (session.query(models.DVRPortBinding).
+              filter_by(port_id=port_id, host=host).first())
+    if record:
+        return record
+
+    try:
+        with session.begin(subtransactions=True):
             record = models.DVRPortBinding(
                 port_id=port_id,
                 host=host,
@@ -109,7 +108,11 @@ def ensure_dvr_port_binding(session, port_id, host, router_id=None):
                 cap_port_filter=False,
                 status=n_const.PORT_STATUS_DOWN)
             session.add(record)
-        return record
+            return record
+    except db_exc.DBDuplicateEntry:
+        LOG.debug("DVR Port %s already bound", port_id)
+        return (session.query(models.DVRPortBinding).
+                filter_by(port_id=port_id, host=host).one())
 
 
 def delete_dvr_port_binding(session, port_id, host):

neutron/tests/unit/ml2/db/test_ml2_dvr_db.py

@@ -13,6 +13,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import mock
+
+from sqlalchemy.orm import query
+
 from neutron import context
 from neutron.db import api as db_api
 from neutron.db import l3_db
@@ -66,6 +70,22 @@ class Ml2DBTestCase(base.BaseTestCase):
             self.ctx.session.add(record)
             return record
 
+    def test_ensure_dvr_port_binding_deals_with_db_duplicate(self):
+        network_id = 'foo_network_id'
+        port_id = 'foo_port_id'
+        router_id = 'foo_router_id'
+        host_id = 'foo_host_id'
+        self._setup_neutron_network(network_id, [port_id])
+        self._setup_dvr_binding(network_id, port_id, router_id, host_id)
+        with mock.patch.object(query.Query, 'first') as query_first:
+            query_first.return_value = []
+            with mock.patch.object(ml2_db.LOG, 'debug') as log_trace:
+                binding = ml2_db.ensure_dvr_port_binding(
+                    self.ctx.session, port_id, host_id, router_id)
+        self.assertTrue(query_first.called)
+        self.assertTrue(log_trace.called)
+        self.assertEqual(port_id, binding.port_id)
+
     def test_ensure_dvr_port_binding(self):
         network_id = 'foo_network_id'
         port_id = 'foo_port_id'