openstack
/
heat
Code Issues Proposed changes

Validate property values in nested stacks again 

In ced6f78aa0 we stopped doing validations of
nested stacks at stack creation time, on the assumption that they had been
validated when the parent stack was created. This assumption was incorrect;
for children of the stack being created, the strict_validate global is
always set during validation, so property values of resources will not be
validated until it comes time to create the resource.

Instead, prevent only redundant non-strict validations of stacks at nested
depth 2 and greater. This means that every stack other than the root will
be validated exactly twice - once without validating property values when
the root is created, and again including property validation when the
nested stack itself is created.

Most of the performance benefits should remain; in the case of a large
ResourceGroup using index substitution, we will now have to validate a lot
of nearly-identical resource properties, however we still will not load
into memory and validate a nested stack for each one as we originally did.
Since that happens synchronously, it was likely the main contributor to RPC
timeouts when dealing with large scaling groups. (During the validation at
the creation of the root stack, only a single member of a ResourceGroup is
validated even when index substitution is used. For scaling groups with
identical members, only one member is validated since
3aebdabf2e78ac9e920b9dd8c748c4fad0d723c3.)

This change reverts commit ced6f78aa0.

Change-Id: I97cf789cee75931edef58b78c88f02da204d2a08
Closes-Bug: #1675589
Related-Bug: #1645336
This commit is contained in:
Zane Bitter 2017-03-31 15:57:54 -04:00
parent edae9f0c45
commit ea2673fb9a
6 changed files with 46 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
heat/engine/resources/stack_resource.py
View File

@ -59,7 +59,11 @@ class StackResource(resource.Resource):
def validate(self):
super(StackResource, self).validate()
self.validate_nested_stack()
# Don't redo a non-strict validation of a nested stack during the
# creation of a child stack; only validate a child stack prior to the
# creation of the root stack.
if self.stack.nested_depth == 0 or not self.stack.strict_validate:
self.validate_nested_stack()
def validate_nested_stack(self):
try:

5
heat/engine/service.py
View File

@ -712,7 +712,7 @@ class EngineService(service.ServiceBase):
self.resource_enforcer.enforce_stack(stack)
self._validate_deferred_auth_context(cnxt, stack)
is_root = stack.nested_depth == 0
stack.validate(validate_resources=is_root)
stack.validate()
# For the root stack, log a summary of the TemplateResources loaded
if is_root:
tmpl.env.registry.log_resource_info(prefix=stack_name)
@ -942,8 +942,7 @@ class EngineService(service.ServiceBase):
updated_stack.parameters.set_stack_id(current_stack.identifier())
self._validate_deferred_auth_context(cnxt, updated_stack)
is_root = current_stack.nested_depth == 0
updated_stack.validate(validate_resources=is_root)
updated_stack.validate()
return tmpl, current_stack, updated_stack

62
heat/engine/stack.py
View File

@ -780,8 +780,7 @@ class Stack(collections.Mapping):
return handler and handler(resource_name)
@profiler.trace('Stack.validate', hide_args=False)
def validate(self, ignorable_errors=None, validate_by_deps=True,
validate_resources=True):
def validate(self, ignorable_errors=None, validate_by_deps=True):
"""Validates the stack."""
# TODO(sdake) Should return line number of invalid reference
@ -828,39 +827,36 @@ class Stack(collections.Mapping):
else:
iter_rsc = six.itervalues(resources)
# Validate resources only for top-level stacks. Nested stacks have
# already had their resources validated by their parent.
if validate_resources:
unique_defns = set(res.t for res in six.itervalues(resources))
unique_defn_names = set(defn.name for defn in unique_defns)
unique_defns = set(res.t for res in six.itervalues(resources))
unique_defn_names = set(defn.name for defn in unique_defns)
for res in iter_rsc:
# Don't validate identical definitions multiple times
if res.name not in unique_defn_names:
continue
try:
if self.resource_validate:
if res.external_id is not None:
res.validate_external()
continue
result = res.validate()
elif res.external_id is None:
result = res.validate_template()
except exception.HeatException as ex:
LOG.debug('%s', ex)
if ignorable_errors and ex.error_code in ignorable_errors:
result = None
else:
raise
except AssertionError:
for res in iter_rsc:
# Don't validate identical definitions multiple times
if res.name not in unique_defn_names:
continue
try:
if self.resource_validate:
if res.external_id is not None:
res.validate_external()
continue
result = res.validate()
elif res.external_id is None:
result = res.validate_template()
except exception.HeatException as ex:
LOG.debug('%s', ex)
if ignorable_errors and ex.error_code in ignorable_errors:
result = None
else:
raise
except Exception as ex:
LOG.info("Exception in stack validation",
exc_info=True)
raise exception.StackValidationFailed(
message=encodeutils.safe_decode(six.text_type(ex)))
if result:
raise exception.StackValidationFailed(message=result)
except AssertionError:
raise
except Exception as ex:
LOG.info("Exception in stack validation",
exc_info=True)
raise exception.StackValidationFailed(
message=encodeutils.safe_decode(six.text_type(ex)))
if result:
raise exception.StackValidationFailed(message=result)
for op_name, output in six.iteritems(self.outputs):
try:

4
heat/tests/engine/service/test_stack_create.py
View File

@ -73,7 +73,7 @@ class StackCreateTest(common.HeatTestCase):
if environment_files:
mock_merge.assert_called_once_with(environment_files, None,
params, mock.ANY)
mock_validate.assert_called_once_with(validate_resources=True)
mock_validate.assert_called_once_with()
def test_stack_create(self):
stack_name = 'service_create_test_stack'
@ -304,7 +304,7 @@ class StackCreateTest(common.HeatTestCase):
convergence=convergence_engine,
parent_resource=None)
mock_validate.assert_called_once_with(validate_resources=False)
mock_validate.assert_called_once_with()
def test_stack_validate(self):
stack_name = 'stack_create_test_validate'

12
heat/tests/engine/service/test_stack_update.py
View File

@ -99,7 +99,7 @@ class ServiceStackUpdateTest(common.HeatTestCase):
user_creds_id=u'1',
username='test_username')
mock_load.assert_called_once_with(self.ctx, stack=s)
mock_validate.assert_called_once_with(validate_resources=True)
mock_validate.assert_called_once_with()
def test_stack_update_with_environment_files(self):
# Setup
@ -188,7 +188,7 @@ class ServiceStackUpdateTest(common.HeatTestCase):
user_creds_id=u'1',
username='test_username')
mock_load.assert_called_once_with(self.ctx, stack=s)
mock_validate.assert_called_once_with(validate_resources=False)
mock_validate.assert_called_once_with()
def test_stack_update_existing_parameters(self):
# Use a template with existing parameters, then update the stack
@ -462,7 +462,7 @@ resources:
self.assertIsInstance(result, dict)
self.assertTrue(result['stack_id'])
mock_validate.assert_called_once_with(validate_resources=True)
mock_validate.assert_called_once_with()
mock_tmpl.assert_called_once_with(template, files=None)
mock_env.assert_called_once_with(params)
mock_load.assert_called_once_with(self.ctx, stack=s)
@ -610,7 +610,7 @@ resources:
timeout_mins=60, user_creds_id=u'1',
username='test_username')
mock_load.assert_called_once_with(self.ctx, stack=s)
mock_validate.assert_called_once_with(validate_resources=True)
mock_validate.assert_called_once_with()
def test_stack_update_stack_id_equal(self):
stack_name = 'test_stack_update_stack_id_equal'
@ -726,7 +726,7 @@ resources:
timeout_mins=60, user_creds_id=u'1',
username='test_username')
mock_load.assert_called_once_with(self.ctx, stack=s)
mock_validate.assert_called_once_with(validate_resources=True)
mock_validate.assert_called_once_with()
def test_stack_update_nonexist(self):
stack_name = 'service_update_nonexist_test_stack'
@ -1043,7 +1043,7 @@ resources:
mock_load.assert_called_once_with(self.ctx, stack=s)
mock_tmpl.assert_called_once_with(new_template, files=None)
mock_env.assert_called_once_with(params)
mock_validate.assert_called_once_with(validate_resources=True)
mock_validate.assert_called_once_with()
if environment_files:
mock_merge.assert_called_once_with(environment_files, None,

4
heat/tests/test_engine_service.py
View File

@ -246,7 +246,7 @@ class StackConvergenceServiceCreateUpdateTest(common.HeatTestCase):
convergence=True).AndReturn(stack)
self.m.StubOutWithMock(stack, 'validate')
stack.validate(validate_resources=True).AndReturn(None)
stack.validate().AndReturn(None)
self.m.ReplayAll()
api_args = {'timeout_mins': 60, 'disable_rollback': False}
@ -298,7 +298,7 @@ class StackConvergenceServiceCreateUpdateTest(common.HeatTestCase):
current_deps=old_stack.current_deps).AndReturn(stack)
self.m.StubOutWithMock(stack, 'validate')
stack.validate(validate_resources=True).AndReturn(None)
stack.validate().AndReturn(None)
self.m.ReplayAll()