Fix listing deleted servers with a marker
Change I1aa3ca6cc70cef65d24dec1e7db9491c9b73f7ab in Queens, which was backported through to Newton, introduced a regression when listing deleted servers with a marker because it assumes that if BuildRequestList.get_by_filters does not raise MarkerNotFound that the marker was found among the build requests and does not account for that get_by_filters method short-circuiting if filtering servers with deleted/cleaned/limit=0. The API code then nulls out the marker which means you'll continue to get the marker instance back in the results even though you shouldn't, and that can cause an infinite loop in some client-side tooling like nova's CLI: nova list --deleted --limit -1 This fixes the bug by raising MarkerNotFound from BuildRequestList.get_by_filters if we have a marker but are short-circuiting and returning early from the method based on limit or filters. Change-Id: Ic2b19c2aa06b3059ab0344b6ac56ffd62b3f755d Closes-Bug: #1849409 (cherry picked from commitdf03499843
) (cherry picked from commit03a2508362
) (cherry picked from commit6038455e1d
) (cherry picked from commitc84846dbf9
) (cherry picked from commit8c18293f93
)
This commit is contained in:
parent
e2ba87cb7b
commit
b277ec154f
|
@ -374,14 +374,16 @@ class BuildRequestList(base.ObjectListBase, base.NovaObject):
|
||||||
@base.remotable_classmethod
|
@base.remotable_classmethod
|
||||||
def get_by_filters(cls, context, filters, limit=None, marker=None,
|
def get_by_filters(cls, context, filters, limit=None, marker=None,
|
||||||
sort_keys=None, sort_dirs=None):
|
sort_keys=None, sort_dirs=None):
|
||||||
if limit == 0:
|
# Short-circuit on anything that will not yield results.
|
||||||
return cls(context, objects=[])
|
|
||||||
# 'deleted' records can not be returned from here since build_requests
|
# 'deleted' records can not be returned from here since build_requests
|
||||||
# are not soft deleted.
|
# are not soft deleted.
|
||||||
if filters.get('deleted', False):
|
|
||||||
return cls(context, objects=[])
|
|
||||||
# 'cleaned' records won't exist as they would need to be deleted.
|
# 'cleaned' records won't exist as they would need to be deleted.
|
||||||
if filters.get('cleaned', False):
|
if (limit == 0 or
|
||||||
|
filters.get('deleted', False) or
|
||||||
|
filters.get('cleaned', False)):
|
||||||
|
# If we have a marker honor the MarkerNotFound semantics.
|
||||||
|
if marker:
|
||||||
|
raise exception.MarkerNotFound(marker=marker)
|
||||||
return cls(context, objects=[])
|
return cls(context, objects=[])
|
||||||
|
|
||||||
# Because the build_requests table stores an instance as a serialized
|
# Because the build_requests table stores an instance as a serialized
|
||||||
|
|
|
@ -63,7 +63,5 @@ class ListDeletedServersWithMarker(test.TestCase,
|
||||||
servers = self.api.get_servers(detail=False,
|
servers = self.api.get_servers(detail=False,
|
||||||
search_opts={'deleted': True,
|
search_opts={'deleted': True,
|
||||||
'marker': server['id']})
|
'marker': server['id']})
|
||||||
# FIXME(mriedem): This is bug 1849409 where the marker param is not
|
|
||||||
# honored correctly when using deleted=True.
|
|
||||||
server_names = [serv['name'] for serv in servers]
|
server_names = [serv['name'] for serv in servers]
|
||||||
self.assertIn(server['name'], server_names)
|
self.assertNotIn(server['name'], server_names)
|
||||||
|
|
Loading…
Reference in New Issue