Do not ignore 'fields' query parameter when building next url
When an user calls the GET on an ironic resource it returns
MAX_LIMIT number of resources at a time along with a next url.
The default MAX_LIMIT is 1000.
If the user requested specific set of fields from ironic API
using the fields query parameter (eg: /v1/resource?fields=f1,f2,f3)
The next url returned by the API ignores fields query parameter.
This results in fields missing from the results after MAX_LIMIT
is reached.
This change fixes this problem by passing the fields as parameter
to collections.get_next method and using the fields argument to
build the query parameter.
Change-Id: I62b59e8148171c72de0ccf63a1517e754b520c76
Story: 2006721
Task: 37093
(cherry picked from commit e36f72d36d
)
This commit is contained in:
parent
10d63e4a02
commit
56193d166a
|
@ -186,7 +186,8 @@ class AllocationCollection(collection.Collection):
|
||||||
Allocation.convert_with_links(p, fields=fields, sanitize=False)
|
Allocation.convert_with_links(p, fields=fields, sanitize=False)
|
||||||
for p in rpc_allocations
|
for p in rpc_allocations
|
||||||
]
|
]
|
||||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
collection.next = collection.get_next(limit, url=url, fields=fields,
|
||||||
|
**kwargs)
|
||||||
|
|
||||||
for item in collection.allocations:
|
for item in collection.allocations:
|
||||||
item.sanitize(fields=fields)
|
item.sanitize(fields=fields)
|
||||||
|
|
|
@ -157,7 +157,8 @@ class ChassisCollection(collection.Collection):
|
||||||
sanitize=False)
|
sanitize=False)
|
||||||
for ch in chassis]
|
for ch in chassis]
|
||||||
url = url or None
|
url = url or None
|
||||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
collection.next = collection.get_next(limit, url=url, fields=fields,
|
||||||
|
**kwargs)
|
||||||
for item in collection.chassis:
|
for item in collection.chassis:
|
||||||
item.sanitize(fields)
|
item.sanitize(fields)
|
||||||
return collection
|
return collection
|
||||||
|
|
|
@ -43,6 +43,11 @@ class Collection(base.APIBase):
|
||||||
return wtypes.Unset
|
return wtypes.Unset
|
||||||
|
|
||||||
resource_url = url or self._type
|
resource_url = url or self._type
|
||||||
|
fields = kwargs.pop('fields', None)
|
||||||
|
# NOTE(saga): If fields argument is present in kwargs and not None. It
|
||||||
|
# is a list so convert it into a comma seperated string.
|
||||||
|
if fields:
|
||||||
|
kwargs['fields'] = ','.join(fields)
|
||||||
q_args = ''.join(['%s=%s&' % (key, kwargs[key]) for key in kwargs])
|
q_args = ''.join(['%s=%s&' % (key, kwargs[key]) for key in kwargs])
|
||||||
next_args = '?%(args)slimit=%(limit)d&marker=%(marker)s' % {
|
next_args = '?%(args)slimit=%(limit)d&marker=%(marker)s' % {
|
||||||
'args': q_args, 'limit': limit,
|
'args': q_args, 'limit': limit,
|
||||||
|
|
|
@ -140,7 +140,8 @@ class ConductorCollection(collection.Collection):
|
||||||
collection = ConductorCollection()
|
collection = ConductorCollection()
|
||||||
collection.conductors = [Conductor.convert_with_links(c, fields=fields)
|
collection.conductors = [Conductor.convert_with_links(c, fields=fields)
|
||||||
for c in conductors]
|
for c in conductors]
|
||||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
collection.next = collection.get_next(limit, url=url, fields=fields,
|
||||||
|
**kwargs)
|
||||||
|
|
||||||
for conductor in collection.conductors:
|
for conductor in collection.conductors:
|
||||||
conductor.sanitize(fields)
|
conductor.sanitize(fields)
|
||||||
|
|
|
@ -238,7 +238,7 @@ class DeployTemplateCollection(collection.Collection):
|
||||||
collection.deploy_templates = [
|
collection.deploy_templates = [
|
||||||
DeployTemplate.convert_with_links(t, fields=fields, sanitize=False)
|
DeployTemplate.convert_with_links(t, fields=fields, sanitize=False)
|
||||||
for t in templates]
|
for t in templates]
|
||||||
collection.next = collection.get_next(limit, **kwargs)
|
collection.next = collection.get_next(limit, fields=fields, **kwargs)
|
||||||
|
|
||||||
for template in collection.deploy_templates:
|
for template in collection.deploy_templates:
|
||||||
template.sanitize(fields)
|
template.sanitize(fields)
|
||||||
|
|
|
@ -1347,7 +1347,8 @@ class NodeCollection(collection.Collection):
|
||||||
collection.nodes = [Node.convert_with_links(n, fields=fields,
|
collection.nodes = [Node.convert_with_links(n, fields=fields,
|
||||||
sanitize=False)
|
sanitize=False)
|
||||||
for n in nodes]
|
for n in nodes]
|
||||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
collection.next = collection.get_next(limit, url=url, fields=fields,
|
||||||
|
**kwargs)
|
||||||
|
|
||||||
for node in collection.nodes:
|
for node in collection.nodes:
|
||||||
node.sanitize(fields)
|
node.sanitize(fields)
|
||||||
|
|
|
@ -306,7 +306,8 @@ class PortCollection(collection.Collection):
|
||||||
|
|
||||||
collection.ports.append(port)
|
collection.ports.append(port)
|
||||||
|
|
||||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
collection.next = collection.get_next(limit, url=url, fields=fields,
|
||||||
|
**kwargs)
|
||||||
|
|
||||||
for item in collection.ports:
|
for item in collection.ports:
|
||||||
item.sanitize(fields=fields)
|
item.sanitize(fields=fields)
|
||||||
|
|
|
@ -233,7 +233,8 @@ class PortgroupCollection(collection.Collection):
|
||||||
collection.portgroups = [Portgroup.convert_with_links(p, fields=fields,
|
collection.portgroups = [Portgroup.convert_with_links(p, fields=fields,
|
||||||
sanitize=False)
|
sanitize=False)
|
||||||
for p in rpc_portgroups]
|
for p in rpc_portgroups]
|
||||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
collection.next = collection.get_next(limit, url=url, fields=fields,
|
||||||
|
**kwargs)
|
||||||
|
|
||||||
for item in collection.portgroups:
|
for item in collection.portgroups:
|
||||||
item.sanitize(fields=fields)
|
item.sanitize(fields=fields)
|
||||||
|
|
|
@ -201,7 +201,8 @@ class VolumeConnectorCollection(collection.Collection):
|
||||||
for p in rpc_connectors]
|
for p in rpc_connectors]
|
||||||
if detail:
|
if detail:
|
||||||
kwargs['detail'] = detail
|
kwargs['detail'] = detail
|
||||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
collection.next = collection.get_next(limit, url=url, fields=fields,
|
||||||
|
**kwargs)
|
||||||
for connector in collection.connectors:
|
for connector in collection.connectors:
|
||||||
connector.sanitize(fields)
|
connector.sanitize(fields)
|
||||||
return collection
|
return collection
|
||||||
|
|
|
@ -217,7 +217,8 @@ class VolumeTargetCollection(collection.Collection):
|
||||||
for p in rpc_targets]
|
for p in rpc_targets]
|
||||||
if detail:
|
if detail:
|
||||||
kwargs['detail'] = detail
|
kwargs['detail'] = detail
|
||||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
collection.next = collection.get_next(limit, url=url, fields=fields,
|
||||||
|
**kwargs)
|
||||||
for target in collection.targets:
|
for target in collection.targets:
|
||||||
target.sanitize(fields)
|
target.sanitize(fields)
|
||||||
return collection
|
return collection
|
||||||
|
|
|
@ -217,6 +217,27 @@ class TestListAllocations(test_api_base.BaseApiTest):
|
||||||
next_marker = data['allocations'][-1]['uuid']
|
next_marker = data['allocations'][-1]['uuid']
|
||||||
self.assertIn(next_marker, data['next'])
|
self.assertIn(next_marker, data['next'])
|
||||||
|
|
||||||
|
def test_collection_links_custom_fields(self):
|
||||||
|
cfg.CONF.set_override('max_limit', 3, 'api')
|
||||||
|
fields = 'uuid,extra'
|
||||||
|
allocations = []
|
||||||
|
for i in range(5):
|
||||||
|
allocation = obj_utils.create_test_allocation(
|
||||||
|
self.context,
|
||||||
|
node_id=self.node.id,
|
||||||
|
uuid=uuidutils.generate_uuid(),
|
||||||
|
name='allocation%s' % i)
|
||||||
|
allocations.append(allocation.uuid)
|
||||||
|
|
||||||
|
data = self.get_json(
|
||||||
|
'/allocations?fields=%s' % fields,
|
||||||
|
headers=self.headers)
|
||||||
|
self.assertEqual(3, len(data['allocations']))
|
||||||
|
|
||||||
|
next_marker = data['allocations'][-1]['uuid']
|
||||||
|
self.assertIn(next_marker, data['next'])
|
||||||
|
self.assertIn('fields', data['next'])
|
||||||
|
|
||||||
def test_get_collection_pagination_no_uuid(self):
|
def test_get_collection_pagination_no_uuid(self):
|
||||||
fields = 'node_uuid'
|
fields = 'node_uuid'
|
||||||
limit = 2
|
limit = 2
|
||||||
|
|
|
@ -230,6 +230,22 @@ class TestListChassis(test_api_base.BaseApiTest):
|
||||||
next_marker = data['chassis'][-1]['uuid']
|
next_marker = data['chassis'][-1]['uuid']
|
||||||
self.assertIn(next_marker, data['next'])
|
self.assertIn(next_marker, data['next'])
|
||||||
|
|
||||||
|
def test_collection_links_custom_fields(self):
|
||||||
|
fields = 'extra,uuid'
|
||||||
|
cfg.CONF.set_override('max_limit', 3, 'api')
|
||||||
|
for i in range(5):
|
||||||
|
obj_utils.create_test_chassis(
|
||||||
|
self.context, uuid=uuidutils.generate_uuid())
|
||||||
|
|
||||||
|
data = self.get_json(
|
||||||
|
'/chassis?fields=%s' % fields,
|
||||||
|
headers={api_base.Version.string: str(api_v1.max_version())})
|
||||||
|
|
||||||
|
self.assertEqual(3, len(data['chassis']))
|
||||||
|
next_marker = data['chassis'][-1]['uuid']
|
||||||
|
self.assertIn(next_marker, data['next'])
|
||||||
|
self.assertIn('fields', data['next'])
|
||||||
|
|
||||||
def test_get_collection_pagination_no_uuid(self):
|
def test_get_collection_pagination_no_uuid(self):
|
||||||
fields = 'extra'
|
fields = 'extra'
|
||||||
limit = 2
|
limit = 2
|
||||||
|
|
|
@ -206,6 +206,24 @@ class TestListConductors(test_api_base.BaseApiTest):
|
||||||
next_marker = data['conductors'][-1]['hostname']
|
next_marker = data['conductors'][-1]['hostname']
|
||||||
self.assertIn(next_marker, data['next'])
|
self.assertIn(next_marker, data['next'])
|
||||||
|
|
||||||
|
def test_collection_links_custom_fields(self):
|
||||||
|
cfg.CONF.set_override('max_limit', 3, 'api')
|
||||||
|
conductors = []
|
||||||
|
fields = 'hostname,alive'
|
||||||
|
for id in range(5):
|
||||||
|
hostname = uuidutils.generate_uuid()
|
||||||
|
conductor = obj_utils.create_test_conductor(self.context,
|
||||||
|
hostname=hostname)
|
||||||
|
conductors.append(conductor.hostname)
|
||||||
|
data = self.get_json(
|
||||||
|
'/conductors?fields=%s' % fields,
|
||||||
|
headers={api_base.Version.string: str(api_v1.max_version())})
|
||||||
|
self.assertEqual(3, len(data['conductors']))
|
||||||
|
|
||||||
|
next_marker = data['conductors'][-1]['hostname']
|
||||||
|
self.assertIn(next_marker, data['next'])
|
||||||
|
self.assertIn('fields', data['next'])
|
||||||
|
|
||||||
def test_sort_key(self):
|
def test_sort_key(self):
|
||||||
conductors = []
|
conductors = []
|
||||||
for id in range(5):
|
for id in range(5):
|
||||||
|
|
|
@ -250,6 +250,23 @@ class TestListDeployTemplates(BaseDeployTemplatesAPITest):
|
||||||
next_marker = data['deploy_templates'][-1]['uuid']
|
next_marker = data['deploy_templates'][-1]['uuid']
|
||||||
self.assertIn(next_marker, data['next'])
|
self.assertIn(next_marker, data['next'])
|
||||||
|
|
||||||
|
def test_collection_links_custom_fields(self):
|
||||||
|
cfg.CONF.set_override('max_limit', 3, 'api')
|
||||||
|
templates = []
|
||||||
|
fields = 'uuid,steps'
|
||||||
|
for i in range(5):
|
||||||
|
template = obj_utils.create_test_deploy_template(
|
||||||
|
self.context,
|
||||||
|
uuid=uuidutils.generate_uuid(),
|
||||||
|
name='CUSTOM_DT%s' % i)
|
||||||
|
templates.append(template.uuid)
|
||||||
|
data = self.get_json('/deploy_templates?fields=%s' % fields,
|
||||||
|
headers=self.headers)
|
||||||
|
self.assertEqual(3, len(data['deploy_templates']))
|
||||||
|
next_marker = data['deploy_templates'][-1]['uuid']
|
||||||
|
self.assertIn(next_marker, data['next'])
|
||||||
|
self.assertIn('fields', data['next'])
|
||||||
|
|
||||||
def test_get_collection_pagination_no_uuid(self):
|
def test_get_collection_pagination_no_uuid(self):
|
||||||
fields = 'name'
|
fields = 'name'
|
||||||
limit = 2
|
limit = 2
|
||||||
|
|
|
@ -907,6 +907,25 @@ class TestListNodes(test_api_base.BaseApiTest):
|
||||||
next_marker = data['nodes'][-1]['uuid']
|
next_marker = data['nodes'][-1]['uuid']
|
||||||
self.assertIn(next_marker, data['next'])
|
self.assertIn(next_marker, data['next'])
|
||||||
|
|
||||||
|
def test_collection_links_custom_fields(self):
|
||||||
|
fields = 'driver_info,uuid'
|
||||||
|
cfg.CONF.set_override('max_limit', 3, 'api')
|
||||||
|
nodes = []
|
||||||
|
for id in range(5):
|
||||||
|
node = obj_utils.create_test_node(self.context,
|
||||||
|
uuid=uuidutils.generate_uuid(),
|
||||||
|
driver_info={'fake': 'value'},
|
||||||
|
properties={'fake': 'bar'})
|
||||||
|
nodes.append(node.uuid)
|
||||||
|
data = self.get_json(
|
||||||
|
'/nodes?fields=%s' % fields,
|
||||||
|
headers={api_base.Version.string: str(api_v1.max_version())})
|
||||||
|
self.assertEqual(3, len(data['nodes']))
|
||||||
|
|
||||||
|
next_marker = data['nodes'][-1]['uuid']
|
||||||
|
self.assertIn(next_marker, data['next'])
|
||||||
|
self.assertIn('fields', data['next'])
|
||||||
|
|
||||||
def test_get_collection_pagination_no_uuid(self):
|
def test_get_collection_pagination_no_uuid(self):
|
||||||
fields = 'name'
|
fields = 'name'
|
||||||
limit = 2
|
limit = 2
|
||||||
|
|
|
@ -641,6 +641,25 @@ class TestListPorts(test_api_base.BaseApiTest):
|
||||||
next_marker = data['ports'][-1]['uuid']
|
next_marker = data['ports'][-1]['uuid']
|
||||||
self.assertIn(next_marker, data['next'])
|
self.assertIn(next_marker, data['next'])
|
||||||
|
|
||||||
|
def test_collection_links_custom_fields(self):
|
||||||
|
fields = 'address,uuid'
|
||||||
|
cfg.CONF.set_override('max_limit', 3, 'api')
|
||||||
|
for i in range(5):
|
||||||
|
obj_utils.create_test_port(
|
||||||
|
self.context,
|
||||||
|
uuid=uuidutils.generate_uuid(),
|
||||||
|
node_id=self.node.id,
|
||||||
|
address='52:54:00:cf:2d:3%s' % i)
|
||||||
|
|
||||||
|
data = self.get_json(
|
||||||
|
'/ports?fields=%s' % fields,
|
||||||
|
headers={api_base.Version.string: str(api_v1.max_version())})
|
||||||
|
|
||||||
|
self.assertEqual(3, len(data['ports']))
|
||||||
|
next_marker = data['ports'][-1]['uuid']
|
||||||
|
self.assertIn(next_marker, data['next'])
|
||||||
|
self.assertIn('fields', data['next'])
|
||||||
|
|
||||||
def test_port_by_address(self):
|
def test_port_by_address(self):
|
||||||
address_template = "aa:bb:cc:dd:ee:f%d"
|
address_template = "aa:bb:cc:dd:ee:f%d"
|
||||||
for id_ in range(3):
|
for id_ in range(3):
|
||||||
|
|
|
@ -327,6 +327,26 @@ class TestListPortgroups(test_api_base.BaseApiTest):
|
||||||
next_marker = data['portgroups'][-1]['uuid']
|
next_marker = data['portgroups'][-1]['uuid']
|
||||||
self.assertIn(next_marker, data['next'])
|
self.assertIn(next_marker, data['next'])
|
||||||
|
|
||||||
|
def test_collection_links_custom_fields(self):
|
||||||
|
fields = 'address,uuid'
|
||||||
|
cfg.CONF.set_override('max_limit', 3, 'api')
|
||||||
|
for i in range(5):
|
||||||
|
obj_utils.create_test_portgroup(
|
||||||
|
self.context,
|
||||||
|
uuid=uuidutils.generate_uuid(),
|
||||||
|
node_id=self.node.id,
|
||||||
|
name='portgroup%s' % i,
|
||||||
|
address='52:54:00:cf:2d:3%s' % i)
|
||||||
|
|
||||||
|
data = self.get_json(
|
||||||
|
'/portgroups?fields=%s' % fields,
|
||||||
|
headers={api_base.Version.string: str(api_v1.max_version())})
|
||||||
|
|
||||||
|
self.assertEqual(3, len(data['portgroups']))
|
||||||
|
next_marker = data['portgroups'][-1]['uuid']
|
||||||
|
self.assertIn(next_marker, data['next'])
|
||||||
|
self.assertIn('fields', data['next'])
|
||||||
|
|
||||||
def test_get_collection_pagination_no_uuid(self):
|
def test_get_collection_pagination_no_uuid(self):
|
||||||
fields = 'address'
|
fields = 'address'
|
||||||
limit = 2
|
limit = 2
|
||||||
|
|
|
@ -273,6 +273,28 @@ class TestListVolumeConnectors(test_api_base.BaseApiTest):
|
||||||
next_marker = data['connectors'][-1]['uuid']
|
next_marker = data['connectors'][-1]['uuid']
|
||||||
self.assertIn(next_marker, data['next'])
|
self.assertIn(next_marker, data['next'])
|
||||||
|
|
||||||
|
def test_collection_links_custom_fields(self):
|
||||||
|
cfg.CONF.set_override('max_limit', 3, 'api')
|
||||||
|
connectors = []
|
||||||
|
fields = 'uuid,extra'
|
||||||
|
for i in range(5):
|
||||||
|
connector = obj_utils.create_test_volume_connector(
|
||||||
|
self.context, node_id=self.node.id,
|
||||||
|
uuid=uuidutils.generate_uuid(),
|
||||||
|
connector_id='test-connector_id-%s' % i)
|
||||||
|
connectors.append(connector.uuid)
|
||||||
|
|
||||||
|
data = self.get_json(
|
||||||
|
'/volume/connectors?fields=%s' % fields,
|
||||||
|
headers=self.headers)
|
||||||
|
|
||||||
|
self.assertEqual(3, len(data['connectors']))
|
||||||
|
self.assertIn('volume/connectors', data['next'])
|
||||||
|
|
||||||
|
next_marker = data['connectors'][-1]['uuid']
|
||||||
|
self.assertIn(next_marker, data['next'])
|
||||||
|
self.assertIn('fields', data['next'])
|
||||||
|
|
||||||
def test_get_collection_pagination_no_uuid(self):
|
def test_get_collection_pagination_no_uuid(self):
|
||||||
fields = 'connector_id'
|
fields = 'connector_id'
|
||||||
limit = 2
|
limit = 2
|
||||||
|
|
|
@ -258,6 +258,24 @@ class TestListVolumeTargets(test_api_base.BaseApiTest):
|
||||||
self.assertIn(next_marker, data['next'])
|
self.assertIn(next_marker, data['next'])
|
||||||
self.assertIn('volume/targets', data['next'])
|
self.assertIn('volume/targets', data['next'])
|
||||||
|
|
||||||
|
def test_collection_links_custom_fields(self):
|
||||||
|
fields = 'uuid,extra'
|
||||||
|
cfg.CONF.set_override('max_limit', 3, 'api')
|
||||||
|
targets = []
|
||||||
|
for id_ in range(5):
|
||||||
|
target = obj_utils.create_test_volume_target(
|
||||||
|
self.context, node_id=self.node.id,
|
||||||
|
uuid=uuidutils.generate_uuid(), boot_index=id_)
|
||||||
|
targets.append(target.uuid)
|
||||||
|
data = self.get_json('/volume/targets?fields=%s' % fields,
|
||||||
|
headers=self.headers)
|
||||||
|
self.assertEqual(3, len(data['targets']))
|
||||||
|
|
||||||
|
next_marker = data['targets'][-1]['uuid']
|
||||||
|
self.assertIn(next_marker, data['next'])
|
||||||
|
self.assertIn('volume/targets', data['next'])
|
||||||
|
self.assertIn('fields', data['next'])
|
||||||
|
|
||||||
def test_get_collection_pagination_no_uuid(self):
|
def test_get_collection_pagination_no_uuid(self):
|
||||||
fields = 'boot_index'
|
fields = 'boot_index'
|
||||||
limit = 2
|
limit = 2
|
||||||
|
|
|
@ -0,0 +1,8 @@
|
||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Fixes issue where the resource list API returned results with requested
|
||||||
|
fields only until the API MAX_LIMIT. After the API MAX_LIMIT is reached the
|
||||||
|
API started ignoring user requested fields. This fix will make sure that
|
||||||
|
the next url generated by the pagination code will include the user
|
||||||
|
requested fields as query parameter.
|
Loading…
Reference in New Issue