diff --git a/neutron/plugins/ml2/db.py b/neutron/plugins/ml2/db.py index b0cc5d43fed..5dc92ff962d 100644 --- a/neutron/plugins/ml2/db.py +++ b/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, diff --git a/neutron/plugins/ml2/driver_context.py b/neutron/plugins/ml2/driver_context.py index cf70ccac90a..822dc59af2f 100644 --- a/neutron/plugins/ml2/driver_context.py +++ b/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() diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index a11295728d2..42324759395 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/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() diff --git a/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py index 89a2c05d63d..87acc64f8d2 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py +++ b/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() diff --git a/neutron/tests/unit/plugins/ml2/test_driver_context.py b/neutron/tests/unit/plugins/ml2/test_driver_context.py index 202763c02f9..c344127f461 100644 --- a/neutron/tests/unit/plugins/ml2/test_driver_context.py +++ b/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'} diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index 3b4b7ddcaa2..2d954696976 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/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) diff --git a/neutron/tests/unit/plugins/ml2/test_port_binding.py b/neutron/tests/unit/plugins/ml2/test_port_binding.py index 99db6770f5c..8e26f379de1 100644 --- a/neutron/tests/unit/plugins/ml2/test_port_binding.py +++ b/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)