Use glance wrapper where appropriate

This makes several places (Nova, Cinder, verify) use the glance
wrapper while accessing glance, which fixes various Glance v2
bugs.

Change-Id: I83b46e223e85306b0e54f1d19cbbc7f21489e759
Co-Authored-By: Srinivas Sakhamuri <srini.openstack@gmail.com>
Co-Authored-By: Yaroslav Lobankov <ylobankov@mirantis.com>

rally-jobs/cinder.yaml

@@ -477,6 +477,23 @@
       sla:
         failure_rate:
           max: 0
+    -
+      args:
+        size: 1
+      runner:
+        type: "constant"
+        times: 1
+        concurrency: 1
+      context:
+        users:
+          tenants: 1
+          users_per_tenant: 1
+        api_versions:
+          glance:
+            version: 2
+      sla:
+        failure_rate:
+          max: 0
 
   CinderVolumes.create_volume_backup:
     -

rally-jobs/nova.yaml

@@ -493,6 +493,26 @@
       sla:
         failure_rate:
           max: 0
+    -
+      args:
+        flavor:
+            name: "m1.tiny"
+        image:
+            name: {{image_name}}
+      runner:
+        type: "constant"
+        times: 2
+        concurrency: 2
+      context:
+        users:
+          tenants: 2
+          users_per_tenant: 2
+        api_versions:
+          glance:
+            version: 2
+      sla:
+        failure_rate:
+          max: 0
 
   NovaServers.boot_server:
     -

rally/plugins/openstack/cleanup/resources.py

@@ -15,6 +15,7 @@
 
 from boto import exception as boto_exception
 from neutronclient.common import exceptions as neutron_exceptions
+from oslo_config import cfg
 from saharaclient.api import base as saharaclient_base
 
 from rally.common import logging
@@ -26,6 +27,9 @@ from rally.plugins.openstack.scenarios.keystone import utils as kutils
 from rally.plugins.openstack.scenarios.nova import utils as nova_utils
 from rally.plugins.openstack.wrappers import glance as glance_wrapper
 from rally.plugins.openstack.wrappers import keystone as keystone_wrapper
+from rally.task import utils as task_utils
+
+CONF = cfg.CONF
 
 LOG = logging.getLogger(__name__)
 
@@ -390,15 +394,24 @@ class ManilaSecurityService(base.ResourceManager):
 @base.resource("glance", "images", order=500, tenant_resource=True)
 class GlanceImage(base.ResourceManager):
 
+    def _client(self):
+        return getattr(self.admin or self.user, self._service)
+
     def _wrapper(self):
-        return glance_wrapper.wrap(
+        return glance_wrapper.wrap(self._client(), self)
-            getattr(self.admin or self.user, self._service), self)
 
     def list(self):
         return self._wrapper().list_images(owner=self.tenant_uuid)
 
     def delete(self):
-        return self._wrapper().delete_image(self.raw_resource)
+        client = self._client()
+        client().images.delete(self.raw_resource.id)
+        task_utils.wait_for_status(
+            self.raw_resource, ["deleted"],
+            check_deletion=True,
+            update_resource=self._wrapper().get_image,
+            timeout=CONF.benchmark.glance_image_delete_timeout,
+            check_interval=CONF.benchmark.glance_image_delete_poll_interval)
 
 
 # SAHARA

rally/plugins/openstack/context/glance/images.py

@@ -12,6 +12,8 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from oslo_config import cfg
+
 from rally.common.i18n import _
 from rally.common import logging
 from rally.common import utils as rutils
@@ -19,7 +21,9 @@ from rally import consts
 from rally import osclients
 from rally.plugins.openstack.wrappers import glance as glance_wrapper
 from rally.task import context
+from rally.task import utils
 
+CONF = cfg.CONF
 
 LOG = logging.getLogger(__name__)
 
@@ -117,4 +121,11 @@ class ImageGenerator(context.Context):
                 api_info=self.context["config"].get("api_versions"))
             glance_wrap = glance_wrapper.wrap(clients.glance, self)
             for image in self.context["tenants"][tenant_id].get("images", []):
-                glance_wrap.delete_image(clients.glance().images.get(image))
+                clients.glance().images.delete(image)
+                utils.wait_for_status(
+                    clients.glance().images.get(image), ["deleted"],
+                    check_deletion=True,
+                    update_resource=glance_wrap.get_image,
+                    timeout=CONF.benchmark.glance_image_delete_timeout,
+                    check_interval=CONF.benchmark.
+                    glance_image_delete_poll_interval)

rally/plugins/openstack/scenarios/cinder/utils.py

@@ -21,6 +21,7 @@ from oslo_config import cfg
 from rally import exceptions
 from rally.plugins.openstack import scenario
 from rally.plugins.openstack.wrappers import cinder as cinder_wrapper
+from rally.plugins.openstack.wrappers import glance as glance_wrapper
 from rally.task import atomic
 from rally.task import utils as bench_utils
 
@@ -233,10 +234,11 @@ class CinderScenario(scenario.OpenStackScenario):
         )
         image_id = img["os-volume_upload_image"]["image_id"]
         image = self.clients("glance").images.get(image_id)
+        wrapper = glance_wrapper.wrap(self._clients.glance, self)
         image = bench_utils.wait_for(
             image,
             ready_statuses=["active"],
-            update_resource=bench_utils.get_from_manager(),
+            update_resource=wrapper.get_image,
             timeout=CONF.benchmark.glance_image_create_timeout,
             check_interval=CONF.benchmark.glance_image_create_poll_interval
         )

rally/plugins/openstack/scenarios/glance/utils.py

@@ -13,9 +13,26 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_config import cfg
+
 from rally.plugins.openstack import scenario
 from rally.plugins.openstack.wrappers import glance as glance_wrapper
 from rally.task import atomic
+from rally.task import utils
+
+GLANCE_BENCHMARK_OPTS = [
+    cfg.FloatOpt("glance_image_delete_timeout",
+                 default=120.0,
+                 help="Time to wait for glance image to be deleted."),
+    cfg.FloatOpt("glance_image_delete_poll_interval",
+                 default=1.0,
+                 help="Interval between checks when waiting for image "
+                      "deletion.")
+]
+
+CONF = cfg.CONF
+benchmark_group = cfg.OptGroup(name="benchmark", title="benchmark options")
+CONF.register_opts(GLANCE_BENCHMARK_OPTS, group=benchmark_group)
 
 
 class GlanceScenario(scenario.OpenStackScenario):
@@ -54,5 +71,11 @@ class GlanceScenario(scenario.OpenStackScenario):
 
         :param image: Image object
         """
-        client = glance_wrapper.wrap(self._clients.glance, self)
+        self.clients("glance").images.delete(image.id)
-        client.delete_image(image)
+        wrapper = glance_wrapper.wrap(self._clients.glance, self)
+        utils.wait_for_status(
+            image, ["deleted"],
+            check_deletion=True,
+            update_resource=wrapper.get_image,
+            timeout=CONF.benchmark.glance_image_delete_timeout,
+            check_interval=CONF.benchmark.glance_image_delete_poll_interval)

rally/plugins/openstack/scenarios/nova/utils.py

@@ -21,6 +21,7 @@ import six
 
 from rally import exceptions
 from rally.plugins.openstack import scenario
+from rally.plugins.openstack.wrappers import glance as glance_wrapper
 from rally.plugins.openstack.wrappers import network as network_wrapper
 from rally.task import atomic
 from rally.task import utils
@@ -464,13 +465,14 @@ class NovaScenario(scenario.OpenStackScenario):
 
         :param image: Image object
         """
-        image.delete()
+        self.clients("glance").images.delete(image.id)
+        wrapper = glance_wrapper.wrap(self._clients.glance, self)
         check_interval = CONF.benchmark.nova_server_image_delete_poll_interval
         utils.wait_for_status(
             image,
             ready_statuses=["deleted"],
             check_deletion=True,
-            update_resource=utils.get_from_manager(),
+            update_resource=wrapper.get_image,
             timeout=CONF.benchmark.nova_server_image_delete_timeout,
             check_interval=check_interval
         )

rally/plugins/openstack/wrappers/glance.py

@@ -39,14 +39,7 @@ GLANCE_BENCHMARK_OPTS = [
     cfg.FloatOpt("glance_image_create_poll_interval",
                  default=1.0,
                  help="Interval between checks when waiting for image "
-                      "creation."),
+                      "creation.")
-    cfg.FloatOpt("glance_image_delete_timeout",
-                 default=120.0,
-                 help="Time to wait for glance image to be deleted."),
-    cfg.FloatOpt("glance_image_delete_poll_interval",
-                 default=1.0,
-                 help="Interval between checks when waiting for image "
-                      "deletion.")
 ]
 
 CONF = cfg.CONF
@@ -60,6 +53,24 @@ class GlanceWrapper(object):
         self.owner = owner
         self.client = client
 
+    def get_image(self, image):
+        """Gets image.
+
+        This serves to fetch the latest data on the image for the
+        various wait_for_*() functions.
+
+        Must raise rally.exceptions.GetResourceNotFound if the
+        resource is not found or deleted.
+        """
+        # NOTE(stpierre): This function actually has a single
+        # implementation that works for both Glance v1 and Glance v2,
+        # but since we need to use this function in both wrappers, it
+        # gets implemented here.
+        try:
+            return self.client.images.get(image.id)
+        except glance_exc.HTTPNotFound:
+            raise exceptions.GetResourceNotFound(resource=image)
+
     @abc.abstractmethod
     def create_image(self, container_format, image_location, disk_format):
         """Creates new image.
@@ -67,10 +78,6 @@ class GlanceWrapper(object):
         Accepts all Glance v2 parameters.
         """
 
-    @abc.abstractmethod
-    def delete_image(self, image):
-        """Deletes image."""
-
     @abc.abstractmethod
     def list_images(self, **filters):
         """List images.
@@ -106,7 +113,7 @@ class GlanceV1Wrapper(GlanceWrapper):
 
             image = utils.wait_for_status(
                 image, ["active"],
-                update_resource=utils.get_from_manager(),
+                update_resource=self.get_image,
                 timeout=CONF.benchmark.glance_image_create_timeout,
                 check_interval=CONF.benchmark.
                 glance_image_create_poll_interval)
@@ -116,15 +123,6 @@ class GlanceV1Wrapper(GlanceWrapper):
 
         return image
 
-    def delete_image(self, image):
-        image.delete()
-        utils.wait_for_status(
-            image, ["deleted"],
-            check_deletion=True,
-            update_resource=utils.get_from_manager(),
-            timeout=CONF.benchmark.glance_image_delete_timeout,
-            check_interval=CONF.benchmark.glance_image_delete_poll_interval)
-
     def list_images(self, **filters):
         kwargs = {"filters": filters}
         if "owner" in filters:
@@ -142,12 +140,6 @@ class GlanceV1Wrapper(GlanceWrapper):
 
 
 class GlanceV2Wrapper(GlanceWrapper):
-    def _update_image(self, image):
-        try:
-            return self.client.images.get(image.id)
-        except glance_exc.HTTPNotFound:
-            raise exceptions.GetResourceNotFound(resource=image)
-
     def create_image(self, container_format, image_location,
                      disk_format, **kwargs):
         kw = {
@@ -167,7 +159,7 @@ class GlanceV2Wrapper(GlanceWrapper):
         start = time.time()
         image = utils.wait_for_status(
             image, ["queued"],
-            update_resource=self._update_image,
+            update_resource=self.get_image,
             timeout=CONF.benchmark.glance_image_create_timeout,
             check_interval=CONF.benchmark.
             glance_image_create_poll_interval)
@@ -190,20 +182,11 @@ class GlanceV2Wrapper(GlanceWrapper):
 
         return utils.wait_for_status(
             image, ["active"],
-            update_resource=self._update_image,
+            update_resource=self.get_image,
             timeout=timeout,
             check_interval=CONF.benchmark.
             glance_image_create_poll_interval)
 
-    def delete_image(self, image):
-        self.client.images.delete(image.id)
-        utils.wait_for_status(
-            image, ["deleted"],
-            check_deletion=True,
-            update_resource=self._update_image,
-            timeout=CONF.benchmark.glance_image_delete_timeout,
-            check_interval=CONF.benchmark.glance_image_delete_poll_interval)
-
     def list_images(self, **filters):
         return self.client.images.list(filters=filters)
 

tests/unit/plugins/openstack/cleanup/test_resources.py

@@ -462,14 +462,18 @@ class GlanceImageTestCase(test.TestCase):
 
     def test_delete(self):
         glance = resources.GlanceImage()
+        glance._client = mock.Mock()
         glance._wrapper = mock.Mock()
         glance.raw_resource = mock.Mock()
 
-        self.assertEqual(
+        client = glance._client.return_value
-            glance.delete(),
+        wrapper = glance._wrapper.return_value
-            glance._wrapper.return_value.delete_image.return_value)
+
-        glance._wrapper.return_value.delete_image.assert_called_once_with(
+        deleted_image = mock.Mock(status="DELETED")
-            glance.raw_resource)
+        wrapper.get_image.side_effect = [glance.raw_resource, deleted_image]
+
+        glance.delete()
+        client().images.delete.assert_called_once_with(glance.raw_resource.id)
 
 
 class CeilometerTestCase(test.TestCase):

tests/unit/plugins/openstack/context/glance/test_images.py

@@ -185,16 +185,17 @@ class ImageGeneratorTestCase(test.ScenarioTestCase):
         images_ctx = images.ImageGenerator(self.context)
         images_ctx.cleanup()
 
-        glance_client = mock_clients.return_value.glance.return_value
-        glance_client.images.get.assert_has_calls([mock.call(i)
-                                                   for i in created_images])
         wrapper_calls = []
         wrapper_calls.extend([mock.call(mock_clients.return_value.glance,
                                         images_ctx)] * tenants_count)
-        wrapper_calls.extend(
-            [mock.call().delete_image(glance_client.images.get.return_value)] *
-            len(created_images))
         mock_wrap.assert_has_calls(wrapper_calls, any_order=True)
+
+        glance_client = mock_clients.return_value.glance.return_value
+        glance_client.images.delete.assert_has_calls([mock.call(i)
+                                                      for i in created_images])
+        glance_client.images.get.assert_has_calls([mock.call(i)
+                                                   for i in created_images])
+
         mock_clients.assert_has_calls(
             [mock.call(mock.ANY, api_info=api_versions)] * tenants_count,
             any_order=True)

tests/unit/plugins/openstack/scenarios/cinder/test_utils.py

@@ -196,7 +196,8 @@ class CinderScenarioTestCase(test.ScenarioTestCase):
         self._test_atomic_action_timer(self.scenario.atomic_actions(),
                                        "cinder.extend_volume")
 
-    def test__upload_volume_to_image(self):
+    @mock.patch("rally.plugins.openstack.wrappers.glance.wrap")
+    def test__upload_volume_to_image(self, mock_wrap):
         volume = mock.Mock()
         image = {"os-volume_upload_image": {"image_id": 1}}
         volume.upload_to_image.return_value = (None, image)
@@ -220,13 +221,12 @@ class CinderScenarioTestCase(test.ScenarioTestCase):
             mock.call(
                 self.clients("glance").images.get.return_value,
                 ready_statuses=["active"],
-                update_resource=self.mock_get_from_manager.mock.return_value,
+                update_resource=mock_wrap.return_value.get_image,
                 timeout=CONF.benchmark.glance_image_create_timeout,
                 check_interval=CONF.benchmark.
                 glance_image_create_poll_interval)
         ])
-        self.mock_get_from_manager.mock.assert_has_calls([mock.call(),
+        self.mock_get_from_manager.mock.assert_called_once_with()
-                                                          mock.call()])
         self.clients("glance").images.get.assert_called_once_with(1)
 
     def test__create_snapshot(self):

tests/unit/plugins/openstack/scenarios/glance/test_utils.py

@@ -75,10 +75,16 @@ class GlanceScenarioTestCase(test.ScenarioTestCase):
 
     @mock.patch("rally.plugins.openstack.wrappers.glance.wrap")
     def test_delete_image(self, mock_wrap):
+        deleted_image = mock.Mock(status="DELETED")
+        wrapper = mock_wrap.return_value
+        wrapper.get_image.side_effect = [self.image, deleted_image]
+
         scenario = utils.GlanceScenario(context=self.context,
                                         clients=self.scenario_clients)
         scenario._delete_image(self.image)
+        self.clients("glance").images.delete.assert_called_once_with(
+            self.image.id)
+
         mock_wrap.assert_called_once_with(scenario._clients.glance, scenario)
-        mock_wrap.return_value.delete_image.assert_called_once_with(self.image)
         self._test_atomic_action_timer(scenario.atomic_actions(),
                                        "glance.delete_image")

tests/unit/plugins/openstack/scenarios/nova/test_utils.py

@@ -407,19 +407,21 @@ class NovaScenarioTestCase(test.ScenarioTestCase):
     def test__force_delete_servers(self):
         self._test_delete_servers(force=True)
 
-    def test__delete_image(self):
+    @mock.patch("rally.plugins.openstack.wrappers.glance.wrap")
+    def test__delete_image(self, mock_wrap):
         nova_scenario = utils.NovaScenario(context=self.context)
+        nova_scenario._clients = mock.Mock()
         nova_scenario._delete_image(self.image)
-        self.image.delete.assert_called_once_with()
+        self.clients("glance").images.delete.assert_called_once_with(
+            self.image.id)
         self.mock_wait_for_status.mock.assert_called_once_with(
             self.image,
             ready_statuses=["deleted"],
             check_deletion=True,
-            update_resource=self.mock_get_from_manager.mock.return_value,
+            update_resource=mock_wrap.return_value.get_image,
             check_interval=CONF.benchmark.
             nova_server_image_delete_poll_interval,
             timeout=CONF.benchmark.nova_server_image_delete_timeout)
-        self.mock_get_from_manager.mock.assert_called_once_with()
         self._test_atomic_action_timer(nova_scenario.atomic_actions(),
                                        "nova.delete_image")
 

tests/unit/plugins/openstack/wrappers/test_glance.py

@@ -61,6 +61,24 @@ class GlanceV1WrapperTestCase(test.ScenarioTestCase):
         self.owner = mock.Mock()
         self.wrapped_client = glance_wrapper.wrap(self.client, self.owner)
 
+    def test_get_image(self):
+        image = mock.Mock()
+
+        return_image = self.wrapped_client.get_image(image)
+
+        self.client.return_value.images.get.assert_called_once_with(image.id)
+        self.assertEqual(return_image,
+                         self.client.return_value.images.get.return_value)
+
+    def test_get_image_not_found(self):
+        image = mock.Mock()
+        self.client.return_value.images.get.side_effect = (
+            glance_exc.HTTPNotFound)
+
+        self.assertRaises(exceptions.GetResourceNotFound,
+                          self.wrapped_client.get_image, image)
+        self.client.return_value.images.get.assert_called_once_with(image.id)
+
     @ddt.data(
         {"location": "image_location", "visibility": "private"},
         {"location": "image_location", "fakearg": "fake"},
@@ -91,25 +109,12 @@ class GlanceV1WrapperTestCase(test.ScenarioTestCase):
 
         self.mock_wait_for_status.mock.assert_called_once_with(
             self.client().images.create.return_value, ["active"],
-            update_resource=self.mock_get_from_manager.mock.return_value,
+            update_resource=self.wrapped_client.get_image,
             check_interval=CONF.benchmark.glance_image_create_poll_interval,
             timeout=CONF.benchmark.glance_image_create_timeout)
-        self.mock_get_from_manager.mock.assert_called_once_with()
         self.assertEqual(self.mock_wait_for_status.mock.return_value,
                          return_image)
 
-    def test_delete_image(self):
-        image = mock.Mock()
-        self.wrapped_client.delete_image(image)
-        image.delete.assert_called_once_with()
-        self.mock_wait_for_status.mock.assert_called_once_with(
-            image, ["deleted"],
-            check_deletion=True,
-            update_resource=self.mock_get_from_manager.mock.return_value,
-            check_interval=CONF.benchmark.glance_image_delete_poll_interval,
-            timeout=CONF.benchmark.glance_image_delete_timeout)
-        self.mock_get_from_manager.mock.assert_called_once_with()
-
     @ddt.data({}, {"fakearg": "fake"})
     def test_list_images_basic(self, filters):
         self.assertEqual(self.wrapped_client.list_images(**filters),
@@ -157,22 +162,22 @@ class GlanceV2WrapperTestCase(test.ScenarioTestCase):
         self.owner = mock.Mock()
         self.wrapped_client = glance_wrapper.wrap(self.client, self.owner)
 
-    def test__update_image(self):
+    def test_get_image(self):
         image = mock.Mock()
 
-        return_image = self.wrapped_client._update_image(image)
+        return_image = self.wrapped_client.get_image(image)
 
         self.client.return_value.images.get.assert_called_once_with(image.id)
         self.assertEqual(return_image,
                          self.client.return_value.images.get.return_value)
 
-    def test__update_image_not_found(self):
+    def test_get_image_not_found(self):
         image = mock.Mock()
         self.client.return_value.images.get.side_effect = (
             glance_exc.HTTPNotFound)
 
         self.assertRaises(exceptions.GetResourceNotFound,
-                          self.wrapped_client._update_image, image)
+                          self.wrapped_client.get_image, image)
         self.client.return_value.images.get.assert_called_once_with(image.id)
 
     @ddt.data(
@@ -185,7 +190,7 @@ class GlanceV2WrapperTestCase(test.ScenarioTestCase):
     @mock.patch("requests.get")
     def test_create_image(self, mock_requests_get, mock_open, location,
                           **kwargs):
-        self.wrapped_client._update_image = mock.Mock()
+        self.wrapped_client.get_image = mock.Mock()
         created_image = mock.Mock()
         uploaded_image = mock.Mock()
         self.mock_wait_for_status.mock.side_effect = [created_image,
@@ -216,30 +221,18 @@ class GlanceV2WrapperTestCase(test.ScenarioTestCase):
         self.mock_wait_for_status.mock.assert_has_calls([
             mock.call(
                 self.client().images.create.return_value, ["queued"],
-                update_resource=self.wrapped_client._update_image,
+                update_resource=self.wrapped_client.get_image,
                 check_interval=CONF.benchmark.
                 glance_image_create_poll_interval,
                 timeout=CONF.benchmark.glance_image_create_timeout),
             mock.call(
                 created_image, ["active"],
-                update_resource=self.wrapped_client._update_image,
+                update_resource=self.wrapped_client.get_image,
                 check_interval=CONF.benchmark.
                 glance_image_create_poll_interval,
                 timeout=mock.ANY)])
         self.assertEqual(uploaded_image, return_image)
 
-    def test_delete_image(self):
-        image = mock.Mock()
-        self.wrapped_client.delete_image(image)
-        self.client.return_value.images.delete.assert_called_once_with(
-            image.id)
-        self.mock_wait_for_status.mock.assert_called_once_with(
-            image, ["deleted"],
-            check_deletion=True,
-            update_resource=self.wrapped_client._update_image,
-            check_interval=CONF.benchmark.glance_image_delete_poll_interval,
-            timeout=CONF.benchmark.glance_image_delete_timeout)
-
     @ddt.data({}, {"fakearg": "fake"})
     def test_list_images(self, filters):
         self.assertEqual(self.wrapped_client.list_images(**filters),