Merge "Show image names at Admin-Instances dashboard"

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,

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(
+        self.assertEqual(self.mock_image_list_detailed.call_count, 4)
-            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_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(
+        self.assertEqual(self.mock_image_list_detailed.call_count, 4)
-            test.IsHttpRequest(), instances_img_ids)
 
     @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(
+        self.assertEqual(self.mock_image_list_detailed.call_count, 4)
-            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_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(
+        self.assertEqual(self.mock_image_list_detailed.call_count, 4)
-            test.IsHttpRequest(), [])
         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(
+        self.assertEqual(self.mock_image_list_detailed.call_count, 4)
-            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_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(
+        self.assertEqual(self.mock_image_list_detailed.call_count, 4)
-            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_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(
+        self.assertEqual(self.mock_image_list_detailed.call_count, 4)
-            test.IsHttpRequest(),
-            [server.image.id for server in servers])
         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(

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.
+            # Community images have to be retrieved separately and merged,
-            img_ids = (instance.image.get('id') for instance in
+            # because their visibility has to be explicitly defined in the
-                       instances if isinstance(instance.image, dict))
+            # API call and the Glance API currently does not support filtering
-            real_img_ids = list(filter(None, img_ids))
+            # by multiple values in the visibility field.
-            images = api.glance.image_list_detailed_by_ids(
+            # TODO(gabriel): Handle pagination.
-                self.request, real_img_ids)
+            images = api.glance.image_list_detailed(self.request)[0]
-            image_map = dict((image.id, image) for image in images)
+            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

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):