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 ee898b213d4814f22af0ba283ed482248e6799d8..e21fb12b6547ab2a8a005bfc67ee365e05830e0e 100644 (file)
--- 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 dfad04414f13de4b2a3be6b7fe02d64899d9523e..89a8c412abd41175381ce833bd27031066d820d3 100644 (file)
--- 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()