From ebeb31e6367e39e81ea4ea62feeebb9f42f03109 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Wed, 12 Aug 2020 09:22:30 -0700 Subject: [PATCH] Poll for final state on test_copy_image_revert_lifecycle() This test currently simulates failure by pre-deleting the store directory for 'file3' which is the second of a two-store import operation. The goal is to assert that the later failure reverts the import of the earlier 'file2' store. However, the way the polling loop works is that we break out once 'file2' has completed, and then assume that 'file3' has already failed and reverted. This is a race, and one we're losing consistently in CI. This patch waits for the failure of 'file3' to be reported, as well as the revert of 'file2' to occur before exiting the polling loop and checking the final state of things to ensure that the revert has actually happened. This addresses the non-determinism inherent in the original test. Change-Id: I11c7edaefc96236d2757acfb70d9c338c0f51348 (cherry picked from commit 6c96319eeb36274bed31466941dd83e7d9713cfd) --- glance/tests/functional/ft_utils.py | 36 +++++++++++++++++++++++ glance/tests/functional/v2/test_images.py | 29 ++++++++++-------- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/glance/tests/functional/ft_utils.py b/glance/tests/functional/ft_utils.py index 26258488a7..cbb0759763 100644 --- a/glance/tests/functional/ft_utils.py +++ b/glance/tests/functional/ft_utils.py @@ -17,6 +17,7 @@ import six import time from oslo_serialization import jsonutils +from oslo_utils import timeutils import requests from six.moves import http_client as http @@ -129,3 +130,38 @@ def wait_for_copying(request_path, request_headers, stores=[], entity_id = request_path.rsplit('/', 1)[1] msg = "Entity {0} failed to copy image to stores '{1}' within {2} sec" raise Exception(msg.format(entity_id, ",".join(stores), max_sec)) + + +def poll_entity(url, headers, callback, max_sec=10, delay_sec=0.2, + require_success=True): + """Poll a given URL passing the parsed entity to a callback. + + This is a utility method that repeatedly GETs a URL, and calls + a callback with the result. The callback determines if we should + keep polling by returning True (up to the timeout). + + :param url: The url to fetch + :param headers: The request headers to use for the fetch + :param callback: A function that takes the parsed entity and is expected + to return True if we should keep polling + :param max_sec: The overall timeout before we fail + :param delay_sec: The time between fetches + :param require_success: Assert resp_code is http.OK each time before + calling the callback + """ + + timer = timeutils.StopWatch(max_sec) + timer.start() + + while not timer.expired(): + resp = requests.get(url, headers=headers) + if require_success and resp.status_code != http.OK: + raise Exception( + 'Received %i response from server' % resp.status_code) + entity = resp.json() + keep_polling = callback(entity) + if keep_polling is not True: + return keep_polling + time.sleep(delay_sec) + + raise Exception('Poll timeout if %i seconds exceeded!' % max_sec) diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index da1d72f758..2656b185a6 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/glance/tests/functional/v2/test_images.py @@ -5588,19 +5588,24 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): response = requests.post(path, headers=headers, data=data) self.assertEqual(http.ACCEPTED, response.status_code) - # Verify image is copied - # NOTE(abhishekk): As import is a async call we need to provide - # some timelap to complete the call. - path = self._url('/v2/images/%s' % image_id) - func_utils.wait_for_copying(request_path=path, - request_headers=self._headers(), - stores=['file2'], - max_sec=10, - delay_sec=0.2, - start_delay_sec=1, - failure_scenario=True) + def poll_callback(image): + # NOTE(danms): We need to wait for the specific + # arrangement we're expecting, which is that file3 has + # failed, nothing else is importing, and file2 has been + # removed from stores by the revert. + return not (image['os_glance_importing_to_stores'] == '' and + image['os_glance_failed_import'] == 'file3' and + image['stores'] == 'file1') - # Ensure data is not deleted from existing stores on failure + func_utils.poll_entity(self._url('/v2/images/%s' % image_id), + self._headers(), + poll_callback) + + # Here we check that the failure of 'file3' caused 'file2' to + # be removed from image['stores'], and that 'file3' is reported + # as failed in the appropriate status list. Since the import + # started with 'store1' being populated, that should remain, + # but 'store2' should be reverted/removed. path = self._url('/v2/images/%s' % image_id) response = requests.get(path, headers=self._headers()) self.assertEqual(http.OK, response.status_code)