Merge "Add an env variable "PROCESS_TAG" in ``ProcessManager``"

etc/neutron/rootwrap.d/rootwrap.filters

@@ -40,17 +40,23 @@ ip_exec: IpNetnsExecFilter, ip, root
 
 # METADATA PROXY
 haproxy: RegExpFilter, haproxy, root, haproxy, -f, .*
+haproxy_env: EnvFilter, env, root, PROCESS_TAG=, haproxy, -f, .*
 
 # DHCP
 dnsmasq: CommandFilter, dnsmasq, root
+dnsmasq_env: EnvFilter, env, root, PROCESS_TAG=, dnsmasq
 
 # DIBBLER
 dibbler-client: CommandFilter, dibbler-client, root
+dibbler-client_env: EnvFilter, env, root, PROCESS_TAG=, dibbler-client
 
 # L3
 radvd: CommandFilter, radvd, root
+radvd_env: EnvFilter, env, root, PROCESS_TAG=, radvd
 keepalived: CommandFilter, keepalived, root
+keepalived_env: EnvFilter, env, root, PROCESS_TAG=, keepalived
 keepalived_state_change: CommandFilter, neutron-keepalived-state-change, root
+keepalived_state_change_env: EnvFilter, env, root, PROCESS_TAG=, neutron-keepalived-state-change
 
 # OPEN VSWITCH
 ovs-ofctl: CommandFilter, ovs-ofctl, root

neutron/agent/linux/external_process.py

@@ -30,6 +30,8 @@ from neutron.conf.agent import common as agent_cfg
 
 
 LOG = logging.getLogger(__name__)
+PROCESS_TAG = 'PROCESS_TAG'
+DEFAULT_SERVICE_NAME = 'default-service'
 
 
 agent_cfg.register_external_process_opts()
@@ -61,7 +63,6 @@ class ProcessManager(MonitoredProcess):
         self.uuid = uuid
         self.namespace = namespace
         self.default_cmd_callback = default_cmd_callback
-        self.cmd_addl_env = cmd_addl_env
         self.pids_path = pids_path or self.conf.external_pids
         self.pid_file = pid_file
         self.run_as_root = run_as_root or self.namespace is not None
@@ -73,7 +74,11 @@ class ProcessManager(MonitoredProcess):
             self.service = service
         else:
             self.service_pid_fname = 'pid'
-            self.service = 'default-service'
+            self.service = DEFAULT_SERVICE_NAME
+
+        process_tag = '%s-%s' % (self.service, self.uuid)
+        self.cmd_addl_env = cmd_addl_env or {}
+        self.cmd_addl_env[PROCESS_TAG] = process_tag
 
         fileutils.ensure_tree(os.path.dirname(self.get_pid_file_name()),
                               mode=0o755)
@@ -110,7 +115,8 @@ class ProcessManager(MonitoredProcess):
                                          privsep_exec=True)
             else:
                 cmd = self.get_kill_cmd(sig, pid)
-                utils.execute(cmd, run_as_root=self.run_as_root,
+                utils.execute(cmd, addl_env=self.cmd_addl_env,
+                              run_as_root=self.run_as_root,
                               privsep_exec=True)
                 # In the case of shutting down, remove the pid file
                 if sig == '9':

neutron/tests/unit/agent/linux/test_external_process.py

@@ -13,11 +13,13 @@
 #    under the License.
 
 import os.path
+import tempfile
 from unittest import mock
 
 from neutron_lib import fixture as lib_fixtures
 from oslo_config import cfg
 from oslo_utils import fileutils
+from oslo_utils import uuidutils
 import psutil
 
 from neutron.agent.linux import external_process as ep
@@ -29,6 +31,15 @@ TEST_UUID = 'test-uuid'
 TEST_SERVICE = 'testsvc'
 TEST_PID = 1234
 TEST_CMDLINE = 'python foo --router_id=%s'
+SCRIPT = """#!/bin/bash
+output_file=$1
+if [ -z "${PROCESS_TAG}" ] ; then
+    echo "Variable PROCESS_TAG not set" > $output_file
+else
+    echo "Variable PROCESS_TAG set: $PROCESS_TAG" > $output_file
+fi
+"""
+DEFAULT_ENVVAR = ep.PROCESS_TAG + '=' + ep.DEFAULT_SERVICE_NAME
 
 
 class BaseTestProcessMonitor(base.BaseTestCase):
@@ -124,7 +135,8 @@ class TestProcessManager(base.BaseTestCase):
 
     def test_enable_no_namespace(self):
         callback = mock.Mock()
-        callback.return_value = ['the', 'cmd']
+        cmd = ['the', 'cmd']
+        callback.return_value = cmd
 
         with mock.patch.object(ep.ProcessManager, 'get_pid_file_name') as name:
             name.return_value = 'pidfile'
@@ -134,7 +146,8 @@ class TestProcessManager(base.BaseTestCase):
                 manager = ep.ProcessManager(self.conf, 'uuid')
                 manager.enable(callback)
                 callback.assert_called_once_with('pidfile')
-                self.execute.assert_called_once_with(['the', 'cmd'],
+                cmd = ['env', DEFAULT_ENVVAR + '-uuid'] + cmd
+                self.execute.assert_called_once_with(cmd,
                                                      check_exit_code=True,
                                                      extra_ok_codes=None,
                                                      run_as_root=False,
@@ -154,10 +167,11 @@ class TestProcessManager(base.BaseTestCase):
                 with mock.patch.object(ep, 'ip_lib') as ip_lib:
                     manager.enable(callback)
                     callback.assert_called_once_with('pidfile')
+                    env = {ep.PROCESS_TAG: ep.DEFAULT_SERVICE_NAME + '-uuid'}
                     ip_lib.assert_has_calls([
                         mock.call.IPWrapper(namespace='ns'),
                         mock.call.IPWrapper().netns.execute(
-                            ['the', 'cmd'], addl_env=None, run_as_root=True)])
+                            ['the', 'cmd'], addl_env=env, run_as_root=True)])
 
     def test_enable_with_namespace_process_active(self):
         callback = mock.Mock()
@@ -189,6 +203,56 @@ class TestProcessManager(base.BaseTestCase):
             except common_utils.WaitTimeout:
                 self.fail('ProcessManager.enable() raised WaitTimeout')
 
+    def _create_env_var_testing_environment(self, script_content, _create_cmd):
+        with tempfile.NamedTemporaryFile('w+', dir='/tmp/',
+                                         delete=False) as script:
+            script.write(script_content)
+        output = tempfile.NamedTemporaryFile('w+', dir='/tmp/', delete=False)
+        os.chmod(script.name, 0o777)
+        service_name = 'my_new_service'
+        uuid = uuidutils.generate_uuid()
+        pm = ep.ProcessManager(self.conf, uuid, service=service_name,
+                               default_cmd_callback=_create_cmd)
+        return script, output, service_name, uuid, pm
+
+    def test_enable_check_process_id_env_var(self):
+        def _create_cmd(*args):
+            return [script.name, output.name]
+
+        self.execute_p.stop()
+        script, output, service_name, uuid, pm = (
+            self._create_env_var_testing_environment(SCRIPT, _create_cmd))
+        with mock.patch.object(ep.ProcessManager, 'active') as active:
+            active.__get__ = mock.Mock(return_value=False)
+            pm.enable()
+
+        with open(output.name, 'r') as f:
+            ret_value = f.readline().strip()
+        expected_value = ('Variable PROCESS_TAG set: %s-%s' %
+                          (service_name, uuid))
+        self.assertEqual(expected_value, ret_value)
+
+    def test_disable_check_process_id_env_var(self):
+        def _create_cmd(*args):
+            return [script.name, output.name]
+
+        self.execute_p.stop()
+        script, output, service_name, uuid, pm = (
+            self._create_env_var_testing_environment(SCRIPT, _create_cmd))
+        with mock.patch.object(ep.ProcessManager, 'active') as active, \
+                mock.patch.object(pm, 'get_kill_cmd') as mock_kill_cmd:
+            active.__get__ = mock.Mock(return_value=True)
+            # NOTE(ralonsoh): the script we are using for testing does not
+            # expect to receive the SIG number as the first argument.
+            mock_kill_cmd.return_value = [script.name, output.name]
+            pm.disable(sig='15')
+
+        with open(output.name, 'r') as f:
+            ret_value = f.readline().strip()
+        expected_value = ('Variable PROCESS_TAG set: %s-%s' %
+                          (service_name, uuid))
+        self.assertEqual(expected_value, ret_value)
+
     def test_reload_cfg_without_custom_reload_callback(self):
         with mock.patch.object(ep.ProcessManager, 'disable') as disable:
             manager = ep.ProcessManager(self.conf, 'uuid', namespace='ns')
@@ -216,6 +280,7 @@ class TestProcessManager(base.BaseTestCase):
                     custom_reload_callback=reload_callback)
                 manager.disable(
                     get_stop_command=manager.custom_reload_callback)
+                cmd = ['env', DEFAULT_ENVVAR + '-uuid'] + cmd
                 self.assertIn(cmd, self.execute.call_args[0])
 
     def test_disable_no_namespace(self):
@@ -227,8 +292,10 @@ class TestProcessManager(base.BaseTestCase):
 
                 with mock.patch.object(ep, 'utils') as utils:
                     manager.disable()
+                    env = {ep.PROCESS_TAG: ep.DEFAULT_SERVICE_NAME + '-uuid'}
                     utils.assert_has_calls([
                         mock.call.execute(['kill', '-9', 4],
+                                          addl_env=env,
                                           run_as_root=False,
                                           privsep_exec=True)])
 
@@ -242,10 +309,11 @@ class TestProcessManager(base.BaseTestCase):
 
                 with mock.patch.object(ep, 'utils') as utils:
                     manager.disable()
+                    env = {ep.PROCESS_TAG: ep.DEFAULT_SERVICE_NAME + '-uuid'}
                     utils.assert_has_calls([
-                        mock.call.execute(['kill', '-9', 4],
-                                          run_as_root=True,
-                                          privsep_exec=True)])
+                        mock.call.execute(
+                            ['kill', '-9', 4], addl_env=env, run_as_root=True,
+                            privsep_exec=True)])
 
     def test_disable_not_active(self):
         with mock.patch.object(ep.ProcessManager, 'pid') as pid:
@@ -280,16 +348,18 @@ class TestProcessManager(base.BaseTestCase):
             pid.__get__ = mock.Mock(return_value=4)
             with mock.patch.object(ep.ProcessManager, 'active') as active:
                 active.__get__ = mock.Mock(return_value=True)
+                service_name = 'test-service'
                 manager = ep.ProcessManager(
                     self.conf, 'uuid', namespace=namespace,
-                    service='test-service')
+                    service=service_name)
                 with mock.patch.object(ep, 'utils') as utils, \
                         mock.patch.object(os.path, 'isfile',
                                           return_value=kill_script_exists):
                     manager.disable()
+                    addl_env = {ep.PROCESS_TAG: service_name + '-uuid'}
                     utils.execute.assert_called_with(
-                        expected_cmd, run_as_root=bool(namespace),
-                        privsep_exec=True)
+                        expected_cmd, addl_env=addl_env,
+                        run_as_root=bool(namespace), privsep_exec=True)
 
     def test_disable_custom_kill_script_no_namespace(self):
         self._test_disable_custom_kill_script(

neutron/tests/unit/agent/metadata/test_driver.py

@@ -24,6 +24,7 @@ from oslo_utils import uuidutils
 
 from neutron.agent.l3 import agent as l3_agent
 from neutron.agent.l3 import router_info
+from neutron.agent.linux import external_process as ep
 from neutron.agent.linux import iptables_manager
 from neutron.agent.linux import utils as linux_utils
 from neutron.agent.metadata import driver as metadata_driver
@@ -142,6 +143,7 @@ class TestMetadataDriverProcess(base.BaseTestCase):
     def test_spawn_metadata_proxy(self):
         router_id = _uuid()
         router_ns = 'qrouter-%s' % router_id
+        service_name = 'haproxy'
         ip_class_path = 'neutron.agent.linux.ip_lib.IPWrapper'
 
         cfg.CONF.set_override('metadata_proxy_user', self.EUNAME)
@@ -181,7 +183,7 @@ class TestMetadataDriverProcess(base.BaseTestCase):
                 router_id=router_id)
 
             netns_execute_args = [
-                'haproxy',
+                service_name,
                 '-f', cfg_file]
 
             log_tag = ("haproxy-" + metadata_driver.METADATA_SERVICE_NAME +
@@ -205,9 +207,10 @@ class TestMetadataDriverProcess(base.BaseTestCase):
                 mock.call().write(cfg_contents)],
                                        any_order=True)
 
+            env = {ep.PROCESS_TAG: service_name + '-' + router_id}
             ip_mock.assert_has_calls([
                 mock.call(namespace=router_ns),
-                mock.call().netns.execute(netns_execute_args, addl_env=None,
+                mock.call().netns.execute(netns_execute_args, addl_env=env,
                                           run_as_root=True)
             ])
 

neutron/tests/unit/agent/ovn/metadata/test_driver.py

@@ -20,6 +20,7 @@ from neutron_lib import fixture as lib_fixtures
 from oslo_config import cfg
 from oslo_utils import uuidutils
 
+from neutron.agent.linux import external_process as ep
 from neutron.agent.ovn.metadata import agent as metadata_agent
 from neutron.agent.ovn.metadata import driver as metadata_driver
 from neutron.common import metadata as comm_meta
@@ -52,6 +53,7 @@ class TestMetadataDriverProcess(base.BaseTestCase):
         datapath_id = _uuid()
         metadata_ns = metadata_agent.NS_PREFIX + datapath_id
         ip_class_path = 'neutron.agent.linux.ip_lib.IPWrapper'
+        service_name = 'haproxy'
 
         cfg.CONF.set_override('metadata_proxy_user', self.EUNAME)
         cfg.CONF.set_override('metadata_proxy_group', self.EGNAME)
@@ -84,11 +86,12 @@ class TestMetadataDriverProcess(base.BaseTestCase):
                 network_id=datapath_id)
 
             netns_execute_args = [
-                'haproxy',
+                service_name,
                 '-f', cfg_file]
 
-            log_tag = 'haproxy-{}-{}'.format(
-                metadata_driver.METADATA_SERVICE_NAME, datapath_id)
+            log_tag = '{}-{}-{}'.format(
+                service_name, metadata_driver.METADATA_SERVICE_NAME,
+                datapath_id)
 
             cfg_contents = metadata_driver._HAPROXY_CONFIG_TEMPLATE % {
                 'user': self.EUNAME,
@@ -107,9 +110,10 @@ class TestMetadataDriverProcess(base.BaseTestCase):
                 mock.call().write(cfg_contents)],
                 any_order=True)
 
+            env = {ep.PROCESS_TAG: service_name + '-' + datapath_id}
             ip_mock.assert_has_calls([
                 mock.call(namespace=metadata_ns),
-                mock.call().netns.execute(netns_execute_args, addl_env=None,
+                mock.call().netns.execute(netns_execute_args, addl_env=env,
                                           run_as_root=True)
             ])
 

releasenotes/notes/process-manager-tag-2181918518972004.yaml

@@ -0,0 +1,8 @@
+---
+other:
+  - |
+    The ``ProcessManager`` class will now, by default, add an environment
+    variable when starting a new process. This default tag is named
+    "PROCESS_TAG" and will contain a unique identifier for this specific
+    process. It could be used, for example, by TripleO to univocally tag
+    any new container spawned and find it using the same tag.