openstack
/
deb-networking-hyperv
Code Issues Proposed changes

Merge "SecurityGroups: Clears port from device cache"

This commit is contained in:
Jenkins 2017-06-17 07:58:16 +00:00 committed by Gerrit Code Review
parent 5a0a361eca d66301dcdd
commit 8ed35a4661
3 changed files with 66 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
hyperv/neutron/_common_utils.py
View File

@ -26,7 +26,8 @@ def get_port_synchronized_decorator(lock_prefix):
# The decorated method is expected to accept the port_id argument.
def wrapper(*args, **kwargs):
call_args = inspect.getcallargs(f, *args, **kwargs)
port_id = call_args['port_id']
port_id = (call_args.get('port_id') or
call_args.get('port', {}).get('id'))
lock_name = lock_prefix + ('port-lock-%s' % port_id)
@synchronized(lock_name)

50
hyperv/neutron/security_groups_driver.py
View File

@ -145,63 +145,70 @@ class HyperVSecurityGroupsDriverMixin(object):
self._sec_group_rules[port['id']] = []
def_sg_rules = self._sg_gen.create_default_sg_rules()
self._add_sg_port_rules(port['id'], def_sg_rules)
self._add_sg_port_rules(port, def_sg_rules)
# Add provider rules
provider_rules = port['security_group_rules']
self._create_port_rules(port['id'], provider_rules)
self._create_port_rules(port, provider_rules)
newrules = self._generate_rules([port])
self._create_port_rules(port['id'], newrules[port['id']])
self._create_port_rules(port, newrules[port['id']])
self._security_ports[port['device']] = port
self._sec_group_rules[port['id']] = newrules[port['id']]
@_ports_synchronized
def _create_port_rules(self, port_id, rules):
def _create_port_rules(self, port, rules):
sg_rules = self._sg_gen.create_security_group_rules(rules)
old_sg_rules = self._sec_group_rules[port_id]
old_sg_rules = self._sec_group_rules[port['id']]
add, rm = self._sg_gen.compute_new_rules_add(old_sg_rules, sg_rules)
self._add_sg_port_rules(port_id, list(set(add)))
self._remove_sg_port_rules(port_id, list(set(rm)))
self._add_sg_port_rules(port, list(set(add)))
self._remove_sg_port_rules(port, list(set(rm)))
@_ports_synchronized
def _remove_port_rules(self, port_id, rules):
def _remove_port_rules(self, port, rules):
sg_rules = self._sg_gen.create_security_group_rules(rules)
self._remove_sg_port_rules(port_id, list(set(sg_rules)))
self._remove_sg_port_rules(port, list(set(sg_rules)))
def _add_sg_port_rules(self, port_id, sg_rules):
def _add_sg_port_rules(self, port, sg_rules):
if not sg_rules:
return
old_sg_rules = self._sec_group_rules[port_id]
old_sg_rules = self._sec_group_rules[port['id']]
try:
self._utils.create_security_rules(port_id, sg_rules)
self._utils.create_security_rules(port['id'], sg_rules)
old_sg_rules.extend(sg_rules)
except exceptions.NotFound:
# port no longer exists.
self._sec_group_rules.pop(port_id, None)
# NOTE(claudiub): In the case of a rebuild / shelve, the
# neutron port is not deleted, and it can still be in the cache.
# We need to make sure the port's caches are cleared since it is
# not valid anymore. The port will be reprocessed in the next
# loop iteration.
self._sec_group_rules.pop(port['id'], None)
self._security_ports.pop(port.get('device'), None)
raise
except Exception:
LOG.exception(_LE('Exception encountered while adding rules for '
'port: %s'), port_id)
'port: %s'), port['id'])
raise
def _remove_sg_port_rules(self, port_id, sg_rules):
def _remove_sg_port_rules(self, port, sg_rules):
if not sg_rules:
return
old_sg_rules = self._sec_group_rules[port_id]
old_sg_rules = self._sec_group_rules[port['id']]
try:
self._utils.remove_security_rules(port_id, sg_rules)
self._utils.remove_security_rules(port['id'], sg_rules)
for rule in sg_rules:
if rule in old_sg_rules:
old_sg_rules.remove(rule)
except exceptions.NotFound:
# port no longer exists.
self._sec_group_rules.pop(port_id, None)
self._sec_group_rules.pop(port['id'], None)
self._security_ports.pop(port.get('device'), None)
raise
except Exception:
LOG.exception(_LE('Exception encountered while removing rules for '
'port: %s'), port_id)
'port: %s'), port['id'])
raise
def apply_port_filter(self, port):
@ -211,6 +218,7 @@ class HyperVSecurityGroupsDriverMixin(object):
if not port.get('port_security_enabled'):
LOG.info(_LI('Port %s does not have security enabled. '
'Removing existing rules if any.'), port['id'])
self._security_ports.pop(port.get('device'), None)
existing_rules = self._sec_group_rules.pop(port['id'], None)
if existing_rules:
self._utils.remove_all_security_rules(port['id'])
@ -252,8 +260,8 @@ class HyperVSecurityGroupsDriverMixin(object):
{'new': len(new_rules),
'old': len(remove_rules)})
self._create_port_rules(port['id'], new_rules)
self._remove_port_rules(old_port['id'], remove_rules)
self._create_port_rules(port, new_rules)
self._remove_port_rules(old_port, remove_rules)
self._security_ports[port['device']] = port
self._sec_group_rules[port['id']] = added_rules[port['id']]

55
hyperv/tests/unit/neutron/test_security_groups_driver.py
View File

@ -167,10 +167,10 @@ class TestHyperVSecurityGroupsDriver(SecurityGroupRuleTestHelper):
mock_gen_rules.assert_called_with([self._driver._security_ports
[self._FAKE_DEVICE]])
mock_add_rules.assert_called_once_with(
self._FAKE_ID, mock_create_default.return_value)
mock_port, mock_create_default.return_value)
self._driver._create_port_rules.assert_called_with(
self._FAKE_ID, [fake_rule])
mock_port, [fake_rule])
def test_prepare_port_filter_security_disabled(self):
mock_port = self._get_port()
@ -203,10 +203,10 @@ class TestHyperVSecurityGroupsDriver(SecurityGroupRuleTestHelper):
self._driver.update_port_filter(new_mock_port)
self._driver._remove_port_rules.assert_called_once_with(
mock_port['id'], mock_port['security_group_rules'])
mock_port, mock_port['security_group_rules'])
self._driver._create_port_rules.assert_called_once_with(
new_mock_port['id'], [new_mock_port['security_group_rules'][0],
fake_rule_new])
new_mock_port, [new_mock_port['security_group_rules'][0],
fake_rule_new])
self.assertEqual(new_mock_port,
self._driver._security_ports[new_mock_port['device']])
@ -239,6 +239,7 @@ class TestHyperVSecurityGroupsDriver(SecurityGroupRuleTestHelper):
mock_port = self._get_port()
mock_port.pop('port_security_enabled')
self._driver._sec_group_rules[mock_port['id']] = mock.ANY
self._driver._security_ports[mock_port['device']] = mock.sentinel.port
self._driver.update_port_filter(mock_port)
@ -284,9 +285,9 @@ class TestHyperVSecurityGroupsDriver(SecurityGroupRuleTestHelper):
self._driver._sg_gen.expand_wildcard_rules.assert_called_once_with(
[fake_rule_new, fake_wildcard_rule_new])
self._driver._remove_port_rules.assert_called_once_with(
mock_port['id'], filtered_remove_rules)
mock_port, filtered_remove_rules)
self._driver._create_port_rules.assert_called_once_with(
new_mock_port['id'], filtered_new_rules)
new_mock_port, filtered_new_rules)
self.assertEqual(new_mock_port,
self._driver._security_ports[new_mock_port['device']])
@ -305,6 +306,7 @@ class TestHyperVSecurityGroupsDriver(SecurityGroupRuleTestHelper):
@mock.patch.object(sg_driver.HyperVSecurityGroupsDriver,
'_remove_sg_port_rules')
def test_create_port_rules(self, mock_remove, mock_add):
mock_port = self._get_port()
mock_rule = mock.MagicMock()
self._driver._sec_group_rules[self._FAKE_ID] = [mock_rule]
self._driver._sg_gen.create_security_group_rules.return_value = [
@ -312,26 +314,28 @@ class TestHyperVSecurityGroupsDriver(SecurityGroupRuleTestHelper):
self._driver._sg_gen.compute_new_rules_add.return_value = (
[mock_rule, mock_rule], [mock_rule, mock_rule])
self._driver._create_port_rules(self._FAKE_ID, [mock_rule])
self._driver._create_port_rules(mock_port, [mock_rule])
self._driver._sg_gen.compute_new_rules_add.assert_called_once_with(
[mock_rule], [mock_rule])
mock_remove.assert_called_once_with(self._FAKE_ID, [mock_rule])
mock_add.assert_called_once_with(self._FAKE_ID, [mock_rule])
mock_remove.assert_called_once_with(mock_port, [mock_rule])
mock_add.assert_called_once_with(mock_port, [mock_rule])
@mock.patch.object(sg_driver.HyperVSecurityGroupsDriver,
'_remove_sg_port_rules')
def test_remove_port_rules(self, mock_remove):
mock_port = self._get_port()
mock_rule = mock.MagicMock()
self._driver._sec_group_rules[self._FAKE_ID] = [mock_rule]
self._driver._sg_gen.create_security_group_rules.return_value = [
mock_rule]
self._driver._remove_port_rules(self._FAKE_ID, [mock_rule])
self._driver._remove_port_rules(mock_port, [mock_rule])
mock_remove.assert_called_once_with(self._FAKE_ID, [mock_rule])
mock_remove.assert_called_once_with(mock_port, [mock_rule])
def test_add_sg_port_rules_exception(self):
mock_port = self._get_port()
mock_rule = mock.MagicMock()
self._driver._sec_group_rules[self._FAKE_ID] = []
self._driver._utils.create_security_rules.side_effect = (
@ -339,36 +343,42 @@ class TestHyperVSecurityGroupsDriver(SecurityGroupRuleTestHelper):
self.assertRaises(exceptions.HyperVException,
self._driver._add_sg_port_rules,
self._FAKE_ID, [mock_rule])
mock_port, [mock_rule])
self.assertNotIn(mock_rule,
self._driver._sec_group_rules[self._FAKE_ID])
def test_add_sg_port_rules_port_not_found(self):
mock_port = self._get_port()
self._driver._sec_group_rules[self._FAKE_ID] = []
self._driver._security_ports[self._FAKE_DEVICE] = mock.sentinel.port
self._driver._utils.create_security_rules.side_effect = (
exceptions.NotFound(resource='port_id'))
self.assertRaises(exceptions.NotFound,
self._driver._add_sg_port_rules,
self._FAKE_ID, [mock.sentinel.rule])
mock_port, [mock.sentinel.rule])
self.assertNotIn(self._FAKE_ID, self._driver._sec_group_rules)
self.assertNotIn(self._FAKE_DEVICE, self._driver._security_ports)
def test_add_sg_port_rules(self):
mock_port = self._get_port()
mock_rule = mock.MagicMock()
self._driver._sec_group_rules[self._FAKE_ID] = []
self._driver._add_sg_port_rules(self._FAKE_ID, [mock_rule])
self._driver._add_sg_port_rules(mock_port, [mock_rule])
self._driver._utils.create_security_rules.assert_called_once_with(
self._FAKE_ID, [mock_rule])
self.assertIn(mock_rule, self._driver._sec_group_rules[self._FAKE_ID])
def test_add_sg_port_rules_empty(self):
self._driver._add_sg_port_rules(mock.sentinel.id, [])
mock_port = self._get_port()
self._driver._add_sg_port_rules(mock_port, [])
self.assertFalse(self._driver._utils.create_security_rules.called)
def test_remove_sg_port_rules_exception(self):
mock_port = self._get_port()
mock_rule = mock.MagicMock()
self._driver._sec_group_rules[self._FAKE_ID] = [mock_rule]
self._driver._utils.remove_security_rules.side_effect = (
@ -376,26 +386,30 @@ class TestHyperVSecurityGroupsDriver(SecurityGroupRuleTestHelper):
self.assertRaises(exceptions.HyperVException,
self._driver._remove_sg_port_rules,
self._FAKE_ID, [mock_rule])
mock_port, [mock_rule])
self.assertIn(mock_rule, self._driver._sec_group_rules[self._FAKE_ID])
def test_remove_sg_port_rules_port_not_found(self):
mock_port = self._get_port()
self._driver._sec_group_rules[self._FAKE_ID] = []
self._driver._security_ports[self._FAKE_DEVICE] = mock.sentinel.port
self._driver._utils.remove_security_rules.side_effect = (
exceptions.NotFound(resource='port_id'))
self.assertRaises(exceptions.NotFound,
self._driver._remove_sg_port_rules,
self._FAKE_ID, [mock.sentinel.rule])
mock_port, [mock.sentinel.rule])
self.assertNotIn(self._FAKE_ID, self._driver._sec_group_rules)
self.assertNotIn(self._FAKE_DEVICE, self._driver._security_ports)
def test_remove_sg_port_rules(self):
mock_port = self._get_port()
mock_rule = mock.MagicMock()
self._driver._sec_group_rules[self._FAKE_ID] = [mock_rule]
self._driver._remove_sg_port_rules(
self._FAKE_ID, [mock_rule, mock.sentinel.other_rule])
mock_port, [mock_rule, mock.sentinel.other_rule])
self._driver._utils.remove_security_rules.assert_called_once_with(
self._FAKE_ID, [mock_rule, mock.sentinel.other_rule])
@ -403,7 +417,8 @@ class TestHyperVSecurityGroupsDriver(SecurityGroupRuleTestHelper):
self._driver._sec_group_rules[self._FAKE_ID])
def test_remove_sg_port_rules_empty(self):
self._driver._remove_sg_port_rules(mock.sentinel.id, [])
mock_port = self._get_port()
self._driver._remove_sg_port_rules(mock_port, [])
self.assertFalse(self._driver._utils.remove_security_rules.called)
def _get_port(self):