diff --git a/neutron/common/utils.py b/neutron/common/utils.py index d8e88df092a..64e0bb3f0f0 100644 --- a/neutron/common/utils.py +++ b/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 diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py index 478d7f3669a..dd45b3b22c4 100644 --- a/neutron/db/l3_hamode_db.py +++ b/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']) diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py index 15ad24e127c..e9046dcfe09 100644 --- a/neutron/scheduler/l3_agent_scheduler.py +++ b/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: diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py index 955de8091df..4d9e3a7540d 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/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