From 2c1e09bfd5a275c74213d200a7455bbf46893a1a Mon Sep 17 00:00:00 2001 From: Doug Wiegley Date: Thu, 14 Feb 2019 20:24:02 -0700 Subject: [PATCH] Add setproctitle support to the workers module - Set the process name of child neutron-servers to be something more readily identifiable than today. - Enable by default for all users of the workers module, neutron will have a conf setting for its workers. Matching neutron change: https://review.openstack.org/637019 Partial-Bug: #1816485 Depends-On: https://review.openstack.org/637024 Change-Id: Ic6eca08f2ccacb3f8bf741c47a45e88cd3877b29 --- lower-constraints.txt | 1 + neutron_lib/tests/unit/test_worker.py | 84 +++++++++++++++++++ neutron_lib/worker.py | 36 +++++++- ...roctitle_for_workers-e8805fcaf34026ab.yaml | 27 ++++++ requirements.txt | 1 + 5 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/setproctitle_for_workers-e8805fcaf34026ab.yaml diff --git a/lower-constraints.txt b/lower-constraints.txt index af92a55bb..eb2a03929 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -78,6 +78,7 @@ requests==2.14.2 requestsexceptions==1.2.0 rfc3986==0.3.1 Routes==2.3.1 +setproctitle==1.1.10 six==1.10.0 snowballstemmer==1.2.1 Sphinx==1.6.2 diff --git a/neutron_lib/tests/unit/test_worker.py b/neutron_lib/tests/unit/test_worker.py index 94af5fb59..bb8981e13 100644 --- a/neutron_lib/tests/unit/test_worker.py +++ b/neutron_lib/tests/unit/test_worker.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +import six + import mock from neutron_lib.callbacks import events @@ -34,6 +36,14 @@ class _BaseWorker(worker.BaseWorker): pass +# Same as _BaseWorker, but looks like a process launch instead of eventlet +class _ProcWorker(_BaseWorker): + + def __init__(self, worker_process_count=1, set_proctitle='on'): + super(_ProcWorker, self).__init__(worker_process_count, set_proctitle) + self._my_pid = -1 # make it appear to be a separate process + + class TestBaseWorker(base.BaseTestCase): def setUp(self): @@ -51,3 +61,77 @@ class TestBaseWorker(base.BaseTestCase): base_worker.start() self._reg.notify.assert_called_once_with( resources.PROCESS, events.AFTER_INIT, base_worker.start) + + # Forked workers, should call setproctitle + + def test_proctitle_default(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker().start() + six.assertRegex(self, spt.call_args[0][0], + '^neutron-server: _ProcWorker \(.*python.*\)$') + + def test_proctitle_custom_desc(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker().start(desc="fancy title") + six.assertRegex(self, spt.call_args[0][0], + '^neutron-server: fancy title \(.*python.*\)$') + + def test_proctitle_custom_name(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker().start(name="tardis") + six.assertRegex(self, spt.call_args[0][0], + '^tardis: _ProcWorker \(.*python.*\)$') + + def test_proctitle_empty(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker().start(desc="") + six.assertRegex(self, spt.call_args[0][0], + '^neutron-server: _ProcWorker \(.*python.*\)$') + + def test_proctitle_nonstring(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker().start(desc=2) + six.assertRegex(self, spt.call_args[0][0], + '^neutron-server: 2 \(.*python.*\)$') + + def test_proctitle_both_empty(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker().start(name="", desc="") + six.assertRegex(self, spt.call_args[0][0], + '^: _ProcWorker \(.*python.*\)$') + + def test_proctitle_name_none(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker().start(name=None) + six.assertRegex(self, spt.call_args[0][0], + '^None: _ProcWorker \(.*python.*\)$') + + # Forked, but proctitle disabled + + def test_proctitle_off(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker(set_proctitle='off').start() + self.assertIsNone(spt.call_args) + + # Eventlet style worker, should never call setproctitle + + def test_proctitle_same_process(self): + with mock.patch('setproctitle.setproctitle') as spt: + _BaseWorker().start() + self.assertIsNone(spt.call_args) + + def test_setproctitle_on(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker(set_proctitle='on').start(name="foo", desc="bar") + six.assertRegex(self, spt.call_args[0][0], + '^foo: bar \(.*python.*\)$') + + def test_setproctitle_off(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker(set_proctitle='off').start(name="foo", desc="bar") + self.assertIsNone(spt.call_args) + + def test_setproctitle_brief(self): + with mock.patch('setproctitle.setproctitle') as spt: + _ProcWorker(set_proctitle='brief').start(name="foo", desc="bar") + self.assertEqual(spt.call_args[0][0], 'foo: bar') diff --git a/neutron_lib/worker.py b/neutron_lib/worker.py index 1f4975f0d..22cdb4f7b 100644 --- a/neutron_lib/worker.py +++ b/neutron_lib/worker.py @@ -12,7 +12,10 @@ # License for the specific language governing permissions and limitations # under the License. +import os + from oslo_service import service +import setproctitle from neutron_lib.callbacks import events from neutron_lib.callbacks import registry @@ -45,15 +48,24 @@ class BaseWorker(service.ServiceBase): # default class value for case when super().__init__ is not called _default_process_count = 1 - def __init__(self, worker_process_count=_default_process_count): + def __init__(self, worker_process_count=_default_process_count, + set_proctitle='on'): """Initialize a worker instance. :param worker_process_count: Defines how many processes to spawn for worker: 0 - spawn 1 new worker thread, 1..N - spawn N new worker processes + set_proctitle: + 'off' - do not change process title + 'on' - set process title to descriptive string and parent + 'brief' - set process title to descriptive string """ self._worker_process_count = worker_process_count + self._my_pid = os.getpid() + self._set_proctitle = set_proctitle + if set_proctitle == 'on': + self._parent_proctitle = setproctitle.getproctitle() @property def worker_process_count(self): @@ -63,13 +75,33 @@ class BaseWorker(service.ServiceBase): """ return self._worker_process_count - def start(self): + def setproctitle(self, name="neutron-server", desc=None): + if self._set_proctitle == "off" or os.getpid() == self._my_pid: + return + + if not desc: + desc = self.__class__.__name__ + + proctitle = "%s: %s" % (name, desc) + if self._set_proctitle == "on": + proctitle += " (%s)" % self._parent_proctitle + + setproctitle.setproctitle(proctitle) + + def start(self, name="neutron-server", desc=None): """Start the worker. If worker_process_count is greater than 0, a callback notification is sent. Subclasses should call this method before doing their own start() work. + + Automatically sets the process title to indicate that this is a + child worker, customizable via the name and desc arguments. + :returns: None """ + + # If we are a child process, set our proctitle to something useful + self.setproctitle(name, desc) if self.worker_process_count > 0: registry.notify(resources.PROCESS, events.AFTER_INIT, self.start) diff --git a/releasenotes/notes/setproctitle_for_workers-e8805fcaf34026ab.yaml b/releasenotes/notes/setproctitle_for_workers-e8805fcaf34026ab.yaml new file mode 100644 index 000000000..18c7877b0 --- /dev/null +++ b/releasenotes/notes/setproctitle_for_workers-e8805fcaf34026ab.yaml @@ -0,0 +1,27 @@ +features: + - | + ``neutron_lib.worker.BaseWorker`` will now set the process title + on process start, if it is a new process. By default, the name + will be "neutron-server", and the description will be the name + of the worker class, followed by the original process title. + Both fields are customizable via the ``name`` and ``desc`` + arguments to ``BaseWorker.start()``, and the change + can be disabled via the ``set_proctitle`` argument to the + ``__init__`` function. ``neutron.conf`` will have a setting + for disabling this functionality for all in-tree workers, but + by default, all out of tree plugin workers will set their name + at fork time. Available settings are 'on' (described above, and + the default), 'off' (same as today), or 'brief', which settings + the process name to just name and description. 'brief' is probably + most useful/simple for deployers, but 'on' is the default in order + to prevent as many script related breakages as possible. +upgrade: + - Any plugin which forks worker processes from neutron-server will + have its proctitle set to "neutron-server" plus a classname in ps + output. Any tool used for monitoring/maintenance that watches + the process table should be modified to only look for the string + ``neutron-server``. On the plus side, it will now be possible + to distinguish which process belongs to which plugin, based on + the new naming. Note that the original process string is still + in the proctitle, so as long as the scripting is not looking for + a perfect string match, it should continue to work. diff --git a/requirements.txt b/requirements.txt index 8e09ff9e6..f581e7a6e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,6 +22,7 @@ oslo.service!=1.28.1,>=1.24.0 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0 oslo.versionedobjects>=1.31.2 # Apache-2.0 osprofiler>=1.4.0 # Apache-2.0 +setproctitle>=1.1.10 # BSD WebOb>=1.7.1 # MIT weakrefmethod>=1.0.2;python_version=='2.7' # PSF os-traits>=0.9.0 # Apache-2.0