diff --git a/doc/source/deploy/install-guide.rst b/doc/source/deploy/install-guide.rst index 981606719a..07882a3906 100644 --- a/doc/source/deploy/install-guide.rst +++ b/doc/source/deploy/install-guide.rst @@ -1460,6 +1460,12 @@ and may be combined if desired. driver_info/ipmi_password=$PASS \ driver_info/ipmi_address=$ADDRESS +.. note:: + If IPMI is running on a port other than 623 (the default). The port must + be added to ``driver_info`` by specifying the ``ipmi_port`` value. + Example: + ironic node-update $NODE_UUID add driver_info/ipmi_port=$PORT_NUMBER + Note that you may also specify all ``driver_info`` parameters during ``node-create`` by passing the **-i** option multiple times. diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 87d07a0a37..3693bd09ea 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -76,6 +76,7 @@ REQUIRED_PROPERTIES = { } OPTIONAL_PROPERTIES = { 'ipmi_password': _("password. Optional."), + 'ipmi_port': _("remote IPMI RMCP port. Optional."), 'ipmi_priv_level': _("privilege level; default is ADMINISTRATOR. One of " "%s. Optional.") % ', '.join(VALID_PRIV_LEVELS), 'ipmi_username': _("username; default is NULL user. Optional."), @@ -255,6 +256,7 @@ def _parse_driver_info(node): address = info.get('ipmi_address') username = info.get('ipmi_username') password = six.text_type(info.get('ipmi_password', '')) + dest_port = info.get('ipmi_port') port = info.get('ipmi_terminal_port') priv_level = info.get('ipmi_priv_level', 'ADMINISTRATOR') bridging_type = info.get('ipmi_bridging', 'no') @@ -276,6 +278,9 @@ def _parse_driver_info(node): if port is not None: port = utils.validate_network_port(port, 'ipmi_terminal_port') + if dest_port is not None: + dest_port = utils.validate_network_port(dest_port, 'ipmi_port') + # check if ipmi_bridging has proper value if bridging_type == 'no': # if bridging is not selected, then set all bridging params to None @@ -325,6 +330,7 @@ def _parse_driver_info(node): return { 'address': address, + 'dest_port': dest_port, 'username': username, 'password': password, 'port': port, @@ -361,6 +367,11 @@ def _exec_ipmitool(driver_info, command): driver_info['address'], '-L', driver_info['priv_level'] ] + + if driver_info['dest_port']: + args.append('-p') + args.append(driver_info['dest_port']) + if driver_info['username']: args.append('-U') args.append(driver_info['username']) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 5644e20e20..9311864e8c 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -3309,7 +3309,7 @@ class ManagerTestProperties(tests_db_base.DbTestCase): def test_driver_properties_fake_ipmitool(self): expected = ['ipmi_address', 'ipmi_terminal_port', - 'ipmi_password', 'ipmi_priv_level', + 'ipmi_password', 'ipmi_port', 'ipmi_priv_level', 'ipmi_username', 'ipmi_bridging', 'ipmi_transit_channel', 'ipmi_transit_address', 'ipmi_target_channel', 'ipmi_target_address', @@ -3346,7 +3346,7 @@ class ManagerTestProperties(tests_db_base.DbTestCase): def test_driver_properties_pxe_ipmitool(self): expected = ['ipmi_address', 'ipmi_terminal_port', - 'ipmi_password', 'ipmi_priv_level', + 'ipmi_password', 'ipmi_port', 'ipmi_priv_level', 'ipmi_username', 'ipmi_bridging', 'ipmi_transit_channel', 'ipmi_transit_address', 'ipmi_target_channel', 'ipmi_target_address', 'ipmi_local_address', diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index 023df1bd81..d2ad41a87c 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -603,6 +603,20 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, ipmi._parse_driver_info, node) + def test__parse_driver_info_invalid_ipmi_port(self, mock_sleep): + info = dict(INFO_DICT) + info['ipmi_port'] = '700000' + node = obj_utils.get_test_node(self.context, driver_info=info) + self.assertRaises(exception.InvalidParameterValue, + ipmi._parse_driver_info, node) + + def test__parse_driver_info_ipmi_port_valid(self, mock_sleep): + info = dict(INFO_DICT) + info['ipmi_port'] = '623' + node = obj_utils.get_test_node(self.context, driver_info=info) + ret = ipmi._parse_driver_info(node) + self.assertEqual(623, ret['dest_port']) + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @mock.patch.object(ipmi, '_make_password_file', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) @@ -1069,6 +1083,38 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): self.assertTrue(mock_pwf.called) mock_exec.assert_called_once_with(*args) + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_make_password_file', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_port(self, mock_exec, mock_pwf, mock_support, + mock_sleep): + self.info['dest_port'] = '1623' + ipmi.LAST_CMD_TIME = {} + pw_file_handle = tempfile.NamedTemporaryFile() + pw_file = pw_file_handle.name + file_handle = open(pw_file, "w") + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-p', '1623', + '-U', self.info['username'], + '-f', file_handle, + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_pwf.return_value = file_handle + mock_exec.return_value = (None, None) + + ipmi._exec_ipmitool(self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + mock_pwf.assert_called_once_with(self.info['password']) + mock_exec.assert_called_once_with(*args) + self.assertFalse(mock_sleep.called) + @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) def test__power_status_on(self, mock_exec, mock_sleep): mock_exec.return_value = ["Chassis Power is on\n", None] diff --git a/releasenotes/notes/Add-port-option-support-to-ipmitool-e125d07fe13c53e7.yaml b/releasenotes/notes/Add-port-option-support-to-ipmitool-e125d07fe13c53e7.yaml new file mode 100644 index 0000000000..dbfb348447 --- /dev/null +++ b/releasenotes/notes/Add-port-option-support-to-ipmitool-e125d07fe13c53e7.yaml @@ -0,0 +1,5 @@ +--- +features: + - Add support for ipmitool's port (-p) option. This + allows ipmitool support for operators that do not + use the default port (623) as their IPMI port.