openstack/ironic
Code Issues Proposed changes

Check shellinabox process during stopping console

Browse Source 
Currently we assume shellinabox is running as long as pid file
exists, which will cause error while executing kill command,
this patch update to use os.kill() and if errno is ESRCH, just
log a warning message.

Change-Id: Ib8968418a1835a4131f2f22fb3e4df5ecb9b0dc5
Closes-Bug: #1513690
This commit is contained in:
Zhenguo Niu 2015-11-12 14:24:14 +08:00
parent 1ef1b397bf
commit 24188760c8
2 changed files with 45 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
ironic/drivers/modules/console_utils.py
View File

@ -19,7 +19,9 @@
Ironic console utilities.
"""
import errno
import os
import signal
import subprocess
import time
@ -122,8 +124,16 @@ def _stop_console(node_uuid):
try:
console_pid = _get_console_pid(node_uuid)
# Allow exitcode 99 (RC_UNAUTHORIZED)
utils.execute('kill', str(console_pid), check_exit_code=[0, 99])
os.kill(console_pid, signal.SIGTERM)
except OSError as exc:
if exc.errno != errno.ESRCH:
msg = (_("Could not stop the console for node '%(node)s'. "
"Reason: %(err)s.") % {'node': node_uuid, 'err': exc})
raise exception.ConsoleError(message=msg)
else:
LOG.warning(_LW("Console process for node %s is not running "
"but pid file exists while trying to stop "
"shellinabox console."), node_uuid)
finally:
utils.unlink_without_raise(_get_console_pid_file(node_uuid))
@ -261,7 +271,3 @@ def stop_shellinabox_console(node_uuid):
except exception.NoConsolePid:
LOG.warning(_LW("No console pid found for node %s while trying to "
"stop shellinabox console."), node_uuid)
except processutils.ProcessExecutionError as exc:
msg = (_("Could not stop the console for node '%(node)s'. "
"Reason: %(err)s.") % {'node': node_uuid, 'err': exc})
raise exception.ConsoleError(message=msg)

59
ironic/tests/unit/drivers/modules/test_console_utils.py
View File

@ -17,14 +17,15 @@
"""Test class for console_utils driver module."""
import errno
import os
import random
import signal
import string
import subprocess
import tempfile
import mock
from oslo_concurrency import processutils
from oslo_config import cfg
from oslo_utils import netutils
@ -136,23 +137,23 @@ class ConsoleUtilsTestCase(db_base.DbTestCase):
self.info['uuid'])
@mock.patch.object(utils, 'unlink_without_raise', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
@mock.patch.object(os, 'kill', autospec=True)
@mock.patch.object(console_utils, '_get_console_pid', autospec=True)
def test__stop_console(self, mock_pid, mock_execute, mock_unlink):
def test__stop_console(self, mock_pid, mock_kill, mock_unlink):
pid_file = console_utils._get_console_pid_file(self.info['uuid'])
mock_pid.return_value = '12345'
mock_pid.return_value = 12345
console_utils._stop_console(self.info['uuid'])
mock_pid.assert_called_once_with(self.info['uuid'])
mock_execute.assert_called_once_with('kill', mock_pid.return_value,
check_exit_code=[0, 99])
mock_kill.assert_called_once_with(mock_pid.return_value,
signal.SIGTERM)
mock_unlink.assert_called_once_with(pid_file)
@mock.patch.object(utils, 'unlink_without_raise', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
@mock.patch.object(os, 'kill', autospec=True)
@mock.patch.object(console_utils, '_get_console_pid', autospec=True)
def test__stop_console_nopid(self, mock_pid, mock_execute, mock_unlink):
def test__stop_console_nopid(self, mock_pid, mock_kill, mock_unlink):
pid_file = console_utils._get_console_pid_file(self.info['uuid'])
mock_pid.side_effect = iter(
[exception.NoConsolePid(pid_path="/tmp/blah")])
@ -162,24 +163,40 @@ class ConsoleUtilsTestCase(db_base.DbTestCase):
self.info['uuid'])
mock_pid.assert_called_once_with(self.info['uuid'])
self.assertFalse(mock_execute.called)
self.assertFalse(mock_kill.called)
mock_unlink.assert_called_once_with(pid_file)
@mock.patch.object(utils, 'unlink_without_raise', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
@mock.patch.object(os, 'kill', autospec=True)
@mock.patch.object(console_utils, '_get_console_pid', autospec=True)
def test__stop_console_nokill(self, mock_pid, mock_execute, mock_unlink):
def test__stop_console_shellinabox_not_running(self, mock_pid,
mock_kill, mock_unlink):
pid_file = console_utils._get_console_pid_file(self.info['uuid'])
mock_pid.return_value = '12345'
mock_execute.side_effect = iter([processutils.ProcessExecutionError()])
mock_pid.return_value = 12345
mock_kill.side_effect = OSError(errno.ESRCH, 'message')
self.assertRaises(processutils.ProcessExecutionError,
console_utils._stop_console(self.info['uuid'])
mock_pid.assert_called_once_with(self.info['uuid'])
mock_kill.assert_called_once_with(mock_pid.return_value,
signal.SIGTERM)
mock_unlink.assert_called_once_with(pid_file)
@mock.patch.object(utils, 'unlink_without_raise', autospec=True)
@mock.patch.object(os, 'kill', autospec=True)
@mock.patch.object(console_utils, '_get_console_pid', autospec=True)
def test__stop_console_exception(self, mock_pid, mock_kill, mock_unlink):
pid_file = console_utils._get_console_pid_file(self.info['uuid'])
mock_pid.return_value = 12345
mock_kill.side_effect = OSError(2, 'message')
self.assertRaises(exception.ConsoleError,
console_utils._stop_console,
self.info['uuid'])
mock_pid.assert_called_once_with(self.info['uuid'])
mock_execute.assert_called_once_with('kill', mock_pid.return_value,
check_exit_code=[0, 99])
mock_kill.assert_called_once_with(mock_pid.return_value,
signal.SIGTERM)
mock_unlink.assert_called_once_with(pid_file)
def _get_shellinabox_console(self, scheme):
@ -336,13 +353,3 @@ class ConsoleUtilsTestCase(db_base.DbTestCase):
console_utils.stop_shellinabox_console(self.info['uuid'])
mock_stop.assert_called_once_with(self.info['uuid'])
@mock.patch.object(console_utils, '_stop_console', autospec=True)
def test_stop_shellinabox_console_fail_nokill(self, mock_stop):
mock_stop.side_effect = iter([processutils.ProcessExecutionError()])
self.assertRaises(exception.ConsoleError,
console_utils.stop_shellinabox_console,
self.info['uuid'])
mock_stop.assert_called_once_with(self.info['uuid'])