Return error message if validate fail when clearing hook

There are some situations:
1. User can run the command "heat hook-clear" on any
resource in the stack, even if there is no hook
defined on it, and no error or warning message is returned.
2. User run the command "heat hook-clear" on resource
to clear the hook which resource unset, and no error or
warning message is returned.
3. User run the command "heat hook-clear" on resource
even if the resource's action is not support to signal, and
no error or warning message is returned.
4. User run the command "heat hook-clear" to clear invalid
hooks, and no error or warnning message is returned.

This patch will check the situations above before call
resource.signal, and will return error messages to user.

Change-Id: Ifb9befad864ebe1bb5f8b419b95d1b3a95530573
Closes-Bug: #1472515
This commit is contained in:
huangtianhua 2015-07-09 09:15:10 +08:00
parent f1cfb9ed39
commit 18ded430d0
8 changed files with 139 additions and 91 deletions

View File

@ -302,6 +302,8 @@ def map_remote_error(ex):
'MissingCredentialError',
'ResourcePropertyConflict',
'PropertyUnspecifiedError',
'NotSupported',
'InvalidBreakPointHook',
)
denied_errors = ('Forbidden', 'NotAuthorized')
already_exists_errors = ('StackExists')

View File

@ -92,6 +92,7 @@ class FaultWrapper(wsgi.Middleware):
'OrphanedObjectError': webob.exc.HTTPBadRequest,
'UnsupportedObjectError': webob.exc.HTTPBadRequest,
'ResourceTypeUnavailable': webob.exc.HTTPBadRequest,
'InvalidBreakPointHook': webob.exc.HTTPBadRequest,
}
def _map_exception_to_error(self, class_exception):

View File

@ -1478,17 +1478,40 @@ class Resource(object):
'''
return base64.b64encode(data)
def signal(self, details=None):
'''
signal the resource. Subclasses should provide a handle_signal() method
to implement the signal, the base-class raise an exception if no
handler is implemented.
'''
def _signal_check_action(self):
if self.action in self.no_signal_actions:
self._add_event(self.action, self.status,
'Cannot signal resource during %s' % self.action)
ex = Exception(_('Cannot signal resource during %s') % self.action)
raise exception.ResourceFailure(ex, self)
msg = _('Signal resource during %s') % self.action
raise exception.NotSupported(feature=msg)
def _signal_check_hook(self, details):
if details and 'unset_hook' in details:
hook = details['unset_hook']
if not environment.valid_hook_type(hook):
msg = (_('Invalid hook type "%(hook)s" for %(resource)s') %
{'hook': hook, 'resource': six.text_type(self)})
raise exception.InvalidBreakPointHook(message=msg)
if not self.has_hook(hook):
msg = (_('The "%(hook)s" hook is not defined '
'on %(resource)s') %
{'hook': hook, 'resource': six.text_type(self)})
raise exception.InvalidBreakPointHook(message=msg)
def _unset_hook(self, details):
# Clear the hook without interfering with resources'
# `handle_signal` callbacks:
hook = details['unset_hook']
self.clear_hook(hook)
LOG.info(_LI('Clearing %(hook)s hook on %(resource)s'),
{'hook': hook, 'resource': six.text_type(self)})
self._add_event(self.action, self.status,
"Hook %s is cleared" % hook)
def _handle_signal(self, details):
if not callable(getattr(self, 'handle_signal', None)):
raise exception.ResourceActionNotSupported(action='signal')
def get_string_details():
if details is None:
@ -1507,22 +1530,6 @@ class Resource(object):
return 'Unknown'
# Clear the hook without interfering with resources'
# `handle_signal` callbacks:
if (details and 'unset_hook' in details and
environment.valid_hook_type(details.get('unset_hook'))):
hook = details['unset_hook']
if self.has_hook(hook):
self.clear_hook(hook)
LOG.info(_LI('Clearing %(hook)s hook on %(resource)s'),
{'hook': hook, 'resource': six.text_type(self)})
self._add_event(self.action, self.status,
"Hook %s is cleared" % hook)
return
if not callable(getattr(self, 'handle_signal', None)):
raise exception.ResourceActionNotSupported(action='signal')
try:
signal_result = self.handle_signal(details)
if signal_result:
@ -1539,6 +1546,20 @@ class Resource(object):
failure = exception.ResourceFailure(ex, self)
raise failure
def signal(self, details=None, need_check=True):
'''
signal the resource. Subclasses should provide a handle_signal() method
to implement the signal, the base-class raise an exception if no
handler is implemented.
'''
if need_check:
self._signal_check_action()
self._signal_check_hook(details)
if details and 'unset_hook' in details:
self._unset_hook(details)
return
self._handle_signal(details)
def handle_update(self, json_snippet=None, tmpl_diff=None, prop_diff=None):
if prop_diff:
raise exception.UpdateReplace(self.name)

View File

@ -1329,9 +1329,9 @@ class EngineService(service.Service):
implementation.
'''
def _resource_signal(stack, rsrc, details):
def _resource_signal(stack, rsrc, details, need_check):
LOG.debug("signaling resource %s:%s" % (stack.name, rsrc.name))
rsrc.signal(details)
rsrc.signal(details, need_check)
if not rsrc.signal_needs_metadata_updates:
return
@ -1355,13 +1355,16 @@ class EngineService(service.Service):
self._verify_stack_resource(stack, resource_name)
rsrc = stack[resource_name]
if callable(rsrc.signal):
rsrc._signal_check_action()
rsrc._signal_check_hook(details)
if sync_call:
_resource_signal(stack, rsrc, details)
_resource_signal(stack, rsrc, details, False)
return rsrc.metadata_get()
else:
self.thread_group_mgr.start(stack.id, _resource_signal,
stack, rsrc, details)
stack, rsrc, details, False)
@context.request_context
def find_physical_resource(self, cnxt, physical_resource_id):

View File

@ -1240,21 +1240,24 @@ class StackServiceTest(common.HeatTestCase):
self.m.VerifyAll()
def test_signal_reception_async(self):
self.eng.thread_group_mgr = tools.DummyThreadGroupMgrLogStart()
stack_name = 'signal_reception_async'
def _stack_create(self, stack_name):
stack = tools.get_stack(stack_name, self.ctx, policy_template)
self.stack = stack
tools.setup_keystone_mocks(self.m, stack)
self.m.ReplayAll()
stack.store()
stack.create()
test_data = {'food': 'yum'}
self.m.StubOutWithMock(service.EngineService, '_get_stack')
s = stack_object.Stack.get_by_id(self.ctx, self.stack.id)
s = stack_object.Stack.get_by_id(self.ctx, stack.id)
service.EngineService._get_stack(self.ctx,
self.stack.identifier()).AndReturn(s)
stack.identifier()).AndReturn(s)
self.m.ReplayAll()
return stack
def test_signal_reception_async(self):
self.eng.thread_group_mgr = tools.DummyThreadGroupMgrLogStart()
stack_name = 'signal_reception_async'
self.stack = self._stack_create(stack_name)
test_data = {'food': 'yum'}
self.m.ReplayAll()
@ -1269,21 +1272,11 @@ class StackServiceTest(common.HeatTestCase):
def test_signal_reception_sync(self):
stack_name = 'signal_reception_sync'
stack = tools.get_stack(stack_name, self.ctx, policy_template)
self.stack = stack
tools.setup_keystone_mocks(self.m, stack)
self.m.ReplayAll()
stack.store()
stack.create()
self.stack = self._stack_create(stack_name)
test_data = {'food': 'yum'}
self.m.StubOutWithMock(service.EngineService, '_get_stack')
s = stack_object.Stack.get_by_id(self.ctx, self.stack.id)
service.EngineService._get_stack(self.ctx,
self.stack.identifier()).AndReturn(s)
self.m.StubOutWithMock(res.Resource, 'signal')
res.Resource.signal(mox.IgnoreArg()).AndReturn(None)
res.Resource.signal(mox.IgnoreArg(), False).AndReturn(None)
self.m.ReplayAll()
self.eng.resource_signal(self.ctx,
@ -1295,20 +1288,9 @@ class StackServiceTest(common.HeatTestCase):
def test_signal_reception_no_resource(self):
stack_name = 'signal_reception_no_resource'
stack = tools.get_stack(stack_name, self.ctx, policy_template)
tools.setup_keystone_mocks(self.m, stack)
self.stack = stack
self.m.ReplayAll()
stack.store()
stack.create()
self.stack = self._stack_create(stack_name)
test_data = {'food': 'yum'}
self.m.StubOutWithMock(service.EngineService, '_get_stack')
s = stack_object.Stack.get_by_id(self.ctx, self.stack.id)
service.EngineService._get_stack(self.ctx,
self.stack.identifier()).AndReturn(s)
self.m.ReplayAll()
ex = self.assertRaises(dispatcher.ExpectedException,
self.eng.resource_signal, self.ctx,
dict(self.stack.identifier()),
@ -1345,23 +1327,13 @@ class StackServiceTest(common.HeatTestCase):
self.m.VerifyAll()
def test_signal_returns_metadata(self):
stack = tools.get_stack('signal_reception', self.ctx, policy_template)
self.stack = stack
tools.setup_keystone_mocks(self.m, stack)
self.m.ReplayAll()
stack.store()
stack.create()
self.stack = self._stack_create('signal_reception')
rsrc = self.stack['WebServerScaleDownPolicy']
test_metadata = {'food': 'yum'}
rsrc = stack['WebServerScaleDownPolicy']
rsrc.metadata_set(test_metadata)
self.m.StubOutWithMock(service.EngineService, '_get_stack')
s = stack_object.Stack.get_by_id(self.ctx, self.stack.id)
service.EngineService._get_stack(self.ctx,
self.stack.identifier()).AndReturn(s)
self.m.StubOutWithMock(res.Resource, 'signal')
res.Resource.signal(mox.IgnoreArg()).AndReturn(None)
res.Resource.signal(mox.IgnoreArg(), False).AndReturn(None)
self.m.ReplayAll()
md = self.eng.resource_signal(self.ctx,
@ -1369,6 +1341,40 @@ class StackServiceTest(common.HeatTestCase):
'WebServerScaleDownPolicy', None,
sync_call=True)
self.assertEqual(test_metadata, md)
self.m.VerifyAll()
def test_signal_unset_invalid_hook(self):
self.stack = self._stack_create('signal_unset_invalid_hook')
details = {'unset_hook': 'invalid_hook'}
ex = self.assertRaises(dispatcher.ExpectedException,
self.eng.resource_signal,
self.ctx,
dict(self.stack.identifier()),
'WebServerScaleDownPolicy',
details)
msg = 'Invalid hook type "invalid_hook"'
self.assertIn(msg, six.text_type(ex.exc_info[1]))
self.assertEqual(exception.InvalidBreakPointHook,
ex.exc_info[0])
self.m.VerifyAll()
def test_signal_unset_not_defined_hook(self):
self.stack = self._stack_create('signal_unset_not_defined_hook')
details = {'unset_hook': 'pre-update'}
ex = self.assertRaises(dispatcher.ExpectedException,
self.eng.resource_signal,
self.ctx,
dict(self.stack.identifier()),
'WebServerScaleDownPolicy',
details)
msg = ('The "pre-update" hook is not defined on '
'AWSScalingPolicy "WebServerScaleDownPolicy"')
self.assertIn(msg, six.text_type(ex.exc_info[1]))
self.assertEqual(exception.InvalidBreakPointHook,
ex.exc_info[0])
self.m.VerifyAll()
def test_signal_calls_metadata_update(self):
@ -1385,7 +1391,7 @@ class StackServiceTest(common.HeatTestCase):
self.stack.identifier()).AndReturn(s)
self.m.StubOutWithMock(res.Resource, 'signal')
res.Resource.signal(mox.IgnoreArg()).AndReturn(None)
res.Resource.signal(mox.IgnoreArg(), False).AndReturn(None)
self.m.StubOutWithMock(res.Resource, 'metadata_update')
# this will be called once for the Random resource
res.Resource.metadata_update().AndReturn(None)
@ -1413,7 +1419,7 @@ class StackServiceTest(common.HeatTestCase):
self.stack.identifier()).AndReturn(s)
self.m.StubOutWithMock(res.Resource, 'signal')
res.Resource.signal(mox.IgnoreArg()).AndReturn(None)
res.Resource.signal(mox.IgnoreArg(), False).AndReturn(None)
# this will never be called
self.m.StubOutWithMock(res.Resource, 'metadata_update')
self.m.ReplayAll()

View File

@ -199,11 +199,10 @@ class ResourceTest(common.HeatTestCase):
for status in res.STATUSES:
res.state_set(action, status)
ev = self.patchobject(res, '_add_event')
ex = self.assertRaises(exception.ResourceFailure,
ex = self.assertRaises(exception.NotSupported,
res.signal)
self.assertEqual('Exception: resources.res: '
'Cannot signal resource during '
'%s' % action, six.text_type(ex))
self.assertEqual('Signal resource during %s is not '
'supported.' % action, six.text_type(ex))
ev.assert_called_with(
action, status,
'Cannot signal resource during %s' % action)
@ -2643,10 +2642,10 @@ class ResourceHookTest(common.HeatTestCase):
self.assertFalse(res.clear_hook.called)
self.assertRaises(exception.ResourceActionNotSupported,
res.signal, {})
res.signal, {'other_hook': 'alarm'})
self.assertFalse(res.clear_hook.called)
self.assertRaises(exception.ResourceActionNotSupported,
self.assertRaises(exception.InvalidBreakPointHook,
res.signal, {'unset_hook': 'unknown_hook'})
self.assertFalse(res.clear_hook.called)
@ -2657,7 +2656,7 @@ class ResourceHookTest(common.HeatTestCase):
res.clear_hook.assert_called_with('pre-update')
res.has_hook = mock.Mock(return_value=False)
self.assertRaises(exception.ResourceActionNotSupported,
self.assertRaises(exception.InvalidBreakPointHook,
res.signal, {'unset_hook': 'pre-create'})

View File

@ -618,9 +618,7 @@ class SignalTest(common.HeatTestCase):
self.m.VerifyAll()
def test_signal_reception_wrong_state(self):
# assert that we get the correct exception when calling a
# resource.signal() that is in having a destructive action.
def _test_signal_not_supported_action(self, action='DELETE'):
self.stack = self.create_stack()
self.m.ReplayAll()
@ -629,14 +627,25 @@ class SignalTest(common.HeatTestCase):
rsrc = self.stack['signal_handler']
self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state)
# manually override the action to DELETE
rsrc.action = rsrc.DELETE
rsrc.action = action
err_metadata = {'Data': 'foo', 'Status': 'SUCCESS', 'UniqueId': '123'}
self.assertRaises(exception.ResourceFailure, rsrc.signal,
details=err_metadata)
msg = 'Signal resource during %s is not supported.' % action
exc = self.assertRaises(exception.NotSupported, rsrc.signal,
details=err_metadata)
self.assertEqual(msg, six.text_type(exc))
self.m.VerifyAll()
def test_signal_in_delete_state(self):
# assert that we get the correct exception when calling a
# resource.signal() that is in delete action.
self._test_signal_not_supported_action()
def test_signal_in_suspend_state(self):
# assert that we get the correct exception when calling a
# resource.signal() that is in suspend action.
self._test_signal_not_supported_action(action='SUSPEND')
def test_signal_reception_failed_call(self):
# assert that we get the correct exception from resource.signal()
# when resource.handle_signal() raises an exception.

View File

@ -13,7 +13,9 @@
import copy
import json
from heatclient import exc
from oslo_log import log as logging
import six
from testtools import matchers
from heat_integrationtests.common import test
@ -728,8 +730,13 @@ outputs:
self._wait_for_resource_status(
stack_identifier, 'JobServerGroup', 'SUSPEND_COMPLETE')
# Send a signal and confirm nothing happened.
self.client.resources.signal(stack_identifier, 'ScaleUpPolicy')
# Send a signal and a exception will raise
ex = self.assertRaises(exc.BadRequest,
self.client.resources.signal,
stack_identifier, 'ScaleUpPolicy')
error_msg = 'Signal resource during SUSPEND is not supported'
self.assertIn(error_msg, six.text_type(ex))
ev = self.wait_for_event_with_reason(
stack_identifier,
reason='Cannot signal resource during SUSPEND',