Merge "honor ipmi_port in serial console drivers"

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'
+    args = _get_ipmitool_args(driver_info)
-                    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])
 
     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')
+        return ' '.join(_get_ipmitool_args(driver_info, pw_file=pw_file))
-        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})
 
     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):

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 "
+                                 "-I lanplus -H %(address)s -L ADMINISTRATOR "
-                                 "-f pw_file" %
+                                 "-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 "
+                                 "-I lanplus -H %(address)s -L ADMINISTRATOR "
-                                 "-f pw_file" %
+                                 "-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 "
+            expected_ipmi_cmd = ("ipmitool -I lanplus -H %(address)s "
-                                 "-U %(user)s -f pw_file" %
+                                 "-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 "
+            expected_ipmi_cmd = ("ipmitool -I lanplus -H %(address)s "
-                                 "-f pw_file" %
+                                 "-L ADMINISTRATOR "
+                                 "-f pw_file -v" %
                                  {'address': driver_info['address']})
         self.assertEqual(expected_ipmi_cmd, ipmi_cmd)
 

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