diff --git a/openstack_dashboard/api/glance.py b/openstack_dashboard/api/glance.py index 71e9525df8..0b5de5718f 100644 --- a/openstack_dashboard/api/glance.py +++ b/openstack_dashboard/api/glance.py @@ -46,18 +46,6 @@ VERSIONS = base.APIVersionManager("image", preferred_version=2) VERSIONS.load_supported_version(2, {"client": client, "version": 2}) -# TODO(e0ne): remove this workaround once glanceclient will raise -# RequestURITooLong exception - -# NOTE(e0ne): set MAX_URI_LEN to 8KB like Neutron does -MAX_URI_LEN = 8192 - -URI_FILTER_PREFIX = "/v2/images?id=in:" -# NOTE(e0ne): 36 is a lengght of UUID, we need tp have '+' for sapparator. -# Decreasing value by 1 to make sure it could be send to a server -MAX_IMGAGES_PER_REQUEST = \ - (MAX_URI_LEN - len(URI_FILTER_PREFIX)) // (36 + 1) - 1 - class Image(base.APIResourceWrapper): _attrs = {"architecture", "container_format", "disk_format", "created_at", @@ -196,19 +184,6 @@ def image_get(request, image_id): return Image(image) -@profiler.trace -def image_list_detailed_by_ids(request, ids=None): - images = [] - if not ids: - return images - for i in range(0, len(ids), MAX_IMGAGES_PER_REQUEST): - ids_to_filter = ids[i:i + MAX_IMGAGES_PER_REQUEST] - filters = {'id': 'in:' + ','.join(ids_to_filter)} - images.extend(image_list_detailed(request, filters=filters)[0]) - - return images - - @profiler.trace def image_list_detailed(request, marker=None, sort_dir='desc', sort_key='created_at', filters=None, paginate=False, diff --git a/openstack_dashboard/dashboards/admin/instances/tests.py b/openstack_dashboard/dashboards/admin/instances/tests.py index ad790829ef..3630cb79ad 100644 --- a/openstack_dashboard/dashboards/admin/instances/tests.py +++ b/openstack_dashboard/dashboards/admin/instances/tests.py @@ -29,20 +29,32 @@ INDEX_TEMPLATE = 'horizon/common/_data_table_view.html' class InstanceViewTest(test.BaseAdminViewTests): + + def _mock_image_list_detailed_side_effect(self, *args, **kwargs): + images = self.images.list() + if 'filters' in kwargs: + return [[image for image in images if + image.visibility == 'community']] + else: + return [[image for image in images if + image.visibility != 'community']] + @test.create_mocks({ api.nova: ['flavor_list', 'server_list_paged'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed_by_ids'], + api.glance: ['image_list_detailed'], + api.cinder: ['volume_list'] }) def test_index(self): servers = self.servers.list() # TODO(vmarkov) instances_img_ids should be in test_data - instances_img_ids = [instance.image.get('id') for instance in - servers if isinstance(instance.image, dict)] self.mock_tenant_list.return_value = [self.tenants.list(), False] - self.mock_image_list_detailed_by_ids.return_value = self.images.list() + + self.mock_image_list_detailed.side_effect =\ + self._mock_image_list_detailed_side_effect self.mock_flavor_list.return_value = self.flavors.list() self.mock_server_list_paged.return_value = [servers, False, False] + self.mock_volume_list.return_value = self.cinder_volumes.list() res = self.client.get(INDEX_URL) self.assertTemplateUsed(res, INDEX_TEMPLATE) @@ -50,8 +62,7 @@ class InstanceViewTest(test.BaseAdminViewTests): self.assertCountEqual(instances, servers) self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest()) - self.mock_image_list_detailed_by_ids.assert_called_once_with( - test.IsHttpRequest(), instances_img_ids) + self.assertEqual(self.mock_image_list_detailed.call_count, 4) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) search_opts = {'marker': None, 'paginate': True, 'all_tenants': True} self.mock_server_list_paged.assert_called_once_with( @@ -62,13 +73,12 @@ class InstanceViewTest(test.BaseAdminViewTests): @test.create_mocks({ api.nova: ['flavor_list', 'flavor_get', 'server_list_paged'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed_by_ids'], + api.glance: ['image_list_detailed'], + api.cinder: ['volume_list'] }) def test_index_flavor_list_exception(self): servers = self.servers.list() flavors = self.flavors.list() - instances_img_ids = [instance.image.get('id') for instance in - servers if hasattr(instance, 'image')] full_flavors = OrderedDict([(f.id, f) for f in flavors]) self.mock_server_list_paged.return_value = [servers, False, False] self.mock_flavor_list.side_effect = self.exceptions.nova @@ -78,8 +88,9 @@ class InstanceViewTest(test.BaseAdminViewTests): return full_flavors[id] self.mock_flavor_get.side_effect = _get_full_flavor - self.mock_image_list_detailed_by_ids.return_value = self.images.list() - + self.mock_image_list_detailed.side_effect =\ + self._mock_image_list_detailed_side_effect + self.mock_volume_list.return_value = self.cinder_volumes.list() res = self.client.get(INDEX_URL) self.assertTemplateUsed(res, INDEX_TEMPLATE) @@ -96,24 +107,24 @@ class InstanceViewTest(test.BaseAdminViewTests): self.mock_flavor_get.assert_has_calls( [mock.call(test.IsHttpRequest(), s.flavor['id']) for s in servers]) self.assertEqual(len(servers), self.mock_flavor_get.call_count) - self.mock_image_list_detailed_by_ids.assert_called_once_with( - test.IsHttpRequest(), instances_img_ids) + self.assertEqual(self.mock_image_list_detailed.call_count, 4) @test.create_mocks({ api.nova: ['flavor_list', 'flavor_get', 'server_list_paged'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed_by_ids'], + api.glance: ['image_list_detailed'], + api.cinder: ['volume_list'] }) def test_index_flavor_get_exception(self): servers = self.servers.list() - instances_img_ids = [instance.image.get('id') for instance in - servers if hasattr(instance, 'image')] # UUIDs generated using indexes are unlikely to match # any of existing flavor ids and are guaranteed to be deterministic. for i, server in enumerate(servers): server.flavor['id'] = str(uuid.UUID(int=i)) - self.mock_image_list_detailed_by_ids.return_value = self.images.list() + self.mock_image_list_detailed.side_effect =\ + self._mock_image_list_detailed_side_effect + self.mock_volume_list.return_value = self.cinder_volumes.list() self.mock_flavor_list.return_value = self.flavors.list() self.mock_server_list_paged.return_value = [servers, False, False] self.mock_tenant_list.return_value = [self.tenants.list(), False] @@ -128,8 +139,7 @@ class InstanceViewTest(test.BaseAdminViewTests): self.assertMessageCount(res, error=1) self.assertCountEqual(instances, servers) - self.mock_image_list_detailed_by_ids.assert_called_once_with( - test.IsHttpRequest(), instances_img_ids) + self.assertEqual(self.mock_image_list_detailed.call_count, 4) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) search_opts = {'marker': None, 'paginate': True, 'all_tenants': True} self.mock_server_list_paged.assert_called_once_with( @@ -144,13 +154,16 @@ class InstanceViewTest(test.BaseAdminViewTests): @test.create_mocks({ api.nova: ['server_list_paged', 'flavor_list'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed_by_ids'], + api.glance: ['image_list_detailed'], + api.cinder: ['volume_list'] }) def test_index_server_list_exception(self): self.mock_server_list_paged.side_effect = self.exceptions.nova self.mock_flavor_list.return_value = self.flavors.list() self.mock_tenant_list.return_value = [self.tenants.list(), False] - self.mock_image_list_detailed_by_ids.return_value = self.images.list() + self.mock_image_list_detailed.side_effect =\ + self._mock_image_list_detailed_side_effect + self.mock_volume_list.return_value = self.cinder_volumes.list() res = self.client.get(INDEX_URL) self.assertTemplateUsed(res, INDEX_TEMPLATE) @@ -162,8 +175,7 @@ class InstanceViewTest(test.BaseAdminViewTests): sort_dir='desc', search_opts=search_opts) self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest()) - self.mock_image_list_detailed_by_ids.assert_called_once_with( - test.IsHttpRequest(), []) + self.assertEqual(self.mock_image_list_detailed.call_count, 4) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) @test.create_mocks({api.nova: ['server_get', 'flavor_get'], @@ -206,14 +218,14 @@ class InstanceViewTest(test.BaseAdminViewTests): @test.create_mocks({ api.nova: ['flavor_list', 'server_list_paged'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed_by_ids'], + api.glance: ['image_list_detailed'], + api.cinder: ['volume_list'] }) def test_index_options_before_migrate(self): - servers = self.servers.list() - instances_img_ids = [instance.image.get('id') for instance in - servers if hasattr(instance, 'image')] self.mock_tenant_list.return_value = [self.tenants.list(), False] - self.mock_image_list_detailed_by_ids.return_value = self.images.list() + self.mock_image_list_detailed.side_effect =\ + self._mock_image_list_detailed_side_effect + self.mock_volume_list.return_value = self.cinder_volumes.list() self.mock_flavor_list.return_value = self.flavors.list() self.mock_server_list_paged.return_value = [ self.servers.list(), False, False] @@ -223,8 +235,7 @@ class InstanceViewTest(test.BaseAdminViewTests): self.assertNotContains(res, "instances__revert") self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest()) - self.mock_image_list_detailed_by_ids.assert_called_once_with( - test.IsHttpRequest(), instances_img_ids) + self.assertEqual(self.mock_image_list_detailed.call_count, 4) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) search_opts = {'marker': None, 'paginate': True, 'all_tenants': True} self.mock_server_list_paged.assert_called_once_with( @@ -235,7 +246,8 @@ class InstanceViewTest(test.BaseAdminViewTests): @test.create_mocks({ api.nova: ['flavor_list', 'server_list_paged'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed_by_ids'], + api.glance: ['image_list_detailed'], + api.cinder: ['volume_list'] }) def test_index_options_after_migrate(self): servers = self.servers.list() @@ -243,10 +255,10 @@ class InstanceViewTest(test.BaseAdminViewTests): server1.status = "VERIFY_RESIZE" server2 = servers[2] server2.status = "VERIFY_RESIZE" - instances_img_ids = [instance.image.get('id') for instance in - servers if hasattr(instance, 'image')] self.mock_tenant_list.return_value = [self.tenants.list(), False] - self.mock_image_list_detailed_by_ids.return_value = self.images.list() + self.mock_image_list_detailed.side_effect =\ + self._mock_image_list_detailed_side_effect + self.mock_volume_list.return_value = self.cinder_volumes.list() self.mock_flavor_list.return_value = self.flavors.list() self.mock_server_list_paged.return_value = [servers, False, False] @@ -256,8 +268,7 @@ class InstanceViewTest(test.BaseAdminViewTests): self.assertNotContains(res, "instances__migrate") self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest()) - self.mock_image_list_detailed_by_ids.assert_called_once_with( - test.IsHttpRequest(), instances_img_ids) + self.assertEqual(self.mock_image_list_detailed.call_count, 4) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) search_opts = {'marker': None, 'paginate': True, 'all_tenants': True} self.mock_server_list_paged.assert_called_once_with( @@ -453,7 +464,8 @@ class InstanceViewTest(test.BaseAdminViewTests): 'flavor_get', 'server_list_paged'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed_by_ids'], + api.glance: ['image_list_detailed'], + api.cinder: ['volume_list'] }) def _test_servers_paginate_do(self, marker, @@ -462,7 +474,6 @@ class InstanceViewTest(test.BaseAdminViewTests): has_prev): flavors = self.flavors.list() tenants = self.tenants.list() - images = self.images.list() # UUID indices are unique and are guaranteed being deterministic. for i, server in enumerate(servers): server.flavor['id'] = str(uuid.UUID(int=i)) @@ -470,7 +481,9 @@ class InstanceViewTest(test.BaseAdminViewTests): self.mock_server_list_paged.return_value = [ servers, has_more, has_prev] self.mock_flavor_list.return_value = flavors - self.mock_image_list_detailed_by_ids.return_value = images + self.mock_image_list_detailed.side_effect =\ + self._mock_image_list_detailed_side_effect + self.mock_volume_list.return_value = self.cinder_volumes.list() self.mock_tenant_list.return_value = [tenants, False] self.mock_flavor_get.side_effect = self.exceptions.nova @@ -483,9 +496,7 @@ class InstanceViewTest(test.BaseAdminViewTests): self.assertEqual(res.status_code, 200) self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest()) - self.mock_image_list_detailed_by_ids.assert_called_once_with( - test.IsHttpRequest(), - [server.image.id for server in servers]) + self.assertEqual(self.mock_image_list_detailed.call_count, 4) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) search_opts = {'marker': marker, 'paginate': True, 'all_tenants': True} self.mock_server_list_paged.assert_called_once_with( diff --git a/openstack_dashboard/dashboards/admin/instances/views.py b/openstack_dashboard/dashboards/admin/instances/views.py index 2bb18bbdb7..9dcd11068c 100644 --- a/openstack_dashboard/dashboards/admin/instances/views.py +++ b/openstack_dashboard/dashboards/admin/instances/views.py @@ -93,16 +93,24 @@ class AdminIndexView(tables.PagedTableMixin, tables.DataTableView): exceptions.handle(self.request, msg) return {} - def _get_images(self, instances=()): + def _get_images(self): # Gather our images to correlate our instances to them try: - # NOTE(aarefiev): request images, instances was booted from. - img_ids = (instance.image.get('id') for instance in - instances if isinstance(instance.image, dict)) - real_img_ids = list(filter(None, img_ids)) - images = api.glance.image_list_detailed_by_ids( - self.request, real_img_ids) - image_map = dict((image.id, image) for image in images) + # Community images have to be retrieved separately and merged, + # because their visibility has to be explicitly defined in the + # API call and the Glance API currently does not support filtering + # by multiple values in the visibility field. + # TODO(gabriel): Handle pagination. + images = api.glance.image_list_detailed(self.request)[0] + community_images = api.glance.image_list_detailed( + self.request, filters={'visibility': 'community'})[0] + image_map = { + image.id: image for image in images + } + # Images have to be filtered by their uuids; some users + # have default access to certain community images. + for image in community_images: + image_map.setdefault(image.id, image) return image_map except Exception: exceptions.handle(self.request, ignore=True) @@ -137,6 +145,15 @@ class AdminIndexView(tables.PagedTableMixin, tables.DataTableView): _('Unable to retrieve instance list.')) return instances + def _get_volumes(self): + # Gather our volumes to get their image metadata for instance + try: + volumes = api.cinder.volume_list(self.request) + return dict((str(volume.id), volume) for volume in volumes) + except Exception: + exceptions.handle(self.request, ignore=True) + return {} + def get_data(self): marker, sort_dir = self._get_marker() default_search_opts = {'marker': marker, @@ -158,9 +175,11 @@ class AdminIndexView(tables.PagedTableMixin, tables.DataTableView): self._needs_filter_first = False results = futurist_utils.call_functions_parallel( + self._get_images, + self._get_volumes, self._get_flavors, self._get_tenants) - flavor_dict, tenant_dict = results + image_dict, volume_dict, flavor_dict, tenant_dict = results non_api_filter_info = [ ('project', 'tenant_id', tenant_dict.values()), @@ -181,10 +200,11 @@ class AdminIndexView(tables.PagedTableMixin, tables.DataTableView): instances = self._get_instances(search_opts, sort_dir) if not filter_by_image_name: - image_dict = self._get_images(tuple(instances)) + image_dict = self._get_images() # Loop through instances to get image, flavor and tenant info. for inst in instances: + self._populate_image_info(inst, image_dict, volume_dict) if hasattr(inst, 'image') and isinstance(inst.image, dict): image_id = inst.image.get('id') if image_id in image_dict: @@ -211,6 +231,49 @@ class AdminIndexView(tables.PagedTableMixin, tables.DataTableView): inst.tenant_name = getattr(tenant, "name", None) return instances + def _populate_image_info(self, instance, image_dict, volume_dict): + if not hasattr(instance, 'image'): + return + # Instance from image returns dict + if isinstance(instance.image, dict): + image_id = instance.image.get('id') + if image_id in image_dict: + instance.image = image_dict[image_id] + # In case image not found in image_dict, set name to empty + # to avoid fallback API call to Glance in api/nova.py + # until the call is deprecated in api itself + else: + instance.image['name'] = _("-") + # Otherwise trying to get image from volume metadata + else: + instance_volumes = [ + attachment + for volume in volume_dict.values() + for attachment in volume.attachments + if attachment['server_id'] == instance.id + ] + # While instance from volume is being created, + # it does not have volumes + if not instance_volumes: + return + # Sorting attached volumes by device name (eg '/dev/sda') + instance_volumes.sort(key=lambda attach: attach['device']) + # Getting volume object, which is as attached + # as the first device + boot_volume = volume_dict[instance_volumes[0]['id']] + # There is a case where volume_image_metadata contains + # only fields other than 'image_id' (See bug 1834747), + # so we try to populate image information only when it is found. + volume_metadata = getattr(boot_volume, "volume_image_metadata", {}) + image_id = volume_metadata.get('image_id') + if image_id: + try: + instance.image = image_dict[image_id].to_dict() + except KeyError: + # KeyError occurs when volume was created from image and + # then this image is deleted. + pass + class LiveMigrateView(forms.ModalFormView): form_class = project_forms.LiveMigrateForm diff --git a/openstack_dashboard/test/unit/api/test_glance.py b/openstack_dashboard/test/unit/api/test_glance.py index 12f5e7aaa4..71edf7db8e 100644 --- a/openstack_dashboard/test/unit/api/test_glance.py +++ b/openstack_dashboard/test/unit/api/test_glance.py @@ -31,39 +31,6 @@ class GlanceApiTests(test.APIMockTestCase): super().setUp() api.glance.VERSIONS.clear_active_cache() - @override_settings(API_RESULT_PAGE_SIZE=2) - @mock.patch.object(api.glance, 'glanceclient') - def test_long_url(self, mock_glanceclient): - servers = self.servers.list() * 100 - api_images = self.images_api.list() * 100 - instances_img_ids = [instance.image.get('id') for instance in - servers if hasattr(instance, 'image')] - expected_images = self.images.list() * 100 - glanceclient = mock_glanceclient.return_value - mock_images_list = glanceclient.images.list - mock_images_list.return_value = iter(api_images) - - images = api.glance.image_list_detailed_by_ids(self.request, - instances_img_ids) - self.assertEqual(images, expected_images) - - @override_settings(API_RESULT_PAGE_SIZE=2) - @mock.patch.object(api.glance, 'glanceclient') - def test_image_list_detailed_by_ids(self, mock_glanceclient): - servers = self.servers.list() - api_images = self.images_api.list() - instances_img_ids = [instance.image.get('id') for instance in - servers if hasattr(instance, 'image')] - expected_images = self.images.list() - - glanceclient = mock_glanceclient.return_value - mock_images_list = glanceclient.images.list - mock_images_list.return_value = iter(api_images) - - images = api.glance.image_list_detailed_by_ids(self.request, - instances_img_ids) - self.assertEqual(images, expected_images) - @override_settings(API_RESULT_PAGE_SIZE=2) @mock.patch.object(api.glance, 'glanceclient') def test_image_list_detailed_no_pagination(self, mock_glanceclient):