From: Eugene Nikanorov <enikanorov@mirantis.com>
Date: Tue, 24 Dec 2013 11:08:22 +0000 (+0400)
Subject: Fix race in get_network(s) in OVS plugin
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=126a26367b40125b3e434c948f21a4980bc86340;p=openstack-build%2Fneutron-build.git

Fix race in get_network(s) in OVS plugin

Load network bindings eagerly with networks.
Otherwise a different db query could try to fetch network bindings
for already deleted networks. The issue is reproducible with
concurrent tempest network API tests.

Closes-Bug: 1263686
Change-Id: I0521ab162220c66a62491ff8e7e4a9f6d7bef6c4
---

diff --git a/neutron/plugins/openvswitch/ovs_db_v2.py b/neutron/plugins/openvswitch/ovs_db_v2.py
index 8155d2493..b28df3e12 100644
--- a/neutron/plugins/openvswitch/ovs_db_v2.py
+++ b/neutron/plugins/openvswitch/ovs_db_v2.py
@@ -55,6 +55,7 @@ def add_network_binding(session, network_id, network_type,
                                                physical_network,
                                                segmentation_id)
         session.add(binding)
+    return binding
 
 
 def sync_vlan_allocations(network_vlan_ranges):
diff --git a/neutron/plugins/openvswitch/ovs_models_v2.py b/neutron/plugins/openvswitch/ovs_models_v2.py
index 3ca34f1c2..ab334c318 100644
--- a/neutron/plugins/openvswitch/ovs_models_v2.py
+++ b/neutron/plugins/openvswitch/ovs_models_v2.py
@@ -20,7 +20,9 @@
 from sqlalchemy import Boolean, Column, ForeignKey, Integer, String
 from sqlalchemy.schema import UniqueConstraint
 
+from neutron.db import models_v2
 from neutron.db.models_v2 import model_base
+from sqlalchemy import orm
 
 
 class VlanAllocation(model_base.BASEV2):
@@ -70,6 +72,11 @@ class NetworkBinding(model_base.BASEV2):
     physical_network = Column(String(64))
     segmentation_id = Column(Integer)  # tunnel_id or vlan_id
 
+    network = orm.relationship(
+        models_v2.Network,
+        backref=orm.backref("binding", lazy='joined',
+                            uselist=False, cascade='delete'))
+
     def __init__(self, network_id, network_type, physical_network,
                  segmentation_id):
         self.network_id = network_id
diff --git a/neutron/plugins/openvswitch/ovs_neutron_plugin.py b/neutron/plugins/openvswitch/ovs_neutron_plugin.py
index e7970a7b5..450ce3dd2 100644
--- a/neutron/plugins/openvswitch/ovs_neutron_plugin.py
+++ b/neutron/plugins/openvswitch/ovs_neutron_plugin.py
@@ -290,6 +290,9 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
             self._aliases = aliases
         return self._aliases
 
+    db_base_plugin_v2.NeutronDbPluginV2.register_dict_extend_funcs(
+        attributes.NETWORKS, ['_extend_network_dict_provider_ovs'])
+
     def __init__(self, configfile=None):
         self.base_binding_dict = {
             portbindings.VIF_TYPE: portbindings.VIF_TYPE_OVS,
@@ -374,9 +377,11 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
                 sys.exit(1)
         LOG.info(_("Tunnel ID ranges: %s"), self.tunnel_id_ranges)
 
-    def _extend_network_dict_provider(self, context, network):
-        binding = ovs_db_v2.get_network_binding(context.session,
-                                                network['id'])
+    def _extend_network_dict_provider_ovs(self, network, net_db,
+                                          net_binding=None):
+        # this method used in two cases: when binding is provided explicitly
+        # and when it is a part of db model object
+        binding = net_db.binding if net_db else net_binding
         network[provider.NETWORK_TYPE] = binding.network_type
         if binding.network_type in constants.TUNNEL_NETWORK_TYPES:
             network[provider.PHYSICAL_NETWORK] = None
@@ -501,11 +506,14 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
                 # no reservation needed for TYPE_LOCAL
             net = super(OVSNeutronPluginV2, self).create_network(context,
                                                                  network)
-            ovs_db_v2.add_network_binding(session, net['id'], network_type,
-                                          physical_network, segmentation_id)
+            binding = ovs_db_v2.add_network_binding(session, net['id'],
+                                                    network_type,
+                                                    physical_network,
+                                                    segmentation_id)
 
             self._process_l3_create(context, net, network['network'])
-            self._extend_network_dict_provider(context, net)
+            # passing None as db model to use binding object
+            self._extend_network_dict_provider_ovs(net, None, binding)
             # note - exception will rollback entire transaction
         LOG.debug(_("Created network: %s"), net['id'])
         return net
@@ -518,7 +526,6 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
             net = super(OVSNeutronPluginV2, self).update_network(context, id,
                                                                  network)
             self._process_l3_update(context, net, network['network'])
-            self._extend_network_dict_provider(context, net)
         return net
 
     def delete_network(self, context, id):
@@ -543,7 +550,6 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
         with session.begin(subtransactions=True):
             net = super(OVSNeutronPluginV2, self).get_network(context,
                                                               id, None)
-            self._extend_network_dict_provider(context, net)
         return self._fields(net, fields)
 
     def get_networks(self, context, filters=None, fields=None,
@@ -554,8 +560,6 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
             nets = super(OVSNeutronPluginV2,
                          self).get_networks(context, filters, None, sorts,
                                             limit, marker, page_reverse)
-            for net in nets:
-                self._extend_network_dict_provider(context, net)
 
         return [self._fields(net, fields) for net in nets]