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 5c8c8757e582b336ae379f0826a5b4aec71e6183..1f92f35c2934cea3b92e02854083ece8a8e4ebcb 100644 (file)
--- 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 08cedfdcf3c99cbf6fa380834c15451dd894e621..0139205740f4c536e96790c2f346ee47ab960eda 100644 (file)
--- 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 9c62f2f87be03daaf49e529940e04f73c2f8fcba..d0e9908802bc81f5ac8191f5cd514176895c1b26 100644 (file)
--- 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 297c566f8e892caddbf7557060978b310f4e98d9..a140be290e8bee3f7482075710edaabd6608092f 100644 (file)
--- 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 87f4df520ed3e18db3d045da0d7c049870591a2b..76f4910724b21a67ff14db06f32e431b6d029932 100644 (file)
--- 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 081aabe16aacb09e0990806bead58143193f875c..36146a2ba9e16ded8a9b433c7030c41418b22198 100644 (file)
--- 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()