Browse Source

Add "prev" link to instance page list pagination

Currently there is no link to previous page at paginated instances
table. This patch resolves that issue by re-using the pagination
code for flavors.

It also supports Ying Zuo's scenario:
After I set only 1 item per page and deleted the instance on the first page,
the expected behavior is showing the next instance in the table after one is
deleted.

xxxIndexView uses PagedTableMixin's _get_marker() from now instead of GET()-
ing the markers

Closes-Bug: #1274427
Co-Authored-By: Dmitry Ratushnyy <dratushn@cisco.com>
Co-Authored-By: Akihiro Motoki <amotoki@gmail.com>
Change-Id: Id8eaae6bf1b5d6f42291291655e14b8715c08bc8
Signed-off-by: Ferenc Cserepkei <ferenc.cserepkei@ericsson.com>
(cherry picked from commit 4676694179)
tags/14.1.0
Ferenc Cserepkei 2 years ago
parent
commit
44a098abcf
7 changed files with 489 additions and 221 deletions
  1. +66
    -28
      openstack_dashboard/api/nova.py
  2. +160
    -24
      openstack_dashboard/dashboards/admin/instances/tests.py
  3. +11
    -8
      openstack_dashboard/dashboards/admin/instances/views.py
  4. +236
    -150
      openstack_dashboard/dashboards/project/instances/tests.py
  5. +11
    -8
      openstack_dashboard/dashboards/project/instances/views.py
  6. +1
    -1
      openstack_dashboard/test/integration_tests/tests/test_instances.py
  7. +4
    -2
      openstack_dashboard/test/unit/api/test_nova.py

+ 66
- 28
openstack_dashboard/api/nova.py View File

@@ -372,8 +372,7 @@ def flavor_list(request, is_public=True, get_extras=False):


@profiler.trace
def update_pagination(entities, page_size, marker, sort_dir, sort_key,
reversed_order):
def update_pagination(entities, page_size, marker, reversed_order=False):
has_more_data = has_prev_data = False
if len(entities) > page_size:
has_more_data = True
@@ -389,9 +388,7 @@ def update_pagination(entities, page_size, marker, sort_dir, sort_key,

# restore the original ordering here
if reversed_order:
entities = sorted(entities, key=lambda entity:
(getattr(entity, sort_key) or '').lower(),
reverse=(sort_dir == 'asc'))
entities.reverse()

return entities, has_more_data, has_prev_data

@@ -415,7 +412,7 @@ def flavor_list_paged(request, is_public=True, get_extras=False, marker=None,
sort_key=sort_key,
sort_dir=sort_dir)
flavors, has_more_data, has_prev_data = update_pagination(
flavors, page_size, marker, sort_dir, sort_key, reversed_order)
flavors, page_size, marker, reversed_order)
else:
flavors = novaclient(request).flavors.list(is_public=is_public)

@@ -546,6 +543,14 @@ def server_create(request, name, image, flavor, key_name, user_data,
@profiler.trace
def server_delete(request, instance_id):
novaclient(request).servers.delete(instance_id)
# Session is available and consistent for the current view
# among Horizon django servers even in load-balancing setup,
# so only the view listing the servers will recognize it as
# own DeleteInstance action performed. Note that dict is passed
# by reference in python. Quote from django's developer manual:
# " You can read it and write to request.session at any point
# in your view. You can edit it multiple times."
request.session['server_deleted'] = instance_id


def get_novaclient_with_locked_status(request):
@@ -565,33 +570,66 @@ def server_get(request, instance_id):


@profiler.trace
def server_list(request, search_opts=None, detailed=True):
def server_list_paged(request,
search_opts=None,
detailed=True,
sort_dir="desc"):
has_more_data = False
has_prev_data = False
nova_client = get_novaclient_with_locked_status(request)
page_size = utils.get_page_size(request)
paginate = False
if search_opts is None:
search_opts = {}
elif 'paginate' in search_opts:
paginate = search_opts.pop('paginate')
if paginate:
search_opts['limit'] = page_size + 1

all_tenants = search_opts.get('all_tenants', False)
if all_tenants:
search_opts['all_tenants'] = True
else:
search_opts = {} if search_opts is None else search_opts
marker = search_opts.get('marker', None)

if not search_opts.get('all_tenants', False):
search_opts['project_id'] = request.user.tenant_id

servers = [Server(s, request)
for s in nova_client.servers.list(detailed, search_opts)]
if search_opts.pop('paginate', False):
reversed_order = sort_dir is "asc"
LOG.debug("Notify received on deleted server: %r",
('server_deleted' in request.session))
deleted = request.session.pop('server_deleted',
None)
view_marker = 'possibly_deleted' if deleted and marker else 'ok'
search_opts['marker'] = deleted if deleted else marker
search_opts['limit'] = page_size + 1
search_opts['sort_dir'] = sort_dir
servers = [Server(s, request)
for s in nova_client.servers.list(detailed, search_opts)]
if view_marker == 'possibly_deleted':
if len(servers) == 0:
view_marker = 'head_deleted'
search_opts['sort_dir'] = 'desc'
reversed_order = False
servers = [Server(s, request)
for s in nova_client.servers.list(detailed,
search_opts)]
if len(servers) == 0:
view_marker = 'tail_deleted'
search_opts['sort_dir'] = 'asc'
reversed_order = True
servers = [Server(s, request)
for s in nova_client.servers.list(detailed,
search_opts)]
(servers, has_more_data, has_prev_data) = update_pagination(
servers, page_size, marker, reversed_order)
has_prev_data = (False
if view_marker == 'head_deleted'
else has_prev_data)
has_more_data = (False
if view_marker == 'tail_deleted'
else has_more_data)
else:
servers = [Server(s, request)
for s in nova_client.servers.list(detailed, search_opts)]
return (servers, has_more_data, has_prev_data)


has_more_data = False
if paginate and len(servers) > page_size:
servers.pop(-1)
has_more_data = True
elif paginate and len(servers) == getattr(settings, 'API_RESULT_LIMIT',
1000):
has_more_data = True
@profiler.trace
def server_list(request, search_opts=None, detailed=True):
(servers, has_more_data, _) = server_list_paged(request,
search_opts,
detailed)
return (servers, has_more_data)



+ 160
- 24
openstack_dashboard/dashboards/admin/instances/tests.py View File

@@ -13,10 +13,13 @@
# under the License.

from collections import OrderedDict

import uuid

import mock

from django.conf import settings
from django.test import override_settings
from django.urls import reverse

from openstack_dashboard import api
@@ -29,7 +32,7 @@ INDEX_TEMPLATE = 'horizon/common/_data_table_view.html'

class InstanceViewTest(test.BaseAdminViewTests):
@test.create_mocks({
api.nova: ['flavor_list', 'server_list', 'extension_supported'],
api.nova: ['flavor_list', 'server_list_paged', 'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
})
@@ -42,7 +45,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.mock_tenant_list.return_value = [self.tenants.list(), 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_server_list_paged.return_value = [servers, False, False]

res = self.client.get(INDEX_URL)
self.assertTemplateUsed(res, INDEX_TEMPLATE)
@@ -59,11 +62,13 @@ class InstanceViewTest(test.BaseAdminViewTests):
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(
test.IsHttpRequest(), search_opts=search_opts)
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)

@test.create_mocks({
api.nova: ['flavor_list', 'flavor_get', 'server_list',
api.nova: ['flavor_list', 'flavor_get', 'server_list_paged',
'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
@@ -74,7 +79,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
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_server_list_paged.return_value = [servers, False, False]
self.mock_extension_supported.return_value = True
self.mock_flavor_list.side_effect = self.exceptions.nova
self.mock_tenant_list.return_value = [self.tenants.list(), False]
@@ -92,8 +97,10 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.assertItemsEqual(instances, servers)

search_opts = {'marker': None, 'paginate': True, 'all_tenants': True}
self.mock_server_list.assert_called_once_with(
test.IsHttpRequest(), search_opts=search_opts)
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
self.mock_extension_supported.assert_has_calls([
mock.call('AdminActions', test.IsHttpRequest()),
mock.call('AdminActions', test.IsHttpRequest()),
@@ -108,7 +115,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
test.IsHttpRequest(), instances_img_ids)

@test.create_mocks({
api.nova: ['flavor_list', 'flavor_get', 'server_list',
api.nova: ['flavor_list', 'flavor_get', 'server_list_paged',
'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
@@ -124,7 +131,7 @@ class InstanceViewTest(test.BaseAdminViewTests):

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_server_list_paged.return_value = [servers, False, False]
self.mock_extension_supported.return_value = True
self.mock_tenant_list.return_value = [self.tenants.list(), False]
self.mock_flavor_get.side_effect = self.exceptions.nova
@@ -142,8 +149,10 @@ class InstanceViewTest(test.BaseAdminViewTests):
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(
test.IsHttpRequest(), search_opts=search_opts)
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
self.mock_extension_supported.assert_has_calls([
mock.call('AdminActions', test.IsHttpRequest()),
mock.call('AdminActions', test.IsHttpRequest()),
@@ -155,12 +164,12 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.assertEqual(len(servers), self.mock_flavor_get.call_count)

@test.create_mocks({
api.nova: ['server_list', 'flavor_list'],
api.nova: ['server_list_paged', 'flavor_list'],
api.keystone: ['tenant_list'],
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_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()
@@ -170,8 +179,9 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.assertEqual(len(res.context['instances_table'].data), 0)

search_opts = {'marker': None, 'paginate': True, 'all_tenants': True}
self.mock_server_list.assert_called_once_with(
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
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(
@@ -223,7 +233,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
test.IsHttpRequest(), [server])

@test.create_mocks({
api.nova: ['flavor_list', 'server_list', 'extension_supported'],
api.nova: ['flavor_list', 'server_list_paged', 'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
})
@@ -234,9 +244,9 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.mock_tenant_list.return_value = [self.tenants.list(), 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_server_list_paged.return_value = [
self.servers.list(), False, False]
self.mock_extension_supported.return_value = True

res = self.client.get(INDEX_URL)
self.assertContains(res, "instances__migrate")
self.assertNotContains(res, "instances__confirm")
@@ -247,8 +257,10 @@ class InstanceViewTest(test.BaseAdminViewTests):
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(
test.IsHttpRequest(), search_opts=search_opts)
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
self.mock_extension_supported.assert_has_calls([
mock.call('AdminActions', test.IsHttpRequest()),
mock.call('AdminActions', test.IsHttpRequest()),
@@ -256,7 +268,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.assertEqual(12, self.mock_extension_supported.call_count)

@test.create_mocks({
api.nova: ['flavor_list', 'server_list', 'extension_supported'],
api.nova: ['flavor_list', 'server_list_paged', 'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
})
@@ -272,7 +284,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
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]
self.mock_server_list_paged.return_value = [servers, False, False]

res = self.client.get(INDEX_URL)
self.assertContains(res, "instances__confirm")
@@ -289,8 +301,10 @@ class InstanceViewTest(test.BaseAdminViewTests):
mock.call('Shelve', test.IsHttpRequest())] * 4)
self.assertEqual(12, self.mock_extension_supported.call_count)
search_opts = {'marker': None, 'paginate': True, 'all_tenants': True}
self.mock_server_list.assert_called_once_with(
test.IsHttpRequest(), search_opts=search_opts)
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)

@test.create_mocks({api.nova: ['service_list',
'server_get']})
@@ -474,3 +488,125 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.assertTemplateUsed(res, INDEX_TEMPLATE)
instances = res.context['table'].data
self.assertItemsEqual(instances, [])

@test.create_mocks({
api.nova: ['flavor_list',
'flavor_get',
'server_list_paged',
'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
})
def _test_servers_paginate_do(self,
marker,
servers,
has_more,
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))

self.mock_server_list_paged.return_value = [
servers, has_more, has_prev]
self.mock_extension_supported.return_value = True
self.mock_flavor_list.return_value = flavors
self.mock_image_list_detailed_by_ids.return_value = images
self.mock_tenant_list.return_value = [tenants, False]
self.mock_flavor_get.side_effect = self.exceptions.nova

if marker:
url = "?".join([INDEX_URL, "marker={}".format(marker)])
else:
url = INDEX_URL
res = self.client.get(url)
self.assertTemplateUsed(res, INDEX_TEMPLATE)
self.assertEqual(res.status_code, 200)

self.mock_extension_supported.assert_has_calls([
mock.call('AdminActions', test.IsHttpRequest()),
mock.call('AdminActions', test.IsHttpRequest()),
mock.call('Shelve', test.IsHttpRequest())])
self.assertEqual(3, self.mock_extension_supported.call_count)
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.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(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
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)

return res

@override_settings(API_RESULT_PAGE_SIZE=1)
def test_severs_index_paginated(self):
size = settings.API_RESULT_PAGE_SIZE
mox_servers = self.servers.list()

# get first page
expected_servers = mox_servers[:size]
res = self._test_servers_paginate_do(
marker=None,
servers=expected_servers,
has_more=True,
has_prev=False)
servers = res.context['table'].data
self.assertItemsEqual(servers, expected_servers)

# get second page
expected_servers = mox_servers[size:2 * size]
marker = expected_servers[0].id
res = self._test_servers_paginate_do(
marker=marker,
servers=expected_servers,
has_more=True,
has_prev=True)
servers = res.context['table'].data
self.assertItemsEqual(servers, expected_servers)

# get last page
expected_servers = mox_servers[-size:]
marker = expected_servers[0].id
res = self._test_servers_paginate_do(
marker=marker,
servers=expected_servers,
has_more=False,
has_prev=True)
servers = res.context['table'].data
self.assertItemsEqual(servers, expected_servers)

@override_settings(API_RESULT_PAGE_SIZE=1)
def test_servers_index_paginated_prev(self):
size = settings.API_RESULT_PAGE_SIZE
mox_servers = self.servers.list()

# prev from some page
expected_servers = mox_servers[size:2 * size]
marker = mox_servers[0].id

res = self._test_servers_paginate_do(
marker=marker,
servers=expected_servers,
has_more=False,
has_prev=True)
servers = res.context['table'].data
self.assertItemsEqual(servers, expected_servers)

# back to first page
expected_servers = mox_servers[:size]
marker = mox_servers[0].id
res = self._test_servers_paginate_do(
marker=marker,
servers=expected_servers,
has_more=True,
has_prev=False)
servers = res.context['table'].data
self.assertItemsEqual(servers, expected_servers)

+ 11
- 8
openstack_dashboard/dashboards/admin/instances/views.py View File

@@ -70,10 +70,13 @@ class AdminUpdateView(views.UpdateView):
success_url = reverse_lazy("horizon:admin:instances:index")


class AdminIndexView(tables.DataTableView):
class AdminIndexView(tables.PagedTableMixin, tables.DataTableView):
table_class = project_tables.AdminInstancesTable
page_title = _("Instances")

def has_prev_data(self, table):
return getattr(self, "_prev", False)

def has_more_data(self, table):
return self._more

@@ -115,21 +118,21 @@ class AdminIndexView(tables.DataTableView):
exceptions.handle(self.request, msg)
return {}

def _get_instances(self, search_opts):
def _get_instances(self, search_opts, sort_dir):
try:
instances, self._more = api.nova.server_list(
instances, self._more, self._prev = api.nova.server_list_paged(
self.request,
search_opts=search_opts)
search_opts=search_opts,
sort_dir=sort_dir)
except Exception:
self._more = False
self._more = self._prev = False
instances = []
exceptions.handle(self.request,
_('Unable to retrieve instance list.'))
return instances

def get_data(self):
marker = self.request.GET.get(
project_tables.AdminInstancesTable._meta.pagination_param, None)
marker, sort_dir = self._get_marker()
default_search_opts = {'marker': marker,
'paginate': True,
'all_tenants': True}
@@ -148,7 +151,7 @@ class AdminIndexView(tables.DataTableView):

self._needs_filter_first = False

instances = self._get_instances(search_opts)
instances = self._get_instances(search_opts, sort_dir)
results = futurist_utils.call_functions_parallel(
(self._get_images, [tuple(instances)]),
self._get_flavors,

+ 236
- 150
openstack_dashboard/dashboards/project/instances/tests.py
File diff suppressed because it is too large
View File


+ 11
- 8
openstack_dashboard/dashboards/project/instances/views.py View File

@@ -59,10 +59,13 @@ from openstack_dashboard.views import get_url_with_pagination
LOG = logging.getLogger(__name__)


class IndexView(tables.DataTableView):
class IndexView(tables.PagedTableMixin, tables.DataTableView):
table_class = project_tables.InstancesTable
page_title = _("Instances")

def has_prev_data(self, table):
return getattr(self, "_prev", False)

def has_more_data(self, table):
return self._more

@@ -85,13 +88,14 @@ class IndexView(tables.DataTableView):
exceptions.handle(self.request, ignore=True)
return {}

def _get_instances(self, search_opts):
def _get_instances(self, search_opts, sort_dir):
try:
instances, self._more = api.nova.server_list(
instances, self._more, self._prev = api.nova.server_list_paged(
self.request,
search_opts=search_opts)
search_opts=search_opts,
sort_dir=sort_dir)
except Exception:
self._more = False
self._more = self._prev = False
instances = []
exceptions.handle(self.request,
_('Unable to retrieve instances.'))
@@ -122,8 +126,7 @@ class IndexView(tables.DataTableView):
return instances

def get_data(self):
marker = self.request.GET.get(
project_tables.InstancesTable._meta.pagination_param, None)
marker, sort_dir = self._get_marker()
search_opts = self.get_filters({'marker': marker, 'paginate': True})

image_dict, flavor_dict = futurist_utils.call_functions_parallel(
@@ -137,7 +140,7 @@ class IndexView(tables.DataTableView):
self._more = False
return []

instances = self._get_instances(search_opts)
instances = self._get_instances(search_opts, sort_dir)

# Loop through instances to get flavor info.
for instance in instances:

+ 1
- 1
openstack_dashboard/test/integration_tests/tests/test_instances.py View File

@@ -73,7 +73,7 @@ class TestInstances(helpers.TestCase):
first_page_definition = {'Next': True, 'Prev': False,
'Count': items_per_page,
'Names': [instance_list[1]]}
second_page_definition = {'Next': False, 'Prev': False,
second_page_definition = {'Next': False, 'Prev': True,
'Count': items_per_page,
'Names': [instance_list[0]]}
settings_page = self.home_pg.go_to_settings_usersettingspage()

+ 4
- 2
openstack_dashboard/test/unit/api/test_nova.py View File

@@ -204,7 +204,8 @@ class ComputeApiTests(test.APIMockTestCase):
True,
{'all_tenants': True,
'marker': None,
'limit': page_size + 1})
'limit': page_size + 1,
'sort_dir': 'desc'})

@override_settings(API_RESULT_PAGE_SIZE=1)
@mock.patch.object(api.nova, 'novaclient')
@@ -229,7 +230,8 @@ class ComputeApiTests(test.APIMockTestCase):
True,
{'all_tenants': True,
'marker': None,
'limit': page_size + 1})
'limit': page_size + 1,
'sort_dir': 'desc'})

@mock.patch.object(api.nova, 'novaclient')
def test_usage_get(self, mock_novaclient):

Loading…
Cancel
Save