From 21d5e907575a2042f1d0daaa9658a8758f619a1c Mon Sep 17 00:00:00 2001
From: Eoghan Glynn <eglynn@redhat.com>
Date: Fri, 25 Jan 2013 15:47:33 +0000
Subject: [PATCH] Avoid stuck task_state on snapshot image failure

Fixes bug LP 1101136

Previously if the glance interaction failed prior to an
instance being snapshot'd or backed up, the task state
remained stuck at image_snapshot/backup.

The normal task state reversion logic did not kick in,
as this is limited to the compute layer, whereas the
intial glance interaction occurs within the API layer.

Now, we avoid this problem by delaying setting the task
state until the initial image creation has completed.

Change-Id: Id498ae6b3674306743013e4fe99837da8e2031b5
---
 nova/compute/api.py                | 23 +++++++++--------
 nova/tests/compute/test_compute.py | 40 ++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 3742a0863ac..419a171cc1a 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1243,17 +1243,6 @@ def _create_image(self, context, instance, name, image_type,
         else:
             raise Exception(_('Image type not recognized %s') % image_type)
 
-        # change instance state and notify
-        old_vm_state = instance["vm_state"]
-        old_task_state = instance["task_state"]
-
-        self.db.instance_test_and_set(
-                context, instance_uuid, 'task_state', [None], task_state)
-
-        notifications.send_update_with_states(context, instance, old_vm_state,
-                instance["vm_state"], old_task_state, instance["task_state"],
-                service="api", verify_states=True)
-
         properties = {
             'instance_uuid': instance_uuid,
             'user_id': str(context.user_id),
@@ -1284,6 +1273,18 @@ def _create_image(self, context, instance, name, image_type,
         sent_meta['properties'] = properties
 
         recv_meta = self.image_service.create(context, sent_meta)
+
+        # change instance state and notify
+        old_vm_state = instance["vm_state"]
+        old_task_state = instance["task_state"]
+
+        self.db.instance_test_and_set(
+                context, instance_uuid, 'task_state', [None], task_state)
+
+        notifications.send_update_with_states(context, instance, old_vm_state,
+                instance["vm_state"], old_task_state, instance["task_state"],
+                service="api", verify_states=True)
+
         self.compute_rpcapi.snapshot_instance(context, instance=instance,
                 image_id=recv_meta['id'], image_type=image_type,
                 backup_type=backup_type, rotation=rotation)
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index e20f1c0fa97..bd84ce3ff30 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -3610,6 +3610,46 @@ def fake_show(*args):
 
         db.instance_destroy(self.context, instance['uuid'])
 
+    def test_snapshot_image_service_fails(self):
+        # Ensure task_state remains at None if image service fails.
+        def fake_create(*args, **kwargs):
+            raise test.TestingException()
+
+        restore = getattr(fake_image._FakeImageService, 'create')
+        self.stubs.Set(fake_image._FakeImageService, 'create', fake_create)
+
+        instance = self._create_fake_instance()
+        self.assertRaises(test.TestingException,
+                          self.compute_api.snapshot,
+                          self.context,
+                          instance,
+                          'no_image_snapshot')
+
+        self.stubs.Set(fake_image._FakeImageService, 'create', restore)
+        db_instance = db.instance_get_all(context.get_admin_context())[0]
+        self.assertTrue(db_instance['task_state'] is None)
+
+    def test_backup_image_service_fails(self):
+        # Ensure task_state remains at None if image service fails.
+        def fake_create(*args, **kwargs):
+            raise test.TestingException()
+
+        restore = getattr(fake_image._FakeImageService, 'create')
+        self.stubs.Set(fake_image._FakeImageService, 'create', fake_create)
+
+        instance = self._create_fake_instance()
+        self.assertRaises(test.TestingException,
+                          self.compute_api.backup,
+                          self.context,
+                          instance,
+                          'no_image_backup',
+                          'DAILY',
+                          0)
+
+        self.stubs.Set(fake_image._FakeImageService, 'create', restore)
+        db_instance = db.instance_get_all(context.get_admin_context())[0]
+        self.assertTrue(db_instance['task_state'] is None)
+
     def test_backup(self):
         """Can't backup an instance which is already being backed up."""
         instance = self._create_fake_instance()