From 4ed651f918f1e3bd4a130b5ba560a4da6b1db436 Mon Sep 17 00:00:00 2001 From: Ionuț Arțăriși Date: Tue, 29 Jan 2013 17:24:51 +0100 Subject: clear up method parameters for _modify_rules 'table_name' is always used anyway, so there's no reason to have it as optional especially as it's not perfectly clear that it's only needed when current_lines is empty 'binary' was never used Change-Id: I2c6bcf3fcff1c097d1f5c5ae34aa3428a94dabdc --- nova/network/linux_net.py | 9 ++++----- nova/tests/test_iptables_network.py | 27 ++++++++++++++++----------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index cb23e77b6..46c3faca7 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -410,10 +410,10 @@ class IptablesManager(object): run_as_root=True, attempts=5) all_lines = all_tables.split('\n') - for table in tables: - start, end = self._find_table(all_lines, table) + for table_name, table in tables.iteritems(): + start, end = self._find_table(all_lines, table_name) all_lines[start:end] = self._modify_rules( - all_lines[start:end], tables[table], table_name=table) + all_lines[start:end], table, table_name) self.execute('%s-restore' % (cmd,), '-c', run_as_root=True, process_input='\n'.join(all_lines), attempts=5) @@ -431,8 +431,7 @@ class IptablesManager(object): end = lines[start:].index('COMMIT') + start + 2 return (start, end) - def _modify_rules(self, current_lines, table, binary=None, - table_name=None): + def _modify_rules(self, current_lines, table, table_name): unwrapped_chains = table.unwrapped_chains chains = table.chains remove_chains = table.remove_chains diff --git a/nova/tests/test_iptables_network.py b/nova/tests/test_iptables_network.py index e64dcf66b..0aaae7df0 100644 --- a/nova/tests/test_iptables_network.py +++ b/nova/tests/test_iptables_network.py @@ -93,12 +93,12 @@ class IptablesManagerTestCase(test.TestCase): table = self.manager.ipv4['filter'] table.add_rule('FORWARD', '-s 1.2.3.4/5 -j DROP') - new_lines = self.manager._modify_rules(current_lines, table) + new_lines = self.manager._modify_rules(current_lines, table, 'filter') self.assertTrue('[0:0] -A %s-FORWARD ' '-s 1.2.3.4/5 -j DROP' % self.binary_name in new_lines) table.remove_rule('FORWARD', '-s 1.2.3.4/5 -j DROP') - new_lines = self.manager._modify_rules(current_lines, table) + new_lines = self.manager._modify_rules(current_lines, table, 'filter') self.assertTrue('[0:0] -A %s-FORWARD ' '-s 1.2.3.4/5 -j DROP' % self.binary_name not in new_lines) @@ -118,22 +118,23 @@ class IptablesManagerTestCase(test.TestCase): ' -o eth0') table.add_rule('PREROUTING', '-d 10.10.10.11 -j DNAT --to 10.0.0.10') table.add_rule('OUTPUT', '-d 10.10.10.11 -j DNAT --to 10.0.0.10') - new_lines = self.manager._modify_rules(current_lines, table) + new_lines = self.manager._modify_rules(current_lines, table, 'nat') self.assertEqual(len(new_lines) - len(current_lines), 8) regex = '.*\s+%s(/32|\s+|$)' num_removed = table.remove_rules_regex(regex % '10.10.10.10') self.assertEqual(num_removed, 4) - new_lines = self.manager._modify_rules(current_lines, table) + new_lines = self.manager._modify_rules(current_lines, table, 'nat') self.assertEqual(len(new_lines) - len(current_lines), 4) num_removed = table.remove_rules_regex(regex % '10.10.10.11') self.assertEqual(num_removed, 4) - new_lines = self.manager._modify_rules(current_lines, table) + new_lines = self.manager._modify_rules(current_lines, table, 'nat') self.assertEqual(new_lines, current_lines) def test_nat_rules(self): current_lines = self.sample_nat new_lines = self.manager._modify_rules(current_lines, - self.manager.ipv4['nat']) + self.manager.ipv4['nat'], + 'nat') for line in [':%s-OUTPUT - [0:0]' % (self.binary_name), ':%s-float-snat - [0:0]' % (self.binary_name), @@ -168,7 +169,8 @@ class IptablesManagerTestCase(test.TestCase): def test_filter_rules(self): current_lines = self.sample_filter new_lines = self.manager._modify_rules(current_lines, - self.manager.ipv4['filter']) + self.manager.ipv4['filter'], + 'nat') for line in [':%s-FORWARD - [0:0]' % (self.binary_name), ':%s-INPUT - [0:0]' % (self.binary_name), @@ -205,7 +207,7 @@ class IptablesManagerTestCase(test.TestCase): current_lines = [] new_lines = self.manager._modify_rules(current_lines, self.manager.ipv4['filter'], - table_name='filter') + 'filter') for line in ['*filter', 'COMMIT']: @@ -226,7 +228,8 @@ class IptablesManagerTestCase(test.TestCase): current_lines[12:12] = ['[0:0] -A FORWARD -j iptables-top-rule'] self.flags(iptables_top_regex='-j iptables-top-rule') new_lines = self.manager._modify_rules(current_lines, - self.manager.ipv4['filter']) + self.manager.ipv4['filter'], + 'filter') self.assertEqual(current_lines, new_lines) def test_iptables_bottom_order(self): @@ -235,7 +238,8 @@ class IptablesManagerTestCase(test.TestCase): current_lines[26:26] = ['[0:0] -A FORWARD -j iptables-bottom-rule'] self.flags(iptables_bottom_regex='-j iptables-bottom-rule') new_lines = self.manager._modify_rules(current_lines, - self.manager.ipv4['filter']) + self.manager.ipv4['filter'], + 'filter') self.assertEqual(current_lines, new_lines) def test_iptables_preserve_order(self): @@ -246,5 +250,6 @@ class IptablesManagerTestCase(test.TestCase): self.flags(iptables_top_regex='-j iptables-top-rule') self.flags(iptables_bottom_regex='-j iptables-bottom-rule') new_lines = self.manager._modify_rules(current_lines, - self.manager.ipv4['filter']) + self.manager.ipv4['filter'], + 'filter') self.assertEqual(current_lines, new_lines) -- cgit