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
This commit is contained in:
Rodolfo Alonso Hernandez 2022-11-14 05:26:52 +01:00
parent 2979cd17fa
commit 3d575f8bd0
6 changed files with 115 additions and 18 deletions

View File

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

View File

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

View File

@ -13,11 +13,13 @@
# under the License. # under the License.
import os.path import os.path
import tempfile
from unittest import mock from unittest import mock
from neutron_lib import fixture as lib_fixtures from neutron_lib import fixture as lib_fixtures
from oslo_config import cfg from oslo_config import cfg
from oslo_utils import fileutils from oslo_utils import fileutils
from oslo_utils import uuidutils
import psutil import psutil
from neutron.agent.linux import external_process as ep from neutron.agent.linux import external_process as ep
@ -29,6 +31,15 @@ TEST_UUID = 'test-uuid'
TEST_SERVICE = 'testsvc' TEST_SERVICE = 'testsvc'
TEST_PID = 1234 TEST_PID = 1234
TEST_CMDLINE = 'python foo --router_id=%s' 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): class BaseTestProcessMonitor(base.BaseTestCase):
@ -124,7 +135,8 @@ class TestProcessManager(base.BaseTestCase):
def test_enable_no_namespace(self): def test_enable_no_namespace(self):
callback = mock.Mock() 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: with mock.patch.object(ep.ProcessManager, 'get_pid_file_name') as name:
name.return_value = 'pidfile' name.return_value = 'pidfile'
@ -134,7 +146,8 @@ class TestProcessManager(base.BaseTestCase):
manager = ep.ProcessManager(self.conf, 'uuid') manager = ep.ProcessManager(self.conf, 'uuid')
manager.enable(callback) manager.enable(callback)
callback.assert_called_once_with('pidfile') 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, check_exit_code=True,
extra_ok_codes=None, extra_ok_codes=None,
run_as_root=False, run_as_root=False,
@ -154,10 +167,11 @@ class TestProcessManager(base.BaseTestCase):
with mock.patch.object(ep, 'ip_lib') as ip_lib: with mock.patch.object(ep, 'ip_lib') as ip_lib:
manager.enable(callback) manager.enable(callback)
callback.assert_called_once_with('pidfile') callback.assert_called_once_with('pidfile')
env = {ep.PROCESS_TAG: ep.DEFAULT_SERVICE_NAME + '-uuid'}
ip_lib.assert_has_calls([ ip_lib.assert_has_calls([
mock.call.IPWrapper(namespace='ns'), mock.call.IPWrapper(namespace='ns'),
mock.call.IPWrapper().netns.execute( 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): def test_enable_with_namespace_process_active(self):
callback = mock.Mock() callback = mock.Mock()
@ -189,6 +203,56 @@ class TestProcessManager(base.BaseTestCase):
except common_utils.WaitTimeout: except common_utils.WaitTimeout:
self.fail('ProcessManager.enable() raised 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): def test_reload_cfg_without_custom_reload_callback(self):
with mock.patch.object(ep.ProcessManager, 'disable') as disable: with mock.patch.object(ep.ProcessManager, 'disable') as disable:
manager = ep.ProcessManager(self.conf, 'uuid', namespace='ns') manager = ep.ProcessManager(self.conf, 'uuid', namespace='ns')
@ -216,6 +280,7 @@ class TestProcessManager(base.BaseTestCase):
custom_reload_callback=reload_callback) custom_reload_callback=reload_callback)
manager.disable( manager.disable(
get_stop_command=manager.custom_reload_callback) get_stop_command=manager.custom_reload_callback)
cmd = ['env', DEFAULT_ENVVAR + '-uuid'] + cmd
self.assertIn(cmd, self.execute.call_args[0]) self.assertIn(cmd, self.execute.call_args[0])
def test_disable_no_namespace(self): def test_disable_no_namespace(self):
@ -227,8 +292,10 @@ class TestProcessManager(base.BaseTestCase):
with mock.patch.object(ep, 'utils') as utils: with mock.patch.object(ep, 'utils') as utils:
manager.disable() manager.disable()
env = {ep.PROCESS_TAG: ep.DEFAULT_SERVICE_NAME + '-uuid'}
utils.assert_has_calls([ utils.assert_has_calls([
mock.call.execute(['kill', '-9', 4], mock.call.execute(['kill', '-9', 4],
addl_env=env,
run_as_root=False, run_as_root=False,
privsep_exec=True)]) privsep_exec=True)])
@ -242,10 +309,11 @@ class TestProcessManager(base.BaseTestCase):
with mock.patch.object(ep, 'utils') as utils: with mock.patch.object(ep, 'utils') as utils:
manager.disable() manager.disable()
env = {ep.PROCESS_TAG: ep.DEFAULT_SERVICE_NAME + '-uuid'}
utils.assert_has_calls([ utils.assert_has_calls([
mock.call.execute(['kill', '-9', 4], mock.call.execute(
run_as_root=True, ['kill', '-9', 4], addl_env=env, run_as_root=True,
privsep_exec=True)]) privsep_exec=True)])
def test_disable_not_active(self): def test_disable_not_active(self):
with mock.patch.object(ep.ProcessManager, 'pid') as pid: with mock.patch.object(ep.ProcessManager, 'pid') as pid:
@ -280,16 +348,18 @@ class TestProcessManager(base.BaseTestCase):
pid.__get__ = mock.Mock(return_value=4) pid.__get__ = mock.Mock(return_value=4)
with mock.patch.object(ep.ProcessManager, 'active') as active: with mock.patch.object(ep.ProcessManager, 'active') as active:
active.__get__ = mock.Mock(return_value=True) active.__get__ = mock.Mock(return_value=True)
service_name = 'test-service'
manager = ep.ProcessManager( manager = ep.ProcessManager(
self.conf, 'uuid', namespace=namespace, self.conf, 'uuid', namespace=namespace,
service='test-service') service=service_name)
with mock.patch.object(ep, 'utils') as utils, \ with mock.patch.object(ep, 'utils') as utils, \
mock.patch.object(os.path, 'isfile', mock.patch.object(os.path, 'isfile',
return_value=kill_script_exists): return_value=kill_script_exists):
manager.disable() manager.disable()
addl_env = {ep.PROCESS_TAG: service_name + '-uuid'}
utils.execute.assert_called_with( utils.execute.assert_called_with(
expected_cmd, run_as_root=bool(namespace), expected_cmd, addl_env=addl_env,
privsep_exec=True) run_as_root=bool(namespace), privsep_exec=True)
def test_disable_custom_kill_script_no_namespace(self): def test_disable_custom_kill_script_no_namespace(self):
self._test_disable_custom_kill_script( self._test_disable_custom_kill_script(

View File

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

View File

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

View File

@ -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.