Merge "Use objects instead of SQLA deep copies in PortContext"

neutron/plugins/ml2/db.py

@@ -82,7 +82,7 @@ def get_locked_port_and_binding(context, port_id):
 def set_binding_levels(context, levels):
     if levels:
         for level in levels:
-            context.session.add(level)
+            level.persist_state_to_session(context.session)
         LOG.debug("For port %(port_id)s, host %(host)s, "
                   "set binding levels %(levels)s",
                   {'port_id': levels[0].port_id,

neutron/plugins/ml2/driver_context.py

@@ -13,13 +13,12 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import copy
-
 from neutron_lib.api.definitions import portbindings
 from neutron_lib import constants
 from neutron_lib.plugins.ml2 import api as ml2_api
 from oslo_log import log
 from oslo_serialization import jsonutils
+import sqlalchemy
 
 from neutron._i18n import _LW
 from neutron.db import segments_db
@@ -28,6 +27,32 @@ from neutron.plugins.ml2 import driver_api as api
 LOG = log.getLogger(__name__)
 
 
+class InstanceSnapshot(object):
+    """Used to avoid holding references to DB objects in PortContext."""
+    def __init__(self, obj):
+        self._model_class = obj.__class__
+        self._identity_key = sqlalchemy.orm.util.identity_key(instance=obj)[1]
+        self._cols = [col.key
+                      for col in sqlalchemy.inspect(self._model_class).columns]
+        for col in self._cols:
+            setattr(self, col, getattr(obj, col))
+
+    def persist_state_to_session(self, session):
+        """Updates the state of the snapshot in the session.
+
+        Finds the SQLA object in the session if it exists or creates a new
+        object and updates the object with the column values stored in this
+        snapshot.
+        """
+        db_obj = session.query(self._model_class).get(self._identity_key)
+        if db_obj:
+            for col in self._cols:
+                setattr(db_obj, col, getattr(self, col))
+        else:
+            session.add(self._model_class(**{col: getattr(self, col)
+                                             for col in self._cols}))
+
+
 class MechanismDriverContext(object):
     """MechanismDriver context base class."""
     def __init__(self, plugin, plugin_context):
@@ -101,10 +126,11 @@ class PortContext(MechanismDriverContext, api.PortContext):
         else:
             self._network_context = NetworkContext(
                 plugin, plugin_context, network) if network else None
-        # NOTE(kevinbenton): these copys can go away once we are working with
-        # OVO objects here instead of native SQLA objects.
-        self._binding = copy.deepcopy(binding)
-        self._binding_levels = copy.deepcopy(binding_levels)
+        # NOTE(kevinbenton): InstanceSnapshot can go away once we are working
+        # with OVO objects instead of native SQLA objects.
+        self._binding = InstanceSnapshot(binding)
+        self._binding_levels = [InstanceSnapshot(l)
+                                for l in (binding_levels or [])]
         self._segments_to_bind = None
         self._new_bound_segment = None
         self._next_segments_to_bind = None
@@ -130,7 +156,7 @@ class PortContext(MechanismDriverContext, api.PortContext):
         self._binding_levels = []
 
     def _push_binding_level(self, binding_level):
-        self._binding_levels.append(binding_level)
+        self._binding_levels.append(InstanceSnapshot(binding_level))
 
     def _pop_binding_level(self):
         return self._binding_levels.pop()

neutron/plugins/ml2/plugin.py

@@ -13,8 +13,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import copy
-
 from eventlet import greenthread
 from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext
 from neutron_lib.api.definitions import port_security as psec
@@ -360,10 +358,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             binding.host = ''
 
         self._update_port_dict_binding(port, binding)
-        # merging here brings binding changes into the session so they can be
-        # committed since the binding attached to the context is detached from
-        # the session
-        plugin_context.session.merge(binding)
+        binding.persist_state_to_session(plugin_context.session)
         return changes
 
     @db_api.retry_db_errors
@@ -537,7 +532,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                 db.set_binding_levels(plugin_context,
                                       bind_context._binding_levels)
                 # refresh context with a snapshot of updated state
-                cur_context._binding = copy.deepcopy(cur_binding)
+                cur_context._binding = driver_context.InstanceSnapshot(
+                    cur_binding)
                 cur_context._binding_levels = bind_context._binding_levels
 
                 # Update PortContext's port dictionary to reflect the
@@ -1369,7 +1365,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         binding.host = attrs and attrs.get(portbindings.HOST_ID)
         binding.router_id = attrs and attrs.get('device_id')
         # merge into session to reflect changes
-        plugin_context.session.merge(binding)
+        binding.persist_state_to_session(plugin_context.session)
 
     @utils.transaction_guard
     @db_api.retry_if_session_inactive()

neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py

@@ -37,6 +37,7 @@ from neutron.plugins.ml2.drivers.l2pop import mech_driver as l2pop_mech_driver
 from neutron.plugins.ml2.drivers.l2pop import rpc as l2pop_rpc
 from neutron.plugins.ml2.drivers.l2pop.rpc_manager import l2population_rpc
 from neutron.plugins.ml2 import managers
+from neutron.plugins.ml2 import models
 from neutron.plugins.ml2 import rpc
 from neutron.scheduler import l3_agent_scheduler
 from neutron.tests import base
@@ -1110,12 +1111,12 @@ class TestL2PopulationRpcTestCase(test_plugin.Ml2PluginV2TestCase):
 
         with self.port() as port:
             port['port'][portbindings.HOST_ID] = host
-            bindings = [mock.Mock()]
+            bindings = [models.PortBindingLevel()]
             port_context = driver_context.PortContext(
                 self.driver, self.context, port['port'],
                 self.driver.get_network(
                     self.context, port['port']['network_id']),
-                None, bindings)
+                models.PortBinding(), bindings)
             mock.patch.object(port_context, '_expand_segment').start()
             # The point is to provide coverage and to assert that no exceptions
             # are raised.
@@ -1139,12 +1140,12 @@ class TestL2PopulationRpcTestCase(test_plugin.Ml2PluginV2TestCase):
                 self.mock_fanout.reset_mock()
 
                 p['port'][portbindings.HOST_ID] = HOST
-                bindings = [mock.Mock()]
+                bindings = [models.PortBindingLevel()]
                 port_context = driver_context.PortContext(
                     self.driver, self.context, p['port'],
                     self.driver.get_network(
                         self.context, p['port']['network_id']),
-                    None, bindings)
+                    models.PortBinding(), bindings)
                 mock.patch.object(port_context, '_expand_segment').start()
                 # The point is to provide coverage and to assert that
                 # no exceptions are raised.
@@ -1160,7 +1161,7 @@ class TestL2PopulationRpcTestCase(test_plugin.Ml2PluginV2TestCase):
                 self.driver, self.context, port['port'],
                 self.driver.get_network(
                     self.context, port['port']['network_id']),
-                None, None)
+                models.PortBinding(), None)
             l2pop_mech._fixed_ips_changed(
                 port_context, None, port['port'], (set(['10.0.0.1']), set()))
 
@@ -1302,8 +1303,8 @@ class TestL2PopulationMechDriver(base.BaseTestCase):
                                              mock.Mock(),
                                              port,
                                              mock.MagicMock(),
-                                             mock.Mock(),
-                                             None,
+                                             models.PortBinding(),
+                                             [models.PortBindingLevel()],
                                              original_port=original_port)
 
         mech_driver = l2pop_mech_driver.L2populationMechanismDriver()

neutron/tests/unit/plugins/ml2/test_driver_context.py

@@ -18,6 +18,7 @@ from neutron_lib.api.definitions import portbindings
 from neutron_lib import constants
 
 from neutron.plugins.ml2 import driver_context
+from neutron.plugins.ml2 import models
 from neutron.tests import base
 
 
@@ -32,7 +33,7 @@ class TestPortContext(base.BaseTestCase):
         plugin = mock.Mock()
         plugin_context = mock.Mock()
         network = mock.MagicMock()
-        binding = mock.Mock()
+        binding = models.PortBinding()
 
         port = {'device_owner': constants.DEVICE_OWNER_DVR_INTERFACE}
         binding.host = 'foohost'
@@ -51,7 +52,7 @@ class TestPortContext(base.BaseTestCase):
         plugin = mock.Mock()
         plugin_context = mock.Mock()
         network = mock.MagicMock()
-        binding = mock.Mock()
+        binding = models.PortBinding()
 
         port = {'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX,
                 portbindings.HOST_ID: 'host'}
@@ -71,7 +72,7 @@ class TestPortContext(base.BaseTestCase):
         plugin = mock.Mock()
         plugin_context = mock.Mock()
         network = mock.MagicMock()
-        binding = mock.Mock()
+        binding = models.PortBinding()
 
         port = {'device_owner': constants.DEVICE_OWNER_DVR_INTERFACE}
         binding.status = 'foostatus'
@@ -90,7 +91,7 @@ class TestPortContext(base.BaseTestCase):
         plugin = mock.Mock()
         plugin_context = mock.Mock()
         network = mock.MagicMock()
-        binding = mock.Mock()
+        binding = models.PortBinding()
 
         port = {'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX,
                 'status': 'status'}

neutron/tests/unit/plugins/ml2/test_plugin.py

@@ -1641,7 +1641,7 @@ class TestMl2PortBinding(Ml2PluginV2TestCase,
             mech_context = driver_context.PortContext(
                 plugin, ctx, None,
                 plugin.get_network(self.context, n['network']['id']),
-                None, None)
+                models.PortBinding(), None)
             with mock.patch.object(plugin, '_attempt_binding') as ab:
                 plugin._bind_port_if_needed(mech_context)
                 self.assertFalse(ab.called)
@@ -1740,16 +1740,16 @@ class TestMl2PortBinding(Ml2PluginV2TestCase,
         plugin = directory.get_plugin()
         mock_network = {'id': 'net_id'}
         mock_port = {'id': 'port_id'}
-        context = mock.Mock()
+        ctxt = context.get_admin_context()
         new_router_id = 'new_router'
         attrs = {'device_id': new_router_id, portbindings.HOST_ID: host_id}
         with mock.patch.object(plugin, '_update_port_dict_binding'):
             with mock.patch.object(segments_db, 'get_network_segments',
                                    return_value=[]):
                 mech_context = driver_context.PortContext(
-                    self, context, mock_port, mock_network, binding, None)
+                    self, ctxt, mock_port, mock_network, binding, None)
                 plugin._process_distributed_port_binding(mech_context,
-                                                         context, attrs)
+                                                         ctxt, attrs)
                 self.assertEqual(new_router_id,
                                  mech_context._binding.router_id)
                 self.assertEqual(host_id, mech_context._binding.host)

neutron/tests/unit/plugins/ml2/test_port_binding.py

@@ -111,8 +111,10 @@ class PortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
         ctx = context.get_admin_context()
         with self.port(name='name') as port:
             # emulating concurrent binding deletion
-            (ctx.session.query(ml2_models.PortBinding).
-             filter_by(port_id=port['port']['id']).delete())
+            with ctx.session.begin():
+                for item in (ctx.session.query(ml2_models.PortBinding).
+                             filter_by(port_id=port['port']['id'])):
+                    ctx.session.delete(item)
             self.assertIsNone(
                 self.plugin.get_bound_port_context(ctx, port['port']['id']))
 
@@ -196,7 +198,7 @@ class PortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
                 profile=jsonutils.dumps(original_port['binding:profile']),
                 vif_type=original_port['binding:vif_type'],
                 vif_details=original_port['binding:vif_details'])
-            levels = 1
+            levels = []
             mech_context = driver_context.PortContext(
                 plugin, ctx, updated_port, network, binding, levels,
                 original_port=original_port)