Merge lp:~rackspace-titan/glance/glance-clear-lp766295 into lp:~hudson-openstack/glance/trunk

[Brian Waldon]

- Require user confirmation for "bin/glance clear" and "bin/glance delete <id>"
- Allow for override with -f/--force command-line option

[Brian Waldon (bcwaldon)]

I didn't add any testing, just because I wasn't quite sure how to do it correctly. If anybody has any ideas I am all ears.

[Jay Pipes (jaypipes)]

nice work, Brian :)

review: Approve

[Dan Prince (dan-prince)]

Looks great.

We don't appear to have a glance bin test on clear all. Probably some overlap with existing tests but we could create a test named 'test_add_list_clear_list' test case or something? I approve w/ or without this test case however.

review: Approve

[Jay Pipes (jaypipes)]

> Looks great.
>
> We don't appear to have a glance bin test on clear all. Probably some overlap
> with existing tests but we could create a test named
> 'test_add_list_clear_list' test case or something? I approve w/ or without
> this test case however.

Sure, but keep in mind clear only deletes *public* images, since it uses a call to index() and then delete()s each one in the index (which is only public images)

-jay

[Dan Prince (dan-prince)]

Sure. Also I'm adding this missing test in my branch to fix the image_properties clearing issue. ;) Wouldn't want to have a conflict.

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~rackspace-titan/glance/glance-clear-lp766295 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: ... ERROR
We test the following: ... ok
Test for LP Bugs #736295, #767203 ... ok
We test the following sequential series of actions: ... ok
A test against the actual datastore backend for the registry ... ok
A test that errors coming from the POST API do not ... 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
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
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
Test that the image contents are checksummed properly ... ok
Test for HEAD /images/<ID> ... ok
test_show_image_basic (tests.unit.test_api.TestGlanceAPI) ... ok
test_show_non_exists_image (tests.unit.test_api.TestGlanceAPI) ... ok
Tests that the /images POST registry API creates the image ... ok
Tests proper exception is raised if a bad disk_format is set ... ok
Tests proper exception is raised if a bad disk_format is set ... ok
Tests proper exception is raised if a bad status is set ... ok
Tests that exception raised for bad matching disk and container ... ok
Tests that the /images DELETE registry API deletes the image ... ok
Tests proper exception is raised if attempt to delete non-existing ... ok
Tests that the /images/detail registry API returns ... ok
Tests that the /images registry API returns list of ... ok
Tests that the root registry API returns "index", ... ok
Tests that the /images PUT registry API updates the image ... ok
Tests proper exception is raised if attempt to update non-existing ... ok
Tests that exception raised trying to set a bad container_format ... ok
Tests that exception raised trying to set a bad disk_format ... ok
Tests that exception raised trying to set a bad status ... ok
Tests that exception raised for bad matching disk and container ... ok
Test ClientConnectionError raised ... ok
Tests proper exception is raised if image with ID already exists ... ok
Tests that we can add image metadata and returns the new id ... ok
Tests a bad stat...

[lp:~rackspace-titan/glance/glance-clear-lp766295]

120. By Brian Waldon on 2011-04-20

adding --force option to test_add_clear

[Brian Waldon (bcwaldon)]

Should have merged trunk after Dan's branch went in today. Sorry, guys! We're good to go now.

[Jay Pipes (jaypipes)]

Cool, looks good.

review: Approve

=== modified file 'bin/glance'
--- bin/glance	2011-04-20 19:12:18 +0000
+++ bin/glance	2011-04-21 00:03:23 +0000
@@ -309,7 +309,6 @@
 %(prog)s delete [options] <ID>
 
 Deletes an image from Glance"""
-    c = get_client(options)
     try:
         image_id = args.pop()
     except IndexError:
@@ -317,6 +316,13 @@
         print "as the first argument"
         return FAILURE
 
+    if not options.force and \
+       not user_confirm("Delete image %s?" % (image_id,)):
+        print 'Not deleting image %s' % (image_id,)
+        return FAILURE
+
+    c = get_client(options)
+
     try:
         c.delete_image(image_id)
         print "Deleted image %s" % image_id
@@ -428,6 +434,10 @@
 %(prog)s clear [options]
 
 Deletes all images from a Glance server"""
+    if not options.force and not user_confirm("Delete all images?"):
+        print 'Not deleting any images'
+        return FAILURE
+
     c = get_client(options)
     images = c.get_images()
     for image in images:
@@ -470,6 +480,10 @@
                       type=int, default=9292,
                       help="Port the Glance API host listens on. "
                            "Default: %default")
+    parser.add_option('-f', '--force', dest="force", metavar="FORCE",
+                      default=False, action="store_true",
+                      help="Prevent select actions from requesting "
+                           "user confirmation")
     parser.add_option('--dry-run', default=False, action="store_true",
                       help="Don't actually execute the command, just print "
                            "output showing what WOULD happen.")
@@ -532,6 +546,14 @@
     print COMMANDS[command].__doc__ % {'prog': os.path.basename(sys.argv[0])}
 
 
+def user_confirm(prompt):
+    try:
+        answer = raw_input("%s [Y/n] " % (prompt,))
+        return answer.lower() in ("yes", "y")
+    except Exception:
+        return False
+
+
 if __name__ == '__main__':
     usage = """
 %prog <command> [options] [args]

=== modified file 'tests/functional/test_bin_glance.py'
--- tests/functional/test_bin_glance.py	2011-04-20 19:12:18 +0000
+++ tests/functional/test_bin_glance.py	2011-04-21 00:03:23 +0000
@@ -72,7 +72,7 @@
         self.assertTrue('MyImage' in image_data_line)
 
         # 3. Delete the image
-        cmd = "bin/glance --port=%d delete 1" % api_port
+        cmd = "bin/glance --port=%d --force delete 1" % api_port
 
         exitcode, out, err = execute(cmd)
 
@@ -195,7 +195,7 @@
             self.assertEqual('Added new image with ID: %i' % i, out.strip())
 
         # 2. Clear all images
-        cmd = "bin/glance --port=%d clear" % api_port
+        cmd = "bin/glance --port=%d --force clear" % api_port
         exitcode, out, err = execute(cmd)
         self.assertEqual(0, exitcode)