Use common "waiters.wait_for_image_status" function everywhere

In Tempest we have the waiters.wait_for_image_status function that
is already used in some places. So this commit replaces the use of the
wait_for_image_status method defined in the images_client.py file with
the use of the waiters.wait_for_image_status function.

Change-Id: If42cfbfa1f7dae5af276e80f38f6294ced00ac5b

tempest/api/compute/servers/test_server_actions.py

@@ -312,7 +312,8 @@ def _clean_oldest_backup(oldest_backup):
 
         image1_id = data_utils.parse_image_id(resp['location'])
         self.addCleanup(_clean_oldest_backup, image1_id)
-        self.os.image_client.wait_for_image_status(image1_id, 'active')
+        waiters.wait_for_image_status(self.os.image_client,
+                                      image1_id, 'active')
 
         backup2 = data_utils.rand_name('backup-2')
         waiters.wait_for_server_status(self.client, self.server_id, 'ACTIVE')
@@ -322,7 +323,8 @@ def _clean_oldest_backup(oldest_backup):
                                          name=backup2).response
         image2_id = data_utils.parse_image_id(resp['location'])
         self.addCleanup(self.os.image_client.delete_image, image2_id)
-        self.os.image_client.wait_for_image_status(image2_id, 'active')
+        waiters.wait_for_image_status(self.os.image_client,
+                                      image2_id, 'active')
 
         # verify they have been created
         properties = {

tempest/api/image/v1/test_images.py

@@ -17,6 +17,7 @@
 
 from tempest.api.image import base
 from tempest.common.utils import data_utils
+from tempest.common import waiters
 from tempest import config
 from tempest import exceptions
 from tempest import test
@@ -95,7 +96,7 @@ def test_register_http_image(self):
         image_id = body.get('id')
         self.assertEqual('New Http Image', body.get('name'))
         self.assertFalse(body.get('is_public'))
-        self.client.wait_for_image_status(image_id, 'active')
+        waiters.wait_for_image_status(self.client, image_id, 'active')
         self.client.show_image(image_id)
 
     @test.idempotent_id('05b19d55-140c-40d0-b36b-fafd774d421b')

tempest/api/volume/test_volumes_actions.py

@@ -125,7 +125,7 @@ def test_volume_upload(self):
             disk_format=CONF.volume.disk_format)['os-volume_upload_image']
         image_id = body["image_id"]
         self.addCleanup(self._cleanup_image, image_id)
-        self.image_client.wait_for_image_status(image_id, 'active')
+        waiters.wait_for_image_status(self.image_client, image_id, 'active')
         waiters.wait_for_volume_status(self.client,
                                        self.volume['id'], 'available')
 

tempest/clients.py

@@ -131,7 +131,8 @@
 from tempest.services.identity.v3.json.users_clients import \
     UsersClient as UsersV3Client
 from tempest.services.image.v1.json.images_client import ImagesClient
-from tempest.services.image.v2.json.images_client import ImagesClientV2
+from tempest.services.image.v2.json.images_client import \
+    ImagesClient as ImagesV2Client
 from tempest.services.network.json.routers_client import RoutersClient
 from tempest.services.object_storage.account_client import AccountClient
 from tempest.services.object_storage.container_client import ContainerClient
@@ -330,7 +331,7 @@ def __init__(self, credentials, service=None):
                 build_interval=CONF.image.build_interval,
                 build_timeout=CONF.image.build_timeout,
                 **self.default_params)
-            self.image_client_v2 = ImagesClientV2(
+            self.image_client_v2 = ImagesV2Client(
                 self.auth_provider,
                 CONF.image.catalog_type,
                 CONF.image.region or CONF.identity.region,

tempest/cmd/javelin.py

@@ -233,7 +233,7 @@ def __init__(self, user, pw, tenant):
                                                   **object_storage_params)
         self.containers = container_client.ContainerClient(
             _auth, **object_storage_params)
-        self.images = images_client.ImagesClientV2(
+        self.images = images_client.ImagesClient(
             _auth,
             CONF.image.catalog_type,
             CONF.image.region or CONF.identity.region,

tempest/common/waiters.py

@@ -19,6 +19,7 @@
 from tempest import exceptions
 from tempest.lib.common.utils import misc as misc_utils
 from tempest.lib import exceptions as lib_exc
+from tempest.services.image.v1.json import images_client as images_v1_client
 
 CONF = config.CONF
 LOG = logging.getLogger(__name__)
@@ -122,42 +123,44 @@ def wait_for_image_status(client, image_id, status):
     The client should have a show_image(image_id) method to get the image.
     The client should also have build_interval and build_timeout attributes.
     """
-    image = client.show_image(image_id)
-    # Compute image client return response wrapped in 'image' element
-    # which is not case with glance image client.
-    if 'image' in image:
-        image = image['image']
+    if isinstance(client, images_v1_client.ImagesClient):
+        # The 'check_image' method is used here because the show_image method
+        # returns image details plus the image itself which is very expensive.
+        # The 'check_image' method returns just image details.
+        show_image = client.check_image
+    else:
+        show_image = client.show_image
+
+    current_status = 'An unknown status'
     start = int(time.time())
-
-    while image['status'] != status:
-        time.sleep(client.build_interval)
-        image = client.show_image(image_id)
-        # Compute image client return response wrapped in 'image' element
-        # which is not case with glance image client.
+    while int(time.time()) - start < client.build_timeout:
+        image = show_image(image_id)
+        # Compute image client returns response wrapped in 'image' element
+        # which is not case with Glance image client.
         if 'image' in image:
             image = image['image']
-        status_curr = image['status']
-        if status_curr == 'ERROR':
-            raise exceptions.AddImageException(image_id=image_id)
 
-        # check the status again to avoid a false negative where we hit
-        # the timeout at the same time that the image reached the expected
-        # status
-        if status_curr == status:
+        current_status = image['status']
+        if current_status == status:
             return
+        if current_status.lower() == 'killed':
+            raise exceptions.ImageKilledException(image_id=image_id,
+                                                  status=status)
+        if current_status.lower() == 'error':
+            raise exceptions.AddImageException(image_id=image_id)
 
-        if int(time.time()) - start >= client.build_timeout:
-            message = ('Image %(image_id)s failed to reach %(status)s state'
-                       '(current state %(status_curr)s) '
-                       'within the required time (%(timeout)s s).' %
-                       {'image_id': image_id,
-                        'status': status,
-                        'status_curr': status_curr,
-                        'timeout': client.build_timeout})
-            caller = misc_utils.find_test_caller()
-            if caller:
-                message = '(%s) %s' % (caller, message)
-            raise exceptions.TimeoutException(message)
+        time.sleep(client.build_interval)
+
+    message = ('Image %(image_id)s failed to reach %(status)s state '
+               '(current state %(current_status)s) within the required '
+               'time (%(timeout)s s).' % {'image_id': image_id,
+                                          'status': status,
+                                          'current_status': current_status,
+                                          'timeout': client.build_timeout})
+    caller = misc_utils.find_test_caller()
+    if caller:
+        message = '(%s) %s' % (caller, message)
+    raise exceptions.TimeoutException(message)
 
 
 def wait_for_volume_status(client, volume_id, status):

tempest/scenario/manager.py

@@ -459,7 +459,7 @@ def create_server_snapshot(self, server, name=None):
         LOG.debug("Creating a snapshot image for server: %s", server['name'])
         image = _images_client.create_image(server['id'], name=name)
         image_id = image.response['location'].split('images/')[1]
-        _image_client.wait_for_image_status(image_id, 'active')
+        waiters.wait_for_image_status(_image_client, image_id, 'active')
         self.addCleanup_with_wait(
             waiter_callable=_image_client.wait_for_resource_deletion,
             thing_id=image_id, thing_id_param='id',

tempest/services/image/v1/json/images_client.py

@@ -16,17 +16,14 @@
 import copy
 import errno
 import os
-import time
 
 from oslo_log import log as logging
 from oslo_serialization import jsonutils as json
 import six
 from six.moves.urllib import parse as urllib
 
 from tempest.common import glance_http
-from tempest import exceptions
 from tempest.lib.common import rest_client
-from tempest.lib.common.utils import misc as misc_utils
 from tempest.lib import exceptions as lib_exc
 
 LOG = logging.getLogger(__name__)
@@ -258,34 +255,3 @@ def delete_member(self, member_id, image_id):
         resp, __ = self.delete(url)
         self.expected_success(204, resp.status)
         return rest_client.ResponseBody(resp)
-
-    # NOTE(afazkas): Wait reinvented again. It is not in the correct layer
-    def wait_for_image_status(self, image_id, status):
-        """Waits for a Image to reach a given status."""
-        start_time = time.time()
-        old_value = value = self.check_image(image_id)['status']
-        while True:
-            dtime = time.time() - start_time
-            time.sleep(self.build_interval)
-            if value != old_value:
-                LOG.info('Value transition from "%s" to "%s"'
-                         'in %d second(s).', old_value,
-                         value, dtime)
-            if value == status:
-                return value
-
-            if value == 'killed':
-                raise exceptions.ImageKilledException(image_id=image_id,
-                                                      status=status)
-            if dtime > self.build_timeout:
-                message = ('Time Limit Exceeded! (%ds)'
-                           'while waiting for %s, '
-                           'but we got %s.' %
-                           (self.build_timeout, status, value))
-                caller = misc_utils.find_test_caller()
-                if caller:
-                    message = '(%s) %s' % (caller, message)
-                raise exceptions.TimeoutException(message)
-            time.sleep(self.build_interval)
-            old_value = value
-            value = self.check_image(image_id)['status']

tempest/services/image/v2/json/images_client.py

@@ -21,10 +21,10 @@
 from tempest.lib import exceptions as lib_exc
 
 
-class ImagesClientV2(rest_client.RestClient):
+class ImagesClient(rest_client.RestClient):
 
     def __init__(self, auth_provider, catalog_type, region, **kwargs):
-        super(ImagesClientV2, self).__init__(
+        super(ImagesClient, self).__init__(
             auth_provider, catalog_type, region, **kwargs)
         self._http = None
         self.dscv = kwargs.get("disable_ssl_certificate_validation")

tempest/tests/common/test_waiters.py

@@ -38,24 +38,20 @@ def test_wait_for_image_status(self):
         # Ensure waiter returns before build_timeout
         self.assertTrue((end_time - start_time) < 10)
 
-    @mock.patch('time.sleep')
-    def test_wait_for_image_status_timeout(self, mock_sleep):
+    def test_wait_for_image_status_timeout(self):
         time_mock = self.patch('time.time')
         time_mock.side_effect = utils.generate_timeout_series(1)
 
         self.client.show_image.return_value = ({'status': 'saving'})
         self.assertRaises(exceptions.TimeoutException,
                           waiters.wait_for_image_status,
                           self.client, 'fake_image_id', 'active')
-        mock_sleep.assert_called_once_with(1)
 
-    @mock.patch('time.sleep')
-    def test_wait_for_image_status_error_on_image_create(self, mock_sleep):
+    def test_wait_for_image_status_error_on_image_create(self):
         self.client.show_image.return_value = ({'status': 'ERROR'})
         self.assertRaises(exceptions.AddImageException,
                           waiters.wait_for_image_status,
                           self.client, 'fake_image_id', 'active')
-        mock_sleep.assert_called_once_with(1)
 
     @mock.patch.object(time, 'sleep')
     def test_wait_for_volume_status_error_restoring(self, mock_sleep):