From 3d575f8bd066ce2eb46353a49a8c6850ba9e4387 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 14 Nov 2022 05:26:52 +0100 Subject: [PATCH] Add an env variable "PROCESS_TAG" in ``ProcessManager`` Added a new environment variable "PROCESS_TAG" in ``ProcessManager``. This environment variable could be read by the process executed and is unique per process. This environment variable can be used to tag the running process; for example, a container manager can use this tag to mark the a container. This feature will be used by TripleO to identify the running containers with a unique tag. This will make the "kill" process easier; it will be needed just to find the container running with this tag. Closes-Bug: #1991000 Change-Id: I234c661720a8b1ceadb5333181890806f79dc21a --- etc/neutron/rootwrap.d/rootwrap.filters | 6 ++ neutron/agent/linux/external_process.py | 12 ++- .../unit/agent/linux/test_external_process.py | 88 +++++++++++++++++-- .../tests/unit/agent/metadata/test_driver.py | 7 +- .../unit/agent/ovn/metadata/test_driver.py | 12 ++- .../process-manager-tag-2181918518972004.yaml | 8 ++ 6 files changed, 115 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/process-manager-tag-2181918518972004.yaml diff --git a/etc/neutron/rootwrap.d/rootwrap.filters b/etc/neutron/rootwrap.d/rootwrap.filters index a2f74e60414..aafc614c3ed 100644 --- a/etc/neutron/rootwrap.d/rootwrap.filters +++ b/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 diff --git a/neutron/agent/linux/external_process.py b/neutron/agent/linux/external_process.py index 5a82f3fdc2d..f1e902eeb18 100644 --- a/neutron/agent/linux/external_process.py +++ b/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': diff --git a/neutron/tests/unit/agent/linux/test_external_process.py b/neutron/tests/unit/agent/linux/test_external_process.py index 3821a30b7b7..6da2b7f5aa9 100644 --- a/neutron/tests/unit/agent/linux/test_external_process.py +++ b/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( diff --git a/neutron/tests/unit/agent/metadata/test_driver.py b/neutron/tests/unit/agent/metadata/test_driver.py index 6fa81cd6740..fc59b7fee8a 100644 --- a/neutron/tests/unit/agent/metadata/test_driver.py +++ b/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) ]) diff --git a/neutron/tests/unit/agent/ovn/metadata/test_driver.py b/neutron/tests/unit/agent/ovn/metadata/test_driver.py index 15fb77521bd..412d5eb5af2 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_driver.py +++ b/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) ]) diff --git a/releasenotes/notes/process-manager-tag-2181918518972004.yaml b/releasenotes/notes/process-manager-tag-2181918518972004.yaml new file mode 100644 index 00000000000..5dc93b893f8 --- /dev/null +++ b/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.