Make web-download revert all stores on fail

If web-download fails to stage the image, no subsequent import to
store tasks could have run. Thus, we should remove all of them from
"importing" and add them to "failed" so that it is obvious from the
outside that we failed.

This is another good reason for having the $image/tasks API, but we
also should not continue advertising "yes we're importing to $store"
when we are not.

NOTE(lyarwood): This includes a partial squashed revert of the
.zuul.yaml changes in Ifd55c44fef4e2187052d77084dc9c0fa9c9a0d16 that
removed nova-ceph-multistore. This is reintroduced here to ensure this
change resolves the stable/victoria specific bug #1919993 and the
original bug #1914826.

Change-Id: Iebbb2dcb767ecf3c965f34f1ca04af20a2039be1
Closes-Bug: #1914826
Closes-Bug: #1919993
(cherry picked from commit cbb2af6e329c1bf6c1c39cb59f95aaad2c0d7a07)

3 files changed, 50 insertions, 15 deletions

diff --git a/.zuul.yaml b/.zuul.yaml
index d140055f0..d1377dd66 100644
--- a/.zuul.yaml
+++ b/.zuul.yaml
@@ -312,6 +312,7 @@
             irrelevant-files: *tempest-irrelevant-files
         - tempest-ipv6-only:
             irrelevant-files: *tempest-irrelevant-files
+        - nova-ceph-multistore
 
     gate:
       jobs:
@@ -325,6 +326,7 @@
             irrelevant-files: *tempest-irrelevant-files
         - tempest-ipv6-only:
             irrelevant-files: *tempest-irrelevant-files
+        - nova-ceph-multistore
     experimental:
       jobs:
         - glance-tox-py38-glance_store-tips
diff --git a/glance/async_/flows/_internal_plugins/web_download.py b/glance/async_/flows/_internal_plugins/web_download.py
index 6b824c0a1..b76346f0f 100644
--- a/glance/async_/flows/_internal_plugins/web_download.py
+++ b/glance/async_/flows/_internal_plugins/web_download.py
@@ -35,12 +35,13 @@ class _WebDownload(task.Task):
 
     default_provides = 'file_uri'
 
-    def __init__(self, task_id, task_type, uri, action_wrapper):
+    def __init__(self, task_id, task_type, uri, action_wrapper, stores):
         self.task_id = task_id
         self.task_type = task_type
         self.image_id = action_wrapper.image_id
         self.uri = uri
         self.action_wrapper = action_wrapper
+        self.stores = stores
         self._path = None
         super(_WebDownload, self).__init__(
             name='%s-WebDownload-%s' % (task_type, task_id))
@@ -140,8 +141,13 @@ class _WebDownload(task.Task):
                        'image_id': self.image_id})
             # NOTE(abhishekk): Revert image state back to 'queued' as
             # something went wrong.
+            # NOTE(danms): If we failed to stage the image, then none
+            # of the _ImportToStore() tasks could have run, so we need
+            # to move all stores out of "importing" and into "failed".
             with self.action_wrapper as action:
                 action.set_image_status('queued')
+                action.remove_importing_stores(self.stores)
+                action.add_failed_stores(self.stores)
 
         # NOTE(abhishekk): Deleting partial image data from staging area
         if self._path is not None:
@@ -171,7 +177,8 @@ def get_flow(**kwargs):
     task_type = kwargs.get('task_type')
     uri = kwargs.get('import_req')['method'].get('uri')
     action_wrapper = kwargs.get('action_wrapper')
+    stores = kwargs.get('backend', [None])
 
     return lf.Flow(task_type).add(
-        _WebDownload(task_id, task_type, uri, action_wrapper),
+        _WebDownload(task_id, task_type, uri, action_wrapper, stores),
     )
diff --git a/glance/tests/unit/async_/flows/test_web_download.py b/glance/tests/unit/async_/flows/test_web_download.py
index 4a1ee9c7c..1f4516a67 100644
--- a/glance/tests/unit/async_/flows/test_web_download.py
+++ b/glance/tests/unit/async_/flows/test_web_download.py
@@ -69,7 +69,8 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
     @mock.patch.object(filesystem.Store, 'add')
     def test_web_download(self, mock_add):
         web_download_task = web_download._WebDownload(
-            self.task.task_id, self.task_type, self.uri, self.action_wrapper)
+            self.task.task_id, self.task_type, self.uri, self.action_wrapper,
+            ['foo'])
         with mock.patch.object(script_utils,
                                'get_image_data_iter') as mock_iter:
             mock_add.return_value = ["path", 4]
@@ -81,7 +82,8 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
     @mock.patch.object(filesystem.Store, 'add')
     def test_web_download_with_content_length(self, mock_add):
         web_download_task = web_download._WebDownload(
-            self.task.task_id, self.task_type, self.uri, self.action_wrapper)
+            self.task.task_id, self.task_type, self.uri, self.action_wrapper,
+            ['foo'])
         with mock.patch.object(script_utils,
                                'get_image_data_iter') as mock_iter:
             mock_iter.return_value.headers = {'content-length': '4'}
@@ -93,7 +95,8 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
     @mock.patch.object(filesystem.Store, 'add')
     def test_web_download_with_invalid_content_length(self, mock_add):
         web_download_task = web_download._WebDownload(
-            self.task.task_id, self.task_type, self.uri, self.action_wrapper)
+            self.task.task_id, self.task_type, self.uri, self.action_wrapper,
+            ['foo'])
         with mock.patch.object(script_utils,
                                'get_image_data_iter') as mock_iter:
             mock_iter.return_value.headers = {'content-length': "not_valid"}
@@ -105,7 +108,8 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
     @mock.patch.object(filesystem.Store, 'add')
     def test_web_download_fails_when_data_size_different(self, mock_add):
         web_download_task = web_download._WebDownload(
-            self.task.task_id, self.task_type, self.uri, self.action_wrapper)
+            self.task.task_id, self.task_type, self.uri, self.action_wrapper,
+            ['foo'])
         with mock.patch.object(script_utils,
                                'get_image_data_iter') as mock_iter:
             mock_iter.return_value.headers = {'content-length': '4'}
@@ -118,7 +122,8 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
         self.config(node_staging_uri=None)
         self.assertRaises(glance.common.exception.BadTaskConfiguration,
                           web_download._WebDownload, self.task.task_id,
-                          self.task_type, self.uri, self.action_wrapper)
+                          self.task_type, self.uri, self.action_wrapper,
+                          ['foo'])
 
     @mock.patch.object(cfg.ConfigOpts, "set_override")
     def test_web_download_node_store_initialization_failed(self,
@@ -127,12 +132,14 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
             mock_load_store.return_value = None
             self.assertRaises(glance.common.exception.BadTaskConfiguration,
                               web_download._WebDownload, self.task.task_id,
-                              self.task_type, self.uri, self.action_wrapper)
+                              self.task_type, self.uri, self.action_wrapper,
+                              ['foo'])
             mock_override.assert_called()
 
     def test_web_download_failed(self):
         web_download_task = web_download._WebDownload(
-            self.task.task_id, self.task_type, self.uri, self.action_wrapper)
+            self.task.task_id, self.task_type, self.uri, self.action_wrapper,
+            ['foo'])
         with mock.patch.object(script_utils,
                                "get_image_data_iter") as mock_iter:
             mock_iter.side_effect = glance.common.exception.NotFound
@@ -180,8 +187,12 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
     @mock.patch("glance.async_.flows._internal_plugins.web_download.store_api")
     def test_web_download_revert_with_failure(self, mock_store_api,
                                               mock_add):
+        image = self.image_repo.get.return_value
+        image.extra_properties['os_glance_importing_to_stores'] = 'foo'
+        image.extra_properties['os_glance_failed_import'] = ''
         web_download_task = web_download._WebDownload(
-            self.task.task_id, self.task_type, self.uri, self.action_wrapper)
+            self.task.task_id, self.task_type, self.uri, self.action_wrapper,
+            ['foo'])
         with mock.patch.object(script_utils,
                                'get_image_data_iter') as mock_iter:
             mock_iter.return_value.headers = {'content-length': '4'}
@@ -196,6 +207,10 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
         # NOTE(danms): Since we told revert that we were not at fault,
         # we should not have updated the image.
         self.image_repo.save.assert_not_called()
+        self.assertEqual(
+            'foo', image.extra_properties['os_glance_importing_to_stores'])
+        self.assertEqual(
+            '', image.extra_properties['os_glance_failed_import'])
 
     @mock.patch("glance.async_.flows._internal_plugins.web_download.store_api")
     def test_web_download_revert_without_failure_multi_store(self,
@@ -206,7 +221,8 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
         }
         self.config(enabled_backends=enabled_backends)
         web_download_task = web_download._WebDownload(
-            self.task.task_id, self.task_type, self.uri, self.action_wrapper)
+            self.task.task_id, self.task_type, self.uri, self.action_wrapper,
+            ['foo'])
         web_download_task._path = "/path/to_downloaded_data"
         web_download_task.revert("/path/to_downloaded_data")
         mock_store_api.delete.assert_called_once_with(
@@ -217,24 +233,33 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
                                                            mock_store_api):
         image = self.image_repo.get.return_value
         image.status = 'importing'
+        image.extra_properties['os_glance_importing_to_stores'] = 'foo'
+        image.extra_properties['os_glance_failed_import'] = ''
         result = failure.Failure.from_exception(
             glance.common.exception.ImportTaskError())
         web_download_task = web_download._WebDownload(
-            self.task.task_id, self.task_type, self.uri, self.action_wrapper)
+            self.task.task_id, self.task_type, self.uri, self.action_wrapper,
+            ['foo'])
         web_download_task.revert(result)
         mock_store_api.delete_from_backend.assert_not_called()
 
         # NOTE(danms): Since we told revert that we were the problem,
-        # we should have updated the image status
+        # we should have updated the image status and moved the stores
+        # to the failed list.
         self.image_repo.save.assert_called_once_with(image, 'importing')
         self.assertEqual('queued', image.status)
+        self.assertEqual(
+            '', image.extra_properties['os_glance_importing_to_stores'])
+        self.assertEqual(
+            'foo', image.extra_properties['os_glance_failed_import'])
 
     @mock.patch("glance.async_.flows._internal_plugins.web_download.store_api")
     def test_web_download_revert_with_failure_with_path(self, mock_store_api):
         result = failure.Failure.from_exception(
             glance.common.exception.ImportTaskError())
         web_download_task = web_download._WebDownload(
-            self.task.task_id, self.task_type, self.uri, self.action_wrapper)
+            self.task.task_id, self.task_type, self.uri, self.action_wrapper,
+            ['foo'])
         web_download_task._path = "/path/to_downloaded_data"
         web_download_task.revert(result)
         mock_store_api.delete_from_backend.assert_called_once_with(
@@ -246,7 +271,8 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
             glance.common.exception.ImportTaskError())
         mock_store_api.delete_from_backend.side_effect = Exception
         web_download_task = web_download._WebDownload(
-            self.task.task_id, self.task_type, self.uri, self.action_wrapper)
+            self.task.task_id, self.task_type, self.uri, self.action_wrapper,
+            ['foo'])
         web_download_task._path = "/path/to_downloaded_data"
         # this will verify that revert does not break because of failure
         # while deleting data in staging area