Use objects instead of SQLA deep copies in PortContext

The workaround of using deepcopy calls on the PortBinding
and PortBindingLevel objects prevents the port relationship
from being loaded to bump its revision because it then fails
to merge.

So in order to allow port bindings to bump the revision we
need to stop using sqlalchemy objects in the PortContext. This
patch adds a new snapshot object that just copies the column
values and provides a method to reconcile them back into the
session.

This workaround can go away after we switch to using OVOs, but
this needs to be backportable so we can't just wait for OVO
adoption.

Partial-Bug: #1699034
Change-Id: Ib85ec8182117fa3c4844dabfffe881e38e68b556
This commit is contained in:
Kevin Benton 2017-06-20 00:00:12 -07:00
parent 1db5ace55e
commit 0f536d5a25
7 changed files with 60 additions and 34 deletions

View File

@ -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,

View File

@ -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()

View File

@ -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()

View File

@ -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()

View File

@ -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'}

View File

@ -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)

View File

@ -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)