From 3f7054ed4de0da80320c55ec42b1464d88bceae8 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 25 Aug 2015 16:12:42 +0200 Subject: [PATCH] Pass -w flag to iptables to make it wait for xtables lock Change-Id: I8969da5e58550f71dba79b458feb63ed28e0585f Closes-Bug: #1484110 --- ironic_inspector/firewall.py | 22 ++++++++++++++++++--- ironic_inspector/test/test_firewall.py | 27 ++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/ironic_inspector/firewall.py b/ironic_inspector/firewall.py index efd7f2215..249f3f024 100644 --- a/ironic_inspector/firewall.py +++ b/ironic_inspector/firewall.py @@ -11,13 +11,14 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os import subprocess from eventlet import semaphore from oslo_config import cfg from oslo_log import log -from ironic_inspector.common.i18n import _LE +from ironic_inspector.common.i18n import _LE, _LW from ironic_inspector import node_cache from ironic_inspector import utils @@ -28,10 +29,12 @@ NEW_CHAIN = None CHAIN = None INTERFACE = None LOCK = semaphore.BoundedSemaphore() +BASE_COMMAND = ('iptables',) def _iptables(*args, **kwargs): - cmd = ('iptables',) + args + # NOTE(dtantsur): -w flag makes it wait for xtables lock + cmd = BASE_COMMAND + args ignore = kwargs.pop('ignore', False) LOG.debug('Running iptables %s', args) kwargs['stderr'] = subprocess.STDOUT @@ -54,11 +57,24 @@ def init(): if not CONF.firewall.manage_firewall: return - global INTERFACE, CHAIN, NEW_CHAIN + global INTERFACE, CHAIN, NEW_CHAIN, BASE_COMMAND INTERFACE = CONF.firewall.dnsmasq_interface CHAIN = CONF.firewall.firewall_chain NEW_CHAIN = CHAIN + '_temp' + # -w flag makes iptables wait for xtables lock, but it's not supported + # everywhere yet + try: + with open(os.devnull, 'wb') as null: + subprocess.check_call(['iptables', '-w', '-h'], + stderr=null, stdout=null) + except subprocess.CalledProcessError: + LOG.warn(_LW('iptables does not support -w flag, please update ' + 'it to at least version 1.4.21')) + BASE_COMMAND = ('iptables',) + else: + BASE_COMMAND = ('iptables', '-w') + _clean_up(CHAIN) # Not really needed, but helps to validate that we have access to iptables _iptables('-N', CHAIN) diff --git a/ironic_inspector/test/test_firewall.py b/ironic_inspector/test/test_firewall.py index a786daaab..1adda78f5 100644 --- a/ironic_inspector/test/test_firewall.py +++ b/ironic_inspector/test/test_firewall.py @@ -13,8 +13,9 @@ # License for the specific language governing permissions and limitations # under the License. -import mock +import subprocess +import mock from oslo_config import cfg from ironic_inspector import firewall @@ -35,7 +36,8 @@ class TestFirewall(test_base.NodeTest): firewall.update_filters() self.assertEqual(0, mock_iptables.call_count) - def test_init_args(self, mock_get_client, mock_iptables): + @mock.patch.object(subprocess, 'check_call') + def test_init_args(self, mock_call, mock_get_client, mock_iptables): firewall.init() init_expected_args = [ ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', '67', @@ -49,6 +51,27 @@ class TestFirewall(test_base.NodeTest): for (args, call) in zip(init_expected_args, call_args_list): self.assertEqual(args, call[0]) + self.assertEqual(('iptables', '-w'), firewall.BASE_COMMAND) + + @mock.patch.object(subprocess, 'check_call') + def test_init_args_old_iptables(self, mock_call, mock_get_client, + mock_iptables): + mock_call.side_effect = subprocess.CalledProcessError(2, '') + firewall.init() + init_expected_args = [ + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', '67', + '-j', CONF.firewall.firewall_chain), + ('-F', CONF.firewall.firewall_chain), + ('-X', CONF.firewall.firewall_chain), + ('-N', CONF.firewall.firewall_chain)] + + call_args_list = mock_iptables.call_args_list + + for (args, call) in zip(init_expected_args, call_args_list): + self.assertEqual(args, call[0]) + + self.assertEqual(('iptables',), firewall.BASE_COMMAND) + def test_init_kwargs(self, mock_get_client, mock_iptables): firewall.init() init_expected_kwargs = [