Merge lp:~johannes.erdfelt/glance/image-undelete into lp:~hudson-openstack/glance/trunk

[Johannes Erdfelt]

Add image restore (undelete) functionality to Glance

Delayed delete is only as useful as being able to undelete images if you've determined that you didn't really want to delete it.

[Josh Kearney (jk0)]

*golf clap*

review: Approve

[Brian Waldon (bcwaldon)]

This will definitely be useful once delayed_delete is in, however, I did notice a few things:

1) There isn't very much testing around this feature. Can you more thoroughly test this at each level it is exposed?

2) The undelete action in db/api.py sets the status to 'active'. I don't think this is the best practice, as it could be undeleting an image with status 'killed' or 'saving'.

3) Can we call it 'restore' instead of 'undelete'? I wouldn't want 'delete' to be 'unadd'...

4) I'm not crazy about member actions in the API (i.e. POST /images/<id>/undelete). Can you rework it to use a PUT? Not sure what the best option would be...

review: Needs Fixing

[Johannes Erdfelt (johannes.erdfelt)]

1) Can you be more detailed what extra testing you think is necessary?

2) I'll work on saving the previous status and restoring that value.

3) I agree that 'restore' is a better term. I'll make those changes shortly as well.

4) I can't say I'm 100% pleased with the /undelete member (as well as /restore). However, I think PUT is worse. PUT stores the request body at the resource specified. Restoring a deleted image doesn't update the body of the resource, just makes metadata changes.

Maybe /images/<id>/actions (ala nova) or /images/restore (taking image id as a POST variable) would be better choices?

There's also the alternative of making this more a client side operation, by allowing clients to retrieve the original status and then making the client use existing API entry points to restore the status. It would be trickier from an implementation standpoint since the existing code returns 404 for deleted (but not purged) images.

It could be that images that are pending delete still let metadata updates, but they aren't returned in list operations.

I think this one of those REST impedance mismatch issues so we'll need to be creative.

[Brian Waldon (bcwaldon)]

> 1) Can you be more detailed what extra testing you think is necessary?

I think it is very necessary to test the glance cli tool, the api client, the api itself, the registry client, and the registy api. Each layer needs to be tested.

> 2) I'll work on saving the previous status and restoring that value.

Sounds good.

> 3) I agree that 'restore' is a better term. I'll make those changes shortly as
> well.

Excellent. If you have a better suggestion, I would love to hear it :)

> 4) I can't say I'm 100% pleased with the /undelete member (as well as
> /restore). However, I think PUT is worse. PUT stores the request body at the
> resource specified. Restoring a deleted image doesn't update the body of the
> resource, just makes metadata changes.
>
> Maybe /images/<id>/actions (ala nova) or /images/restore (taking image id as a
> POST variable) would be better choices?
>
> There's also the alternative of making this more a client side operation, by
> allowing clients to retrieve the original status and then making the client
> use existing API entry points to restore the status. It would be trickier from
> an implementation standpoint since the existing code returns 404 for deleted
> (but not purged) images.
>
> It could be that images that are pending delete still let metadata updates,
> but they aren't returned in list operations.
>
> I think this one of those REST impedance mismatch issues so we'll need to be
> creative.

I agree there isn't an obvious solution, so we will have to get creative. /images/<id>/action sounds best to me. Also, I'm thinking we should 404 on 'pending_delete', disallowing metadata updates. It should essentially be deleted.

[Johannes Erdfelt (johannes.erdfelt)]

Ok, this should now have all of the changes bcwaldon requested.

[lp:~johannes.erdfelt/glance/image-undelete]

194. By Johannes Erdfelt on 2011-07-28

Merge with trunk

195. By Johannes Erdfelt on 2011-07-29

Merge with trunk

[Brian Waldon (bcwaldon)]

Testing looks great. Two minor nits:
1) Can you align restore_image and image_restore?
2) can you move /images/<id>/actions to /images/<id>/action? Nova uses 'action' for this kind of thing, while it uses 'actions' for admin-only reporting functionality

[lp:~johannes.erdfelt/glance/image-undelete]

196. By Johannes Erdfelt on 2011-07-29

Merge with trunk

197. By Johannes Erdfelt on 2011-07-29

actions -> action

[Johannes Erdfelt (johannes.erdfelt)]

I've pushed up a fix that changes 'actions' to 'action'.

As far as restore_image vs image_restore, glance as a whole is very inconsistent. For instance, the client APIs generally follow <action>_<object>, whereas the DB API and some other code follow <object>_<action>.

I followed the surrounding code to determine which naming scheme to follow. My branch should be consistent within each module.

I'm indifferent which one is better, but I don't want this branch to explode in scope to fixing naming to be consistent within all of Glance. That's probably better done in another branch.

[Brian Waldon (bcwaldon)]

I'm cool with the naming, and thanks for updating the action collection. Looks good.

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~johannes.erdfelt/glance/image-undelete into lp:glance failed. Below is the output from the failed tests.

running test
running egg_info
creating glance.egg-info
writing glance.egg-info/PKG-INFO
writing top-level names to glance.egg-info/top_level.txt
writing dependency_links to glance.egg-info/dependency_links.txt
writing manifest file 'glance.egg-info/SOURCES.txt'
reading manifest file 'glance.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching 'ChangeLog'
writing manifest file 'glance.egg-info/SOURCES.txt'
running build_ext

We test the following: ... ok
We test the following: ... ok
Test for LP Bugs #736295, #767203 ... ok
We test the following: ... ok
We test conditions that produced LP Bug #768969, where an image ... ok
Set up three test images and ensure each query param filter works ... ok
We test the following sequential series of actions: ... ok
Ensure marker and limit query params work ... ok
Set up three test images and ensure each query param filter works ... ok
We test the process flow where a user registers an image ... ok
A test against the actual datastore backend for the registry ... ok
A test that errors coming from the POST API do not ... ok
We test that various calls to the images and root endpoints are ... ok
Upload image, delete it, then restore it ... ok
Upload initial image, then attempt to upload duplicate image ... ok
Set up four test images and ensure each query param filter works ... ok
We test the following sequential series of actions: ... ok
Ensure marker and limit query params work ... ok
Set up three test images and ensure each query param filter works ... ok
We test the process flow where a user registers an image ... ok
A test against the actual datastore backend for the registry ... ok
A test that errors coming from the POST API do not ... ok
We test that various calls to the images and root endpoints are ... ok
Test logging output proper when verbose and debug ... ok
Test logging output proper when verbose and debug ... ok
A test for LP bug #704854 -- Exception thrown by registry ... ok
We test the following: ... ok
We test the following: ... ok
test that images don't get deleted immediatly and that the scrubber ... ok
test that images get deleted immediately by default ... ok
test that images can be restored after delayed delete ... ok
Tests raises BadRequest for invalid store header ... ok
Tests to add a basic image in the file store ... ok
Tests creates a queued image for no body and no loc header ... ok
Tests creates a queued image for no body and no loc header ... ok
Test that the image contents are checksummed properly ... ok
test_bad_container_format (tests.unit.test_api.TestGlanceAPI) ... ok
test_bad_disk_format (tests.unit.test_api.TestGlanceAPI) ... ok
test_delete_image (tests.unit.test_api.TestGlanceAPI) ... ok
test_delete_non_exists_image (tests.unit.test_api.TestGlanceAPI) ... ok
Here, we try to delete an image that is in the queued state. ... ok
Test that the ETag header matches the x-image-meta-checksum ... ok
Tests that the /images/detail registry API returns a 400 ... ok
Tests that the /images registry API returns list of ...

[lp:~johannes.erdfelt/glance/image-undelete]

198. By Johannes Erdfelt on 2011-08-01

Merge with trunk

[Johannes Erdfelt (johannes.erdfelt)]

Yeah, so the unit tests work fine on my development system. I even spent a while trying to figure out a way to cause the tests to fail like they did on the jenkins server.

Should we just assume it was a transient failure and try to merge again?

Unmerged revisions

198. By Johannes Erdfelt on 2011-08-01

Merge with trunk

197. By Johannes Erdfelt on 2011-07-29

actions -> action

196. By Johannes Erdfelt on 2011-07-29

Merge with trunk

195. By Johannes Erdfelt on 2011-07-29

Merge with trunk

194. By Johannes Erdfelt on 2011-07-28

Merge with trunk

193. By Johannes Erdfelt on 2011-07-28

Add test for bin/glance, the HTTP API and the registry client

192. By Johannes Erdfelt on 2011-07-28

Fix debug message

191. By Johannes Erdfelt on 2011-07-27

Save status of image since restoring an image may not always mean setting it
to active

190. By Johannes Erdfelt on 2011-07-27

Clarify help for restore command

189. By Johannes Erdfelt on 2011-07-27

Fix PEP8 warning

=== modified file '.mailmap'
--- .mailmap	2011-04-27 18:00:49 +0000
+++ .mailmap	2011-08-01 18:29:24 +0000
@@ -3,6 +3,7 @@
 # <preferred e-mail> <other e-mail 2>
 <[e-mail]> <[e-mail]>
 <[e-mail]> <jsuh@bespin>
+<[e-mail]> <johannes@squeeze>
 <[e-mail]> <[e-mail]>
 <[e-mail]> <[e-mail]> 
 <[e-mail]> <[e-mail]>

=== modified file 'bin/glance'
--- bin/glance	2011-07-25 22:01:26 +0000
+++ bin/glance	2011-08-01 18:29:24 +0000
@@ -366,6 +366,29 @@
         return FAILURE
 
 
+def image_restore(options, args):
+    """
+%(prog)s restore [options] <ID>
+
+Restores a previously deleted image from Glance"""
+    try:
+        image_id = args.pop()
+    except IndexError:
+        print "Please specify the ID of the image you wish to restore "
+        print "as the first argument"
+        return FAILURE
+
+    c = get_client(options)
+
+    try:
+        c.restore_image(image_id)
+        print "Restored image %s" % image_id
+        return SUCCESS
+    except exception.NotFound:
+        print "No image with ID %s was found" % image_id
+        return FAILURE
+
+
 def image_show(options, args):
     """
 %(prog)s show [options] <ID>
@@ -816,7 +839,8 @@
                 'index': images_index,
                 'details': images_detailed,
                 'show': image_show,
-                'clear': images_clear}
+                'clear': images_clear,
+                'restore': image_restore}
 
     CACHE_COMMANDS = {
                 'cache-index': cache_index,
@@ -878,6 +902,8 @@
 
     delete          Deletes an image from Glance
 
+    restore         Restores a previously deleted image from Glance
+
     index           Return brief information about images in Glance
 
     details         Return detailed information about images in

=== modified file 'glance/api/v1/__init__.py'
--- glance/api/v1/__init__.py	2011-07-21 23:22:18 +0000
+++ glance/api/v1/__init__.py	2011-08-01 18:29:24 +0000
@@ -34,7 +34,8 @@
         mapper = routes.Mapper()
         resource = images.create_resource(options)
         mapper.resource("image", "images", controller=resource,
-                        collection={'detail': 'GET'})
+                        collection={'detail': 'GET'},
+                        member={'action': 'POST'})
         mapper.connect("/", controller=resource, action="index")
         mapper.connect("/images/{id}", controller=resource, action="meta",
                        conditions=dict(method=["HEAD"]))

=== modified file 'glance/api/v1/images.py'
--- glance/api/v1/images.py	2011-07-28 23:01:40 +0000
+++ glance/api/v1/images.py	2011-08-01 18:29:24 +0000
@@ -68,6 +68,8 @@
         PUT /images/<ID> -- Update image metadata and/or upload image
                             data for a previously-reserved image
         DELETE /images/<ID> -- Delete the image with id <ID>
+        POST /images/<ID>/action -- Perform action on image with id <ID>
+                                    (only 'restore' right now)
     """
 
     def __init__(self, options):
@@ -557,10 +559,42 @@
         # to delete the image if the backend doesn't yet store it.
         # See https://bugs.launchpad.net/glance/+bug/747799
         if image['location']:
-            schedule_delete_from_backend(image['location'], self.options,
-                                         req.context, id)
+            schedule_delete_from_backend(self.options, req.context, image)
         registry.delete_image_metadata(self.options, req.context, id)
 
+    def action(self, req, id, body):
+        """
+        Performs action on image
+
+        :param request: The WSGI/Webob Request object
+        :retval There is no response body
+
+        :raises HttpBadRequest if image registry is invalid
+        :raises HttpNotFound if image or any chunk is not available
+        :raises HttpNotAuthorized if image or any chunk is not
+                deleteable by the requesting user
+        """
+        if req.context.read_only:
+            msg = "Read-only access"
+            logger.debug(msg)
+            raise HTTPForbidden(msg, request=req,
+                                content_type="text/plain")
+
+        action = body['action']
+        if action == 'restore':
+            try:
+                registry.restore_image(self.options, req.context, id)
+            except exception.NotFound:
+                msg = "Image with identifier %s not found" % id
+                logger.debug(msg)
+                raise HTTPNotFound(msg, request=req,
+                                   content_type='text/plain')
+        else:
+            msg = "Unknown action %s" % action
+            raise HTTPBadRequest(msg, request=req, content_type='text/plain')
+
+        return ''
+
     def get_store_or_400(self, request, store_name):
         """
         Grabs the storage backend for the supplied store name

=== modified file 'glance/client.py'
--- glance/client.py	2011-07-25 21:59:56 +0000
+++ glance/client.py	2011-08-01 18:29:24 +0000
@@ -164,6 +164,14 @@
         self.do_request("DELETE", "/images/%s" % image_id)
         return True
 
+    def restore_image(self, image_id):
+        """
+        Restores an image that is in pending_delete
+        """
+        self.do_request("POST", "/images/%s/action" % image_id,
+                        json.dumps({"action": "restore"}))
+        return True
+
     def get_cached_images(self, **kwargs):
         """
         Returns a list of images stored in the image cache.

=== modified file 'glance/registry/__init__.py'
--- glance/registry/__init__.py	2011-07-20 22:53:44 +0000
+++ glance/registry/__init__.py	2011-08-01 18:29:24 +0000
@@ -86,6 +86,12 @@
     return c.delete_image(image_id)
 
 
+def restore_image(options, context, image_id):
+    logger.debug("Restoring image %s...", image_id)
+    c = get_registry_client(options, context)
+    return c.restore_image(image_id)
+
+
 def _debug_print_metadata(image_meta):
     data = image_meta.copy()
     properties = data.pop('properties', None)

=== modified file 'glance/registry/client.py'
--- glance/registry/client.py	2011-07-20 22:53:44 +0000
+++ glance/registry/client.py	2011-08-01 18:29:24 +0000
@@ -126,3 +126,11 @@
         """
         self.do_request("DELETE", "/images/%s" % image_id)
         return True
+
+    def restore_image(self, image_id):
+        """
+        Restores an image that is pending deletion
+        """
+        self.do_request("POST", "/images/%s/action" % image_id,
+                        json.dumps({"action": "restore"}))
+        return True

=== modified file 'glance/registry/db/api.py'
--- glance/registry/db/api.py	2011-07-23 03:38:20 +0000
+++ glance/registry/db/api.py	2011-08-01 18:29:24 +0000
@@ -118,6 +118,31 @@
             image_property_delete(context, prop_ref, session=session)
 
 
+def image_restore(context, image_id):
+    """Restore image that is pending deletion.
+
+    Raises NotFound if image does not exist.
+
+    """
+    session = get_session()
+    with session.begin():
+        try:
+            image_ref = session.query(models.Image).\
+                       options(joinedload(models.Image.properties)).\
+                       filter_by(id=image_id).\
+                       one()
+        except exc.NoResultFound:
+            raise exception.NotFound("No image found with ID %s" % image_id)
+
+        if image_ref['status'] != 'pending_delete':
+            raise exception.ApiError('Image not in pending_delete state', 400)
+
+        image_ref.update({'status': image_ref['saved_status'],
+                          'saved_status': None,
+                          'deleted': False})
+        image_ref.save(session=session)
+
+
 def image_get(context, image_id, session=None):
     """Get an image or raise if it does not exist."""
     session = session or get_session()

=== added file 'glance/registry/db/migrate_repo/versions/008_add_saved_status.py'
--- glance/registry/db/migrate_repo/versions/008_add_saved_status.py	1970-01-01 00:00:00 +0000
+++ glance/registry/db/migrate_repo/versions/008_add_saved_status.py	2011-08-01 18:29:24 +0000
@@ -0,0 +1,41 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from sqlalchemy import MetaData, Table, Column, String
+
+
+meta = MetaData()
+
+saved_status = Column('saved_status', String(30))
+
+
+def upgrade(migrate_engine):
+    meta.bind = migrate_engine
+
+    images = Table('images', meta, autoload=True,
+                   autoload_with=migrate_engine)
+
+    images.create_column(saved_status)
+
+
+def downgrade(migrate_engine):
+    meta.bind = migrate_engine
+
+    images = Table('images', meta, autoload=True,
+                   autoload_with=migrate_engine)
+
+    images.drop_column(saved_status)

=== modified file 'glance/registry/db/models.py'
--- glance/registry/db/models.py	2011-07-20 20:27:33 +0000
+++ glance/registry/db/models.py	2011-08-01 18:29:24 +0000
@@ -102,6 +102,7 @@
     container_format = Column(String(20))
     size = Column(BigInteger)
     status = Column(String(30), nullable=False)
+    saved_status = Column(String(30))
     is_public = Column(Boolean, nullable=False, default=False)
     location = Column(Text)
     checksum = Column(String(32))

=== modified file 'glance/registry/server.py'
--- glance/registry/server.py	2011-07-22 23:35:24 +0000
+++ glance/registry/server.py	2011-08-01 18:29:24 +0000
@@ -345,6 +345,35 @@
                                request=req,
                                content_type='text/plain')
 
+    def action(self, req, id, body):
+        """Restores image pending deletion.
+
+        :param req: Request body.  Ignored.
+        :param id:  The opaque internal identifier for the image
+
+        :retval Returns the updated image information as a mapping,
+
+        """
+        context = None
+        action = body['action']
+        if action == 'restore':
+            try:
+                logger.debug("Restoring image %(id)s" % locals())
+                db_api.image_restore(context, id)
+                return ''
+            except exception.Invalid, e:
+                msg = "Failed to restore image."
+                logger.error(msg)
+                return exc.HTTPBadRequest(msg)
+            except exception.NotFound:
+                raise exc.HTTPNotFound(body='Image not found',
+                                       request=req,
+                                       content_type='text/plain')
+        else:
+            msg = "Unknown action %s." % action
+            logger.error(msg)
+            return exc.HTTPBadRequest(msg)
+
 
 def create_resource(options):
     """Images resource factory method."""
@@ -360,7 +389,8 @@
         mapper = routes.Mapper()
         resource = create_resource(options)
         mapper.resource("image", "images", controller=resource,
-                       collection={'detail': 'GET'})
+                       collection={'detail': 'GET'},
+                       member={'action': 'POST'})
         mapper.connect("/", controller=resource, action="index")
         super(API, self).__init__(mapper)
 

=== modified file 'glance/store/__init__.py'
--- glance/store/__init__.py	2011-07-28 23:01:40 +0000
+++ glance/store/__init__.py	2011-08-01 18:29:24 +0000
@@ -111,20 +111,21 @@
     return loc.store_name
 
 
-def schedule_delete_from_backend(uri, options, context, id, **kwargs):
+def schedule_delete_from_backend(options, context, image, **kwargs):
     """
     Given a uri and a time, schedule the deletion of an image.
     """
     use_delay = config.get_option(options, 'delayed_delete', type='bool',
                                   default=False)
     if not use_delay:
-        registry.update_image_metadata(options, context, id,
+        registry.update_image_metadata(options, context, image['id'],
                                        {'status': 'deleted'})
         try:
-            return delete_from_backend(uri, **kwargs)
+            return delete_from_backend(image['location'], **kwargs)
         except (UnsupportedBackend, exception.NotFound):
             msg = "Failed to delete image from store (%s). "
-            logger.error(msg % uri)
+            logger.error(msg % image['location'])
 
-    registry.update_image_metadata(options, context, id,
-                                   {'status': 'pending_delete'})
+    registry.update_image_metadata(options, context, image['id'],
+                                   {'status': 'pending_delete',
+                                    'saved_status': image['status']})

=== modified file 'tests/functional/test_bin_glance.py'
--- tests/functional/test_bin_glance.py	2011-06-28 14:37:31 +0000
+++ tests/functional/test_bin_glance.py	2011-08-01 18:29:24 +0000
@@ -288,3 +288,88 @@
             self.assertEqual(0, rec[0])
 
         self.stop_servers()
+
+    def test_delete_restore(self):
+        """
+        We test the following:
+
+            0. Verify no public images in index
+            1. Add a public image with a location attr
+               and no image data
+            2. Check that image exists in index
+            3. Delete the image
+            4. Verify no longer in index
+            5. Restore the image
+            6. Check that image exists in index
+        """
+        self.cleanup()
+        self.start_servers(delayed_delete=True)
+
+        api_port = self.api_port
+        registry_port = self.registry_port
+
+        # 0. Verify no public images
+        cmd = "bin/glance --port=%d index" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        self.assertEqual('No public images found.', out.strip())
+
+        # 1. Add public image
+        cmd = "bin/glance --port=%d add is_public=True name=MyImage" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        self.assertEqual('Added new image with ID: 1', out.strip())
+
+        # 2. Verify image added as public image
+        cmd = "bin/glance --port=%d index" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        lines = out.split("\n")
+        first_line = lines[0]
+        image_data_line = lines[3]
+        self.assertEqual('Found 1 public images...', first_line)
+        self.assertTrue('MyImage' in image_data_line)
+
+        # 3. Delete the image
+        cmd = "bin/glance --port=%d --force delete 1" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        self.assertEqual('Deleted image 1', out.strip())
+
+        # 4. Verify no public images
+        cmd = "bin/glance --port=%d index" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        self.assertEqual('No public images found.', out.strip())
+
+        # 5. Restore the image
+        cmd = "bin/glance --port=%d restore 1" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        self.assertEqual('Restored image 1', out.strip())
+
+        # 6. Verify image listed as public image
+        cmd = "bin/glance --port=%d index" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        lines = out.split("\n")
+        first_line = lines[0]
+        image_data_line = lines[3]
+        self.assertEqual('Found 1 public images...', first_line)
+        self.assertTrue('MyImage' in image_data_line)
+
+        self.stop_servers()

=== modified file 'tests/functional/test_httplib2_api.py'
--- tests/functional/test_httplib2_api.py	2011-07-21 12:42:38 +0000
+++ tests/functional/test_httplib2_api.py	2011-08-01 18:29:24 +0000
@@ -1076,3 +1076,66 @@
                         "Could not find '%s' in '%s'" % (expected, content))
 
         self.stop_servers()
+
+    def test_delete_restore(self):
+        """
+        Upload image, delete it, then restore it
+        """
+        self.cleanup()
+        self.start_servers(delayed_delete=True)
+
+        # 0. GET /images
+        # Verify no public images
+        path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port)
+        http = httplib2.Http()
+        response, content = http.request(path, 'GET')
+        self.assertEqual(response.status, 200)
+        self.assertEqual(content, '{"images": []}')
+
+        # 1. POST /images with public image named Image1
+        headers = {'Content-Type': 'application/octet-stream',
+                   'X-Image-Meta-Name': 'Image1',
+                   'X-Image-Meta-Status': 'active',
+                   'X-Image-Meta-Container-Format': 'ovf',
+                   'X-Image-Meta-Disk-Format': 'vdi',
+                   'X-Image-Meta-Size': '19',
+                   'X-Image-Meta-Is-Public': 'True'}
+        path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port)
+        http = httplib2.Http()
+        response, content = http.request(path, 'POST', headers=headers,
+                                         body='x' * 19)
+        self.assertEqual(response.status, 201)
+
+        # 2. GET /images
+        # Verify 1 public image
+        path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port)
+        http = httplib2.Http()
+        response, content = http.request(path, 'GET')
+        self.assertEqual(response.status, 200)
+        body = json.loads(content)
+        self.assertEqual(len(body['images']), 1)
+
+        # 3. DELETE /images/1
+        path = "http://%s:%d/v1/images/1" % ("0.0.0.0", self.api_port)
+        http = httplib2.Http()
+        response, content = http.request(path, 'DELETE')
+        self.assertEqual(response.status, 200)
+
+        # 4. GET /images
+        # Verify no public images
+        path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port)
+        http = httplib2.Http()
+        response, content = http.request(path, 'GET')
+        self.assertEqual(response.status, 200)
+        self.assertEqual(content, '{"images": []}')
+
+        # 5. POST /images/1/action to restore image
+        headers = {'Content-Type': 'application/json'}
+        body = {'action': 'restore'}
+        path = "http://%s:%d/v1/images/1/action" % ("0.0.0.0", self.api_port)
+        http = httplib2.Http()
+        response, content = http.request(path, 'POST', headers=headers,
+                                         body=json.dumps(body))
+        self.assertEqual(response.status, 200)
+
+        self.stop_servers()

=== modified file 'tests/functional/test_scrubber.py'
--- tests/functional/test_scrubber.py	2011-07-26 14:59:22 +0000
+++ tests/functional/test_scrubber.py	2011-08-01 18:29:24 +0000
@@ -125,3 +125,31 @@
             self.assertEqual(rec['status'], 'deleted')
 
         self.stop_servers()
+
+    @functional.runs_sql
+    def test_restore(self):
+        """
+        test that images can be restored after delayed delete
+        """
+
+        def _get_all_pending_delete():
+            sql = "SELECT * FROM images WHERE status = 'pending_delete'"
+            return list(self.run_sql_cmd(sql))
+
+        self.cleanup()
+        registry_db = self.registry_server.sql_connection
+        self.start_servers(delayed_delete=True, sql_connection=registry_db,
+                           daemon=True)
+
+        client = self._get_client()
+        meta = client.add_image(TEST_IMAGE_META, TEST_IMAGE_DATA)
+        id = meta['id']
+        self.assertFalse(_get_all_pending_delete())
+
+        client.delete_image(id)
+        self.assertTrue(_get_all_pending_delete())
+
+        client.restore_image(id)
+        self.assertFalse(_get_all_pending_delete())
+
+        self.stop_servers()

=== modified file 'tests/stubs.py'
--- tests/stubs.py	2011-07-23 00:07:38 +0000
+++ tests/stubs.py	2011-08-01 18:29:24 +0000
@@ -276,6 +276,7 @@
             {'id': 1,
                 'name': 'fake image #1',
                 'status': 'active',
+                'saved_status': None,
                 'disk_format': 'ami',
                 'container_format': 'ami',
                 'is_public': False,
@@ -292,6 +293,7 @@
             {'id': 2,
                 'name': 'fake image #2',
                 'status': 'active',
+                'saved_status': None,
                 'disk_format': 'vhd',
                 'container_format': 'ovf',
                 'is_public': True,
@@ -324,6 +326,7 @@
             values['deleted'] = False
             values['properties'] = values.get('properties', {})
             values['location'] = values.get('location')
+            values['saved_status'] = values.get('saved_status')
 
             now = datetime.datetime.utcnow()
             values['created_at'] = values.get('created_at', now)
@@ -382,6 +385,11 @@
             image['deleted_at'] = datetime.datetime.utcnow()
             self.deleted_images.append(image)
 
+        def image_restore(self, _context, image_id):
+            image = self.image_get(_context, image_id)
+            image['status'] = image['saved_status']
+            image['saved_status'] = None
+
         def image_get(self, _context, image_id):
 
             images = [i for i in self.images if str(i['id']) == str(image_id)]
@@ -469,6 +477,8 @@
               fake_datastore.image_update)
     stubs.Set(glance.registry.db.api, 'image_destroy',
               fake_datastore.image_destroy)
+    stubs.Set(glance.registry.db.api, 'image_restore',
+              fake_datastore.image_restore)
     stubs.Set(glance.registry.db.api, 'image_get',
               fake_datastore.image_get)
     stubs.Set(glance.registry.db.api, 'image_get_all_pending_delete',

=== modified file 'tests/unit/test_clients.py'
--- tests/unit/test_clients.py	2011-07-26 10:11:38 +0000
+++ tests/unit/test_clients.py	2011-08-01 18:29:24 +0000
@@ -883,6 +883,26 @@
                           self.client.delete_image,
                           3)
 
+    def test_restore(self):
+        """Tests restoring deleted image"""
+        extra_fixture = {'id': 3,
+                         'status': 'pending_delete',
+                         'saved_status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        self.assertTrue(self.client.restore_image(3))
+
+        data = self.client.get_image(3)
+
+        self.assertEquals(data['status'], 'active')
+
 
 class TestClient(unittest.TestCase):