openstack/neutron
Code Issues Proposed changes

Catch when the process does not exist when killing it

Browse Source 
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
This commit is contained in:
Rodolfo Alonso Hernandez 2024-11-13 22:12:17 +00:00
parent 1da3fbf4f0
commit 0c29e730db
3 changed files with 99 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
neutron/agent/linux/external_process.py
View File

@ -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):

69
neutron/tests/functional/agent/linux/test_external_process.py Normal file
View File

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

21
neutron/tests/unit/agent/linux/test_external_process.py
View File

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