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
(cherry picked from commit 737dfca83c
)
This commit is contained in:
parent
931d39ec38
commit
2d5e84390e
|
@ -24,6 +24,7 @@ from oslo_utils import encodeutils
|
||||||
from oslo_utils import timeutils
|
from oslo_utils import timeutils
|
||||||
from oslo_utils import units
|
from oslo_utils import units
|
||||||
import six
|
import six
|
||||||
|
import taskflow
|
||||||
from taskflow.patterns import linear_flow as lf
|
from taskflow.patterns import linear_flow as lf
|
||||||
from taskflow import retry
|
from taskflow import retry
|
||||||
from taskflow import task
|
from taskflow import task
|
||||||
|
@ -513,6 +514,10 @@ class _ImportToStore(task.Task):
|
||||||
"""
|
"""
|
||||||
with self.action_wrapper as action:
|
with self.action_wrapper as action:
|
||||||
action.remove_location_for_store(self.backend)
|
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):
|
class _VerifyImageState(task.Task):
|
||||||
|
|
|
@ -5606,15 +5606,8 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest):
|
||||||
self.assertNotIn('file3', jsonutils.loads(response.text)['stores'])
|
self.assertNotIn('file3', jsonutils.loads(response.text)['stores'])
|
||||||
fail_key = 'os_glance_failed_import'
|
fail_key = 'os_glance_failed_import'
|
||||||
pend_key = 'os_glance_importing_to_stores'
|
pend_key = 'os_glance_importing_to_stores'
|
||||||
# NOTE(danms): This is bug #1891352, where a failed import when
|
self.assertEqual('file3', jsonutils.loads(response.text)[fail_key])
|
||||||
# all_stores_must_succeed=True will leave the failed store in the
|
self.assertEqual('', jsonutils.loads(response.text)[pend_key])
|
||||||
# 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])
|
|
||||||
|
|
||||||
# Copy newly created image to file2 and file3 stores and
|
# Copy newly created image to file2 and file3 stores and
|
||||||
# all_stores_must_succeed set to false.
|
# all_stores_must_succeed set to false.
|
||||||
|
|
|
@ -13,10 +13,12 @@
|
||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
|
import sys
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
from oslo_utils import units
|
from oslo_utils import units
|
||||||
|
import taskflow
|
||||||
|
|
||||||
import glance.async_.flows.api_image_import as import_flow
|
import glance.async_.flows.api_image_import as import_flow
|
||||||
from glance.common import exception
|
from glance.common import exception
|
||||||
|
@ -277,6 +279,36 @@ class TestImportToStoreTask(test_utils.BaseTestCase):
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
image.extra_properties['os_glance_importing_to_stores'], "store2")
|
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")
|
@mock.patch("glance.async_.flows.api_image_import.image_import")
|
||||||
def test_raises_when_all_stores_must_succeed(self, mock_import):
|
def test_raises_when_all_stores_must_succeed(self, mock_import):
|
||||||
img_repo = mock.MagicMock()
|
img_repo = mock.MagicMock()
|
||||||
|
|
Loading…
Reference in New Issue