Merge "Make create_object_with_dependency cleanup"

neutron/common/utils.py

@@ -518,7 +518,7 @@ def port_rule_masking(port_min, port_max):
 
 
 def create_object_with_dependency(creator, dep_getter, dep_creator,
-                                  dep_id_attr):
+                                  dep_id_attr, dep_deleter):
     """Creates an object that binds to a dependency while handling races.
 
     creator is a function that expected to take the result of either
@@ -528,6 +528,9 @@ def create_object_with_dependency(creator, dep_getter, dep_creator,
     dep_id_attr be used to determine if the dependency changed during object
     creation.
 
+    dep_deleter will be called with a the result of dep_creator if the creator
+    function fails due to a non-dependency reason or the retries are exceeded.
+
     dep_getter should return None if the dependency does not exist.
 
     dep_creator can raise a DBDuplicateEntry to indicate that a concurrent
@@ -542,17 +545,16 @@ def create_object_with_dependency(creator, dep_getter, dep_creator,
     process of creating the dependency if one no longer exists. It will
     give up after neutron.db.api.MAX_RETRIES and raise the exception it
     encounters after that.
-
-    TODO(kevinbenton): currently this does not try to delete the dependency
-    it created. This matches the semantics of the HA network logic it is used
-    for but it should be modified to cleanup in the future.
     """
-    result, dependency, dep_id = None, None, None
+    result, dependency, dep_id, made_locally = None, None, None, False
     for attempts in range(1, db_api.MAX_RETRIES + 1):
         # we go to max + 1 here so the exception handlers can raise their
         # errors at the end
         try:
-            dependency = dep_getter() or dep_creator()
+            dependency = dep_getter()
+            if not dependency:
+                dependency = dep_creator()
+                made_locally = True
             dep_id = getattr(dependency, dep_id_attr)
         except db_exc.DBDuplicateEntry:
             # dependency was concurrently created.
@@ -576,6 +578,16 @@ def create_object_with_dependency(creator, dep_getter, dep_creator,
                     if not dependency or dep_id != getattr(dependency,
                                                            dep_id_attr):
                         ctx.reraise = False
+                        continue
+                # we have exceeded retries or have encountered a non-dependency
+                # related failure so we try to clean up the dependency if we
+                # created it before re-raising
+                if made_locally and dependency:
+                    try:
+                        dep_deleter(dependency)
+                    except Exception:
+                        LOG.exception(_LE("Failed cleaning up dependency %s"),
+                                      dep_id)
     return result, dependency
 
 

neutron/db/l3_hamode_db.py

@@ -457,9 +457,10 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin,
                                        context, router_db.tenant_id)
         dep_creator = functools.partial(self._create_ha_network,
                                         context, router_db.tenant_id)
+        dep_deleter = functools.partial(self._delete_ha_network, context)
         dep_id_attr = 'network_id'
         return n_utils.create_object_with_dependency(
-            creator, dep_getter, dep_creator, dep_id_attr)
+            creator, dep_getter, dep_creator, dep_id_attr, dep_deleter)
 
     def create_router(self, context, router):
         is_ha = self._is_ha(router['router'])

neutron/scheduler/l3_agent_scheduler.py

@@ -275,10 +275,11 @@ class L3Scheduler(object):
         dep_getter = functools.partial(plugin.get_ha_network, ctxt, tenant_id)
         dep_creator = functools.partial(plugin._create_ha_network,
                                         ctxt, tenant_id)
+        dep_deleter = functools.partial(plugin._delete_ha_network, ctxt)
         dep_id_attr = 'network_id'
         try:
             port_binding = utils.create_object_with_dependency(
-                creator, dep_getter, dep_creator, dep_id_attr)[0]
+                creator, dep_getter, dep_creator, dep_id_attr, dep_deleter)[0]
             with db_api.autonested_transaction(context.session):
                 port_binding.l3_agent_id = agent['id']
         except db_exc.DBDuplicateEntry:

neutron/tests/unit/db/test_l3_hamode_db.py

@@ -647,6 +647,14 @@ class L3HATestCase(L3HATestFramework):
                 self.admin_ctx, router_db)
             self.assertTrue(_create_ha_interfaces.called)
 
+    def test_create_ha_interfaces_and_ensure_network_interface_failure(self):
+
+        def _create_ha_interfaces(ctx, rdb, ha_net):
+            raise ValueError('broken')
+        with testtools.ExpectedException(ValueError):
+            self._test_ensure_with_patched_int_create(_create_ha_interfaces)
+        self.assertEqual([], self.core_plugin.get_networks(self.admin_ctx))
+
     def test_create_ha_interfaces_and_ensure_network_concurrent_delete(self):
         orig_create = self.plugin._create_ha_interfaces