x/vmware-nsx
Code Issues Proposed changes

netns: ip netns exec <name> kill doesn't make sense

Browse Source 
It seems confusing netns with pidns.
Although 'ip netns exec' doesn't make sense,
'ip netns exec <netns> kill <pid>' itself success as expected.
But as side effects, dentry of /proc/<pid>/ns/net becomes young,
which increases the possibility to fail to delete netns. That's not good.

Fixes: Bug #1158589
Change-Id: I9aa717ccb86d8bf00bb1e707d39bfb65d043532b
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
This commit is contained in:
Isaku Yamahata 2013-03-19 06:56:15 +09:00
parent 7f814639ef
commit 4d72093de5
6 changed files with 27 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
quantum/agent/linux/dhcp.py
View File

@ -127,12 +127,7 @@ class DhcpLocalProcess(DhcpBase):
if self.active:
cmd = ['kill', '-9', pid]
if self.namespace:
ip_wrapper = ip_lib.IPWrapper(self.root_helper, self.namespace)
ip_wrapper.netns.execute(cmd)
else:
utils.execute(cmd, self.root_helper)
utils.execute(cmd, self.root_helper)
if not retain_port:
self.device_delegate.destroy(self.network, self.interface_name)
@ -307,12 +302,7 @@ class Dnsmasq(DhcpLocalProcess):
self._output_hosts_file()
self._output_opts_file()
cmd = ['kill', '-HUP', self.pid]
if self.namespace:
ip_wrapper = ip_lib.IPWrapper(self.root_helper, self.namespace)
ip_wrapper.netns.execute(cmd)
else:
utils.execute(cmd, self.root_helper)
utils.execute(cmd, self.root_helper)
LOG.debug(_('Reloading allocations for network: %s'), self.network.id)
def _output_hosts_file(self):

7
quantum/agent/linux/external_process.py
View File

@ -62,12 +62,7 @@ class ProcessManager(object):
if self.active:
cmd = ['kill', '-9', pid]
if self.namespace:
ip_wrapper = ip_lib.IPWrapper(self.root_helper, self.namespace)
ip_wrapper.netns.execute(cmd)
else:
utils.execute(cmd, self.root_helper)
utils.execute(cmd, self.root_helper)
elif pid:
LOG.debug(_('Process for %(uuid)s pid %(pid)d is stale, ignoring '
'command'), {'uuid': self.uuid, 'pid': pid})

9
quantum/plugins/services/agent_loadbalancer/drivers/haproxy/namespace_driver.py
View File

@ -22,6 +22,7 @@ import socket
import netaddr
from quantum.agent.linux import ip_lib
from quantum.agent.linux import utils
from quantum.common import exceptions
from quantum.openstack.common import log as logging
from quantum.plugins.services.agent_loadbalancer.drivers.haproxy import (
@ -79,7 +80,7 @@ class HaproxyNSDriver(object):
sock_path = self._get_state_file_path(pool_id, 'sock')
# kill the process
kill_pids_in_file(ns, pid_path)
kill_pids_in_file(self.root_helper, pid_path)
# unplug the ports
if pool_id in self.pool_to_port_id:
@ -199,15 +200,13 @@ def get_ns_name(namespace_id):
return NS_PREFIX + namespace_id
def kill_pids_in_file(namespace_wrapper, pid_path):
def kill_pids_in_file(root_helper, pid_path):
if os.path.exists(pid_path):
with open(pid_path, 'r') as pids:
for pid in pids:
pid = pid.strip()
try:
namespace_wrapper.netns.execute(
['kill', '-9', pid.strip()]
)
utils.execute(['kill', '-9', pid], root_helper)
except RuntimeError:
LOG.exception(
_('Unable to kill haproxy process: %s'),

21
quantum/tests/unit/services/agent_loadbalancer/driver/haproxy/test_namespace_driver.py
View File

@ -101,7 +101,7 @@ class TestHaproxyNSDriver(base.BaseTestCase):
self.driver.destroy('pool_id')
kill.assert_called_once_with(ip_wrap(), '/pool/pid')
kill.assert_called_once_with('sudo', '/pool/pid')
unplug.assert_called_once_with('qlbaas-pool_id', 'port_id')
isdir.called_once_with('/pool')
rmtree.assert_called_once_with('/pool')
@ -222,25 +222,26 @@ class TestHaproxyNSDriver(base.BaseTestCase):
def test_kill_pids_in_file(self):
with contextlib.nested(
mock.patch('os.path.exists'),
mock.patch('__builtin__.open')
) as (path_exists, mock_open):
mock.patch('os.path.exists'),
mock.patch('__builtin__.open'),
mock.patch('quantum.agent.linux.utils.execute')
) as (path_exists, mock_open, mock_execute):
file_mock = mock.MagicMock()
mock_open.return_value = file_mock
file_mock.__enter__.return_value = file_mock
file_mock.__iter__.return_value = iter(['123'])
ns_wrapper = mock.Mock()
path_exists.return_value = False
namespace_driver.kill_pids_in_file(ns_wrapper, 'test_path')
namespace_driver.kill_pids_in_file('sudo', 'test_path')
path_exists.assert_called_once_with('test_path')
self.assertFalse(mock_open.called)
self.assertFalse(mock_execute.called)
path_exists.return_value = True
ns_wrapper.netns.execute.side_effect = RuntimeError
namespace_driver.kill_pids_in_file(ns_wrapper, 'test_path')
ns_wrapper.netns.execute.assert_called_once_with(['kill', '-9',
'123'])
mock_execute.side_effect = RuntimeError
namespace_driver.kill_pids_in_file('sudo', 'test_path')
mock_execute.assert_called_once_with(
['kill', '-9', '123'], 'sudo')
def test_get_state_file_path(self):
with mock.patch('os.makedirs') as mkdir:

15
quantum/tests/unit/test_linux_dhcp.py
View File

@ -328,9 +328,8 @@ class TestDhcpLocalProcess(TestBase):
lp.disable(retain_port=True)
self.assertFalse(delegate.called)
exp_args = ['ip', 'netns', 'exec', 'qdhcp-ns', 'kill', '-9', 5]
self.execute.assert_called_once_with(exp_args, root_helper='sudo',
check_exit_code=True)
exp_args = ['kill', '-9', 5]
self.execute.assert_called_once_with(exp_args, 'sudo')
def test_disable(self):
attrs_to_mock = dict([(a, mock.DEFAULT) for a in
@ -346,9 +345,8 @@ class TestDhcpLocalProcess(TestBase):
lp.disable()
delegate.assert_has_calls([mock.call.destroy(network, 'tap0')])
exp_args = ['ip', 'netns', 'exec', 'qdhcp-ns', 'kill', '-9', 5]
self.execute.assert_called_once_with(exp_args, root_helper='sudo',
check_exit_code=True)
exp_args = ['kill', '-9', 5]
self.execute.assert_called_once_with(exp_args, 'sudo')
def test_pid(self):
with mock.patch('__builtin__.open') as mock_open:
@ -538,7 +536,7 @@ tag:tag1,option:classless-static-route,%s,%s""".lstrip() % (fake_v6,
fake_v6_cidr,
fake_v6)
exp_args = ['ip', 'netns', 'exec', 'qdhcp-ns', 'kill', '-HUP', 5]
exp_args = ['kill', '-HUP', 5]
with mock.patch('os.path.isdir') as isdir:
isdir.return_value = True
@ -555,8 +553,7 @@ tag:tag1,option:classless-static-route,%s,%s""".lstrip() % (fake_v6,
self.safe.assert_has_calls([mock.call(exp_host_name, exp_host_data),
mock.call(exp_opt_name, exp_opt_data)])
self.execute.assert_called_once_with(exp_args, root_helper='sudo',
check_exit_code=True)
self.execute.assert_called_once_with(exp_args, 'sudo')
def test_make_subnet_interface_ip_map(self):
with mock.patch('quantum.agent.linux.ip_lib.IPDevice') as ip_dev:

8
quantum/tests/unit/test_linux_external_process.py
View File

@ -95,12 +95,10 @@ class TestProcessManager(base.BaseTestCase):
manager = ep.ProcessManager(self.conf, 'uuid', namespace='ns')
with mock.patch.object(ep, 'ip_lib') as ip_lib:
with mock.patch.object(ep, 'utils') as utils:
manager.disable()
ip_lib.assert_has_calls([
mock.call.IPWrapper('sudo', 'ns'),
mock.call.IPWrapper().netns.execute(['kill', '-9', 4])]
)
utils.assert_has_calls(
mock.call.execute(['kill', '-9', 4], 'sudo'))
def test_disable_not_active(self):
with mock.patch.object(ep.ProcessManager, 'pid') as pid: