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

diff --git a/neutron/services/firewall/fwaas_plugin.py b/neutron/services/firewall/fwaas_plugin.py
index 4db179b99ec7d95f64bcdeb391d9f3cf7d76a108..b3c7d67f709e9d9c63e7f8d4b8fbd402516e5406 100644 (file)
--- 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 40c91de9698511d8560de3421a541bb3c56baddc..73e94698b0e0f3f57a836da0a524dff57ef63949 100644 (file)
--- 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: