Merge "Remove long db transaction for metadata access network"
This commit is contained in:
Jenkins
Gerrit Code Review
commit
e1b6ab10c9
@ -1801,32 +1801,10 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
|
||||
else:
|
||||
raise l3.RouterInterfaceNotFoundForSubnet(router_id=router_id,
|
||||
subnet_id=subnet_id)
|
||||
results = nvplib.query_lswitch_lports(
|
||||
self.cluster, '*', relations="LogicalPortAttachment",
|
||||
filters={'tag': port_id, 'tag_scope': 'q_port_id'})
|
||||
lrouter_port_id = None
|
||||
if results:
|
||||
lport = results[0]
|
||||
attachment_data = lport['_relations'].get('LogicalPortAttachment')
|
||||
lrouter_port_id = (attachment_data and
|
||||
attachment_data.get('peer_port_uuid'))
|
||||
else:
|
||||
LOG.warning(_("The port %(port_id)s, connected to the router "
|
||||
"%(router_id)s was not found on the NVP backend"),
|
||||
{'port_id': port_id, 'router_id': router_id})
|
||||
# Finally remove the data from the Neutron DB
|
||||
# This will also destroy the port on the logical switch
|
||||
info = super(NvpPluginV2, self).remove_router_interface(
|
||||
context, router_id, interface_info)
|
||||
# Destroy router port (no need to unplug the attachment)
|
||||
# FIXME(salvatore-orlando): In case of failures in the Neutron plugin
|
||||
# this migth leave a dangling port. We perform the operation here
|
||||
# to leverage validation performed in the base class
|
||||
if not lrouter_port_id:
|
||||
LOG.warning(_("Unable to find NVP logical router port for "
|
||||
"Neutron port id:%s. Was this port ever paired "
|
||||
"with a logical router?"), port_id)
|
||||
return info
|
||||
|
||||
# Ensure the connection to the 'metadata access network'
|
||||
# is removed (with the network) if this the last subnet
|
||||
@ -1850,8 +1828,8 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
|
||||
destination_ip_addresses=subnet['cidr'])
|
||||
except NvpApiClient.ResourceNotFound:
|
||||
raise nvp_exc.NvpPluginException(
|
||||
err_msg=(_("Logical router port resource %s not found "
|
||||
"on NVP platform"), lrouter_port_id))
|
||||
err_msg=(_("Logical router resource %s not found "
|
||||
"on NVP platform") % router_id))
|
||||
except NvpApiClient.NvpApiException:
|
||||
raise nvp_exc.NvpPluginException(
|
||||
err_msg=(_("Unable to update logical router"
|
||||
|
||||
@ -17,13 +17,15 @@
|
||||
#
|
||||
# @author: Salvatore Orlando, VMware
|
||||
|
||||
from eventlet import greenthread
|
||||
import netaddr
|
||||
from oslo.config import cfg
|
||||
|
||||
from neutron.api.rpc.agentnotifiers import dhcp_rpc_agent_api
|
||||
from neutron.api.v2 import attributes
|
||||
from neutron.common import constants
|
||||
from neutron.common import exceptions as q_exc
|
||||
from neutron.common import exceptions as ntn_exc
|
||||
from neutron.db import db_base_plugin_v2
|
||||
from neutron.db import l3_db
|
||||
from neutron.db import models_v2
|
||||
from neutron.openstack.common import log as logging
|
||||
@ -50,18 +52,21 @@ class NvpMetadataAccess(object):
|
||||
return port
|
||||
|
||||
def _create_metadata_access_network(self, context, router_id):
|
||||
# This will still ensure atomicity on Neutron DB
|
||||
with context.session.begin(subtransactions=True):
|
||||
# Add network
|
||||
# Network name is likely to be truncated on NVP
|
||||
net_data = {'name': 'meta-%s' % router_id,
|
||||
'tenant_id': '', # intentionally not set
|
||||
'admin_state_up': True,
|
||||
'port_security_enabled': False,
|
||||
'shared': False,
|
||||
'status': constants.NET_STATUS_ACTIVE}
|
||||
meta_net = self.create_network(context,
|
||||
{'network': net_data})
|
||||
# Add network
|
||||
# Network name is likely to be truncated on NVP
|
||||
net_data = {'name': 'meta-%s' % router_id,
|
||||
'tenant_id': '', # intentionally not set
|
||||
'admin_state_up': True,
|
||||
'port_security_enabled': False,
|
||||
'shared': False,
|
||||
'status': constants.NET_STATUS_ACTIVE}
|
||||
meta_net = self.create_network(context,
|
||||
{'network': net_data})
|
||||
greenthread.sleep(0) # yield
|
||||
# From this point on there will be resources to garbage-collect
|
||||
# in case of failures
|
||||
meta_sub = None
|
||||
try:
|
||||
# Add subnet
|
||||
subnet_data = {'network_id': meta_net['id'],
|
||||
'tenant_id': '', # intentionally not set
|
||||
@ -77,34 +82,52 @@ class NvpMetadataAccess(object):
|
||||
'host_routes': []}
|
||||
meta_sub = self.create_subnet(context,
|
||||
{'subnet': subnet_data})
|
||||
greenthread.sleep(0) # yield
|
||||
self.add_router_interface(context, router_id,
|
||||
{'subnet_id': meta_sub['id']})
|
||||
if cfg.CONF.dhcp_agent_notification:
|
||||
# We need to send a notification to the dhcp agent in
|
||||
# order to start the metadata agent proxy
|
||||
dhcp_notifier = dhcp_rpc_agent_api.DhcpAgentNotifyAPI()
|
||||
dhcp_notifier.notify(context, {'network': meta_net},
|
||||
'network.create.end')
|
||||
greenthread.sleep(0) # yield
|
||||
except (ntn_exc.NeutronException,
|
||||
nvp_exc.NvpPluginException,
|
||||
NvpApiClient.NvpApiException):
|
||||
# It is not necessary to explicitly delete the subnet
|
||||
# as it will be removed with the network
|
||||
self.delete_network(context, meta_net['id'])
|
||||
|
||||
if cfg.CONF.dhcp_agent_notification:
|
||||
# We need to send a notification to the dhcp agent in
|
||||
# order to start the metadata agent proxy
|
||||
dhcp_notifier = dhcp_rpc_agent_api.DhcpAgentNotifyAPI()
|
||||
dhcp_notifier.notify(context, {'network': meta_net},
|
||||
'network.create.end')
|
||||
|
||||
def _destroy_metadata_access_network(self, context, router_id, ports):
|
||||
# This will still ensure atomicity on Neutron DB
|
||||
with context.session.begin(subtransactions=True):
|
||||
if ports:
|
||||
meta_port = self._find_metadata_port(context, ports)
|
||||
if not meta_port:
|
||||
return
|
||||
meta_net_id = meta_port['network_id']
|
||||
self.remove_router_interface(
|
||||
context, router_id, {'port_id': meta_port['id']})
|
||||
# Remove network (this will remove the subnet too)
|
||||
self.delete_network(context, meta_net_id)
|
||||
if cfg.CONF.dhcp_agent_notification:
|
||||
# We need to send a notification to the dhcp agent in
|
||||
# order to stop the metadata agent proxy
|
||||
dhcp_notifier = dhcp_rpc_agent_api.DhcpAgentNotifyAPI()
|
||||
dhcp_notifier.notify(context,
|
||||
{'network': {'id': meta_net_id}},
|
||||
'network.delete.end')
|
||||
if not ports:
|
||||
return
|
||||
meta_port = self._find_metadata_port(context, ports)
|
||||
if not meta_port:
|
||||
return
|
||||
meta_net_id = meta_port['network_id']
|
||||
meta_sub_id = meta_port['fixed_ips'][0]['subnet_id']
|
||||
self.remove_router_interface(
|
||||
context, router_id, {'port_id': meta_port['id']})
|
||||
greenthread.sleep(0) # yield
|
||||
try:
|
||||
# Remove network (this will remove the subnet too)
|
||||
self.delete_network(context, meta_net_id)
|
||||
greenthread.sleep(0) # yield
|
||||
except (ntn_exc.NeutronException, nvp_exc.NvpPluginException,
|
||||
NvpApiClient.NvpApiException):
|
||||
# must re-add the router interface
|
||||
self.add_router_interface(context, router_id,
|
||||
{'subnet_id': meta_sub_id})
|
||||
|
||||
if cfg.CONF.dhcp_agent_notification:
|
||||
# We need to send a notification to the dhcp agent in
|
||||
# order to stop the metadata agent proxy
|
||||
dhcp_notifier = dhcp_rpc_agent_api.DhcpAgentNotifyAPI()
|
||||
dhcp_notifier.notify(context,
|
||||
{'network': {'id': meta_net_id}},
|
||||
'network.delete.end')
|
||||
|
||||
def _handle_metadata_access_network(self, context, router_id,
|
||||
do_create=True):
|
||||
@ -120,35 +143,32 @@ class NvpMetadataAccess(object):
|
||||
ctx_elevated = context.elevated()
|
||||
device_filter = {'device_id': [router_id],
|
||||
'device_owner': [l3_db.DEVICE_OWNER_ROUTER_INTF]}
|
||||
with ctx_elevated.session.begin(subtransactions=True):
|
||||
# Retrieve ports without going to plugin
|
||||
ports = [self._make_port_dict(port)
|
||||
for port in self._get_ports_query(
|
||||
ctx_elevated, filters=device_filter)
|
||||
if port['fixed_ips']]
|
||||
try:
|
||||
if ports:
|
||||
if (do_create and
|
||||
not self._find_metadata_port(ctx_elevated, ports)):
|
||||
self._create_metadata_access_network(ctx_elevated,
|
||||
router_id)
|
||||
elif len(ports) == 1:
|
||||
# The only port left is the metadata port
|
||||
self._destroy_metadata_access_network(ctx_elevated,
|
||||
router_id,
|
||||
ports)
|
||||
else:
|
||||
LOG.debug(_("No router interface found for router '%s'. "
|
||||
"No metadata access network should be "
|
||||
"created or destroyed"), router_id)
|
||||
# TODO(salvatore-orlando): A better exception handling in the
|
||||
# NVP plugin would allow us to improve error handling here
|
||||
except (q_exc.NeutronException, nvp_exc.NvpPluginException,
|
||||
NvpApiClient.NvpApiException):
|
||||
# Any exception here should be regarded as non-fatal
|
||||
LOG.exception(_("An error occurred while operating on the "
|
||||
"metadata access network for router:'%s'"),
|
||||
router_id)
|
||||
# Retrieve ports calling database plugin
|
||||
ports = db_base_plugin_v2.NeutronDbPluginV2.get_ports(
|
||||
self, context, filters=device_filter)
|
||||
try:
|
||||
if ports:
|
||||
if (do_create and
|
||||
not self._find_metadata_port(ctx_elevated, ports)):
|
||||
self._create_metadata_access_network(ctx_elevated,
|
||||
router_id)
|
||||
elif len(ports) == 1:
|
||||
# The only port left might be the metadata port
|
||||
self._destroy_metadata_access_network(ctx_elevated,
|
||||
router_id,
|
||||
ports)
|
||||
else:
|
||||
LOG.debug(_("No router interface found for router '%s'. "
|
||||
"No metadata access network should be "
|
||||
"created or destroyed"), router_id)
|
||||
# TODO(salvatore-orlando): A better exception handling in the
|
||||
# NVP plugin would allow us to improve error handling here
|
||||
except (ntn_exc.NeutronException, nvp_exc.NvpPluginException,
|
||||
NvpApiClient.NvpApiException):
|
||||
# Any exception here should be regarded as non-fatal
|
||||
LOG.exception(_("An error occurred while operating on the "
|
||||
"metadata access network for router:'%s'"),
|
||||
router_id)
|
||||
|
||||
def _ensure_metadata_host_route(self, context, fixed_ip_data,
|
||||
is_delete=False):
|
||||
|
||||
@ -553,7 +553,59 @@ class TestNiciraL3NatTestCase(test_l3_plugin.L3NatDBTestCase,
|
||||
None)
|
||||
self._nvp_metadata_teardown()
|
||||
|
||||
def test_metadatata_network_removed_with_router_interface_remove(self):
|
||||
def test_metadata_network_create_rollback_on_create_subnet_failure(self):
|
||||
self._nvp_metadata_setup()
|
||||
with self.router() as r:
|
||||
with self.subnet() as s:
|
||||
# Raise a NeutronException (eg: NotFound)
|
||||
with mock.patch.object(NeutronPlugin.NvpPluginV2,
|
||||
'create_subnet',
|
||||
side_effect=ntn_exc.NotFound):
|
||||
self._router_interface_action(
|
||||
'add', r['router']['id'], s['subnet']['id'], None)
|
||||
# Ensure metadata network was removed
|
||||
nets = self._list('networks')['networks']
|
||||
self.assertEqual(len(nets), 1)
|
||||
# Needed to avoid 409
|
||||
self._router_interface_action('remove',
|
||||
r['router']['id'],
|
||||
s['subnet']['id'],
|
||||
None)
|
||||
self._nvp_metadata_teardown()
|
||||
|
||||
def test_metadata_network_create_rollback_on_add_rtr_iface_failure(self):
|
||||
self._nvp_metadata_setup()
|
||||
with self.router() as r:
|
||||
with self.subnet() as s:
|
||||
# Raise a NeutronException when adding metadata subnet
|
||||
# to router
|
||||
# save function being mocked
|
||||
real_func = NeutronPlugin.NvpPluginV2.add_router_interface
|
||||
plugin_instance = manager.NeutronManager.get_plugin()
|
||||
|
||||
def side_effect(*args):
|
||||
if args[-1]['subnet_id'] == s['subnet']['id']:
|
||||
# do the real thing
|
||||
return real_func(plugin_instance, *args)
|
||||
# otherwise raise
|
||||
raise NvpApiClient.NvpApiException()
|
||||
|
||||
with mock.patch.object(NeutronPlugin.NvpPluginV2,
|
||||
'add_router_interface',
|
||||
side_effect=side_effect):
|
||||
self._router_interface_action(
|
||||
'add', r['router']['id'], s['subnet']['id'], None)
|
||||
# Ensure metadata network was removed
|
||||
nets = self._list('networks')['networks']
|
||||
self.assertEqual(len(nets), 1)
|
||||
# Needed to avoid 409
|
||||
self._router_interface_action('remove',
|
||||
r['router']['id'],
|
||||
s['subnet']['id'],
|
||||
None)
|
||||
self._nvp_metadata_teardown()
|
||||
|
||||
def test_metadata_network_removed_with_router_interface_remove(self):
|
||||
self._nvp_metadata_setup()
|
||||
with self.router() as r:
|
||||
with self.subnet() as s:
|
||||
@ -582,6 +634,45 @@ class TestNiciraL3NatTestCase(test_l3_plugin.L3NatDBTestCase,
|
||||
webob.exc.HTTPNotFound.code)
|
||||
self._nvp_metadata_teardown()
|
||||
|
||||
def test_metadata_network_remove_rollback_on_failure(self):
|
||||
self._nvp_metadata_setup()
|
||||
with self.router() as r:
|
||||
with self.subnet() as s:
|
||||
self._router_interface_action('add', r['router']['id'],
|
||||
s['subnet']['id'], None)
|
||||
networks = self._list('networks')['networks']
|
||||
for network in networks:
|
||||
if network['id'] != s['subnet']['network_id']:
|
||||
meta_net_id = network['id']
|
||||
ports = self._list(
|
||||
'ports',
|
||||
query_params='network_id=%s' % meta_net_id)['ports']
|
||||
meta_port_id = ports[0]['id']
|
||||
# Raise a NeutronException when removing
|
||||
# metadata subnet from router
|
||||
# save function being mocked
|
||||
real_func = NeutronPlugin.NvpPluginV2.remove_router_interface
|
||||
plugin_instance = manager.NeutronManager.get_plugin()
|
||||
|
||||
def side_effect(*args):
|
||||
if args[-1].get('subnet_id') == s['subnet']['id']:
|
||||
# do the real thing
|
||||
return real_func(plugin_instance, *args)
|
||||
# otherwise raise
|
||||
raise NvpApiClient.NvpApiException()
|
||||
|
||||
with mock.patch.object(NeutronPlugin.NvpPluginV2,
|
||||
'remove_router_interface',
|
||||
side_effect=side_effect):
|
||||
self._router_interface_action('remove', r['router']['id'],
|
||||
s['subnet']['id'], None)
|
||||
# Metadata network and subnet should still be there
|
||||
self._show('networks', meta_net_id,
|
||||
webob.exc.HTTPOk.code)
|
||||
self._show('ports', meta_port_id,
|
||||
webob.exc.HTTPOk.code)
|
||||
self._nvp_metadata_teardown()
|
||||
|
||||
def test_metadata_dhcp_host_route(self):
|
||||
cfg.CONF.set_override('metadata_mode', 'dhcp_host_route', 'NVP')
|
||||
subnets = self._list('subnets')['subnets']
|
||||
|
||||
Loading…
Reference in New Issue
Block a user