diff --git a/neutron/plugins/ml2/db.py b/neutron/plugins/ml2/db.py index 5de9838911c..3a13397b24a 100644 --- a/neutron/plugins/ml2/db.py +++ b/neutron/plugins/ml2/db.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +from debtcollector import removals from neutron_lib import constants as n_const from neutron_lib.plugins import directory from oslo_db import exception as db_exc @@ -48,6 +49,11 @@ def add_port_binding(context, port_id): return record +@removals.remove( + message="Use get_port from inside of a transaction. The revision plugin " + "provides protection against concurrent updates to the same " + "resource with compare and swap updates of the revision_number.", + removal_version='Queens') def get_locked_port_and_binding(context, port_id): """Get port and port binding records for update within transaction.""" diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 264dc54ac83..6daf2ddb2a5 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -442,8 +442,17 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, # Get the current port state and build a new PortContext # reflecting this state as original state for subsequent # mechanism driver update_port_*commit() calls. - port_db, cur_binding = db.get_locked_port_and_binding( - plugin_context, port_id) + try: + port_db = self._get_port(plugin_context, port_id) + cur_binding = port_db.port_binding + except exc.PortNotFound: + port_db, cur_binding = None, None + if not port_db or not cur_binding: + # The port has been deleted concurrently, so just + # return the unbound result from the initial + # transaction that completed before the deletion. + LOG.debug("Port %s has been deleted concurrently", port_id) + return orig_context, False, False # Since the mechanism driver bind_port() calls must be made # outside a DB transaction locking the port state, it is # possible (but unlikely) that the port's state could change @@ -453,19 +462,13 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, # used. If attributes such as binding:host_id, binding:profile, # or binding:vnic_type are updated concurrently, the try_again # flag is returned to indicate that the commit was unsuccessful. - if not port_db: - # The port has been deleted concurrently, so just - # return the unbound result from the initial - # transaction that completed before the deletion. - LOG.debug("Port %s has been deleted concurrently", port_id) - return orig_context, False, False oport = self._make_port_dict(port_db) port = self._make_port_dict(port_db) network = bind_context.network.current if port['device_owner'] == const.DEVICE_OWNER_DVR_INTERFACE: # REVISIT(rkukura): The PortBinding instance from the # ml2_port_bindings table, returned as cur_binding - # from db.get_locked_port_and_binding() above, is + # from port_db.port_binding above, is # currently not used for DVR distributed ports, and is # replaced here with the DistributedPortBinding instance from # the ml2_distributed_port_bindings table specific to the host @@ -1395,8 +1398,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, bound_mech_contexts = [] with session.begin(subtransactions=True): - port_db, binding = db.get_locked_port_and_binding(context, id) - if not port_db: + port_db = self._get_port(context, id) + binding = port_db.port_binding + if not binding: raise exc.PortNotFound(port_id=id) mac_address_updated = self._check_mac_update_allowed( port_db, attrs, binding) @@ -1601,8 +1605,10 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, session = context.session with session.begin(subtransactions=True): - port_db, binding = db.get_locked_port_and_binding(context, id) - if not port_db: + try: + port_db = self._get_port(context, id) + binding = port_db.port_binding + except exc.PortNotFound: LOG.debug("The port '%s' was deleted", id) return port = self._make_port_dict(port_db) diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index 61150ac5fb5..50d4d62dfeb 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -1072,9 +1072,10 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase): self.assertIsNone(l3plugin.disassociate_floatingips(ctx, port_id)) def test_create_port_tolerates_db_deadlock(self): + plugin = directory.get_plugin() with self.network() as net: with self.subnet(network=net) as subnet: - _orig = ml2_db.get_locked_port_and_binding + _orig = plugin._get_port self._failed = False def fail_once(*args, **kwargs): @@ -1082,9 +1083,8 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase): self._failed = True raise db_exc.DBDeadlock() return _orig(*args, **kwargs) - with mock.patch('neutron.plugins.ml2.plugin.' - 'db.get_locked_port_and_binding', - side_effect=fail_once) as get_port_mock: + with mock.patch.object(plugin, '_get_port', + side_effect=fail_once) as get_port_mock: port_kwargs = {portbindings.HOST_ID: 'host1', 'subnet': subnet, 'device_id': 'deadlocktest'} @@ -1103,18 +1103,15 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase): ctx = context.get_admin_context() plugin = directory.get_plugin() with self.port() as port: - port_db, binding = ml2_db.get_locked_port_and_binding( - ctx, port['port']['id']) - with mock.patch('neutron.plugins.ml2.plugin.' - 'db.get_locked_port_and_binding') as lock: - lock.side_effect = [db_exc.DBDeadlock, - (port_db, binding)] + port_db = plugin._get_port(ctx, port['port']['id']) + with mock.patch.object(plugin, '_get_port') as gp: + gp.side_effect = [db_exc.DBDeadlock] + [port_db] * 3 req = self.new_delete_request('ports', port['port']['id']) res = req.get_response(self.api) self.assertEqual(204, res.status_int) - self.assertEqual(2, lock.call_count) - self.assertRaises( - exc.PortNotFound, plugin.get_port, ctx, port['port']['id']) + self.assertGreater(gp.call_count, 1) + self.assertRaises( + exc.PortNotFound, plugin.get_port, ctx, port['port']['id']) def test_port_create_resillient_to_duplicate_records(self): @@ -1523,29 +1520,29 @@ class TestMl2PortBinding(Ml2PluginV2TestCase, # create a port and delete it so we have an expired mechanism context with self.port() as port: plugin = directory.get_plugin() - binding = ml2_db.get_locked_port_and_binding(self.context, - port['port']['id'])[1] + binding = plugin._get_port(self.context, + port['port']['id']).port_binding binding['host'] = 'test' mech_context = driver_context.PortContext( plugin, self.context, port['port'], plugin.get_network(self.context, port['port']['network_id']), binding, None) - with mock.patch( - 'neutron.plugins.ml2.plugin.' 'db.get_locked_port_and_binding', - return_value=(None, None)) as glpab_mock,\ + side = exc.PortNotFound(port_id=port['port']['id']) + with mock.patch.object(plugin, '_get_port', + side_effect=side) as gp_mock,\ mock.patch('neutron.plugins.ml2.plugin.Ml2Plugin.' '_make_port_dict') as mpd_mock: plugin._bind_port_if_needed(mech_context) # called during deletion to get port - self.assertTrue(glpab_mock.mock_calls) + self.assertTrue(gp_mock.mock_calls) # should have returned before calling _make_port_dict self.assertFalse(mpd_mock.mock_calls) def _create_port_and_bound_context(self, port_vif_type, bound_vif_type): with self.port() as port: plugin = directory.get_plugin() - binding = ml2_db.get_locked_port_and_binding(self.context, - port['port']['id'])[1] + binding = plugin._get_port( + self.context, port['port']['id']).port_binding binding['host'] = 'fake_host' binding['vif_type'] = port_vif_type # Generates port context to be used before the bind. @@ -1644,8 +1641,8 @@ class TestMl2PortBinding(Ml2PluginV2TestCase, def test_update_port_binding_host_id_none(self): with self.port() as port: plugin = directory.get_plugin() - binding = ml2_db.get_locked_port_and_binding(self.context, - port['port']['id'])[1] + binding = plugin._get_port( + self.context, port['port']['id']).port_binding binding['host'] = 'test' mech_context = driver_context.PortContext( plugin, self.context, port['port'], @@ -1661,8 +1658,8 @@ class TestMl2PortBinding(Ml2PluginV2TestCase, def test_update_port_binding_host_id_not_changed(self): with self.port() as port: plugin = directory.get_plugin() - binding = ml2_db.get_locked_port_and_binding(self.context, - port['port']['id'])[1] + binding = plugin._get_port( + self.context, port['port']['id']).port_binding binding['host'] = 'test' mech_context = driver_context.PortContext( plugin, self.context, port['port'], @@ -2600,8 +2597,8 @@ class TestML2Segments(Ml2PluginV2TestCase): ml2_db.subscribe() plugin = directory.get_plugin() with self.port(device_owner=fake_owner_compute) as port: - binding = ml2_db.get_locked_port_and_binding(self.context, - port['port']['id'])[1] + binding = plugin._get_port( + self.context, port['port']['id']).port_binding binding['host'] = 'host-ovs-no_filter' mech_context = driver_context.PortContext( plugin, self.context, port['port'],