From c9e1b50542d214621b4cb928f92f0b5092a6163f Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Tue, 13 Oct 2015 22:56:24 -0400 Subject: [PATCH] Create ipset set_name_exists() method All the callers of set_exists() already know the set name, so there's no reason to re-calculate it based on the id and ethertype. Renamed to set_name_exists() to be clear on what it is checking. Change-Id: Ifd2a2c7877c81a440067f9e8a654b49e63199f8c --- neutron/agent/linux/ipset_manager.py | 14 ++++++-------- neutron/agent/linux/iptables_firewall.py | 2 +- .../tests/unit/agent/linux/test_ipset_manager.py | 5 +++-- .../unit/agent/linux/test_iptables_firewall.py | 6 +++--- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/neutron/agent/linux/ipset_manager.py b/neutron/agent/linux/ipset_manager.py index e2a8a5296d3..86c5d87608d 100644 --- a/neutron/agent/linux/ipset_manager.py +++ b/neutron/agent/linux/ipset_manager.py @@ -65,9 +65,8 @@ class IpsetManager(object): name = NET_PREFIX + ethertype + id return name[:IPSET_NAME_MAX_LENGTH] - def set_exists(self, id, ethertype): - """Returns true if the id+ethertype pair is known to the manager.""" - set_name = self.get_name(id, ethertype) + def set_name_exists(self, set_name): + """Returns true if the set name is known to the manager.""" return set_name in self.ipset_sets def set_members(self, id, ethertype, member_ips): @@ -80,15 +79,14 @@ class IpsetManager(object): set_name = self.get_name(id, ethertype) add_ips = self._get_new_set_ips(set_name, member_ips) del_ips = self._get_deleted_set_ips(set_name, member_ips) - if not add_ips and not del_ips and self.set_exists(id, ethertype): + if not add_ips and not del_ips and self.set_name_exists(set_name): # nothing to do because no membership changes and the ipset exists return - self.set_members_mutate(id, ethertype, member_ips) + self.set_members_mutate(set_name, ethertype, member_ips) @utils.synchronized('ipset', external=True) - def set_members_mutate(self, id, ethertype, member_ips): - set_name = self.get_name(id, ethertype) - if not self.set_exists(id, ethertype): + def set_members_mutate(self, set_name, ethertype, member_ips): + if not self.set_name_exists(set_name): # The initial creation is handled with create/refresh to # avoid any downtime for existing sets (i.e. avoiding # a flush/restore), as the restore operation of ipset is diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py index f6cc18987d0..6f3f01597fb 100644 --- a/neutron/agent/linux/iptables_firewall.py +++ b/neutron/agent/linux/iptables_firewall.py @@ -525,7 +525,7 @@ class IptablesFirewallDriver(firewall.FirewallDriver): def _generate_ipset_rule_args(self, sg_rule, remote_gid): ethertype = sg_rule.get('ethertype') ipset_name = self.ipset.get_name(remote_gid, ethertype) - if not self.ipset.set_exists(remote_gid, ethertype): + if not self.ipset.set_name_exists(ipset_name): #NOTE(mangelajo): ipsets for empty groups are not created # thus we can't reference them. return None diff --git a/neutron/tests/unit/agent/linux/test_ipset_manager.py b/neutron/tests/unit/agent/linux/test_ipset_manager.py index 8b5b91cb8cd..bb7984638dd 100644 --- a/neutron/tests/unit/agent/linux/test_ipset_manager.py +++ b/neutron/tests/unit/agent/linux/test_ipset_manager.py @@ -121,9 +121,10 @@ class BaseIpsetManagerTest(base.BaseTestCase): class IpsetManagerTestCase(BaseIpsetManagerTest): - def test_set_exists(self): + def test_set_name_exists(self): self.add_first_ip() - self.assertTrue(self.ipset.set_exists(TEST_SET_ID, ETHERTYPE)) + self.assertTrue(self.ipset.set_name_exists('N' + ETHERTYPE + + TEST_SET_ID)) def test_set_members_with_first_add_member(self): self.add_first_ip() diff --git a/neutron/tests/unit/agent/linux/test_iptables_firewall.py b/neutron/tests/unit/agent/linux/test_iptables_firewall.py index 3a857c9fb65..28fb884bbde 100644 --- a/neutron/tests/unit/agent/linux/test_iptables_firewall.py +++ b/neutron/tests/unit/agent/linux/test_iptables_firewall.py @@ -1538,7 +1538,7 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): self.firewall.ipset = mock.Mock() self.firewall.ipset.get_name.side_effect = ( ipset_manager.IpsetManager.get_name) - self.firewall.ipset.set_exists.return_value = True + self.firewall.ipset.set_name_exists.return_value = True def _fake_port(self, sg_id=FAKE_SGID): return {'device': 'tapfake_dev', @@ -1751,9 +1751,9 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): mock.call.set_members('fake_sgid', 'IPv4', ['10.0.0.1']), mock.call.set_members('fake_sgid', 'IPv6', ['fe80::1']), mock.call.get_name('fake_sgid', 'IPv4'), - mock.call.set_exists('fake_sgid', 'IPv4'), + mock.call.set_name_exists('NIPv4fake_sgid'), mock.call.get_name('fake_sgid', 'IPv6'), - mock.call.set_exists('fake_sgid', 'IPv6'), + mock.call.set_name_exists('NIPv6fake_sgid'), mock.call.destroy('fake_sgid', 'IPv4'), mock.call.destroy('fake_sgid', 'IPv6')]