diff --git a/heat/engine/resources/aws/autoscaling/autoscaling_group.py b/heat/engine/resources/aws/autoscaling/autoscaling_group.py index 7ce917d45..e76574b64 100644 --- a/heat/engine/resources/aws/autoscaling/autoscaling_group.py +++ b/heat/engine/resources/aws/autoscaling/autoscaling_group.py @@ -281,19 +281,22 @@ class AutoScalingGroup(instgrp.InstanceGroup, cooldown.CooldownMixin): adjustment_type=sc_util.CFN_CHANGE_IN_CAPACITY, min_adjustment_step=None): """Adjust the size of the scaling group if the cooldown permits.""" - if not self._is_scaling_allowed(): - LOG.info(_LI("%(name)s NOT performing scaling adjustment, " - "cooldown %(cooldown)s"), - {'name': self.name, - 'cooldown': self.properties[self.COOLDOWN]}) - raise exception.NoActionRequired() - capacity = grouputils.get_size(self) new_capacity = self._get_new_capacity(capacity, adjustment, adjustment_type, min_adjustment_step) + if new_capacity == capacity: + LOG.info(_LI("%s NOT performing scaling adjustment, " + "as there is no change in capacity.") % self.name) + raise exception.NoActionRequired() + + if not self._is_scaling_allowed(): + LOG.info(_LI("%(name)s NOT performing scaling adjustment, " + "cooldown %(cooldown)s") % { + 'name': self.name, + 'cooldown': self.properties[self.COOLDOWN]}) + raise exception.NoActionRequired() - changed_size = new_capacity != capacity # send a notification before, on-error and on-success. notif = { 'stack': self.stack, @@ -305,31 +308,37 @@ class AutoScalingGroup(instgrp.InstanceGroup, cooldown.CooldownMixin): 'group': self.FnGetRefId()}, 'suffix': 'start', } - notification.send(**notif) + size_changed = False try: - self.resize(new_capacity) - except Exception as resize_ex: - with excutils.save_and_reraise_exception(): - try: - notif.update({'suffix': 'error', - 'message': six.text_type(resize_ex), - 'capacity': grouputils.get_size(self), - }) - notification.send(**notif) - except Exception: - LOG.exception(_LE('Failed sending error notification')) - else: - notif.update({ - 'suffix': 'end', - 'capacity': new_capacity, - 'message': _("End resizing the group %(group)s") % { - 'group': notif['groupname']}, - }) notification.send(**notif) + try: + self.resize(new_capacity) + except Exception as resize_ex: + with excutils.save_and_reraise_exception(): + try: + notif.update({'suffix': 'error', + 'message': six.text_type(resize_ex), + 'capacity': grouputils.get_size(self), + }) + notification.send(**notif) + except Exception: + LOG.exception(_LE('Failed sending error notification')) + else: + size_changed = True + notif.update({ + 'suffix': 'end', + 'capacity': new_capacity, + 'message': _("End resizing the group %(group)s") % { + 'group': notif['groupname']}, + }) + notification.send(**notif) + except Exception: + LOG.error(_LE("Error in performing scaling adjustment for" + "group %s.") % self.name) + raise finally: self._finished_scaling("%s : %s" % (adjustment_type, adjustment), - changed_size=changed_size) - return changed_size + size_changed=size_changed) def _tags(self): """Add Identifying Tags to all servers in the group. diff --git a/heat/engine/resources/openstack/heat/scaling_policy.py b/heat/engine/resources/openstack/heat/scaling_policy.py index 32f4a23b4..7e77992ce 100644 --- a/heat/engine/resources/openstack/heat/scaling_policy.py +++ b/heat/engine/resources/openstack/heat/scaling_policy.py @@ -16,6 +16,7 @@ import six from heat.common import exception from heat.common.i18n import _ +from heat.common.i18n import _LE from heat.common.i18n import _LI from heat.engine import attributes from heat.engine import constraints @@ -161,37 +162,48 @@ class AutoScalingPolicy(signal_responder.SignalResponder, if alarm_state != 'alarm': raise exception.NoActionRequired() - if not self._is_scaling_allowed(): - LOG.info(_LI("%(name)s NOT performing scaling action, " - "cooldown %(cooldown)s"), - {'name': self.name, - 'cooldown': self.properties[self.COOLDOWN]}) - raise exception.NoActionRequired() asgn_id = self.properties[self.AUTO_SCALING_GROUP_NAME] group = self.stack.resource_by_refid(asgn_id) - changed_size = False - try: - if group is None: - raise exception.NotFound(_('Alarm %(alarm)s could not find ' - 'scaling group named "%(group)s"' - ) % {'alarm': self.name, - 'group': asgn_id}) - LOG.info(_LI('%(name)s Alarm, adjusting Group %(group)s with id ' - '%(asgn_id)s by %(filter)s'), - {'name': self.name, 'group': group.name, - 'asgn_id': asgn_id, - 'filter': self.properties[self.SCALING_ADJUSTMENT]}) - changed_size = group.adjust( + if group is None: + raise exception.NotFound(_('Alarm %(alarm)s could not find ' + 'scaling group named "%(group)s"' + ) % {'alarm': self.name, + 'group': asgn_id}) + + if not self._is_scaling_allowed(): + LOG.info(_LI("%(name)s NOT performing scaling action, " + "cooldown %(cooldown)s") % { + 'name': self.name, + 'cooldown': self.properties[self.COOLDOWN]}) + raise exception.NoActionRequired() + + LOG.info(_LI('%(name)s alarm, adjusting group %(group)s with id ' + '%(asgn_id)s by %(filter)s') % { + 'name': self.name, 'group': group.name, + 'asgn_id': asgn_id, + 'filter': self.properties[self.SCALING_ADJUSTMENT]}) + + size_changed = False + try: + group.adjust( self.properties[self.SCALING_ADJUSTMENT], self.properties[self.ADJUSTMENT_TYPE], self.properties[self.MIN_ADJUSTMENT_STEP]) + size_changed = True + except Exception as ex: + if not isinstance(ex, exception.NoActionRequired): + LOG.error(_LE("Error in performing scaling adjustment with " + "%(name)s alarm for group %(group)s.") % { + 'name': self.name, + 'group': group.name}) + raise finally: self._finished_scaling("%s : %s" % ( self.properties[self.ADJUSTMENT_TYPE], self.properties[self.SCALING_ADJUSTMENT]), - changed_size=changed_size) + size_changed=size_changed) def _resolve_attribute(self, name): if self.resource_id is None: diff --git a/heat/scaling/cooldown.py b/heat/scaling/cooldown.py index effa4e6b3..b4517c1f6 100644 --- a/heat/scaling/cooldown.py +++ b/heat/scaling/cooldown.py @@ -49,13 +49,12 @@ class CooldownMixin(object): self.metadata_set(metadata) return True - def _finished_scaling(self, cooldown_reason, - changed_size=True): + def _finished_scaling(self, cooldown_reason, size_changed=True): # If we wanted to implement the AutoScaling API like AWS does, # we could maintain event history here, but since we only need # the latest event for cooldown, just store that for now metadata = self.metadata_get() - if changed_size: + if size_changed: now = timeutils.utcnow().isoformat() metadata['cooldown'] = {now: cooldown_reason} metadata['scaling_in_progress'] = False diff --git a/heat/tests/autoscaling/test_heat_scaling_group.py b/heat/tests/autoscaling/test_heat_scaling_group.py index 0e8900212..a3fa74a37 100644 --- a/heat/tests/autoscaling/test_heat_scaling_group.py +++ b/heat/tests/autoscaling/test_heat_scaling_group.py @@ -114,7 +114,7 @@ class TestGroupAdjust(common.HeatTestCase): def test_scaling_policy_cooldown_toosoon(self): """If _is_scaling_allowed() returns False don't progress.""" - dont_call = self.patchobject(grouputils, 'get_size') + dont_call = self.patchobject(self.group, 'resize') self.patchobject(self.group, '_is_scaling_allowed', return_value=False) self.assertRaises(exception.NoActionRequired, @@ -122,35 +122,18 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual([], dont_call.call_args_list) def test_scaling_same_capacity(self): - """Alway resize even if the capacity is the same.""" + """Don't resize when capacity is the same.""" self.patchobject(grouputils, 'get_size', return_value=3) resize = self.patchobject(self.group, 'resize') finished_scaling = self.patchobject(self.group, '_finished_scaling') notify = self.patch('heat.engine.notification.autoscaling.send') - self.patchobject(self.group, '_is_scaling_allowed', - return_value=True) - self.group.adjust(3, adjustment_type='ExactCapacity') - - expected_notifies = [ - mock.call( - capacity=3, suffix='start', - adjustment_type='ExactCapacity', - groupname=u'my-group', - message=u'Start resizing the group my-group', - adjustment=3, - stack=self.group.stack), - mock.call( - capacity=3, suffix='end', - adjustment_type='ExactCapacity', - groupname=u'my-group', - message=u'End resizing the group my-group', - adjustment=3, - stack=self.group.stack)] - + self.assertRaises(exception.NoActionRequired, + self.group.adjust, 3, + adjustment_type='ExactCapacity') + expected_notifies = [] self.assertEqual(expected_notifies, notify.call_args_list) - resize.assert_called_once_with(3) - finished_scaling.assert_called_once_with('ExactCapacity : 3', - changed_size=False) + self.assertEqual(0, resize.call_count) + self.assertEqual(0, finished_scaling.call_count) def test_scale_up_min_adjustment(self): self.patchobject(grouputils, 'get_size', return_value=1) @@ -181,7 +164,8 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual(expected_notifies, notify.call_args_list) resize.assert_called_once_with(3) finished_scaling.assert_called_once_with( - 'PercentChangeInCapacity : 33', changed_size=True) + 'PercentChangeInCapacity : 33', + size_changed=True) def test_scale_down_min_adjustment(self): self.patchobject(grouputils, 'get_size', return_value=3) @@ -212,7 +196,8 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual(expected_notifies, notify.call_args_list) resize.assert_called_once_with(1) finished_scaling.assert_called_once_with( - 'PercentChangeInCapacity : -33', changed_size=True) + 'PercentChangeInCapacity : -33', + size_changed=True) def test_scaling_policy_cooldown_ok(self): self.patchobject(grouputils, 'get_size', return_value=0) @@ -241,7 +226,7 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual(expected_notifies, notify.call_args_list) resize.assert_called_once_with(1) finished_scaling.assert_called_once_with('ChangeInCapacity : 1', - changed_size=True) + size_changed=True) grouputils.get_size.assert_called_once_with(self.group) def test_scaling_policy_resize_fail(self): diff --git a/heat/tests/autoscaling/test_heat_scaling_policy.py b/heat/tests/autoscaling/test_heat_scaling_policy.py index 56c67ff0b..244fc2b66 100644 --- a/heat/tests/autoscaling/test_heat_scaling_policy.py +++ b/heat/tests/autoscaling/test_heat_scaling_policy.py @@ -74,6 +74,38 @@ class TestAutoScalingPolicy(common.HeatTestCase): self.assertIn('Alarm my-policy could ' 'not find scaling group', six.text_type(ex)) + def test_scaling_policy_adjust_no_action(self): + t = template_format.parse(as_template) + stack = utils.parse_stack(t, params=as_params) + up_policy = self.create_scaling_policy(t, stack, + 'my-policy') + group = stack['my-group'] + self.patchobject(group, 'adjust', + side_effect=exception.NoActionRequired()) + mock_fin_scaling = self.patchobject(up_policy, '_finished_scaling') + with mock.patch.object(up_policy, '_is_scaling_allowed', + return_value=True) as mock_isa: + self.assertRaises(exception.NoActionRequired, + up_policy.handle_signal) + mock_isa.assert_called_once_with() + mock_fin_scaling.assert_called_once_with('change_in_capacity : 1', + size_changed=False) + + def test_scaling_policy_adjust_size_changed(self): + t = template_format.parse(as_template) + stack = utils.parse_stack(t, params=as_params) + up_policy = self.create_scaling_policy(t, stack, + 'my-policy') + group = stack['my-group'] + self.patchobject(group, 'adjust') + mock_fin_scaling = self.patchobject(up_policy, '_finished_scaling') + with mock.patch.object(up_policy, '_is_scaling_allowed', + return_value=True) as mock_isa: + self.assertIsNone(up_policy.handle_signal()) + mock_isa.assert_called_once_with() + mock_fin_scaling.assert_called_once_with('change_in_capacity : 1', + size_changed=True) + def test_scaling_policy_not_alarm_state(self): """If the details don't have 'alarm' then don't progress.""" t = template_format.parse(as_template) @@ -92,9 +124,10 @@ class TestAutoScalingPolicy(common.HeatTestCase): t = template_format.parse(as_template) stack = utils.parse_stack(t, params=as_params) pol = self.create_scaling_policy(t, stack, 'my-policy') + group = stack['my-group'] test = {'current': 'alarm'} - with mock.patch.object(pol.stack, 'resource_by_refid', + with mock.patch.object(group, 'adjust', side_effect=AssertionError) as dont_call: with mock.patch.object(pol, '_is_scaling_allowed', return_value=False) as mock_cip: @@ -226,7 +259,7 @@ class TestCooldownMixin(common.HeatTestCase): reason = 'cool as' meta_set = self.patchobject(pol, 'metadata_set') self.patchobject(timeutils, 'utcnow', return_value=nowish) - pol._finished_scaling(reason) + pol._finished_scaling(reason, size_changed=True) meta_set.assert_called_once_with( {'cooldown': {nowish.isoformat(): reason}, 'scaling_in_progress': False}) diff --git a/heat/tests/autoscaling/test_scaling_group.py b/heat/tests/autoscaling/test_scaling_group.py index e2f2adc6b..a669fd718 100644 --- a/heat/tests/autoscaling/test_scaling_group.py +++ b/heat/tests/autoscaling/test_scaling_group.py @@ -301,7 +301,7 @@ class TestGroupAdjust(common.HeatTestCase): def test_scaling_policy_cooldown_toosoon(self): """If _is_scaling_allowed() returns False don't progress.""" - dont_call = self.patchobject(grouputils, 'get_size') + dont_call = self.patchobject(self.group, 'resize') self.patchobject(self.group, '_is_scaling_allowed', return_value=False) self.assertRaises(exception.NoActionRequired, @@ -309,35 +309,18 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual([], dont_call.call_args_list) def test_scaling_same_capacity(self): - """Alway resize even if the capacity is the same.""" + """Don't resize when capacity is the same.""" self.patchobject(grouputils, 'get_size', return_value=3) resize = self.patchobject(self.group, 'resize') finished_scaling = self.patchobject(self.group, '_finished_scaling') notify = self.patch('heat.engine.notification.autoscaling.send') - self.patchobject(self.group, '_is_scaling_allowed', - return_value=True) - self.group.adjust(3, adjustment_type='ExactCapacity') - - expected_notifies = [ - mock.call( - capacity=3, suffix='start', - adjustment_type='ExactCapacity', - groupname=u'WebServerGroup', - message=u'Start resizing the group WebServerGroup', - adjustment=3, - stack=self.group.stack), - mock.call( - capacity=3, suffix='end', - adjustment_type='ExactCapacity', - groupname=u'WebServerGroup', - message=u'End resizing the group WebServerGroup', - adjustment=3, - stack=self.group.stack)] - + self.assertRaises(exception.NoActionRequired, + self.group.adjust, 3, + adjustment_type='ExactCapacity') + expected_notifies = [] self.assertEqual(expected_notifies, notify.call_args_list) - resize.assert_called_once_with(3) - finished_scaling.assert_called_once_with('ExactCapacity : 3', - changed_size=False) + self.assertEqual(0, resize.call_count) + self.assertEqual(0, finished_scaling.call_count) def test_scale_up_min_adjustment(self): self.patchobject(grouputils, 'get_size', return_value=1) @@ -368,7 +351,8 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual(expected_notifies, notify.call_args_list) resize.assert_called_once_with(3) finished_scaling.assert_called_once_with( - 'PercentChangeInCapacity : 33', changed_size=True) + 'PercentChangeInCapacity : 33', + size_changed=True) def test_scale_down_min_adjustment(self): self.patchobject(grouputils, 'get_size', return_value=5) @@ -399,7 +383,8 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual(expected_notifies, notify.call_args_list) resize.assert_called_once_with(3) finished_scaling.assert_called_once_with( - 'PercentChangeInCapacity : -33', changed_size=True) + 'PercentChangeInCapacity : -33', + size_changed=True) def test_scaling_policy_cooldown_ok(self): self.patchobject(grouputils, 'get_size', return_value=0) @@ -428,7 +413,7 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual(expected_notifies, notify.call_args_list) resize.assert_called_once_with(1) finished_scaling.assert_called_once_with('ChangeInCapacity : 1', - changed_size=True) + size_changed=True) grouputils.get_size.assert_called_once_with(self.group) def test_scaling_policy_resize_fail(self): diff --git a/heat/tests/autoscaling/test_scaling_policy.py b/heat/tests/autoscaling/test_scaling_policy.py index 3931a5b74..7e39f79ed 100644 --- a/heat/tests/autoscaling/test_scaling_policy.py +++ b/heat/tests/autoscaling/test_scaling_policy.py @@ -80,6 +80,38 @@ class TestAutoScalingPolicy(common.HeatTestCase): self.assertIn('Alarm WebServerScaleUpPolicy could ' 'not find scaling group', six.text_type(ex)) + def test_scaling_policy_adjust_no_action(self): + t = template_format.parse(as_template) + stack = utils.parse_stack(t, params=as_params) + up_policy = self.create_scaling_policy(t, stack, + 'WebServerScaleUpPolicy') + group = stack['WebServerGroup'] + self.patchobject(group, 'adjust', + side_effect=exception.NoActionRequired()) + mock_fin_scaling = self.patchobject(up_policy, '_finished_scaling') + with mock.patch.object(up_policy, '_is_scaling_allowed', + return_value=True) as mock_isa: + self.assertRaises(exception.NoActionRequired, + up_policy.handle_signal) + mock_isa.assert_called_once_with() + mock_fin_scaling.assert_called_once_with('ChangeInCapacity : 1', + size_changed=False) + + def test_scaling_policy_adjust_size_changed(self): + t = template_format.parse(as_template) + stack = utils.parse_stack(t, params=as_params) + up_policy = self.create_scaling_policy(t, stack, + 'WebServerScaleUpPolicy') + group = stack['WebServerGroup'] + self.patchobject(group, 'adjust') + mock_fin_scaling = self.patchobject(up_policy, '_finished_scaling') + with mock.patch.object(up_policy, '_is_scaling_allowed', + return_value=True) as mock_isa: + self.assertIsNone(up_policy.handle_signal()) + mock_isa.assert_called_once_with() + mock_fin_scaling.assert_called_once_with('ChangeInCapacity : 1', + size_changed=True) + def test_scaling_policy_not_alarm_state(self): """If the details don't have 'alarm' then don't progress.""" t = template_format.parse(as_template) @@ -98,9 +130,10 @@ class TestAutoScalingPolicy(common.HeatTestCase): t = template_format.parse(as_template) stack = utils.parse_stack(t, params=as_params) pol = self.create_scaling_policy(t, stack, 'WebServerScaleUpPolicy') + group = stack['WebServerGroup'] test = {'current': 'alarm'} - with mock.patch.object(pol.stack, 'resource_by_refid', + with mock.patch.object(group, 'adjust', side_effect=AssertionError) as dont_call: with mock.patch.object(pol, '_is_scaling_allowed', return_value=False) as mock_isa: