openstack
/
heat
Code Issues Proposed changes

Merge "Handle resource plugins without default_client_name"

This commit is contained in:
Zuul 2018-07-05 18:13:59 +00:00 committed by Gerrit Code Review
parent 3a6d724923 714d9eea4c
commit ab55acf43b
6 changed files with 92 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
heat/engine/clients/default_client_plugin.py Normal file
View File

@ -0,0 +1,25 @@
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from heat.engine.clients import client_plugin
class DefaultClientPlugin(client_plugin.ClientPlugin):
"""A ClientPlugin that has no client.
This is provided so that Resource can make use of the is_not_found() and
is_conflict() methods even if the resource plugin has not specified a
client plugin.
"""
def _create(self, version=None):
return super(DefaultClientPlugin, self)._create(version)

59
heat/engine/resource.py
View File

@ -33,6 +33,7 @@ from heat.common import timeutils
from heat.engine import attributes
from heat.engine.cfn import template as cfn_tmpl
from heat.engine import clients
from heat.engine.clients import default_client_plugin
from heat.engine import environment
from heat.engine import event
from heat.engine import function
@ -770,6 +771,21 @@ class Resource(status.ResourceStatus):
assert client_name, "Must specify client name"
return self.stack.clients.client_plugin(client_name)
def _default_client_plugin(self):
"""Always return a client plugin.
This will be the client_plugin if the resource has defined a
default_client_name, or a no-op plugin if it does not. Thus, the
result of this call always has e.g. is_not_found() and is_conflict()
methods.
"""
cp = None
if self.default_client_name:
cp = self.client_plugin()
if cp is None:
cp = default_client_plugin.DefaultClientPlugin(self.context)
return cp
@classmethod
def is_service_available(cls, context):
# NOTE(kanagaraj-manickam): return True to satisfy the cases like
@ -1164,7 +1180,7 @@ class Resource(status.ResourceStatus):
self.resource_id = self.external_id
self._show_resource()
except Exception as ex:
if self.client_plugin().is_not_found(ex):
if self._default_client_plugin().is_not_found(ex):
error_message = (_("Invalid external resource: Resource "
"%(external_id)s (%(type)s) can not "
"be found.") %
@ -1929,15 +1945,11 @@ class Resource(status.ResourceStatus):
def handle_delete(self):
"""Default implementation; should be overridden by resources."""
if self.entity and self.resource_id is not None:
try:
with self._default_client_plugin().ignore_not_found:
obj = getattr(self.client(), self.entity)
obj.delete(self.resource_id)
except Exception as ex:
if self.default_client_name is not None:
self.client_plugin().ignore_not_found(ex)
return None
raise
return self.resource_id
return self.resource_id
return None
@scheduler.wrappertask
def delete(self):
@ -1950,10 +1962,8 @@ class Resource(status.ResourceStatus):
def should_retry(exc):
if count >= retry_limit:
return False
if self.default_client_name:
return (self.client_plugin().is_conflict(exc) or
isinstance(exc, exception.PhysicalResourceExists))
return isinstance(exc, exception.PhysicalResourceExists)
return (self._default_client_plugin().is_conflict(exc) or
isinstance(exc, exception.PhysicalResourceExists))
action = self.DELETE
@ -2156,30 +2166,20 @@ class Resource(status.ResourceStatus):
if attr in self.base_attributes_schema:
# check resource_id, because usually it is required for getting
# information about resource
if not self.resource_id:
return None
try:
return getattr(self, '_{0}_resource'.format(attr))()
except Exception as ex:
if self.default_client_name is not None:
self.client_plugin().ignore_not_found(ex)
return None
raise
if self.resource_id is not None:
with self._default_client_plugin().ignore_not_found:
return getattr(self, '_{0}_resource'.format(attr))()
else:
try:
with self._default_client_plugin().ignore_not_found:
return self._resolve_attribute(attr)
except Exception as ex:
if self.default_client_name is not None:
self.client_plugin().ignore_not_found(ex)
return None
raise
return None
def _show_resource(self):
"""Default implementation; should be overridden by resources.
:returns: the map of resource information or None
"""
if self.entity:
if self.entity and self.default_client_name is not None:
try:
obj = getattr(self.client(), self.entity)
resource = obj.get(self.resource_id)
@ -2200,8 +2200,7 @@ class Resource(status.ResourceStatus):
try:
resource_data = self._show_resource()
except Exception as ex:
if (self.default_client_name is not None and
self.client_plugin().is_not_found(ex)):
if self._default_client_plugin().is_not_found(ex):
raise exception.EntityNotFound(
entity='Resource', name=self.name)
raise

2
heat/tests/openstack/manila/test_share_network.py
View File

@ -62,7 +62,7 @@ class ManilaShareNetworkTest(common.HeatTestCase):
self.client = mock.Mock()
self.patchobject(share_network.ManilaShareNetwork, 'client',
return_value=self.client)
self.client_plugin = mock.Mock()
self.client_plugin = mock.MagicMock()
def resolve_neutron(resource_type, name):
return name

1
heat/tests/openstack/mistral/test_external_resource.py
View File

@ -67,7 +67,6 @@ class TestMistralExternalResource(common.HeatTestCase):
self.patchobject(external_resource.MistralExternalResource,
'client',
return_value=self.mistral)
self.patchobject(client, 'mistral_base')
self.patchobject(client.MistralClientPlugin, '_create')
self.client = client.MistralClientPlugin(self.ctx)

2
heat/tests/openstack/mistral/test_workflow.py
View File

@ -388,8 +388,6 @@ class TestMistralWorkflow(common.HeatTestCase):
'_create_user'))
self.patches.append(mock.patch.object(signal_responder.SignalResponder,
'_create_keypair'))
self.patches.append(mock.patch.object(client,
'mistral_base'))
self.patches.append(mock.patch.object(client.MistralClientPlugin,
'_create'))
for patch in self.patches:

68
heat/tests/test_resource.py
View File

@ -372,8 +372,8 @@ class ResourceTest(common.HeatTestCase):
'test_resource', 'GenericResourceType',
external_id=external_id)
res = generic_rsrc.GenericResource('test_resource', tmpl, self.stack)
res.client_plugin = mock.Mock()
self.patchobject(res.client_plugin, 'is_not_found',
res._default_client_plugin = mock.Mock()
self.patchobject(res._default_client_plugin(), 'is_not_found',
return_value=True)
self.patchobject(res, '_show_resource', side_effect=Exception())
e = self.assertRaises(exception.StackValidationFailed,
@ -2508,6 +2508,9 @@ class ResourceTest(common.HeatTestCase):
stack = self.create_resource_for_attributes_tests()
res = stack['res']
class MyException(Exception):
pass
with mock.patch.object(res, '_show_resource') as show_attr:
# return None, if resource_id is None
self.assertIsNone(res.FnGetAtt('show'))
@ -2522,12 +2525,10 @@ class ResourceTest(common.HeatTestCase):
# clean resolved_values
res.attributes.reset_resolved_values()
with mock.patch.object(res, 'client_plugin') as client_plugin:
# generate error during calling _show_resource
show_attr.side_effect = [Exception]
self.assertIsNone(res.FnGetAtt('show'))
self.assertEqual(2, show_attr.call_count)
self.assertEqual(1, client_plugin.call_count)
# generate error during calling _show_resource
show_attr.side_effect = [MyException]
self.assertRaises(MyException, res.FnGetAtt, 'show')
self.assertEqual(2, show_attr.call_count)
def test_resolve_attributes_stuff_custom_attribute(self):
# check path with resolve_attribute
@ -2535,16 +2536,17 @@ class ResourceTest(common.HeatTestCase):
res = stack['res']
res.default_client_name = 'foo'
class MyException(Exception):
pass
with mock.patch.object(res, '_resolve_attribute') as res_attr:
res_attr.side_effect = ['Works', Exception]
res_attr.side_effect = ['Works', MyException]
self.assertEqual('Works', res.FnGetAtt('Foo'))
res_attr.assert_called_once_with('Foo')
# clean resolved_values
res.attributes.reset_resolved_values()
with mock.patch.object(res, 'client_plugin') as client_plugin:
self.assertIsNone(res.FnGetAtt('Foo'))
self.assertEqual(1, client_plugin.call_count)
self.assertRaises(MyException, res.FnGetAtt, 'Foo')
def test_resolve_attributes_no_default_client_name(self):
class MyException(Exception):
@ -2571,6 +2573,7 @@ class ResourceTest(common.HeatTestCase):
# set entity and recheck
res.resource_id = 'test_resource_id'
res.entity = 'test'
res.default_client_name = 'whatever'
# mock getting resource info
res.client = mock.Mock()
@ -3906,6 +3909,7 @@ class ResourceAvailabilityTest(common.HeatTestCase):
delete = mock.Mock()
res.client().entity.delete = delete
res.entity = 'entity'
res.default_client_name = 'something'
res.resource_id = '12345'
self.assertEqual('12345', res.handle_delete())
@ -3918,26 +3922,26 @@ class ResourceAvailabilityTest(common.HeatTestCase):
snippet = rsrc_defn.ResourceDefinition('aresource',
'OS::Heat::None')
res = resource.Resource('aresource', snippet, self.stack)
res.entity = 'entity'
res.default_client_name = 'foo'
res.resource_id = '12345'
FakeClient = collections.namedtuple('Client', ['entity'])
client = FakeClient(collections.namedtuple('entity', ['delete']))
client_plugin = res._default_client_plugin()
class FakeClientPlugin(object):
def ignore_not_found(self, ex):
if not isinstance(ex, exception.NotFound):
raise ex
def is_not_found(ex):
return isinstance(ex, exception.NotFound)
client_plugin.is_not_found = mock.Mock(side_effect=is_not_found)
self.patchobject(resource.Resource, 'client', return_value=client)
self.patchobject(resource.Resource, 'client_plugin',
return_value=FakeClientPlugin())
delete = mock.Mock()
delete.side_effect = [exception.NotFound()]
res.client().entity.delete = delete
res.entity = 'entity'
res.resource_id = '12345'
res.default_client_name = 'foo'
self.assertIsNone(res.handle_delete())
with mock.patch.object(res, '_default_client_plugin',
return_value=client_plugin):
self.assertIsNone(res.handle_delete())
delete.assert_called_once_with('12345')
def test_handle_delete_raise_error(self):
@ -3947,25 +3951,26 @@ class ResourceAvailabilityTest(common.HeatTestCase):
snippet = rsrc_defn.ResourceDefinition('aresource',
'OS::Heat::None')
res = resource.Resource('aresource', snippet, self.stack)
res.entity = 'entity'
res.default_client_name = 'something'
res.resource_id = '12345'
FakeClient = collections.namedtuple('Client', ['entity'])
client = FakeClient(collections.namedtuple('entity', ['delete']))
client_plugin = res._default_client_plugin()
class FakeClientPlugin(object):
def ignore_not_found(self, ex):
if not isinstance(ex, exception.NotFound):
raise ex
def is_not_found(ex):
return isinstance(ex, exception.NotFound)
client_plugin.is_not_found = mock.Mock(side_effect=is_not_found)
self.patchobject(resource.Resource, 'client', return_value=client)
self.patchobject(resource.Resource, 'client_plugin',
return_value=FakeClientPlugin())
delete = mock.Mock()
delete.side_effect = [exception.Error('boom!')]
res.client().entity.delete = delete
res.entity = 'entity'
res.resource_id = '12345'
ex = self.assertRaises(exception.Error, res.handle_delete)
with mock.patch.object(res, '_default_client_plugin',
return_value=client_plugin):
ex = self.assertRaises(exception.Error, res.handle_delete)
self.assertEqual('boom!', six.text_type(ex))
delete.assert_called_once_with('12345')
@ -4003,6 +4008,7 @@ class ResourceAvailabilityTest(common.HeatTestCase):
delete = mock.Mock()
res.client().entity.delete = delete
res.entity = 'entity'
res.default_client_name = 'something'
res.resource_id = None
self.assertIsNone(res.handle_delete())