Merge "Separate NVP create lport operation and neutron db transaction"

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:
+        except (NvpApiClient.NvpApiException, q_exc.NeutronException):
-            msg = (_("An exception occured while plugging the interface "
+            self._handle_create_port_exception(
-                     "into network:%s") % port_data['network_id'])
+                context, port_data['id'],
-            LOG.exception(msg)
+                selected_lswitch and selected_lswitch['uuid'],
-            raise q_exc.NeutronException(message=msg)
+                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,
+        lport = None
-                                                           port_data)
+        selected_lswitch = None
-        # Do not apply port security here!
+        try:
-        lport = self._nvp_create_port_helper(self.cluster,
+            selected_lswitch = self._nvp_find_lswitch_for_port(
-                                             selected_lswitch['uuid'],
+                context, port_data)
-                                             port_data,
+            # Do not apply port security here!
-                                             False)
+            lport = self._nvp_create_port_helper(
-        nicira_db.add_neutron_nvp_port_mapping(
+                self.cluster, selected_lswitch['uuid'],
-            context.session, port_data['id'], lport['uuid'])
+                port_data, False)
-        LOG.debug(_("_nvp_create_port completed for port %(name)s on "
+            nicira_db.add_neutron_nvp_port_mapping(
-                    "network %(network_id)s. The new port id is %(id)s."),
+                context.session, port_data['id'], lport['uuid'])
-                  port_data)
+            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,
+        lport = None
-                                                           port_data)
+        try:
-        lport = self._nvp_create_port_helper(self.cluster,
+            selected_lswitch = self._nvp_find_lswitch_for_port(
-                                             selected_lswitch['uuid'],
+                context, port_data)
-                                             port_data,
+            lport = self._nvp_create_port_helper(
-                                             True)
+                self.cluster,
-        nicira_db.add_neutron_nvp_port_mapping(
+                selected_lswitch['uuid'],
-            context.session, port_data['id'], lport['uuid'])
+                port_data,
-        nvplib.plug_l2_gw_service(
+                True)
-            self.cluster,
+            nicira_db.add_neutron_nvp_port_mapping(
-            port_data['network_id'],
+                context.session, port_data['id'], lport['uuid'])
-            lport['uuid'],
+            nvplib.plug_l2_gw_service(
-            port_data['device_id'],
+                self.cluster,
-            int(port_data.get('gw:segmentation_id') or 0))
+                port_data['network_id'],
-        LOG.debug(_("_nvp_create_port completed for port %(name)s "
+                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:

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(

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):