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 3046c4ae22
Change-Id: Ib2f460cc095bbea8f9c767dcb9b4d4b66f1a7811
Closes-Bug: 1393925
This commit is contained in:
parent
bd08363605
commit
39357ac208
@ -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
|
||||
|
@ -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()
|
||||
|
Loading…
Reference in New Issue
Block a user