Don't return null-byte separated string from ExternalProcess.cmdline()

Instead construct the string using psutil.Process.cmdline() method that
will parse the data from kernel for us and will return a proper
space-separated command line (plus will provide some more safety
guarantees, e.g. against zombies).

This change also introduces a new psutil dependency for neutron. Before
that, the library was used in functional tests only, and with a minimal
version number that is not compatible with global-requirements.txt or
.cmdline(). Moved the requirement into runtime requirements.

Related-Bug: #1672921
Change-Id: I780ea10bd1870c86c1a048071863843bc2993958
This commit is contained in:
Ihar Hrachyshka 2017-03-08 05:30:04 +00:00
parent 3d8fd54d7f
commit 9c7ca5cc02
4 changed files with 19 additions and 23 deletions

View File

@ -21,6 +21,7 @@ from oslo_concurrency import lockutils
from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import fileutils
import psutil
import six
from neutron._i18n import _, _LW, _LE
@ -146,12 +147,9 @@ class ProcessManager(MonitoredProcess):
pid = self.pid
if not pid:
return
cmdline = '/proc/%s/cmdline' % pid
try:
with open(cmdline, "r") as f:
return f.readline()
except IOError:
return ' '.join(psutil.Process(pid).cmdline())
except (psutil.NoSuchProcess, psutil.AccessDenied):
return

View File

@ -4,5 +4,4 @@
# of appearance. Changing the order has an impact on the overall integration
# process, which may cause wedges in the gate later.
psutil>=1.1.1,<2.0.0
psycopg2

View File

@ -16,6 +16,7 @@ import mock
import os.path
from oslo_utils import fileutils
import psutil
from neutron.agent.linux import external_process as ep
from neutron.tests import base
@ -285,22 +286,19 @@ class TestProcessManager(base.BaseTestCase):
self.assertFalse(manager.active)
def test_cmdline(self):
mock_open = self.useFixture(
tools.OpenFixture('/proc/4/cmdline', TEST_CMDLINE % 'uuid')
).mock_open
with mock.patch.object(ep.ProcessManager, 'pid') as pid:
pid.__get__ = mock.Mock(return_value=4)
manager = ep.ProcessManager(self.conf, 'uuid')
self.assertEqual(TEST_CMDLINE % 'uuid', manager.cmdline)
mock_open.assert_called_once_with('/proc/4/cmdline', 'r')
with mock.patch.object(psutil, 'Process') as proc:
proc().cmdline.return_value = (TEST_CMDLINE % 'uuid').split(' ')
with mock.patch.object(ep.ProcessManager, 'pid') as pid:
pid.__get__ = mock.Mock(return_value=4)
manager = ep.ProcessManager(self.conf, 'uuid')
self.assertEqual(TEST_CMDLINE % 'uuid', manager.cmdline)
proc().cmdline.assert_called_once_with()
def test_cmdline_none(self):
mock_open = self.useFixture(
tools.OpenFixture('/proc/4/cmdline', TEST_CMDLINE % 'uuid')
).mock_open
mock_open.side_effect = IOError()
with mock.patch.object(ep.ProcessManager, 'pid') as pid:
pid.__get__ = mock.Mock(return_value=4)
manager = ep.ProcessManager(self.conf, 'uuid')
self.assertIsNone(manager.cmdline)
mock_open.assert_called_once_with('/proc/4/cmdline', 'r')
with mock.patch.object(psutil, 'Process') as proc:
proc.side_effect = psutil.NoSuchProcess(4)
with mock.patch.object(ep.ProcessManager, 'pid') as pid:
pid.__get__ = mock.Mock(return_value=4)
manager = ep.ProcessManager(self.conf, 'uuid')
self.assertIsNone(manager.cmdline)
proc.assert_called_once_with(4)

View File

@ -44,6 +44,7 @@ oslo.utils>=3.20.0 # Apache-2.0
oslo.versionedobjects>=1.17.0 # Apache-2.0
osprofiler>=1.4.0 # Apache-2.0
ovs>=2.6.1 # Apache-2.0
psutil>=3.2.2 # BSD
pyroute2>=0.4.12 # Apache-2.0 (+ dual licensed GPL2)
weakrefmethod>=1.0.2;python_version=='2.7' # PSF