Avoid loading nested stack in some grouputils functions

We want to avoid loading a nested stack in memory wherever possible, since
that is known to cause memory high-water-mark issues. The grouputils
functions are among the worst offenders at doing this. Some of the data
that they return is easily obtained from an RPC call to
list_stack_resources, so swap out the implementations using nested().

Rather than simply add more utility functions, a GroupInspector class is
created that can cache the data returned. In future this will allow groups
that need to access multiple functions from the grouputils to do so without
making multiple RPC calls. (Previously, the data was cached in the group's
nested Stack.)

Change-Id: Icd35d91bce30ee36d9592b0516767ef273a9f34d
Partial-Bug: #1731349
This commit is contained in:
Zane Bitter 2018-01-08 17:23:12 -05:00
parent e5707618f3
commit ce120bfda8
4 changed files with 328 additions and 53 deletions

View File

@ -15,20 +15,102 @@ import six
from heat.common import exception
from heat.common.i18n import _
from heat.engine import status
from heat.engine import template
from heat.rpc import api as rpc_api
class GroupInspector(object):
"""A class for returning data about a scaling group.
All data is fetched over RPC, and the group's stack is never loaded into
memory locally. Data is cached so it will be fetched only once. To
refresh the data, create a new GroupInspector.
"""
def __init__(self, context, rpc_client, group_identity):
"""Initialise with a context, rpc_client, and stack identifier."""
self._context = context
self._rpc_client = rpc_client
self._identity = group_identity
self._member_data = None
self._template_data = None
@classmethod
def from_parent_resource(cls, parent_resource):
"""Create a GroupInspector from a parent resource.
This is a convenience method to instantiate a GroupInspector from a
Heat StackResource object.
"""
return cls(parent_resource.context, parent_resource.rpc_client(),
parent_resource.nested_identifier())
def _get_member_data(self):
if self._identity is None:
return []
if self._member_data is None:
rsrcs = self._rpc_client.list_stack_resources(self._context,
dict(self._identity))
def sort_key(r):
return (r[rpc_api.RES_STATUS] != status.ResourceStatus.FAILED,
r[rpc_api.RES_CREATION_TIME],
r[rpc_api.RES_NAME])
self._member_data = sorted(rsrcs, key=sort_key)
return self._member_data
def _members(self, include_failed):
return (r for r in self._get_member_data()
if (include_failed or
r[rpc_api.RES_STATUS] != status.ResourceStatus.FAILED))
def size(self, include_failed):
"""Return the size of the group.
If include_failed is False, only members not in a FAILED state will
be counted.
"""
return sum(1 for m in self._members(include_failed))
def member_names(self, include_failed):
"""Return an iterator over the names of the group members
If include_failed is False, only members not in a FAILED state will
be included.
"""
return (m[rpc_api.RES_NAME] for m in self._members(include_failed))
def _get_template_data(self):
if self._identity is None:
return None
if self._template_data is None:
self._template_data = self._rpc_client.get_template(self._context,
self._identity)
return self._template_data
def template(self):
"""Return a Template object representing the group's current template.
Note that this does *not* include any environment data.
"""
data = self._get_template_data()
if data is None:
return None
return template.Template(data)
def get_size(group, include_failed=False):
"""Get number of member resources managed by the specified group.
The size exclude failed members default, set include_failed=True
to get total size.
The size excludes failed members by default; set include_failed=True
to get the total size.
"""
if group.nested():
resources = [r for r in six.itervalues(group.nested())
if include_failed or r.status != r.FAILED]
return len(resources)
else:
return 0
return GroupInspector.from_parent_resource(group).size(include_failed)
def get_members(group, include_failed=False):
@ -67,7 +149,8 @@ def get_member_names(group):
Failed resources will be ignored.
"""
return [r.name for r in get_members(group)]
inspector = GroupInspector.from_parent_resource(group)
return list(inspector.member_names(include_failed=False))
def get_resource(stack, resource_name, use_indices, key=None):
@ -109,9 +192,14 @@ def get_nested_attrs(stack, key, use_indices, *path):
def get_member_definitions(group, include_failed=False):
"""Get member definitions in (name, ResourceDefinition) pair for group.
The List is sorted first by created_time then by name.
If include_failed is set, failed members will be put first in the
List sorted by created_time then by name.
The List is sorted first by created_time then by name.
If include_failed is set, failed members will be put first in the
List sorted by created_time then by name.
"""
return [(resource.name, resource.t)
for resource in get_members(group, include_failed)]
inspector = GroupInspector.from_parent_resource(group)
template = inspector.template()
if template is None:
return []
definitions = template.resource_definitions(None)
return [(name, definitions[name])
for name in inspector.member_names(include_failed=include_failed)]

View File

@ -411,22 +411,21 @@ class ResizeWithFailedInstancesTest(InstanceGroupWithNestedStack):
def setUp(self):
super(ResizeWithFailedInstancesTest, self).setUp()
self.group._nested = self.get_fake_nested_stack(4)
self.nested = self.group.nested()
self.group.nested = mock.Mock(return_value=self.nested)
nested = self.get_fake_nested_stack(4)
def set_failed_instance(self, instance):
for r in six.itervalues(self.group.nested()):
if r.name == instance:
r.status = "FAILED"
inspector = mock.Mock(spec=grouputils.GroupInspector)
self.patchobject(grouputils.GroupInspector, 'from_parent_resource',
return_value=inspector)
inspector.member_names.return_value = (self.failed +
sorted(self.content -
set(self.failed)))
inspector.template.return_value = nested.defn._template
def test_resize(self):
for inst in self.failed:
self.set_failed_instance(inst)
self.group.resize(self.size)
tmpl = self.group.update_with_template.call_args[0][0]
resources = tmpl.resource_definitions(self.group.nested())
self.assertEqual(set(resources.keys()), self.content)
resources = tmpl.resource_definitions(None)
self.assertEqual(self.content, set(resources.keys()))
class TestGetBatches(common.HeatTestCase):

View File

@ -138,7 +138,11 @@ template_server = {
class ResourceGroupTest(common.HeatTestCase):
def setUp(self):
common.HeatTestCase.setUp(self)
super(ResourceGroupTest, self).setUp()
self.inspector = mock.Mock(spec=grouputils.GroupInspector)
self.patchobject(grouputils.GroupInspector, 'from_parent_resource',
return_value=self.inspector)
def test_assemble_nested(self):
"""Tests nested stack creation based on props.
@ -259,7 +263,9 @@ class ResourceGroupTest(common.HeatTestCase):
stack = utils.parse_stack(template)
snip = stack.t.resource_definitions(stack)['group1']
resg = resource_group.ResourceGroup('test', snip, stack)
resg._nested = get_fake_nested_stack(['0', '1'])
nested = get_fake_nested_stack(['0', '1'])
self.inspector.template.return_value = nested.defn._template
self.inspector.member_names.return_value = ['0', '1']
resg.build_resource_definition = mock.Mock(return_value=resource_def)
self.assertEqual(expect, resg._assemble_for_rolling_update(2, 1).t)
@ -290,7 +296,9 @@ class ResourceGroupTest(common.HeatTestCase):
stack = utils.parse_stack(template)
snip = stack.t.resource_definitions(stack)['group1']
resg = resource_group.ResourceGroup('test', snip, stack)
resg._nested = get_fake_nested_stack(['0', '1'])
nested = get_fake_nested_stack(['0', '1'])
self.inspector.template.return_value = nested.defn._template
self.inspector.member_names.return_value = ['0', '1']
resg.build_resource_definition = mock.Mock(return_value=resource_def)
self.assertEqual(expect, resg._assemble_for_rolling_update(2, 0).t)
@ -320,9 +328,9 @@ class ResourceGroupTest(common.HeatTestCase):
stack = utils.parse_stack(template)
snip = stack.t.resource_definitions(stack)['group1']
resg = resource_group.ResourceGroup('test', snip, stack)
resg._nested = get_fake_nested_stack(['0', '1'])
res0 = resg._nested['0']
res0.status = res0.FAILED
nested = get_fake_nested_stack(['0', '1'])
self.inspector.template.return_value = nested.defn._template
self.inspector.member_names.return_value = ['1']
resg.build_resource_definition = mock.Mock(return_value=resource_def)
self.assertEqual(expect, resg._assemble_for_rolling_update(2, 1).t)
@ -564,6 +572,7 @@ class ResourceGroupTest(common.HeatTestCase):
self.assertEqual(1, resgrp.create_with_template.call_count)
def test_handle_create_with_batching(self):
self.inspector.member_names.return_value = []
stack = utils.parse_stack(tmpl_with_default_updt_policy())
defn = stack.t.resource_definitions(stack)['group1']
props = stack.t.t['resources']['group1']['properties'].copy()
@ -576,6 +585,7 @@ class ResourceGroupTest(common.HeatTestCase):
self.assertEqual(4, len(checkers))
def test_handle_create_with_batching_zero_count(self):
self.inspector.member_names.return_value = []
stack = utils.parse_stack(tmpl_with_default_updt_policy())
defn = stack.t.resource_definitions(stack)['group1']
props = stack.t.t['resources']['group1']['properties'].copy()
@ -983,6 +993,11 @@ class ReplaceTest(common.HeatTestCase):
self.group.update_with_template = mock.Mock()
self.group.check_update_complete = mock.Mock()
inspector = mock.Mock(spec=grouputils.GroupInspector)
self.patchobject(grouputils.GroupInspector, 'from_parent_resource',
return_value=inspector)
inspector.member_names.return_value = self.existing
def test_rolling_updates(self):
self.group._nested = get_fake_nested_stack(self.existing)
self.group.get_size = mock.Mock(return_value=self.count)
@ -990,8 +1005,7 @@ class ReplaceTest(common.HeatTestCase):
return_value=set(self.black_listed))
tasks = self.group._replace(self.min_in_service, self.batch_size,
self.pause_sec)
self.assertEqual(self.tasks,
len(tasks))
self.assertEqual(self.tasks, len(tasks))
def tmpl_with_bad_updt_policy():
@ -1201,10 +1215,14 @@ class TestUtils(common.HeatTestCase):
]
def test_count_black_listed(self):
inspector = mock.Mock(spec=grouputils.GroupInspector)
self.patchobject(grouputils.GroupInspector, 'from_parent_resource',
return_value=inspector)
inspector.member_names.return_value = self.existing
stack = utils.parse_stack(template2)
snip = stack.t.resource_definitions(stack)['group1']
resgrp = resource_group.ResourceGroup('test', snip, stack)
resgrp._nested = get_fake_nested_stack(self.existing)
resgrp._name_blacklist = mock.Mock(return_value=set(self.black_listed))
rcount = resgrp._count_black_listed()
self.assertEqual(self.count, rcount)

View File

@ -15,8 +15,9 @@ import mock
import six
from heat.common import grouputils
from heat.common import identifier
from heat.common import template_format
from heat.engine import rsrc_defn
from heat.rpc import client as rpc_client
from heat.tests import common
from heat.tests import utils
@ -34,7 +35,8 @@ class GroupUtilsTest(common.HeatTestCase):
def test_non_nested_resource(self):
group = mock.Mock()
self.patchobject(group, 'nested', return_value=None)
group.nested_identifier.return_value = None
group.nested.return_value = None
self.assertEqual(0, grouputils.get_size(group))
self.assertEqual([], grouputils.get_members(group))
@ -45,9 +47,7 @@ class GroupUtilsTest(common.HeatTestCase):
group = mock.Mock()
t = template_format.parse(nested_stack)
stack = utils.parse_stack(t)
# group size
self.patchobject(group, 'nested', return_value=stack)
self.assertEqual(2, grouputils.get_size(group))
group.nested.return_value = stack
# member list (sorted)
members = [r for r in six.itervalues(stack)]
@ -61,18 +61,6 @@ class GroupUtilsTest(common.HeatTestCase):
partial_ids = grouputils.get_member_refids(group, exclude=['ID-r1'])
self.assertEqual(['ID-r0'], partial_ids)
# names
names = grouputils.get_member_names(group)
self.assertEqual(['r0', 'r1'], names)
# defn snippets as list
expected = rsrc_defn.ResourceDefinition(
None,
"OverwrittenFnGetRefIdType")
member_defs = grouputils.get_member_definitions(group)
self.assertEqual([(x, expected) for x in names], member_defs)
def test_group_with_failed_members(self):
group = mock.Mock()
t = template_format.parse(nested_stack)
@ -84,7 +72,189 @@ class GroupUtilsTest(common.HeatTestCase):
rsrc_err.status = rsrc_err.FAILED
rsrc_ok = stack.resources['r1']
self.assertEqual(1, grouputils.get_size(group))
self.assertEqual([rsrc_ok], grouputils.get_members(group))
self.assertEqual(['ID-r1'], grouputils.get_member_refids(group))
self.assertEqual(['r1'], grouputils.get_member_names(group))
class GroupInspectorTest(common.HeatTestCase):
resources = [
{
'updated_time': '2018-01-01T12:00',
'creation_time': '2018-01-01T02:00',
'resource_name': 'A',
'physical_resource_id': 'a',
'resource_action': 'UPDATE',
'resource_status': 'COMPLETE',
'resource_status_reason': 'resource changed',
'resource_type': 'OS::Heat::Test',
'resource_id': 'aaaaaaaa',
'stack_identity': 'bar',
'stack_name': 'nested_test',
'required_by': [],
'parent_resource': 'stack_resource',
},
{
'updated_time': '2018-01-01T10:00',
'creation_time': '2018-01-01T03:00',
'resource_name': 'E',
'physical_resource_id': 'e',
'resource_action': 'UPDATE',
'resource_status': 'FAILED',
'resource_status_reason': 'reasons',
'resource_type': 'OS::Heat::Test',
'resource_id': 'eeeeeeee',
'stack_identity': 'bar',
'stack_name': 'nested_test',
'required_by': [],
'parent_resource': 'stack_resource',
},
{
'updated_time': '2018-01-01T11:00',
'creation_time': '2018-01-01T03:00',
'resource_name': 'B',
'physical_resource_id': 'b',
'resource_action': 'UPDATE',
'resource_status': 'FAILED',
'resource_status_reason': 'reasons',
'resource_type': 'OS::Heat::Test',
'resource_id': 'bbbbbbbb',
'stack_identity': 'bar',
'stack_name': 'nested_test',
'required_by': [],
'parent_resource': 'stack_resource',
},
{
'updated_time': '2018-01-01T13:00',
'creation_time': '2018-01-01T01:00',
'resource_name': 'C',
'physical_resource_id': 'c',
'resource_action': 'UPDATE',
'resource_status': 'COMPLETE',
'resource_status_reason': 'resource changed',
'resource_type': 'OS::Heat::Test',
'resource_id': 'cccccccc',
'stack_identity': 'bar',
'stack_name': 'nested_test',
'required_by': [],
'parent_resource': 'stack_resource',
},
{
'updated_time': '2018-01-01T04:00',
'creation_time': '2018-01-01T04:00',
'resource_name': 'F',
'physical_resource_id': 'f',
'resource_action': 'CREATE',
'resource_status': 'COMPLETE',
'resource_status_reason': 'resource changed',
'resource_type': 'OS::Heat::Test',
'resource_id': 'ffffffff',
'stack_identity': 'bar',
'stack_name': 'nested_test',
'required_by': [],
'parent_resource': 'stack_resource',
},
{
'updated_time': '2018-01-01T04:00',
'creation_time': '2018-01-01T04:00',
'resource_name': 'D',
'physical_resource_id': 'd',
'resource_action': 'CREATE',
'resource_status': 'COMPLETE',
'resource_status_reason': 'resource changed',
'resource_type': 'OS::Heat::Test',
'resource_id': 'dddddddd',
'stack_identity': 'bar',
'stack_name': 'nested_test',
'required_by': [],
'parent_resource': 'stack_resource',
},
]
template = {
'heat_template_version': 'newton',
'resources': {
'A': {
'type': 'OS::Heat::TestResource',
},
},
}
def setUp(self):
super(GroupInspectorTest, self).setUp()
self.ctx = mock.Mock()
self.rpc_client = mock.Mock(spec=rpc_client.EngineClient)
self.identity = identifier.HeatIdentifier('foo', 'nested_test', 'bar')
self.list_rsrcs = self.rpc_client.list_stack_resources
self.get_tmpl = self.rpc_client.get_template
self.insp = grouputils.GroupInspector(self.ctx, self.rpc_client,
self.identity)
def test_no_identity(self):
self.insp = grouputils.GroupInspector(self.ctx, self.rpc_client, None)
self.assertEqual(0, self.insp.size(include_failed=True))
self.assertEqual([], list(self.insp.member_names(include_failed=True)))
self.assertIsNone(self.insp.template())
self.list_rsrcs.assert_not_called()
self.get_tmpl.assert_not_called()
def test_size_include_failed(self):
self.list_rsrcs.return_value = self.resources
self.assertEqual(6, self.insp.size(include_failed=True))
self.list_rsrcs.assert_called_once_with(self.ctx, dict(self.identity))
def test_size_exclude_failed(self):
self.list_rsrcs.return_value = self.resources
self.assertEqual(4, self.insp.size(include_failed=False))
self.list_rsrcs.assert_called_once_with(self.ctx, dict(self.identity))
def test_member_names_include_failed(self):
self.list_rsrcs.return_value = self.resources
self.assertEqual(['B', 'E', 'C', 'A', 'D', 'F'],
list(self.insp.member_names(include_failed=True)))
self.list_rsrcs.assert_called_once_with(self.ctx, dict(self.identity))
def test_member_names_exclude_failed(self):
self.list_rsrcs.return_value = self.resources
self.assertEqual(['C', 'A', 'D', 'F'],
list(self.insp.member_names(include_failed=False)))
self.list_rsrcs.assert_called_once_with(self.ctx, dict(self.identity))
def test_list_rsrc_caching(self):
self.list_rsrcs.return_value = self.resources
self.insp.size(include_failed=False)
list(self.insp.member_names(include_failed=True))
self.insp.size(include_failed=True)
list(self.insp.member_names(include_failed=False))
self.list_rsrcs.assert_called_once_with(self.ctx, dict(self.identity))
self.get_tmpl.assert_not_called()
def test_get_template(self):
self.get_tmpl.return_value = self.template
tmpl = self.insp.template()
self.assertEqual(self.template, tmpl.t)
self.get_tmpl.assert_called_once_with(self.ctx, dict(self.identity))
def test_get_tmpl_caching(self):
self.get_tmpl.return_value = self.template
self.insp.template()
self.insp.template()
self.get_tmpl.assert_called_once_with(self.ctx, dict(self.identity))
self.list_rsrcs.assert_not_called()