openstack/neutron
Code Issues Proposed changes

Merge "Destroy ipset when the corresponding rule is removed"

Browse Source
This commit is contained in:
Jenkins 2015-08-13 02:03:40 +00:00 committed by Gerrit Code Review
commit 6688b7b73f
2 changed files with 29 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
neutron/agent/linux/iptables_firewall.py
View File

@ -638,11 +638,10 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
filtered_ports) filtered_ports)
for ip_version, remote_sg_ids in six.iteritems(remote_sgs_to_remove): for ip_version, remote_sg_ids in six.iteritems(remote_sgs_to_remove):
self._clear_sg_members(ip_version, remote_sg_ids)
if self.enable_ipset: if self.enable_ipset:
self._remove_ipsets_for_remote_sgs(ip_version, remote_sg_ids) self._remove_ipsets_for_remote_sgs(ip_version, remote_sg_ids)
self._remove_unused_sg_members() self._remove_sg_members(remote_sgs_to_remove)
# Remove unused security group rules # Remove unused security group rules
for remove_group_id in self._determine_sg_rules_to_remove( for remove_group_id in self._determine_sg_rules_to_remove(
@ -690,23 +689,17 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
port_group_ids.update(port.get('security_groups', [])) port_group_ids.update(port.get('security_groups', []))
return port_group_ids return port_group_ids
def _clear_sg_members(self, ip_version, remote_sg_ids):
"""Clear our internal cache of sg members matching the parameters."""
for remote_sg_id in remote_sg_ids:
if self.sg_members[remote_sg_id][ip_version]:
self.sg_members[remote_sg_id][ip_version] = []
def _remove_ipsets_for_remote_sgs(self, ip_version, remote_sg_ids): def _remove_ipsets_for_remote_sgs(self, ip_version, remote_sg_ids):
"""Remove system ipsets matching the provided parameters.""" """Remove system ipsets matching the provided parameters."""
for remote_sg_id in remote_sg_ids: for remote_sg_id in remote_sg_ids:
self.ipset.destroy(remote_sg_id, ip_version) self.ipset.destroy(remote_sg_id, ip_version)
def _remove_unused_sg_members(self): def _remove_sg_members(self, remote_sgs_to_remove):
"""Remove sg_member entries where no IPv4 or IPv6 is associated.""" """Remove sg_member entries."""
for sg_id in list(self.sg_members.keys()): ipv4_sec_group_set = remote_sgs_to_remove.get(constants.IPv4)
sg_has_members = (self.sg_members[sg_id][constants.IPv4] or ipv6_sec_group_set = remote_sgs_to_remove.get(constants.IPv6)
self.sg_members[sg_id][constants.IPv6]) for sg_id in (ipv4_sec_group_set & ipv6_sec_group_set):
if not sg_has_members: if sg_id in self.sg_members:
del self.sg_members[sg_id] del self.sg_members[sg_id]
def _find_deleted_sg_rules(self, sg_id): def _find_deleted_sg_rules(self, sg_id):

39
neutron/tests/unit/agent/linux/test_iptables_firewall.py
View File

@ -1620,20 +1620,12 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase):
self.assertEqual(sg_ids, self.assertEqual(sg_ids,
self.firewall._get_sg_ids_set_for_ports(ports)) self.firewall._get_sg_ids_set_for_ports(ports))
def test_clear_sg_members(self): def test_remove_sg_members(self):
self.firewall.sg_members = self._fake_sg_members(
sg_ids=[FAKE_SGID, OTHER_SGID])
self.firewall._clear_sg_members(_IPv4, [OTHER_SGID])
self.assertEqual(0, len(self.firewall.sg_members[OTHER_SGID][_IPv4]))
def test_remove_unused_sg_members(self):
self.firewall.sg_members = self._fake_sg_members([FAKE_SGID, self.firewall.sg_members = self._fake_sg_members([FAKE_SGID,
OTHER_SGID]) OTHER_SGID])
self.firewall.sg_members[FAKE_SGID][_IPv4] = [] remote_sgs_to_remove = {_IPv4: set([FAKE_SGID]),
self.firewall.sg_members[FAKE_SGID][_IPv6] = [] _IPv6: set([FAKE_SGID, OTHER_SGID])}
self.firewall.sg_members[OTHER_SGID][_IPv6] = [] self.firewall._remove_sg_members(remote_sgs_to_remove)
self.firewall._remove_unused_sg_members()
self.assertIn(OTHER_SGID, self.firewall.sg_members) self.assertIn(OTHER_SGID, self.firewall.sg_members)
self.assertNotIn(FAKE_SGID, self.firewall.sg_members) self.assertNotIn(FAKE_SGID, self.firewall.sg_members)
@ -1652,13 +1644,26 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase):
self.assertNotIn(OTHER_SGID, self.firewall.sg_rules) self.assertNotIn(OTHER_SGID, self.firewall.sg_rules)
def test_remove_unused_security_group_info(self): def test_remove_unused_security_group_info(self):
self._setup_fake_firewall_members_and_rules(self.firewall) self.firewall.sg_members = {OTHER_SGID: {_IPv4: [], _IPv6: []}}
# no filtered ports in 'fake_sgid', so all rules and members self.firewall.pre_sg_members = self.firewall.sg_members
# are not needed and we expect them to be cleaned up self.firewall.sg_rules = self._fake_sg_rules(
self.firewall.prepare_port_filter(self._fake_port(OTHER_SGID)) remote_groups={_IPv4: [FAKE_SGID], _IPv6: [FAKE_SGID]})
self.firewall.pre_sg_rules = self.firewall.sg_rules
port = self._fake_port()
self.firewall.filtered_ports['tapfake_dev'] = port
self.firewall._remove_unused_security_group_info() self.firewall._remove_unused_security_group_info()
self.assertNotIn(OTHER_SGID, self.firewall.sg_members)
self.assertNotIn(FAKE_SGID, self.firewall.sg_members) def test_not_remove_used_security_group_info(self):
self.firewall.sg_members = {OTHER_SGID: {_IPv4: [], _IPv6: []}}
self.firewall.pre_sg_members = self.firewall.sg_members
self.firewall.sg_rules = self._fake_sg_rules(
remote_groups={_IPv4: [OTHER_SGID], _IPv6: [OTHER_SGID]})
self.firewall.pre_sg_rules = self.firewall.sg_rules
port = self._fake_port()
self.firewall.filtered_ports['tapfake_dev'] = port
self.firewall._remove_unused_security_group_info()
self.assertIn(OTHER_SGID, self.firewall.sg_members)
def test_remove_all_unused_info(self): def test_remove_all_unused_info(self):
self._setup_fake_firewall_members_and_rules(self.firewall) self._setup_fake_firewall_members_and_rules(self.firewall)