From 9c2d0cdd85695a6845c359ea28e7197f86936329 Mon Sep 17 00:00:00 2001
From: Galyna Zholtkevych <gzholtkevych@mirantis.com>
Date: Fri, 3 Mar 2017 13:33:21 +0200
Subject: [PATCH] Correct failure message output when downloading

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
---
 ironic_python_agent/errors.py                  |  1 +
 ironic_python_agent/extensions/standby.py      | 12 +++++-------
 .../tests/unit/extensions/test_standby.py      | 18 ++++++++++++++++++
 ...hen-downloading-image-39f93838d1ed2928.yaml |  3 +++
 4 files changed, 27 insertions(+), 7 deletions(-)
 create mode 100644 releasenotes/notes/correction-failure-output-when-downloading-image-39f93838d1ed2928.yaml

diff --git a/ironic_python_agent/errors.py b/ironic_python_agent/errors.py
index 5536e0fd6..2c49ffc88 100644
--- 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
index 1ded90816..7f351febb 100644
--- a/ironic_python_agent/extensions/standby.py
+++ b/ironic_python_agent/extensions/standby.py
@@ -204,17 +204,15 @@ class ImageDownload(object):
                 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
index 82a090c97..71d020f0b 100644
--- a/ironic_python_agent/tests/unit/extensions/test_standby.py
+++ b/ironic_python_agent/tests/unit/extensions/test_standby.py
@@ -899,3 +899,21 @@ class TestImageDownload(test_base.BaseTestCase):
                                               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
new file mode 100644
index 000000000..5beadaf39
--- /dev/null
+++ b/releasenotes/notes/correction-failure-output-when-downloading-image-39f93838d1ed2928.yaml
@@ -0,0 +1,3 @@
+---
+fixes:
+  - Correct message output when failed to download image.