From 6a48c45bfd189086017e437c01b0bf7c7d3eb0fe Mon Sep 17 00:00:00 2001 From: Angus Salkeld Date: Wed, 6 May 2015 11:28:44 +1000 Subject: [PATCH] Don't create events when signals don't perform an action If we get repeat signals that cause no concrete actions this can cause large quantities of senseless events that suppress useful events. Change-Id: I79374d27648319f241f36ab041784fab37823ddb Closes-bug: #1445361 --- heat/engine/resource.py | 7 +++++ .../openstack/heat/scaling_policy.py | 4 +-- .../autoscaling/test_heat_scaling_policy.py | 6 +++-- heat/tests/autoscaling/test_scaling_policy.py | 7 +++-- heat/tests/test_signal.py | 27 +++++++++++++++++++ 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index a10aef3f86..088498d960 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -63,6 +63,10 @@ class UpdateReplace(Exception): super(Exception, self).__init__(six.text_type(msg)) +class NoActionRequired(Exception): + pass + + class ResourceInError(exception.HeatException): msg_fmt = _('Went to status %(resource_status)s ' 'due to "%(status_reason)s"') @@ -1180,6 +1184,9 @@ class Resource(object): else: reason_string = get_string_details() self._add_event('SIGNAL', self.status, reason_string) + except NoActionRequired: + # Don't log an event as it just spams the user. + pass except Exception as ex: LOG.exception(_LE('signal %(name)s : %(msg)s') % {'name': six.text_type(self), 'msg': ex}) diff --git a/heat/engine/resources/openstack/heat/scaling_policy.py b/heat/engine/resources/openstack/heat/scaling_policy.py index a39882118b..e836166e5a 100644 --- a/heat/engine/resources/openstack/heat/scaling_policy.py +++ b/heat/engine/resources/openstack/heat/scaling_policy.py @@ -153,13 +153,13 @@ class AutoScalingPolicy(signal_responder.SignalResponder, {'name': self.name, 'state': alarm_state}) if alarm_state != 'alarm': - return + raise resource.NoActionRequired() if self._cooldown_inprogress(): LOG.info(_LI("%(name)s NOT performing scaling action, " "cooldown %(cooldown)s"), {'name': self.name, 'cooldown': self.properties[self.COOLDOWN]}) - return + raise resource.NoActionRequired() asgn_id = self.properties[self.AUTO_SCALING_GROUP_NAME] group = self.stack.resource_by_refid(asgn_id) diff --git a/heat/tests/autoscaling/test_heat_scaling_policy.py b/heat/tests/autoscaling/test_heat_scaling_policy.py index 9b78eb00fb..39fadf1596 100644 --- a/heat/tests/autoscaling/test_heat_scaling_policy.py +++ b/heat/tests/autoscaling/test_heat_scaling_policy.py @@ -92,7 +92,8 @@ class TestAutoScalingPolicy(common.HeatTestCase): test = {'current': 'not_an_alarm'} with mock.patch.object(pol, '_cooldown_inprogress', side_effect=AssertionError()) as dont_call: - pol.handle_signal(details=test) + self.assertRaises(resource.NoActionRequired, + pol.handle_signal, details=test) self.assertEqual([], dont_call.call_args_list) def test_scaling_policy_cooldown_toosoon(self): @@ -106,7 +107,8 @@ class TestAutoScalingPolicy(common.HeatTestCase): side_effect=AssertionError) as dont_call: with mock.patch.object(pol, '_cooldown_inprogress', return_value=True) as mock_cip: - pol.handle_signal(details=test) + self.assertRaises(resource.NoActionRequired, + pol.handle_signal, details=test) mock_cip.assert_called_once_with() self.assertEqual([], dont_call.call_args_list) diff --git a/heat/tests/autoscaling/test_scaling_policy.py b/heat/tests/autoscaling/test_scaling_policy.py index ddc3fb4b43..a85f9123c4 100644 --- a/heat/tests/autoscaling/test_scaling_policy.py +++ b/heat/tests/autoscaling/test_scaling_policy.py @@ -20,6 +20,7 @@ import six from heat.common import exception from heat.common import template_format +from heat.engine import resource from heat.engine import scheduler from heat.tests.autoscaling import inline_templates from heat.tests import common @@ -91,7 +92,8 @@ class TestAutoScalingPolicy(common.HeatTestCase): test = {'current': 'not_an_alarm'} with mock.patch.object(pol, '_cooldown_inprogress', side_effect=AssertionError()) as dont_call: - pol.handle_signal(details=test) + self.assertRaises(resource.NoActionRequired, + pol.handle_signal, details=test) self.assertEqual([], dont_call.call_args_list) def test_scaling_policy_cooldown_toosoon(self): @@ -105,7 +107,8 @@ class TestAutoScalingPolicy(common.HeatTestCase): side_effect=AssertionError) as dont_call: with mock.patch.object(pol, '_cooldown_inprogress', return_value=True) as mock_cip: - pol.handle_signal(details=test) + self.assertRaises(resource.NoActionRequired, + pol.handle_signal, details=test) mock_cip.assert_called_once_with() self.assertEqual([], dont_call.call_args_list) diff --git a/heat/tests/test_signal.py b/heat/tests/test_signal.py index b252acf9e2..468daefc83 100644 --- a/heat/tests/test_signal.py +++ b/heat/tests/test_signal.py @@ -244,6 +244,33 @@ class SignalTest(common.HeatTestCase): self.m.VerifyAll() + def test_signal_no_action(self): + test_d = {'Data': 'foo', 'Reason': 'bar', + 'Status': 'SUCCESS', 'UniqueId': '123'} + + self.stack = self.create_stack() + self.stack.create() + + # mock a NoActionRequired from handle_signal() + self.m.StubOutWithMock(generic_resource.SignalResource, + 'handle_signal') + generic_resource.SignalResource.handle_signal(test_d).AndRaise( + resource.NoActionRequired()) + + # _add_event should not be called. + self.m.StubOutWithMock(generic_resource.SignalResource, + '_add_event') + + self.m.ReplayAll() + + rsrc = self.stack['signal_handler'] + self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) + self.assertTrue(rsrc.requires_deferred_auth) + + rsrc.signal(details=test_d) + + self.m.VerifyAll() + def test_signal_different_reason_types(self): self.stack = self.create_stack() self.stack.create()