Merge "Do not make unnecessary calls to Nova from FIPs and Volumes tabs"

openstack_dashboard/dashboards/admin/volumes/tabs.py

@@ -39,7 +39,9 @@ class VolumeTab(volumes_tabs.PagedTableMixin, tabs.TableTab,
 
     def get_volumes_data(self):
         volumes = self._get_volumes(search_opts={'all_tenants': True})
-        instances = self._get_instances(search_opts={'all_tenants': True})
+        attached_instance_ids = self._get_attached_instance_ids(volumes)
+        instances = self._get_instances(search_opts={'all_tenants': True},
+                                        instance_ids=attached_instance_ids)
         volume_ids_with_snapshots = self._get_volumes_ids_with_snapshots(
             search_opts={'all_tenants': True})
         self._set_volume_attributes(

openstack_dashboard/dashboards/admin/volumes/tests.py

@@ -12,6 +12,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import copy
+
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django import http
@@ -37,16 +39,22 @@ class VolumeTests(test.BaseAdminViewTests):
                         cinder: ('volume_list_paged',
                                  'volume_snapshot_list'),
                         keystone: ('tenant_list',)})
-    def test_index(self):
+    def _test_index(self, instanceless_volumes=False):
+        volumes = self.cinder_volumes.list()
+        if instanceless_volumes:
+            for volume in volumes:
+                volume.attachments = []
+
         cinder.volume_list_paged(IsA(http.HttpRequest), sort_dir="desc",
                                  marker=None, paginate=True,
                                  search_opts={'all_tenants': True})\
-            .AndReturn([self.cinder_volumes.list(), False, False])
+            .AndReturn([volumes, False, False])
         cinder.volume_snapshot_list(IsA(http.HttpRequest), search_opts={
             'all_tenants': True}).AndReturn([])
-        api.nova.server_list(IsA(http.HttpRequest), search_opts={
+        if not instanceless_volumes:
-                             'all_tenants': True}) \
+            api.nova.server_list(IsA(http.HttpRequest), search_opts={
-            .AndReturn([self.servers.list(), False])
+                                 'all_tenants': True}) \
+                .AndReturn([self.servers.list(), False])
         keystone.tenant_list(IsA(http.HttpRequest)) \
             .AndReturn([self.tenants.list(), False])
 
@@ -57,6 +65,12 @@ class VolumeTests(test.BaseAdminViewTests):
         volumes = res.context['volumes_table'].data
         self.assertItemsEqual(volumes, self.cinder_volumes.list())
 
+    def test_index_without_attachments(self):
+        self._test_index(instanceless_volumes=True)
+
+    def test_index_with_attachments(self):
+        self._test_index(instanceless_volumes=False)
+
     @test.create_stubs({api.nova: ('server_list',),
                         cinder: ('volume_list_paged',
                                  'volume_snapshot_list'),
@@ -86,10 +100,18 @@ class VolumeTests(test.BaseAdminViewTests):
         self.mox.UnsetStubs()
         return res
 
+    def ensure_attachments_exist(self, volumes):
+        volumes = copy.copy(volumes)
+        for volume in volumes:
+            if not volume.attachments:
+                volume.attachments.append({
+                    "id": "1", "server_id": '1', "device": "/dev/hda"})
+        return volumes
+
     @override_settings(API_RESULT_PAGE_SIZE=2)
     def test_index_paginated(self):
         size = settings.API_RESULT_PAGE_SIZE
-        mox_volumes = self.cinder_volumes.list()
+        mox_volumes = self.ensure_attachments_exist(self.cinder_volumes.list())
 
         # get first page
         expected_volumes = mox_volumes[:size]
@@ -125,7 +147,7 @@ class VolumeTests(test.BaseAdminViewTests):
     @override_settings(API_RESULT_PAGE_SIZE=2)
     def test_index_paginated_prev(self):
         size = settings.API_RESULT_PAGE_SIZE
-        mox_volumes = self.cinder_volumes.list()
+        mox_volumes = self.ensure_attachments_exist(self.cinder_volumes.list())
 
         # prev from some page
         expected_volumes = mox_volumes[size:2 * size]

openstack_dashboard/dashboards/project/access_and_security/tabs.py

@@ -104,18 +104,23 @@ class FloatingIPsTab(tabs.TableTab):
                               _('Unable to retrieve floating IP pools.'))
         pool_dict = dict([(obj.id, obj.name) for obj in floating_ip_pools])
 
-        instances = []
+        attached_instance_ids = [ip.instance_id for ip in floating_ips
-        try:
+                                 if ip.instance_id is not None]
-            instances, has_more = nova.server_list(self.request)
+        if attached_instance_ids:
-        except Exception:
+            instances = []
-            exceptions.handle(self.request,
+            try:
-                              _('Unable to retrieve instance list.'))
+                # TODO(tsufiev): we should pass attached_instance_ids to
+                # nova.server_list as soon as Nova API allows for this
+                instances, has_more = nova.server_list(self.request)
+            except Exception:
+                exceptions.handle(self.request,
+                                  _('Unable to retrieve instance list.'))
 
-        instances_dict = dict([(obj.id, obj.name) for obj in instances])
+            instances_dict = dict([(obj.id, obj.name) for obj in instances])
 
-        for ip in floating_ips:
+            for ip in floating_ips:
-            ip.instance_name = instances_dict.get(ip.instance_id)
+                ip.instance_name = instances_dict.get(ip.instance_id)
-            ip.pool_name = pool_dict.get(ip.pool, ip.pool)
+                ip.pool_name = pool_dict.get(ip.pool, ip.pool)
 
         return floating_ips
 

openstack_dashboard/dashboards/project/access_and_security/tests.py

@@ -44,16 +44,20 @@ class AccessAndSecurityTests(test.TestCase):
                                    'server_list',),
                         api.base: ('is_service_enabled',),
                         quotas: ('tenant_quota_usages',)})
-    def _test_index(self, ec2_enabled):
+    def _test_index(self, ec2_enabled=True, instanceless_ips=False):
         keypairs = self.keypairs.list()
         sec_groups = self.security_groups.list()
         floating_ips = self.floating_ips.list()
+        if instanceless_ips:
+            for fip in floating_ips:
+                fip.instance_id = None
         quota_data = self.quota_usages.first()
         quota_data['security_groups']['available'] = 10
 
-        api.nova.server_list(
+        if not instanceless_ips:
-            IsA(http.HttpRequest)) \
+            api.nova.server_list(
-            .AndReturn([self.servers.list(), False])
+                IsA(http.HttpRequest)) \
+                .AndReturn([self.servers.list(), False])
         api.nova.keypair_list(
             IsA(http.HttpRequest)) \
             .AndReturn(keypairs)
@@ -118,6 +122,9 @@ class AccessAndSecurityTests(test.TestCase):
     def test_index_with_ec2_disabled(self):
         self._test_index(ec2_enabled=False)
 
+    def test_index_with_instanceless_fips(self):
+        self._test_index(instanceless_ips=True)
+
     @test.create_stubs({api.network: ('floating_ip_target_list',
                                       'tenant_floating_ip_list',)})
     def test_association(self):

openstack_dashboard/dashboards/project/volumes/tabs.py

@@ -50,8 +50,12 @@ class VolumeTableMixIn(object):
                               _('Unable to retrieve volume list.'))
             return []
 
-    def _get_instances(self, search_opts=None):
+    def _get_instances(self, search_opts=None, instance_ids=None):
+        if not instance_ids:
+            return []
         try:
+            # TODO(tsufiev): we should pass attached_instance_ids to
+            # nova.server_list as soon as Nova API allows for this
             instances, has_more = api.nova.server_list(self.request,
                                                        search_opts=search_opts)
             return instances
@@ -75,6 +79,15 @@ class VolumeTableMixIn(object):
 
         return volume_ids
 
+    def _get_attached_instance_ids(self, volumes):
+        attached_instance_ids = []
+        for volume in volumes:
+            for att in volume.attachments:
+                server_id = att.get('server_id', None)
+                if server_id is not None:
+                    attached_instance_ids.append(server_id)
+        return attached_instance_ids
+
     # set attachment string and if volume has snapshots
     def _set_volume_attributes(self,
                                volumes,
@@ -85,9 +98,10 @@ class VolumeTableMixIn(object):
             if volume_ids_with_snapshots:
                 if volume.id in volume_ids_with_snapshots:
                     setattr(volume, 'has_snapshot', True)
-            for att in volume.attachments:
+            if instances:
-                server_id = att.get('server_id', None)
+                for att in volume.attachments:
-                att['instance'] = instances.get(server_id, None)
+                    server_id = att.get('server_id', None)
+                    att['instance'] = instances.get(server_id, None)
 
 
 class PagedTableMixin(object):
@@ -123,7 +137,8 @@ class VolumeTab(PagedTableMixin, tabs.TableTab, VolumeTableMixIn):
 
     def get_volumes_data(self):
         volumes = self._get_volumes()
-        instances = self._get_instances()
+        attached_instance_ids = self._get_attached_instance_ids(volumes)
+        instances = self._get_instances(instance_ids=attached_instance_ids)
         volume_ids_with_snapshots = self._get_volumes_ids_with_snapshots()
         self._set_volume_attributes(
             volumes, instances, volume_ids_with_snapshots)

openstack_dashboard/dashboards/project/volumes/test.py

@@ -12,6 +12,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import copy
+
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django import http
@@ -44,10 +46,13 @@ class VolumeAndSnapshotsAndBackupsTests(test.TestCase):
                                      'volume_backup_list_paged',
                                      ),
                         api.nova: ('server_list',)})
-    def _test_index(self, backup_supported=True):
+    def _test_index(self, backup_supported=True, instanceless_volumes=False):
         vol_backups = self.cinder_volume_backups.list()
         vol_snaps = self.cinder_volume_snapshots.list()
         volumes = self.cinder_volumes.list()
+        if instanceless_volumes:
+            for volume in volumes:
+                volume.attachments = []
 
         api.cinder.volume_backup_supported(IsA(http.HttpRequest)).\
             MultipleTimes().AndReturn(backup_supported)
@@ -55,8 +60,9 @@ class VolumeAndSnapshotsAndBackupsTests(test.TestCase):
             IsA(http.HttpRequest), marker=None, search_opts=None,
             sort_dir='desc', paginate=True).\
             AndReturn([volumes, False, False])
-        api.nova.server_list(IsA(http.HttpRequest), search_opts=None).\
+        if not instanceless_volumes:
-            AndReturn([self.servers.list(), False])
+            api.nova.server_list(IsA(http.HttpRequest), search_opts=None).\
+                AndReturn([self.servers.list(), False])
         api.cinder.volume_snapshot_list(IsA(http.HttpRequest)).\
             AndReturn(vol_snaps)
 
@@ -93,6 +99,9 @@ class VolumeAndSnapshotsAndBackupsTests(test.TestCase):
     def test_index_backup_not_supported(self):
         self._test_index(backup_supported=False)
 
+    def test_index_no_volume_attachments(self):
+        self._test_index(instanceless_volumes=True)
+
     @test.create_stubs({api.cinder: ('tenant_absolute_limits',
                                      'volume_list_paged',
                                      'volume_backup_supported',
@@ -124,9 +133,17 @@ class VolumeAndSnapshotsAndBackupsTests(test.TestCase):
         self.mox.UnsetStubs()
         return res
 
+    def ensure_attachments_exist(self, volumes):
+        volumes = copy.copy(volumes)
+        for volume in volumes:
+            if not volume.attachments:
+                volume.attachments.append({
+                    "id": "1", "server_id": '1', "device": "/dev/hda"})
+        return volumes
+
     @override_settings(API_RESULT_PAGE_SIZE=2)
     def test_index_paginated(self):
-        mox_volumes = self.cinder_volumes.list()
+        mox_volumes = self.ensure_attachments_exist(self.cinder_volumes.list())
         size = settings.API_RESULT_PAGE_SIZE
 
         # get first page
@@ -162,7 +179,7 @@ class VolumeAndSnapshotsAndBackupsTests(test.TestCase):
 
     @override_settings(API_RESULT_PAGE_SIZE=2)
     def test_index_paginated_prev_page(self):
-        mox_volumes = self.cinder_volumes.list()
+        mox_volumes = self.ensure_attachments_exist(self.cinder_volumes.list())
         size = settings.API_RESULT_PAGE_SIZE
 
         # prev from some page