diff --git a/neutron_fwaas/services/firewall/fwaas_plugin.py b/neutron_fwaas/services/firewall/fwaas_plugin.py index 50f4e6f2d..53358e837 100644 --- a/neutron_fwaas/services/firewall/fwaas_plugin.py +++ b/neutron_fwaas/services/firewall/fwaas_plugin.py @@ -22,7 +22,7 @@ from oslo_config import cfg from oslo_log import log as logging import oslo_messaging -from neutron_fwaas._i18n import _LW +from neutron_fwaas._i18n import _LI, _LW from neutron_fwaas.common import fwaas_constants as f_const from neutron_fwaas.db.firewall import firewall_db from neutron_fwaas.db.firewall import firewall_router_insertion_db @@ -60,18 +60,22 @@ class FirewallCallbacks(object): def firewall_deleted(self, context, firewall_id, **kwargs): """Agent uses this to indicate firewall is deleted.""" LOG.debug("firewall_deleted() called") - with context.session.begin(subtransactions=True): - fw_db = self.plugin._get_firewall(context, firewall_id) - # allow to delete firewalls in ERROR state - if fw_db.status in (n_const.PENDING_DELETE, n_const.ERROR): - self.plugin.delete_db_firewall_object(context, firewall_id) - return True - else: - LOG.warning(_LW('Firewall %(fw)s unexpectedly deleted by ' - 'agent, status was %(status)s'), - {'fw': firewall_id, 'status': fw_db.status}) - fw_db.update({"status": n_const.ERROR}) - return False + try: + with context.session.begin(subtransactions=True): + fw_db = self.plugin._get_firewall(context, firewall_id) + # allow to delete firewalls in ERROR state + if fw_db.status in (n_const.PENDING_DELETE, n_const.ERROR): + self.plugin.delete_db_firewall_object(context, firewall_id) + return True + else: + LOG.warning(_LW('Firewall %(fw)s unexpectedly deleted by ' + 'agent, status was %(status)s'), + {'fw': firewall_id, 'status': fw_db.status}) + fw_db.update({"status": n_const.ERROR}) + return False + except fw_ext.FirewallNotFound: + LOG.info(_LI('Firewall %s already deleted'), firewall_id) + return True def get_firewalls_for_tenant(self, context, **kwargs): """Agent uses this to get all firewalls and rules for a tenant.""" diff --git a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin.py b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin.py index 9671776da..4ac227fdc 100644 --- a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin.py +++ b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin.py @@ -195,6 +195,47 @@ class TestFirewallCallbacks(TestFirewallRouterInsertionBase): self.plugin.get_firewall, ctx, fw_id) + def test_firewall_deleted_concurrently(self): + ctx = context.get_admin_context() + alt_ctx = context.get_admin_context() + + _get_firewall = self.plugin._get_firewall + + def getdelete(context, firewall_id): + fw_db = _get_firewall(context, firewall_id) + # NOTE(cby): Use a different session to simulate a concurrent del + self.plugin.delete_db_firewall_object(alt_ctx, firewall_id) + return fw_db + + with self.firewall_policy() as fwp: + fwp_id = fwp['firewall_policy']['id'] + with self.firewall( + firewall_policy_id=fwp_id, + admin_state_up=test_db_firewall.ADMIN_STATE_UP, + do_delete=False + ) as fw: + fw_id = fw['firewall']['id'] + with ctx.session.begin(subtransactions=True): + fw_db = self.plugin._get_firewall(ctx, fw_id) + fw_db['status'] = const.PENDING_DELETE + ctx.session.flush() + + with mock.patch.object( + self.plugin, '_get_firewall', side_effect=getdelete + ): + observed = self.callbacks.firewall_deleted( + ctx, fw_id, host='dummy') + self.assertTrue(observed) + + self.assertRaises(firewall.FirewallNotFound, + self.plugin.get_firewall, + ctx, fw_id) + + def test_firewall_deleted_not_found(self): + ctx = context.get_admin_context() + observed = self.callbacks.firewall_deleted(ctx, 'notfound', host='hh') + self.assertTrue(observed) + def test_firewall_deleted_error(self): ctx = context.get_admin_context() with self.firewall_policy() as fwp: