From: Oleg Bondarev <obondarev@mirantis.com>
Date: Wed, 12 Aug 2015 17:02:01 +0000 (+0300)
Subject: Avoid DB errors when deleting network's ports and subnets
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a55e10cfd6369533f0cc22edd6611c9549b8f1b4;p=openstack-build%2Fneutron-build.git

Avoid DB errors when deleting network's ports and subnets

DB errors may occur when accessing query results
after the transaction was closed (like ObjectDeletedError).
Hence it's better to avoid DB object access especially
when it's not needed.
This patch changes _delete_ports() and _delete_subnets() to accept
only ids. Indeed, there is no need to pass db objects to these methods.

Closes-Bug: #1484135
Related-Bug: #1454408
Change-Id: I7507cb1c85defb2e6d5144e5832aea713d6251ae
---

diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index 8035e6a21..0366b8291 100644
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -699,32 +699,30 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
 
         return [self._fields(net, fields) for net in nets]
 
-    def _delete_ports(self, context, ports):
-        for port in ports:
+    def _delete_ports(self, context, port_ids):
+        for port_id in port_ids:
             try:
-                self.delete_port(context, port.id)
-            except (exc.PortNotFound, sa_exc.ObjectDeletedError):
-                context.session.expunge(port)
+                self.delete_port(context, port_id)
+            except exc.PortNotFound:
                 # concurrent port deletion can be performed by
                 # release_dhcp_port caused by concurrent subnet_delete
-                LOG.info(_LI("Port %s was deleted concurrently"), port.id)
+                LOG.info(_LI("Port %s was deleted concurrently"), port_id)
             except Exception:
                 with excutils.save_and_reraise_exception():
                     LOG.exception(_LE("Exception auto-deleting port %s"),
-                                  port.id)
+                                  port_id)
 
-    def _delete_subnets(self, context, subnets):
-        for subnet in subnets:
+    def _delete_subnets(self, context, subnet_ids):
+        for subnet_id in subnet_ids:
             try:
-                self.delete_subnet(context, subnet.id)
-            except (exc.SubnetNotFound, sa_exc.ObjectDeletedError):
-                context.session.expunge(subnet)
+                self.delete_subnet(context, subnet_id)
+            except exc.SubnetNotFound:
                 LOG.info(_LI("Subnet %s was deleted concurrently"),
-                         subnet.id)
+                         subnet_id)
             except Exception:
                 with excutils.save_and_reraise_exception():
                     LOG.exception(_LE("Exception auto-deleting subnet %s"),
-                                  subnet.id)
+                                  subnet_id)
 
     def delete_network(self, context, id):
         # REVISIT(rkukura) The super(Ml2Plugin, self).delete_network()
@@ -788,6 +786,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                         # network record, so explicit removal is not necessary.
                         LOG.debug("Committing transaction")
                         break
+
+                    port_ids = [port.id for port in ports]
+                    subnet_ids = [subnet.id for subnet in subnets]
             except os_db_exception.DBError as e:
                 with excutils.save_and_reraise_exception() as ctxt:
                     if isinstance(e.inner_exception, sql_exc.IntegrityError):
@@ -795,8 +796,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                         LOG.warning(_LW("A concurrent port creation has "
                                         "occurred"))
                         continue
-            self._delete_ports(context, ports)
-            self._delete_subnets(context, subnets)
+            self._delete_ports(context, port_ids)
+            self._delete_subnets(context, subnet_ids)
 
         try:
             self.mechanism_manager.delete_network_postcommit(mech_context)
diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py
index 56c0a3d27..7dc84b50c 100644
--- a/neutron/tests/unit/plugins/ml2/test_plugin.py
+++ b/neutron/tests/unit/plugins/ml2/test_plugin.py
@@ -24,7 +24,6 @@ import webob
 
 from oslo_db import exception as db_exc
 from oslo_utils import uuidutils
-from sqlalchemy.orm import exc as sqla_exc
 
 from neutron.callbacks import registry
 from neutron.common import constants
@@ -220,18 +219,12 @@ class TestMl2NetworksV2(test_plugin.TestNetworksV2,
         with mock.patch.object(plugin, "delete_port",
                                side_effect=exc.PortNotFound(port_id="123")):
             plugin._delete_ports(mock.MagicMock(), [mock.MagicMock()])
-        with mock.patch.object(plugin, "delete_port",
-                               side_effect=sqla_exc.ObjectDeletedError(None)):
-            plugin._delete_ports(mock.MagicMock(), [mock.MagicMock()])
 
     def test_subnet_delete_helper_tolerates_failure(self):
         plugin = manager.NeutronManager.get_plugin()
         with mock.patch.object(plugin, "delete_subnet",
                                side_effect=exc.SubnetNotFound(subnet_id="1")):
             plugin._delete_subnets(mock.MagicMock(), [mock.MagicMock()])
-        with mock.patch.object(plugin, "delete_subnet",
-                               side_effect=sqla_exc.ObjectDeletedError(None)):
-            plugin._delete_subnets(mock.MagicMock(), [mock.MagicMock()])
 
     def _create_and_verify_networks(self, networks):
         for net_idx, net in enumerate(networks):