diff --git a/ironic/drivers/modules/console_utils.py b/ironic/drivers/modules/console_utils.py index b4172a9622..bcf8fe0cbb 100644 --- a/ironic/drivers/modules/console_utils.py +++ b/ironic/drivers/modules/console_utils.py @@ -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) diff --git a/ironic/tests/unit/drivers/modules/test_console_utils.py b/ironic/tests/unit/drivers/modules/test_console_utils.py index 23762e9f85..bddb421621 100644 --- a/ironic/tests/unit/drivers/modules/test_console_utils.py +++ b/ironic/tests/unit/drivers/modules/test_console_utils.py @@ -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'])