From 273ed4797e2b3b8ce962bf82e8c993e61955966e Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Wed, 22 Mar 2017 20:55:51 +0000 Subject: [PATCH] Update glanceclient from v1 to v2 to avoid deprecation errors. Currently, the app-catalog/images view in Horizon is throwing a 300 HttpMultipleChoices exception, with the details: "Requested version of OpenStack Images API is not available." Also, the glance v1 client is deprecated, so we should move to glance v2 wherever possible. This patch, therefore, removes glanceclient v1 dependency from murano dashboard as much as possible, especially given that Glance team intends on removing it from Pike [0]. The only remaining places where the glanceclient v1 is kept are: - in _ensure_images in muranodashboard/packages/views.py, which uses copy_from functionality from glanceclient v1 to load images automatically. - done in ImportPackageWizard, which sets each package to public if the image that was uploaded was public [0] https://review.openstack.org/#/c/328390/ Closes-Bug: #1675171 Partially-Implements blueprint: migrate-to-glance-v2 Change-Id: Id5348bc34216d5f5ed7fbb8caf71139d64db3f61 --- muranodashboard/dynamic_ui/fields.py | 4 +- muranodashboard/images/forms.py | 4 +- muranodashboard/images/views.py | 14 +----- muranodashboard/packages/views.py | 30 ++---------- muranodashboard/tests/functional/base.py | 36 ++++++++++---- muranodashboard/tests/functional/consts.py | 2 + .../tests/functional/sanity_check.py | 9 ++-- .../tests/unit/dynamic_ui/test_fields.py | 6 +-- .../tests/unit/images/test_forms.py | 6 +-- .../tests/unit/images/test_views.py | 27 ++-------- .../tests/unit/packages/test_views.py | 49 ------------------- 11 files changed, 56 insertions(+), 131 deletions(-) diff --git a/muranodashboard/dynamic_ui/fields.py b/muranodashboard/dynamic_ui/fields.py index 77a3fc720..2a03af013 100644 --- a/muranodashboard/dynamic_ui/fields.py +++ b/muranodashboard/dynamic_ui/fields.py @@ -119,7 +119,9 @@ def get_murano_images(request): images = filter( lambda x: x.properties.get("image_type", '') != 'snapshot', images) for image in images: - murano_property = image.properties.get('murano_image_info') + # Additional properties, whose value is always a string data type, are + # only included in the response if they have a value. + murano_property = getattr(image, 'murano_image_info', None) if murano_property: try: murano_metadata = json.loads(murano_property) diff --git a/muranodashboard/images/forms.py b/muranodashboard/images/forms.py index 5ca1bb278..df74ea54f 100644 --- a/muranodashboard/images/forms.py +++ b/muranodashboard/images/forms.py @@ -32,7 +32,9 @@ def filter_murano_images(images, request=None): lambda x: x.properties.get("image_type", '') != 'snapshot', images) marked_images = [] for image in images: - metadata = image.properties.get('murano_image_info') + # Additional properties, whose value is always a string data type, are + # only included in the response if they have a value. + metadata = getattr(image, 'murano_image_info', None) if metadata: try: metadata = json.loads(metadata) diff --git a/muranodashboard/images/views.py b/muranodashboard/images/views.py index 8122a5d4b..6c11b9efe 100644 --- a/muranodashboard/images/views.py +++ b/muranodashboard/images/views.py @@ -62,20 +62,10 @@ class MarkedImagesView(horizon_tables.DataTableView): self._prev = False self._more = False - # TODO(kzaitsev) add v2 client support for marking images - try: - glance_v1_client = glance.glanceclient(self.request, "1") - except Exception: - # Horizon seems to raise ImportError which doesn't look - # specific enough. Let's catch any exceptions. - msg = _('Unable to create v1 glance client. Marking images ' - 'from murano-dashboard will be unavailable.') - uri = reverse('horizon:app-catalog:catalog:index') - - exceptions.handle(self.request, msg, redirect=uri) + glance_v2_client = glance.glanceclient(self.request, "2") try: - images_iter = glance_v1_client.images.list( + images_iter = glance_v2_client.images.list( **kwargs) except Exception: msg = _('Unable to retrieve list of images') diff --git a/muranodashboard/packages/views.py b/muranodashboard/packages/views.py index 84b2ae213..c86d44a08 100644 --- a/muranodashboard/packages/views.py +++ b/muranodashboard/packages/views.py @@ -71,40 +71,16 @@ def is_app(wizard): def _ensure_images(name, package, request, step_data=None): - try: - glance_client = glance.glanceclient( - request, version='1') - except Exception: - glance_client = None + glance_client = glance.glanceclient( + request, version='2') base_url = packages_consts.MURANO_REPO_URL image_specs = package.images() - if not glance_client and len(image_specs): - # NOTE(kzaitsev): no glance_client. Probably v1 client - # is not available. Add warning, to let user know that - # we were unable to load images automagically - # since v2 does not have copy_from - download_urls = [] - for image_spec in image_specs: - download_url = muranoclient_utils.to_url( - image_spec.get("Url", image_spec['Name']), - base_url=base_url, - path='images/', - ) - download_urls.append(download_url) - msg = _("Couldn't initialise glance v1 client, " - "therefore could not download images for " - "'{0}' package. You may need to download them " - "manually from these locations: {1}").format( - name, ' '.join(download_urls)) - messages.error(request, msg) - LOG.error(msg) - return try: imgs = muranoclient_utils.ensure_images( glance_client=glance_client, - image_specs=package.images(), + image_specs=image_specs, base_url=base_url) for img in imgs: msg = _("Trying to add {0} image to glance. " diff --git a/muranodashboard/tests/functional/base.py b/muranodashboard/tests/functional/base.py index 4f210c55c..7bb96fbcc 100644 --- a/muranodashboard/tests/functional/base.py +++ b/muranodashboard/tests/functional/base.py @@ -14,6 +14,7 @@ import contextlib import json import logging import os +import six import six.moves.urllib.parse as urlparse import testtools import time @@ -216,6 +217,8 @@ class UITestCase(testtools.TestCase): user_menu.find_element(by.By.PARTIAL_LINK_TEXT, 'Sign Out').click() def fill_field(self, by_find, field, value): + self.check_element_on_page(by_find, field) + self.wait_element_is_clickable(by_find, field) self.driver.find_element(by=by_find, value=field).clear() self.driver.find_element(by=by_find, value=field).send_keys(value) @@ -426,7 +429,7 @@ class ImageTestCase(PackageBase): def setUpClass(cls): super(ImageTestCase, cls).setUpClass() glance_endpoint = cls.service_catalog.url_for(service_type='image') - cls.glance = gclient.Client('1', endpoint=glance_endpoint, + cls.glance = gclient.Client('2', endpoint=glance_endpoint, session=cls.keystone_client.session) def setUp(self): @@ -441,13 +444,14 @@ class ImageTestCase(PackageBase): @classmethod def upload_image(cls, title): try: - property = {'murano_image_info': json.dumps({'title': title, - 'type': 'linux'})} + murano_property = json.dumps({'title': title, 'type': 'linux'}) image = cls.glance.images.create(name='TestImage', disk_format='qcow2', - size=0, - is_public=True, - properties=property) + container_format='bare', + is_public='True', + murano_image_info=murano_property) + image_data = six.BytesIO(None) + cls.glance.images.upload(image['id'], image_data) except Exception: logger.error("Unable to create or update image in Glance") raise @@ -566,14 +570,26 @@ class ApplicationTestCase(ImageTestCase): consts.InputSubmit).click() self.select_from_list('osImage', self.image.id) - self.wait_element_is_clickable(by.By.XPATH, - consts.InputSubmit).click() if env_id: - self.wait_element_is_clickable(by.By.XPATH, - consts.InputSubmit).click() + # If another app is added, then env_id is passed in. In this case, + # the 'Next' followed by 'Create' must be clicked. + self.check_element_on_page(by.By.CSS_SELECTOR, + consts.NextWizardSubmit) + self.wait_element_is_clickable( + by.By.CSS_SELECTOR, consts.NextWizardSubmit).click() + self.check_element_on_page(by.By.CSS_SELECTOR, + consts.CreateWizardSubmit) + self.wait_element_is_clickable( + by.By.CSS_SELECTOR, consts.CreateWizardSubmit).click() + self.wait_element_is_clickable(by.By.ID, consts.AddComponent) self.check_element_on_page(by.By.LINK_TEXT, app_name) else: + # Otherwise, only 'Create' needs to be clicked. + self.check_element_on_page(by.By.CSS_SELECTOR, + consts.CreateWizardSubmit) + self.wait_element_is_clickable( + by.By.CSS_SELECTOR, consts.CreateWizardSubmit).click() self.wait_for_alert_message() def execute_action_from_table_view(self, env_name, table_action): diff --git a/muranodashboard/tests/functional/consts.py b/muranodashboard/tests/functional/consts.py index 6d9f15673..664ed47d3 100644 --- a/muranodashboard/tests/functional/consts.py +++ b/muranodashboard/tests/functional/consts.py @@ -58,6 +58,8 @@ TableDropdownAction = "//tr[contains(@data-display, '{0}')]//button[contains("\ # Buttons ButtonSubmit = ".//*[@name='wizard_goto_step'][2]" InputSubmit = "//input[@type='submit']" +NextWizardSubmit = 'div.modal-footer input[value="Next"]' +CreateWizardSubmit = 'div.modal-footer input[value="Create"]' ConfirmDeletion = "//div[@class='modal-footer']//a[contains(text(), 'Delete')]" # noqa ConfirmAbandon = "//div[@class='modal-footer']//a[contains(text(), 'Abandon')]" # noqa UploadPackage = 'packages__action_upload_package' diff --git a/muranodashboard/tests/functional/sanity_check.py b/muranodashboard/tests/functional/sanity_check.py index 95807ba56..5bdee0116 100644 --- a/muranodashboard/tests/functional/sanity_check.py +++ b/muranodashboard/tests/functional/sanity_check.py @@ -1440,9 +1440,12 @@ class TestSuiteApplications(base.ApplicationTestCase): c.Status.format('Ready'), sec=90) - self.driver.find_element_by_link_text('Deployment History').click() - self.driver.find_element_by_link_text('Show Details').click() - self.driver.find_element_by_link_text('Logs').click() + self.wait_element_is_clickable( + by.By.PARTIAL_LINK_TEXT, 'Deployment History').click() + self.wait_element_is_clickable( + by.By.PARTIAL_LINK_TEXT, 'Show Details').click() + self.wait_element_is_clickable( + by.By.PARTIAL_LINK_TEXT, 'Logs').click() self.assertIn('Follow the white rabbit', self.driver.find_element_by_class_name('logs').text) diff --git a/muranodashboard/tests/unit/dynamic_ui/test_fields.py b/muranodashboard/tests/unit/dynamic_ui/test_fields.py index 2e342503d..e8a13560d 100644 --- a/muranodashboard/tests/unit/dynamic_ui/test_fields.py +++ b/muranodashboard/tests/unit/dynamic_ui/test_fields.py @@ -129,9 +129,9 @@ class TestFields(testtools.TestCase): @mock.patch.object(fields, 'glance') def test_get_murano_images(self, mock_glance): foo_image = mock.Mock(murano_property=None) - foo_image.properties = {"murano_image_info": '{"foo": "foo_val"}'} + foo_image.murano_image_info = '{"foo": "foo_val"}' bar_image = mock.Mock(murano_property=None) - bar_image.properties = {"murano_image_info": '{"bar": "bar_val"}'} + bar_image.murano_image_info = '{"bar": "bar_val"}' mock_glance.image_list_detailed.return_value = [ [foo_image, bar_image], None ] @@ -168,7 +168,7 @@ class TestFields(testtools.TestCase): def test_murano_images_except_value_error(self, mock_glance, mock_log, mock_messages): foo_image = mock.Mock(murano_property=None) - foo_image.properties = {"murano_image_info": "{'foo': 'foo_val'}"} + foo_image.murano_image_info = "{'foo': 'foo_val'}" mock_glance.image_list_detailed.return_value = [ [foo_image], None ] diff --git a/muranodashboard/tests/unit/images/test_forms.py b/muranodashboard/tests/unit/images/test_forms.py index 9aac49a6c..135ab1bbc 100644 --- a/muranodashboard/tests/unit/images/test_forms.py +++ b/muranodashboard/tests/unit/images/test_forms.py @@ -23,14 +23,14 @@ from muranodashboard.images import forms class TestImagesForms(testtools.TestCase): def setUp(self): super(TestImagesForms, self).setUp() - metadata = {'murano_image_info': '{"title": "title", "type": "type"}'} - self.mock_img = mock.MagicMock(id=12, properties=metadata) + metadata = '{"title": "title", "type": "type"}' + self.mock_img = mock.MagicMock(id=12, murano_image_info=metadata) self.mock_request = mock.MagicMock() @mock.patch.object(forms, 'LOG') def test_filter_murano_images(self, mock_log): mock_blank_img = \ - mock.MagicMock(id=13, properties={"murano_image_info": "info"}) + mock.MagicMock(id=13, murano_image_info="info") images = [mock_blank_img] msg = _('Invalid metadata for image: {0}').format(images[0].id) self.assertEqual(images, diff --git a/muranodashboard/tests/unit/images/test_views.py b/muranodashboard/tests/unit/images/test_views.py index e89f5ddec..b9826bc64 100644 --- a/muranodashboard/tests/unit/images/test_views.py +++ b/muranodashboard/tests/unit/images/test_views.py @@ -48,8 +48,7 @@ class TestMarkedImagesView(testtools.TestCase): "title": "{0}_title".format(prefix), "type": "{0}_type".format(prefix) } - mock_image = mock.Mock( - properties={'murano_image_info': json.dumps(image_info)}) + mock_image = mock.Mock(**{'murano_image_info': json.dumps(image_info)}) return mock_image def test_has_prev_data(self): @@ -89,7 +88,7 @@ class TestMarkedImagesView(testtools.TestCase): self.images_view.request.GET.get.assert_called_once_with( tables.MarkedImagesTable._meta.prev_pagination_param, None) mock_glance.glanceclient.assert_called_once_with( - self.images_view.request, "1") + self.images_view.request, "2") @mock.patch.object(views, 'glance', autospec=True) def test_get_data_with_desc_sort_dir(self, mock_glance): @@ -122,7 +121,7 @@ class TestMarkedImagesView(testtools.TestCase): mock.call(tables.MarkedImagesTable._meta.pagination_param, None) ]) mock_glance.glanceclient.assert_called_once_with( - self.images_view.request, "1") + self.images_view.request, "2") @mock.patch.object(views, 'glance', autospec=True) def test_get_data_with_more_results(self, mock_glance): @@ -158,23 +157,7 @@ class TestMarkedImagesView(testtools.TestCase): self.images_view.request.GET.get.assert_called_once_with( tables.MarkedImagesTable._meta.prev_pagination_param, None) mock_glance.glanceclient.assert_called_once_with( - self.images_view.request, "1") - - @mock.patch.object(views, 'reverse', autospec=True) - @mock.patch.object(views, 'glance', autospec=True) - def test_get_data_except_glance_exception(self, mock_glance, mock_reverse): - """Test that glance.glanceclient exception is handled.""" - mock_glance.glanceclient.side_effect = Exception() - mock_reverse.return_value = 'foo_reverse_url' - self.images_view.request.GET.get.return_value = None - - e = self.assertRaises(exceptions.Http302, self.images_view.get_data) - self.assertEqual('foo_reverse_url', e.location) - - mock_glance.glanceclient.assert_called_once_with( - self.images_view.request, "1") - mock_reverse.assert_called_once_with( - 'horizon:app-catalog:catalog:index') + self.images_view.request, "2") @mock.patch.object(views, 'reverse', autospec=True) @mock.patch.object(views, 'glance', autospec=True) @@ -191,6 +174,6 @@ class TestMarkedImagesView(testtools.TestCase): self.assertEqual('foo_reverse_url', e.location) mock_glance.glanceclient.assert_called_once_with( - self.images_view.request, "1") + self.images_view.request, "2") mock_reverse.assert_called_once_with( 'horizon:app-catalog:catalog:index') diff --git a/muranodashboard/tests/unit/packages/test_views.py b/muranodashboard/tests/unit/packages/test_views.py index 63e86bc05..b7295d23b 100644 --- a/muranodashboard/tests/unit/packages/test_views.py +++ b/muranodashboard/tests/unit/packages/test_views.py @@ -86,55 +86,6 @@ class TestPackageView(helpers.APITestCase): mock_wizard.storage.get_step_data.return_value = mock_step_data self.assertFalse(views.is_app(mock_wizard)) - @mock.patch.object(views, 'LOG') - @mock.patch.object(views, 'messages') - @mock.patch.object(views, 'muranoclient_utils') - @mock.patch.object(views, 'glance') - def test_ensure_image_except_glance_exception( - self, mock_glance, mock_murano_utils, mock_messages, mock_log): - mock_glance.glanceclient.side_effect = Exception - mock_package = mock.Mock() - mock_package.images.return_value = [ - {'Url': 'foo_url', 'Name': 'foo_image_spec'}, - {'Url': 'bar_url', 'Name': 'bar_image_spec'} - ] - mock_murano_utils.to_url.side_effect = [ - 'foo_url', 'bar_url' - ] - expected_error = "Couldn't initialise glance v1 client, therefore "\ - "could not download images for 'foo' package. You "\ - "may need to download them manually from these "\ - "locations: foo_url bar_url" - - views._ensure_images('foo', mock_package, self.mock_request) - - for url in ('foo_url', 'bar_url'): - mock_murano_utils.to_url.assert_any_call( - url, base_url=packages_consts.MURANO_REPO_URL, path='images/') - mock_messages.error.assert_called_once_with( - self.mock_request, expected_error) - mock_log.error.assert_called_once_with(expected_error) - - @mock.patch.object(views, 'LOG') - @mock.patch.object(views, 'messages') - @mock.patch.object(views, 'muranoclient_utils') - @mock.patch.object(views, 'glance') - def test_ensure_image_except_exception( - self, mock_glance, mock_murano_utils, mock_messages, mock_log): - mock_glance.glanceclient.side_effect = Exception - mock_murano_utils.ensure_images.side_effect =\ - Exception('test_exception') - mock_package = mock.Mock() - mock_package.images.return_value = [] - expected_error = "Error test_exception occurred while installing "\ - "images for foo" - - views._ensure_images('foo', mock_package, self.mock_request) - - mock_messages.error.assert_called_once_with(self.mock_request, - expected_error) - mock_log.exception.assert_called_once_with(expected_error) - class TestDetailView(helpers.APITestCase):