From: Carl Baldwin Date: Tue, 4 Mar 2014 04:24:55 +0000 (+0000) Subject: Refactor netns.execute so that it is not necessary to check namespace X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=40390598c5a440d1bbfa4f229130eeedf5cd4dba;p=openstack-build%2Fneutron-build.git Refactor netns.execute so that it is not necessary to check namespace 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 --- diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index 5c8c8757e..1f92f35c2 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -23,7 +23,6 @@ from neutron.agent.linux import interface from neutron.agent.linux import ip_lib from neutron.agent.linux import iptables_manager from neutron.agent.linux import ovs_lib # noqa -from neutron.agent.linux import utils from neutron.agent import rpc as agent_rpc from neutron.common import constants as l3_constants from neutron.common import legacy @@ -579,13 +578,9 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): '-c', self.conf.send_arp_for_ha, ip_address] try: - if self.conf.use_namespaces: - ip_wrapper = ip_lib.IPWrapper(self.root_helper, - namespace=ri.ns_name()) - ip_wrapper.netns.execute(arping_cmd, check_exit_code=True) - else: - utils.execute(arping_cmd, check_exit_code=True, - root_helper=self.root_helper) + ip_wrapper = ip_lib.IPWrapper(self.root_helper, + namespace=ri.ns_name()) + ip_wrapper.netns.execute(arping_cmd, check_exit_code=True) except Exception as 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'] if ex_gw_port['subnet']['gateway_ip']: cmd = ['route', 'add', 'default', 'gw', gw_ip] - if self.conf.use_namespaces: - ip_wrapper = ip_lib.IPWrapper(self.root_helper, - namespace=ri.ns_name()) - ip_wrapper.netns.execute(cmd, check_exit_code=False) - else: - utils.execute(cmd, check_exit_code=False, - root_helper=self.root_helper) + ip_wrapper = ip_lib.IPWrapper(self.root_helper, + namespace=ri.ns_name()) + ip_wrapper.netns.execute(cmd, check_exit_code=False) def external_gateway_removed(self, ri, ex_gw_port, interface_name, internal_cidrs): @@ -847,14 +838,9 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): def _update_routing_table(self, ri, operation, route): cmd = ['ip', 'route', operation, 'to', route['destination'], 'via', route['nexthop']] - #TODO(nati) move this code to iplib - if self.conf.use_namespaces: - ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper, - 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) + ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper, + namespace=ri.ns_name()) + ip_wrapper.netns.execute(cmd, check_exit_code=False) def routes_updated(self, ri): new_routes = ri.router['routes'] diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 08cedfdcf..013920574 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -367,24 +367,16 @@ class Dnsmasq(DhcpLocalProcess): if self.conf.dhcp_domain: cmd.append('--domain=%s' % self.conf.dhcp_domain) - if self.network.namespace: - ip_wrapper = ip_lib.IPWrapper(self.root_helper, - self.network.namespace) - 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) + ip_wrapper = ip_lib.IPWrapper(self.root_helper, + self.network.namespace) + ip_wrapper.netns.execute(cmd, addl_env=env) def _release_lease(self, mac_address, ip): """Release a DHCP lease.""" cmd = ['dhcp_release', self.interface_name, ip, mac_address] - if self.network.namespace: - ip_wrapper = ip_lib.IPWrapper(self.root_helper, - self.network.namespace) - ip_wrapper.netns.execute(cmd) - else: - utils.execute(cmd, self.root_helper) + ip_wrapper = ip_lib.IPWrapper(self.root_helper, + self.network.namespace) + ip_wrapper.netns.execute(cmd) def reload_allocations(self): """Rebuild the dnsmasq config and signal the dnsmasq to reload.""" diff --git a/neutron/agent/linux/external_process.py b/neutron/agent/linux/external_process.py index 9c62f2f87..d0e990880 100644 --- a/neutron/agent/linux/external_process.py +++ b/neutron/agent/linux/external_process.py @@ -50,12 +50,8 @@ class ProcessManager(object): if not self.active: 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.netns.execute(cmd) - else: - # For normal sudo prepend the env vars before command - utils.execute(cmd, self.root_helper) + ip_wrapper = ip_lib.IPWrapper(self.root_helper, self.namespace) + ip_wrapper.netns.execute(cmd) def disable(self): pid = self.pid diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 297c566f8..a140be290 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -452,18 +452,18 @@ class IpNetnsCommand(IpCommandBase): def execute(self, cmds, addl_env={}, check_exit_code=True): if not self._parent.root_helper: raise exceptions.SudoRequired() - elif not self._parent.namespace: - raise Exception(_('No namespace defined for parent')) - else: - env_params = [] - if addl_env: - env_params = (['env'] + - ['%s=%s' % pair for pair in addl_env.items()]) - return utils.execute( - ['ip', 'netns', 'exec', self._parent.namespace] + - env_params + list(cmds), - root_helper=self._parent.root_helper, - check_exit_code=check_exit_code) + ns_params = [] + if self._parent.namespace: + ns_params = ['ip', 'netns', 'exec', self._parent.namespace] + + env_params = [] + if addl_env: + env_params = (['env'] + + ['%s=%s' % pair for pair in addl_env.items()]) + return utils.execute( + ns_params + env_params + list(cmds), + root_helper=self._parent.root_helper, + check_exit_code=check_exit_code) def exists(self, name): output = self._as_root('list', options='o', use_root_namespace=True) diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 87f4df520..76f491072 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -211,13 +211,8 @@ class TestBasicRouterOperations(base.BaseTestCase): '-I', interface_name, '-c', self.conf.send_arp_for_ha, floating_ip] - if self.conf.use_namespaces: - self.mock_ip.netns.execute.assert_any_call( - 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) + self.mock_ip.netns.execute.assert_any_call( + arping_cmd, check_exit_code=True) def test_arping_namespace(self): self._test_arping(namespace=True) @@ -229,15 +224,9 @@ class TestBasicRouterOperations(base.BaseTestCase): self._test_external_gateway_action('remove') def _check_agent_method_called(self, agent, calls, namespace): - if namespace: - self.mock_ip.netns.execute.assert_has_calls( - [mock.call(call, check_exit_code=False) for call in calls], - 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) + self.mock_ip.netns.execute.assert_has_calls( + [mock.call(call, check_exit_code=False) for call in calls], + any_order=True) def _test_routing_table_update(self, namespace): if not namespace: diff --git a/neutron/tests/unit/test_linux_external_process.py b/neutron/tests/unit/test_linux_external_process.py index 081aabe16..36146a2ba 100644 --- a/neutron/tests/unit/test_linux_external_process.py +++ b/neutron/tests/unit/test_linux_external_process.py @@ -44,7 +44,9 @@ class TestProcessManager(base.BaseTestCase): manager.enable(callback) callback.assert_called_once_with('pidfile') 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): callback = mock.Mock()