Fix import failure status reporting when all_stores_must_succeed=True
As described in the referenced bug, if a store fails with all_stores_must_succeed=True, we never add that store to the failed list, nor remove it from the pending-import list, leaving a polling client to wait forever. This fixes that by making the revert handler of the import task do that if we are the one that failed. Closes-Bug: #1891352 Change-Id: I3571960bbfb4f8f0a716937b541f5b1594cd0e16
This commit is contained in:
parent
ec372377b1
commit
737dfca83c
|
@ -24,6 +24,7 @@ from oslo_utils import encodeutils
|
|||
from oslo_utils import timeutils
|
||||
from oslo_utils import units
|
||||
import six
|
||||
import taskflow
|
||||
from taskflow.patterns import linear_flow as lf
|
||||
from taskflow import retry
|
||||
from taskflow import task
|
||||
|
@ -513,6 +514,10 @@ class _ImportToStore(task.Task):
|
|||
"""
|
||||
with self.action_wrapper as action:
|
||||
action.remove_location_for_store(self.backend)
|
||||
action.remove_importing_stores([self.backend])
|
||||
if isinstance(result, taskflow.types.failure.Failure):
|
||||
# We are the store that failed, so add us to the failed list
|
||||
action.add_failed_stores([self.backend])
|
||||
|
||||
|
||||
class _VerifyImageState(task.Task):
|
||||
|
|
|
@ -5689,15 +5689,8 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest):
|
|||
self.assertNotIn('file3', jsonutils.loads(response.text)['stores'])
|
||||
fail_key = 'os_glance_failed_import'
|
||||
pend_key = 'os_glance_importing_to_stores'
|
||||
# NOTE(danms): This is bug #1891352, where a failed import when
|
||||
# all_stores_must_succeed=True will leave the failed store in the
|
||||
# importing list forever and never put it into the failed list.
|
||||
# When the bug is fixed, this is what should happen:
|
||||
# self.assertEqual('file3', jsonutils.loads(response.text)[fail_key])
|
||||
# self.assertEqual('', jsonutils.loads(response.text)[pend_key])
|
||||
# but until it is fixed, this asserts the *broken* behavior:
|
||||
self.assertEqual('', jsonutils.loads(response.text)[fail_key])
|
||||
self.assertEqual('file3', jsonutils.loads(response.text)[pend_key])
|
||||
self.assertEqual('file3', jsonutils.loads(response.text)[fail_key])
|
||||
self.assertEqual('', jsonutils.loads(response.text)[pend_key])
|
||||
|
||||
# Copy newly created image to file2 and file3 stores and
|
||||
# all_stores_must_succeed set to false.
|
||||
|
|
|
@ -13,11 +13,13 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import sys
|
||||
from unittest import mock
|
||||
|
||||
from glance_store import exceptions as store_exceptions
|
||||
from oslo_config import cfg
|
||||
from oslo_utils import units
|
||||
import taskflow
|
||||
|
||||
import glance.async_.flows.api_image_import as import_flow
|
||||
from glance.common import exception
|
||||
|
@ -277,6 +279,36 @@ class TestImportToStoreTask(test_utils.BaseTestCase):
|
|||
self.assertEqual(
|
||||
image.extra_properties['os_glance_importing_to_stores'], "store2")
|
||||
|
||||
def test_revert_updates_status_keys(self):
|
||||
img_repo = mock.MagicMock()
|
||||
task_repo = mock.MagicMock()
|
||||
wrapper = import_flow.ImportActionWrapper(img_repo, IMAGE_ID1)
|
||||
image_import = import_flow._ImportToStore(TASK_ID1, TASK_TYPE,
|
||||
task_repo, wrapper,
|
||||
"http://url",
|
||||
"store1", True,
|
||||
True)
|
||||
extra_properties = {"os_glance_importing_to_stores": "store1,store2"}
|
||||
image = self.img_factory.new_image(image_id=UUID1,
|
||||
extra_properties=extra_properties)
|
||||
img_repo.get.return_value = image
|
||||
|
||||
fail_key = 'os_glance_failed_import'
|
||||
pend_key = 'os_glance_importing_to_stores'
|
||||
|
||||
image_import.revert(None)
|
||||
self.assertEqual('store2', image.extra_properties[pend_key])
|
||||
|
||||
try:
|
||||
raise Exception('foo')
|
||||
except Exception:
|
||||
fake_exc_info = sys.exc_info()
|
||||
|
||||
extra_properties = {"os_glance_importing_to_stores": "store1,store2"}
|
||||
image_import.revert(taskflow.types.failure.Failure(fake_exc_info))
|
||||
self.assertEqual('store2', image.extra_properties[pend_key])
|
||||
self.assertEqual('store1', image.extra_properties[fail_key])
|
||||
|
||||
@mock.patch("glance.async_.flows.api_image_import.image_import")
|
||||
def test_raises_when_all_stores_must_succeed(self, mock_import):
|
||||
img_repo = mock.MagicMock()
|
||||
|
|
Loading…
Reference in New Issue