Update unittests and use NamedTemporaryFile

Update the unittests for _make_password_file to text exception handling.
Also switch to using tempfile.NamedTemporaryFile() to create the
temporary file.

The _make_password_file function was corrected so that if an exception
occurred during the 'yield' portion, it would not catch the error and
convert it into a PasswordFileFailedToCreate exception.  The unit tests
are being updated to test this new behavior and also test for when it
should raise a PasswordFileFailedToCreate exception.

Also it was recommended to use tempfile.NamedTemporaryFile() instead of
tempfile.mkstemp()

Partial-Bug: 1428722
Change-Id: Iedb6baa97133d2aa8f88a4aaf2945403766d7fd5

ironic/drivers/modules/ipmitool.py

@@ -182,17 +182,29 @@ def _make_password_file(password):
     :raises: PasswordFileFailedToCreate from creating or writing to the
              temporary file
     """
+    f = None
     try:
-        fd, path = tempfile.mkstemp()
-        with os.fdopen(fd, "w") as f:
-            f.write(str(password))
-    except Exception as exc:
-        utils.delete_if_exists(path)
+        f = tempfile.NamedTemporaryFile(mode='w', dir=CONF.tempdir)
+        f.write(str(password))
+        f.flush()
+    except (IOError, OSError) as exc:
+        if f is not None:
+            f.close()
         raise exception.PasswordFileFailedToCreate(error=exc)
+    except Exception:
+        if f is not None:
+            f.close()
+        raise
+
     try:
-        yield path
+        # NOTE(jlvillal): This yield can not be in the try/except block above
+        # because an exception by the caller of this function would then get
+        # changed to a PasswordFileFailedToCreate exception which would mislead
+        # about the problem and its cause.
+        yield f.name
     finally:
-        utils.delete_if_exists(path)
+        if f is not None:
+            f.close()
 
 
 def _parse_driver_info(node):

ironic/tests/drivers/test_ipmitool.py

@@ -223,15 +223,21 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase):
                 driver_info=INFO_DICT)
         self.info = ipmi._parse_driver_info(self.node)
 
-    def _test__make_password_file(self, mock_sleep, input_password):
-        with ipmi._make_password_file(input_password) as pw_file:
-            del_chk_pw_file = pw_file
-            self.assertTrue(os.path.isfile(pw_file))
-            self.assertEqual(0o600, os.stat(pw_file)[stat.ST_MODE] & 0o777)
-            with open(pw_file, "r") as f:
-                password = f.read()
-            self.assertEqual(str(input_password), password)
-        self.assertFalse(os.path.isfile(del_chk_pw_file))
+    def _test__make_password_file(self, mock_sleep, input_password,
+                                  exception_to_raise=None):
+        pw_file = None
+        try:
+            with ipmi._make_password_file(input_password) as pw_file:
+                if exception_to_raise is not None:
+                    raise exception_to_raise
+                self.assertTrue(os.path.isfile(pw_file))
+                self.assertEqual(0o600, os.stat(pw_file)[stat.ST_MODE] & 0o777)
+                with open(pw_file, "r") as f:
+                    password = f.read()
+                self.assertEqual(str(input_password), password)
+        finally:
+            if pw_file is not None:
+                self.assertFalse(os.path.isfile(pw_file))
 
     def test__make_password_file_str_password(self, mock_sleep):
         self._test__make_password_file(mock_sleep, self.info.get('password'))
@@ -239,6 +245,44 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase):
     def test__make_password_file_with_numeric_password(self, mock_sleep):
         self._test__make_password_file(mock_sleep, 12345)
 
+    def test__make_password_file_caller_exception(self, mock_sleep):
+        # Test caller raising exception
+        result = self.assertRaises(
+            ValueError,
+            self._test__make_password_file,
+            mock_sleep, 12345, ValueError('we should fail'))
+        self.assertEqual(result.message, 'we should fail')
+
+    @mock.patch.object(tempfile, 'NamedTemporaryFile',
+                       new=mock.MagicMock(side_effect=OSError('Test Error')))
+    def test__make_password_file_tempfile_known_exception(self, mock_sleep):
+        # Test OSError exception in _make_password_file for
+        # tempfile.NamedTemporaryFile
+        self.assertRaises(
+            exception.PasswordFileFailedToCreate,
+            self._test__make_password_file, mock_sleep, 12345)
+
+    @mock.patch.object(
+        tempfile, 'NamedTemporaryFile',
+        new=mock.MagicMock(side_effect=OverflowError('Test Error')))
+    def test__make_password_file_tempfile_unknown_exception(self, mock_sleep):
+        # Test exception in _make_password_file for tempfile.NamedTemporaryFile
+        result = self.assertRaises(
+            OverflowError,
+            self._test__make_password_file, mock_sleep, 12345)
+        self.assertEqual(result.message, 'Test Error')
+
+    def test__make_password_file_write_exception(self, mock_sleep):
+        # Test exception in _make_password_file for write()
+        mock_namedtemp = mock.mock_open(mock.MagicMock(name='JLV'))
+        with mock.patch('tempfile.NamedTemporaryFile', mock_namedtemp):
+            mock_filehandle = mock_namedtemp.return_value
+            mock_write = mock_filehandle.write
+            mock_write.side_effect = OSError('Test 2 Error')
+            self.assertRaises(
+                exception.PasswordFileFailedToCreate,
+                self._test__make_password_file, mock_sleep, 12345)
+
     def test__parse_driver_info(self, mock_sleep):
         # make sure we get back the expected things
         _OPTIONS = ['address', 'username', 'password', 'uuid']