openstack
/
horizon
Code Issues Proposed changes

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

If none of the floating IPs listed within Horizon are attached to an
instance, do not request a list of Nova instances (because we have no
use for it). Apply the same behavior for Volumes tab (both Admin and
Project). This patch both saves us from making unnecessary and
potentially expensive call to Nova and lays the ground for a more
selective request of instance details from Nova. The latter will be
possible once Nova supports filtering instance details by >1
instance_id. For reference see
https://blueprints.launchpad.net/nova/+spec/get-multi-servers-filter

Change-Id: Ie7563b9e03b286b4cf51507463213162af3383b1
Partial-Bug: #1442310
This commit is contained in:
Timur Sufiev 2015-08-14 20:20:45 +03:00
parent e8fec35f71
commit ed076e6234
6 changed files with 100 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
openstack_dashboard/dashboards/admin/volumes/tabs.py
View File

@ -40,7 +40,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(

36
openstack_dashboard/dashboards/admin/volumes/tests.py
View File

@ -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={
'all_tenants': True}) \
.AndReturn([self.servers.list(), False])
if not instanceless_volumes:
api.nova.server_list(IsA(http.HttpRequest), search_opts={
'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',),
keystone: ('tenant_list',)})
@ -82,10 +96,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]
@ -121,7 +143,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]

25
openstack_dashboard/dashboards/project/access_and_security/tabs.py
View File

@ -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 = []
try:
instances, has_more = nova.server_list(self.request)
except Exception:
exceptions.handle(self.request,
_('Unable to retrieve instance list.'))
attached_instance_ids = [ip.instance_id for ip in floating_ips
if ip.instance_id is not None]
if attached_instance_ids:
instances = []
try:
# 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:
ip.instance_name = instances_dict.get(ip.instance_id)
ip.pool_name = pool_dict.get(ip.pool, ip.pool)
for ip in floating_ips:
ip.instance_name = instances_dict.get(ip.instance_id)
ip.pool_name = pool_dict.get(ip.pool, ip.pool)
return floating_ips

15
openstack_dashboard/dashboards/project/access_and_security/tests.py
View File

@ -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(
IsA(http.HttpRequest)) \
.AndReturn([self.servers.list(), False])
if not instanceless_ips:
api.nova.server_list(
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):

25
openstack_dashboard/dashboards/project/volumes/tabs.py
View File

@ -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:
server_id = att.get('server_id', None)
att['instance'] = instances.get(server_id, None)
if instances:
for att in volume.attachments:
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)

27
openstack_dashboard/dashboards/project/volumes/test.py
View File

@ -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).\
AndReturn([self.servers.list(), False])
if not instanceless_volumes:
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',
@ -121,9 +130,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
@ -159,7 +176,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