From d5d345b975cbbfd27fbc215dacfa9fbb5c1f8046 Mon Sep 17 00:00:00 2001 From: Eugene Nikanorov Date: Tue, 10 Jun 2014 07:55:40 +0400 Subject: [PATCH] Fix race condition with firewall deletion In some cases when firewall is created and then deleted in short period of time, there could be a race condition of firewall status changes. Agent may change firewall status from PENDING_DELETE to ACTIVE because the agent has just set it up on the backend. Delete request then is not properly served and firewall remains in ERROR state and can't be deleted at all. To fix this changing status from PENDING_DELETE is not allowed. Deleting firewall in ERROR state is allowed. Change-Id: Iec3cfcb1e03b33dda8e1f10ca51bd9b61fa8030d Closes-Bug: #1328162 (cherry picked from commit 58e6bb5893186517edafe1a4d51710c1362bc9cc) --- neutron/services/firewall/fwaas_plugin.py | 12 +++++++++++- .../services/firewall/test_fwaas_plugin.py | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/neutron/services/firewall/fwaas_plugin.py b/neutron/services/firewall/fwaas_plugin.py index 793eb870bf6..ee0737dbf96 100644 --- a/neutron/services/firewall/fwaas_plugin.py +++ b/neutron/services/firewall/fwaas_plugin.py @@ -49,6 +49,15 @@ class FirewallCallbacks(object): LOG.debug(_("set_firewall_status() called")) with context.session.begin(subtransactions=True): fw_db = self.plugin._get_firewall(context, firewall_id) + # ignore changing status if firewall expects to be deleted + # That case means that while some pending operation has been + # performed on the backend, neutron server received delete request + # and changed firewall status to const.PENDING_DELETE + if fw_db.status == const.PENDING_DELETE: + LOG.debug(_("Firewall %(fw_id)s in PENDING_DELETE state, " + "not changing to %(status)s"), + {'fw_id': firewall_id, 'status': status}) + return False #TODO(xuhanp): Remove INACTIVE status and use DOWN to # be consistent with other network resources if status in (const.ACTIVE, const.INACTIVE, const.DOWN): @@ -63,7 +72,8 @@ class FirewallCallbacks(object): LOG.debug(_("firewall_deleted() called")) with context.session.begin(subtransactions=True): fw_db = self.plugin._get_firewall(context, firewall_id) - if fw_db.status == const.PENDING_DELETE: + # allow to delete firewalls in ERROR state + if fw_db.status in (const.PENDING_DELETE, const.ERROR): self.plugin.delete_db_firewall_object(context, firewall_id) return True else: diff --git a/neutron/tests/unit/services/firewall/test_fwaas_plugin.py b/neutron/tests/unit/services/firewall/test_fwaas_plugin.py index 9fc0aa47ffd..5f59c1f1115 100644 --- a/neutron/tests/unit/services/firewall/test_fwaas_plugin.py +++ b/neutron/tests/unit/services/firewall/test_fwaas_plugin.py @@ -63,6 +63,24 @@ class TestFirewallCallbacks(test_db_firewall.FirewallPluginDbTestCase): self.assertEqual(fw_db['status'], const.ERROR) self.assertFalse(res) + def test_set_firewall_status_pending_delete(self): + ctx = context.get_admin_context() + 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) as fw: + fw_id = fw['firewall']['id'] + fw_db = self.plugin._get_firewall(ctx, fw_id) + fw_db['status'] = const.PENDING_DELETE + ctx.session.flush() + res = self.callbacks.set_firewall_status(ctx, fw_id, + const.ACTIVE, + host='dummy') + fw_db = self.plugin.get_firewall(ctx, fw_id) + self.assertEqual(fw_db['status'], const.PENDING_DELETE) + self.assertFalse(res) + def test_firewall_deleted(self): ctx = context.get_admin_context() with self.firewall_policy() as fwp: