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:
parent
1db5ace55e
commit
0f536d5a25
|
@ -82,7 +82,7 @@ def get_locked_port_and_binding(context, port_id):
|
||||||
def set_binding_levels(context, levels):
|
def set_binding_levels(context, levels):
|
||||||
if levels:
|
if levels:
|
||||||
for level in 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, "
|
LOG.debug("For port %(port_id)s, host %(host)s, "
|
||||||
"set binding levels %(levels)s",
|
"set binding levels %(levels)s",
|
||||||
{'port_id': levels[0].port_id,
|
{'port_id': levels[0].port_id,
|
||||||
|
|
|
@ -13,13 +13,12 @@
|
||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
import copy
|
|
||||||
|
|
||||||
from neutron_lib.api.definitions import portbindings
|
from neutron_lib.api.definitions import portbindings
|
||||||
from neutron_lib import constants
|
from neutron_lib import constants
|
||||||
from neutron_lib.plugins.ml2 import api as ml2_api
|
from neutron_lib.plugins.ml2 import api as ml2_api
|
||||||
from oslo_log import log
|
from oslo_log import log
|
||||||
from oslo_serialization import jsonutils
|
from oslo_serialization import jsonutils
|
||||||
|
import sqlalchemy
|
||||||
|
|
||||||
from neutron._i18n import _LW
|
from neutron._i18n import _LW
|
||||||
from neutron.db import segments_db
|
from neutron.db import segments_db
|
||||||
|
@ -28,6 +27,32 @@ from neutron.plugins.ml2 import driver_api as api
|
||||||
LOG = log.getLogger(__name__)
|
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):
|
class MechanismDriverContext(object):
|
||||||
"""MechanismDriver context base class."""
|
"""MechanismDriver context base class."""
|
||||||
def __init__(self, plugin, plugin_context):
|
def __init__(self, plugin, plugin_context):
|
||||||
|
@ -101,10 +126,11 @@ class PortContext(MechanismDriverContext, api.PortContext):
|
||||||
else:
|
else:
|
||||||
self._network_context = NetworkContext(
|
self._network_context = NetworkContext(
|
||||||
plugin, plugin_context, network) if network else None
|
plugin, plugin_context, network) if network else None
|
||||||
# NOTE(kevinbenton): these copys can go away once we are working with
|
# NOTE(kevinbenton): InstanceSnapshot can go away once we are working
|
||||||
# OVO objects here instead of native SQLA objects.
|
# with OVO objects instead of native SQLA objects.
|
||||||
self._binding = copy.deepcopy(binding)
|
self._binding = InstanceSnapshot(binding)
|
||||||
self._binding_levels = copy.deepcopy(binding_levels)
|
self._binding_levels = [InstanceSnapshot(l)
|
||||||
|
for l in (binding_levels or [])]
|
||||||
self._segments_to_bind = None
|
self._segments_to_bind = None
|
||||||
self._new_bound_segment = None
|
self._new_bound_segment = None
|
||||||
self._next_segments_to_bind = None
|
self._next_segments_to_bind = None
|
||||||
|
@ -130,7 +156,7 @@ class PortContext(MechanismDriverContext, api.PortContext):
|
||||||
self._binding_levels = []
|
self._binding_levels = []
|
||||||
|
|
||||||
def _push_binding_level(self, binding_level):
|
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):
|
def _pop_binding_level(self):
|
||||||
return self._binding_levels.pop()
|
return self._binding_levels.pop()
|
||||||
|
|
|
@ -13,8 +13,6 @@
|
||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
import copy
|
|
||||||
|
|
||||||
from eventlet import greenthread
|
from eventlet import greenthread
|
||||||
from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext
|
from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext
|
||||||
from neutron_lib.api.definitions import port_security as psec
|
from neutron_lib.api.definitions import port_security as psec
|
||||||
|
@ -360,10 +358,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
||||||
binding.host = ''
|
binding.host = ''
|
||||||
|
|
||||||
self._update_port_dict_binding(port, binding)
|
self._update_port_dict_binding(port, binding)
|
||||||
# merging here brings binding changes into the session so they can be
|
binding.persist_state_to_session(plugin_context.session)
|
||||||
# committed since the binding attached to the context is detached from
|
|
||||||
# the session
|
|
||||||
plugin_context.session.merge(binding)
|
|
||||||
return changes
|
return changes
|
||||||
|
|
||||||
@db_api.retry_db_errors
|
@db_api.retry_db_errors
|
||||||
|
@ -537,7 +532,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
||||||
db.set_binding_levels(plugin_context,
|
db.set_binding_levels(plugin_context,
|
||||||
bind_context._binding_levels)
|
bind_context._binding_levels)
|
||||||
# refresh context with a snapshot of updated state
|
# 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
|
cur_context._binding_levels = bind_context._binding_levels
|
||||||
|
|
||||||
# Update PortContext's port dictionary to reflect the
|
# 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.host = attrs and attrs.get(portbindings.HOST_ID)
|
||||||
binding.router_id = attrs and attrs.get('device_id')
|
binding.router_id = attrs and attrs.get('device_id')
|
||||||
# merge into session to reflect changes
|
# merge into session to reflect changes
|
||||||
plugin_context.session.merge(binding)
|
binding.persist_state_to_session(plugin_context.session)
|
||||||
|
|
||||||
@utils.transaction_guard
|
@utils.transaction_guard
|
||||||
@db_api.retry_if_session_inactive()
|
@db_api.retry_if_session_inactive()
|
||||||
|
|
|
@ -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 import rpc as l2pop_rpc
|
||||||
from neutron.plugins.ml2.drivers.l2pop.rpc_manager import l2population_rpc
|
from neutron.plugins.ml2.drivers.l2pop.rpc_manager import l2population_rpc
|
||||||
from neutron.plugins.ml2 import managers
|
from neutron.plugins.ml2 import managers
|
||||||
|
from neutron.plugins.ml2 import models
|
||||||
from neutron.plugins.ml2 import rpc
|
from neutron.plugins.ml2 import rpc
|
||||||
from neutron.scheduler import l3_agent_scheduler
|
from neutron.scheduler import l3_agent_scheduler
|
||||||
from neutron.tests import base
|
from neutron.tests import base
|
||||||
|
@ -1110,12 +1111,12 @@ class TestL2PopulationRpcTestCase(test_plugin.Ml2PluginV2TestCase):
|
||||||
|
|
||||||
with self.port() as port:
|
with self.port() as port:
|
||||||
port['port'][portbindings.HOST_ID] = host
|
port['port'][portbindings.HOST_ID] = host
|
||||||
bindings = [mock.Mock()]
|
bindings = [models.PortBindingLevel()]
|
||||||
port_context = driver_context.PortContext(
|
port_context = driver_context.PortContext(
|
||||||
self.driver, self.context, port['port'],
|
self.driver, self.context, port['port'],
|
||||||
self.driver.get_network(
|
self.driver.get_network(
|
||||||
self.context, port['port']['network_id']),
|
self.context, port['port']['network_id']),
|
||||||
None, bindings)
|
models.PortBinding(), bindings)
|
||||||
mock.patch.object(port_context, '_expand_segment').start()
|
mock.patch.object(port_context, '_expand_segment').start()
|
||||||
# The point is to provide coverage and to assert that no exceptions
|
# The point is to provide coverage and to assert that no exceptions
|
||||||
# are raised.
|
# are raised.
|
||||||
|
@ -1139,12 +1140,12 @@ class TestL2PopulationRpcTestCase(test_plugin.Ml2PluginV2TestCase):
|
||||||
self.mock_fanout.reset_mock()
|
self.mock_fanout.reset_mock()
|
||||||
|
|
||||||
p['port'][portbindings.HOST_ID] = HOST
|
p['port'][portbindings.HOST_ID] = HOST
|
||||||
bindings = [mock.Mock()]
|
bindings = [models.PortBindingLevel()]
|
||||||
port_context = driver_context.PortContext(
|
port_context = driver_context.PortContext(
|
||||||
self.driver, self.context, p['port'],
|
self.driver, self.context, p['port'],
|
||||||
self.driver.get_network(
|
self.driver.get_network(
|
||||||
self.context, p['port']['network_id']),
|
self.context, p['port']['network_id']),
|
||||||
None, bindings)
|
models.PortBinding(), bindings)
|
||||||
mock.patch.object(port_context, '_expand_segment').start()
|
mock.patch.object(port_context, '_expand_segment').start()
|
||||||
# The point is to provide coverage and to assert that
|
# The point is to provide coverage and to assert that
|
||||||
# no exceptions are raised.
|
# no exceptions are raised.
|
||||||
|
@ -1160,7 +1161,7 @@ class TestL2PopulationRpcTestCase(test_plugin.Ml2PluginV2TestCase):
|
||||||
self.driver, self.context, port['port'],
|
self.driver, self.context, port['port'],
|
||||||
self.driver.get_network(
|
self.driver.get_network(
|
||||||
self.context, port['port']['network_id']),
|
self.context, port['port']['network_id']),
|
||||||
None, None)
|
models.PortBinding(), None)
|
||||||
l2pop_mech._fixed_ips_changed(
|
l2pop_mech._fixed_ips_changed(
|
||||||
port_context, None, port['port'], (set(['10.0.0.1']), set()))
|
port_context, None, port['port'], (set(['10.0.0.1']), set()))
|
||||||
|
|
||||||
|
@ -1302,8 +1303,8 @@ class TestL2PopulationMechDriver(base.BaseTestCase):
|
||||||
mock.Mock(),
|
mock.Mock(),
|
||||||
port,
|
port,
|
||||||
mock.MagicMock(),
|
mock.MagicMock(),
|
||||||
mock.Mock(),
|
models.PortBinding(),
|
||||||
None,
|
[models.PortBindingLevel()],
|
||||||
original_port=original_port)
|
original_port=original_port)
|
||||||
|
|
||||||
mech_driver = l2pop_mech_driver.L2populationMechanismDriver()
|
mech_driver = l2pop_mech_driver.L2populationMechanismDriver()
|
||||||
|
|
|
@ -18,6 +18,7 @@ from neutron_lib.api.definitions import portbindings
|
||||||
from neutron_lib import constants
|
from neutron_lib import constants
|
||||||
|
|
||||||
from neutron.plugins.ml2 import driver_context
|
from neutron.plugins.ml2 import driver_context
|
||||||
|
from neutron.plugins.ml2 import models
|
||||||
from neutron.tests import base
|
from neutron.tests import base
|
||||||
|
|
||||||
|
|
||||||
|
@ -32,7 +33,7 @@ class TestPortContext(base.BaseTestCase):
|
||||||
plugin = mock.Mock()
|
plugin = mock.Mock()
|
||||||
plugin_context = mock.Mock()
|
plugin_context = mock.Mock()
|
||||||
network = mock.MagicMock()
|
network = mock.MagicMock()
|
||||||
binding = mock.Mock()
|
binding = models.PortBinding()
|
||||||
|
|
||||||
port = {'device_owner': constants.DEVICE_OWNER_DVR_INTERFACE}
|
port = {'device_owner': constants.DEVICE_OWNER_DVR_INTERFACE}
|
||||||
binding.host = 'foohost'
|
binding.host = 'foohost'
|
||||||
|
@ -51,7 +52,7 @@ class TestPortContext(base.BaseTestCase):
|
||||||
plugin = mock.Mock()
|
plugin = mock.Mock()
|
||||||
plugin_context = mock.Mock()
|
plugin_context = mock.Mock()
|
||||||
network = mock.MagicMock()
|
network = mock.MagicMock()
|
||||||
binding = mock.Mock()
|
binding = models.PortBinding()
|
||||||
|
|
||||||
port = {'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX,
|
port = {'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX,
|
||||||
portbindings.HOST_ID: 'host'}
|
portbindings.HOST_ID: 'host'}
|
||||||
|
@ -71,7 +72,7 @@ class TestPortContext(base.BaseTestCase):
|
||||||
plugin = mock.Mock()
|
plugin = mock.Mock()
|
||||||
plugin_context = mock.Mock()
|
plugin_context = mock.Mock()
|
||||||
network = mock.MagicMock()
|
network = mock.MagicMock()
|
||||||
binding = mock.Mock()
|
binding = models.PortBinding()
|
||||||
|
|
||||||
port = {'device_owner': constants.DEVICE_OWNER_DVR_INTERFACE}
|
port = {'device_owner': constants.DEVICE_OWNER_DVR_INTERFACE}
|
||||||
binding.status = 'foostatus'
|
binding.status = 'foostatus'
|
||||||
|
@ -90,7 +91,7 @@ class TestPortContext(base.BaseTestCase):
|
||||||
plugin = mock.Mock()
|
plugin = mock.Mock()
|
||||||
plugin_context = mock.Mock()
|
plugin_context = mock.Mock()
|
||||||
network = mock.MagicMock()
|
network = mock.MagicMock()
|
||||||
binding = mock.Mock()
|
binding = models.PortBinding()
|
||||||
|
|
||||||
port = {'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX,
|
port = {'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX,
|
||||||
'status': 'status'}
|
'status': 'status'}
|
||||||
|
|
|
@ -1641,7 +1641,7 @@ class TestMl2PortBinding(Ml2PluginV2TestCase,
|
||||||
mech_context = driver_context.PortContext(
|
mech_context = driver_context.PortContext(
|
||||||
plugin, ctx, None,
|
plugin, ctx, None,
|
||||||
plugin.get_network(self.context, n['network']['id']),
|
plugin.get_network(self.context, n['network']['id']),
|
||||||
None, None)
|
models.PortBinding(), None)
|
||||||
with mock.patch.object(plugin, '_attempt_binding') as ab:
|
with mock.patch.object(plugin, '_attempt_binding') as ab:
|
||||||
plugin._bind_port_if_needed(mech_context)
|
plugin._bind_port_if_needed(mech_context)
|
||||||
self.assertFalse(ab.called)
|
self.assertFalse(ab.called)
|
||||||
|
@ -1740,16 +1740,16 @@ class TestMl2PortBinding(Ml2PluginV2TestCase,
|
||||||
plugin = directory.get_plugin()
|
plugin = directory.get_plugin()
|
||||||
mock_network = {'id': 'net_id'}
|
mock_network = {'id': 'net_id'}
|
||||||
mock_port = {'id': 'port_id'}
|
mock_port = {'id': 'port_id'}
|
||||||
context = mock.Mock()
|
ctxt = context.get_admin_context()
|
||||||
new_router_id = 'new_router'
|
new_router_id = 'new_router'
|
||||||
attrs = {'device_id': new_router_id, portbindings.HOST_ID: host_id}
|
attrs = {'device_id': new_router_id, portbindings.HOST_ID: host_id}
|
||||||
with mock.patch.object(plugin, '_update_port_dict_binding'):
|
with mock.patch.object(plugin, '_update_port_dict_binding'):
|
||||||
with mock.patch.object(segments_db, 'get_network_segments',
|
with mock.patch.object(segments_db, 'get_network_segments',
|
||||||
return_value=[]):
|
return_value=[]):
|
||||||
mech_context = driver_context.PortContext(
|
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,
|
plugin._process_distributed_port_binding(mech_context,
|
||||||
context, attrs)
|
ctxt, attrs)
|
||||||
self.assertEqual(new_router_id,
|
self.assertEqual(new_router_id,
|
||||||
mech_context._binding.router_id)
|
mech_context._binding.router_id)
|
||||||
self.assertEqual(host_id, mech_context._binding.host)
|
self.assertEqual(host_id, mech_context._binding.host)
|
||||||
|
|
|
@ -111,8 +111,10 @@ class PortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
|
||||||
ctx = context.get_admin_context()
|
ctx = context.get_admin_context()
|
||||||
with self.port(name='name') as port:
|
with self.port(name='name') as port:
|
||||||
# emulating concurrent binding deletion
|
# emulating concurrent binding deletion
|
||||||
(ctx.session.query(ml2_models.PortBinding).
|
with ctx.session.begin():
|
||||||
filter_by(port_id=port['port']['id']).delete())
|
for item in (ctx.session.query(ml2_models.PortBinding).
|
||||||
|
filter_by(port_id=port['port']['id'])):
|
||||||
|
ctx.session.delete(item)
|
||||||
self.assertIsNone(
|
self.assertIsNone(
|
||||||
self.plugin.get_bound_port_context(ctx, port['port']['id']))
|
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']),
|
profile=jsonutils.dumps(original_port['binding:profile']),
|
||||||
vif_type=original_port['binding:vif_type'],
|
vif_type=original_port['binding:vif_type'],
|
||||||
vif_details=original_port['binding:vif_details'])
|
vif_details=original_port['binding:vif_details'])
|
||||||
levels = 1
|
levels = []
|
||||||
mech_context = driver_context.PortContext(
|
mech_context = driver_context.PortContext(
|
||||||
plugin, ctx, updated_port, network, binding, levels,
|
plugin, ctx, updated_port, network, binding, levels,
|
||||||
original_port=original_port)
|
original_port=original_port)
|
||||||
|
|
Loading…
Reference in New Issue