x/vmware-nsx
Code Issues Proposed changes

Merge "Remove long db transaction for metadata access network"

Browse Source
This commit is contained in:
Jenkins 2013-08-15 21:42:15 +00:00 committed by Gerrit Code Review
commit 6dc7f321c6
3 changed files with 180 additions and 91 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
neutron/plugins/nicira/NeutronPlugin.py
View File

@ -1801,32 +1801,10 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
else: else:
raise l3.RouterInterfaceNotFoundForSubnet(router_id=router_id, raise l3.RouterInterfaceNotFoundForSubnet(router_id=router_id,
subnet_id=subnet_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 # Finally remove the data from the Neutron DB
# This will also destroy the port on the logical switch # This will also destroy the port on the logical switch
info = super(NvpPluginV2, self).remove_router_interface( info = super(NvpPluginV2, self).remove_router_interface(
context, router_id, interface_info) 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' # Ensure the connection to the 'metadata access network'
# is removed (with the network) if this the last subnet # 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']) destination_ip_addresses=subnet['cidr'])
except NvpApiClient.ResourceNotFound: except NvpApiClient.ResourceNotFound:
raise nvp_exc.NvpPluginException( raise nvp_exc.NvpPluginException(
err_msg=(_("Logical router port resource %s not found " err_msg=(_("Logical router resource %s not found "
"on NVP platform"), lrouter_port_id)) "on NVP platform") % router_id))
except NvpApiClient.NvpApiException: except NvpApiClient.NvpApiException:
raise nvp_exc.NvpPluginException( raise nvp_exc.NvpPluginException(
err_msg=(_("Unable to update logical router" err_msg=(_("Unable to update logical router"

152
neutron/plugins/nicira/common/metadata_access.py
View File

@ -17,13 +17,15 @@
# #
# @author: Salvatore Orlando, VMware # @author: Salvatore Orlando, VMware
from eventlet import greenthread
import netaddr import netaddr
from oslo.config import cfg from oslo.config import cfg
from neutron.api.rpc.agentnotifiers import dhcp_rpc_agent_api from neutron.api.rpc.agentnotifiers import dhcp_rpc_agent_api
from neutron.api.v2 import attributes from neutron.api.v2 import attributes
from neutron.common import constants 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 l3_db
from neutron.db import models_v2 from neutron.db import models_v2
from neutron.openstack.common import log as logging from neutron.openstack.common import log as logging
@ -50,18 +52,21 @@ class NvpMetadataAccess(object):
return port return port
def _create_metadata_access_network(self, context, router_id): def _create_metadata_access_network(self, context, router_id):
# This will still ensure atomicity on Neutron DB # Add network
with context.session.begin(subtransactions=True): # Network name is likely to be truncated on NVP
# Add network net_data = {'name': 'meta-%s' % router_id,
# Network name is likely to be truncated on NVP 'tenant_id': '', # intentionally not set
net_data = {'name': 'meta-%s' % router_id, 'admin_state_up': True,
'tenant_id': '', # intentionally not set 'port_security_enabled': False,
'admin_state_up': True, 'shared': False,
'port_security_enabled': False, 'status': constants.NET_STATUS_ACTIVE}
'shared': False, meta_net = self.create_network(context,
'status': constants.NET_STATUS_ACTIVE} {'network': net_data})
meta_net = self.create_network(context, greenthread.sleep(0) # yield
{'network': net_data}) # From this point on there will be resources to garbage-collect
# in case of failures
meta_sub = None
try:
# Add subnet # Add subnet
subnet_data = {'network_id': meta_net['id'], subnet_data = {'network_id': meta_net['id'],
'tenant_id': '', # intentionally not set 'tenant_id': '', # intentionally not set
@ -77,34 +82,52 @@ class NvpMetadataAccess(object):
'host_routes': []} 'host_routes': []}
meta_sub = self.create_subnet(context, meta_sub = self.create_subnet(context,
{'subnet': subnet_data}) {'subnet': subnet_data})
greenthread.sleep(0) # yield
self.add_router_interface(context, router_id, self.add_router_interface(context, router_id,
{'subnet_id': meta_sub['id']}) {'subnet_id': meta_sub['id']})
if cfg.CONF.dhcp_agent_notification: greenthread.sleep(0) # yield
# We need to send a notification to the dhcp agent in except (ntn_exc.NeutronException,
# order to start the metadata agent proxy nvp_exc.NvpPluginException,
dhcp_notifier = dhcp_rpc_agent_api.DhcpAgentNotifyAPI() NvpApiClient.NvpApiException):
dhcp_notifier.notify(context, {'network': meta_net}, # It is not necessary to explicitly delete the subnet
'network.create.end') # 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): def _destroy_metadata_access_network(self, context, router_id, ports):
# This will still ensure atomicity on Neutron DB if not ports:
with context.session.begin(subtransactions=True): return
if ports: meta_port = self._find_metadata_port(context, ports)
meta_port = self._find_metadata_port(context, ports) if not meta_port:
if not meta_port: return
return meta_net_id = meta_port['network_id']
meta_net_id = meta_port['network_id'] meta_sub_id = meta_port['fixed_ips'][0]['subnet_id']
self.remove_router_interface( self.remove_router_interface(
context, router_id, {'port_id': meta_port['id']}) context, router_id, {'port_id': meta_port['id']})
# Remove network (this will remove the subnet too) greenthread.sleep(0) # yield
self.delete_network(context, meta_net_id) try:
if cfg.CONF.dhcp_agent_notification: # Remove network (this will remove the subnet too)
# We need to send a notification to the dhcp agent in self.delete_network(context, meta_net_id)
# order to stop the metadata agent proxy greenthread.sleep(0) # yield
dhcp_notifier = dhcp_rpc_agent_api.DhcpAgentNotifyAPI() except (ntn_exc.NeutronException, nvp_exc.NvpPluginException,
dhcp_notifier.notify(context, NvpApiClient.NvpApiException):
{'network': {'id': meta_net_id}}, # must re-add the router interface
'network.delete.end') 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, def _handle_metadata_access_network(self, context, router_id,
do_create=True): do_create=True):
@ -120,35 +143,32 @@ class NvpMetadataAccess(object):
ctx_elevated = context.elevated() ctx_elevated = context.elevated()
device_filter = {'device_id': [router_id], device_filter = {'device_id': [router_id],
'device_owner': [l3_db.DEVICE_OWNER_ROUTER_INTF]} 'device_owner': [l3_db.DEVICE_OWNER_ROUTER_INTF]}
with ctx_elevated.session.begin(subtransactions=True): # Retrieve ports calling database plugin
# Retrieve ports without going to plugin ports = db_base_plugin_v2.NeutronDbPluginV2.get_ports(
ports = [self._make_port_dict(port) self, context, filters=device_filter)
for port in self._get_ports_query( try:
ctx_elevated, filters=device_filter) if ports:
if port['fixed_ips']] if (do_create and
try: not self._find_metadata_port(ctx_elevated, ports)):
if ports: self._create_metadata_access_network(ctx_elevated,
if (do_create and router_id)
not self._find_metadata_port(ctx_elevated, ports)): elif len(ports) == 1:
self._create_metadata_access_network(ctx_elevated, # The only port left might be the metadata port
router_id) self._destroy_metadata_access_network(ctx_elevated,
elif len(ports) == 1: router_id,
# The only port left is the metadata port ports)
self._destroy_metadata_access_network(ctx_elevated, else:
router_id, LOG.debug(_("No router interface found for router '%s'. "
ports) "No metadata access network should be "
else: "created or destroyed"), router_id)
LOG.debug(_("No router interface found for router '%s'. " # TODO(salvatore-orlando): A better exception handling in the
"No metadata access network should be " # NVP plugin would allow us to improve error handling here
"created or destroyed"), router_id) except (ntn_exc.NeutronException, nvp_exc.NvpPluginException,
# TODO(salvatore-orlando): A better exception handling in the NvpApiClient.NvpApiException):
# NVP plugin would allow us to improve error handling here # Any exception here should be regarded as non-fatal
except (q_exc.NeutronException, nvp_exc.NvpPluginException, LOG.exception(_("An error occurred while operating on the "
NvpApiClient.NvpApiException): "metadata access network for router:'%s'"),
# Any exception here should be regarded as non-fatal router_id)
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, def _ensure_metadata_host_route(self, context, fixed_ip_data,
is_delete=False): is_delete=False):

93
neutron/tests/unit/nicira/test_nicira_plugin.py
View File

@ -553,7 +553,59 @@ class TestNiciraL3NatTestCase(test_l3_plugin.L3NatDBTestCase,
None) None)
self._nvp_metadata_teardown() 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() self._nvp_metadata_setup()
with self.router() as r: with self.router() as r:
with self.subnet() as s: with self.subnet() as s:
@ -582,6 +634,45 @@ class TestNiciraL3NatTestCase(test_l3_plugin.L3NatDBTestCase,
webob.exc.HTTPNotFound.code) webob.exc.HTTPNotFound.code)
self._nvp_metadata_teardown() 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): def test_metadata_dhcp_host_route(self):
cfg.CONF.set_override('metadata_mode', 'dhcp_host_route', 'NVP') cfg.CONF.set_override('metadata_mode', 'dhcp_host_route', 'NVP')
subnets = self._list('subnets')['subnets'] subnets = self._list('subnets')['subnets']