diff --git a/heat/engine/resources/aws/autoscaling/autoscaling_group.py b/heat/engine/resources/aws/autoscaling/autoscaling_group.py index e7613ecf41..481cca7cdd 100644 --- a/heat/engine/resources/aws/autoscaling/autoscaling_group.py +++ b/heat/engine/resources/aws/autoscaling/autoscaling_group.py @@ -223,19 +223,21 @@ class AutoScalingGroup(cooldown.CooldownMixin, instgrp.InstanceGroup): def check_create_complete(self, task): """Update cooldown timestamp after create succeeds.""" done = super(AutoScalingGroup, self).check_create_complete(task) + cooldown = self.properties[self.COOLDOWN] if done: - self._finished_scaling( - "%s : %s" % (sc_util.CFN_EXACT_CAPACITY, - grouputils.get_size(self))) + self._finished_scaling(cooldown, + "%s : %s" % (sc_util.CFN_EXACT_CAPACITY, + grouputils.get_size(self))) return done def check_update_complete(self, cookie): """Update the cooldown timestamp after update succeeds.""" done = super(AutoScalingGroup, self).check_update_complete(cookie) + cooldown = self.properties[self.COOLDOWN] if done: - self._finished_scaling( - "%s : %s" % (sc_util.CFN_EXACT_CAPACITY, - grouputils.get_size(self))) + self._finished_scaling(cooldown, + "%s : %s" % (sc_util.CFN_EXACT_CAPACITY, + grouputils.get_size(self))) return done def _get_new_capacity(self, capacity, @@ -284,7 +286,7 @@ class AutoScalingGroup(cooldown.CooldownMixin, instgrp.InstanceGroup): def adjust(self, adjustment, adjustment_type=sc_util.CFN_CHANGE_IN_CAPACITY, - min_adjustment_step=None): + min_adjustment_step=None, cooldown=None): """Adjust the size of the scaling group if the cooldown permits.""" if self.status != self.COMPLETE: LOG.info("%s NOT performing scaling adjustment, " @@ -300,7 +302,10 @@ class AutoScalingGroup(cooldown.CooldownMixin, instgrp.InstanceGroup): "as there is no change in capacity.", self.name) raise resource.NoActionRequired - self._check_scaling_allowed() + if cooldown is None: + cooldown = self.properties[self.COOLDOWN] + + self._check_scaling_allowed(cooldown) # send a notification before, on-error and on-success. notif = { @@ -342,7 +347,8 @@ class AutoScalingGroup(cooldown.CooldownMixin, instgrp.InstanceGroup): "group %s.", self.name) raise finally: - self._finished_scaling("%s : %s" % (adjustment_type, adjustment), + self._finished_scaling(cooldown, + "%s : %s" % (adjustment_type, adjustment), size_changed=size_changed) def _tags(self): diff --git a/heat/engine/resources/openstack/heat/scaling_policy.py b/heat/engine/resources/openstack/heat/scaling_policy.py index 90738f6885..aa40e7b1de 100644 --- a/heat/engine/resources/openstack/heat/scaling_policy.py +++ b/heat/engine/resources/openstack/heat/scaling_policy.py @@ -172,7 +172,7 @@ class AutoScalingPolicy(cooldown.CooldownMixin, ) % {'alarm': self.name, 'group': asgn_id}) - self._check_scaling_allowed() + self._check_scaling_allowed(self.properties[self.COOLDOWN]) LOG.info('%(name)s alarm, adjusting group %(group)s with id ' '%(asgn_id)s by %(filter)s', @@ -196,9 +196,10 @@ class AutoScalingPolicy(cooldown.CooldownMixin, {'name': self.name, 'group': group.name}) raise finally: - self._finished_scaling("%s : %s" % ( - self.properties[self.ADJUSTMENT_TYPE], - self.properties[self.SCALING_ADJUSTMENT]), + self._finished_scaling( + self.properties[self.COOLDOWN], + "%s : %s" % (self.properties[self.ADJUSTMENT_TYPE], + self.properties[self.SCALING_ADJUSTMENT]), size_changed=size_changed) def _resolve_attribute(self, name): diff --git a/heat/scaling/cooldown.py b/heat/scaling/cooldown.py index e3724856a3..5fbab16b2b 100644 --- a/heat/scaling/cooldown.py +++ b/heat/scaling/cooldown.py @@ -11,11 +11,12 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_log import log as logging +import datetime from heat.common import exception from heat.common.i18n import _ from heat.engine import resource +from oslo_log import log as logging from oslo_utils import timeutils import six @@ -29,7 +30,12 @@ class CooldownMixin(object): This logic includes both cooldown timestamp comparing and scaling in progress checking. """ - def _check_scaling_allowed(self): + def _sanitize_cooldown(self, cooldown): + if cooldown is None: + return 0 + return max(0, cooldown) + + def _check_scaling_allowed(self, cooldown): metadata = self.metadata_get() if metadata.get('scaling_in_progress'): LOG.info("Can not perform scaling action: resource %s " @@ -37,51 +43,58 @@ class CooldownMixin(object): reason = _('due to scaling activity') raise resource.NoActionRequired(res_name=self.name, reason=reason) - try: - # Negative values don't make sense, so they are clamped to zero - cooldown = max(0, self.properties[self.COOLDOWN]) - except TypeError: - # If not specified, it will be None, same as cooldown == 0 - cooldown = 0 + cooldown = self._sanitize_cooldown(cooldown) + # if both cooldown and cooldown_end not in metadata + if all(k not in metadata for k in ('cooldown', 'cooldown_end')): + # Note: this is for supporting old version cooldown checking + metadata.pop('scaling_in_progress', None) + if metadata and cooldown != 0: + last_adjust = next(six.iterkeys(metadata)) + if not timeutils.is_older_than(last_adjust, cooldown): + self._log_and_raise_no_action(cooldown) - if cooldown != 0: - try: - if 'cooldown' not in metadata: - # Note: this is for supporting old version cooldown logic - if metadata: - last_adjust = next(six.iterkeys(metadata)) - self._cooldown_check(cooldown, last_adjust) - else: - last_adjust = next(six.iterkeys(metadata['cooldown'])) - self._cooldown_check(cooldown, last_adjust) - except ValueError: - # occurs when metadata has only {scaling_in_progress: False} - pass + elif 'cooldown_end' in metadata: + cooldown_end = next(six.iterkeys(metadata['cooldown_end'])) + now = timeutils.utcnow().isoformat() + if now < cooldown_end: + self._log_and_raise_no_action(cooldown) + + elif cooldown != 0: + # Note: this is also for supporting old version cooldown checking + last_adjust = next(six.iterkeys(metadata['cooldown'])) + if not timeutils.is_older_than(last_adjust, cooldown): + self._log_and_raise_no_action(cooldown) # Assumes _finished_scaling is called # after the scaling operation completes metadata['scaling_in_progress'] = True self.metadata_set(metadata) - def _cooldown_check(self, cooldown, last_adjust): - if not timeutils.is_older_than(last_adjust, cooldown): - LOG.info("Can not perform scaling action: " - "resource %(name)s is in cooldown (%(cooldown)s).", - {'name': self.name, - 'cooldown': cooldown}) - reason = _('due to cooldown, ' - 'cooldown %s') % cooldown - raise resource.NoActionRequired( - res_name=self.name, reason=reason) + def _log_and_raise_no_action(self, cooldown): + LOG.info("Can not perform scaling action: " + "resource %(name)s is in cooldown (%(cooldown)s).", + {'name': self.name, + 'cooldown': cooldown}) + reason = _('due to cooldown, ' + 'cooldown %s') % cooldown + raise resource.NoActionRequired( + res_name=self.name, reason=reason) - def _finished_scaling(self, cooldown_reason, size_changed=True): + def _finished_scaling(self, cooldown, + 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 size_changed: - now = timeutils.utcnow().isoformat() - metadata['cooldown'] = {now: cooldown_reason} + cooldown = self._sanitize_cooldown(cooldown) + cooldown_end = (timeutils.utcnow() + datetime.timedelta( + seconds=cooldown)).isoformat() + if 'cooldown_end' in metadata: + cooldown_end = max( + next(six.iterkeys(metadata['cooldown_end'])), + cooldown_end) + metadata['cooldown_end'] = {cooldown_end: cooldown_reason} metadata['scaling_in_progress'] = False try: self.metadata_set(metadata) diff --git a/heat/tests/autoscaling/test_heat_scaling_group.py b/heat/tests/autoscaling/test_heat_scaling_group.py index e98a0753b1..50e7517b26 100644 --- a/heat/tests/autoscaling/test_heat_scaling_group.py +++ b/heat/tests/autoscaling/test_heat_scaling_group.py @@ -159,6 +159,7 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual(expected_notifies, notify.call_args_list) resize.assert_called_once_with(3) finished_scaling.assert_called_once_with( + None, 'PercentChangeInCapacity : 33', size_changed=True) @@ -190,6 +191,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( + None, 'PercentChangeInCapacity : -33', size_changed=True) @@ -218,7 +220,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('ChangeInCapacity : 1', + finished_scaling.assert_called_once_with(None, + 'ChangeInCapacity : 1', size_changed=True) grouputils.get_size.assert_called_once_with(self.group) diff --git a/heat/tests/autoscaling/test_heat_scaling_policy.py b/heat/tests/autoscaling/test_heat_scaling_policy.py index 65fb7695c9..05a4d4fa75 100644 --- a/heat/tests/autoscaling/test_heat_scaling_policy.py +++ b/heat/tests/autoscaling/test_heat_scaling_policy.py @@ -89,8 +89,9 @@ class TestAutoScalingPolicy(common.HeatTestCase): '_check_scaling_allowed') as mock_isa: self.assertRaises(resource.NoActionRequired, up_policy.handle_signal) - mock_isa.assert_called_once_with() - mock_fin_scaling.assert_called_once_with('change_in_capacity : 1', + mock_isa.assert_called_once_with(60) + mock_fin_scaling.assert_called_once_with(60, + 'change_in_capacity : 1', size_changed=False) def test_scaling_policy_adjust_size_changed(self): @@ -104,8 +105,9 @@ class TestAutoScalingPolicy(common.HeatTestCase): with mock.patch.object(up_policy, '_check_scaling_allowed') 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', + mock_isa.assert_called_once_with(60) + mock_fin_scaling.assert_called_once_with(60, + 'change_in_capacity : 1', size_changed=True) def test_scaling_policy_cooldown_toosoon(self): @@ -122,7 +124,7 @@ class TestAutoScalingPolicy(common.HeatTestCase): side_effect=resource.NoActionRequired) as mock_cip: self.assertRaises(resource.NoActionRequired, pol.handle_signal, details=test) - mock_cip.assert_called_once_with() + mock_cip.assert_called_once_with(60) self.assertEqual([], dont_call.call_args_list) def test_policy_metadata_reset(self): @@ -146,7 +148,7 @@ class TestAutoScalingPolicy(common.HeatTestCase): group.name = 'fluffy' with mock.patch.object(pol, '_check_scaling_allowed') as mock_isa: pol.handle_signal(details=test) - mock_isa.assert_called_once_with() + mock_isa.assert_called_once_with(60) group.adjust.assert_called_once_with(1, 'change_in_capacity', None) def test_scaling_policy_refid(self): @@ -183,12 +185,14 @@ class TestCooldownMixin(common.HeatTestCase): stack = utils.parse_stack(t, params=as_params) pol = self.create_scaling_policy(t, stack, 'my-policy') + properties = t['resources']['my-policy']['properties'] now = timeutils.utcnow() previous_meta = {'cooldown': { now.isoformat(): 'change_in_capacity : 1'}} self.patchobject(pol, 'metadata_get', return_value=previous_meta) ex = self.assertRaises(resource.NoActionRequired, - pol._check_scaling_allowed) + pol._check_scaling_allowed, + properties['cooldown']) self.assertIn('due to cooldown', six.text_type(ex)) def test_cooldown_is_in_progress_scaling_unfinished(self): @@ -196,10 +200,12 @@ class TestCooldownMixin(common.HeatTestCase): stack = utils.parse_stack(t, params=as_params) pol = self.create_scaling_policy(t, stack, 'my-policy') + properties = t['resources']['my-policy']['properties'] previous_meta = {'scaling_in_progress': True} self.patchobject(pol, 'metadata_get', return_value=previous_meta) ex = self.assertRaises(resource.NoActionRequired, - pol._check_scaling_allowed) + pol._check_scaling_allowed, + properties['cooldown']) self.assertIn('due to scaling activity', six.text_type(ex)) def test_cooldown_not_in_progress(self): @@ -215,15 +221,11 @@ class TestCooldownMixin(common.HeatTestCase): 'scaling_in_progress': False } self.patchobject(pol, 'metadata_get', return_value=previous_meta) - self.assertIsNone(pol._check_scaling_allowed()) + self.assertIsNone(pol._check_scaling_allowed(60)) def test_scaling_policy_cooldown_zero(self): t = template_format.parse(as_template) - # Create the scaling policy (with cooldown=0) and scale up one - properties = t['resources']['my-policy']['properties'] - properties['cooldown'] = '0' - stack = utils.parse_stack(t, params=as_params) pol = self.create_scaling_policy(t, stack, 'my-policy') @@ -231,16 +233,11 @@ class TestCooldownMixin(common.HeatTestCase): previous_meta = {'cooldown': { now.isoformat(): 'change_in_capacity : 1'}} self.patchobject(pol, 'metadata_get', return_value=previous_meta) - self.assertIsNone(pol._check_scaling_allowed()) + self.assertIsNone(pol._check_scaling_allowed(0)) def test_scaling_policy_cooldown_none(self): t = template_format.parse(as_template) - # Create the scaling policy no cooldown property, should behave the - # same as when cooldown==0 - properties = t['resources']['my-policy']['properties'] - del properties['cooldown'] - stack = utils.parse_stack(t, params=as_params) pol = self.create_scaling_policy(t, stack, 'my-policy') @@ -248,17 +245,18 @@ class TestCooldownMixin(common.HeatTestCase): previous_meta = {'cooldown': { now.isoformat(): 'change_in_capacity : 1'}} self.patchobject(pol, 'metadata_get', return_value=previous_meta) - self.assertIsNone(pol._check_scaling_allowed()) + self.assertIsNone(pol._check_scaling_allowed(None)) def test_no_cooldown_no_scaling_in_progress(self): t = template_format.parse(as_template) stack = utils.parse_stack(t, params=as_params) pol = self.create_scaling_policy(t, stack, 'my-policy') - # no cooldown entry in the metadata - previous_meta = {'scaling_in_progress': False} + awhile_ago = timeutils.utcnow() - datetime.timedelta(seconds=100) + previous_meta = {'scaling_in_progress': False, + awhile_ago.isoformat(): 'change_in_capacity : 1'} self.patchobject(pol, 'metadata_get', return_value=previous_meta) - self.assertIsNone(pol._check_scaling_allowed()) + self.assertIsNone(pol._check_scaling_allowed(60)) def test_metadata_is_written(self): t = template_format.parse(as_template) @@ -269,9 +267,9 @@ 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, size_changed=True) + pol._finished_scaling(0, reason, size_changed=True) meta_set.assert_called_once_with( - {'cooldown': {nowish.isoformat(): reason}, + {'cooldown_end': {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 233cf5076f..122eec1843 100644 --- a/heat/tests/autoscaling/test_scaling_group.py +++ b/heat/tests/autoscaling/test_scaling_group.py @@ -357,6 +357,7 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual(expected_notifies, notify.call_args_list) resize.assert_called_once_with(3) finished_scaling.assert_called_once_with( + None, 'PercentChangeInCapacity : 33', size_changed=True) @@ -388,6 +389,7 @@ class TestGroupAdjust(common.HeatTestCase): self.assertEqual(expected_notifies, notify.call_args_list) resize.assert_called_once_with(3) finished_scaling.assert_called_once_with( + None, 'PercentChangeInCapacity : -33', size_changed=True) @@ -416,7 +418,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('ChangeInCapacity : 1', + finished_scaling.assert_called_once_with(None, + 'ChangeInCapacity : 1', size_changed=True) grouputils.get_size.assert_called_once_with(self.group) diff --git a/heat/tests/autoscaling/test_scaling_policy.py b/heat/tests/autoscaling/test_scaling_policy.py index dff0fe536a..fc59606343 100644 --- a/heat/tests/autoscaling/test_scaling_policy.py +++ b/heat/tests/autoscaling/test_scaling_policy.py @@ -92,8 +92,9 @@ class TestAutoScalingPolicy(common.HeatTestCase): '_check_scaling_allowed') as mock_isa: self.assertRaises(resource.NoActionRequired, up_policy.handle_signal) - mock_isa.assert_called_once_with() - mock_fin_scaling.assert_called_once_with('ChangeInCapacity : 1', + mock_isa.assert_called_once_with(60) + mock_fin_scaling.assert_called_once_with(60, + 'ChangeInCapacity : 1', size_changed=False) def test_scaling_policy_adjust_size_changed(self): @@ -107,8 +108,9 @@ class TestAutoScalingPolicy(common.HeatTestCase): with mock.patch.object(up_policy, '_check_scaling_allowed') as mock_isa: self.assertIsNone(up_policy.handle_signal()) - mock_isa.assert_called_once_with() - mock_fin_scaling.assert_called_once_with('ChangeInCapacity : 1', + mock_isa.assert_called_once_with(60) + mock_fin_scaling.assert_called_once_with(60, + 'ChangeInCapacity : 1', size_changed=True) def test_scaling_policy_cooldown_toosoon(self): @@ -125,7 +127,7 @@ class TestAutoScalingPolicy(common.HeatTestCase): side_effect=resource.NoActionRequired) as mock_isa: self.assertRaises(resource.NoActionRequired, pol.handle_signal, details=test) - mock_isa.assert_called_once_with() + mock_isa.assert_called_once_with(60) self.assertEqual([], dont_call.call_args_list) def test_scaling_policy_cooldown_ok(self): @@ -138,7 +140,7 @@ class TestAutoScalingPolicy(common.HeatTestCase): group.name = 'fluffy' with mock.patch.object(pol, '_check_scaling_allowed') as mock_isa: pol.handle_signal(details=test) - mock_isa.assert_called_once_with() + mock_isa.assert_called_once_with(60) group.adjust.assert_called_once_with(1, 'ChangeInCapacity', None) @mock.patch.object(aws_sp.AWSScalingPolicy, '_get_ec2_signed_url') @@ -188,7 +190,8 @@ class TestCooldownMixin(common.HeatTestCase): now.isoformat(): 'ChangeInCapacity : 1'}} self.patchobject(pol, 'metadata_get', return_value=previous_meta) ex = self.assertRaises(resource.NoActionRequired, - pol._check_scaling_allowed) + pol._check_scaling_allowed, + 60) self.assertIn('due to cooldown', six.text_type(ex)) def test_cooldown_is_in_progress_scaling_unfinished(self): @@ -199,7 +202,7 @@ class TestCooldownMixin(common.HeatTestCase): previous_meta = {'scaling_in_progress': True} self.patchobject(pol, 'metadata_get', return_value=previous_meta) ex = self.assertRaises(resource.NoActionRequired, - pol._check_scaling_allowed) + pol._check_scaling_allowed, 60) self.assertIn('due to scaling activity', six.text_type(ex)) def test_cooldown_not_in_progress(self): @@ -215,22 +218,18 @@ class TestCooldownMixin(common.HeatTestCase): 'scaling_in_progress': False } self.patchobject(pol, 'metadata_get', return_value=previous_meta) - self.assertIsNone(pol._check_scaling_allowed()) + self.assertIsNone(pol._check_scaling_allowed(60)) def test_scaling_policy_cooldown_zero(self): t = template_format.parse(as_template) - # Create the scaling policy (with Cooldown=0) and scale up one - properties = t['Resources']['WebServerScaleUpPolicy']['Properties'] - properties['Cooldown'] = '0' - stack = utils.parse_stack(t, params=as_params) pol = self.create_scaling_policy(t, stack, 'WebServerScaleUpPolicy') now = timeutils.utcnow() previous_meta = {now.isoformat(): 'ChangeInCapacity : 1'} self.patchobject(pol, 'metadata_get', return_value=previous_meta) - self.assertIsNone(pol._check_scaling_allowed()) + self.assertIsNone(pol._check_scaling_allowed(0)) def test_scaling_policy_cooldown_none(self): t = template_format.parse(as_template) @@ -246,7 +245,7 @@ class TestCooldownMixin(common.HeatTestCase): now = timeutils.utcnow() previous_meta = {now.isoformat(): 'ChangeInCapacity : 1'} self.patchobject(pol, 'metadata_get', return_value=previous_meta) - self.assertIsNone(pol._check_scaling_allowed()) + self.assertIsNone(pol._check_scaling_allowed(None)) def test_metadata_is_written(self): t = template_format.parse(as_template) @@ -257,9 +256,9 @@ 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(0, reason) meta_set.assert_called_once_with( - {'cooldown': {nowish.isoformat(): reason}, + {'cooldown_end': {nowish.isoformat(): reason}, 'scaling_in_progress': False})