openstack/neutron
Code Issues Proposed changes

Merge "Move sysctl out of IPDevice class"

Browse Source
This commit is contained in:
Jenkins 2016-11-23 03:45:03 +00:00 committed by Gerrit Code Review
commit 9cb96b873e
7 changed files with 64 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
neutron/agent/l3/dvr_fip_ns.py
View File

@ -138,10 +138,9 @@ class FipNamespace(namespaces.Namespace):
# 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_lib.set_ip_nonlocal_bind(
failed = ip_lib.set_ip_nonlocal_bind(
value=1, namespace=self.name, log_fail_as_error=False)
except RuntimeError:
if failed:
LOG.debug('DVR: fip namespace (%s) does not support setting '
'net.ipv4.ip_nonlocal_bind, trying in root namespace',
self.name)

12
neutron/agent/l3/dvr_local_router.py
View File

@ -320,11 +320,10 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
def _snat_redirect_modify(self, gateway, sn_port, sn_int, is_add):
"""Adds or removes rules and routes for SNAT redirection."""
cmd = ['net.ipv4.conf.%s.send_redirects=0' % sn_int]
try:
ns_ipr = ip_lib.IPRule(namespace=self.ns_name)
ns_ipd = ip_lib.IPDevice(sn_int, namespace=self.ns_name)
if is_add:
ns_ipwrapr = ip_lib.IPWrapper(namespace=self.ns_name)
for port_fixed_ip in sn_port['fixed_ips']:
# Iterate and find the gateway IP address matching
# the IP version
@ -342,9 +341,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
ns_ipr.rule.add(ip=sn_port_cidr,
table=snat_idx,
priority=snat_idx)
ns_ipwrapr.netns.execute(
['sysctl', '-w',
'net.ipv4.conf.%s.send_redirects=0' % sn_int])
ip_lib.sysctl(cmd, namespace=self.ns_name)
else:
self._delete_gateway_device_if_exists(ns_ipd,
gw_ip_addr,
@ -426,9 +423,8 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
# TODO(Carl) Refactor external_gateway_added/updated/removed to use
# super class implementation where possible. Looks like preserve_ips,
# and ns_name are the key differences.
ip_wrapr = ip_lib.IPWrapper(namespace=self.ns_name)
ip_wrapr.netns.execute(['sysctl', '-w',
'net.ipv4.conf.all.send_redirects=0'])
cmd = ['net.ipv4.conf.all.send_redirects=0']
ip_lib.sysctl(cmd, namespace=self.ns_name)
for p in self.internal_ports:
gateway = self.get_snat_port_for_internal_port(p)
id_name = self.get_internal_device_name(p['id'])

4
neutron/agent/linux/interface.py
View File

@ -216,8 +216,8 @@ class LinuxInterfaceDriver(object):
def configure_ipv6_ra(namespace, dev_name):
"""Configure acceptance of IPv6 route advertisements on an intf."""
# Learn the default router's IP address via RAs
ip_lib.IPWrapper(namespace=namespace).netns.execute(
['sysctl', '-w', 'net.ipv6.conf.%s.accept_ra=2' % dev_name])
cmd = ['net.ipv6.conf.%s.accept_ra=2' % dev_name]
ip_lib.sysctl(cmd, namespace=namespace)
@abc.abstractmethod
def plug_new(self, network_id, port_id, device_name, mac_address,

90
neutron/agent/linux/ip_lib.py
View File

@ -329,36 +329,10 @@ class IPDevice(SubProcessBase):
LOG.exception(_LE("Failed deleting egress connection state of"
" floatingip %s"), ip_str)
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 = IPWrapper(self.namespace)
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
def disable_ipv6(self):
sysctl_name = re.sub(r'\.', '/', self.name)
cmd = 'net.ipv6.conf.%s.disable_ipv6=1' % sysctl_name
return self._sysctl([cmd])
cmd = ['net.ipv6.conf.%s.disable_ipv6=1' % sysctl_name]
return sysctl(cmd, namespace=self.namespace)
@property
def name(self):
@ -1085,6 +1059,43 @@ def send_ip_addr_adv_notif(
eventlet.spawn_n(arping)
def sysctl(cmd, namespace=None, log_fail_as_error=True):
"""Run sysctl command 'cmd'
@param cmd: a list containing the sysctl command to run
@param namespace: network namespace to run command in
@param log_fail_as_error: failure logged as LOG.error
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 = IPWrapper(namespace=namespace)
try:
ip_wrapper.netns.execute(cmd, run_as_root=True,
log_fail_as_error=log_fail_as_error)
except RuntimeError as rte:
LOG.warning(
_LW("Setting %(cmd)s in namespace %(ns)s failed: %(err)s."),
{'cmd': cmd,
'ns': namespace,
'err': rte})
return 1
return 0
def add_namespace_to_cmd(cmd, namespace=None):
"""Add an optional namespace to the command."""
@ -1108,25 +1119,20 @@ def get_ip_nonlocal_bind(namespace=None):
def set_ip_nonlocal_bind(value, namespace=None, log_fail_as_error=True):
"""Set sysctl knob of ip_nonlocal_bind to given value."""
cmd = ['sysctl', '-w', '%s=%d' % (IP_NONLOCAL_BIND, value)]
ip_wrapper = IPWrapper(namespace)
ip_wrapper.netns.execute(
cmd, run_as_root=True, log_fail_as_error=log_fail_as_error)
cmd = ['%s=%d' % (IP_NONLOCAL_BIND, value)]
return sysctl(cmd, namespace=namespace,
log_fail_as_error=log_fail_as_error)
def set_ip_nonlocal_bind_for_namespace(namespace):
"""Set ip_nonlocal_bind but don't raise exception on failure."""
try:
set_ip_nonlocal_bind(
value=0, namespace=namespace, log_fail_as_error=False)
except RuntimeError as rte:
failed = set_ip_nonlocal_bind(value=0, namespace=namespace,
log_fail_as_error=False)
if failed:
LOG.warning(
_LW("Setting %(knob)s=0 in namespace %(ns)s failed: %(err)s. It "
"will not be set to 0 in the root namespace in order to not "
"break DVR, which requires this value be set to 1. This "
_LW("%s will not be set to 0 in the root namespace in order to "
"not break DVR, which requires this value be set to 1. This "
"may introduce a race between moving a floating IP to a "
"different network node, and the peer side getting a "
"populated ARP cache for a given floating IP address."),
{'knob': IP_NONLOCAL_BIND,
'ns': namespace,
'err': rte})
IP_NONLOCAL_BIND)

6
neutron/tests/functional/agent/linux/test_ip_lib.py
View File

@ -221,8 +221,9 @@ class TestSetIpNonlocalBind(functional_base.BaseSudoTestCase):
def test_assigned_value(self):
namespace = self.useFixture(net_helpers.NamespaceFixture())
for expected in (0, 1):
failed = ip_lib.set_ip_nonlocal_bind(expected, namespace.name)
try:
ip_lib.set_ip_nonlocal_bind(expected, namespace.name)
observed = ip_lib.get_ip_nonlocal_bind(namespace.name)
except RuntimeError as rte:
stat_message = (
'cannot stat /proc/sys/net/ipv4/ip_nonlocal_bind')
@ -231,5 +232,6 @@ class TestSetIpNonlocalBind(functional_base.BaseSudoTestCase):
"This kernel doesn't support %s in network "
"namespaces." % ip_lib.IP_NONLOCAL_BIND)
raise
observed = ip_lib.get_ip_nonlocal_bind(namespace.name)
self.assertFalse(failed)
self.assertEqual(expected, observed)

6
neutron/tests/unit/agent/linux/test_bridge_lib.py
View File

@ -36,9 +36,9 @@ 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):
def _verify_bridge_sysctl_mock(self, cmd):
self.execute.assert_called_once_with(cmd, run_as_root=True,
check_exit_code=True)
log_fail_as_error=True)
self.execute.reset_mock()
def test_is_bridged_interface(self):
@ -69,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_check_exit_code(['sysctl', '-w', cmd])
self._verify_bridge_sysctl_mock(['sysctl', '-w', cmd])
br.addif(self._IF_NAME)
self._verify_bridge_mock(

3
neutron/tests/unit/agent/linux/test_ip_lib.py
View File

@ -1434,6 +1434,5 @@ class TestAddNamespaceToCmd(base.BaseTestCase):
class TestSetIpNonlocalBindForHaNamespace(base.BaseTestCase):
def test_setting_failure(self):
"""Make sure message is formatted correctly."""
with mock.patch.object(
ip_lib, 'set_ip_nonlocal_bind', side_effect=RuntimeError):
with mock.patch.object(ip_lib, 'set_ip_nonlocal_bind', return_value=1):
ip_lib.set_ip_nonlocal_bind_for_namespace('foo')