From 165fb8ace4678627fc5347df131f03079a2f646c Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Mon, 24 Jul 2017 17:07:48 -0400 Subject: [PATCH] Only check service availability during validation There's no need to check it every time we create a Resource object. Change-Id: I92082d64e6060c2d8d29d866fdabeea2846253e9 Closes-Bug: #1706197 --- heat/engine/resource.py | 15 ++++-------- heat/engine/service.py | 10 ++------ heat/engine/stack.py | 16 ++++--------- heat/tests/generic_resource.py | 4 ++++ .../openstack/neutron/lbaas/test_l7policy.py | 3 +++ .../openstack/neutron/lbaas/test_l7rule.py | 3 +++ .../openstack/neutron/lbaas/test_listener.py | 3 +++ .../openstack/neutron/lbaas/test_pool.py | 3 +++ .../neutron/test_neutron_rbac_policy.py | 12 ++++++++++ .../test_neutron_security_group_rule.py | 8 +++++++ heat/tests/test_engine_service.py | 3 +-- heat/tests/test_metadata_refresh.py | 5 +--- heat/tests/test_resource.py | 24 +++++++++---------- heat/tests/test_stack.py | 3 +-- 14 files changed, 61 insertions(+), 51 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 6f255becad..2174ebc7ca 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -171,12 +171,6 @@ class Resource(status.ResourceStatus): assert issubclass(ResourceClass, Resource) - if not stack.service_check_defer: - ResourceClass._validate_service_availability( - stack.context, - definition.resource_type - ) - return super(Resource, cls).__new__(ResourceClass) @classmethod @@ -1743,11 +1737,10 @@ class Resource(status.ResourceStatus): in an overridden validate() such as accessing properties may not work. """ - if self.stack.service_check_defer: - self._validate_service_availability( - self.stack.context, - self.t.resource_type - ) + self._validate_service_availability( + self.stack.context, + self.t.resource_type + ) path = '.'.join([self.stack.t.RESOURCES, self.name]) function.validate(self.t, path) self.validate_deletion_policy(self.t.deletion_policy()) diff --git a/heat/engine/service.py b/heat/engine/service.py index a0f9c18583..64eec4bb6f 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1230,7 +1230,6 @@ class EngineService(service.ServiceBase): msg = _("No Template provided.") return webob.exc.HTTPBadRequest(explanation=msg) - service_check_defer = False if ignorable_errors: invalid_codes = (set(ignorable_errors) - set(exception.ERROR_CODE_MAP.keys())) @@ -1239,8 +1238,6 @@ class EngineService(service.ServiceBase): list(invalid_codes)) return webob.exc.HTTPBadRequest(explanation=msg) - service_check_defer = True - tmpl = templatem.Template(template, files=files) env_util.merge_environments(environment_files, files, params, tmpl.all_param_schemata(files)) @@ -1252,8 +1249,7 @@ class EngineService(service.ServiceBase): stack_name = 'dummy' stack = parser.Stack(cnxt, stack_name, tmpl, - strict_validate=False, - service_check_defer=service_check_defer) + strict_validate=False) try: stack.validate(ignorable_errors=ignorable_errors, validate_res_tmpl_only=True) @@ -2391,7 +2387,6 @@ class EngineService(service.ServiceBase): try: for st in stacks: if not st.convergence: - st.service_check_defer = True st.migrate_to_convergence() sess.commit() except Exception: @@ -2470,8 +2465,7 @@ class EngineService(service.ServiceBase): lock.release() continue - stk = parser.Stack.load(cnxt, stack=s, - service_check_defer=True) + stk = parser.Stack.load(cnxt, stack=s) LOG.info('Engine %(engine)s went down when stack ' '%(stack_id)s was in action %(action)s', {'engine': engine_id, 'action': stk.action, diff --git a/heat/engine/stack.py b/heat/engine/stack.py index d28617adba..89cd46abc3 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -121,7 +121,7 @@ class Stack(collections.Mapping): nested_depth=0, strict_validate=True, convergence=False, current_traversal=None, tags=None, prev_raw_template_id=None, current_deps=None, cache_data=None, - service_check_defer=False, deleted_time=None): + deleted_time=None): """Initialise the Stack. @@ -188,12 +188,6 @@ class Stack(collections.Mapping): # for not-yet-created resources (which return None) self.strict_validate = strict_validate - # service_check_defer can be used to defer the validation of service - # availability for a given resource, which helps to create the resource - # dependency tree completely when respective service is not available, - # especially during template_validate - self.service_check_defer = service_check_defer - self.in_convergence_check = cache_data is not None if use_stored_context: @@ -520,7 +514,7 @@ class Stack(collections.Mapping): @classmethod def load(cls, context, stack_id=None, stack=None, show_deleted=True, use_stored_context=False, force_reload=False, cache_data=None, - service_check_defer=False, load_template=True): + load_template=True): """Retrieve a Stack from the database.""" if stack is None: stack = stack_object.Stack.get_by_id( @@ -537,7 +531,6 @@ class Stack(collections.Mapping): return cls._from_db(context, stack, use_stored_context=use_stored_context, cache_data=cache_data, - service_check_defer=service_check_defer, load_template=load_template) @classmethod @@ -572,7 +565,7 @@ class Stack(collections.Mapping): @classmethod def _from_db(cls, context, stack, use_stored_context=False, cache_data=None, - service_check_defer=False, load_template=True): + load_template=True): if load_template: template = tmpl.Template.load( context, stack.raw_template_id, stack.raw_template) @@ -596,8 +589,7 @@ class Stack(collections.Mapping): prev_raw_template_id=stack.prev_raw_template_id, current_deps=stack.current_deps, cache_data=cache_data, nested_depth=stack.nested_depth, - deleted_time=stack.deleted_at, - service_check_defer=service_check_defer) + deleted_time=stack.deleted_at) def get_kwargs_for_cloning(self, keep_status=False, only_db=False): """Get common kwargs for calling Stack() for cloning. diff --git a/heat/tests/generic_resource.py b/heat/tests/generic_resource.py index 5e1d728deb..9f59945699 100644 --- a/heat/tests/generic_resource.py +++ b/heat/tests/generic_resource.py @@ -301,11 +301,15 @@ class ResourceWithAttributeType(GenericResource): class ResourceWithDefaultClientName(resource.Resource): default_client_name = 'sample' + properties_schema = {} + class ResourceWithDefaultClientNameExt(resource.Resource): default_client_name = 'sample' required_service_extension = 'foo' + properties_schema = {} + class ResourceWithFnGetAttType(GenericResource): def get_attribute(self, name): diff --git a/heat/tests/openstack/neutron/lbaas/test_l7policy.py b/heat/tests/openstack/neutron/lbaas/test_l7policy.py index bf377d2eb6..95ab88779f 100644 --- a/heat/tests/openstack/neutron/lbaas/test_l7policy.py +++ b/heat/tests/openstack/neutron/lbaas/test_l7policy.py @@ -147,6 +147,9 @@ class L7PolicyTest(common.HeatTestCase): self.assertTrue(self.l7policy.check_create_complete(props)) def test_create_missing_properties(self): + self.patchobject(l7policy.L7Policy, 'is_service_available', + return_value=(True, None)) + for prop in ('action', 'listener'): tmpl = yaml.load(inline_templates.L7POLICY_TEMPLATE) del tmpl['resources']['l7policy']['properties'][prop] diff --git a/heat/tests/openstack/neutron/lbaas/test_l7rule.py b/heat/tests/openstack/neutron/lbaas/test_l7rule.py index 8789b092b4..258343ffc6 100644 --- a/heat/tests/openstack/neutron/lbaas/test_l7rule.py +++ b/heat/tests/openstack/neutron/lbaas/test_l7rule.py @@ -96,6 +96,9 @@ class L7RuleTest(common.HeatTestCase): self.assertTrue(self.l7rule.check_create_complete(props)) def test_create_missing_properties(self): + self.patchobject(l7rule.L7Rule, 'is_service_available', + return_value=(True, None)) + for prop in ('l7policy', 'type', 'compare_type', 'value'): tmpl = yaml.load(inline_templates.L7RULE_TEMPLATE) del tmpl['resources']['l7rule']['properties'][prop] diff --git a/heat/tests/openstack/neutron/lbaas/test_listener.py b/heat/tests/openstack/neutron/lbaas/test_listener.py index b866d4f571..0b5887e8e9 100644 --- a/heat/tests/openstack/neutron/lbaas/test_listener.py +++ b/heat/tests/openstack/neutron/lbaas/test_listener.py @@ -50,6 +50,9 @@ class ListenerTest(common.HeatTestCase): return_value=self.neutron_client) def test_validate_terminated_https(self): + self.patchobject(listener.Listener, 'is_service_available', + return_value=(True, None)) + tmpl = yaml.load(inline_templates.LISTENER_TEMPLATE) props = tmpl['resources']['listener']['properties'] props['protocol'] = 'TERMINATED_HTTPS' diff --git a/heat/tests/openstack/neutron/lbaas/test_pool.py b/heat/tests/openstack/neutron/lbaas/test_pool.py index 1e19727680..b71c25bc83 100644 --- a/heat/tests/openstack/neutron/lbaas/test_pool.py +++ b/heat/tests/openstack/neutron/lbaas/test_pool.py @@ -113,6 +113,9 @@ class PoolTest(common.HeatTestCase): self.assertTrue(self.pool.check_create_complete(props)) def test_create_missing_properties(self): + self.patchobject(pool.Pool, 'is_service_available', + return_value=(True, None)) + for prop in ('lb_algorithm', 'listener', 'protocol'): tmpl = yaml.load(inline_templates.POOL_TEMPLATE) del tmpl['resources']['pool']['properties']['loadbalancer'] diff --git a/heat/tests/openstack/neutron/test_neutron_rbac_policy.py b/heat/tests/openstack/neutron/test_neutron_rbac_policy.py index ac99f13c0f..a806c16216 100644 --- a/heat/tests/openstack/neutron/test_neutron_rbac_policy.py +++ b/heat/tests/openstack/neutron/test_neutron_rbac_policy.py @@ -67,6 +67,10 @@ class RBACPolicyTest(common.HeatTestCase): msg = ("Invalid action %(action)s for object type %(type)s." % {'action': invalid_action, 'type': obj_type}) + + self.patchobject(type(self.rbac), 'is_service_available', + return_value=(True, None)) + self.assertRaisesRegex(exception.StackValidationFailed, msg, self.rbac.validate) @@ -86,11 +90,19 @@ class RBACPolicyTest(common.HeatTestCase): tpl['resources']['rbac']['properties']['object_type'] = 'networks' self._create_stack(tmpl=yaml.safe_dump(tpl)) msg = "Invalid object_type: networks. " + + self.patchobject(type(self.rbac), 'is_service_available', + return_value=(True, None)) + self.assertRaisesRegex(exception.StackValidationFailed, msg, self.rbac.validate) def test_validate_object_id_reference(self): self._create_stack(tmpl=inline_templates.RBAC_REFERENCE_TEMPLATE) + + self.patchobject(type(self.rbac), 'is_service_available', + return_value=(True, None)) + # won't check the object_id, so validate() is success self.rbac.validate() diff --git a/heat/tests/openstack/neutron/test_neutron_security_group_rule.py b/heat/tests/openstack/neutron/test_neutron_security_group_rule.py index 01b7036d59..5e8a5e43f8 100644 --- a/heat/tests/openstack/neutron/test_neutron_security_group_rule.py +++ b/heat/tests/openstack/neutron/test_neutron_security_group_rule.py @@ -63,6 +63,10 @@ class SecurityGroupRuleTest(common.HeatTestCase): expected) def test_validate_conflict_props(self): + self.patchobject(security_group_rule.SecurityGroupRule, + 'is_service_available', + return_value=(True, None)) + tmpl = inline_templates.SECURITY_GROUP_RULE_TEMPLATE tmpl += ' remote_ip_prefix: "123"' self._create_stack(tmpl=tmpl) @@ -71,6 +75,10 @@ class SecurityGroupRuleTest(common.HeatTestCase): self.sg_rule.validate) def test_validate_max_port_less_than_min_port(self): + self.patchobject(security_group_rule.SecurityGroupRule, + 'is_service_available', + return_value=(True, None)) + tmpl = inline_templates.SECURITY_GROUP_RULE_TEMPLATE tmpl += ' port_range_max: 50' self._create_stack(tmpl=tmpl) diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index 0be4edb7bc..de5eb55cdd 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -1337,8 +1337,7 @@ class StackServiceTest(common.HeatTestCase): mock.call(self.ctx, 'bar'), ]) mock_stack_load.assert_called_once_with(self.ctx, - stack=db_stack, - service_check_defer=True) + stack=db_stack) self.assertTrue(lock2.release.called) reason = ('Engine went down during stack %s' % fake_stack.action) mock_thread.start_with_acquired_lock.assert_called_once_with( diff --git a/heat/tests/test_metadata_refresh.py b/heat/tests/test_metadata_refresh.py index 94992599c5..1741fe28ac 100644 --- a/heat/tests/test_metadata_refresh.py +++ b/heat/tests/test_metadata_refresh.py @@ -225,14 +225,12 @@ class WaitConditionMetadataUpdateTest(common.HeatTestCase): @mock.patch.object(glance.GlanceClientPlugin, 'find_image_by_name_or_id') @mock.patch.object(instance.Instance, 'handle_create') @mock.patch.object(instance.Instance, 'check_create_complete') - @mock.patch.object(instance.Instance, 'is_service_available') @mock.patch.object(scheduler.TaskRunner, '_sleep') @mock.patch.object(WaitConditionHandle, 'identifier') - def test_wait_metadata(self, mock_identifier, mock_sleep, mock_available, + def test_wait_metadata(self, mock_identifier, mock_sleep, mock_check, mock_handle, *args): """Tests a wait condition metadata update after a signal call.""" - mock_available.return_value = (True, None) # Setup Stack temp = template_format.parse(TEST_TEMPLATE_WAIT_CONDITION) template = tmpl.Template(temp) @@ -298,7 +296,6 @@ class WaitConditionMetadataUpdateTest(common.HeatTestCase): jsonutils.loads(inst.metadata_get(refresh=True)['test'])) # Verify outgoing calls - self.assertGreater(mock_available.call_count, 0) self.assertEqual(2, mock_handle.call_count) self.assertEqual(2, mock_check.call_count) diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index bfd39ccd98..c75e023117 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -3909,15 +3909,15 @@ class ResourceAvailabilityTest(common.HeatTestCase): resource_type='UnavailableResourceType') mock_stack = mock.MagicMock() - mock_stack.service_check_defer = False + mock_stack.in_convergence_check = False + mock_stack.db_resource_get.return_value = None + rsrc = generic_rsrc.ResourceWithDefaultClientName('test_res', + definition, + mock_stack) ex = self.assertRaises( exception.ResourceTypeUnavailable, - generic_rsrc.ResourceWithDefaultClientName.__new__, - cls=generic_rsrc.ResourceWithDefaultClientName, - name='test_stack', - definition=definition, - stack=mock_stack) + rsrc.validate_template) msg = ('HEAT-E99001 Service sample is not available for resource ' 'type UnavailableResourceType, reason: ' @@ -3945,15 +3945,15 @@ class ResourceAvailabilityTest(common.HeatTestCase): resource_type='UnavailableResourceType') mock_stack = mock.MagicMock() - mock_stack.service_check_defer = False + mock_stack.in_convergence_check = False + mock_stack.db_resource_get.return_value = None + rsrc = generic_rsrc.ResourceWithDefaultClientName('test_res', + definition, + mock_stack) ex = self.assertRaises( exception.ResourceTypeUnavailable, - generic_rsrc.ResourceWithDefaultClientName.__new__, - cls=generic_rsrc.ResourceWithDefaultClientName, - name='test_stack', - definition=definition, - stack=mock_stack) + rsrc.validate_template) msg = ('HEAT-E99001 Service sample is not available for resource ' 'type UnavailableResourceType, reason: ' diff --git a/heat/tests/test_stack.py b/heat/tests/test_stack.py index 94f42ea2c1..2416a620a9 100644 --- a/heat/tests/test_stack.py +++ b/heat/tests/test_stack.py @@ -454,8 +454,7 @@ class StackTest(common.HeatTestCase): prev_raw_template_id=None, current_deps=None, cache_data=None, nested_depth=0, - deleted_time=None, - service_check_defer=False) + deleted_time=None) self.m.ReplayAll() stack.Stack.load(self.ctx, stack_id=self.stack.id)