Fix active image when all uploads fail

This fix is for the case where all stores fails and
the end result is active image with no image data.

Change-Id: Ie7558eeae488406a1696f0ee1bf40b644ae6d8cc
Partial-Bug: #1889640

glance/async_/flows/api_image_import.py

@@ -76,6 +76,12 @@ CONF.register_opts(api_import_opts, group='image_import_opts')
 # need to duplicate what we have already for example in base_import.py.
 
 
+class _NoStoresSucceeded(exception.GlanceException):
+
+    def __init__(self, message):
+        super(_NoStoresSucceeded, self).__init__(message)
+
+
 class ImportActionWrapper(object):
     """Wrapper for all the image metadata operations we do during an import.
 
@@ -466,29 +472,30 @@ class _ImportToStore(task.Task):
             action.remove_location_for_store(self.backend)
 
 
-class _SaveImage(task.Task):
+class _VerifyImageState(task.Task):
 
     def __init__(self, task_id, task_type, action_wrapper, import_method):
         self.task_id = task_id
         self.task_type = task_type
         self.action_wrapper = action_wrapper
         self.import_method = import_method
-        super(_SaveImage, self).__init__(
+        super(_VerifyImageState, self).__init__(
-            name='%s-SaveImage-%s' % (task_type, task_id))
+            name='%s-VerifyImageState-%s' % (task_type, task_id))
 
     def execute(self):
-        """Transition image status to active
+        """Verify we have active image
 
         :param image_id: Glance Image ID
         """
         with self.action_wrapper as action:
-            if (self.import_method != 'copy-image'
+            if action.image_status != 'active':
-                    and action.image_status == 'importing'):
+                raise _NoStoresSucceeded(_('None of the uploads finished!'))
-                # NOTE(flaper87): THIS IS WRONG!
+
-                # we should be doing atomic updates to avoid
+    def revert(self, result, **kwargs):
-                # race conditions. This happens in other places
+        """Set back to queued if this wasn't copy-image job."""
-                # too.
+        with self.action_wrapper as action:
-                action.set_image_status('active')
+            if self.import_method != 'copy-image':
+                action.set_image_status('queued')
 
 
 class _CompleteTask(task.Task):
@@ -613,11 +620,11 @@ def get_flow(**kwargs):
     delete_task = lf.Flow(task_type).add(_DeleteFromFS(task_id, task_type))
     flow.add(delete_task)
 
-    save_task = _SaveImage(task_id,
+    verify_task = _VerifyImageState(task_id,
-                           task_type,
+                                    task_type,
-                           action_wrapper,
+                                    action_wrapper,
-                           import_method)
+                                    import_method)
-    flow.add(save_task)
+    flow.add(verify_task)
 
     complete_task = _CompleteTask(task_id,
                                   task_type,

glance/tests/unit/async_/flows/test_api_image_import.py

@@ -213,6 +213,42 @@ class TestImportCopyImageTask(test_utils.BaseTestCase):
         img_repo.save.assert_not_called()
 
 
+class TestVerifyImageStateTask(test_utils.BaseTestCase):
+    def test_verify_active_status(self):
+        fake_img = mock.MagicMock(status='active')
+        mock_repo = mock.MagicMock()
+        mock_repo.get.return_value = fake_img
+        wrapper = import_flow.ImportActionWrapper(mock_repo, IMAGE_ID1)
+
+        task = import_flow._VerifyImageState(TASK_ID1, TASK_TYPE,
+                                             wrapper, 'anything!')
+
+        task.execute()
+
+        fake_img.status = 'importing'
+        self.assertRaises(import_flow._NoStoresSucceeded,
+                          task.execute)
+
+    def test_revert_copy_status_unchanged(self):
+        wrapper = mock.MagicMock()
+        task = import_flow._VerifyImageState(TASK_ID1, TASK_TYPE,
+                                             wrapper, 'copy-image')
+        task.revert(mock.sentinel.result)
+
+        # If we are doing copy-image, no state update should be made
+        wrapper.__enter__.return_value.set_image_status.assert_not_called()
+
+    def test_reverts_state_nocopy(self):
+        wrapper = mock.MagicMock()
+        task = import_flow._VerifyImageState(TASK_ID1, TASK_TYPE,
+                                             wrapper, 'glance-direct')
+        task.revert(mock.sentinel.result)
+
+        # Except for copy-image, image state should revert to queued
+        action = wrapper.__enter__.return_value
+        action.set_image_status.assert_called_once_with('queued')
+
+
 class TestImportActionWrapper(test_utils.BaseTestCase):
     def test_wrapper_success(self):
         mock_repo = mock.MagicMock()

glance/tests/unit/async_/test_async.py

@@ -80,7 +80,7 @@ class TestImportTaskFlow(test_utils.BaseTestCase):
         self.config(node_staging_uri='file:///tmp/staging')
         store.create_stores(CONF)
         self.base_flow = ['ConfigureStaging', 'ImportToStore',
-                          'DeleteFromFS', 'SaveImage',
+                          'DeleteFromFS', 'VerifyImageState',
                           'CompleteTask']
         self.import_plugins = ['Convert_Image',
                                'Decompress_Image',