From 0c29e730db2629c084de0c114a0d1e8e6939ac25 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 13 Nov 2024 22:12:17 +0000 Subject: [PATCH] Catch when the process does not exist when killing it In ``ProcessManager.disable``, it could happen that the process to be stopped is no longer present in the system. In that case, catch this exception and dismiss it. If the goal of the ``disable`` method is to stop the process, it should not fail in the case of not being present anymore. Closes-Bug: #2088154 Change-Id: I5c6f7648d69e3a939445273f8d94241818538fc9 --- neutron/agent/linux/external_process.py | 26 ++++--- .../agent/linux/test_external_process.py | 69 +++++++++++++++++++ .../unit/agent/linux/test_external_process.py | 21 +++--- 3 files changed, 99 insertions(+), 17 deletions(-) create mode 100644 neutron/tests/functional/agent/linux/test_external_process.py diff --git a/neutron/agent/linux/external_process.py b/neutron/agent/linux/external_process.py index 27c28103c85..4126feb6ec3 100644 --- a/neutron/agent/linux/external_process.py +++ b/neutron/agent/linux/external_process.py @@ -17,9 +17,11 @@ import collections import os.path import eventlet +from neutron_lib import exceptions as n_exc from oslo_concurrency import lockutils from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import excutils from oslo_utils import fileutils import psutil @@ -134,6 +136,20 @@ class ProcessManager(MonitoredProcess): else: self.disable('HUP', delete_pid_file=False) + def _kill_process(self, cmd, pid): + try: + ip_wrapper = ip_lib.IPWrapper(namespace=self.namespace) + ip_wrapper.netns.execute(cmd, addl_env=self.cmd_addl_env, + run_as_root=self.run_as_root, + privsep_exec=True) + except n_exc.ProcessExecutionError as exc: + with excutils.save_and_reraise_exception() as ctxt: + if ('No such process' in str(exc) or + 'Cannot open network namespace' in str(exc)): + LOG.debug('Process %s not present when "kill" command ' + 'sent', pid) + ctxt.reraise = False + def disable(self, sig='9', get_stop_command=None, delete_pid_file=True): pid = self.pid delete_pid_file = delete_pid_file or sig == '9' @@ -141,15 +157,9 @@ class ProcessManager(MonitoredProcess): if self.active: if get_stop_command: cmd = get_stop_command(self.get_pid_file_name()) - ip_wrapper = ip_lib.IPWrapper(namespace=self.namespace) - ip_wrapper.netns.execute(cmd, addl_env=self.cmd_addl_env, - run_as_root=self.run_as_root, - privsep_exec=True) else: cmd = self.get_kill_cmd(sig, pid) - utils.execute(cmd, addl_env=self.cmd_addl_env, - run_as_root=self.run_as_root, - privsep_exec=True) + self._kill_process(cmd, pid) if delete_pid_file: utils.delete_if_exists(self.get_pid_file_name(), @@ -168,7 +178,7 @@ class ProcessManager(MonitoredProcess): kill_file = "%s-kill" % self.service kill_file_path = os.path.join(self.kill_scripts_path, kill_file) if os.path.isfile(kill_file_path): - return [kill_file_path, sig, pid] + return [kill_file_path, str(sig), pid] return ['kill', '-%s' % (sig), pid] def get_pid_file_name(self): diff --git a/neutron/tests/functional/agent/linux/test_external_process.py b/neutron/tests/functional/agent/linux/test_external_process.py new file mode 100644 index 00000000000..b3dcc4a2b58 --- /dev/null +++ b/neutron/tests/functional/agent/linux/test_external_process.py @@ -0,0 +1,69 @@ +# Copyright (c) 2024 Red Hat, Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import os +import signal +import tempfile + +from oslo_config import cfg + +from neutron.agent.common import async_process +from neutron.agent.linux import external_process as ep +from neutron.agent.linux import utils as agent_utils +from neutron.common import utils as common_utils +from neutron.tests.common import net_helpers +from neutron.tests.functional import base as functional_base + + +class ProcessManagerTestCase(functional_base.BaseSudoTestCase): + + def _create_sleep_process(self, time=None): + if time is None: + cmd = ['sleep', 'infinity'] + else: + cmd = ['sleep', str(time)] + process = async_process.AsyncProcess(cmd) + process.start() + + with tempfile.NamedTemporaryFile('w+', delete=False) as pid_file: + pid_file.write(process.pid) + os.chmod(pid_file.name, 0o777) + uuid = 'sleep infinity' + namespace = self.useFixture(net_helpers.NamespaceFixture()).name + return ep.ProcessManager(cfg.CONF, uuid, namespace, + pid_file=pid_file.name) + + def test__kill_process(self): + pm = self._create_sleep_process() + self.assertTrue(pm.active) + + pm._kill_process(pm.get_kill_cmd(int(signal.SIGKILL), pm.pid), pm.pid) + # Delete the PID file used by ``pm.active``. + agent_utils.delete_if_exists(pm.get_pid_file_name()) + self.assertFalse(pm.active) + + def test__kill_process_process_not_present(self): + pm = self._create_sleep_process(time=0) + # "sleep 0" should end immediately, but we add an active wait of 3 + # seconds just to avoid any race condition. + try: + common_utils.wait_until_true(lambda: not pm.active, timeout=3) + except common_utils.WaitTimeout: + self.fail('The process "sleep 0" (PID: %s) did not finish' % + pm.pid) + + # '_kill_process' should not raise any exception. + pm._kill_process(pm.get_kill_cmd(int(signal.SIGKILL), pm.pid), pm.pid) + self.assertFalse(pm.active) diff --git a/neutron/tests/unit/agent/linux/test_external_process.py b/neutron/tests/unit/agent/linux/test_external_process.py index b35144065a4..92f717eab7a 100644 --- a/neutron/tests/unit/agent/linux/test_external_process.py +++ b/neutron/tests/unit/agent/linux/test_external_process.py @@ -24,6 +24,7 @@ from oslo_utils import uuidutils import psutil from neutron.agent.linux import external_process as ep +from neutron.agent.linux import ip_lib from neutron.common import utils as common_utils from neutron.tests import base @@ -269,10 +270,11 @@ class TestProcessManager(base.BaseTestCase): active.__get__ = mock.Mock(return_value=True) manager = ep.ProcessManager(self.conf, 'uuid') - with mock.patch.object(ep, 'utils') as utils: + with mock.patch.object(ip_lib.IpNetnsCommand, 'execute') as \ + mock_execute: manager.disable() env = {ep.PROCESS_TAG: ep.DEFAULT_SERVICE_NAME + '-uuid'} - utils.assert_has_calls([ + mock_execute.assert_has_calls([ mock.call.execute(['kill', '-9', 4], addl_env=env, run_as_root=False, @@ -286,10 +288,11 @@ class TestProcessManager(base.BaseTestCase): manager = ep.ProcessManager(self.conf, 'uuid', namespace='ns') - with mock.patch.object(ep, 'utils') as utils: + with mock.patch.object(ip_lib.IpNetnsCommand, 'execute') as \ + mock_execute: manager.disable() env = {ep.PROCESS_TAG: ep.DEFAULT_SERVICE_NAME + '-uuid'} - utils.assert_has_calls([ + mock_execute.assert_has_calls([ mock.call.execute( ['kill', '-9', 4], addl_env=env, run_as_root=True, privsep_exec=True)]) @@ -323,7 +326,7 @@ class TestProcessManager(base.BaseTestCase): else: expected_cmd = ['kill', '-9', 4] - with mock.patch.object(ep.ProcessManager, 'pid') as pid: + with (mock.patch.object(ep.ProcessManager, 'pid') as pid): pid.__get__ = mock.Mock(return_value=4) with mock.patch.object(ep.ProcessManager, 'active') as active: active.__get__ = mock.Mock(return_value=True) @@ -331,12 +334,12 @@ class TestProcessManager(base.BaseTestCase): manager = ep.ProcessManager( self.conf, 'uuid', namespace=namespace, service=service_name) - with mock.patch.object(ep, 'utils') as utils, \ - mock.patch.object(os.path, 'isfile', - return_value=kill_script_exists): + with mock.patch.object(ip_lib.IpNetnsCommand, 'execute') as \ + execute_mock, 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( + execute_mock.assert_called_with( expected_cmd, addl_env=addl_env, run_as_root=bool(namespace), privsep_exec=True)