Merge "Refactor CooldownMixin"

This commit is contained in:
Zuul 2018-01-12 21:19:24 +00:00 committed by Gerrit Code Review
commit 67b9f75d74
7 changed files with 114 additions and 91 deletions

View File

@ -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):

View File

@ -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):

View File

@ -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)

View File

@ -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)

View File

@ -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})

View File

@ -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)

View File

@ -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})