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
This commit is contained in:
parent
a4ad579260
commit
273ed4797e
@ -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)
|
||||
|
@ -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)
|
||||
|
@ -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')
|
||||
|
@ -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
|
||||
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. "
|
||||
|
@ -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):
|
||||
|
@ -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'
|
||||
|
@ -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)
|
||||
|
@ -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
|
||||
]
|
||||
|
@ -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,
|
||||
|
@ -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')
|
||||
|
@ -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):
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user