Correct failure message output when downloading

Galyna Zholtkevych · galynazholtkevych · commit 9c2d0cdd8569 · 2017-03-10T19:16:07.000Z
This fixes unreadable output on download image failure.
Adding new instance variable to exception `ImageDownloadError` class
to avoid redundant logs.

Change-Id: I51782abd572588adfc62745eeab9c559eb8346dd
Closes-Bug: #1657691
diff --git a/ironic_python_agent/errors.py b/ironic_python_agent/errors.py
@@ -143,6 +143,7 @@ class ImageDownloadError(RESTError):
 
     def __init__(self, image_id, msg):
         details = 'Download of image id {} failed: {}'.format(image_id, msg)
+        self.secondary_message = msg
         super(ImageDownloadError, self).__init__(details)
 
 
diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py
@@ -204,17 +204,15 @@ def __init__(self, image_info, time_obj=None):
                 failtime = time.time() - self._time
                 log_msg = ('URL: {}; time: {} '
                            'seconds. Error: {}').format(
-                    url, failtime, e.details)
-                LOG.warning('Image download failed. %s', log_msg)
-                details += log_msg
+                    url, failtime, e.secondary_message)
+                LOG.warning(log_msg)
+                details.append(log_msg)
                 continue
             else:
                 break
         else:
-            details = '/n'.join(details)
-            msg = ('Image download failed for all URLs with following errors: '
-                   '{}'.format(details))
-            raise errors.ImageDownloadError(image_info['id'], msg)
+            details = '\n '.join(details)
+            raise errors.ImageDownloadError(image_info['id'], details)
 
     def _download_file(self, image_info, url):
         """Opens a download stream for the given URL.
diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py
@@ -899,3 +899,21 @@ def test_download_image(self, requests_mock, md5_mock):
                                               cert=None, verify=True,
                                               stream=True, proxies={})
         self.assertEqual(image_info['checksum'], image_download.md5sum())
+
+    @mock.patch('time.time', autospec=True)
+    @mock.patch('requests.get', autospec=True)
+    def test_download_image_fail(self, requests_mock, time_mock):
+        response = requests_mock.return_value
+        response.status_code = 401
+        response.text = 'Unauthorized'
+        time_mock.return_value = 0.0
+        image_info = _build_fake_image_info()
+        msg = ('Error downloading image: Download of image id fake_id failed: '
+               'URL: http://example.org; time: 0.0 seconds. Error: '
+               'Received status code 401 from http://example.org, expected '
+               '200. Response body: Unauthorized')
+        self.assertRaisesRegexp(errors.ImageDownloadError, msg,
+                                standby.ImageDownload, image_info)
+        requests_mock.assert_called_once_with(image_info['urls'][0],
+                                              cert=None, verify=True,
+                                              stream=True, proxies={})
diff --git a/releasenotes/notes/correction-failure-output-when-downloading-image-39f93838d1ed2928.yaml b/releasenotes/notes/correction-failure-output-when-downloading-image-39f93838d1ed2928.yaml
@@ -0,0 +1,3 @@
+---
+fixes:
+  - Correct message output when failed to download image.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+---`
	`2`	`+fixes:`
	`3`	`+ - Correct message output when failed to download image.`