Handle racing condition in OFC port deletion

Multiple delete_port operations can run in parallel.
For such case later operation(s) will receive 404 (NotFound)
error from OFC or NotResultFound sqlalchemy exception.
These are valid exceptions and they should be ignored
in delete_port operations.

Closes-Bug: #1281574

OFCConsistencyBroken is renamed to OFCMappingNotFound
because when multiple delete_port operations run in parallel
OFCConsistencyBroken can occur and it is a valid case
so the excepion name looks inappropriate.

Change-Id: I1511d55994c88b8828f0ff62610c18ddc6dfac8f

diff --git a/neutron/plugins/nec/common/exceptions.py b/neutron/plugins/nec/common/exceptions.py
index d6b2b3b5003a63d99d16c2434ed167b74b4c3a1e..0045b54607e28b2cc0c01c4559af44d8e4ae33b5 100644 (file)
--- a/neutron/plugins/nec/common/exceptions.py
+++ b/neutron/plugins/nec/common/exceptions.py
@@ -28,13 +28,18 @@ class OFCException(qexc.NeutronException):
         self.err_code = kwargs.get('err_code')
 
 
+class OFCResourceNotFound(qexc.NotFound):
+    message = _("The specified OFC resource (%(resource)s) is not found.")
+
+
 class NECDBException(qexc.NeutronException):
     message = _("An exception occurred in NECPluginV2 DB: %(reason)s")
 
 
-class OFCConsistencyBroken(qexc.NeutronException):
-    message = _("Consistency of neutron-OFC resource map is broken: "
-                "%(reason)s")
+class OFCMappingNotFound(qexc.NotFound):
+    message = _("Neutron-OFC resource mapping for "
+                "%(resource)s %(neutron_id)s is not found. "
+                "It may be deleted during processing.")
 
 
 class PortInfoNotFound(qexc.NotFound):

diff --git a/neutron/plugins/nec/common/ofc_client.py b/neutron/plugins/nec/common/ofc_client.py
index 4bc49b42a982bf9235e72ce268ae9f6db2846eae..7fbfe9ec459304e589629ff4dd048718e7d91aeb 100644 (file)
--- a/neutron/plugins/nec/common/ofc_client.py
+++ b/neutron/plugins/nec/common/ofc_client.py
@@ -95,6 +95,10 @@ class OFCClient(object):
                               httplib.ACCEPTED,
                               httplib.NO_CONTENT):
                 return data
+            elif res.status == httplib.NOT_FOUND:
+                LOG.info(_("Specified resource %s does not exist on OFC "),
+                         action)
+                raise nexc.OFCResourceNotFound(resource=action)
             else:
                 LOG.warning(_("Operation on OFC failed: "
                               "status=%(status)s, detail=%(detail)s"),

diff --git a/neutron/plugins/nec/db/api.py b/neutron/plugins/nec/db/api.py
index e606861ff2247cdb9298fb66b587cd6c247c4c19..85c5f8601f55211af15bd7f887e491e63ce62c65 100644 (file)
--- a/neutron/plugins/nec/db/api.py
+++ b/neutron/plugins/nec/db/api.py
@@ -144,9 +144,8 @@ def get_ofc_id_lookup_both(session, resource, neutron_id):
         ofc_id = get_ofc_id(session, resource, neutron_id,
                             old_style=True)
     if not ofc_id:
-        reason = (_("NotFound %(resource)s for neutron_id=%(id)s.")
-                  % {'resource': resource, 'id': neutron_id})
-        raise nexc.OFCConsistencyBroken(reason=reason)
+        raise nexc.OFCMappingNotFound(resource=resource,
+                                      neutron_id=neutron_id)
     return ofc_id
 
 

diff --git a/neutron/plugins/nec/nec_plugin.py b/neutron/plugins/nec/nec_plugin.py
index 41aaf61f7c96e473a6f298d148fa4a6dd142a0bf..519da2ccfd2d8ca1dd9b49bd5cd8f2711bb4253f 100644 (file)
--- a/neutron/plugins/nec/nec_plugin.py
+++ b/neutron/plugins/nec/nec_plugin.py
@@ -187,7 +187,7 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
                 else:
                     LOG.debug(_('_cleanup_ofc_tenant: No OFC tenant for %s'),
                               tenant_id)
-            except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+            except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
                 reason = _("delete_ofc_tenant() failed due to %s") % exc
                 LOG.warn(reason)
 
@@ -223,7 +223,7 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
         try:
             self.ofc.create_ofc_port(context, port['id'], port)
             port_status = const.PORT_STATUS_ACTIVE
-        except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+        except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
             LOG.error(_("create_ofc_port() failed due to %s"), exc)
             port_status = const.PORT_STATUS_ERROR
 
@@ -244,7 +244,23 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
         try:
             self.ofc.delete_ofc_port(context, port['id'], port)
             port_status = const.PORT_STATUS_DOWN
-        except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+        except (nexc.OFCResourceNotFound, nexc.OFCMappingNotFound):
+            # There is a case where multiple delete_port operation are
+            # running concurrently. For example, delete_port from
+            # release_dhcp_port and deletion of network owned ports in
+            # delete_network. In such cases delete_ofc_port may receive
+            # 404 error from OFC.
+            # Also there is a case where neutron port is deleted
+            # between exists_ofc_port and get_ofc_id in delete_ofc_port.
+            # In this case OFCMappingNotFound is raised.
+            # These two cases are valid situations.
+            LOG.info(_("deactivate_port(): OFC port for port=%s is "
+                       "already removed."), port['id'])
+            # The port is already removed, so there is no need
+            # to update status in the database.
+            port['status'] = const.PORT_STATUS_DOWN
+            return port
+        except nexc.OFCException as exc:
             LOG.error(_("delete_ofc_port() failed due to %s"), exc)
             port_status = const.PORT_STATUS_ERROR
 
@@ -282,7 +298,7 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
             if not self.ofc.exists_ofc_tenant(context, tenant_id):
                 self.ofc.create_ofc_tenant(context, tenant_id)
             self.ofc.create_ofc_network(context, tenant_id, net_id, net_name)
-        except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+        except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
             LOG.error(_("Failed to create network id=%(id)s on "
                         "OFC: %(exc)s"), {'id': net_id, 'exc': exc})
             network['network']['status'] = const.NET_STATUS_ERROR
@@ -368,7 +384,7 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
 
         try:
             self.ofc.delete_ofc_network(context, id, net_db)
-        except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+        except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
             reason = _("delete_network() failed due to %s") % exc
             LOG.error(reason)
             self._update_resource_status(context, "network", net_db['id'],

diff --git a/neutron/plugins/nec/packet_filter.py b/neutron/plugins/nec/packet_filter.py
index 8c30bd420f2c3a45a0e1144148df0850e16774e0..2c831e641a80b704a25aa3846ffc9caacfb6a821 100644 (file)
--- a/neutron/plugins/nec/packet_filter.py
+++ b/neutron/plugins/nec/packet_filter.py
@@ -127,7 +127,7 @@ class PacketFilterMixin(pf_db.PacketFilterDbMixin):
                 self.ofc.create_ofc_packet_filter(context, pf_id,
                                                   packet_filter)
                 pf_status = pf_db.PF_STATUS_ACTIVE
-            except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+            except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
                 LOG.error(_("Failed to create packet_filter id=%(id)s on "
                             "OFC: %(exc)s"), {'id': pf_id, 'exc': str(exc)})
                 pf_status = pf_db.PF_STATUS_ERROR
@@ -153,7 +153,7 @@ class PacketFilterMixin(pf_db.PacketFilterDbMixin):
             try:
                 self.ofc.delete_ofc_packet_filter(context, pf_id)
                 pf_status = pf_db.PF_STATUS_DOWN
-            except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+            except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
                 LOG.error(_("Failed to delete packet_filter id=%(id)s from "
                             "OFC: %(exc)s"), {'id': pf_id, 'exc': str(exc)})
                 pf_status = pf_db.PF_STATUS_ERROR

diff --git a/neutron/plugins/nec/router_drivers.py b/neutron/plugins/nec/router_drivers.py
index 9781f0b6e0ec58597f35616ccb4553cbd154cb50..f1ac10cf36a1dbf5c6358e572c035bf833be2816 100644 (file)
--- a/neutron/plugins/nec/router_drivers.py
+++ b/neutron/plugins/nec/router_drivers.py
@@ -119,7 +119,7 @@ class RouterOpenFlowDriver(RouterDriverBase):
                                                 new_status)
             router['status'] = new_status
             return router
-        except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+        except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
             with excutils.save_and_reraise_exception():
                 if (isinstance(exc, nexc.OFCException) and
                     exc.status == httplib.CONFLICT):
@@ -151,7 +151,7 @@ class RouterOpenFlowDriver(RouterDriverBase):
                 self.plugin._update_resource_status(
                     context, "router", router_id, new_status)
                 new_router['status'] = new_status
-            except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+            except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
                 with excutils.save_and_reraise_exception():
                     reason = _("_update_ofc_routes() failed due to %s") % exc
                     LOG.error(reason)
@@ -164,7 +164,7 @@ class RouterOpenFlowDriver(RouterDriverBase):
     def delete_router(self, context, router_id, router):
         try:
             self.ofc.delete_ofc_router(context, router_id, router)
-        except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+        except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
             with excutils.save_and_reraise_exception():
                 LOG.error(_("delete_router() failed due to %s"), exc)
                 self.plugin._update_resource_status(
@@ -195,7 +195,7 @@ class RouterOpenFlowDriver(RouterDriverBase):
             self.plugin._update_resource_status(
                 context, "port", port_id, new_status)
             return port
-        except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+        except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
             with excutils.save_and_reraise_exception():
                 reason = _("add_router_interface() failed due to %s") % exc
                 LOG.error(reason)
@@ -213,7 +213,7 @@ class RouterOpenFlowDriver(RouterDriverBase):
                                                 new_status)
             port['status'] = new_status
             return port
-        except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc:
+        except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
             with excutils.save_and_reraise_exception():
                 reason = _("delete_router_interface() failed due to %s") % exc
                 LOG.error(reason)

diff --git a/neutron/tests/unit/nec/test_nec_plugin.py b/neutron/tests/unit/nec/test_nec_plugin.py
index 727308a9eae5df85281ad02702e1399a27c90e7c..fa061cfad347385c3d052579e0dec5503d764af4 100644 (file)
--- a/neutron/tests/unit/nec/test_nec_plugin.py
+++ b/neutron/tests/unit/nec/test_nec_plugin.py
@@ -843,3 +843,40 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
         ]
         self.ofc.assert_has_calls(expected)
         self.assertEqual(self.ofc.delete_ofc_port.call_count, 2)
+
+    def _test_delete_port_for_disappeared_ofc_port(self, raised_exc):
+        self.ofc.set_raise_exc('delete_ofc_port', raised_exc)
+
+        with self.port(no_delete=True) as port:
+            port_id = port['port']['id']
+
+            portinfo = {'id': port_id, 'port_no': 123}
+            self.rpcapi_update_ports(added=[portinfo])
+
+            self._delete('ports', port_id)
+
+            # Check the port on neutron db is deleted. NotFound for
+            # neutron port itself should be handled by called. It is
+            # consistent with ML2 behavior, but it may need to be
+            # revisit.
+            self._show('ports', port_id,
+                       expected_code=webob.exc.HTTPNotFound.code)
+
+        ctx = mock.ANY
+        port = mock.ANY
+        expected = [
+            mock.call.exists_ofc_port(ctx, port_id),
+            mock.call.create_ofc_port(ctx, port_id, port),
+            mock.call.exists_ofc_port(ctx, port_id),
+            mock.call.delete_ofc_port(ctx, port_id, port),
+        ]
+        self.ofc.assert_has_calls(expected)
+        self.assertEqual(self.ofc.delete_ofc_port.call_count, 1)
+
+    def test_delete_port_for_nonexist_ofc_port(self):
+        self._test_delete_port_for_disappeared_ofc_port(
+            nexc.OFCResourceNotFound(resource='ofc_port'))
+
+    def test_delete_port_for_noofcmap_ofc_port(self):
+        self._test_delete_port_for_disappeared_ofc_port(
+            nexc.OFCMappingNotFound(resource='port', neutron_id='port1'))

diff --git a/neutron/tests/unit/nec/test_ofc_client.py b/neutron/tests/unit/nec/test_ofc_client.py
index 1567ae9e0e5721f8b689ecf2abbec8fd8e8fb737..aba037edfa26e59aa7979b91e5427baaa14b56d8 100644 (file)
--- a/neutron/tests/unit/nec/test_ofc_client.py
+++ b/neutron/tests/unit/nec/test_ofc_client.py
@@ -72,6 +72,11 @@ class OFCClientTest(base.BaseTestCase):
         for status in [201, 202, 204]:
             self._test_do_request(status, None, None)
 
+    def test_do_request_returns_404(self):
+        resbody = ''
+        errmsg = _("The specified OFC resource (/somewhere) is not found.")
+        self._test_do_request(404, resbody, errmsg, nexc.OFCResourceNotFound)
+
     def test_do_request_error_no_body(self):
         errmsg = _("An OFC exception has occurred: Operation on OFC failed")
         exc_checks = {'status': 400, 'err_code': None, 'err_msg': None}