Set ip_nonlocal_bind in namespace if it exists

Somewhere in the 3.19 kernel timeframe ip_nonlocal_bind was
changed to be a per-namespace attribute. To be backwards
compatible we need to try that first, then fall-back to
setting the one in the root namespace if it fails.

Closes-Bug: #1507078
Change-Id: I018e800bc8d4e85d067aaae865c9b04bf030c980

diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py
index 628bfbc2fb21010c43080eafd3ae1244c27af185..6d4cf7bc3dcba7d30d7793b0c701a803f50bd5c3 100644 (file)
--- a/neutron/agent/l3/dvr_fip_ns.py
+++ b/neutron/agent/l3/dvr_fip_ns.py
@@ -130,13 +130,27 @@ class FipNamespace(namespaces.Namespace):
 
     def create(self):
         # TODO(Carl) Get this functionality from mlavelle's namespace baseclass
-        LOG.debug("add fip-namespace(%s)", self.name)
+        LOG.debug("DVR: add fip namespace: %s", self.name)
         ip_wrapper_root = ip_lib.IPWrapper()
-        ip_wrapper_root.netns.execute(['sysctl',
-                                       '-w',
-                                       'net.ipv4.ip_nonlocal_bind=1'],
-                                      run_as_root=True)
         ip_wrapper = ip_wrapper_root.ensure_namespace(self.get_name())
+        # Somewhere in the 3.19 kernel timeframe ip_nonlocal_bind was
+        # changed to be a per-namespace attribute.  To be backwards
+        # compatible we need to try both if at first we fail.
+        try:
+            ip_wrapper.netns.execute(['sysctl',
+                                      '-w',
+                                      'net.ipv4.ip_nonlocal_bind=1'],
+                                     log_fail_as_error=False,
+                                     run_as_root=True)
+        except RuntimeError:
+            LOG.debug('DVR: fip namespace (%s) does not support setting '
+                      'net.ipv4.ip_nonlocal_bind, trying in root namespace',
+                      self.name)
+            ip_wrapper_root.netns.execute(['sysctl',
+                                           '-w',
+                                           'net.ipv4.ip_nonlocal_bind=1'],
+                                          run_as_root=True)
+
         ip_wrapper.netns.execute(['sysctl', '-w', 'net.ipv4.ip_forward=1'])
         if self.use_ipv6:
             ip_wrapper.netns.execute(['sysctl', '-w',
@@ -166,7 +180,7 @@ class FipNamespace(namespaces.Namespace):
         self.agent_gateway_port = None
 
         # TODO(mrsmith): add LOG warn if fip count != 0
-        LOG.debug('DVR: destroy fip ns: %s', self.name)
+        LOG.debug('DVR: destroy fip namespace: %s', self.name)
         super(FipNamespace, self).delete()
 
     def create_gateway_port(self, agent_gateway_port):

diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py
index abbacb1412a681851b07303cd1368d55c6910084..95026cfbc76108377ab207fe375e540cce0237d2 100644 (file)
--- a/neutron/agent/linux/ip_lib.py
+++ b/neutron/agent/linux/ip_lib.py
@@ -821,7 +821,8 @@ class IpNetnsCommand(IpCommandBase):
         self._as_root([], ('delete', name), use_root_namespace=True)
 
     def execute(self, cmds, addl_env=None, check_exit_code=True,
-                extra_ok_codes=None, run_as_root=False):
+                log_fail_as_error=True, extra_ok_codes=None,
+                run_as_root=False):
         ns_params = []
         kwargs = {'run_as_root': run_as_root}
         if self._parent.namespace:
@@ -834,7 +835,8 @@ class IpNetnsCommand(IpCommandBase):
                           ['%s=%s' % pair for pair in addl_env.items()])
         cmd = ns_params + env_params + list(cmds)
         return utils.execute(cmd, check_exit_code=check_exit_code,
-                             extra_ok_codes=extra_ok_codes, **kwargs)
+                             extra_ok_codes=extra_ok_codes,
+                             log_fail_as_error=log_fail_as_error, **kwargs)
 
     def exists(self, name):
         if not cfg.CONF.AGENT.use_helper_for_ns_read:

diff --git a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
index a87aa31243d6fed63298bc35d3c40bb57a59c24a..bb558ceb70a7ba450d5772150c0bcff28edba8fa 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
+++ b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
@@ -15,9 +15,11 @@
 import mock
 from oslo_utils import uuidutils
 
+from neutron.agent.common import utils
 from neutron.agent.l3 import dvr_fip_ns
 from neutron.agent.l3 import link_local_allocator as lla
 from neutron.agent.linux import ip_lib
+from neutron.agent.linux import iptables_manager
 from neutron.tests import base
 
 _uuid = uuidutils.generate_uuid
@@ -94,6 +96,41 @@ class TestDvrFipNs(base.BaseTestCase):
                                                '20.0.0.30',
                                                mock.ANY)
 
+    @mock.patch.object(iptables_manager, 'IptablesManager')
+    @mock.patch.object(utils, 'execute')
+    @mock.patch.object(ip_lib.IpNetnsCommand, 'exists')
+    def _test_create(self, old_kernel, exists, execute, IPTables):
+        exists.return_value = True
+        # There are up to four sysctl calls - two for ip_nonlocal_bind,
+        # and two to enable forwarding
+        execute.side_effect = [RuntimeError if old_kernel else None,
+                               None, None, None]
+
+        self.fip_ns._iptables_manager = IPTables()
+        self.fip_ns.create()
+
+        ns_name = self.fip_ns.get_name()
+
+        netns_cmd = ['ip', 'netns', 'exec', ns_name]
+        bind_cmd = ['sysctl', '-w', 'net.ipv4.ip_nonlocal_bind=1']
+        expected = [mock.call(netns_cmd + bind_cmd, check_exit_code=True,
+                              extra_ok_codes=None, log_fail_as_error=False,
+                              run_as_root=True)]
+
+        if old_kernel:
+            expected.append(mock.call(bind_cmd, check_exit_code=True,
+                                      extra_ok_codes=None,
+                                      log_fail_as_error=True,
+                                      run_as_root=True))
+
+        execute.assert_has_calls(expected)
+
+    def test_create_old_kernel(self):
+        self._test_create(True)
+
+    def test_create_new_kernel(self):
+        self._test_create(False)
+
     @mock.patch.object(ip_lib, 'IPWrapper')
     def test_destroy(self, IPWrapper):
         ip_wrapper = IPWrapper()

diff --git a/neutron/tests/unit/agent/linux/test_external_process.py b/neutron/tests/unit/agent/linux/test_external_process.py
index a94774d7a507ffe106ac72dd5943e36f7d7ad7af..e5ff67211c19a7f104f986c84e32c14d0624a826 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_external_process.py
+++ b/neutron/tests/unit/agent/linux/test_external_process.py
@@ -131,7 +131,8 @@ class TestProcessManager(base.BaseTestCase):
                 self.execute.assert_called_once_with(['the', 'cmd'],
                                                      check_exit_code=True,
                                                      extra_ok_codes=None,
-                                                     run_as_root=False)
+                                                     run_as_root=False,
+                                                     log_fail_as_error=True)
 
     def test_enable_with_namespace(self):
         callback = mock.Mock()

diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py
index 2aa57ccc65e114206c6ef971306fa2e5a4e3b191..cd043c7f4c124a6e73148b9f23c5834c0a22671f 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_ip_lib.py
+++ b/neutron/tests/unit/agent/linux/test_ip_lib.py
@@ -1181,7 +1181,8 @@ class TestIpNetnsCommand(TestIPCmdBase):
             execute.assert_called_once_with(
                 ['ip', 'netns', 'exec', 'ns',
                  'sysctl', '-w', 'net.ipv4.conf.all.promote_secondaries=1'],
-                run_as_root=True, check_exit_code=True, extra_ok_codes=None)
+                run_as_root=True, check_exit_code=True, extra_ok_codes=None,
+                log_fail_as_error=True)
 
     def test_delete_namespace(self):
         with mock.patch('neutron.agent.common.utils.execute'):
@@ -1196,7 +1197,8 @@ class TestIpNetnsCommand(TestIPCmdBase):
                                              'link', 'list'],
                                             run_as_root=True,
                                             check_exit_code=True,
-                                            extra_ok_codes=None)
+                                            extra_ok_codes=None,
+                                            log_fail_as_error=True)
 
     def test_execute_env_var_prepend(self):
         self.parent.namespace = 'ns'
@@ -1207,7 +1209,8 @@ class TestIpNetnsCommand(TestIPCmdBase):
                 ['ip', 'netns', 'exec', 'ns', 'env'] +
                 ['%s=%s' % (k, v) for k, v in env.items()] +
                 ['ip', 'link', 'list'],
-                run_as_root=True, check_exit_code=True, extra_ok_codes=None)
+                run_as_root=True, check_exit_code=True, extra_ok_codes=None,
+                log_fail_as_error=True)
 
     def test_execute_nosudo_with_no_namespace(self):
         with mock.patch('neutron.agent.common.utils.execute') as execute:
@@ -1216,7 +1219,8 @@ class TestIpNetnsCommand(TestIPCmdBase):
             execute.assert_called_once_with(['test'],
                                             check_exit_code=True,
                                             extra_ok_codes=None,
-                                            run_as_root=False)
+                                            run_as_root=False,
+                                            log_fail_as_error=True)
 
 
 class TestDeviceExists(base.BaseTestCase):