diff --git a/ironic/drivers/modules/ipmi.py b/ironic/drivers/modules/ipmi.py index 94be7d4046..ac5baf2d9c 100644 --- a/ironic/drivers/modules/ipmi.py +++ b/ironic/drivers/modules/ipmi.py @@ -18,9 +18,10 @@ # under the License. """ -Baremetal IPMI power manager. +Ironic IPMI power manager. """ +import contextlib import os import stat import tempfile @@ -33,6 +34,7 @@ from ironic.common import states from ironic.common import utils from ironic.drivers import base from ironic.manager import task_manager +from ironic.openstack.common import excutils from ironic.openstack.common import jsonutils as json from ironic.openstack.common import log as logging from ironic.openstack.common import loopingcall @@ -60,14 +62,19 @@ LOG = logging.getLogger(__name__) VALID_BOOT_DEVICES = ['pxe', 'disk', 'safe', 'cdrom', 'bios'] -# TODO(deva): replace this with remove_path_on_error once that is available -# https://review.openstack.org/#/c/31030 +@contextlib.contextmanager def _make_password_file(password): - fd, path = tempfile.mkstemp() - os.fchmod(fd, stat.S_IRUSR | stat.S_IWUSR) - with os.fdopen(fd, "w") as f: - f.write(password) - return path + try: + fd, path = tempfile.mkstemp() + os.fchmod(fd, stat.S_IRUSR | stat.S_IWUSR) + with os.fdopen(fd, "w") as f: + f.write(password) + + yield path + utils.delete_if_exists(path) + except Exception: + with excutils.save_and_reraise_exception(): + utils.delete_if_exists(path) def _parse_driver_info(node): @@ -100,16 +107,13 @@ def _exec_ipmitool(driver_info, command): '-U', driver_info['username'], '-f'] - try: - pwfile = _make_password_file(driver_info['password']) - args.append(pwfile) + with _make_password_file(driver_info['password']) as pw_file: + args.append(pw_file) args.extend(command.split(" ")) out, err = utils.execute(*args, attempts=3) LOG.debug(_("ipmitool stdout: '%(out)s', stderr: '%(err)s'"), locals()) return out, err - finally: - utils.delete_if_exists(pwfile) def _power_on(driver_info): diff --git a/ironic/tests/drivers/test_ipmi.py b/ironic/tests/drivers/test_ipmi.py index 45fcb7f20c..a40348658b 100644 --- a/ironic/tests/drivers/test_ipmi.py +++ b/ironic/tests/drivers/test_ipmi.py @@ -50,16 +50,14 @@ class IPMIPrivateMethodTestCase(base.TestCase): self.info = ipmi._parse_driver_info(self.node) def test__make_password_file(self): - fakepass = 'this is a fake password' - pw_file = ipmi._make_password_file(fakepass) - try: + with ipmi._make_password_file(self.info.get('password')) as pw_file: + del_chk_pw_file = pw_file self.assertTrue(os.path.isfile(pw_file)) self.assertEqual(os.stat(pw_file)[stat.ST_MODE] & 0777, 0600) with open(pw_file, "r") as f: password = f.read() - self.assertEqual(password, fakepass) - finally: - os.unlink(pw_file) + self.assertEqual(password, self.info.get('password')) + self.assertFalse(os.path.isfile(del_chk_pw_file)) def test__parse_driver_info(self): # make sure we get back the expected things @@ -83,21 +81,20 @@ class IPMIPrivateMethodTestCase(base.TestCase): def test__exec_ipmitool(self): pw_file = '/tmp/password_file' + file_handle = open(pw_file, "w") self.mox.StubOutWithMock(ipmi, '_make_password_file') self.mox.StubOutWithMock(utils, 'execute') - self.mox.StubOutWithMock(utils, 'delete_if_exists') - ipmi._make_password_file(self.info['password']).AndReturn(pw_file) args = [ - 'ipmitool', - '-I', 'lanplus', - '-H', self.info['address'], - '-U', self.info['username'], - '-f', pw_file, - 'A', 'B', 'C', - ] - utils.execute(*args, attempts=3).AndReturn(('', '')) - utils.delete_if_exists(pw_file).AndReturn(None) + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-U', self.info['username'], + '-f', file_handle, + 'A', 'B', 'C', + ] + ipmi._make_password_file(self.info['password']).AndReturn(file_handle) + utils.execute(*args, attempts=3).AndReturn((None, None)) self.mox.ReplayAll() ipmi._exec_ipmitool(self.info, 'A B C')