From: Brian Haley <brian.haley@hp.com>
Date: Wed, 19 Nov 2014 17:10:57 +0000 (-0500)
Subject: Fix a race condition adding a security group rule
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=ee0e41d09d6ffc3cb7bf8406c8e8b009dfb92e17;p=openstack-build%2Fneutron-build.git

Fix a race condition adding a security group rule

setup_port_filters() needs to grab self.devices_to_refilter
before it calls prepare_devices_filter(), else it could skip
processing a device if an RPC arrives while it's processing
new devices.  That device will now be handled the next time
it's called.

Bug introduced in commit 3046c4ae22b1

Change-Id: Ib2f460cc095bbea8f9c767dcb9b4d4b66f1a7811
Closes-Bug: 1393925
---

diff --git a/neutron/agent/securitygroups_rpc.py b/neutron/agent/securitygroups_rpc.py
index ee898b213..e21fb12b6 100644
--- a/neutron/agent/securitygroups_rpc.py
+++ b/neutron/agent/securitygroups_rpc.py
@@ -327,16 +327,20 @@ class SecurityGroupAgentRpcMixin(object):
         :param updated_devices: set containing identifiers for
         updated devices
         """
-        if new_devices:
-            LOG.debug("Preparing device filters for %d new devices",
-                      len(new_devices))
-            self.prepare_devices_filter(new_devices)
         # These data structures are cleared here in order to avoid
         # losing updates occurring during firewall refresh
         devices_to_refilter = self.devices_to_refilter
         global_refresh_firewall = self.global_refresh_firewall
         self.devices_to_refilter = set()
         self.global_refresh_firewall = False
+        # We must call prepare_devices_filter() after we've grabbed
+        # self.devices_to_refilter since an update for a new port
+        # could arrive while we're processing, and we need to make
+        # sure we don't skip it.  It will get handled the next time.
+        if new_devices:
+            LOG.debug("Preparing device filters for %d new devices",
+                      len(new_devices))
+            self.prepare_devices_filter(new_devices)
         # TODO(salv-orlando): Avoid if possible ever performing the global
         # refresh providing a precise list of devices for which firewall
         # should be refreshed
diff --git a/neutron/tests/unit/test_security_groups_rpc.py b/neutron/tests/unit/test_security_groups_rpc.py
index dfad04414..89a8c412a 100644
--- a/neutron/tests/unit/test_security_groups_rpc.py
+++ b/neutron/tests/unit/test_security_groups_rpc.py
@@ -1433,6 +1433,25 @@ class SecurityGroupAgentRpcWithDeferredRefreshTestCase(
         self.agent.refresh_firewall.assert_called_once_with(
             set(['fake_device']))
 
+    def _test_prepare_devices_filter(self, devices):
+        # simulate an RPC arriving and calling _security_group_updated()
+        self.agent.devices_to_refilter |= set(['fake_new_device'])
+
+    def test_setup_port_filters_new_port_and_rpc(self):
+        # Make sure that if an RPC arrives and adds a device to
+        # devices_to_refilter while we are in setup_port_filters()
+        # that it is not cleared, and will be processed later.
+        self.agent.prepare_devices_filter = self._test_prepare_devices_filter
+        self.agent.refresh_firewall = mock.Mock()
+        self.agent.devices_to_refilter = set(['new_device', 'fake_device'])
+        self.agent.global_refresh_firewall = False
+        self.agent.setup_port_filters(set(['new_device']), set())
+        self.assertEqual(self.agent.devices_to_refilter,
+                         set(['fake_new_device']))
+        self.assertFalse(self.agent.global_refresh_firewall)
+        self.agent.refresh_firewall.assert_called_once_with(
+            set(['fake_device']))
+
     def test_setup_port_filters_sg_updates_and_updated_ports(self):
         self.agent.prepare_devices_filter = mock.Mock()
         self.agent.refresh_firewall = mock.Mock()