diff --git a/ironic_inspector/firewall.py b/ironic_inspector/firewall.py index 2f160a9fd..da51bb541 100644 --- a/ironic_inspector/firewall.py +++ b/ironic_inspector/firewall.py @@ -30,6 +30,7 @@ CHAIN = None INTERFACE = None LOCK = semaphore.BoundedSemaphore() BASE_COMMAND = None +BLACKLIST_CACHE = None def _iptables(*args, **kwargs): @@ -58,7 +59,8 @@ def init(): if not CONF.firewall.manage_firewall: return - global INTERFACE, CHAIN, NEW_CHAIN, BASE_COMMAND + global INTERFACE, CHAIN, NEW_CHAIN, BASE_COMMAND, BLACKLIST_CACHE + BLACKLIST_CACHE = None INTERFACE = CONF.firewall.dnsmasq_interface CHAIN = CONF.firewall.firewall_chain NEW_CHAIN = CHAIN + '_temp' @@ -117,6 +119,8 @@ def update_filters(ironic=None): :param ironic: Ironic client instance, optional. """ + global BLACKLIST_CACHE + if not CONF.firewall.manage_firewall: return @@ -126,7 +130,14 @@ def update_filters(ironic=None): with LOCK: macs_active = set(p.address for p in ironic.port.list(limit=0)) to_blacklist = macs_active - node_cache.active_macs() + if BLACKLIST_CACHE is not None and to_blacklist == BLACKLIST_CACHE: + LOG.debug('Not updating iptables - no changes in MAC list %s', + to_blacklist) + return + LOG.debug('Blacklisting active MAC\'s %s', to_blacklist) + # Force update on the next iteration if this attempt fails + BLACKLIST_CACHE = None # Clean up a bit to account for possible troubles on previous run _clean_up(NEW_CHAIN) @@ -148,3 +159,6 @@ def update_filters(ironic=None): _iptables('-F', CHAIN, ignore=True) _iptables('-X', CHAIN, ignore=True) _iptables('-E', NEW_CHAIN, CHAIN) + + # Cache result of successful iptables update + BLACKLIST_CACHE = to_blacklist diff --git a/ironic_inspector/test/test_firewall.py b/ironic_inspector/test/test_firewall.py index 6fba22100..2f7aaf6e9 100644 --- a/ironic_inspector/test/test_firewall.py +++ b/ironic_inspector/test/test_firewall.py @@ -192,3 +192,55 @@ class TestFirewall(test_base.NodeTest): for (args, call) in zip(update_filters_expected_args, call_args_list): self.assertEqual(args, call[0]) + + # check caching + + mock_iptables.reset_mock() + firewall.update_filters(mock_get_client) + self.assertFalse(mock_iptables.called) + + def test_update_filters_clean_cache_on_error(self, mock_call, + mock_get_client, + mock_iptables): + active_macs = ['11:22:33:44:55:66', '66:55:44:33:22:11'] + inactive_mac = ['AA:BB:CC:DD:EE:FF'] + self.macs = active_macs + inactive_mac + self.ports = [mock.Mock(address=m) for m in self.macs] + mock_get_client.port.list.return_value = self.ports + node_cache.add_node(self.node.uuid, mac=active_macs, + bmc_address='1.2.3.4', foo=None) + firewall.init() + + update_filters_expected_args = [ + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-j', firewall.NEW_CHAIN), + ('-F', firewall.NEW_CHAIN), + ('-X', firewall.NEW_CHAIN), + ('-N', firewall.NEW_CHAIN), + # Blacklist + ('-A', firewall.NEW_CHAIN, '-m', 'mac', '--mac-source', + inactive_mac[0], '-j', 'DROP'), + ('-A', firewall.NEW_CHAIN, '-j', 'ACCEPT'), + ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-j', firewall.NEW_CHAIN), + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-j', CONF.firewall.firewall_chain), + ('-F', CONF.firewall.firewall_chain), + ('-X', CONF.firewall.firewall_chain), + ('-E', firewall.NEW_CHAIN, CONF.firewall.firewall_chain) + ] + + mock_iptables.side_effect = [None, None, RuntimeError()] + self.assertRaises(RuntimeError, firewall.update_filters, + mock_get_client) + + # check caching + + mock_iptables.reset_mock() + mock_iptables.side_effect = None + firewall.update_filters(mock_get_client) + call_args_list = mock_iptables.call_args_list + + for (args, call) in zip(update_filters_expected_args, + call_args_list): + self.assertEqual(args, call[0]) diff --git a/releasenotes/notes/less-iptables-calls-759e89d103df504c.yaml b/releasenotes/notes/less-iptables-calls-759e89d103df504c.yaml new file mode 100644 index 000000000..3cda8aaea --- /dev/null +++ b/releasenotes/notes/less-iptables-calls-759e89d103df504c.yaml @@ -0,0 +1,3 @@ +--- +fixes: + - Only issue iptables calls when list of active MAC's changes.