summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKevin Benton <kevin@benton.pub>2017-06-20 00:00:12 -0700
committerKevin Benton <kevin@benton.pub>2017-06-20 01:34:10 -0700
commit0f536d5a25d410362e5a7b1390bc627cc3dc5071 (patch)
treed9ecb013487230338042833c80fbe22d14afeadf
parent1db5ace55e28891feaa3424fd9f637b8a1cc7e75 (diff)
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
Notes
Notes (review): Code-Review+2: Armando Migliaccio <armamig@gmail.com> Code-Review+2: Ann Taraday <akamyshnikova@mirantis.com> Workflow+1: Ann Taraday <akamyshnikova@mirantis.com> Verified+2: Jenkins Submitted-by: Jenkins Submitted-at: Thu, 22 Jun 2017 09:35:13 +0000 Reviewed-on: https://review.openstack.org/475650 Project: openstack/neutron Branch: refs/heads/master
-rw-r--r--neutron/plugins/ml2/db.py2
-rw-r--r--neutron/plugins/ml2/driver_context.py40
-rw-r--r--neutron/plugins/ml2/plugin.py12
-rw-r--r--neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py15
-rw-r--r--neutron/tests/unit/plugins/ml2/test_driver_context.py9
-rw-r--r--neutron/tests/unit/plugins/ml2/test_plugin.py8
-rw-r--r--neutron/tests/unit/plugins/ml2/test_port_binding.py8
7 files changed, 60 insertions, 34 deletions
diff --git a/neutron/plugins/ml2/db.py b/neutron/plugins/ml2/db.py
index b0cc5d4..5dc92ff 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):
82def set_binding_levels(context, levels): 82def set_binding_levels(context, levels):
83 if levels: 83 if levels:
84 for level in levels: 84 for level in levels:
85 context.session.add(level) 85 level.persist_state_to_session(context.session)
86 LOG.debug("For port %(port_id)s, host %(host)s, " 86 LOG.debug("For port %(port_id)s, host %(host)s, "
87 "set binding levels %(levels)s", 87 "set binding levels %(levels)s",
88 {'port_id': levels[0].port_id, 88 {'port_id': levels[0].port_id,
diff --git a/neutron/plugins/ml2/driver_context.py b/neutron/plugins/ml2/driver_context.py
index cf70cca..822dc59 100644
--- a/neutron/plugins/ml2/driver_context.py
+++ b/neutron/plugins/ml2/driver_context.py
@@ -13,13 +13,12 @@
13# License for the specific language governing permissions and limitations 13# License for the specific language governing permissions and limitations
14# under the License. 14# under the License.
15 15
16import copy
17
18from neutron_lib.api.definitions import portbindings 16from neutron_lib.api.definitions import portbindings
19from neutron_lib import constants 17from neutron_lib import constants
20from neutron_lib.plugins.ml2 import api as ml2_api 18from neutron_lib.plugins.ml2 import api as ml2_api
21from oslo_log import log 19from oslo_log import log
22from oslo_serialization import jsonutils 20from oslo_serialization import jsonutils
21import sqlalchemy
23 22
24from neutron._i18n import _LW 23from neutron._i18n import _LW
25from neutron.db import segments_db 24from neutron.db import segments_db
@@ -28,6 +27,32 @@ from neutron.plugins.ml2 import driver_api as api
28LOG = log.getLogger(__name__) 27LOG = log.getLogger(__name__)
29 28
30 29
30class InstanceSnapshot(object):
31 """Used to avoid holding references to DB objects in PortContext."""
32 def __init__(self, obj):
33 self._model_class = obj.__class__
34 self._identity_key = sqlalchemy.orm.util.identity_key(instance=obj)[1]
35 self._cols = [col.key
36 for col in sqlalchemy.inspect(self._model_class).columns]
37 for col in self._cols:
38 setattr(self, col, getattr(obj, col))
39
40 def persist_state_to_session(self, session):
41 """Updates the state of the snapshot in the session.
42
43 Finds the SQLA object in the session if it exists or creates a new
44 object and updates the object with the column values stored in this
45 snapshot.
46 """
47 db_obj = session.query(self._model_class).get(self._identity_key)
48 if db_obj:
49 for col in self._cols:
50 setattr(db_obj, col, getattr(self, col))
51 else:
52 session.add(self._model_class(**{col: getattr(self, col)
53 for col in self._cols}))
54
55
31class MechanismDriverContext(object): 56class MechanismDriverContext(object):
32 """MechanismDriver context base class.""" 57 """MechanismDriver context base class."""
33 def __init__(self, plugin, plugin_context): 58 def __init__(self, plugin, plugin_context):
@@ -101,10 +126,11 @@ class PortContext(MechanismDriverContext, api.PortContext):
101 else: 126 else:
102 self._network_context = NetworkContext( 127 self._network_context = NetworkContext(
103 plugin, plugin_context, network) if network else None 128 plugin, plugin_context, network) if network else None
104 # NOTE(kevinbenton): these copys can go away once we are working with 129 # NOTE(kevinbenton): InstanceSnapshot can go away once we are working
105 # OVO objects here instead of native SQLA objects. 130 # with OVO objects instead of native SQLA objects.
106 self._binding = copy.deepcopy(binding) 131 self._binding = InstanceSnapshot(binding)
107 self._binding_levels = copy.deepcopy(binding_levels) 132 self._binding_levels = [InstanceSnapshot(l)
133 for l in (binding_levels or [])]
108 self._segments_to_bind = None 134 self._segments_to_bind = None
109 self._new_bound_segment = None 135 self._new_bound_segment = None
110 self._next_segments_to_bind = None 136 self._next_segments_to_bind = None
@@ -130,7 +156,7 @@ class PortContext(MechanismDriverContext, api.PortContext):
130 self._binding_levels = [] 156 self._binding_levels = []
131 157
132 def _push_binding_level(self, binding_level): 158 def _push_binding_level(self, binding_level):
133 self._binding_levels.append(binding_level) 159 self._binding_levels.append(InstanceSnapshot(binding_level))
134 160
135 def _pop_binding_level(self): 161 def _pop_binding_level(self):
136 return self._binding_levels.pop() 162 return self._binding_levels.pop()
diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index a112957..4232475 100644
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -13,8 +13,6 @@
13# License for the specific language governing permissions and limitations 13# License for the specific language governing permissions and limitations
14# under the License. 14# under the License.
15 15
16import copy
17
18from eventlet import greenthread 16from eventlet import greenthread
19from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext 17from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext
20from neutron_lib.api.definitions import port_security as psec 18from neutron_lib.api.definitions import port_security as psec
@@ -360,10 +358,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
360 binding.host = '' 358 binding.host = ''
361 359
362 self._update_port_dict_binding(port, binding) 360 self._update_port_dict_binding(port, binding)
363 # merging here brings binding changes into the session so they can be 361 binding.persist_state_to_session(plugin_context.session)
364 # committed since the binding attached to the context is detached from
365 # the session
366 plugin_context.session.merge(binding)
367 return changes 362 return changes
368 363
369 @db_api.retry_db_errors 364 @db_api.retry_db_errors
@@ -537,7 +532,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
537 db.set_binding_levels(plugin_context, 532 db.set_binding_levels(plugin_context,
538 bind_context._binding_levels) 533 bind_context._binding_levels)
539 # refresh context with a snapshot of updated state 534 # refresh context with a snapshot of updated state
540 cur_context._binding = copy.deepcopy(cur_binding) 535 cur_context._binding = driver_context.InstanceSnapshot(
536 cur_binding)
541 cur_context._binding_levels = bind_context._binding_levels 537 cur_context._binding_levels = bind_context._binding_levels
542 538
543 # Update PortContext's port dictionary to reflect the 539 # Update PortContext's port dictionary to reflect the
@@ -1369,7 +1365,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
1369 binding.host = attrs and attrs.get(portbindings.HOST_ID) 1365 binding.host = attrs and attrs.get(portbindings.HOST_ID)
1370 binding.router_id = attrs and attrs.get('device_id') 1366 binding.router_id = attrs and attrs.get('device_id')
1371 # merge into session to reflect changes 1367 # merge into session to reflect changes
1372 plugin_context.session.merge(binding) 1368 binding.persist_state_to_session(plugin_context.session)
1373 1369
1374 @utils.transaction_guard 1370 @utils.transaction_guard
1375 @db_api.retry_if_session_inactive() 1371 @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 89a2c05..87acc64 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
37from neutron.plugins.ml2.drivers.l2pop import rpc as l2pop_rpc 37from neutron.plugins.ml2.drivers.l2pop import rpc as l2pop_rpc
38from neutron.plugins.ml2.drivers.l2pop.rpc_manager import l2population_rpc 38from neutron.plugins.ml2.drivers.l2pop.rpc_manager import l2population_rpc
39from neutron.plugins.ml2 import managers 39from neutron.plugins.ml2 import managers
40from neutron.plugins.ml2 import models
40from neutron.plugins.ml2 import rpc 41from neutron.plugins.ml2 import rpc
41from neutron.scheduler import l3_agent_scheduler 42from neutron.scheduler import l3_agent_scheduler
42from neutron.tests import base 43from neutron.tests import base
@@ -1110,12 +1111,12 @@ class TestL2PopulationRpcTestCase(test_plugin.Ml2PluginV2TestCase):
1110 1111
1111 with self.port() as port: 1112 with self.port() as port:
1112 port['port'][portbindings.HOST_ID] = host 1113 port['port'][portbindings.HOST_ID] = host
1113 bindings = [mock.Mock()] 1114 bindings = [models.PortBindingLevel()]
1114 port_context = driver_context.PortContext( 1115 port_context = driver_context.PortContext(
1115 self.driver, self.context, port['port'], 1116 self.driver, self.context, port['port'],
1116 self.driver.get_network( 1117 self.driver.get_network(
1117 self.context, port['port']['network_id']), 1118 self.context, port['port']['network_id']),
1118 None, bindings) 1119 models.PortBinding(), bindings)
1119 mock.patch.object(port_context, '_expand_segment').start() 1120 mock.patch.object(port_context, '_expand_segment').start()
1120 # The point is to provide coverage and to assert that no exceptions 1121 # The point is to provide coverage and to assert that no exceptions
1121 # are raised. 1122 # are raised.
@@ -1139,12 +1140,12 @@ class TestL2PopulationRpcTestCase(test_plugin.Ml2PluginV2TestCase):
1139 self.mock_fanout.reset_mock() 1140 self.mock_fanout.reset_mock()
1140 1141
1141 p['port'][portbindings.HOST_ID] = HOST 1142 p['port'][portbindings.HOST_ID] = HOST
1142 bindings = [mock.Mock()] 1143 bindings = [models.PortBindingLevel()]
1143 port_context = driver_context.PortContext( 1144 port_context = driver_context.PortContext(
1144 self.driver, self.context, p['port'], 1145 self.driver, self.context, p['port'],
1145 self.driver.get_network( 1146 self.driver.get_network(
1146 self.context, p['port']['network_id']), 1147 self.context, p['port']['network_id']),
1147 None, bindings) 1148 models.PortBinding(), bindings)
1148 mock.patch.object(port_context, '_expand_segment').start() 1149 mock.patch.object(port_context, '_expand_segment').start()
1149 # The point is to provide coverage and to assert that 1150 # The point is to provide coverage and to assert that
1150 # no exceptions are raised. 1151 # no exceptions are raised.
@@ -1160,7 +1161,7 @@ class TestL2PopulationRpcTestCase(test_plugin.Ml2PluginV2TestCase):
1160 self.driver, self.context, port['port'], 1161 self.driver, self.context, port['port'],
1161 self.driver.get_network( 1162 self.driver.get_network(
1162 self.context, port['port']['network_id']), 1163 self.context, port['port']['network_id']),
1163 None, None) 1164 models.PortBinding(), None)
1164 l2pop_mech._fixed_ips_changed( 1165 l2pop_mech._fixed_ips_changed(
1165 port_context, None, port['port'], (set(['10.0.0.1']), set())) 1166 port_context, None, port['port'], (set(['10.0.0.1']), set()))
1166 1167
@@ -1302,8 +1303,8 @@ class TestL2PopulationMechDriver(base.BaseTestCase):
1302 mock.Mock(), 1303 mock.Mock(),
1303 port, 1304 port,
1304 mock.MagicMock(), 1305 mock.MagicMock(),
1305 mock.Mock(), 1306 models.PortBinding(),
1306 None, 1307 [models.PortBindingLevel()],
1307 original_port=original_port) 1308 original_port=original_port)
1308 1309
1309 mech_driver = l2pop_mech_driver.L2populationMechanismDriver() 1310 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 202763c..c344127 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
18from neutron_lib import constants 18from neutron_lib import constants
19 19
20from neutron.plugins.ml2 import driver_context 20from neutron.plugins.ml2 import driver_context
21from neutron.plugins.ml2 import models
21from neutron.tests import base 22from neutron.tests import base
22 23
23 24
@@ -32,7 +33,7 @@ class TestPortContext(base.BaseTestCase):
32 plugin = mock.Mock() 33 plugin = mock.Mock()
33 plugin_context = mock.Mock() 34 plugin_context = mock.Mock()
34 network = mock.MagicMock() 35 network = mock.MagicMock()
35 binding = mock.Mock() 36 binding = models.PortBinding()
36 37
37 port = {'device_owner': constants.DEVICE_OWNER_DVR_INTERFACE} 38 port = {'device_owner': constants.DEVICE_OWNER_DVR_INTERFACE}
38 binding.host = 'foohost' 39 binding.host = 'foohost'
@@ -51,7 +52,7 @@ class TestPortContext(base.BaseTestCase):
51 plugin = mock.Mock() 52 plugin = mock.Mock()
52 plugin_context = mock.Mock() 53 plugin_context = mock.Mock()
53 network = mock.MagicMock() 54 network = mock.MagicMock()
54 binding = mock.Mock() 55 binding = models.PortBinding()
55 56
56 port = {'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX, 57 port = {'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX,
57 portbindings.HOST_ID: 'host'} 58 portbindings.HOST_ID: 'host'}
@@ -71,7 +72,7 @@ class TestPortContext(base.BaseTestCase):
71 plugin = mock.Mock() 72 plugin = mock.Mock()
72 plugin_context = mock.Mock() 73 plugin_context = mock.Mock()
73 network = mock.MagicMock() 74 network = mock.MagicMock()
74 binding = mock.Mock() 75 binding = models.PortBinding()
75 76
76 port = {'device_owner': constants.DEVICE_OWNER_DVR_INTERFACE} 77 port = {'device_owner': constants.DEVICE_OWNER_DVR_INTERFACE}
77 binding.status = 'foostatus' 78 binding.status = 'foostatus'
@@ -90,7 +91,7 @@ class TestPortContext(base.BaseTestCase):
90 plugin = mock.Mock() 91 plugin = mock.Mock()
91 plugin_context = mock.Mock() 92 plugin_context = mock.Mock()
92 network = mock.MagicMock() 93 network = mock.MagicMock()
93 binding = mock.Mock() 94 binding = models.PortBinding()
94 95
95 port = {'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX, 96 port = {'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX,
96 'status': 'status'} 97 'status': 'status'}
diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py
index 3b4b7dd..2d95469 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,
1641 mech_context = driver_context.PortContext( 1641 mech_context = driver_context.PortContext(
1642 plugin, ctx, None, 1642 plugin, ctx, None,
1643 plugin.get_network(self.context, n['network']['id']), 1643 plugin.get_network(self.context, n['network']['id']),
1644 None, None) 1644 models.PortBinding(), None)
1645 with mock.patch.object(plugin, '_attempt_binding') as ab: 1645 with mock.patch.object(plugin, '_attempt_binding') as ab:
1646 plugin._bind_port_if_needed(mech_context) 1646 plugin._bind_port_if_needed(mech_context)
1647 self.assertFalse(ab.called) 1647 self.assertFalse(ab.called)
@@ -1740,16 +1740,16 @@ class TestMl2PortBinding(Ml2PluginV2TestCase,
1740 plugin = directory.get_plugin() 1740 plugin = directory.get_plugin()
1741 mock_network = {'id': 'net_id'} 1741 mock_network = {'id': 'net_id'}
1742 mock_port = {'id': 'port_id'} 1742 mock_port = {'id': 'port_id'}
1743 context = mock.Mock() 1743 ctxt = context.get_admin_context()
1744 new_router_id = 'new_router' 1744 new_router_id = 'new_router'
1745 attrs = {'device_id': new_router_id, portbindings.HOST_ID: host_id} 1745 attrs = {'device_id': new_router_id, portbindings.HOST_ID: host_id}
1746 with mock.patch.object(plugin, '_update_port_dict_binding'): 1746 with mock.patch.object(plugin, '_update_port_dict_binding'):
1747 with mock.patch.object(segments_db, 'get_network_segments', 1747 with mock.patch.object(segments_db, 'get_network_segments',
1748 return_value=[]): 1748 return_value=[]):
1749 mech_context = driver_context.PortContext( 1749 mech_context = driver_context.PortContext(
1750 self, context, mock_port, mock_network, binding, None) 1750 self, ctxt, mock_port, mock_network, binding, None)
1751 plugin._process_distributed_port_binding(mech_context, 1751 plugin._process_distributed_port_binding(mech_context,
1752 context, attrs) 1752 ctxt, attrs)
1753 self.assertEqual(new_router_id, 1753 self.assertEqual(new_router_id,
1754 mech_context._binding.router_id) 1754 mech_context._binding.router_id)
1755 self.assertEqual(host_id, mech_context._binding.host) 1755 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 99db677..8e26f37 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):
111 ctx = context.get_admin_context() 111 ctx = context.get_admin_context()
112 with self.port(name='name') as port: 112 with self.port(name='name') as port:
113 # emulating concurrent binding deletion 113 # emulating concurrent binding deletion
114 (ctx.session.query(ml2_models.PortBinding). 114 with ctx.session.begin():
115 filter_by(port_id=port['port']['id']).delete()) 115 for item in (ctx.session.query(ml2_models.PortBinding).
116 filter_by(port_id=port['port']['id'])):
117 ctx.session.delete(item)
116 self.assertIsNone( 118 self.assertIsNone(
117 self.plugin.get_bound_port_context(ctx, port['port']['id'])) 119 self.plugin.get_bound_port_context(ctx, port['port']['id']))
118 120
@@ -196,7 +198,7 @@ class PortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
196 profile=jsonutils.dumps(original_port['binding:profile']), 198 profile=jsonutils.dumps(original_port['binding:profile']),
197 vif_type=original_port['binding:vif_type'], 199 vif_type=original_port['binding:vif_type'],
198 vif_details=original_port['binding:vif_details']) 200 vif_details=original_port['binding:vif_details'])
199 levels = 1 201 levels = []
200 mech_context = driver_context.PortContext( 202 mech_context = driver_context.PortContext(
201 plugin, ctx, updated_port, network, binding, levels, 203 plugin, ctx, updated_port, network, binding, levels,
202 original_port=original_port) 204 original_port=original_port)