x/vmware-nsx
Code Issues Proposed changes

Refactor netns.execute so that it is not necessary to check namespace

Browse Source 
I saw some code in a couple of reviews today that check whether a
namespace is set and run it under "ip netns exec ..." if it is.
Otherwise, it runs the command without it in the default namespace.

Change-Id: I55e8f4f3523ec7a7c5a6f082addf918952a05741
Closes-Bug: #1287524
This commit is contained in:
Carl Baldwin 2014-03-04 04:24:55 +00:00
parent 473dd1042d
commit 5a48896bc9
6 changed files with 37 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
neutron/agent/l3_agent.py
View File

@ -23,7 +23,6 @@ from neutron.agent.linux import interface
from neutron.agent.linux import ip_lib from neutron.agent.linux import ip_lib
from neutron.agent.linux import iptables_manager from neutron.agent.linux import iptables_manager
from neutron.agent.linux import ovs_lib # noqa from neutron.agent.linux import ovs_lib # noqa
from neutron.agent.linux import utils
from neutron.agent import rpc as agent_rpc from neutron.agent import rpc as agent_rpc
from neutron.common import constants as l3_constants from neutron.common import constants as l3_constants
from neutron.common import legacy from neutron.common import legacy
@ -579,13 +578,9 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
'-c', self.conf.send_arp_for_ha, '-c', self.conf.send_arp_for_ha,
ip_address] ip_address]
try: try:
if self.conf.use_namespaces: ip_wrapper = ip_lib.IPWrapper(self.root_helper,
ip_wrapper = ip_lib.IPWrapper(self.root_helper, namespace=ri.ns_name())
namespace=ri.ns_name()) ip_wrapper.netns.execute(arping_cmd, check_exit_code=True)
ip_wrapper.netns.execute(arping_cmd, check_exit_code=True)
else:
utils.execute(arping_cmd, check_exit_code=True,
root_helper=self.root_helper)
except Exception as e: except Exception as e:
LOG.error(_("Failed sending gratuitous ARP: %s"), str(e)) LOG.error(_("Failed sending gratuitous ARP: %s"), str(e))
@ -628,13 +623,9 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
gw_ip = ex_gw_port['subnet']['gateway_ip'] gw_ip = ex_gw_port['subnet']['gateway_ip']
if ex_gw_port['subnet']['gateway_ip']: if ex_gw_port['subnet']['gateway_ip']:
cmd = ['route', 'add', 'default', 'gw', gw_ip] cmd = ['route', 'add', 'default', 'gw', gw_ip]
if self.conf.use_namespaces: ip_wrapper = ip_lib.IPWrapper(self.root_helper,
ip_wrapper = ip_lib.IPWrapper(self.root_helper, namespace=ri.ns_name())
namespace=ri.ns_name()) ip_wrapper.netns.execute(cmd, check_exit_code=False)
ip_wrapper.netns.execute(cmd, check_exit_code=False)
else:
utils.execute(cmd, check_exit_code=False,
root_helper=self.root_helper)
def external_gateway_removed(self, ri, ex_gw_port, def external_gateway_removed(self, ri, ex_gw_port,
interface_name, internal_cidrs): interface_name, internal_cidrs):
@ -847,14 +838,9 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
def _update_routing_table(self, ri, operation, route): def _update_routing_table(self, ri, operation, route):
cmd = ['ip', 'route', operation, 'to', route['destination'], cmd = ['ip', 'route', operation, 'to', route['destination'],
'via', route['nexthop']] 'via', route['nexthop']]
#TODO(nati) move this code to iplib ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper,
if self.conf.use_namespaces: namespace=ri.ns_name())
ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper, ip_wrapper.netns.execute(cmd, check_exit_code=False)
namespace=ri.ns_name())
ip_wrapper.netns.execute(cmd, check_exit_code=False)
else:
utils.execute(cmd, check_exit_code=False,
root_helper=self.conf.root_helper)
def routes_updated(self, ri): def routes_updated(self, ri):
new_routes = ri.router['routes'] new_routes = ri.router['routes']

20
neutron/agent/linux/dhcp.py
View File

@ -367,24 +367,16 @@ class Dnsmasq(DhcpLocalProcess):
if self.conf.dhcp_domain: if self.conf.dhcp_domain:
cmd.append('--domain=%s' % self.conf.dhcp_domain) cmd.append('--domain=%s' % self.conf.dhcp_domain)
if self.network.namespace: ip_wrapper = ip_lib.IPWrapper(self.root_helper,
ip_wrapper = ip_lib.IPWrapper(self.root_helper, self.network.namespace)
self.network.namespace) ip_wrapper.netns.execute(cmd, addl_env=env)
ip_wrapper.netns.execute(cmd, addl_env=env)
else:
# For normal sudo prepend the env vars before command
cmd = ['%s=%s' % pair for pair in env.items()] + cmd
utils.execute(cmd, self.root_helper)
def _release_lease(self, mac_address, ip): def _release_lease(self, mac_address, ip):
"""Release a DHCP lease.""" """Release a DHCP lease."""
cmd = ['dhcp_release', self.interface_name, ip, mac_address] cmd = ['dhcp_release', self.interface_name, ip, mac_address]
if self.network.namespace: ip_wrapper = ip_lib.IPWrapper(self.root_helper,
ip_wrapper = ip_lib.IPWrapper(self.root_helper, self.network.namespace)
self.network.namespace) ip_wrapper.netns.execute(cmd)
ip_wrapper.netns.execute(cmd)
else:
utils.execute(cmd, self.root_helper)
def reload_allocations(self): def reload_allocations(self):
"""Rebuild the dnsmasq config and signal the dnsmasq to reload.""" """Rebuild the dnsmasq config and signal the dnsmasq to reload."""

8
neutron/agent/linux/external_process.py
View File

@ -50,12 +50,8 @@ class ProcessManager(object):
if not self.active: if not self.active:
cmd = cmd_callback(self.get_pid_file_name(ensure_pids_dir=True)) cmd = cmd_callback(self.get_pid_file_name(ensure_pids_dir=True))
if self.namespace: ip_wrapper = ip_lib.IPWrapper(self.root_helper, self.namespace)
ip_wrapper = ip_lib.IPWrapper(self.root_helper, self.namespace) ip_wrapper.netns.execute(cmd)
ip_wrapper.netns.execute(cmd)
else:
# For normal sudo prepend the env vars before command
utils.execute(cmd, self.root_helper)
def disable(self): def disable(self):
pid = self.pid pid = self.pid

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

@ -452,18 +452,18 @@ class IpNetnsCommand(IpCommandBase):
def execute(self, cmds, addl_env={}, check_exit_code=True): def execute(self, cmds, addl_env={}, check_exit_code=True):
if not self._parent.root_helper: if not self._parent.root_helper:
raise exceptions.SudoRequired() raise exceptions.SudoRequired()
elif not self._parent.namespace: ns_params = []
raise Exception(_('No namespace defined for parent')) if self._parent.namespace:
else: ns_params = ['ip', 'netns', 'exec', self._parent.namespace]
env_params = []
if addl_env: env_params = []
env_params = (['env'] + if addl_env:
['%s=%s' % pair for pair in addl_env.items()]) env_params = (['env'] +
return utils.execute( ['%s=%s' % pair for pair in addl_env.items()])
['ip', 'netns', 'exec', self._parent.namespace] + return utils.execute(
env_params + list(cmds), ns_params + env_params + list(cmds),
root_helper=self._parent.root_helper, root_helper=self._parent.root_helper,
check_exit_code=check_exit_code) check_exit_code=check_exit_code)
def exists(self, name): def exists(self, name):
output = self._as_root('list', options='o', use_root_namespace=True) output = self._as_root('list', options='o', use_root_namespace=True)

21
neutron/tests/unit/test_l3_agent.py
View File

@ -211,13 +211,8 @@ class TestBasicRouterOperations(base.BaseTestCase):
'-I', interface_name, '-I', interface_name,
'-c', self.conf.send_arp_for_ha, '-c', self.conf.send_arp_for_ha,
floating_ip] floating_ip]
if self.conf.use_namespaces: self.mock_ip.netns.execute.assert_any_call(
self.mock_ip.netns.execute.assert_any_call( arping_cmd, check_exit_code=True)
arping_cmd, check_exit_code=True)
else:
self.utils_exec.assert_any_call(arping_cmd,
check_exit_code=True,
root_helper=self.conf.root_helper)
def test_arping_namespace(self): def test_arping_namespace(self):
self._test_arping(namespace=True) self._test_arping(namespace=True)
@ -229,15 +224,9 @@ class TestBasicRouterOperations(base.BaseTestCase):
self._test_external_gateway_action('remove') self._test_external_gateway_action('remove')
def _check_agent_method_called(self, agent, calls, namespace): def _check_agent_method_called(self, agent, calls, namespace):
if namespace: self.mock_ip.netns.execute.assert_has_calls(
self.mock_ip.netns.execute.assert_has_calls( [mock.call(call, check_exit_code=False) for call in calls],
[mock.call(call, check_exit_code=False) for call in calls], any_order=True)
any_order=True)
else:
self.utils_exec.assert_has_calls([
mock.call(call, root_helper='sudo',
check_exit_code=False) for call in calls],
any_order=True)
def _test_routing_table_update(self, namespace): def _test_routing_table_update(self, namespace):
if not namespace: if not namespace:

4
neutron/tests/unit/test_linux_external_process.py
View File

@ -44,7 +44,9 @@ class TestProcessManager(base.BaseTestCase):
manager.enable(callback) manager.enable(callback)
callback.assert_called_once_with('pidfile') callback.assert_called_once_with('pidfile')
name.assert_called_once_with(ensure_pids_dir=True) name.assert_called_once_with(ensure_pids_dir=True)
self.execute.assert_called_once_with(['the', 'cmd'], 'sudo') self.execute.assert_called_once_with(['the', 'cmd'],
root_helper='sudo',
check_exit_code=True)
def test_enable_with_namespace(self): def test_enable_with_namespace(self):
callback = mock.Mock() callback = mock.Mock()