openstack/deb-heat
Code Issues Proposed changes

Don't try scaling when no change in size

Browse Source 
Don't resize and send notifications when there is no
change in size i.e. max/min has been reached.

This also refactors some exception handling logic.

Change-Id: I1bb226b3067178dbdab2947609c53f3434aff9fe
Related-Bug: #1555748
This commit is contained in:
Rabi Mishra 2016-03-19 19:55:55 +05:30
parent 7933a8bc3d
commit 480271d896
7 changed files with 167 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
heat/engine/resources/aws/autoscaling/autoscaling_group.py
View File

@ -281,19 +281,22 @@ class AutoScalingGroup(instgrp.InstanceGroup, cooldown.CooldownMixin):
adjustment_type=sc_util.CFN_CHANGE_IN_CAPACITY, adjustment_type=sc_util.CFN_CHANGE_IN_CAPACITY,
min_adjustment_step=None): min_adjustment_step=None):
"""Adjust the size of the scaling group if the cooldown permits.""" """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) capacity = grouputils.get_size(self)
new_capacity = self._get_new_capacity(capacity, adjustment, new_capacity = self._get_new_capacity(capacity, adjustment,
adjustment_type, adjustment_type,
min_adjustment_step) 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. # send a notification before, on-error and on-success.
notif = { notif = {
'stack': self.stack, 'stack': self.stack,
@ -305,6 +308,8 @@ class AutoScalingGroup(instgrp.InstanceGroup, cooldown.CooldownMixin):
'group': self.FnGetRefId()}, 'group': self.FnGetRefId()},
'suffix': 'start', 'suffix': 'start',
} }
size_changed = False
try:
notification.send(**notif) notification.send(**notif)
try: try:
self.resize(new_capacity) self.resize(new_capacity)
@ -319,6 +324,7 @@ class AutoScalingGroup(instgrp.InstanceGroup, cooldown.CooldownMixin):
except Exception: except Exception:
LOG.exception(_LE('Failed sending error notification')) LOG.exception(_LE('Failed sending error notification'))
else: else:
size_changed = True
notif.update({ notif.update({
'suffix': 'end', 'suffix': 'end',
'capacity': new_capacity, 'capacity': new_capacity,
@ -326,10 +332,13 @@ class AutoScalingGroup(instgrp.InstanceGroup, cooldown.CooldownMixin):
'group': notif['groupname']}, 'group': notif['groupname']},
}) })
notification.send(**notif) notification.send(**notif)
except Exception:
LOG.error(_LE("Error in performing scaling adjustment for"
"group %s.") % self.name)
raise
finally: finally:
self._finished_scaling("%s : %s" % (adjustment_type, adjustment), self._finished_scaling("%s : %s" % (adjustment_type, adjustment),
changed_size=changed_size) size_changed=size_changed)
return changed_size
def _tags(self): def _tags(self):
"""Add Identifying Tags to all servers in the group. """Add Identifying Tags to all servers in the group.

38
heat/engine/resources/openstack/heat/scaling_policy.py
View File

@ -16,6 +16,7 @@ import six
from heat.common import exception from heat.common import exception
from heat.common.i18n import _ from heat.common.i18n import _
from heat.common.i18n import _LE
from heat.common.i18n import _LI from heat.common.i18n import _LI
from heat.engine import attributes from heat.engine import attributes
from heat.engine import constraints from heat.engine import constraints
@ -161,37 +162,48 @@ class AutoScalingPolicy(signal_responder.SignalResponder,
if alarm_state != 'alarm': if alarm_state != 'alarm':
raise exception.NoActionRequired() 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] asgn_id = self.properties[self.AUTO_SCALING_GROUP_NAME]
group = self.stack.resource_by_refid(asgn_id) group = self.stack.resource_by_refid(asgn_id)
changed_size = False
try:
if group is None: if group is None:
raise exception.NotFound(_('Alarm %(alarm)s could not find ' raise exception.NotFound(_('Alarm %(alarm)s could not find '
'scaling group named "%(group)s"' 'scaling group named "%(group)s"'
) % {'alarm': self.name, ) % {'alarm': self.name,
'group': asgn_id}) 'group': asgn_id})
LOG.info(_LI('%(name)s Alarm, adjusting Group %(group)s with id ' if not self._is_scaling_allowed():
'%(asgn_id)s by %(filter)s'), LOG.info(_LI("%(name)s NOT performing scaling action, "
{'name': self.name, 'group': group.name, "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, 'asgn_id': asgn_id,
'filter': self.properties[self.SCALING_ADJUSTMENT]}) 'filter': self.properties[self.SCALING_ADJUSTMENT]})
changed_size = group.adjust(
size_changed = False
try:
group.adjust(
self.properties[self.SCALING_ADJUSTMENT], self.properties[self.SCALING_ADJUSTMENT],
self.properties[self.ADJUSTMENT_TYPE], self.properties[self.ADJUSTMENT_TYPE],
self.properties[self.MIN_ADJUSTMENT_STEP]) 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: finally:
self._finished_scaling("%s : %s" % ( self._finished_scaling("%s : %s" % (
self.properties[self.ADJUSTMENT_TYPE], self.properties[self.ADJUSTMENT_TYPE],
self.properties[self.SCALING_ADJUSTMENT]), self.properties[self.SCALING_ADJUSTMENT]),
changed_size=changed_size) size_changed=size_changed)
def _resolve_attribute(self, name): def _resolve_attribute(self, name):
if self.resource_id is None: if self.resource_id is None:

5
heat/scaling/cooldown.py
View File

@ -49,13 +49,12 @@ class CooldownMixin(object):
self.metadata_set(metadata) self.metadata_set(metadata)
return True return True
def _finished_scaling(self, cooldown_reason, def _finished_scaling(self, cooldown_reason, size_changed=True):
changed_size=True):
# If we wanted to implement the AutoScaling API like AWS does, # If we wanted to implement the AutoScaling API like AWS does,
# we could maintain event history here, but since we only need # we could maintain event history here, but since we only need
# the latest event for cooldown, just store that for now # the latest event for cooldown, just store that for now
metadata = self.metadata_get() metadata = self.metadata_get()
if changed_size: if size_changed:
now = timeutils.utcnow().isoformat() now = timeutils.utcnow().isoformat()
metadata['cooldown'] = {now: cooldown_reason} metadata['cooldown'] = {now: cooldown_reason}
metadata['scaling_in_progress'] = False metadata['scaling_in_progress'] = False

41
heat/tests/autoscaling/test_heat_scaling_group.py
View File

@ -114,7 +114,7 @@ class TestGroupAdjust(common.HeatTestCase):
def test_scaling_policy_cooldown_toosoon(self): def test_scaling_policy_cooldown_toosoon(self):
"""If _is_scaling_allowed() returns False don't progress.""" """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', self.patchobject(self.group, '_is_scaling_allowed',
return_value=False) return_value=False)
self.assertRaises(exception.NoActionRequired, self.assertRaises(exception.NoActionRequired,
@ -122,35 +122,18 @@ class TestGroupAdjust(common.HeatTestCase):
self.assertEqual([], dont_call.call_args_list) self.assertEqual([], dont_call.call_args_list)
def test_scaling_same_capacity(self): 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) self.patchobject(grouputils, 'get_size', return_value=3)
resize = self.patchobject(self.group, 'resize') resize = self.patchobject(self.group, 'resize')
finished_scaling = self.patchobject(self.group, '_finished_scaling') finished_scaling = self.patchobject(self.group, '_finished_scaling')
notify = self.patch('heat.engine.notification.autoscaling.send') notify = self.patch('heat.engine.notification.autoscaling.send')
self.patchobject(self.group, '_is_scaling_allowed', self.assertRaises(exception.NoActionRequired,
return_value=True) self.group.adjust, 3,
self.group.adjust(3, adjustment_type='ExactCapacity') adjustment_type='ExactCapacity')
expected_notifies = []
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.assertEqual(expected_notifies, notify.call_args_list) self.assertEqual(expected_notifies, notify.call_args_list)
resize.assert_called_once_with(3) self.assertEqual(0, resize.call_count)
finished_scaling.assert_called_once_with('ExactCapacity : 3', self.assertEqual(0, finished_scaling.call_count)
changed_size=False)
def test_scale_up_min_adjustment(self): def test_scale_up_min_adjustment(self):
self.patchobject(grouputils, 'get_size', return_value=1) self.patchobject(grouputils, 'get_size', return_value=1)
@ -181,7 +164,8 @@ class TestGroupAdjust(common.HeatTestCase):
self.assertEqual(expected_notifies, notify.call_args_list) self.assertEqual(expected_notifies, notify.call_args_list)
resize.assert_called_once_with(3) resize.assert_called_once_with(3)
finished_scaling.assert_called_once_with( finished_scaling.assert_called_once_with(
'PercentChangeInCapacity : 33', changed_size=True) 'PercentChangeInCapacity : 33',
size_changed=True)
def test_scale_down_min_adjustment(self): def test_scale_down_min_adjustment(self):
self.patchobject(grouputils, 'get_size', return_value=3) self.patchobject(grouputils, 'get_size', return_value=3)
@ -212,7 +196,8 @@ class TestGroupAdjust(common.HeatTestCase):
self.assertEqual(expected_notifies, notify.call_args_list) self.assertEqual(expected_notifies, notify.call_args_list)
resize.assert_called_once_with(1) resize.assert_called_once_with(1)
finished_scaling.assert_called_once_with( finished_scaling.assert_called_once_with(
'PercentChangeInCapacity : -33', changed_size=True) 'PercentChangeInCapacity : -33',
size_changed=True)
def test_scaling_policy_cooldown_ok(self): def test_scaling_policy_cooldown_ok(self):
self.patchobject(grouputils, 'get_size', return_value=0) self.patchobject(grouputils, 'get_size', return_value=0)
@ -241,7 +226,7 @@ class TestGroupAdjust(common.HeatTestCase):
self.assertEqual(expected_notifies, notify.call_args_list) self.assertEqual(expected_notifies, notify.call_args_list)
resize.assert_called_once_with(1) resize.assert_called_once_with(1)
finished_scaling.assert_called_once_with('ChangeInCapacity : 1', finished_scaling.assert_called_once_with('ChangeInCapacity : 1',
changed_size=True) size_changed=True)
grouputils.get_size.assert_called_once_with(self.group) grouputils.get_size.assert_called_once_with(self.group)
def test_scaling_policy_resize_fail(self): def test_scaling_policy_resize_fail(self):

37
heat/tests/autoscaling/test_heat_scaling_policy.py
View File

@ -74,6 +74,38 @@ class TestAutoScalingPolicy(common.HeatTestCase):
self.assertIn('Alarm my-policy could ' self.assertIn('Alarm my-policy could '
'not find scaling group', six.text_type(ex)) '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): def test_scaling_policy_not_alarm_state(self):
"""If the details don't have 'alarm' then don't progress.""" """If the details don't have 'alarm' then don't progress."""
t = template_format.parse(as_template) t = template_format.parse(as_template)
@ -92,9 +124,10 @@ class TestAutoScalingPolicy(common.HeatTestCase):
t = template_format.parse(as_template) t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params) stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'my-policy') pol = self.create_scaling_policy(t, stack, 'my-policy')
group = stack['my-group']
test = {'current': 'alarm'} test = {'current': 'alarm'}
with mock.patch.object(pol.stack, 'resource_by_refid', with mock.patch.object(group, 'adjust',
side_effect=AssertionError) as dont_call: side_effect=AssertionError) as dont_call:
with mock.patch.object(pol, '_is_scaling_allowed', with mock.patch.object(pol, '_is_scaling_allowed',
return_value=False) as mock_cip: return_value=False) as mock_cip:
@ -226,7 +259,7 @@ class TestCooldownMixin(common.HeatTestCase):
reason = 'cool as' reason = 'cool as'
meta_set = self.patchobject(pol, 'metadata_set') meta_set = self.patchobject(pol, 'metadata_set')
self.patchobject(timeutils, 'utcnow', return_value=nowish) self.patchobject(timeutils, 'utcnow', return_value=nowish)
pol._finished_scaling(reason) pol._finished_scaling(reason, size_changed=True)
meta_set.assert_called_once_with( meta_set.assert_called_once_with(
{'cooldown': {nowish.isoformat(): reason}, {'cooldown': {nowish.isoformat(): reason},
'scaling_in_progress': False}) 'scaling_in_progress': False})

41
heat/tests/autoscaling/test_scaling_group.py
View File

@ -301,7 +301,7 @@ class TestGroupAdjust(common.HeatTestCase):
def test_scaling_policy_cooldown_toosoon(self): def test_scaling_policy_cooldown_toosoon(self):
"""If _is_scaling_allowed() returns False don't progress.""" """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', self.patchobject(self.group, '_is_scaling_allowed',
return_value=False) return_value=False)
self.assertRaises(exception.NoActionRequired, self.assertRaises(exception.NoActionRequired,
@ -309,35 +309,18 @@ class TestGroupAdjust(common.HeatTestCase):
self.assertEqual([], dont_call.call_args_list) self.assertEqual([], dont_call.call_args_list)
def test_scaling_same_capacity(self): 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) self.patchobject(grouputils, 'get_size', return_value=3)
resize = self.patchobject(self.group, 'resize') resize = self.patchobject(self.group, 'resize')
finished_scaling = self.patchobject(self.group, '_finished_scaling') finished_scaling = self.patchobject(self.group, '_finished_scaling')
notify = self.patch('heat.engine.notification.autoscaling.send') notify = self.patch('heat.engine.notification.autoscaling.send')
self.patchobject(self.group, '_is_scaling_allowed', self.assertRaises(exception.NoActionRequired,
return_value=True) self.group.adjust, 3,
self.group.adjust(3, adjustment_type='ExactCapacity') adjustment_type='ExactCapacity')
expected_notifies = []
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.assertEqual(expected_notifies, notify.call_args_list) self.assertEqual(expected_notifies, notify.call_args_list)
resize.assert_called_once_with(3) self.assertEqual(0, resize.call_count)
finished_scaling.assert_called_once_with('ExactCapacity : 3', self.assertEqual(0, finished_scaling.call_count)
changed_size=False)
def test_scale_up_min_adjustment(self): def test_scale_up_min_adjustment(self):
self.patchobject(grouputils, 'get_size', return_value=1) self.patchobject(grouputils, 'get_size', return_value=1)
@ -368,7 +351,8 @@ class TestGroupAdjust(common.HeatTestCase):
self.assertEqual(expected_notifies, notify.call_args_list) self.assertEqual(expected_notifies, notify.call_args_list)
resize.assert_called_once_with(3) resize.assert_called_once_with(3)
finished_scaling.assert_called_once_with( finished_scaling.assert_called_once_with(
'PercentChangeInCapacity : 33', changed_size=True) 'PercentChangeInCapacity : 33',
size_changed=True)
def test_scale_down_min_adjustment(self): def test_scale_down_min_adjustment(self):
self.patchobject(grouputils, 'get_size', return_value=5) self.patchobject(grouputils, 'get_size', return_value=5)
@ -399,7 +383,8 @@ class TestGroupAdjust(common.HeatTestCase):
self.assertEqual(expected_notifies, notify.call_args_list) self.assertEqual(expected_notifies, notify.call_args_list)
resize.assert_called_once_with(3) resize.assert_called_once_with(3)
finished_scaling.assert_called_once_with( finished_scaling.assert_called_once_with(
'PercentChangeInCapacity : -33', changed_size=True) 'PercentChangeInCapacity : -33',
size_changed=True)
def test_scaling_policy_cooldown_ok(self): def test_scaling_policy_cooldown_ok(self):
self.patchobject(grouputils, 'get_size', return_value=0) self.patchobject(grouputils, 'get_size', return_value=0)
@ -428,7 +413,7 @@ class TestGroupAdjust(common.HeatTestCase):
self.assertEqual(expected_notifies, notify.call_args_list) self.assertEqual(expected_notifies, notify.call_args_list)
resize.assert_called_once_with(1) resize.assert_called_once_with(1)
finished_scaling.assert_called_once_with('ChangeInCapacity : 1', finished_scaling.assert_called_once_with('ChangeInCapacity : 1',
changed_size=True) size_changed=True)
grouputils.get_size.assert_called_once_with(self.group) grouputils.get_size.assert_called_once_with(self.group)
def test_scaling_policy_resize_fail(self): def test_scaling_policy_resize_fail(self):

35
heat/tests/autoscaling/test_scaling_policy.py
View File

@ -80,6 +80,38 @@ class TestAutoScalingPolicy(common.HeatTestCase):
self.assertIn('Alarm WebServerScaleUpPolicy could ' self.assertIn('Alarm WebServerScaleUpPolicy could '
'not find scaling group', six.text_type(ex)) '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): def test_scaling_policy_not_alarm_state(self):
"""If the details don't have 'alarm' then don't progress.""" """If the details don't have 'alarm' then don't progress."""
t = template_format.parse(as_template) t = template_format.parse(as_template)
@ -98,9 +130,10 @@ class TestAutoScalingPolicy(common.HeatTestCase):
t = template_format.parse(as_template) t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params) stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'WebServerScaleUpPolicy') pol = self.create_scaling_policy(t, stack, 'WebServerScaleUpPolicy')
group = stack['WebServerGroup']
test = {'current': 'alarm'} test = {'current': 'alarm'}
with mock.patch.object(pol.stack, 'resource_by_refid', with mock.patch.object(group, 'adjust',
side_effect=AssertionError) as dont_call: side_effect=AssertionError) as dont_call:
with mock.patch.object(pol, '_is_scaling_allowed', with mock.patch.object(pol, '_is_scaling_allowed',
return_value=False) as mock_isa: return_value=False) as mock_isa: