diff --git a/neutron/agent/linux/bridge_lib.py b/neutron/agent/linux/bridge_lib.py index 13610f4cf88..625ae9404f8 100644 --- a/neutron/agent/linux/bridge_lib.py +++ b/neutron/agent/linux/bridge_lib.py @@ -17,8 +17,12 @@ # under the License. import os +from oslo_log import log as logging from neutron.agent.linux import ip_lib +from neutron.i18n import _LE + +LOG = logging.getLogger(__name__) # NOTE(toabctl): Don't use /sys/devices/virtual/net here because not all tap # devices are listed here (i.e. when using Xen) @@ -47,9 +51,30 @@ class BridgeDevice(ip_lib.IPDevice): return ip_wrapper.netns.execute(cmd, run_as_root=True) def _sysctl(self, cmd): + """execute() doesn't return the exit status of the command it runs, + it returns stdout and stderr. Setting check_exit_code=True will cause + it to raise a RuntimeError if the exit status of the command is + non-zero, which in sysctl's case is an error. So we're normalizing + that into zero (success) and one (failure) here to mimic what + "echo $?" in a shell would be. + + This is all because sysctl is too verbose and prints the value you + just set on success, unlike most other utilities that print nothing. + + execute() will have dumped a message to the logs with the actual + output on failure, so it's not lost, and we don't need to print it + here. + """ cmd = ['sysctl', '-w'] + cmd ip_wrapper = ip_lib.IPWrapper(self.namespace) - return ip_wrapper.netns.execute(cmd, run_as_root=True) + try: + ip_wrapper.netns.execute(cmd, run_as_root=True, + check_exit_code=True) + except RuntimeError: + LOG.exception(_LE("Failed running %s"), cmd) + return 1 + + return 0 @classmethod def addbr(cls, name, namespace=None): diff --git a/neutron/tests/functional/agent/linux/test_bridge_lib.py b/neutron/tests/functional/agent/linux/test_bridge_lib.py index 7c88b8cb9da..5eb90d12c8d 100644 --- a/neutron/tests/functional/agent/linux/test_bridge_lib.py +++ b/neutron/tests/functional/agent/linux/test_bridge_lib.py @@ -59,3 +59,17 @@ class BridgeLibTestCase(base.BaseSudoTestCase): def test_get_interfaces_no_bridge(self): bridge = bridge_lib.BridgeDevice('--fake--') self.assertEqual([], bridge.get_interfaces()) + + def test_disable_ipv6(self): + sysfs_path = ("/proc/sys/net/ipv6/conf/%s/disable_ipv6" % + self.bridge.name) + + # first, make sure it's enabled + with open(sysfs_path, 'r') as sysfs_disable_ipv6_file: + sysfs_disable_ipv6 = sysfs_disable_ipv6_file.read() + self.assertEqual("0\n", sysfs_disable_ipv6) + + self.assertEqual(0, self.bridge.disable_ipv6()) + with open(sysfs_path, 'r') as sysfs_disable_ipv6_file: + sysfs_disable_ipv6 = sysfs_disable_ipv6_file.read() + self.assertEqual("1\n", sysfs_disable_ipv6) diff --git a/neutron/tests/unit/agent/linux/test_bridge_lib.py b/neutron/tests/unit/agent/linux/test_bridge_lib.py index d1c842c425b..0c28ac8df2a 100644 --- a/neutron/tests/unit/agent/linux/test_bridge_lib.py +++ b/neutron/tests/unit/agent/linux/test_bridge_lib.py @@ -36,6 +36,11 @@ class BridgeLibTest(base.BaseTestCase): self.execute.assert_called_once_with(cmd, run_as_root=True) self.execute.reset_mock() + def _verify_bridge_mock_check_exit_code(self, cmd): + self.execute.assert_called_once_with(cmd, run_as_root=True, + check_exit_code=True) + self.execute.reset_mock() + def test_is_bridged_interface(self): exists = lambda path: path == "/sys/class/net/tapOK/brport" with mock.patch('os.path.exists', side_effect=exists): @@ -64,7 +69,7 @@ class BridgeLibTest(base.BaseTestCase): br.disable_ipv6() cmd = 'net.ipv6.conf.%s.disable_ipv6=1' % self._BR_NAME - self._verify_bridge_mock(['sysctl', '-w', cmd]) + self._verify_bridge_mock_check_exit_code(['sysctl', '-w', cmd]) br.addif(self._IF_NAME) self._verify_bridge_mock(