diff --git a/etc/neutron/rootwrap.d/dhcp.filters b/etc/neutron/rootwrap.d/dhcp.filters index 26e2e5664..02e091375 100644 --- a/etc/neutron/rootwrap.d/dhcp.filters +++ b/etc/neutron/rootwrap.d/dhcp.filters @@ -16,8 +16,6 @@ dnsmasq: EnvFilter, dnsmasq, root, NEUTRON_NETWORK_ID= kill_dnsmasq: KillFilter, root, /sbin/dnsmasq, -9, -HUP kill_dnsmasq_usr: KillFilter, root, /usr/sbin/dnsmasq, -9, -HUP -# dhcp-agent uses cat -cat: RegExpFilter, cat, root, cat, /proc/\d+/cmdline ovs-vsctl: CommandFilter, ovs-vsctl, root ivs-ctl: CommandFilter, ivs-ctl, root dhcp_release: CommandFilter, dhcp_release, root diff --git a/etc/neutron/rootwrap.d/lbaas-haproxy.filters b/etc/neutron/rootwrap.d/lbaas-haproxy.filters index 447560402..aaf6be399 100644 --- a/etc/neutron/rootwrap.d/lbaas-haproxy.filters +++ b/etc/neutron/rootwrap.d/lbaas-haproxy.filters @@ -14,9 +14,6 @@ haproxy: CommandFilter, haproxy, root # lbaas-agent uses kill as well, that's handled by the generic KillFilter kill_haproxy_usr: KillFilter, root, /usr/sbin/haproxy, -9, -HUP -# lbaas-agent uses cat -cat: RegExpFilter, cat, root, cat, /proc/\d+/cmdline - ovs-vsctl: CommandFilter, ovs-vsctl, root # ip_lib diff --git a/neutron/agent/linux/daemon.py b/neutron/agent/linux/daemon.py index 3df12d580..38c1bb5db 100644 --- a/neutron/agent/linux/daemon.py +++ b/neutron/agent/linux/daemon.py @@ -22,14 +22,13 @@ import os import signal import sys -from neutron.agent.linux import utils from neutron.openstack.common import log as logging LOG = logging.getLogger(__name__) class Pidfile(object): - def __init__(self, pidfile, procname, uuid=None, root_helper='sudo'): + def __init__(self, pidfile, procname, uuid=None): try: self.fd = os.open(pidfile, os.O_CREAT | os.O_RDWR) except IOError: @@ -38,7 +37,6 @@ class Pidfile(object): self.pidfile = pidfile self.procname = procname self.uuid = uuid - self.root_helper = root_helper if not not fcntl.flock(self.fd, fcntl.LOCK_EX): raise IOError(_('Unable to lock pid file')) @@ -67,12 +65,13 @@ class Pidfile(object): if not pid: return False - cmd = ['cat', '/proc/%s/cmdline' % pid] + cmdline = '/proc/%s/cmdline' % pid try: - exec_out = utils.execute(cmd, self.root_helper) + with open(cmdline, "r") as f: + exec_out = f.readline() return self.procname in exec_out and (not self.uuid or self.uuid in exec_out) - except RuntimeError: + except IOError: return False @@ -82,13 +81,12 @@ class Daemon(object): Usage: subclass the Daemon class and override the run() method """ def __init__(self, pidfile, stdin='/dev/null', stdout='/dev/null', - stderr='/dev/null', procname='python', uuid=None, - root_helper='sudo'): + stderr='/dev/null', procname='python', uuid=None): self.stdin = stdin self.stdout = stdout self.stderr = stderr self.procname = procname - self.pidfile = Pidfile(pidfile, procname, uuid, root_helper) + self.pidfile = Pidfile(pidfile, procname, uuid) def _fork(self): try: diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index a0f656b29..9e7481d7c 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -232,10 +232,11 @@ class DhcpLocalProcess(DhcpBase): if pid is None: return False - cmd = ['cat', '/proc/%s/cmdline' % pid] + cmdline = '/proc/%s/cmdline' % pid try: - return self.network.id in utils.execute(cmd, self.root_helper) - except RuntimeError: + with open(cmdline, "r") as f: + return self.network.id in f.readline() + except IOError: return False @property diff --git a/neutron/agent/linux/external_process.py b/neutron/agent/linux/external_process.py index 1748c6c4e..9c62f2f87 100644 --- a/neutron/agent/linux/external_process.py +++ b/neutron/agent/linux/external_process.py @@ -100,8 +100,9 @@ class ProcessManager(object): if pid is None: return False - cmd = ['cat', '/proc/%s/cmdline' % pid] + cmdline = '/proc/%s/cmdline' % pid try: - return self.uuid in utils.execute(cmd, self.root_helper) - except RuntimeError: + with open(cmdline, "r") as f: + return self.uuid in f.readline() + except IOError: return False diff --git a/neutron/tests/unit/test_linux_daemon.py b/neutron/tests/unit/test_linux_daemon.py index c4cf3223c..9c40c7b82 100644 --- a/neutron/tests/unit/test_linux_daemon.py +++ b/neutron/tests/unit/test_linux_daemon.py @@ -84,40 +84,43 @@ class TestPidfile(base.BaseTestCase): self.assertEqual(34, p.read()) def test_is_running(self): - with mock.patch('neutron.agent.linux.utils.execute') as execute: - execute.return_value = 'python' + with mock.patch('__builtin__.open') as mock_open: p = daemon.Pidfile('thefile', 'python') + mock_open.return_value.__enter__ = lambda s: s + mock_open.return_value.__exit__ = mock.Mock() + mock_open.return_value.readline.return_value = 'python' with mock.patch.object(p, 'read') as read: read.return_value = 34 self.assertTrue(p.is_running()) - execute.assert_called_once_with( - ['cat', '/proc/34/cmdline'], 'sudo') + mock_open.assert_called_once_with('/proc/34/cmdline', 'r') def test_is_running_uuid_true(self): - with mock.patch('neutron.agent.linux.utils.execute') as execute: - execute.return_value = 'python 1234' + with mock.patch('__builtin__.open') as mock_open: p = daemon.Pidfile('thefile', 'python', uuid='1234') + mock_open.return_value.__enter__ = lambda s: s + mock_open.return_value.__exit__ = mock.Mock() + mock_open.return_value.readline.return_value = 'python 1234' with mock.patch.object(p, 'read') as read: read.return_value = 34 self.assertTrue(p.is_running()) - execute.assert_called_once_with( - ['cat', '/proc/34/cmdline'], 'sudo') + mock_open.assert_called_once_with('/proc/34/cmdline', 'r') def test_is_running_uuid_false(self): - with mock.patch('neutron.agent.linux.utils.execute') as execute: - execute.return_value = 'python 1234' + with mock.patch('__builtin__.open') as mock_open: p = daemon.Pidfile('thefile', 'python', uuid='6789') + mock_open.return_value.__enter__ = lambda s: s + mock_open.return_value.__exit__ = mock.Mock() + mock_open.return_value.readline.return_value = 'python 1234' with mock.patch.object(p, 'read') as read: read.return_value = 34 self.assertFalse(p.is_running()) - execute.assert_called_once_with( - ['cat', '/proc/34/cmdline'], 'sudo') + mock_open.assert_called_once_with('/proc/34/cmdline', 'r') class TestDaemon(base.BaseTestCase): diff --git a/neutron/tests/unit/test_linux_dhcp.py b/neutron/tests/unit/test_linux_dhcp.py index 25a416cd2..7842acb6f 100644 --- a/neutron/tests/unit/test_linux_dhcp.py +++ b/neutron/tests/unit/test_linux_dhcp.py @@ -365,14 +365,18 @@ class TestDhcpBase(TestBase): class TestDhcpLocalProcess(TestBase): def test_active(self): - dummy_cmd_line = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' - self.execute.return_value = (dummy_cmd_line, '') - with mock.patch.object(LocalChild, 'pid') as pid: - pid.__get__ = mock.Mock(return_value=4) - lp = LocalChild(self.conf, FakeV4Network()) - self.assertTrue(lp.active) - self.execute.assert_called_once_with(['cat', '/proc/4/cmdline'], - 'sudo') + with mock.patch('__builtin__.open') as mock_open: + mock_open.return_value.__enter__ = lambda s: s + mock_open.return_value.__exit__ = mock.Mock() + mock_open.return_value.readline.return_value = \ + 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' + + with mock.patch.object(LocalChild, 'pid') as pid: + pid.__get__ = mock.Mock(return_value=4) + lp = LocalChild(self.conf, FakeV4Network()) + self.assertTrue(lp.active) + + mock_open.assert_called_once_with('/proc/4/cmdline', 'r') def test_active_none(self): dummy_cmd_line = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' @@ -383,14 +387,18 @@ class TestDhcpLocalProcess(TestBase): self.assertFalse(lp.active) def test_active_cmd_mismatch(self): - dummy_cmd_line = 'bbbbbbbb-bbbb-bbbb-aaaa-aaaaaaaaaaaa' - self.execute.return_value = (dummy_cmd_line, '') - with mock.patch.object(LocalChild, 'pid') as pid: - pid.__get__ = mock.Mock(return_value=4) - lp = LocalChild(self.conf, FakeV4Network()) - self.assertFalse(lp.active) - self.execute.assert_called_once_with(['cat', '/proc/4/cmdline'], - 'sudo') + with mock.patch('__builtin__.open') as mock_open: + mock_open.return_value.__enter__ = lambda s: s + mock_open.return_value.__exit__ = mock.Mock() + mock_open.return_value.readline.return_value = \ + 'bbbbbbbb-bbbb-bbbb-aaaa-aaaaaaaaaaaa' + + with mock.patch.object(LocalChild, 'pid') as pid: + pid.__get__ = mock.Mock(return_value=4) + lp = LocalChild(self.conf, FakeV4Network()) + self.assertFalse(lp.active) + + mock_open.assert_called_once_with('/proc/4/cmdline', 'r') def test_get_conf_file_name(self): tpl = '/dhcp/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/dev' @@ -886,24 +894,28 @@ tag:tag1,249,%s,%s""".lstrip() % (fake_v6, fake_v6_cidr, fake_v6, fake_v6_cidr, fake_v6) - exp_args = ['cat', '/proc/5/cmdline'] + with mock.patch('__builtin__.open') as mock_open: + mock_open.return_value.__enter__ = lambda s: s + mock_open.return_value.__exit__ = mock.Mock() + mock_open.return_value.readline.return_value = None - with mock.patch('os.path.isdir') as isdir: - isdir.return_value = True - with mock.patch.object(dhcp.Dnsmasq, 'pid') as pid: - pid.__get__ = mock.Mock(return_value=5) - dm = dhcp.Dnsmasq(self.conf, FakeDualNetwork(), - version=float(2.59)) + with mock.patch('os.path.isdir') as isdir: + isdir.return_value = True + with mock.patch.object(dhcp.Dnsmasq, 'pid') as pid: + pid.__get__ = mock.Mock(return_value=5) + dm = dhcp.Dnsmasq(self.conf, FakeDualNetwork(), + version=float(2.59)) - method_name = '_make_subnet_interface_ip_map' - with mock.patch.object(dhcp.Dnsmasq, method_name) as ip_map: - ip_map.return_value = {} - dm.reload_allocations() - self.assertTrue(ip_map.called) + method_name = '_make_subnet_interface_ip_map' + with mock.patch.object(dhcp.Dnsmasq, method_name) as ipmap: + ipmap.return_value = {} + dm.reload_allocations() + self.assertTrue(ipmap.called) - 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, 'sudo') + self.safe.assert_has_calls([mock.call(exp_host_name, + exp_host_data), + mock.call(exp_opt_name, exp_opt_data)]) + mock_open.assert_called_once_with('/proc/5/cmdline', 'r') def test_make_subnet_interface_ip_map(self): with mock.patch('neutron.agent.linux.ip_lib.IPDevice') as ip_dev: diff --git a/neutron/tests/unit/test_linux_external_process.py b/neutron/tests/unit/test_linux_external_process.py index 452827460..081aabe16 100644 --- a/neutron/tests/unit/test_linux_external_process.py +++ b/neutron/tests/unit/test_linux_external_process.py @@ -167,14 +167,17 @@ class TestProcessManager(base.BaseTestCase): self.assertIsNone(manager.pid) def test_active(self): - dummy_cmd_line = 'python foo --router_id=uuid' - self.execute.return_value = dummy_cmd_line - with mock.patch.object(ep.ProcessManager, 'pid') as pid: - pid.__get__ = mock.Mock(return_value=4) - manager = ep.ProcessManager(self.conf, 'uuid') - self.assertTrue(manager.active) - self.execute.assert_called_once_with(['cat', '/proc/4/cmdline'], - 'sudo') + with mock.patch('__builtin__.open') as mock_open: + mock_open.return_value.__enter__ = lambda s: s + mock_open.return_value.__exit__ = mock.Mock() + mock_open.return_value.readline.return_value = \ + 'python foo --router_id=uuid' + with mock.patch.object(ep.ProcessManager, 'pid') as pid: + pid.__get__ = mock.Mock(return_value=4) + manager = ep.ProcessManager(self.conf, 'uuid') + self.assertTrue(manager.active) + + mock_open.assert_called_once_with('/proc/4/cmdline', 'r') def test_active_none(self): dummy_cmd_line = 'python foo --router_id=uuid' @@ -185,11 +188,14 @@ class TestProcessManager(base.BaseTestCase): self.assertFalse(manager.active) def test_active_cmd_mismatch(self): - dummy_cmd_line = 'python foo --router_id=anotherid' - self.execute.return_value = dummy_cmd_line - with mock.patch.object(ep.ProcessManager, 'pid') as pid: - pid.__get__ = mock.Mock(return_value=4) - manager = ep.ProcessManager(self.conf, 'uuid') - self.assertFalse(manager.active) - self.execute.assert_called_once_with(['cat', '/proc/4/cmdline'], - 'sudo') + with mock.patch('__builtin__.open') as mock_open: + mock_open.return_value.__enter__ = lambda s: s + mock_open.return_value.__exit__ = mock.Mock() + mock_open.return_value.readline.return_value = \ + 'python foo --router_id=anotherid' + with mock.patch.object(ep.ProcessManager, 'pid') as pid: + pid.__get__ = mock.Mock(return_value=4) + manager = ep.ProcessManager(self.conf, 'uuid') + self.assertFalse(manager.active) + + mock_open.assert_called_once_with('/proc/4/cmdline', 'r')