From 06a713c4456203cd561f16721dc8ac3bcbb37a39 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 24 Nov 2015 12:29:38 -0500 Subject: [PATCH] Add a separate get_class_to_instantiate() method to Environment We were previously using get_class for two different purposes - to get a resource plugin on which we could perform introspection to obtain the properties and attributes schema, and to get a resource plugin we could instantiate to create a Resource object. These are both the same except in the case of a TemplateResource, where having two different use cases for the same piece of code was adding considerable extra complexity. Combining the use cases in this way also made the error handling confusing (leading to bug 1518458). This change separates out the two cases. Change-Id: I3bde081cd4537810c8c6a0948ab447c3fb7ca9bc Related-Bug: #1447194 Related-Bug: #1518458 Related-Bug: #1508115 --- heat/engine/environment.py | 21 ++++++++++++++++++- heat/engine/resource.py | 11 +++------- .../openstack/heat/resource_group.py | 6 +----- heat/engine/service.py | 15 +++++-------- heat/tests/test_provider_template.py | 4 ---- heat/tests/test_resource.py | 5 +++-- 6 files changed, 32 insertions(+), 30 deletions(-) diff --git a/heat/engine/environment.py b/heat/engine/environment.py index 691dad0727..4cfacc5705 100644 --- a/heat/engine/environment.py +++ b/heat/engine/environment.py @@ -118,6 +118,12 @@ class ResourceInfo(object): def matches(self, resource_type): return False + def get_class(self): + raise NotImplemented + + def get_class_to_instantiate(self): + return self.get_class() + def __str__(self): return '[%s](User:%s) %s -> %s' % (self.description, self.user_resource, @@ -151,6 +157,10 @@ class TemplateResourceInfo(ResourceInfo): self.template_name, env, files=files) + def get_class_to_instantiate(self): + from heat.engine.resources import template_resource + return template_resource.TemplateResource + class MapResourceInfo(ResourceInfo): """Store the mapping of one resource type to another. @@ -424,6 +434,11 @@ class ResourceRegistry(object): name=resource_type) def get_class(self, resource_type, resource_name=None, files=None): + info = self.get_resource_info(resource_type, + resource_name=resource_name) + return info.get_class(files=files) + + def get_class_to_instantiate(self, resource_type, resource_name=None): if resource_type == "": msg = _('Resource "%s" has no type') % resource_name raise exception.StackValidationFailed(message=msg) @@ -441,7 +456,7 @@ class ResourceRegistry(object): except exception.EntityNotFound as exc: raise exception.StackValidationFailed(message=six.text_type(exc)) - return info.get_class(files=files) + return info.get_class_to_instantiate() def as_dict(self): """Return user resources in a dict format.""" @@ -589,6 +604,10 @@ class Environment(object): return self.registry.get_class(resource_type, resource_name, files=files) + def get_class_to_instantiate(self, resource_type, resource_name=None): + return self.registry.get_class_to_instantiate(resource_type, + resource_name) + def get_types(self, cnxt=None, support_status=None, diff --git a/heat/engine/resource.py b/heat/engine/resource.py index b4075662f1..ab904626a0 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -132,15 +132,10 @@ class Resource(object): # Call is already for a subclass, so pass it through ResourceClass = cls else: - from heat.engine.resources import template_resource - registry = stack.env.registry - try: - ResourceClass = registry.get_class(definition.resource_type, - resource_name=name, - files=stack.t.files) - except exception.NotFound: - ResourceClass = template_resource.TemplateResource + ResourceClass = registry.get_class_to_instantiate( + definition.resource_type, + resource_name=name) assert issubclass(ResourceClass, Resource) diff --git a/heat/engine/resources/openstack/heat/resource_group.py b/heat/engine/resources/openstack/heat/resource_group.py index a3d7b1a1ae..4d245493ed 100644 --- a/heat/engine/resources/openstack/heat/resource_group.py +++ b/heat/engine/resources/openstack/heat/resource_group.py @@ -274,11 +274,7 @@ class ResourceGroup(stack_resource.StackResource): res_def = next(six.itervalues( test_tmpl.resource_definitions(self.stack))) # make sure we can resolve the nested resource type - try: - self.stack.env.get_class(res_def.resource_type) - except exception.NotFound: - # its a template resource - pass + self.stack.env.get_class_to_instantiate(res_def.resource_type) try: name = "%s-%s" % (self.stack.name, self.name) diff --git a/heat/engine/service.py b/heat/engine/service.py index 074609d346..c8a1111ada 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1269,9 +1269,6 @@ class EngineService(service.Service): self.resource_enforcer.enforce(cnxt, type_name) try: resource_class = resources.global_env().get_class(type_name) - except exception.StackValidationFailed: - raise exception.EntityNotFound(entity='Resource Type', - name=type_name) except exception.NotFound: LOG.exception(_LE('Error loading resource type %s ' 'from global environment.'), @@ -1318,18 +1315,16 @@ class EngineService(service.Service): self.resource_enforcer.enforce(cnxt, type_name) try: resource_class = resources.global_env().get_class(type_name) - if resource_class.support_status.status == support.HIDDEN: - raise exception.NotSupported(type_name) - return resource_class.resource_to_template(type_name, - template_type) - except exception.StackValidationFailed: - raise exception.EntityNotFound(entity='Resource Type', - name=type_name) except exception.NotFound: LOG.exception(_LE('Error loading resource type %s ' 'from global environment.'), type_name) raise exception.InvalidGlobalResource(type_name=type_name) + else: + if resource_class.support_status.status == support.HIDDEN: + raise exception.NotSupported(type_name) + return resource_class.resource_to_template(type_name, + template_type) @context.request_context def list_events(self, cnxt, stack_identity, filters=None, limit=None, diff --git a/heat/tests/test_provider_template.py b/heat/tests/test_provider_template.py index 813b4ee2ff..a65785593a 100644 --- a/heat/tests/test_provider_template.py +++ b/heat/tests/test_provider_template.py @@ -690,10 +690,6 @@ class ProviderTemplateTest(common.HeatTestCase): test_templ = test_templ_file.read() self.assertTrue(test_templ, "Empty test template") self.m.StubOutWithMock(urlfetch, "get") - urlfetch.get(test_templ_name, - allowed_schemes=('file',) - ).AndRaise(urlfetch.URLFetchError( - _('Failed to retrieve template'))) urlfetch.get(test_templ_name, allowed_schemes=('http', 'https')).AndReturn(test_templ) parsed_test_templ = template_format.parse(test_templ) diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index 354d7e002c..382bc04629 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -69,12 +69,13 @@ class ResourceTest(common.HeatTestCase): self.dummy_timeout = 10 def test_get_class_ok(self): - cls = resources.global_env().get_class('GenericResourceType') + cls = resources.global_env().get_class_to_instantiate( + 'GenericResourceType') self.assertEqual(generic_rsrc.GenericResource, cls) def test_get_class_noexist(self): self.assertRaises(exception.StackValidationFailed, - resources.global_env().get_class, + resources.global_env().get_class_to_instantiate, 'NoExistResourceType') def test_resource_new_ok(self):