From 197e862bfeb7d6951d09cfd0a1bb61f02dc416d9 Mon Sep 17 00:00:00 2001
From: "jaypipes@gmail.com" <>
Date: Fri, 22 Apr 2011 13:57:19 -0400
Subject: [PATCH] Fix for LP Bug #768969.

Two things were happening that this patch corrects:

a) If adding an image fails, the glance add output says that
   adding the image failed, however, doing a glance index would
   show the image. This was because the call to get all public
   images was not correctly filtering the result for 'active'
   status images. The image failing to be added causes the image
   status to be 'killed', and so killed images should not appear
   in the output of glance index or glance detail.
b) glance show <ID> was not showing the status of the image, so
   it was not clear that the image, while not added successfully,
   was still in the registry, but in a 'killed' status.

I added a note to the output of the failed add command that the
Glance registry may still have an image record, but the status
would likely be in the 'killed' state.

Added a functional test case that verified the behaviour in the bug
and verified the fix, once coded.
---
 bin/glance                          |  3 ++
 glance/registry/db/api.py           |  1 +
 tests/functional/__init__.py        |  3 +-
 tests/functional/test_bin_glance.py | 65 ++++++++++++++++++++++++++++-
 tests/utils.py                      | 15 ++++++-
 5 files changed, 83 insertions(+), 4 deletions(-)

diff --git a/bin/glance b/bin/glance
index 6963e219d1..9f73ab68a8 100755
--- a/bin/glance
+++ b/bin/glance
@@ -77,6 +77,7 @@ def print_image_formatted(client, image):
     print "Id: %s" % image['id']
     print "Public: " + (image['is_public'] and "Yes" or "No")
     print "Name: %s" % image['name']
+    print "Status: %s" % image['status']
     print "Size: %d" % int(image['size'])
     print "Location: %s" % image['location']
     print "Disk format: %s" % image['disk_format']
@@ -208,6 +209,8 @@ EXAMPLES
             pieces = str(e).split('\n')
             for piece in pieces:
                 print piece
+            print ("Note: Your image metadata may still be in the registry, but "
+                   "the image's status will likely be 'killed'.")
             return FAILURE
     else:
         print "Dry run. We would have done the following:"
diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py
index 8cc48d590a..87cb90d6b1 100644
--- a/glance/registry/db/api.py
+++ b/glance/registry/db/api.py
@@ -146,6 +146,7 @@ def image_get_all_public(context):
                    options(joinedload(models.Image.properties)).\
                    filter_by(deleted=_deleted(context)).\
                    filter_by(is_public=True).\
+                   filter_by(status='active').\
                    all()
 
 
diff --git a/tests/functional/__init__.py b/tests/functional/__init__.py
index d5d8b7d27a..239e200e28 100644
--- a/tests/functional/__init__.py
+++ b/tests/functional/__init__.py
@@ -71,6 +71,7 @@ class FunctionalTest(unittest.TestCase):
 
         self.verbose = True
         self.debug = True
+        self.default_store = 'file'
         self.test_id = random.randint(0, 100000)
         self.test_dir = os.path.join("/", "tmp", "test.%d" % self.test_id)
 
@@ -175,7 +176,7 @@ debug = %(debug)s
 [app:glance-api]
 paste.app_factory = glance.server:app_factory
 filesystem_store_datadir=%(image_dir)s
-default_store = file
+default_store = %(default_store)s
 bind_host = 0.0.0.0
 bind_port = %(api_port)s
 registry_host = 0.0.0.0
diff --git a/tests/functional/test_bin_glance.py b/tests/functional/test_bin_glance.py
index 9ad84b8cfc..55c1faaaf6 100644
--- a/tests/functional/test_bin_glance.py
+++ b/tests/functional/test_bin_glance.py
@@ -18,6 +18,7 @@
 """Functional test case that utilizes the bin/glance CLI tool"""
 
 import os
+import tempfile
 import unittest
 
 from tests import functional
@@ -122,7 +123,7 @@ class TestBinGlance(functional.FunctionalTest):
         self.assertEqual(0, exitcode)
         self.assertEqual('Added new image with ID: 1', out.strip())
 
-        # 2. Verify image added as public image
+        # 2. Verify image does not appear as a public image
         cmd = "bin/glance --port=%d index" % api_port
 
         exitcode, out, err = execute(cmd)
@@ -171,6 +172,68 @@ class TestBinGlance(functional.FunctionalTest):
 
         self.stop_servers()
 
+    def test_killed_image_not_in_index(self):
+        """
+        We test conditions that produced LP Bug #768969, where an image
+        in the 'killed' status is displayed in the output of glance index,
+        and the status column is not displayed in the output of
+        glance show <ID>.
+
+            Start servers with Swift backend and a bad auth URL, and then:
+            0. Verify no public images in index
+            1. Attempt to add an image
+            2. Verify the image does NOT appear in the index output
+            3. Verify the status of the image is displayed in the show output
+               and is in status 'killed'
+        """
+
+        self.cleanup()
+
+        # Start servers with a Swift backend and a bad auth URL
+        options = {'default_store': 'swift',
+                   'swift_store_auth_address': 'badurl'}
+        api_port, reg_port, conf_file = self.start_servers(**options)
+
+        # 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. Attempt to add an image
+        with tempfile.NamedTemporaryFile() as image_file:
+            image_file.write("XXX")
+            image_file.flush()
+            image_file_name = image_file.name
+            cmd = ("bin/glance --port=%d add name=Jonas is_public=True "
+                   "disk_format=qcow2 container_format=bare < %s"
+                   % (api_port, image_file_name))
+
+            exitcode, out, err = execute(cmd, raise_error=False)
+
+            self.assertNotEqual(0, exitcode)
+            self.assertTrue('Failed to add image.' in out)
+
+        # 2. Verify image does not appear as public image
+        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())
+
+        # 3. Verify image status in show is 'killed'
+        cmd = "bin/glance --port=%d show 1" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        self.assertTrue('Status: killed' in out)
+
+        self.stop_servers()
+
     @functional.runs_sql
     def test_add_clear(self):
         """
diff --git a/tests/utils.py b/tests/utils.py
index 40436136d7..b0da82ef08 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -22,7 +22,18 @@ import socket
 import subprocess
 
 
-def execute(cmd):
+def execute(cmd, raise_error=True):
+    """
+    Executes a command in a subprocess. Returns a tuple
+    of (exitcode, out, err), where out is the string output
+    from stdout and err is the string output from stderr when
+    executing the command.
+
+    :param cmd: Command string to execute
+    :param raise_error: If returncode is not 0 (success), then
+                        raise a RuntimeError? Default: True)
+    """
+
     env = os.environ.copy()
 
     # Make sure that we use the programs in the
@@ -37,7 +48,7 @@ def execute(cmd):
     result = process.communicate()
     (out, err) = result
     exitcode = process.returncode
-    if process.returncode != 0:
+    if process.returncode != 0 and raise_error:
         msg = "Command %(cmd)s did not succeed. Returned an exit "\
               "code of %(exitcode)d."\
               "\n\nSTDOUT: %(out)s"\