diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 39b52a016e..d759e8b75a 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -404,6 +404,43 @@ def _exec_ipmitool_wait(timeout, driver_info, popen_obj): {'node': driver_info['uuid'], 'cmd': popen_obj.cmd}) +def _get_ipmitool_args(driver_info, pw_file=None): + ipmi_version = ('lanplus' + if driver_info['protocol_version'] == '2.0' + else 'lan') + + args = ['ipmitool', + '-I', ipmi_version, + '-H', 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']) + + for name, option in BRIDGING_OPTIONS: + if driver_info[name] is not None: + args.append(option) + args.append(driver_info[name]) + + if pw_file: + args.append('-f') + args.append(pw_file) + + if CONF.debug: + args.append('-v') + + # ensure all arguments are strings + args = [str(arg) for arg in args] + + return args + + def _exec_ipmitool(driver_info, command, check_exit_code=None, kill_on_timeout=False): """Execute the ipmitool command. @@ -420,29 +457,7 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None, :raises: processutils.ProcessExecutionError from executing the command. """ - ipmi_version = ('lanplus' - if driver_info['protocol_version'] == '2.0' - else 'lan') - args = ['ipmitool', - '-I', - ipmi_version, - '-H', - 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']) - - for name, option in BRIDGING_OPTIONS: - if driver_info[name] is not None: - args.append(option) - args.append(driver_info[name]) + args = _get_ipmitool_args(driver_info) timeout = CONF.ipmi.command_retry_timeout @@ -1277,13 +1292,7 @@ class IPMIConsole(base.ConsoleInterface): :param pw_file: password file to be used in ipmitool command :returns: returns a command string for ipmitool """ - user = driver_info.get('username') - user = ' -U {}'.format(user) if user else '' - return ("ipmitool -H %(address)s -I lanplus" - "%(user)s -f %(pwfile)s" - % {'address': driver_info['address'], - 'user': user, - 'pwfile': pw_file}) + return ' '.join(_get_ipmitool_args(driver_info, pw_file=pw_file)) def _start_console(self, driver_info, start_method): """Start a remote console for the node. @@ -1301,15 +1310,8 @@ class IPMIConsole(base.ConsoleInterface): pw_file = console_utils.make_persistent_password_file( path, driver_info['password'] or '\0') ipmi_cmd = self._get_ipmi_cmd(driver_info, pw_file) + ipmi_cmd += ' sol activate' - for name, option in BRIDGING_OPTIONS: - if driver_info[name] is not None: - ipmi_cmd = " ".join([ipmi_cmd, - option, driver_info[name]]) - - if CONF.debug: - ipmi_cmd += " -v" - ipmi_cmd += " sol activate" try: start_method(driver_info['uuid'], driver_info['port'], ipmi_cmd) except (exception.ConsoleError, exception.ConsoleSubprocessFailed): diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index 6712e7b7bb..2ba10090da 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -822,6 +822,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -847,6 +848,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ], [ @@ -855,6 +857,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'D', 'E', 'F', ]] @@ -884,6 +887,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ], [ @@ -892,6 +896,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'D', 'E', 'F', ]] @@ -923,6 +928,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ], [ @@ -931,6 +937,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', '127.127.127.127', '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'D', 'E', 'F', ]] @@ -959,6 +966,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -982,6 +990,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-R', '12', '-N', '5', '-f', awesome_password_filename, @@ -1017,6 +1026,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-I', 'lanplus', '-H', self.info['address'], '-L', self.info['priv_level'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -1040,6 +1050,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-I', 'lanplus', '-H', self.info['address'], '-L', self.info['priv_level'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -1066,6 +1077,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -1093,6 +1105,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -1127,6 +1140,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-T', info['transit_address'], '-b', info['target_channel'], '-t', info['target_address'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -1164,6 +1178,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-m', info['local_address'], '-b', info['target_channel'], '-t', info['target_address'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -1187,6 +1202,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -1213,6 +1229,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -1236,6 +1253,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-L', self.info['priv_level'], '-p', '1623', '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -1260,6 +1278,7 @@ class IPMIToolPrivateMethodTestCase(Base): '-H', self.info['address'], '-L', self.info['priv_level'], '-U', self.info['username'], + '-v', '-f', awesome_password_filename, 'A', 'B', 'C', ] @@ -2461,8 +2480,8 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase): driver_info = ipmi._parse_driver_info(task.node) ipmi_cmd = self.console._get_ipmi_cmd(driver_info, 'pw_file') expected_ipmi_cmd = ("/:%(uid)s:%(gid)s:HOME:ipmitool " - "-H %(address)s -I lanplus -U %(user)s " - "-f pw_file" % + "-I lanplus -H %(address)s -L ADMINISTRATOR " + "-U %(user)s -f pw_file -v" % {'uid': os.getuid(), 'gid': os.getgid(), 'address': driver_info['address'], 'user': driver_info['username']}) @@ -2475,8 +2494,8 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase): driver_info['username'] = None ipmi_cmd = self.console._get_ipmi_cmd(driver_info, 'pw_file') expected_ipmi_cmd = ("/:%(uid)s:%(gid)s:HOME:ipmitool " - "-H %(address)s -I lanplus " - "-f pw_file" % + "-I lanplus -H %(address)s -L ADMINISTRATOR " + "-f pw_file -v" % {'uid': os.getuid(), 'gid': os.getgid(), 'address': driver_info['address']}) self.assertEqual(expected_ipmi_cmd, ipmi_cmd) @@ -2608,8 +2627,9 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase): self.node.uuid) as task: driver_info = ipmi._parse_driver_info(task.node) ipmi_cmd = self.console._get_ipmi_cmd(driver_info, 'pw_file') - expected_ipmi_cmd = ("ipmitool -H %(address)s -I lanplus " - "-U %(user)s -f pw_file" % + expected_ipmi_cmd = ("ipmitool -I lanplus -H %(address)s " + "-L ADMINISTRATOR -U %(user)s " + "-f pw_file -v" % {'address': driver_info['address'], 'user': driver_info['username']}) self.assertEqual(expected_ipmi_cmd, ipmi_cmd) @@ -2620,8 +2640,9 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase): driver_info = ipmi._parse_driver_info(task.node) driver_info['username'] = None ipmi_cmd = self.console._get_ipmi_cmd(driver_info, 'pw_file') - expected_ipmi_cmd = ("ipmitool -H %(address)s -I lanplus " - "-f pw_file" % + expected_ipmi_cmd = ("ipmitool -I lanplus -H %(address)s " + "-L ADMINISTRATOR " + "-f pw_file -v" % {'address': driver_info['address']}) self.assertEqual(expected_ipmi_cmd, ipmi_cmd) diff --git a/releasenotes/notes/ipmi-console-port-ec6348df4eee6746.yaml b/releasenotes/notes/ipmi-console-port-ec6348df4eee6746.yaml new file mode 100644 index 0000000000..969b261963 --- /dev/null +++ b/releasenotes/notes/ipmi-console-port-ec6348df4eee6746.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes the IPMI console implementation to respect all supported IPMI + ``driver_info`` and configuration options, particularly ``ipmi_port``.