Merge "IPMITool: add IPMISocatConsole and IPMIConsole class"

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.
 

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."""
 

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):
+class IPMIConsole(base.ConsoleInterface):
-    """A ConsoleInterface that uses ipmitool and shellinabox."""
+    """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'],
+            start_method(driver_info['uuid'], driver_info['port'], ipmi_cmd)
-                                                    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}

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.
 

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_pass,
-                                                 mock_sleep):
+                                                 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,
+    def test__exec_ipmitool_with_port(self, mock_exec, mock_pass,
-                                      mock_sleep):
+                                      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")
+        if terminal is None:
-        self.driver = driver_factory.get_driver("fake_ipmitool")
+            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',
+    @mock.patch.object(ipmi.IPMIConsole, '_start_console', autospec=True)
-                       autospec=True)
+    def test_start_console(self, mock_start):
-    def test_start_console(self, mock_exec):
+        mock_start.return_value = None
-        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()
 
         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,
+                              self.driver.console._start_console,
-                              task)
+                              driver_info,
-        mock_exec.assert_called_once_with(self.node.uuid, mock.ANY, mock.ANY)
+                              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'],
+        mock_start.assert_called_once_with(self.info['uuid'],
-                                          self.info['port'],
+                                           self.info['port'],
-                                          mock.ANY)
+                                           mock.ANY)
 
     @mock.patch.object(console_utils, 'stop_shellinabox_console',
                        autospec=True)
-    def test_stop_console(self, mock_exec):
+    def test_stop_console(self, mock_stop):
-        mock_exec.return_value = None
+        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'])
+        mock_stop.assert_called_once_with(self.info['uuid'])
-        self.assertTrue(mock_exec.called)
 
     @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'])
+        mock_get.assert_called_once_with(self.info['port'])
-        self.assertTrue(mock_exec.called)
 
     @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'])

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

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

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.

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