From 81ae39a03bc2ba3068b7e3731950fff195c83ecc Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Fri, 11 Mar 2016 14:04:37 -0600 Subject: [PATCH] Fix glance cleanup filter In Glance v2, filters must be passed as a dict in the 'filters' kwarg; in Glance v1, they must be passed as first-order keyword args. Implements: blueprint cleanup-refactoring Change-Id: Ie39d778644306965ed74344dbb4f50bae8692a38 --- rally-jobs/rally.yaml | 23 ++++++ rally/plugins/openstack/cleanup/resources.py | 20 ++++- rally/plugins/openstack/wrappers/glance.py | 30 ++++++-- .../openstack/cleanup/test_resources.py | 75 +++++++++++++++++-- .../plugins/openstack/wrappers/test_glance.py | 45 +++++++++-- 5 files changed, 174 insertions(+), 19 deletions(-) diff --git a/rally-jobs/rally.yaml b/rally-jobs/rally.yaml index ff62c53e..3f157f82 100644 --- a/rally-jobs/rally.yaml +++ b/rally-jobs/rally.yaml @@ -859,6 +859,29 @@ users: tenants: 1 users_per_tenant: 1 + api_versions: + glance: + version: 1 + sla: + failure_rate: + max: 0 + + - + args: + image_location: "~/.rally/extra/fake-image.img" + container_format: "bare" + disk_format: "qcow2" + 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 diff --git a/rally/plugins/openstack/cleanup/resources.py b/rally/plugins/openstack/cleanup/resources.py index b4bfe464..4b579608 100644 --- a/rally/plugins/openstack/cleanup/resources.py +++ b/rally/plugins/openstack/cleanup/resources.py @@ -24,7 +24,9 @@ from rally.plugins.openstack.cleanup import base from rally.plugins.openstack.scenarios.fuel import utils as futils 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 LOG = logging.getLogger(__name__) @@ -384,8 +386,24 @@ class ManilaSecurityService(base.ResourceManager): @base.resource("glance", "images", order=500, tenant_resource=True) class GlanceImage(base.ResourceManager): + def _manager(self): + return glance_wrapper.wrap( + getattr(self.admin or self.user, self._service), self) + def list(self): - return self._manager().list(owner=self.tenant_uuid) + return self._manager().list_images(owner=self.tenant_uuid) + + def is_deleted(self): + try: + resource = self._manager().get_image(self.raw_resource) + except Exception as e: + return getattr(e, "code", getattr(e, "http_status", 400)) == 404 + + return task_utils.get_status(resource) in ("DELETED", + "DELETE_COMPLETE") + + def delete(self): + return self._manager().delete_image(self.raw_resource) # SAHARA diff --git a/rally/plugins/openstack/wrappers/glance.py b/rally/plugins/openstack/wrappers/glance.py index d2f4cd1e..c1f27bb0 100644 --- a/rally/plugins/openstack/wrappers/glance.py +++ b/rally/plugins/openstack/wrappers/glance.py @@ -60,6 +60,10 @@ class GlanceWrapper(object): self.owner = owner self.client = client + @abc.abstractmethod + def get_image(self, image): + """Refresh an image from an image object.""" + @abc.abstractmethod def create_image(self, container_format, image_location, disk_format): """Creates new image.""" @@ -68,8 +72,15 @@ class GlanceWrapper(object): def delete_image(self, image): """Deletes image.""" + @abc.abstractmethod + def list_images(self, **filters): + """List images.""" + class GlanceV1Wrapper(GlanceWrapper): + def get_image(self, image): + return image.manager.get(image.id) + def create_image(self, container_format, image_location, disk_format, **kwargs): kw = { @@ -112,11 +123,17 @@ class GlanceV1Wrapper(GlanceWrapper): 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) + class GlanceV2Wrapper(GlanceWrapper): - def _get_image(self, image): + def get_image(self, image): + return self.client.images.get(image.id) + + def _update_image(self, image): try: - return self.client.images.get(image.id) + return self.get_image(image) except glance_exc.HTTPNotFound: raise exceptions.GetResourceNotFound(resource=image) @@ -139,7 +156,7 @@ class GlanceV2Wrapper(GlanceWrapper): start = time.time() image = utils.wait_for_status( image, ["queued"], - update_resource=self._get_image, + update_resource=self._update_image, timeout=CONF.benchmark.glance_image_create_timeout, check_interval=CONF.benchmark. glance_image_create_poll_interval) @@ -159,7 +176,7 @@ class GlanceV2Wrapper(GlanceWrapper): return utils.wait_for_status( image, ["active"], - update_resource=self._get_image, + update_resource=self._update_image, timeout=timeout, check_interval=CONF.benchmark. glance_image_create_poll_interval) @@ -169,10 +186,13 @@ class GlanceV2Wrapper(GlanceWrapper): utils.wait_for_status( image, ["deleted"], check_deletion=True, - update_resource=self._get_image, + 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) + def wrap(client, owner): """Returns glanceclient wrapper based on glance client version.""" diff --git a/tests/unit/plugins/openstack/cleanup/test_resources.py b/tests/unit/plugins/openstack/cleanup/test_resources.py index 6f73bfe0..34c89b00 100644 --- a/tests/unit/plugins/openstack/cleanup/test_resources.py +++ b/tests/unit/plugins/openstack/cleanup/test_resources.py @@ -14,6 +14,8 @@ # under the License. from boto import exception as boto_exception +import ddt +from glanceclient import exc as glance_exc import mock from neutronclient.common import exceptions as neutron_exceptions @@ -391,19 +393,82 @@ class NeutronQuotaTestCase(test.TestCase): self.assertEqual("foo", getattr(admin, res._service)()) +@ddt.ddt class GlanceImageTestCase(test.TestCase): + @mock.patch("rally.plugins.openstack.wrappers.glance.wrap") + def test__manager_admin(self, mock_glance_wrap): + admin = mock.Mock() + glance = resources.GlanceImage(admin=admin) + manager = glance._manager() + + mock_glance_wrap.assert_called_once_with(admin.glance, glance) + self.assertEqual(manager, mock_glance_wrap.return_value) + + @mock.patch("rally.plugins.openstack.wrappers.glance.wrap") + def test__manager_user(self, mock_glance_wrap): + user = mock.Mock() + glance = resources.GlanceImage(user=user) + manager = glance._manager() + + mock_glance_wrap.assert_called_once_with(user.glance, glance) + self.assertEqual(manager, mock_glance_wrap.return_value) + + @mock.patch("rally.plugins.openstack.wrappers.glance.wrap") + def test__manager_admin_preferred(self, mock_glance_wrap): + admin = mock.Mock() + user = mock.Mock() + glance = resources.GlanceImage(admin=admin, user=user) + manager = glance._manager() + + mock_glance_wrap.assert_called_once_with(admin.glance, glance) + self.assertEqual(manager, mock_glance_wrap.return_value) + @mock.patch("%s.GlanceImage._manager" % BASE) def test_list(self, mock_glance_image__manager): glance = resources.GlanceImage() - glance.tenant_uuid = mock.MagicMock() + glance.tenant_uuid = mock.Mock() - mock_glance_image__manager().list.return_value = ["a", "b", "c"] - - self.assertEqual(["a", "b", "c"], glance.list()) - mock_glance_image__manager().list.assert_called_once_with( + manager = mock_glance_image__manager.return_value + self.assertEqual(glance.list(), + manager.list_images.return_value) + manager.list_images.assert_called_once_with( owner=glance.tenant_uuid) + @ddt.data( + {"resource": mock.Mock(), "deleted": False}, + {"resource": mock.Mock(status="deleted"), "deleted": True}, + {"resource": mock.Mock(status="active"), "deleted": False}, + {"resource": mock.Mock(status="delete_complete"), "deleted": True}, + {"exception": glance_exc.HTTPNotFound, "deleted": True}, + {"exception": glance_exc.HTTPUnauthorized, "deleted": False}, + {"exception": Exception, "deleted": False}) + @ddt.unpack + @mock.patch("%s.GlanceImage._manager" % BASE) + def test_is_deleted(self, mock_glance_image__manager, resource=None, + exception=None, deleted=True): + if resource is None: + resource = mock.Mock() + manager = mock_glance_image__manager.return_value + if exception is not None: + manager.get_image.side_effect = exception + else: + manager.get_image.return_value = resource + + glance = resources.GlanceImage(resource=resource) + self.assertEqual(deleted, glance.is_deleted()) + manager.get_image.assert_called_once_with(resource) + + @mock.patch("%s.GlanceImage._manager" % BASE) + def test_delete(self, mock_glance_image__manager): + glance = resources.GlanceImage() + glance.raw_resource = mock.Mock() + + manager = mock_glance_image__manager.return_value + self.assertEqual(glance.delete(), + manager.delete_image.return_value) + manager.delete_image.assert_called_once_with(glance.raw_resource) + class CeilometerTestCase(test.TestCase): diff --git a/tests/unit/plugins/openstack/wrappers/test_glance.py b/tests/unit/plugins/openstack/wrappers/test_glance.py index 68cb9c41..10951577 100644 --- a/tests/unit/plugins/openstack/wrappers/test_glance.py +++ b/tests/unit/plugins/openstack/wrappers/test_glance.py @@ -47,6 +47,14 @@ class GlanceV1WrapperTestCase(test.ScenarioTestCase, GlanceWrapperTestBase): 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) + + image.manager.get.assert_called_once_with(image.id) + self.assertEqual(return_image, image.manager.get.return_value) + @ddt.data( {"location": "image_location"}, {"location": "image_location", "fakearg": "fake"}, @@ -94,6 +102,12 @@ class GlanceV1WrapperTestCase(test.ScenarioTestCase, GlanceWrapperTestBase): 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(self, filters): + self.assertEqual(self.wrapped_client.list_images(**filters), + self.client().images.list.return_value) + self.client().images.list.assert_called_once_with(**filters) + @ddt.ddt class GlanceV2WrapperTestCase(test.ScenarioTestCase, GlanceWrapperTestBase): @@ -106,22 +120,31 @@ class GlanceV2WrapperTestCase(test.ScenarioTestCase, GlanceWrapperTestBase): self.owner = mock.Mock() self.wrapped_client = glance_wrapper.wrap(self.client, self.owner) - def test__get_image(self): + def test_get_image(self): image = mock.Mock() - return_image = self.wrapped_client._get_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__get_image_not_found(self): + def test__update_image(self): + image = mock.Mock() + + return_image = self.wrapped_client._update_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): 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.wrapped_client._update_image, image) self.client.return_value.images.get.assert_called_once_with(image.id) @ddt.data( @@ -134,7 +157,7 @@ class GlanceV2WrapperTestCase(test.ScenarioTestCase, GlanceWrapperTestBase): @mock.patch("requests.get") def test_create_image(self, mock_requests_get, mock_open, location, **kwargs): - self.wrapped_client._get_image = mock.Mock() + self.wrapped_client._update_image = mock.Mock() created_image = mock.Mock() uploaded_image = mock.Mock() self.mock_wait_for_status.mock.side_effect = [created_image, @@ -165,13 +188,13 @@ class GlanceV2WrapperTestCase(test.ScenarioTestCase, GlanceWrapperTestBase): self.mock_wait_for_status.mock.assert_has_calls([ mock.call( self.client().images.create.return_value, ["queued"], - update_resource=self.wrapped_client._get_image, + update_resource=self.wrapped_client._update_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._get_image, + update_resource=self.wrapped_client._update_image, check_interval=CONF.benchmark. glance_image_create_poll_interval, timeout=mock.ANY)]) @@ -185,6 +208,12 @@ class GlanceV2WrapperTestCase(test.ScenarioTestCase, GlanceWrapperTestBase): self.mock_wait_for_status.mock.assert_called_once_with( image, ["deleted"], check_deletion=True, - update_resource=self.wrapped_client._get_image, + 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), + self.client().images.list.return_value) + self.client().images.list.assert_called_once_with(filters=filters)