Merge "Use common "waiters.wait_for_image_status" function everywhere"
diff --git a/tempest/api/compute/servers/test_server_actions.py b/tempest/api/compute/servers/test_server_actions.py
index 5ce41c8..66aec84 100644
--- a/tempest/api/compute/servers/test_server_actions.py
+++ b/tempest/api/compute/servers/test_server_actions.py
@@ -341,7 +341,8 @@
 
         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')
@@ -351,7 +352,8 @@
                                          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 = {
diff --git a/tempest/api/image/v1/test_images.py b/tempest/api/image/v1/test_images.py
index b2f12ca..6d5559d 100644
--- a/tempest/api/image/v1/test_images.py
+++ b/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 @@
         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')
diff --git a/tempest/api/volume/test_volumes_actions.py b/tempest/api/volume/test_volumes_actions.py
index e52216f..5975231 100644
--- a/tempest/api/volume/test_volumes_actions.py
+++ b/tempest/api/volume/test_volumes_actions.py
@@ -125,7 +125,7 @@
             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')
 
diff --git a/tempest/clients.py b/tempest/clients.py
index b0f779f..19f1a2a 100644
--- a/tempest/clients.py
+++ b/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
@@ -331,7 +332,7 @@
                 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,
diff --git a/tempest/cmd/javelin.py b/tempest/cmd/javelin.py
index 9b3cac7..17ee618 100755
--- a/tempest/cmd/javelin.py
+++ b/tempest/cmd/javelin.py
@@ -233,7 +233,7 @@
                                                   **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,
diff --git a/tempest/common/waiters.py b/tempest/common/waiters.py
index 95305f3..23d7f88 100644
--- a/tempest/common/waiters.py
+++ b/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 @@
     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']
-    start = int(time.time())
+    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
 
-    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.
+    current_status = 'An unknown status'
+    start = int(time.time())
+    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':
+
+        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)
 
-        # 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:
-            return
+        time.sleep(client.build_interval)
 
-        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)
+    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):
diff --git a/tempest/scenario/manager.py b/tempest/scenario/manager.py
index 262ec50..65677a0 100644
--- a/tempest/scenario/manager.py
+++ b/tempest/scenario/manager.py
@@ -459,7 +459,7 @@
         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',
diff --git a/tempest/services/image/v1/json/images_client.py b/tempest/services/image/v1/json/images_client.py
index 7274e5f..a36075f 100644
--- a/tempest/services/image/v1/json/images_client.py
+++ b/tempest/services/image/v1/json/images_client.py
@@ -16,7 +16,6 @@
 import copy
 import errno
 import os
-import time
 
 from oslo_log import log as logging
 from oslo_serialization import jsonutils as json
@@ -24,9 +23,7 @@
 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 @@
         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']
diff --git a/tempest/services/image/v2/json/images_client.py b/tempest/services/image/v2/json/images_client.py
index 4e037af..a61f31d 100644
--- a/tempest/services/image/v2/json/images_client.py
+++ b/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")
diff --git a/tempest/tests/common/test_waiters.py b/tempest/tests/common/test_waiters.py
index 492bdca..a56f837 100644
--- a/tempest/tests/common/test_waiters.py
+++ b/tempest/tests/common/test_waiters.py
@@ -38,8 +38,7 @@
         # 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)
 
@@ -47,15 +46,12 @@
         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):