From 270fd80391ea1dd8f3be13d38df81be71ae751ee Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Wed, 10 Jul 2013 14:44:12 +0200 Subject: [PATCH] Separate NVP create lport operation and neutron db transaction Bug 1200001 This patch removes the NVP call for creating a logical port from the SQL transaction context used for creating the Neutron port. It also ensures orphaned data are properly removed from both the Neutron DB and the NVP backend. Change-Id: I028a1493ecf732f2422e0eaa2020bac4ebdbb457 --- neutron/plugins/nicira/NeutronPlugin.py | 149 +++++++++++------- neutron/plugins/nicira/dbexts/nicira_db.py | 5 + .../tests/unit/nicira/test_nicira_plugin.py | 30 ++++ 3 files changed, 130 insertions(+), 54 deletions(-) diff --git a/neutron/plugins/nicira/NeutronPlugin.py b/neutron/plugins/nicira/NeutronPlugin.py index 597c5aa75..7da652096 100644 --- a/neutron/plugins/nicira/NeutronPlugin.py +++ b/neutron/plugins/nicira/NeutronPlugin.py @@ -435,6 +435,22 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, port_data[ext_qos.QUEUE], port_data.get(mac_ext.MAC_LEARNING)) + def _handle_create_port_exception(self, context, port_id, + ls_uuid, lp_uuid): + with excutils.save_and_reraise_exception(): + # rollback nvp logical port only if it was successfully + # created on NVP. Should this command fail the original + # exception will be raised. + if lp_uuid: + # Remove orphaned port from NVP + nvplib.delete_port(self.cluster, ls_uuid, lp_uuid) + # rollback the neutron-nvp port mapping + nicira_db.delete_neutron_nvp_port_mapping(context.session, + port_id) + msg = (_("An exception occured while creating the " + "quantum port %s on the NVP plaform") % port_id) + LOG.exception(msg) + def _nvp_create_port(self, context, port_data): """Driver for creating a logical switch port on NVP platform.""" # FIXME(salvatore-orlando): On the NVP platform we do not really have @@ -448,6 +464,8 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, port_data['network_id']) # No need to actually update the DB state - the default is down return port_data + lport = None + selected_lswitch = None try: selected_lswitch = self._nvp_find_lswitch_for_port(context, port_data) @@ -466,11 +484,11 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, LOG.debug(_("_nvp_create_port completed for port %(name)s " "on network %(network_id)s. The new port id is " "%(id)s."), port_data) - except NvpApiClient.NvpApiException: - msg = (_("An exception occured while plugging the interface " - "into network:%s") % port_data['network_id']) - LOG.exception(msg) - raise q_exc.NeutronException(message=msg) + except (NvpApiClient.NvpApiException, q_exc.NeutronException): + self._handle_create_port_exception( + context, port_data['id'], + selected_lswitch and selected_lswitch['uuid'], + lport and lport['uuid']) def _nvp_delete_port(self, context, port_data): # FIXME(salvatore-orlando): On the NVP platform we do not really have @@ -515,7 +533,7 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, lrouter_id, port_data['network_id'], nvp_port_id) - except (NvpApiClient.NvpApiException, NvpApiClient.ResourceNotFound): + except NvpApiClient.NvpApiException: # Do not raise because the issue might as well be that the # router has already been deleted, so there would be nothing # to do here @@ -534,18 +552,26 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, err_msg=(_("It is not allowed to create router interface " "ports on external networks as '%s'") % port_data['network_id'])) - selected_lswitch = self._nvp_find_lswitch_for_port(context, - port_data) - # Do not apply port security here! - lport = self._nvp_create_port_helper(self.cluster, - selected_lswitch['uuid'], - port_data, - False) - nicira_db.add_neutron_nvp_port_mapping( - context.session, port_data['id'], lport['uuid']) - LOG.debug(_("_nvp_create_port completed for port %(name)s on " - "network %(network_id)s. The new port id is %(id)s."), - port_data) + lport = None + selected_lswitch = None + try: + selected_lswitch = self._nvp_find_lswitch_for_port( + context, port_data) + # Do not apply port security here! + lport = self._nvp_create_port_helper( + self.cluster, selected_lswitch['uuid'], + port_data, False) + nicira_db.add_neutron_nvp_port_mapping( + context.session, port_data['id'], lport['uuid']) + LOG.debug(_("_nvp_create_router_port completed for port " + "%(name)s on network %(network_id)s. The new " + "port id is %(id)s."), + port_data) + except (NvpApiClient.NvpApiException, q_exc.NeutronException): + self._handle_create_port_exception( + context, port_data['id'], + selected_lswitch and selected_lswitch['uuid'], + lport and lport['uuid']) def _find_router_gw_port(self, context, port_data): router_id = port_data['device_id'] @@ -651,21 +677,30 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, port_data['network_id']) # No need to actually update the DB state - the default is down return port_data - selected_lswitch = self._nvp_find_lswitch_for_port(context, - port_data) - lport = self._nvp_create_port_helper(self.cluster, - selected_lswitch['uuid'], - port_data, - True) - nicira_db.add_neutron_nvp_port_mapping( - context.session, port_data['id'], lport['uuid']) - nvplib.plug_l2_gw_service( - self.cluster, - port_data['network_id'], - lport['uuid'], - port_data['device_id'], - int(port_data.get('gw:segmentation_id') or 0)) - LOG.debug(_("_nvp_create_port completed for port %(name)s " + lport = None + try: + selected_lswitch = self._nvp_find_lswitch_for_port( + context, port_data) + lport = self._nvp_create_port_helper( + self.cluster, + selected_lswitch['uuid'], + port_data, + True) + nicira_db.add_neutron_nvp_port_mapping( + context.session, port_data['id'], lport['uuid']) + nvplib.plug_l2_gw_service( + self.cluster, + port_data['network_id'], + lport['uuid'], + port_data['device_id'], + int(port_data.get('gw:segmentation_id') or 0)) + except Exception: + with excutils.save_and_reraise_exception(): + if lport: + nvplib.delete_port(self.cluster, + selected_lswitch['uuid'], + lport['uuid']) + LOG.debug(_("_nvp_create_l2_gw_port completed for port %(name)s " "on network %(network_id)s. The new port id " "is %(id)s."), port_data) @@ -1205,6 +1240,7 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, with context.session.begin(subtransactions=True): # First we allocate port in neutron database neutron_db = super(NvpPluginV2, self).create_port(context, port) + neutron_port_id = neutron_db['id'] # Update fields obtained from neutron db (eg: MAC address) port["port"].update(neutron_db) # metadata_dhcp_host_route @@ -1236,27 +1272,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, self._create_mac_learning_state(context, port_data) elif mac_ext.MAC_LEARNING in port_data: port_data.pop(mac_ext.MAC_LEARNING) - # provider networking extension checks - # Fetch the network and network binding from neutron db - try: - port_data = port['port'].copy() - port_create_func = self._port_drivers['create'].get( - port_data['device_owner'], - self._port_drivers['create']['default']) - - port_create_func(context, port_data) - except q_exc.NotFound: - LOG.warning(_("Network %s was not found in NVP."), - port_data['network_id']) - port_data['status'] = constants.PORT_STATUS_ERROR - except Exception: - with excutils.save_and_reraise_exception(): - # FIXME (arosen) or the plugin_interface call failed in - # which case we need to garbage collect the left over - # port in nvp. - err_msg = _("Unable to create port or set port " - "attachment in NVP.") - LOG.error(err_msg) LOG.debug(_("create_port completed on NVP for tenant " "%(tenant_id)s: (%(id)s)"), port_data) @@ -1267,6 +1282,32 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, self._extend_port_qos_queue(context, port_data) self._process_portbindings_create_and_update(context, port, port_data) + # DB Operation is complete, perform NVP operation + try: + port_data = port['port'].copy() + port_create_func = self._port_drivers['create'].get( + port_data['device_owner'], + self._port_drivers['create']['default']) + port_create_func(context, port_data) + except q_exc.NotFound: + LOG.warning(_("Logical switch for network %s was not " + "found in NVP."), port_data['network_id']) + # Put port in error on quantum DB + with context.session.begin(subtransactions=True): + port = self._get_port(context, neutron_port_id) + port_data['status'] = constants.PORT_STATUS_ERROR + port['status'] = port_data['status'] + context.session.add(port) + except Exception: + # Port must be removed from Quantum DB + with excutils.save_and_reraise_exception(): + LOG.error(_("Unable to create port or set port " + "attachment in NVP.")) + with context.session.begin(subtransactions=True): + self._delete_port(context, neutron_port_id) + + # Port has been created both on DB and NVP - proceed with + # scheduling network and notifying DHCP agent net = self.get_network(context, port_data['network_id']) self.schedule_network(context, net) if notify_dhcp_agent: diff --git a/neutron/plugins/nicira/dbexts/nicira_db.py b/neutron/plugins/nicira/dbexts/nicira_db.py index 86b546231..68f165c41 100644 --- a/neutron/plugins/nicira/dbexts/nicira_db.py +++ b/neutron/plugins/nicira/dbexts/nicira_db.py @@ -72,6 +72,11 @@ def get_nvp_port_id(session, neutron_id): return +def delete_neutron_nvp_port_mapping(session, neutron_id): + return (session.query(nicira_models.NeutronNvpPortMapping). + filter_by(quantum_id=neutron_id).delete()) + + def unset_default_network_gateways(session): with session.begin(subtransactions=True): session.query(nicira_networkgw_db.NetworkGateway).update( diff --git a/neutron/tests/unit/nicira/test_nicira_plugin.py b/neutron/tests/unit/nicira/test_nicira_plugin.py index 94215b355..57ea077a0 100644 --- a/neutron/tests/unit/nicira/test_nicira_plugin.py +++ b/neutron/tests/unit/nicira/test_nicira_plugin.py @@ -21,6 +21,7 @@ from oslo.config import cfg import webob.exc from neutron.common import constants +from neutron.common import exceptions as ntn_exc import neutron.common.test_lib as test_lib from neutron import context from neutron.extensions import l3 @@ -29,6 +30,7 @@ from neutron.extensions import providernet as pnet from neutron.extensions import securitygroup as secgrp from neutron import manager from neutron.openstack.common import uuidutils +from neutron.plugins.nicira.dbexts import nicira_db from neutron.plugins.nicira.dbexts import nicira_qos_db as qos_db from neutron.plugins.nicira.extensions import nvp_networkgw from neutron.plugins.nicira.extensions import nvp_qos as ext_qos @@ -162,6 +164,34 @@ class TestNiciraPortsV2(test_plugin.TestPortsV2, # Assert the neutron name is not truncated self.assertEqual(name, port['port']['name']) + def _verify_no_orphan_left(self, net_id): + # Verify no port exists on net + # ie: cleanup on db was successful + query_params = "network_id=%s" % net_id + self._test_list_resources('port', [], + query_params=query_params) + # Also verify no orphan port was left on nvp + # no port should be there at all + self.assertFalse(self.fc._fake_lswitch_lport_dict) + + def test_create_port_nvp_error_no_orphan_left(self): + with mock.patch.object(nvplib, 'create_lport', + side_effect=NvpApiClient.NvpApiException): + with self.network() as net: + net_id = net['network']['id'] + self._create_port(self.fmt, net_id, + webob.exc.HTTPInternalServerError.code) + self._verify_no_orphan_left(net_id) + + def test_create_port_neutron_error_no_orphan_left(self): + with mock.patch.object(nicira_db, 'add_neutron_nvp_port_mapping', + side_effect=ntn_exc.NeutronException): + with self.network() as net: + net_id = net['network']['id'] + self._create_port(self.fmt, net_id, + webob.exc.HTTPInternalServerError.code) + self._verify_no_orphan_left(net_id) + class TestNiciraNetworksV2(test_plugin.TestNetworksV2, NiciraPluginV2TestCase): -- 2.45.2