Merge "delete deleted image 500 bug"

5 files changed, 35 insertions, 4 deletions

diff --git a/nova/api/openstack/compute/images.py b/nova/api/openstack/compute/images.py
index 82db21c9e..3d54da0b5 100644
--- a/nova/api/openstack/compute/images.py
+++ b/nova/api/openstack/compute/images.py
@@ -159,6 +159,11 @@ class Controller(wsgi.Controller):
         except exception.ImageNotFound:
             explanation = _("Image not found.")
             raise webob.exc.HTTPNotFound(explanation=explanation)
+        except exception.ImageNotAuthorized:
+            # The image service raises this exception on delete if glanceclient
+            # raises HTTPForbidden.
+            explanation = _("You are not allowed to delete the image.")
+            raise webob.exc.HTTPForbidden(explanation=explanation)
         return webob.exc.HTTPNoContent()
 
     @wsgi.serializers(xml=MinimalImagesTemplate)
diff --git a/nova/tests/api/openstack/compute/test_images.py b/nova/tests/api/openstack/compute/test_images.py
index db0ad51f7..a35dc6e51 100644
--- a/nova/tests/api/openstack/compute/test_images.py
+++ b/nova/tests/api/openstack/compute/test_images.py
@@ -665,6 +665,18 @@ class ImagesControllerTest(test.TestCase):
         response = self.controller.delete(request, '124')
         self.assertEqual(response.status_int, 204)
 
+    def test_delete_deleted_image(self):
+        """If you try to delete a deleted image, you get back 403 Forbidden."""
+
+        deleted_image_id = 128
+        # see nova.tests.api.openstack.fakes:_make_image_fixtures
+
+        request = fakes.HTTPRequest.blank(
+              '/v2/fake/images/%s' % deleted_image_id)
+        request.method = 'DELETE'
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
+             request, '%s' % deleted_image_id)
+
     def test_delete_image_not_found(self):
         request = fakes.HTTPRequest.blank('/v2/fake/images/300')
         request.method = 'DELETE'
diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py
index 97321212c..a7bf55521 100644
--- a/nova/tests/api/openstack/fakes.py
+++ b/nova/tests/api/openstack/fakes.py
@@ -217,12 +217,10 @@ def _make_image_fixtures():
     NOW_GLANCE_FORMAT = "2010-10-11T10:30:22"
 
     image_id = 123
-    base_attrs = {'deleted': False}
 
     fixtures = []
 
     def add_fixture(**kwargs):
-        kwargs.update(base_attrs)
         fixtures.append(kwargs)
 
     # Public image
@@ -237,9 +235,11 @@ def _make_image_fixtures():
     snapshot_properties = {'instance_uuid': uuid, 'user_id': 'fake'}
     for status in ('queued', 'saving', 'active', 'killed',
                    'deleted', 'pending_delete'):
+        deleted = False if status != 'deleted' else True
         add_fixture(id=image_id, name='%s snapshot' % status,
                     is_public=False, status=status,
-                    properties=snapshot_properties, size='25165824')
+                    properties=snapshot_properties, size='25165824',
+                    deleted=deleted)
         image_id += 1
 
     # Image without a name
diff --git a/nova/tests/glance/stubs.py b/nova/tests/glance/stubs.py
index 076afeffc..56dae7ffc 100644
--- a/nova/tests/glance/stubs.py
+++ b/nova/tests/glance/stubs.py
@@ -83,7 +83,13 @@ class StubGlanceClient(object):
     def delete(self, image_id):
         for i, image in enumerate(self._images):
             if image.id == image_id:
-                del self._images[i]
+                # When you delete an image from glance, it sets the status to
+                # DELETED. If you try to delete a DELETED image, it raises
+                # HTTPForbidden.
+                image_data = self._images[i]
+                if image_data.deleted:
+                    raise glanceclient.exc.HTTPForbidden()
+                image_data.deleted = True
                 return
         raise glanceclient.exc.NotFound(image_id)
 
diff --git a/nova/tests/image/test_glance.py b/nova/tests/image/test_glance.py
index 2eca4b969..433a25e61 100644
--- a/nova/tests/image/test_glance.py
+++ b/nova/tests/image/test_glance.py
@@ -363,8 +363,16 @@ class TestGlanceImageService(test.TestCase):
         self.assertEquals(2, num_images)
 
         self.service.delete(self.context, ids[0])
+        # When you delete an image from glance, it sets the status to DELETED
+        # and doesn't actually remove the image.
 
+        # Check the image is still there.
         num_images = len(self.service.detail(self.context))
+        self.assertEquals(2, num_images)
+
+        # Check the image is marked as deleted.
+        num_images = reduce(lambda x, y: x + (0 if y['deleted'] else 1),
+                            self.service.detail(self.context), 0)
         self.assertEquals(1, num_images)
 
     def test_show_passes_through_to_client(self):