diff --git a/openstack_dashboard/api/glance.py b/openstack_dashboard/api/glance.py index 5ee121730d..9f85b1507c 100644 --- a/openstack_dashboard/api/glance.py +++ b/openstack_dashboard/api/glance.py @@ -17,6 +17,7 @@ # under the License. from __future__ import absolute_import +from __future__ import division import collections import itertools @@ -56,6 +57,18 @@ try: except ImportError: pass +# 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", @@ -255,6 +268,19 @@ 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 89d403938c..d994cd18f0 100644 --- a/openstack_dashboard/dashboards/admin/instances/tests.py +++ b/openstack_dashboard/dashboards/admin/instances/tests.py @@ -31,14 +31,16 @@ class InstanceViewTest(test.BaseAdminViewTests): @test.create_mocks({ api.nova: ['flavor_list', 'server_list', 'extension_supported'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed'], + api.glance: ['image_list_detailed_by_ids'], }) 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_extension_supported.return_value = True self.mock_tenant_list.return_value = [self.tenants.list(), False] - self.mock_image_list_detailed.return_value = (self.images.list(), - False, False) + self.mock_image_list_detailed_by_ids.return_value = self.images.list() self.mock_flavor_list.return_value = self.flavors.list() self.mock_server_list.return_value = [servers, False] @@ -53,8 +55,8 @@ class InstanceViewTest(test.BaseAdminViewTests): mock.call('Shelve', test.IsHttpRequest())] * 4) self.assertEqual(12, self.mock_extension_supported.call_count) self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest()) - self.mock_image_list_detailed.assert_called_once_with( - test.IsHttpRequest()) + self.mock_image_list_detailed_by_ids.assert_called_once_with( + test.IsHttpRequest(), instances_img_ids) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) search_opts = {'marker': None, 'paginate': True, 'all_tenants': True} self.mock_server_list.assert_called_once_with( @@ -64,11 +66,13 @@ class InstanceViewTest(test.BaseAdminViewTests): api.nova: ['flavor_list', 'flavor_get', 'server_list', 'extension_supported'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed'], + api.glance: ['image_list_detailed_by_ids'], }) 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.return_value = [servers, False] self.mock_extension_supported.return_value = True @@ -79,8 +83,7 @@ class InstanceViewTest(test.BaseAdminViewTests): return full_flavors[id] self.mock_flavor_get.side_effect = _get_full_flavor - self.mock_image_list_detailed.return_value = (self.images.list(), - False, False) + self.mock_image_list_detailed_by_ids.return_value = self.images.list() res = self.client.get(INDEX_URL) @@ -101,24 +104,25 @@ 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.assert_called_once_with( - test.IsHttpRequest()) + self.mock_image_list_detailed_by_ids.assert_called_once_with( + test.IsHttpRequest(), instances_img_ids) @test.create_mocks({ api.nova: ['flavor_list', 'flavor_get', 'server_list', 'extension_supported'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed'], + api.glance: ['image_list_detailed_by_ids'], }) 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.return_value = \ - (self.images.list(), False, False) + self.mock_image_list_detailed_by_ids.return_value = self.images.list() self.mock_flavor_list.return_value = self.flavors.list() self.mock_server_list.return_value = [servers, False] self.mock_extension_supported.return_value = True @@ -134,8 +138,8 @@ class InstanceViewTest(test.BaseAdminViewTests): self.assertMessageCount(res, error=1) self.assertItemsEqual(instances, servers) - self.mock_image_list_detailed.assert_called_once_with( - test.IsHttpRequest()) + self.mock_image_list_detailed_by_ids.assert_called_once_with( + test.IsHttpRequest(), instances_img_ids) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) search_opts = {'marker': None, 'paginate': True, 'all_tenants': True} self.mock_server_list.assert_called_once_with( @@ -153,14 +157,13 @@ class InstanceViewTest(test.BaseAdminViewTests): @test.create_mocks({ api.nova: ['server_list', 'flavor_list'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed'], + api.glance: ['image_list_detailed_by_ids'], }) def test_index_server_list_exception(self): self.mock_server_list.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.return_value = (self.images.list(), - False, False) + self.mock_image_list_detailed_by_ids.return_value = self.images.list() res = self.client.get(INDEX_URL) self.assertTemplateUsed(res, INDEX_TEMPLATE) @@ -171,8 +174,8 @@ class InstanceViewTest(test.BaseAdminViewTests): test.IsHttpRequest(), search_opts=search_opts) self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest()) - self.mock_image_list_detailed.assert_called_once_with( - test.IsHttpRequest()) + self.mock_image_list_detailed_by_ids.assert_called_once_with( + test.IsHttpRequest(), []) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) @test.create_mocks({api.nova: ['server_get', 'flavor_get', @@ -222,12 +225,14 @@ class InstanceViewTest(test.BaseAdminViewTests): @test.create_mocks({ api.nova: ['flavor_list', 'server_list', 'extension_supported'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed'], + api.glance: ['image_list_detailed_by_ids'], }) 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.return_value = (self.images.list(), - False, False) + self.mock_image_list_detailed_by_ids.return_value = self.images.list() self.mock_flavor_list.return_value = self.flavors.list() self.mock_server_list.return_value = [self.servers.list(), False] self.mock_extension_supported.return_value = True @@ -238,8 +243,8 @@ class InstanceViewTest(test.BaseAdminViewTests): self.assertNotContains(res, "instances__revert") self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest()) - self.mock_image_list_detailed.assert_called_once_with( - test.IsHttpRequest()) + self.mock_image_list_detailed_by_ids.assert_called_once_with( + test.IsHttpRequest(), instances_img_ids) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) search_opts = {'marker': None, 'paginate': True, 'all_tenants': True} self.mock_server_list.assert_called_once_with( @@ -253,7 +258,7 @@ class InstanceViewTest(test.BaseAdminViewTests): @test.create_mocks({ api.nova: ['flavor_list', 'server_list', 'extension_supported'], api.keystone: ['tenant_list'], - api.glance: ['image_list_detailed'], + api.glance: ['image_list_detailed_by_ids'], }) def test_index_options_after_migrate(self): servers = self.servers.list() @@ -261,9 +266,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.return_value = (self.images.list(), - False, False) + self.mock_image_list_detailed_by_ids.return_value = self.images.list() self.mock_flavor_list.return_value = self.flavors.list() self.mock_extension_supported.return_value = True self.mock_server_list.return_value = [servers, False] @@ -274,8 +280,8 @@ class InstanceViewTest(test.BaseAdminViewTests): self.assertNotContains(res, "instances__migrate") self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest()) - self.mock_image_list_detailed.assert_called_once_with( - test.IsHttpRequest()) + self.mock_image_list_detailed_by_ids.assert_called_once_with( + test.IsHttpRequest(), instances_img_ids) self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest()) self.mock_extension_supported.assert_has_calls([ mock.call('AdminActions', test.IsHttpRequest()), diff --git a/openstack_dashboard/dashboards/admin/instances/views.py b/openstack_dashboard/dashboards/admin/instances/views.py index 0c906de5a1..4825d2c42d 100644 --- a/openstack_dashboard/dashboards/admin/instances/views.py +++ b/openstack_dashboard/dashboards/admin/instances/views.py @@ -90,14 +90,19 @@ class AdminIndexView(tables.DataTableView): exceptions.handle(self.request, msg) return {} - def _get_images(self): - # Gather our images to correlate againts IDs + def _get_images(self, instances=()): + # Gather our images to correlate our instances to them try: - images, __, __ = api.glance.image_list_detailed(self.request) - return dict([(image.id, image) for image in images]) + # 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) + return image_map except Exception: - msg = _("Unable to retrieve image list.") - exceptions.handle(self.request, msg) + exceptions.handle(self.request, ignore=True) return {} def _get_flavors(self): @@ -123,8 +128,6 @@ class AdminIndexView(tables.DataTableView): return instances def get_data(self): - instances = [] - marker = self.request.GET.get( project_tables.AdminInstancesTable._meta.pagination_param, None) default_search_opts = {'marker': marker, @@ -145,8 +148,11 @@ class AdminIndexView(tables.DataTableView): self._needs_filter_first = False + instances = self._get_instances(search_opts) results = futurist_utils.call_functions_parallel( - self._get_images, self._get_flavors, self._get_tenants) + (self._get_images, [tuple(instances)]), + self._get_flavors, + self._get_tenants) image_dict, flavor_dict, tenant_dict = results non_api_filter_info = ( @@ -158,8 +164,6 @@ class AdminIndexView(tables.DataTableView): self._more = False return [] - instances = self._get_instances(search_opts) - # Loop through instances to get image, flavor and tenant info. for inst in instances: if hasattr(inst, 'image') and isinstance(inst.image, dict): diff --git a/openstack_dashboard/test/unit/api/test_glance.py b/openstack_dashboard/test/unit/api/test_glance.py index 433fa23610..72482f3ecb 100644 --- a/openstack_dashboard/test/unit/api/test_glance.py +++ b/openstack_dashboard/test/unit/api/test_glance.py @@ -30,6 +30,39 @@ class GlanceApiTests(test.APIMockTestCase): super(GlanceApiTests, self).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):