From 5f27609352a8f7f8bfdca1df14afad8c15dc0f93 Mon Sep 17 00:00:00 2001 From: KeithMnemonic Date: Wed, 18 Dec 2019 11:22:09 -0800 Subject: [PATCH] Fix "prev" link pagination for instances with identical timestamps This patch resolves an issue with the "prev" link when instances have identical "created_at" values. This can occur when creating instance using the "min/max count" option. The reverse sort does not work correctly as the server list returned from nova is not an exact reverse as the forward sort. It looks like the combination of sort_keys must be unique to ensure the forward and reverse pagination properly. As a workaround 'uuid' (server ID) is added to 'sort_keys'. In addition, 'display_name' is added before 'uuid' in 'sort_keys' to list servers in the alphabetical order (which sounds natural). Closes-Bug #1856243 Change-Id: I73234b2c69ce8ea648b4a9721abe4f5670031909 (cherry picked from commit 9637d733749d741d5aac3b89b92e100d32fbdbb0) --- openstack_dashboard/api/nova.py | 30 ++++++++++++++----- .../test/unit/api/test_nova.py | 10 ++++--- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/openstack_dashboard/api/nova.py b/openstack_dashboard/api/nova.py index b07a4b09b7..b3f20fb43d 100644 --- a/openstack_dashboard/api/nova.py +++ b/openstack_dashboard/api/nova.py @@ -489,24 +489,38 @@ def server_list_paged(request, 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 + # NOTE(amotoki): It looks like the 'sort_keys' must be unique to make + # the pagination in the nova API works as expected. Multiple servers + # can have a same 'created_at' as its resolution is a second. + # To ensure the uniqueness we add 'uuid' to the sort keys. + # 'display_name' is added before 'uuid' to list servers in the + # alphabetical order. + sort_keys = ['created_at', 'display_name', 'uuid'] + servers = [Server(s, request) - for s in nova_client.servers.list(detailed, search_opts)] + for s in nova_client.servers.list(detailed, search_opts, + sort_keys=sort_keys, + sort_dirs=[sort_dir] * 3)] + if view_marker == 'possibly_deleted': if not servers: 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)] + for s in + nova_client.servers.list(detailed, + search_opts, + sort_keys=sort_keys, + sort_dirs=['desc'] * 3)] if not servers: 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)] + for s in + nova_client.servers.list(detailed, + search_opts, + sort_keys=sort_keys, + sort_dirs=['asc'] * 3)] (servers, has_more_data, has_prev_data) = update_pagination( servers, page_size, marker, reversed_order) has_prev_data = (False diff --git a/openstack_dashboard/test/unit/api/test_nova.py b/openstack_dashboard/test/unit/api/test_nova.py index 52e71f8300..d5b36a294f 100644 --- a/openstack_dashboard/test/unit/api/test_nova.py +++ b/openstack_dashboard/test/unit/api/test_nova.py @@ -204,8 +204,9 @@ class ComputeApiTests(test.APIMockTestCase): True, {'all_tenants': True, 'marker': None, - 'limit': page_size + 1, - 'sort_dir': 'desc'}) + 'limit': page_size + 1}, + sort_dirs=['desc', 'desc', 'desc'], + sort_keys=['created_at', 'display_name', 'uuid']) @override_settings(API_RESULT_PAGE_SIZE=1) @mock.patch.object(api._nova, 'novaclient') @@ -230,8 +231,9 @@ class ComputeApiTests(test.APIMockTestCase): True, {'all_tenants': True, 'marker': None, - 'limit': page_size + 1, - 'sort_dir': 'desc'}) + 'limit': page_size + 1}, + sort_dirs=['desc', 'desc', 'desc'], + sort_keys=['created_at', 'display_name', 'uuid']) @mock.patch.object(api._nova, 'novaclient') def test_usage_get(self, mock_novaclient):