diff --git a/neutron_fwaas/services/firewall/fwaas_plugin_v2.py b/neutron_fwaas/services/firewall/fwaas_plugin_v2.py index a82f7102b..4fbcfe872 100644 --- a/neutron_fwaas/services/firewall/fwaas_plugin_v2.py +++ b/neutron_fwaas/services/firewall/fwaas_plugin_v2.py @@ -20,6 +20,7 @@ from oslo_config import cfg from oslo_log import log as logging import oslo_messaging +from neutron_fwaas._i18n import _LI from neutron_fwaas.common import fwaas_constants as f_const from neutron_fwaas.db.firewall.v2 import firewall_db_v2 from neutron_fwaas.extensions import firewall_v2 as fw_ext @@ -84,18 +85,23 @@ class FirewallCallbacks(object): def firewall_group_deleted(self, context, fwg_id, **kwargs): """Agent uses this to indicate firewall is deleted.""" LOG.debug("firewall_group_deleted() called") - with context.session.begin(subtransactions=True): - fwg_db = self.plugin._get_firewall_group(context, fwg_id) - # allow to delete firewalls in ERROR state - if fwg_db.status in (n_const.PENDING_DELETE, n_const.ERROR): - self.plugin.delete_db_firewall_group_object(context, fwg_id) - return True - else: - LOG.warning(('Firewall %(fwg)s unexpectedly deleted by ' - 'agent, status was %(status)s'), - {'fwg': fwg_id, 'status': fwg_db.status}) - fwg_db.update({"status": n_const.ERROR}) - return False + try: + with context.session.begin(subtransactions=True): + fwg_db = self.plugin._get_firewall_group(context, fwg_id) + # allow to delete firewalls in ERROR state + if fwg_db.status in (n_const.PENDING_DELETE, n_const.ERROR): + self.plugin.delete_db_firewall_group_object(context, + fwg_id) + return True + else: + LOG.warning(('Firewall %(fwg)s unexpectedly deleted by ' + 'agent, status was %(status)s'), + {'fwg': fwg_id, 'status': fwg_db.status}) + fwg_db.update({"status": n_const.ERROR}) + return False + except fw_ext.FirewallGroupNotFound: + LOG.info(_LI('Firewall group %s already deleted'), fwg_id) + return True def get_firewall_groups_for_project(self, context, **kwargs): """Gets all firewall_groups and rules on a project.""" diff --git a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py index a10994eae..03e8274fe 100644 --- a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py +++ b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py @@ -157,6 +157,85 @@ class TestFirewallCallbacks(TestFirewallRouterPortBase): self.assertEqual(const.ERROR, fwg_db['status']) self.assertFalse(res) + def test_firewall_group_deleted(self): + ctx = context.get_admin_context() + with self.firewall_policy() as fwp: + fwp_id = fwp['firewall_policy']['id'] + with self.firewall_group( + ingress_firewall_policy_id=fwp_id, + admin_state_up=test_db_firewall.ADMIN_STATE_UP, + do_delete=False + ) as fwg: + fwg_id = fwg['firewall_group']['id'] + with ctx.session.begin(subtransactions=True): + fwg_db = self.plugin._get_firewall_group(ctx, fwg_id) + fwg_db['status'] = const.PENDING_DELETE + + observed = self.callbacks.firewall_group_deleted( + ctx, fwg_id, host='dummy') + self.assertTrue(observed) + + self.assertRaises(firewall_v2.FirewallGroupNotFound, + self.plugin.get_firewall_group, + ctx, fwg_id) + + def test_firewall_group_deleted_concurrently(self): + ctx = context.get_admin_context() + alt_ctx = context.get_admin_context() + + _get_firewall_group = self.plugin._get_firewall_group + + def getdelete(context, fwg_id): + fwg_db = _get_firewall_group(context, fwg_id) + # NOTE(cby): Use a different session to simulate a concurrent del + self.plugin.delete_db_firewall_group_object(alt_ctx, fwg_id) + return fwg_db + + with self.firewall_policy() as fwp: + fwp_id = fwp['firewall_policy']['id'] + with self.firewall_group( + firewall_policy_id=fwp_id, + admin_state_up=test_db_firewall.ADMIN_STATE_UP, + do_delete=False + ) as fwg: + fwg_id = fwg['firewall_group']['id'] + with ctx.session.begin(subtransactions=True): + fwg_db = self.plugin._get_firewall_group(ctx, fwg_id) + fwg_db['status'] = const.PENDING_DELETE + ctx.session.flush() + + with mock.patch.object( + self.plugin, '_get_firewall_group', side_effect=getdelete + ): + observed = self.callbacks.firewall_group_deleted( + ctx, fwg_id, host='dummy') + self.assertTrue(observed) + + self.assertRaises(firewall_v2.FirewallGroupNotFound, + self.plugin.get_firewall_group, + ctx, fwg_id) + + def test_firewall_group_deleted_not_found(self): + ctx = context.get_admin_context() + observed = self.callbacks.firewall_group_deleted( + ctx, 'notfound', host='hh') + self.assertTrue(observed) + + def test_firewall_group_deleted_error(self): + ctx = context.get_admin_context() + with self.firewall_policy() as fwp: + fwp_id = fwp['firewall_policy']['id'] + with self.firewall_group( + firewall_policy_id=fwp_id, + admin_state_up=test_db_firewall.ADMIN_STATE_UP, + ) as fwg: + fwg_id = fwg['firewall_group']['id'] + observed = self.callbacks.firewall_group_deleted( + ctx, fwg_id, host='dummy') + self.assertFalse(observed) + fwg_db = self.plugin._get_firewall_group(ctx, fwg_id) + self.assertEqual(const.ERROR, fwg_db['status']) + class TestFirewallPluginBasev2(TestFirewallRouterPortBase, test_l3_plugin.L3NatTestCaseMixin):