Notify error not called on upload errors in V2

The 'upload' method in glance/api/v2/image_data.py catches a couple of
exceptions, but does not send notifications for any of them. They should
also be sending notifications as is being done in v1.

Fixes: Bug #1217837
Change-Id: Ie38307f514c1716dce31403d9ef57eae8290450d

glance/notifier/__init__.py

@@ -20,6 +20,7 @@
 import uuid
 
 from oslo.config import cfg
+import webob
 
 from glance.common import exception
 import glance.domain
@@ -200,11 +201,47 @@ def set_data(self, data, size=None):
         try:
             self.image.set_data(data, size)
         except exception.StorageFull as e:
-            msg = _("Image storage media is full: %s") % e
+            msg = (_("Image storage media is full: %s") % e)
             self.notifier.error('image.upload', msg)
+            raise webob.exc.HTTPRequestEntityTooLarge(explanation=msg)
         except exception.StorageWriteDenied as e:
-            msg = _("Insufficient permissions on image storage media: %s") % e
+            msg = (_("Insufficient permissions on image storage media: %s")
+                   % e)
             self.notifier.error('image.upload', msg)
+            raise webob.exc.HTTPServiceUnavailable(explanation=msg)
+        except ValueError as e:
+            msg = (_("Cannot save data for image %s: %s")
+                   % (self.image.image_id, e))
+            self.notifier.error('image.upload', msg)
+            raise webob.exc.HTTPBadRequest(explanation=unicode(e))
+        except exception.Duplicate as e:
+            msg = (_("Unable to upload duplicate image data for image %s: %s")
+                   % (self.image.image_id, e))
+            self.notifier.error('image.upload', msg)
+            raise webob.exc.HTTPConflict(explanation=msg)
+        except exception.Forbidden as e:
+            msg = (_("Not allowed to upload image data for image %s: %s")
+                   % (self.image.image_id, e))
+            self.notifier.error('image.upload', msg)
+            raise webob.exc.HTTPForbidden(explanation=msg)
+        except exception.NotFound as e:
+            msg = (_("Image %s could not be found after upload. The image may "
+                     "have been deleted during the upload: %s")
+                   % (self.image.image_id, e))
+            self.notifier.error('image.upload', msg)
+            raise webob.exc.HTTPNotFound(explanation=unicode(e))
+        except webob.exc.HTTPError as e:
+            msg = (_("Failed to upload image data for image %s"
+                     " due to HTTP error: %s")
+                   % (self.image.image_id, e))
+            self.notifier.error('image.upload', msg)
+            raise
+        except Exception as e:
+            msg = (_("Failed to upload image data for image %s "
+                     "due to internal error: %s")
+                   % (self.image.image_id, e))
+            self.notifier.error('image.upload', msg)
+            raise
         else:
             payload = format_image_notification(self.image)
             self.notifier.info('image.upload', payload)

glance/tests/unit/test_notifier.py

@@ -21,6 +21,7 @@
 import qpid
 import qpid.messaging
 import stubout
+import webob
 
 from glance.common import exception
 import glance.context
@@ -590,8 +591,8 @@ def data_iterator():
             yield 'abcde'
             raise exception.StorageFull('Modern Major General')
 
-        self.image_proxy.set_data(data_iterator(), 10)
-
+        self.assertRaises(webob.exc.HTTPRequestEntityTooLarge,
+                          self.image_proxy.set_data, data_iterator(), 10)
         output_logs = self.notifier.get_logs()
         self.assertEqual(len(output_logs), 1)
 
@@ -600,13 +601,48 @@ def data_iterator():
         self.assertEqual(output_log['event_type'], 'image.upload')
         self.assertTrue('Modern Major General' in output_log['payload'])
 
+    def test_image_set_data_value_error(self):
+        def data_iterator():
+            self.notifier.log = []
+            yield 'abcde'
+            raise ValueError('value wrong')
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.image_proxy.set_data, data_iterator(), 10)
+
+        output_logs = self.notifier.get_logs()
+        self.assertEqual(len(output_logs), 1)
+
+        output_log = output_logs[0]
+        self.assertEqual(output_log['notification_type'], 'ERROR')
+        self.assertEqual(output_log['event_type'], 'image.upload')
+        self.assertTrue('value wrong' in output_log['payload'])
+
+    def test_image_set_data_duplicate(self):
+        def data_iterator():
+            self.notifier.log = []
+            yield 'abcde'
+            raise exception.Duplicate('Cant have duplicates')
+
+        self.assertRaises(webob.exc.HTTPConflict,
+                          self.image_proxy.set_data, data_iterator(), 10)
+
+        output_logs = self.notifier.get_logs()
+        self.assertEqual(len(output_logs), 1)
+
+        output_log = output_logs[0]
+        self.assertEqual(output_log['notification_type'], 'ERROR')
+        self.assertEqual(output_log['event_type'], 'image.upload')
+        self.assertTrue('Cant have duplicates' in output_log['payload'])
+
     def test_image_set_data_storage_write_denied(self):
         def data_iterator():
             self.notifier.log = []
             yield 'abcde'
             raise exception.StorageWriteDenied('The Very Model')
 
-        self.image_proxy.set_data(data_iterator(), 10)
+        self.assertRaises(webob.exc.HTTPServiceUnavailable,
+                          self.image_proxy.set_data, data_iterator(), 10)
 
         output_logs = self.notifier.get_logs()
         self.assertEqual(len(output_logs), 1)
@@ -615,3 +651,71 @@ def data_iterator():
         self.assertEqual(output_log['notification_type'], 'ERROR')
         self.assertEqual(output_log['event_type'], 'image.upload')
         self.assertTrue('The Very Model' in output_log['payload'])
+
+    def test_image_set_data_forbidden(self):
+        def data_iterator():
+            self.notifier.log = []
+            yield 'abcde'
+            raise exception.Forbidden('Not allowed')
+
+        self.assertRaises(webob.exc.HTTPForbidden,
+                          self.image_proxy.set_data, data_iterator(), 10)
+
+        output_logs = self.notifier.get_logs()
+        self.assertEqual(len(output_logs), 1)
+
+        output_log = output_logs[0]
+        self.assertEqual(output_log['notification_type'], 'ERROR')
+        self.assertEqual(output_log['event_type'], 'image.upload')
+        self.assertTrue('Not allowed' in output_log['payload'])
+
+    def test_image_set_data_not_found(self):
+        def data_iterator():
+            self.notifier.log = []
+            yield 'abcde'
+            raise exception.NotFound('Not found')
+
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.image_proxy.set_data, data_iterator(), 10)
+
+        output_logs = self.notifier.get_logs()
+        self.assertEqual(len(output_logs), 1)
+
+        output_log = output_logs[0]
+        self.assertEqual(output_log['notification_type'], 'ERROR')
+        self.assertEqual(output_log['event_type'], 'image.upload')
+        self.assertTrue('Not found' in output_log['payload'])
+
+    def test_image_set_data_HTTP_error(self):
+        def data_iterator():
+            self.notifier.log = []
+            yield 'abcde'
+            raise webob.exc.HTTPError('Http issue')
+
+        self.assertRaises(webob.exc.HTTPError,
+                          self.image_proxy.set_data, data_iterator(), 10)
+
+        output_logs = self.notifier.get_logs()
+        self.assertEqual(len(output_logs), 1)
+
+        output_log = output_logs[0]
+        self.assertEqual(output_log['notification_type'], 'ERROR')
+        self.assertEqual(output_log['event_type'], 'image.upload')
+        self.assertTrue('Http issue' in output_log['payload'])
+
+    def test_image_set_data_error(self):
+        def data_iterator():
+            self.notifier.log = []
+            yield 'abcde'
+            raise Exception('Failed')
+
+        self.assertRaises(Exception,
+                          self.image_proxy.set_data, data_iterator(), 10)
+
+        output_logs = self.notifier.get_logs()
+        self.assertEqual(len(output_logs), 1)
+
+        output_log = output_logs[0]
+        self.assertEqual(output_log['notification_type'], 'ERROR')
+        self.assertEqual(output_log['event_type'], 'image.upload')
+        self.assertTrue('Failed' in output_log['payload'])