From 857372a2269cdd0f8a1ae5b9e9f6e0ee193f01be Mon Sep 17 00:00:00 2001 From: Yuiko Takada Mori Date: Thu, 14 Jul 2016 11:17:24 +0900 Subject: [PATCH] IPMITool: add IPMISocatConsole and IPMIConsole class IPMISocatConsole is a new console interface class using ipmitool and socat. It has the same APIs of IPMIShellinaboxConsole class and provides TCP4/TCP6 connection service to connect serial-on-LAN of nodes. IPMIConsole is a new console interface class using ipmitool and used as parent class of IPMIShellinaboxConsole and IPMISocatConsol class. This patch set implements new console driver interfaces IPMISocatConsole. To use PXE + IPMItool + socat, specify pxe_ipmitool_socat. To use IPA + IPMItool + socat, specify agent_ipmitool_socat. Spec: https://review.openstack.org/#/c/319505/ Partial-Bug: #1553083 Change-Id: I35a7dcb7e89baf16d096501fd44dbc12adc8c61e --- ironic/drivers/agent.py | 19 + ironic/drivers/fake.py | 11 + ironic/drivers/modules/ipmitool.py | 92 +++- ironic/drivers/pxe.py | 19 + .../unit/drivers/modules/test_ipmitool.py | 402 ++++++++++++++---- ironic/tests/unit/drivers/test_agent.py | 37 ++ ironic/tests/unit/drivers/test_pxe.py | 13 + ...cat-console-ipmitool-ab4402ec976c5c96.yaml | 5 + setup.cfg | 3 + 9 files changed, 498 insertions(+), 103 deletions(-) create mode 100644 releasenotes/notes/add-socat-console-ipmitool-ab4402ec976c5c96.yaml diff --git a/ironic/drivers/agent.py b/ironic/drivers/agent.py index 56f3d57443..1b1a000566 100644 --- a/ironic/drivers/agent.py +++ b/ironic/drivers/agent.py @@ -66,6 +66,25 @@ class AgentAndIPMIToolDriver(base.BaseDriver): 'AgentAndIPMIToolDriver') +class AgentAndIPMIToolAndSocatDriver(AgentAndIPMIToolDriver): + """Agent + IPMITool + socat driver. + + This driver implements the `core` functionality, combining + :class:`ironic.drivers.modules.ipmitool.IPMIPower` (for power on/off and + reboot) with :class:`ironic.drivers.modules.agent.AgentDeploy` (for + image deployment) and with + :class:`ironic.drivers.modules.ipmitool.IPMISocatConsole`. + This driver uses the socat console interface instead of the shellinabox + one. + Implementations are in those respective classes; this class is merely the + glue between them. + """ + + def __init__(self): + AgentAndIPMIToolDriver.__init__(self) + self.console = ipmitool.IPMISocatConsole() + + class AgentAndIPMINativeDriver(base.BaseDriver): """Agent + IPMINative driver. diff --git a/ironic/drivers/fake.py b/ironic/drivers/fake.py index 88ccefd61a..46912cd41c 100644 --- a/ironic/drivers/fake.py +++ b/ironic/drivers/fake.py @@ -90,6 +90,17 @@ class FakeIPMIToolDriver(base.BaseDriver): self.management = ipmitool.IPMIManagement() +class FakeIPMIToolSocatDriver(base.BaseDriver): + """Example implementation of a Driver.""" + + def __init__(self): + self.power = ipmitool.IPMIPower() + self.console = ipmitool.IPMISocatConsole() + self.deploy = fake.FakeDeploy() + self.vendor = ipmitool.VendorPassthru() + self.management = ipmitool.IPMIManagement() + + class FakePXEDriver(base.BaseDriver): """Example implementation of a Driver.""" diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index cef19d25b9..173c56acfe 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -355,11 +355,12 @@ def _parse_driver_info(node): } -def _exec_ipmitool(driver_info, command): +def _exec_ipmitool(driver_info, command, check_exit_code=None): """Execute the ipmitool command. :param driver_info: the ipmitool parameters for accessing a node. :param command: the ipmitool command to be executed. + :param check_exit_code: Single bool, int, or list of allowed exit codes. :returns: (stdout, stderr) from executing the command. :raises: PasswordFileFailedToCreate from creating or writing to the temporary file. @@ -414,6 +415,9 @@ def _exec_ipmitool(driver_info, command): # Resetting the list that will be utilized so the password arguments # from any previous execution are preserved. cmd_args = args[:] + extra_args = {} + if check_exit_code is not None: + extra_args['check_exit_code'] = check_exit_code # 'ipmitool' command will prompt password if there is no '-f' # option, we set it to '\0' to write a password file to support # empty password @@ -422,7 +426,7 @@ def _exec_ipmitool(driver_info, command): cmd_args.append(pw_file) cmd_args.extend(command.split(" ")) try: - out, err = utils.execute(*cmd_args) + out, err = utils.execute(*cmd_args, **extra_args) return out, err except processutils.ProcessExecutionError as e: with excutils.save_and_reraise_exception() as ctxt: @@ -1090,8 +1094,8 @@ class VendorPassthru(base.VendorInterface): _parse_driver_info(task.node) -class IPMIShellinaboxConsole(base.ConsoleInterface): - """A ConsoleInterface that uses ipmitool and shellinabox.""" +class IPMIConsole(base.ConsoleInterface): + """A base ConsoleInterface that uses ipmitool.""" def __init__(self): try: @@ -1128,10 +1132,11 @@ class IPMIShellinaboxConsole(base.ConsoleInterface): "Check the 'ipmi_protocol_version' parameter in " "node's driver_info")) - def start_console(self, task): + def _start_console(self, driver_info, start_method): """Start a remote console for the node. :param task: a task from TaskManager + :param start_method: console_utils method to start console :raises: InvalidParameterValue if required ipmi parameters are missing :raises: PasswordFileFailedToCreate if unable to create a file containing the password @@ -1139,8 +1144,6 @@ class IPMIShellinaboxConsole(base.ConsoleInterface): created :raises: ConsoleSubprocessFailed when invoking the subprocess failed """ - driver_info = _parse_driver_info(task.node) - path = _console_pwfile_path(driver_info['uuid']) pw_file = console_utils.make_persistent_password_file( path, driver_info['password'] or '\0') @@ -1162,13 +1165,30 @@ class IPMIShellinaboxConsole(base.ConsoleInterface): ipmi_cmd += " -v" ipmi_cmd += " sol activate" try: - console_utils.start_shellinabox_console(driver_info['uuid'], - driver_info['port'], - ipmi_cmd) + start_method(driver_info['uuid'], driver_info['port'], ipmi_cmd) except (exception.ConsoleError, exception.ConsoleSubprocessFailed): with excutils.save_and_reraise_exception(): ironic_utils.unlink_without_raise(path) + +class IPMIShellinaboxConsole(IPMIConsole): + """A ConsoleInterface that uses ipmitool and shellinabox.""" + + def start_console(self, task): + """Start a remote console for the node. + + :param task: a task from TaskManager + :raises: InvalidParameterValue if required ipmi parameters are missing + :raises: PasswordFileFailedToCreate if unable to create a file + containing the password + :raises: ConsoleError if the directory for the PID file cannot be + created + :raises: ConsoleSubprocessFailed when invoking the subprocess failed + """ + driver_info = _parse_driver_info(task.node) + self._start_console(driver_info, + console_utils.start_shellinabox_console) + def stop_console(self, task): """Stop the remote console session for the node. @@ -1186,3 +1206,55 @@ class IPMIShellinaboxConsole(base.ConsoleInterface): driver_info = _parse_driver_info(task.node) url = console_utils.get_shellinabox_console_url(driver_info['port']) return {'type': 'shellinabox', 'url': url} + + +class IPMISocatConsole(IPMIConsole): + """A ConsoleInterface that uses ipmitool and socat.""" + + def start_console(self, task): + """Start a remote console for the node. + + :param task: a task from TaskManager + :raises: InvalidParameterValue if required ipmi parameters are missing + :raises: PasswordFileFailedToCreate if unable to create a file + containing the password + :raises: ConsoleError if the directory for the PID file cannot be + created + :raises: ConsoleSubprocessFailed when invoking the subprocess failed + """ + driver_info = _parse_driver_info(task.node) + try: + self._exec_stop_console(driver_info) + except OSError: + # We need to drop any existing sol sessions with sol deactivate. + # OSError is raised when sol session is deactive, so we can + # ignore it. + pass + self._start_console(driver_info, console_utils.start_socat_console) + + def stop_console(self, task): + """Stop the remote console session for the node. + + :param task: a task from TaskManager + :raises: ConsoleError if unable to stop the console + """ + driver_info = _parse_driver_info(task.node) + try: + console_utils.stop_socat_console(task.node.uuid) + finally: + ironic_utils.unlink_without_raise( + _console_pwfile_path(task.node.uuid)) + self._exec_stop_console(driver_info) + + def _exec_stop_console(self, driver_info): + cmd = "sol deactivate" + _exec_ipmitool(driver_info, cmd, check_exit_code=[0, 1]) + + def get_console(self, task): + """Get the type and connection information about the console. + + :param task: a task from TaskManager + """ + driver_info = _parse_driver_info(task.node) + url = console_utils.get_socat_console_url(driver_info['port']) + return {'type': 'socat', 'url': url} diff --git a/ironic/drivers/pxe.py b/ironic/drivers/pxe.py index 3ec78c6359..d37fee90ca 100644 --- a/ironic/drivers/pxe.py +++ b/ironic/drivers/pxe.py @@ -85,6 +85,25 @@ class PXEAndIPMIToolDriver(base.BaseDriver): self.raid = agent.AgentRAID() +class PXEAndIPMIToolAndSocatDriver(PXEAndIPMIToolDriver): + """PXE + IPMITool + socat driver. + + This driver implements the `core` functionality, combining + :class:`ironic.drivers.modules.ipmi.IPMI` for power on/off + and reboot with + :class:`ironic.drivers.modules.iscsi_deploy.ISCSIDeploy` (for + image deployment) and with + :class:`ironic.drivers.modules.ipmitool.IPMISocatConsole`. + This driver uses the socat console interface instead of the shellinabox + one. + Implementations are in those respective + classes; this class is merely the glue between them. + """ + def __init__(self): + PXEAndIPMIToolDriver.__init__(self) + self.console = ipmitool.IPMISocatConsole() + + class PXEAndSSHDriver(base.BaseDriver): """PXE + SSH driver. diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index aacc986f4d..5fd3aee6bd 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -28,6 +28,7 @@ import tempfile import time import types +from ironic_lib import utils as ironic_utils import mock from oslo_concurrency import processutils from oslo_config import cfg @@ -108,7 +109,7 @@ class IPMIToolCheckInitTestCase(base.TestCase): ipmi.TMP_DIR_CHECKED = True ipmi.IPMIPower() mock_support.assert_called_with(mock.ANY) - self.assertEqual(0, mock_check_dir.call_count) + self.assertFalse(mock_check_dir.called) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @mock.patch.object(utils, 'check_dir', autospec=True) @@ -130,7 +131,7 @@ class IPMIToolCheckInitTestCase(base.TestCase): ipmi.IPMIManagement() mock_support.assert_called_with(mock.ANY) - self.assertEqual(0, mock_check_dir.call_count) + self.assertFalse(mock_check_dir.called) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @mock.patch.object(utils, 'check_dir', autospec=True) @@ -150,7 +151,7 @@ class IPMIToolCheckInitTestCase(base.TestCase): ipmi.TMP_DIR_CHECKED = True ipmi.VendorPassthru() mock_support.assert_called_with(mock.ANY) - self.assertEqual(0, mock_check_dir.call_count) + self.assertFalse(mock_check_dir.called) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @mock.patch.object(utils, 'check_dir', autospec=True) @@ -170,7 +171,29 @@ class IPMIToolCheckInitTestCase(base.TestCase): ipmi.TMP_DIR_CHECKED = True ipmi.IPMIShellinaboxConsole() mock_support.assert_called_with(mock.ANY) - self.assertEqual(0, mock_check_dir.call_count) + self.assertFalse(mock_check_dir.called) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(utils, 'check_dir', autospec=True) + def test_console_init_calls_for_socat(self, mock_check_dir, mock_support): + with mock.patch.object(ipmi, 'TMP_DIR_CHECKED'): + mock_support.return_value = True + ipmi.TMP_DIR_CHECKED = None + ipmi.IPMISocatConsole() + mock_support.assert_called_with(mock.ANY) + mock_check_dir.assert_called_once_with() + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(utils, 'check_dir', autospec=True) + def test_console_init_calls_for_socat_already_checked(self, + mock_check_dir, + mock_support): + with mock.patch.object(ipmi, 'TMP_DIR_CHECKED'): + mock_support.return_value = True + ipmi.TMP_DIR_CHECKED = True + ipmi.IPMISocatConsole() + mock_support.assert_called_with(mock.ANY) + self.assertFalse(mock_check_dir.call_count) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @@ -977,14 +1000,14 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_with_single_bridging(self, mock_exec, - mock_support, - mock_sleep): + mock_pass, + mock_support): single_bridge_info = dict(BRIDGE_INFO_DICT) single_bridge_info['ipmi_bridging'] = 'single' node = obj_utils.get_test_node(self.context, driver='fake_ipmitool', driver_info=single_bridge_info) # when support for single bridge command is called returns True - mock_support.return_value = True + mock_pass.return_value = True info = ipmi._parse_driver_info(node) info['transit_channel'] = info['transit_address'] = None @@ -1004,17 +1027,17 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): expected = [mock.call('single_bridge'), mock.call('timing')] # When support for timing command is called returns False - mock_support.return_value = False + mock_pass.return_value = False mock_exec.return_value = (None, None) ipmi._exec_ipmitool(info, 'A B C') - self.assertEqual(expected, mock_support.call_args_list) + self.assertEqual(expected, mock_pass.call_args_list) mock_exec.assert_called_once_with(*args) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_exception( - self, mock_exec, mock_support, mock_sleep): + self, mock_exec, mock_pass, mock_support): args = [ 'ipmitool', '-I', 'lanplus', @@ -1025,12 +1048,12 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): 'A', 'B', 'C', ] - mock_support.return_value = False + mock_pass.return_value = False mock_exec.side_effect = processutils.ProcessExecutionError("x") self.assertRaises(processutils.ProcessExecutionError, ipmi._exec_ipmitool, self.info, 'A B C') - mock_support.assert_called_once_with('timing') + mock_pass.assert_called_once_with('timing') mock_exec.assert_called_once_with(*args) self.assertEqual(1, mock_exec.call_count) @@ -1117,7 +1140,7 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_IPMI_version_1_5( - self, mock_exec, mock_support, mock_sleep): + self, mock_exec, mock_pass, mock_support): self.info['protocol_version'] = '1.5' # Assert it uses "-I lan" (1.5) instead of "-I lanplus" (2.0) args = [ @@ -1130,17 +1153,17 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): 'A', 'B', 'C', ] - mock_support.return_value = False + mock_pass.return_value = False mock_exec.return_value = (None, None) ipmi._exec_ipmitool(self.info, 'A B C') - mock_support.assert_called_once_with('timing') + mock_pass.assert_called_once_with('timing') mock_exec.assert_called_once_with(*args) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) @mock.patch.object(utils, 'execute', autospec=True) - def test__exec_ipmitool_with_port(self, mock_exec, mock_support, - mock_sleep): + def test__exec_ipmitool_with_port(self, mock_exec, mock_pass, + mock_support): self.info['dest_port'] = '1623' ipmi.LAST_CMD_TIME = {} args = [ @@ -1154,14 +1177,34 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): 'A', 'B', 'C', ] - mock_support.return_value = False + mock_pass.return_value = False mock_exec.return_value = (None, None) ipmi._exec_ipmitool(self.info, 'A B C') - mock_support.assert_called_once_with('timing') + mock_pass.assert_called_once_with('timing') mock_exec.assert_called_once_with(*args) - self.assertFalse(mock_sleep.called) + self.assertFalse(mock_support.called) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_check_exit_code(self, mock_exec, + mock_pass, mock_support): + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-f', awesome_password_filename, + 'A', 'B', 'C', + ] + mock_pass.return_value = False + mock_exec.return_value = (None, None) + ipmi._exec_ipmitool(self.info, 'A B C', check_exit_code=[0, 1]) + mock_pass.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, check_exit_code=[0, 1]) @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) def test__power_status_on(self, mock_exec, mock_sleep): @@ -1222,13 +1265,18 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): class IPMIToolDriverTestCase(db_base.DbTestCase): - def setUp(self): + def setUp(self, terminal=None): super(IPMIToolDriverTestCase, self).setUp() - mgr_utils.mock_the_extension_manager(driver="fake_ipmitool") - self.driver = driver_factory.get_driver("fake_ipmitool") + if terminal is None: + self.driver_name = "fake_ipmitool" + else: + self.driver_name = "fake_ipmitool_socat" + + mgr_utils.mock_the_extension_manager(driver=self.driver_name) + self.driver = driver_factory.get_driver(self.driver_name) self.node = obj_utils.create_test_node(self.context, - driver='fake_ipmitool', + driver=self.driver_name, driver_info=INFO_DICT) self.info = ipmi._parse_driver_info(self.node) @@ -1290,7 +1338,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock_on.return_value = states.POWER_ON with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.driver.power.set_power_state(task, states.POWER_ON) @@ -1306,7 +1354,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock_on.return_value = states.POWER_ON with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.driver.power.set_power_state(task, states.POWER_ON) mock_next_boot.assert_called_once_with(task, self.info) @@ -1322,7 +1370,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock_off.return_value = states.POWER_OFF with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.driver.power.set_power_state(task, states.POWER_OFF) @@ -1336,7 +1384,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock_on.return_value = states.ERROR with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.assertRaises(exception.PowerStateFailure, self.driver.power.set_power_state, task, @@ -1346,7 +1394,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): self.assertFalse(mock_off.called) def test_set_power_invalid_state(self): - with task_manager.acquire(self.context, self.node['uuid']) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: self.assertRaises(exception.InvalidParameterValue, self.driver.power.set_power_state, task, @@ -1357,7 +1405,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock_exec.return_value = [None, None] with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.driver.vendor.send_raw(task, http_method='POST', raw_bytes='0x00 0x01') @@ -1368,7 +1416,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock_exec.side_effect = exception.PasswordFileFailedToCreate('error') with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.assertRaises(exception.IPMIFailure, self.driver.vendor.send_raw, task, @@ -1380,7 +1428,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock_exec.return_value = [None, None] with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.driver.vendor.bmc_reset(task, 'POST') mock_exec.assert_called_once_with(self.info, 'bmc reset warm') @@ -1390,7 +1438,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock_exec.return_value = [None, None] with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.driver.vendor.bmc_reset(task, 'POST', warm=False) mock_exec.assert_called_once_with(self.info, 'bmc reset cold') @@ -1400,7 +1448,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock_exec.side_effect = processutils.ProcessExecutionError() with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.assertRaises(exception.IPMIFailure, self.driver.vendor.bmc_reset, task, 'POST') @@ -1418,7 +1466,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock.call.power_on(self.info)] with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.driver.power.reboot(task) mock_next_boot.assert_called_once_with(task, self.info) @@ -1436,7 +1484,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock.call.power_on(self.info)] with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.assertRaises(exception.PowerStateFailure, self.driver.power.reboot, task) @@ -1446,28 +1494,28 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): @mock.patch.object(ipmi, '_parse_driver_info', autospec=True) def test_vendor_passthru_validate__parse_driver_info_fail(self, info_mock): info_mock.side_effect = exception.InvalidParameterValue("bad") - with task_manager.acquire(self.context, self.node['uuid']) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: self.assertRaises(exception.InvalidParameterValue, self.driver.vendor.validate, task, method='send_raw', raw_bytes='0x00 0x01') info_mock.assert_called_once_with(task.node) def test_vendor_passthru_validate__send_raw_bytes_good(self): - with task_manager.acquire(self.context, self.node['uuid']) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: self.driver.vendor.validate(task, method='send_raw', http_method='POST', raw_bytes='0x00 0x01') def test_vendor_passthru_validate__send_raw_bytes_fail(self): - with task_manager.acquire(self.context, self.node['uuid']) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: self.assertRaises(exception.MissingParameterValue, self.driver.vendor.validate, task, method='send_raw') @mock.patch.object(ipmi.VendorPassthru, 'send_raw', autospec=True) def test_vendor_passthru_call_send_raw_bytes(self, raw_bytes_mock): - with task_manager.acquire(self.context, self.node['uuid'], + with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: self.driver.vendor.send_raw(task, http_method='POST', raw_bytes='0x00 0x01') @@ -1476,18 +1524,18 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): raw_bytes='0x00 0x01') def test_vendor_passthru_validate__bmc_reset_good(self): - with task_manager.acquire(self.context, self.node['uuid']) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: self.driver.vendor.validate(task, method='bmc_reset') def test_vendor_passthru_validate__bmc_reset_warm_good(self): - with task_manager.acquire(self.context, self.node['uuid']) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: self.driver.vendor.validate(task, method='bmc_reset', warm=True) def test_vendor_passthru_validate__bmc_reset_cold_good(self): - with task_manager.acquire(self.context, self.node['uuid']) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: self.driver.vendor.validate(task, method='bmc_reset', warm=False) @@ -1496,7 +1544,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): def _vendor_passthru_call_bmc_reset(self, warm, expected, mock_exec): mock_exec.return_value = [None, None] - with task_manager.acquire(self.context, self.node['uuid'], + with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: self.driver.vendor.bmc_reset(task, 'POST', warm=warm) mock_exec.assert_called_once_with( @@ -1553,49 +1601,66 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, task.driver.console.validate, task) - @mock.patch.object(console_utils, 'start_shellinabox_console', - autospec=True) - def test_start_console(self, mock_exec): - mock_exec.return_value = None - - with task_manager.acquire(self.context, - self.node['uuid']) as task: - self.driver.console.start_console(task) - - mock_exec.assert_called_once_with(self.info['uuid'], - self.info['port'], - mock.ANY) - self.assertTrue(mock_exec.called) - - @mock.patch.object(console_utils, 'start_shellinabox_console', - autospec=True) - def test_start_console_fail(self, mock_exec): - mock_exec.side_effect = exception.ConsoleSubprocessFailed( - error='error') - - with task_manager.acquire(self.context, - self.node['uuid']) as task: - self.assertRaises(exception.ConsoleSubprocessFailed, - self.driver.console.start_console, - task) - - @mock.patch.object(console_utils, 'start_shellinabox_console', - autospec=True) - def test_start_console_fail_nodir(self, mock_exec): - mock_exec.side_effect = exception.ConsoleError() + @mock.patch.object(ipmi.IPMIConsole, '_start_console', autospec=True) + def test_start_console(self, mock_start): + mock_start.return_value = None with task_manager.acquire(self.context, self.node.uuid) as task: + self.driver.console.start_console(task) + driver_info = ipmi._parse_driver_info(task.node) + mock_start.assert_called_once_with( + self.driver.console, driver_info, + console_utils.start_shellinabox_console) + + @mock.patch.object(console_utils, 'start_shellinabox_console', + autospec=True) + def test__start_console(self, mock_start): + mock_start.return_value = None + + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + self.driver.console._start_console( + driver_info, console_utils.start_shellinabox_console) + + mock_start.assert_called_once_with(self.info['uuid'], + self.info['port'], + mock.ANY) + + @mock.patch.object(console_utils, 'start_shellinabox_console', + autospec=True) + def test__start_console_fail(self, mock_start): + mock_start.side_effect = exception.ConsoleSubprocessFailed( + error='error') + + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + self.assertRaises(exception.ConsoleSubprocessFailed, + self.driver.console._start_console, + driver_info, + console_utils.start_shellinabox_console) + + @mock.patch.object(console_utils, 'start_shellinabox_console', + autospec=True) + def test__start_console_fail_nodir(self, mock_start): + mock_start.side_effect = exception.ConsoleError() + + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) self.assertRaises(exception.ConsoleError, - self.driver.console.start_console, - task) - mock_exec.assert_called_once_with(self.node.uuid, mock.ANY, mock.ANY) + self.driver.console._start_console, + driver_info, + console_utils.start_shellinabox_console) + mock_start.assert_called_once_with(self.node.uuid, mock.ANY, mock.ANY) @mock.patch.object(console_utils, 'make_persistent_password_file', autospec=True) @mock.patch.object(console_utils, 'start_shellinabox_console', autospec=True) - def test_start_console_empty_password(self, mock_exec, mock_pass): + def test__start_console_empty_password(self, mock_start, mock_pass): driver_info = self.node.driver_info del driver_info['ipmi_password'] self.node.driver_info = driver_info @@ -1603,24 +1668,25 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid) as task: - self.driver.console.start_console(task) + driver_info = ipmi._parse_driver_info(task.node) + self.driver.console._start_console( + driver_info, console_utils.start_shellinabox_console) mock_pass.assert_called_once_with(mock.ANY, '\0') - mock_exec.assert_called_once_with(self.info['uuid'], - self.info['port'], - mock.ANY) + mock_start.assert_called_once_with(self.info['uuid'], + self.info['port'], + mock.ANY) @mock.patch.object(console_utils, 'stop_shellinabox_console', autospec=True) - def test_stop_console(self, mock_exec): - mock_exec.return_value = None + def test_stop_console(self, mock_stop): + mock_stop.return_value = None with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: self.driver.console.stop_console(task) - mock_exec.assert_called_once_with(self.info['uuid']) - self.assertTrue(mock_exec.called) + mock_stop.assert_called_once_with(self.info['uuid']) @mock.patch.object(console_utils, 'stop_shellinabox_console', autospec=True) @@ -1637,18 +1703,17 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): @mock.patch.object(console_utils, 'get_shellinabox_console_url', autospec=True) - def test_get_console(self, mock_exec): + def test_get_console(self, mock_get): url = 'http://localhost:4201' - mock_exec.return_value = url + mock_get.return_value = url expected = {'type': 'shellinabox', 'url': url} with task_manager.acquire(self.context, - self.node['uuid']) as task: + self.node.uuid) as task: console_info = self.driver.console.get_console(task) self.assertEqual(expected, console_info) - mock_exec.assert_called_once_with(self.info['port']) - self.assertTrue(mock_exec.called) + mock_get.assert_called_once_with(self.info['port']) @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) def test_management_interface_set_boot_device_ok(self, mock_exec): @@ -1810,7 +1875,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): # Missing IPMI driver_info information node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid(), - driver='fake_ipmitool') + driver=self.driver_name) with task_manager.acquire(self.context, node.uuid) as task: self.assertRaises(exception.MissingParameterValue, task.driver.management.validate, task) @@ -2046,3 +2111,154 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): ret = ipmi.send_raw(task, 'fake raw') self.assertEqual(fake_ret, ret) + + +class IPMIToolSocatDriverTestCase(IPMIToolDriverTestCase): + + def setUp(self): + super(IPMIToolSocatDriverTestCase, self).setUp(terminal="socat") + + @mock.patch.object(ipmi.IPMIConsole, '_start_console', autospec=True) + @mock.patch.object(ipmi.IPMISocatConsole, '_exec_stop_console', + autospec=True) + def test_start_console(self, mock_stop, mock_start): + mock_start.return_value = None + mock_stop.return_value = None + + with task_manager.acquire(self.context, + self.node.uuid) as task: + self.driver.console.start_console(task) + driver_info = ipmi._parse_driver_info(task.node) + mock_stop.assert_called_once_with(self.driver.console, driver_info) + mock_start.assert_called_once_with( + self.driver.console, driver_info, + console_utils.start_socat_console) + + @mock.patch.object(console_utils, 'start_socat_console', + autospec=True) + def test__start_console(self, mock_start): + mock_start.return_value = None + + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + self.driver.console._start_console( + driver_info, console_utils.start_socat_console) + + mock_start.assert_called_once_with(self.info['uuid'], + self.info['port'], + mock.ANY) + + @mock.patch.object(console_utils, 'start_socat_console', + autospec=True) + def test__start_console_fail(self, mock_start): + mock_start.side_effect = exception.ConsoleSubprocessFailed( + error='error') + + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + self.assertRaises(exception.ConsoleSubprocessFailed, + self.driver.console._start_console, + driver_info, + console_utils.start_socat_console) + + mock_start.assert_called_once_with(self.info['uuid'], + self.info['port'], + mock.ANY) + + @mock.patch.object(console_utils, 'start_socat_console', + autospec=True) + def test__start_console_fail_nodir(self, mock_start): + mock_start.side_effect = exception.ConsoleError() + + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + self.assertRaises(exception.ConsoleError, + self.driver.console._start_console, + driver_info, + console_utils.start_socat_console) + mock_start.assert_called_once_with(self.node.uuid, mock.ANY, mock.ANY) + + @mock.patch.object(console_utils, 'make_persistent_password_file', + autospec=True) + @mock.patch.object(console_utils, 'start_socat_console', + autospec=True) + def test__start_console_empty_password(self, mock_start, mock_pass): + driver_info = self.node.driver_info + del driver_info['ipmi_password'] + self.node.driver_info = driver_info + self.node.save() + + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + self.driver.console._start_console( + driver_info, console_utils.start_socat_console) + + mock_pass.assert_called_once_with(mock.ANY, '\0') + mock_start.assert_called_once_with(self.info['uuid'], + self.info['port'], + mock.ANY) + + @mock.patch.object(ipmi.IPMISocatConsole, '_exec_stop_console', + autospec=True) + @mock.patch.object(console_utils, 'stop_socat_console', + autospec=True) + def test_stop_console(self, mock_stop, mock_exec_stop): + mock_stop.return_value = None + + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + self.driver.console.stop_console(task) + + mock_stop.assert_called_once_with(self.info['uuid']) + mock_exec_stop.assert_called_once_with(self.driver.console, + driver_info) + + @mock.patch.object(ipmi.IPMISocatConsole, '_exec_stop_console', + autospec=True) + @mock.patch.object(ironic_utils, 'unlink_without_raise', + autospec=True) + @mock.patch.object(console_utils, 'stop_socat_console', + autospec=True) + def test_stop_console_fail(self, mock_stop, mock_unlink, mock_exec_stop): + mock_stop.side_effect = exception.ConsoleError() + + with task_manager.acquire(self.context, + self.node.uuid) as task: + self.assertRaises(exception.ConsoleError, + self.driver.console.stop_console, + task) + + mock_stop.assert_called_once_with(self.node.uuid) + mock_unlink.assert_called_once_with( + ipmi._console_pwfile_path(self.node.uuid)) + self.assertFalse(mock_exec_stop.call_count) + + @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) + def test__exec_stop_console(self, mock_exec): + with task_manager.acquire(self.context, + self.node.uuid) as task: + + driver_info = ipmi._parse_driver_info(task.node) + self.driver.console._exec_stop_console(driver_info) + + mock_exec.assert_called_once_with( + driver_info, 'sol deactivate', check_exit_code=[0, 1]) + + @mock.patch.object(console_utils, 'get_socat_console_url', + autospec=True) + def test_get_console(self, mock_get_url): + url = 'tcp://localhost:4201' + mock_get_url.return_value = url + expected = {'type': 'socat', 'url': url} + + with task_manager.acquire(self.context, + self.node.uuid) as task: + console_info = self.driver.console.get_console(task) + + self.assertEqual(expected, console_info) + mock_get_url.assert_called_once_with(self.info['port']) diff --git a/ironic/tests/unit/drivers/test_agent.py b/ironic/tests/unit/drivers/test_agent.py index 1a1439282c..23cdb87515 100644 --- a/ironic/tests/unit/drivers/test_agent.py +++ b/ironic/tests/unit/drivers/test_agent.py @@ -25,8 +25,45 @@ from ironic.drivers.modules import agent as agent_module from ironic.drivers.modules.amt import management as amt_management from ironic.drivers.modules.amt import power as amt_power from ironic.drivers.modules import iboot +from ironic.drivers.modules import ipmitool from ironic.drivers.modules import pxe from ironic.drivers.modules import wol +from ironic.drivers import utils +from ironic.tests import base + + +class AgentAndIPMIToolDriverTestCase(base.TestCase): + + def test___init__(self): + driver = agent.AgentAndIPMIToolDriver() + + self.assertIsInstance(driver.power, ipmitool.IPMIPower) + self.assertIsInstance(driver.console, ipmitool.IPMIShellinaboxConsole) + self.assertIsInstance(driver.boot, pxe.PXEBoot) + self.assertIsInstance(driver.deploy, agent_module.AgentDeploy) + self.assertIsInstance(driver.management, ipmitool.IPMIManagement) + self.assertIsInstance(driver.agent_vendor, + agent_module.AgentVendorInterface) + self.assertIsInstance(driver.ipmi_vendor, ipmitool.VendorPassthru) + self.assertIsInstance(driver.vendor, utils.MixinVendorInterface) + self.assertIsInstance(driver.raid, agent_module.AgentRAID) + + +class AgentAndIPMIToolAndSocatDriverTestCase(base.TestCase): + + def test___init__(self): + driver = agent.AgentAndIPMIToolAndSocatDriver() + + self.assertIsInstance(driver.power, ipmitool.IPMIPower) + self.assertIsInstance(driver.console, ipmitool.IPMISocatConsole) + self.assertIsInstance(driver.boot, pxe.PXEBoot) + self.assertIsInstance(driver.deploy, agent_module.AgentDeploy) + self.assertIsInstance(driver.management, ipmitool.IPMIManagement) + self.assertIsInstance(driver.agent_vendor, + agent_module.AgentVendorInterface) + self.assertIsInstance(driver.ipmi_vendor, ipmitool.VendorPassthru) + self.assertIsInstance(driver.vendor, utils.MixinVendorInterface) + self.assertIsInstance(driver.raid, agent_module.AgentRAID) class AgentAndAMTDriverTestCase(testtools.TestCase): diff --git a/ironic/tests/unit/drivers/test_pxe.py b/ironic/tests/unit/drivers/test_pxe.py index 1039e7a6b5..13ec8144c0 100644 --- a/ironic/tests/unit/drivers/test_pxe.py +++ b/ironic/tests/unit/drivers/test_pxe.py @@ -65,6 +65,19 @@ class PXEDriversTestCase(testtools.TestCase): self.assertIsInstance(driver.vendor, utils.MixinVendorInterface) self.assertIsInstance(driver.raid, agent.AgentRAID) + def test_pxe_ipmitool_socat_driver(self): + driver = pxe.PXEAndIPMIToolAndSocatDriver() + + self.assertIsInstance(driver.power, ipmitool.IPMIPower) + self.assertIsInstance(driver.console, ipmitool.IPMISocatConsole) + self.assertIsInstance(driver.boot, pxe_module.PXEBoot) + self.assertIsInstance(driver.deploy, iscsi_deploy.ISCSIDeploy) + self.assertIsInstance(driver.management, ipmitool.IPMIManagement) + self.assertIsNone(driver.inspect) + # TODO(rameshg87): Need better way of asserting the routes. + self.assertIsInstance(driver.vendor, utils.MixinVendorInterface) + self.assertIsInstance(driver.raid, agent.AgentRAID) + def test_pxe_ssh_driver(self): driver = pxe.PXEAndSSHDriver() diff --git a/releasenotes/notes/add-socat-console-ipmitool-ab4402ec976c5c96.yaml b/releasenotes/notes/add-socat-console-ipmitool-ab4402ec976c5c96.yaml new file mode 100644 index 0000000000..3fbc66ab99 --- /dev/null +++ b/releasenotes/notes/add-socat-console-ipmitool-ab4402ec976c5c96.yaml @@ -0,0 +1,5 @@ +--- +features: + - Adds support for socat-based serial console to ipmitool-based drivers. + These are available by using the agent_ipmitool_socat and + pxe_ipmitool_socat drivers. diff --git a/setup.cfg b/setup.cfg index becd3df3e0..de639b38c2 100644 --- a/setup.cfg +++ b/setup.cfg @@ -40,6 +40,7 @@ ironic.drivers = agent_iboot = ironic.drivers.agent:AgentAndIBootDriver agent_ilo = ironic.drivers.ilo:IloVirtualMediaAgentDriver agent_ipmitool = ironic.drivers.agent:AgentAndIPMIToolDriver + agent_ipmitool_socat = ironic.drivers.agent:AgentAndIPMIToolAndSocatDriver agent_irmc = ironic.drivers.irmc:IRMCVirtualMediaAgentDriver agent_pxe_oneview = ironic.drivers.oneview:AgentPXEOneViewDriver agent_pyghmi = ironic.drivers.agent:AgentAndIPMINativeDriver @@ -51,6 +52,7 @@ ironic.drivers = fake_agent = ironic.drivers.fake:FakeAgentDriver fake_inspector = ironic.drivers.fake:FakeIPMIToolInspectorDriver fake_ipmitool = ironic.drivers.fake:FakeIPMIToolDriver + fake_ipmitool_socat = ironic.drivers.fake:FakeIPMIToolSocatDriver fake_ipminative = ironic.drivers.fake:FakeIPMINativeDriver fake_ssh = ironic.drivers.fake:FakeSSHDriver fake_pxe = ironic.drivers.fake:FakePXEDriver @@ -71,6 +73,7 @@ ironic.drivers = iscsi_irmc = ironic.drivers.irmc:IRMCVirtualMediaIscsiDriver iscsi_pxe_oneview = ironic.drivers.oneview:ISCSIPXEOneViewDriver pxe_ipmitool = ironic.drivers.pxe:PXEAndIPMIToolDriver + pxe_ipmitool_socat = ironic.drivers.pxe:PXEAndIPMIToolAndSocatDriver pxe_ipminative = ironic.drivers.pxe:PXEAndIPMINativeDriver pxe_ssh = ironic.drivers.pxe:PXEAndSSHDriver pxe_vbox = ironic.drivers.pxe:PXEAndVirtualBoxDriver