openstack
/
heat
Code Issues Proposed changes

Don't calculate attributes for metadata request 

The RPC API call describe_stack_resource is used by two separate ReST API
calls and one CFN API call: Resource.show and Resource.metadata in heat-api
and StackController.describe_stack_resource. The metadata call in
particular throws away all of the results of the describe_stack_resource
output except for the metadata itself. Either this call or the cfn one
(depending on configuration) is also used very, very often: it's the one
that every instance running os-collect-config polls for information about
software deployments (so, to a first approximation, every server in every
stack will be calling this once every 30s).

Since blueprint detailed-resource-show merged in
6d8a5cb35c, every call to the RPC
describe_stack_resource API has resulted in Heat fetching the attribute
values for every single attribute in the resource from its corresponding
OpenStack API (including those calls originating from a request for just
the metadata, or from the cfn API which similarly discards the attribute
values).

This is insanely inefficient (we're making OpenStack API calls to fetch
data that we don't even return to the user). It's quite possibly a
substantial part of the reason why we've had so much difficulty scaling up
Heat to deal with large numbers of servers polling it for config. And it
tends to leave a lot of annoying warnings in the log messages, not just of
Heat but also the other services it is polling, since polling the metadata
generally happens using the heat_stack_user's credentials, not those of the
actual owner of the resource - it can't actually find the resource and
returns a 404.

The fix is to change the default "with_attr" value in the RPC client to
False, and only pass None (the previous default) or a list of extra
attributes to include in the specific case where the ReST API will actually
return the attribute values to the user and not just discard them. Since
the server previously treated any falsey value for the parameter (e.g.
None, False or []) as equivalent, and since nothing was previously passing
False, this change is safe to backport to stable branches. The api and
engine can be updated in any order without change in behaviour until a new
api is talking to a new server (at which point the bug is fixed).

This change also updates the ReST API unit tests to accurately reflect the
data returned from the engine.

Change-Id: Ifffeaa552d3205ca06a79adda09d35a77099a2bf
Closes-Bug: #1507568
This commit is contained in:
Zane Bitter 2016-01-22 12:10:40 -05:00
parent bc4e5ffd15
commit f0fa7312a0
6 changed files with 20 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
heat/api/openstack/v1/resources.py
View File

@ -116,6 +116,8 @@ class ResourceController(object):
whitelist = {'with_attr': 'multi'}
params = util.get_allowed_params(req.params, whitelist)
if 'with_attr' not in params:
params['with_attr'] = None
res = self.rpc_client.describe_stack_resource(req.context,
identity,
resource_name,

5
heat/engine/api.py
View File

@ -325,8 +325,9 @@ def format_stack_resource(resource, detail=True, with_props=False,
if detail:
res[rpc_api.RES_DESCRIPTION] = resource.t.description
res[rpc_api.RES_METADATA] = resource.metadata_get()
res[rpc_api.RES_SCHEMA_ATTRIBUTES] = format_resource_attributes(
resource, with_attr)
if with_attr is not False:
res[rpc_api.RES_SCHEMA_ATTRIBUTES] = format_resource_attributes(
resource, with_attr)
if with_props:
res[rpc_api.RES_SCHEMA_PROPERTIES] = format_resource_properties(

2
heat/rpc/client.py
View File

@ -432,7 +432,7 @@ class EngineClient(object):
sort_dir=sort_dir))
def describe_stack_resource(self, ctxt, stack_identity, resource_name,
with_attr=None):
with_attr=False):
"""Get detailed resource information about a particular resource.
:param ctxt: RPC context.

4
heat/tests/api/cfn/test_api_cfn_v1.py
View File

@ -1326,7 +1326,7 @@ class CfnStackControllerTest(common.HeatTestCase):
args = {
'stack_identity': identity,
'resource_name': dummy_req.params.get('LogicalResourceId'),
'with_attr': None,
'with_attr': False,
}
rpc_client.EngineClient.call(
dummy_req.context, ('describe_stack_resource', args), version='1.2'
@ -1391,7 +1391,7 @@ class CfnStackControllerTest(common.HeatTestCase):
args = {
'stack_identity': identity,
'resource_name': dummy_req.params.get('LogicalResourceId'),
'with_attr': None,
'with_attr': False,
}
rpc_client.EngineClient.call(
dummy_req.context, ('describe_stack_resource', args), version='1.2'

9
heat/tests/api/openstack_v1/test_resources.py
View File

@ -291,6 +291,7 @@ class ResourceControllerTest(tools.ControllerTest, common.HeatTestCase):
u'physical_resource_id':
u'a3455d8c-9f88-404d-a85b-5315293e67de',
u'resource_type': u'AWS::EC2::Instance',
u'attributes': {u'foo': 'bar'},
u'metadata': {u'ensureRunning': u'true'}
}
self.m.StubOutWithMock(rpc_client.EngineClient, 'call')
@ -323,6 +324,7 @@ class ResourceControllerTest(tools.ControllerTest, common.HeatTestCase):
u'physical_resource_id':
u'a3455d8c-9f88-404d-a85b-5315293e67de',
u'resource_type': u'AWS::EC2::Instance',
u'attributes': {u'foo': 'bar'},
}
}
@ -354,6 +356,7 @@ class ResourceControllerTest(tools.ControllerTest, common.HeatTestCase):
u'physical_resource_id':
u'a3455d8c-9f88-404d-a85b-5315293e67de',
u'resource_type': u'AWS::EC2::Instance',
u'attributes': {u'foo': 'bar'},
u'metadata': {u'ensureRunning': u'true'},
u'nested_stack_id': dict(nested_stack_identity)
}
@ -572,7 +575,7 @@ class ResourceControllerTest(tools.ControllerTest, common.HeatTestCase):
req.context,
('describe_stack_resource',
{'stack_identity': stack_identity, 'resource_name': res_name,
'with_attr': None}),
'with_attr': False}),
version='1.2'
).AndReturn(engine_resp)
self.m.ReplayAll()
@ -603,7 +606,7 @@ class ResourceControllerTest(tools.ControllerTest, common.HeatTestCase):
req.context,
('describe_stack_resource',
{'stack_identity': stack_identity, 'resource_name': res_name,
'with_attr': None}),
'with_attr': False}),
version='1.2'
).AndRaise(tools.to_remote_error(error))
self.m.ReplayAll()
@ -636,7 +639,7 @@ class ResourceControllerTest(tools.ControllerTest, common.HeatTestCase):
req.context,
('describe_stack_resource',
{'stack_identity': stack_identity, 'resource_name': res_name,
'with_attr': None}),
'with_attr': False}),
version='1.2'
).AndRaise(tools.to_remote_error(error))
self.m.ReplayAll()

6
heat/tests/test_engine_api_utils.py
View File

@ -98,6 +98,12 @@ class FormatTest(common.HeatTestCase):
formatted[rpc_api.RES_UPDATED_TIME])
self.assertEqual(res.INIT, formatted[rpc_api.RES_ACTION])
def test_format_stack_resource_no_attrs(self):
res = self.stack['generic1']
formatted = api.format_stack_resource(res, True, with_attr=False)
self.assertNotIn(rpc_api.RES_SCHEMA_ATTRIBUTES, formatted)
self.assertIn(rpc_api.RES_METADATA, formatted)
def test_format_stack_resource_has_been_deleted(self):
# assume the stack and resource have been deleted,
# to test the resource's action inherit from stack