Avoid notifying while inside transaction opened in delete_port()
delete_port() calls to disassociate_floatingips() while in transaction.
The latter method sends RPC notification which may result in eventlet
yield. If yield switches a thread to another one that tries to access
the same floating IP object in db as disassociate_floatingips() method
does, we're locked and get db timeout.
We should avoid calling to notifier while under transaction.
To achieve this, I introduce a do_notify argument that controls whether
notification is done by disassociate_floatingips() itself or delegated
to caller. Callers that call to disassociate_floatingips() from under
transactions should handle notifications on their own. For this,
disassociate_floatingips() returns a set of routers that require
notification.
Updated drivers to reflect new behaviour. Added unit test.
Conflicts:
neutron/db/l3_db.py
neutron/plugins/bigswitch/plugin.py
neutron/plugins/nuage/plugin.py
Change-Id: I2411f2aa778ea088be416d062c4816c16f49d2bf
Closes-Bug: 1330955
(cherry picked from commit 876c2c25e1
)
This commit is contained in:
parent
543e3ac99e
commit
9c94d96031
|
@ -747,12 +747,14 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
|
|||
{'port_id': port_db['id'],
|
||||
'port_owner': port_db['device_owner']})
|
||||
|
||||
def disassociate_floatingips(self, context, port_id):
|
||||
def disassociate_floatingips(self, context, port_id, do_notify=True):
|
||||
router_ids = []
|
||||
|
||||
with context.session.begin(subtransactions=True):
|
||||
try:
|
||||
fip_qry = context.session.query(FloatingIP)
|
||||
floating_ip = fip_qry.filter_by(fixed_port_id=port_id).one()
|
||||
router_id = floating_ip['router_id']
|
||||
router_ids.append(floating_ip['router_id'])
|
||||
floating_ip.update({'fixed_port_id': None,
|
||||
'fixed_ip_address': None,
|
||||
'router_id': None})
|
||||
|
@ -762,9 +764,18 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
|
|||
# should never happen
|
||||
raise Exception(_('Multiple floating IPs found for port %s')
|
||||
% port_id)
|
||||
if router_id:
|
||||
if do_notify:
|
||||
self.notify_routers_updated(context, router_ids)
|
||||
# since caller assumes that we handled notifications on its
|
||||
# behalf, return nothing
|
||||
return
|
||||
|
||||
return router_ids
|
||||
|
||||
def notify_routers_updated(self, context, router_ids):
|
||||
if router_ids:
|
||||
self.l3_rpc_notifier.routers_updated(
|
||||
context, [router_id])
|
||||
context, router_ids)
|
||||
|
||||
def _build_routers_list(self, routers, gw_ports):
|
||||
gw_port_id_gw_port_dict = dict((gw_port['id'], gw_port)
|
||||
|
|
|
@ -817,7 +817,8 @@ class NeutronRestProxyV2(NeutronRestProxyV2Base,
|
|||
if l3_port_check:
|
||||
self.prevent_l3_port_deletion(context, port_id)
|
||||
with context.session.begin(subtransactions=True):
|
||||
self.disassociate_floatingips(context, port_id)
|
||||
router_ids = self.disassociate_floatingips(
|
||||
context, port_id, do_notify=False)
|
||||
self._delete_port_security_group_bindings(context, port_id)
|
||||
port = super(NeutronRestProxyV2, self).get_port(context, port_id)
|
||||
# Tenant ID must come from network in case the network is shared
|
||||
|
@ -825,6 +826,9 @@ class NeutronRestProxyV2(NeutronRestProxyV2Base,
|
|||
self._delete_port(context, port_id)
|
||||
self.servers.rest_delete_port(tenid, port['network_id'], port_id)
|
||||
|
||||
# now that we've left db transaction, we are safe to notify
|
||||
self.notify_routers_updated(context, router_ids)
|
||||
|
||||
@put_context_in_serverpool
|
||||
def create_subnet(self, context, subnet):
|
||||
LOG.debug(_("NeutronRestProxyV2: create_subnet() called"))
|
||||
|
@ -1095,11 +1099,12 @@ class NeutronRestProxyV2(NeutronRestProxyV2Base,
|
|||
self._send_floatingip_update(context)
|
||||
|
||||
@put_context_in_serverpool
|
||||
def disassociate_floatingips(self, context, port_id):
|
||||
def disassociate_floatingips(self, context, port_id, do_notify=True):
|
||||
LOG.debug(_("NeutronRestProxyV2: diassociate_floatingips() called"))
|
||||
super(NeutronRestProxyV2, self).disassociate_floatingips(context,
|
||||
port_id)
|
||||
router_ids = super(NeutronRestProxyV2, self).disassociate_floatingips(
|
||||
context, port_id, do_notify=do_notify)
|
||||
self._send_floatingip_update(context)
|
||||
return router_ids
|
||||
|
||||
def _send_floatingip_update(self, context):
|
||||
try:
|
||||
|
|
|
@ -1229,8 +1229,13 @@ class N1kvNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
n1kv_db_v2.delete_vm_network(context.session,
|
||||
port[n1kv.PROFILE_ID],
|
||||
port['network_id'])
|
||||
self.disassociate_floatingips(context, id)
|
||||
router_ids = self.disassociate_floatingips(
|
||||
context, id, do_notify=False)
|
||||
super(N1kvNeutronPluginV2, self).delete_port(context, id)
|
||||
|
||||
# now that we've left db transaction, we are safe to notify
|
||||
self.notify_routers_updated(context, router_ids)
|
||||
|
||||
self._send_delete_port_request(context, port, vm_network)
|
||||
|
||||
def get_port(self, context, id, fields=None):
|
||||
|
|
|
@ -353,14 +353,15 @@ class EmbranePlugin(object):
|
|||
args=(nat_info,))
|
||||
return result
|
||||
|
||||
def disassociate_floatingips(self, context, port_id):
|
||||
def disassociate_floatingips(self, context, port_id, do_notify=True):
|
||||
try:
|
||||
fip_qry = context.session.query(l3_db.FloatingIP)
|
||||
floating_ip = fip_qry.filter_by(fixed_port_id=port_id).one()
|
||||
router_id = floating_ip["router_id"]
|
||||
except exc.NoResultFound:
|
||||
return
|
||||
self._l3super.disassociate_floatingips(self, context, port_id)
|
||||
router_ids = self._l3super.disassociate_floatingips(
|
||||
self, context, port_id, do_notify=do_notify)
|
||||
if router_id:
|
||||
neutron_router = self._get_router(context, router_id)
|
||||
fip_id = floating_ip["id"]
|
||||
|
@ -373,3 +374,4 @@ class EmbranePlugin(object):
|
|||
p_con.Events.RESET_NAT_RULE, neutron_router, context,
|
||||
state_change),
|
||||
args=(fip_id,))
|
||||
return router_ids
|
||||
|
|
|
@ -524,11 +524,14 @@ class LinuxBridgePluginV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
|
||||
session = context.session
|
||||
with session.begin(subtransactions=True):
|
||||
self.disassociate_floatingips(context, id)
|
||||
router_ids = self.disassociate_floatingips(
|
||||
context, id, do_notify=False)
|
||||
port = self.get_port(context, id)
|
||||
self._delete_port_security_group_bindings(context, id)
|
||||
super(LinuxBridgePluginV2, self).delete_port(context, id)
|
||||
|
||||
# now that we've left db transaction, we are safe to notify
|
||||
self.notify_routers_updated(context, router_ids)
|
||||
self.notify_security_groups_member_updated(context, port)
|
||||
|
||||
def _notify_port_updated(self, context, port):
|
||||
|
|
|
@ -736,10 +736,15 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
self._delete_port_security_group_bindings(context, id)
|
||||
LOG.debug(_("Calling base delete_port"))
|
||||
if l3plugin:
|
||||
l3plugin.disassociate_floatingips(context, id)
|
||||
router_ids = l3plugin.disassociate_floatingips(
|
||||
context, id, do_notify=False)
|
||||
|
||||
super(Ml2Plugin, self).delete_port(context, id)
|
||||
|
||||
# now that we've left db transaction, we are safe to notify
|
||||
if l3plugin:
|
||||
l3plugin.notify_routers_updated(context, router_ids)
|
||||
|
||||
try:
|
||||
self.mechanism_manager.delete_port_postcommit(mech_context)
|
||||
except ml2_exc.MechanismDriverError:
|
||||
|
|
|
@ -502,9 +502,12 @@ class MellanoxEswitchPlugin(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
|
||||
session = context.session
|
||||
with session.begin(subtransactions=True):
|
||||
self.disassociate_floatingips(context, port_id)
|
||||
router_ids = self.disassociate_floatingips(
|
||||
context, port_id, do_notify=False)
|
||||
port = self.get_port(context, port_id)
|
||||
self._delete_port_security_group_bindings(context, port_id)
|
||||
super(MellanoxEswitchPlugin, self).delete_port(context, port_id)
|
||||
|
||||
# now that we've left db transaction, we are safe to notify
|
||||
self.notify_routers_updated(context, router_ids)
|
||||
self.notify_security_groups_member_updated(context, port)
|
||||
|
|
|
@ -652,9 +652,13 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
if l3_port_check:
|
||||
self.prevent_l3_port_deletion(context, id)
|
||||
with context.session.begin(subtransactions=True):
|
||||
self.disassociate_floatingips(context, id)
|
||||
router_ids = self.disassociate_floatingips(
|
||||
context, id, do_notify=False)
|
||||
self._delete_port_security_group_bindings(context, id)
|
||||
super(NECPluginV2, self).delete_port(context, id)
|
||||
|
||||
# now that we've left db transaction, we are safe to notify
|
||||
self.notify_routers_updated(context, router_ids)
|
||||
self.notify_security_groups_member_updated(context, port)
|
||||
|
||||
|
||||
|
|
|
@ -356,7 +356,8 @@ class OneConvergencePluginV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
|
||||
self._delete_port_security_group_bindings(context, port_id)
|
||||
|
||||
self.disassociate_floatingips(context, port_id)
|
||||
router_ids = self.disassociate_floatingips(
|
||||
context, port_id, do_notify=False)
|
||||
|
||||
super(OneConvergencePluginV2, self).delete_port(context, port_id)
|
||||
|
||||
|
@ -365,6 +366,8 @@ class OneConvergencePluginV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
|
||||
self.nvsdlib.delete_port(port_id, neutron_port)
|
||||
|
||||
# now that we've left db transaction, we are safe to notify
|
||||
self.notify_routers_updated(context, router_ids)
|
||||
self.notify_security_groups_member_updated(context, neutron_port)
|
||||
|
||||
def create_floatingip(self, context, floatingip):
|
||||
|
|
|
@ -627,9 +627,12 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
|
||||
session = context.session
|
||||
with session.begin(subtransactions=True):
|
||||
self.disassociate_floatingips(context, id)
|
||||
router_ids = self.disassociate_floatingips(
|
||||
context, id, do_notify=False)
|
||||
port = self.get_port(context, id)
|
||||
self._delete_port_security_group_bindings(context, id)
|
||||
super(OVSNeutronPluginV2, self).delete_port(context, id)
|
||||
|
||||
# now that we've left db transaction, we are safe to notify
|
||||
self.notify_routers_updated(context, router_ids)
|
||||
self.notify_security_groups_member_updated(context, port)
|
||||
|
|
|
@ -242,7 +242,8 @@ class NeutronPluginPLUMgridV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
# Plugin DB - Port Create and Return port
|
||||
port_db = super(NeutronPluginPLUMgridV2,
|
||||
self).get_port(context, port_id)
|
||||
self.disassociate_floatingips(context, port_id)
|
||||
router_ids = self.disassociate_floatingips(
|
||||
context, port_id, do_notify=False)
|
||||
super(NeutronPluginPLUMgridV2, self).delete_port(context, port_id)
|
||||
|
||||
if port_db["device_owner"] == constants.DEVICE_OWNER_ROUTER_GW:
|
||||
|
@ -257,6 +258,9 @@ class NeutronPluginPLUMgridV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
except Exception as err_message:
|
||||
raise plum_excep.PLUMgridException(err_msg=err_message)
|
||||
|
||||
# now that we've left db transaction, we are safe to notify
|
||||
self.notify_routers_updated(context, router_ids)
|
||||
|
||||
def get_port(self, context, id, fields=None):
|
||||
with context.session.begin(subtransactions=True):
|
||||
port_db = super(NeutronPluginPLUMgridV2,
|
||||
|
@ -528,7 +532,7 @@ class NeutronPluginPLUMgridV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
except Exception as err_message:
|
||||
raise plum_excep.PLUMgridException(err_msg=err_message)
|
||||
|
||||
def disassociate_floatingips(self, context, port_id):
|
||||
def disassociate_floatingips(self, context, port_id, do_notify=True):
|
||||
LOG.debug(_("Neutron PLUMgrid Director: disassociate_floatingips() "
|
||||
"called"))
|
||||
|
||||
|
@ -546,8 +550,8 @@ class NeutronPluginPLUMgridV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
except Exception as err_message:
|
||||
raise plum_excep.PLUMgridException(err_msg=err_message)
|
||||
|
||||
super(NeutronPluginPLUMgridV2,
|
||||
self).disassociate_floatingips(context, port_id)
|
||||
return super(NeutronPluginPLUMgridV2, self).disassociate_floatingips(
|
||||
context, port_id, do_notify=do_notify)
|
||||
|
||||
"""
|
||||
Internal PLUMgrid Fuctions
|
||||
|
|
|
@ -235,11 +235,14 @@ class RyuNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
self.prevent_l3_port_deletion(context, id)
|
||||
|
||||
with context.session.begin(subtransactions=True):
|
||||
self.disassociate_floatingips(context, id)
|
||||
router_ids = self.disassociate_floatingips(
|
||||
context, id, do_notify=False)
|
||||
port = self.get_port(context, id)
|
||||
self._delete_port_security_group_bindings(context, id)
|
||||
super(RyuNeutronPluginV2, self).delete_port(context, id)
|
||||
|
||||
# now that we've left db transaction, we are safe to notify
|
||||
self.notify_routers_updated(context, router_ids)
|
||||
self.notify_security_groups_member_updated(context, port)
|
||||
|
||||
def update_port(self, context, id, port):
|
||||
|
|
|
@ -13,6 +13,7 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import contextlib
|
||||
import mock
|
||||
import testtools
|
||||
import webob
|
||||
|
@ -23,6 +24,7 @@ from neutron.extensions import multiprovidernet as mpnet
|
|||
from neutron.extensions import portbindings
|
||||
from neutron.extensions import providernet as pnet
|
||||
from neutron import manager
|
||||
from neutron.plugins.common import constants as service_constants
|
||||
from neutron.plugins.ml2.common import exceptions as ml2_exc
|
||||
from neutron.plugins.ml2 import config
|
||||
from neutron.plugins.ml2 import plugin as ml2_plugin
|
||||
|
@ -119,6 +121,42 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase):
|
|||
mock.call(_("The port '%s' was deleted"), 'invalid-uuid')
|
||||
])
|
||||
|
||||
def test_delete_port_no_notify_in_disassociate_floatingips(self):
|
||||
ctx = context.get_admin_context()
|
||||
plugin = manager.NeutronManager.get_plugin()
|
||||
l3plugin = manager.NeutronManager.get_service_plugins().get(
|
||||
service_constants.L3_ROUTER_NAT)
|
||||
with contextlib.nested(
|
||||
self.port(no_delete=True),
|
||||
mock.patch.object(l3plugin, 'disassociate_floatingips'),
|
||||
mock.patch.object(l3plugin, 'notify_routers_updated')
|
||||
) as (port, disassociate_floatingips, notify):
|
||||
|
||||
port_id = port['port']['id']
|
||||
plugin.delete_port(ctx, port_id)
|
||||
|
||||
# check that no notification was requested while under
|
||||
# transaction
|
||||
disassociate_floatingips.assert_has_calls([
|
||||
mock.call(ctx, port_id, do_notify=False)
|
||||
])
|
||||
|
||||
# check that notifier was still triggered
|
||||
notify.assert_has_calls([
|
||||
mock.call(ctx, disassociate_floatingips.return_value)
|
||||
])
|
||||
|
||||
def test_disassociate_floatingips_do_notify_returns_nothing(self):
|
||||
ctx = context.get_admin_context()
|
||||
l3plugin = manager.NeutronManager.get_service_plugins().get(
|
||||
service_constants.L3_ROUTER_NAT)
|
||||
with self.port() as port:
|
||||
|
||||
port_id = port['port']['id']
|
||||
# check that nothing is returned when notifications are handled
|
||||
# by the called method
|
||||
self.assertIsNone(l3plugin.disassociate_floatingips(ctx, port_id))
|
||||
|
||||
|
||||
class TestMl2PortBinding(Ml2PluginV2TestCase,
|
||||
test_bindings.PortBindingsTestCase):
|
||||
|
|
Loading…
Reference in New Issue