From 232ecfe6f242db861d30e46524a16f3f4a3a9c8b Mon Sep 17 00:00:00 2001 From: Andrey Shestakov Date: Wed, 31 Aug 2016 16:16:45 +0300 Subject: [PATCH] IPMI command should depend on console type This change implements _get_impi_cmd method for IPMI consoles. Depends on type of console, ipmi command should contain different arguments. Change-Id: I55e712a1a91aed3d533f636e519f0bae1f9be2d4 Closes-bug: #1611285 (cherry picked from commit 0acbf378bb543a3428856d80036d4de9c5d133a7) --- ironic/drivers/modules/ipmitool.py | 38 +++++++++--- .../unit/drivers/modules/test_ipmitool.py | 62 ++++++++++++++++++- ...d-for-impmi-consoles-2e1104f22df3efcd.yaml | 3 + 3 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/ipmi-cmd-for-impmi-consoles-2e1104f22df3efcd.yaml diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 13d8e57b5e..14e22f7ece 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -1159,6 +1159,21 @@ class IPMIConsole(base.ConsoleInterface): "Check the 'ipmi_protocol_version' parameter in " "node's driver_info")) + def _get_ipmi_cmd(self, driver_info, pw_file): + """Get ipmi command for ipmitool usage. + + :param driver_info: driver info with the ipmitool parameters + :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}) + def _start_console(self, driver_info, start_method): """Start a remote console for the node. @@ -1174,14 +1189,7 @@ class IPMIConsole(base.ConsoleInterface): path = _console_pwfile_path(driver_info['uuid']) pw_file = console_utils.make_persistent_password_file( path, driver_info['password'] or '\0') - - ipmi_cmd = ("/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s" - " -I lanplus -U %(user)s -f %(pwfile)s" - % {'uid': os.getuid(), - 'gid': os.getgid(), - 'address': driver_info['address'], - 'user': driver_info['username'], - 'pwfile': pw_file}) + ipmi_cmd = self._get_ipmi_cmd(driver_info, pw_file) for name, option in BRIDGING_OPTIONS: if driver_info[name] is not None: @@ -1201,6 +1209,20 @@ class IPMIConsole(base.ConsoleInterface): class IPMIShellinaboxConsole(IPMIConsole): """A ConsoleInterface that uses ipmitool and shellinabox.""" + def _get_ipmi_cmd(self, driver_info, pw_file): + """Get ipmi command for ipmitool usage. + + :param driver_info: driver info with the ipmitool parameters + :param pw_file: password file to be used in ipmitool command + :returns: returns a command string for ipmitool + """ + command = super(IPMIShellinaboxConsole, self)._get_ipmi_cmd( + driver_info, pw_file) + return ("/:%(uid)s:%(gid)s:HOME:%(basic_command)s" + % {'uid': os.getuid(), + 'gid': os.getgid(), + 'basic_command': command}) + @METRICS.timer('IPMIShellinaboxConsole.start_console') def start_console(self, task): """Start a remote console for the node. diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index 5b0b1d0bfd..eecdcf402a 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -1618,6 +1618,34 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, task.driver.console.validate, task) + def test__get_ipmi_cmd(self): + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + ipmi_cmd = self.driver.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" % + {'uid': os.getuid(), 'gid': os.getgid(), + 'address': driver_info['address'], + 'user': driver_info['username']}) + self.assertEqual(expected_ipmi_cmd, ipmi_cmd) + + def test__get_ipmi_cmd_without_user(self): + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + driver_info['username'] = None + ipmi_cmd = self.driver.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" % + {'uid': os.getuid(), 'gid': os.getgid(), + 'address': driver_info['address']}) + self.assertEqual(expected_ipmi_cmd, ipmi_cmd) + @mock.patch.object(ipmi.IPMIConsole, '_start_console', autospec=True) def test_start_console(self, mock_start): mock_start.return_value = None @@ -1630,9 +1658,10 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): self.driver.console, driver_info, console_utils.start_shellinabox_console) + @mock.patch.object(ipmi.IPMIConsole, '_get_ipmi_cmd', autospec=True) @mock.patch.object(console_utils, 'start_shellinabox_console', autospec=True) - def test__start_console(self, mock_start): + def test__start_console(self, mock_start, mock_ipmi_cmd): mock_start.return_value = None with task_manager.acquire(self.context, @@ -1644,6 +1673,8 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): mock_start.assert_called_once_with(self.info['uuid'], self.info['port'], mock.ANY) + mock_ipmi_cmd.assert_called_once_with(self.driver.console, + driver_info, mock.ANY) @mock.patch.object(console_utils, 'start_shellinabox_console', autospec=True) @@ -2167,6 +2198,30 @@ class IPMIToolSocatDriverTestCase(IPMIToolDriverTestCase): def setUp(self): super(IPMIToolSocatDriverTestCase, self).setUp(terminal="socat") + def test__get_ipmi_cmd(self): + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + ipmi_cmd = self.driver.console._get_ipmi_cmd(driver_info, + 'pw_file') + expected_ipmi_cmd = ("ipmitool -H %(address)s -I lanplus " + "-U %(user)s -f pw_file" % + {'address': driver_info['address'], + 'user': driver_info['username']}) + self.assertEqual(expected_ipmi_cmd, ipmi_cmd) + + def test__get_ipmi_cmd_without_user(self): + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + driver_info['username'] = None + ipmi_cmd = self.driver.console._get_ipmi_cmd(driver_info, + 'pw_file') + expected_ipmi_cmd = ("ipmitool -H %(address)s -I lanplus " + "-f pw_file" % + {'address': driver_info['address']}) + self.assertEqual(expected_ipmi_cmd, ipmi_cmd) + @mock.patch.object(ipmi.IPMIConsole, '_start_console', autospec=True) @mock.patch.object(ipmi.IPMISocatConsole, '_exec_stop_console', autospec=True) @@ -2183,9 +2238,10 @@ class IPMIToolSocatDriverTestCase(IPMIToolDriverTestCase): self.driver.console, driver_info, console_utils.start_socat_console) + @mock.patch.object(ipmi.IPMISocatConsole, '_get_ipmi_cmd', autospec=True) @mock.patch.object(console_utils, 'start_socat_console', autospec=True) - def test__start_console(self, mock_start): + def test__start_console(self, mock_start, mock_ipmi_cmd): mock_start.return_value = None with task_manager.acquire(self.context, @@ -2197,6 +2253,8 @@ class IPMIToolSocatDriverTestCase(IPMIToolDriverTestCase): mock_start.assert_called_once_with(self.info['uuid'], self.info['port'], mock.ANY) + mock_ipmi_cmd.assert_called_once_with(self.driver.console, + driver_info, mock.ANY) @mock.patch.object(console_utils, 'start_socat_console', autospec=True) diff --git a/releasenotes/notes/ipmi-cmd-for-impmi-consoles-2e1104f22df3efcd.yaml b/releasenotes/notes/ipmi-cmd-for-impmi-consoles-2e1104f22df3efcd.yaml new file mode 100644 index 0000000000..adbb853619 --- /dev/null +++ b/releasenotes/notes/ipmi-cmd-for-impmi-consoles-2e1104f22df3efcd.yaml @@ -0,0 +1,3 @@ +fixes: + - Fixes a bug with incorrect base socat command, + which prevented the usage of console.