openstack/heat
Code Issues Proposed changes

Remove use of CooldownMixin with scaling policy

Browse Source 
Removes the multiple use of the mixin, so that we don't
set metadata for both policy and group. Setting the cooldown
metadata only for group would suffice.

Change-Id: I241a32b52e0708264c80c3eca313a97534927415
Related-Bug: #1555748
This commit is contained in:
rabi 2017-04-25 13:32:05 +05:30
parent 3a2497a46c
commit 0558b10c48
7 changed files with 109 additions and 151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -19,17 +19,14 @@ from heat.common.i18n import _
from heat.engine import attributes
from heat.engine import constraints
from heat.engine import properties
from heat.engine import resource
from heat.engine.resources import signal_responder
from heat.engine import support
from heat.scaling import cooldown
from heat.scaling import scalingutil as sc_util
LOG = logging.getLogger(__name__)
class AutoScalingPolicy(cooldown.CooldownMixin,
signal_responder.SignalResponder):
class AutoScalingPolicy(signal_responder.SignalResponder):
"""A resource to manage scaling of `OS::Heat::AutoScalingGroup`.
**Note** while it may incidentally support
@ -172,35 +169,18 @@ class AutoScalingPolicy(cooldown.CooldownMixin,
) % {'alarm': self.name,
'group': asgn_id})
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',
{'name': self.name, 'group': group.name,
'asgn_id': asgn_id,
'filter': self.properties[self.SCALING_ADJUSTMENT]})
size_changed = False
try:
with group.frozen_properties():
group.adjust(
self.properties[self.SCALING_ADJUSTMENT],
self.properties[self.ADJUSTMENT_TYPE],
self.properties[self.MIN_ADJUSTMENT_STEP])
size_changed = True
except resource.NoActionRequired:
raise
except Exception:
LOG.error("Error in performing scaling adjustment with "
"%(name)s alarm for group %(group)s.",
{'name': self.name, 'group': group.name})
raise
finally:
self._finished_scaling(
self.properties[self.COOLDOWN],
"%s : %s" % (self.properties[self.ADJUSTMENT_TYPE],
self.properties[self.SCALING_ADJUSTMENT]),
size_changed=size_changed)
with group.frozen_properties():
group.adjust(
self.properties[self.SCALING_ADJUSTMENT],
self.properties[self.ADJUSTMENT_TYPE],
self.properties[self.MIN_ADJUSTMENT_STEP],
self.properties[self.COOLDOWN])
def _resolve_attribute(self, name):
if self.resource_id is None:

1
heat/scaling/cooldown.py
View File

@ -26,7 +26,6 @@ LOG = logging.getLogger(__name__)
class CooldownMixin(object):
"""Utility class to encapsulate Cooldown related logic.
This class is shared between AutoScalingGroup and ScalingPolicy.
This logic includes both cooldown timestamp comparing and scaling in
progress checking.
"""

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

@ -116,6 +116,15 @@ class TestGroupAdjust(common.HeatTestCase):
self.stub_SnapshotConstraint_validate()
self.assertIsNone(self.group.validate())
def test_group_metadata_reset(self):
self.group.state_set('CREATE', 'COMPLETE')
metadata = {'scaling_in_progress': True}
self.group.metadata_set(metadata)
self.group.handle_metadata_reset()
new_metadata = self.group.metadata_get()
self.assertEqual({'scaling_in_progress': False}, new_metadata)
def test_scaling_policy_cooldown_toosoon(self):
dont_call = self.patchobject(self.group, 'resize')
self.patchobject(self.group, '_check_scaling_allowed',

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

@ -69,8 +69,7 @@ class TestAutoScalingPolicy(common.HeatTestCase):
def test_scaling_policy_bad_group(self):
t = template_format.parse(inline_templates.as_heat_template_bad_group)
stack = utils.parse_stack(t)
up_policy = self.create_scaling_policy(t, stack,
'my-policy')
up_policy = self.create_scaling_policy(t, stack, 'my-policy')
ex = self.assertRaises(exception.ResourceFailure, up_policy.signal)
self.assertIn('Alarm my-policy could '
@ -79,30 +78,23 @@ class TestAutoScalingPolicy(common.HeatTestCase):
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')
up_policy = self.create_scaling_policy(t, stack, 'my-policy')
group = stack['my-group']
self.patchobject(group, 'adjust',
side_effect=resource.NoActionRequired())
mock_fin_scaling = self.patchobject(up_policy, '_finished_scaling')
with mock.patch.object(up_policy,
'_check_scaling_allowed') as mock_isa:
self.assertRaises(resource.NoActionRequired,
up_policy.handle_signal)
mock_isa.assert_called_once_with(60)
mock_fin_scaling.assert_called_once_with(60,
'change_in_capacity : 1',
size_changed=False)
self.assertRaises(resource.NoActionRequired,
up_policy.handle_signal)
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')
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,
self.patchobject(group, 'resize')
self.patchobject(group, '_lb_reload')
mock_fin_scaling = self.patchobject(group, '_finished_scaling')
with mock.patch.object(group,
'_check_scaling_allowed') as mock_isa:
self.assertIsNone(up_policy.handle_signal())
mock_isa.assert_called_once_with(60)
@ -117,39 +109,27 @@ class TestAutoScalingPolicy(common.HeatTestCase):
group = stack['my-group']
test = {'current': 'alarm'}
with mock.patch.object(group, 'adjust',
side_effect=AssertionError) as dont_call:
with mock.patch.object(
pol, '_check_scaling_allowed',
side_effect=resource.NoActionRequired) as mock_cip:
self.assertRaises(resource.NoActionRequired,
pol.handle_signal, details=test)
mock_cip.assert_called_once_with(60)
self.assertEqual([], dont_call.call_args_list)
def test_policy_metadata_reset(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'my-policy')
metadata = {'scaling_in_progress': True}
pol.metadata_set(metadata)
pol.handle_metadata_reset()
new_metadata = pol.metadata_get()
self.assertEqual({'scaling_in_progress': False}, new_metadata)
with mock.patch.object(
group, '_check_scaling_allowed',
side_effect=resource.NoActionRequired) as mock_cip:
self.assertRaises(resource.NoActionRequired,
pol.handle_signal, details=test)
mock_cip.assert_called_once_with(60)
def test_scaling_policy_cooldown_ok(self):
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'}
self.patchobject(group, '_finished_scaling')
self.patchobject(group, '_lb_reload')
mock_resize = self.patchobject(group, 'resize')
group = self.patchobject(pol.stack, 'resource_by_refid').return_value
group.name = 'fluffy'
with mock.patch.object(pol, '_check_scaling_allowed') as mock_isa:
with mock.patch.object(group, '_check_scaling_allowed') as mock_isa:
pol.handle_signal(details=test)
mock_isa.assert_called_once_with(60)
group.adjust.assert_called_once_with(1, 'change_in_capacity', None)
mock_resize.assert_called_once_with(1)
def test_scaling_policy_refid(self):
t = template_format.parse(as_template)
@ -173,45 +153,42 @@ class TestAutoScalingPolicy(common.HeatTestCase):
class TestCooldownMixin(common.HeatTestCase):
def create_scaling_policy(self, t, stack, resource_name):
def create_scaling_group(self, t, stack, resource_name):
stack.store()
rsrc = stack[resource_name]
self.assertIsNone(rsrc.validate())
scheduler.TaskRunner(rsrc.create)()
self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state)
rsrc.state_set('CREATE', 'COMPLETE')
return rsrc
def test_cooldown_is_in_progress_toosoon(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'my-policy')
group = self.create_scaling_group(t, stack, 'my-group')
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)
self.patchobject(group, 'metadata_get', return_value=previous_meta)
ex = self.assertRaises(resource.NoActionRequired,
pol._check_scaling_allowed,
properties['cooldown'])
group._check_scaling_allowed,
60)
self.assertIn('due to cooldown', six.text_type(ex))
def test_cooldown_is_in_progress_scaling_unfinished(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'my-policy')
group = self.create_scaling_group(t, stack, 'my-group')
properties = t['resources']['my-policy']['properties']
previous_meta = {'scaling_in_progress': True}
self.patchobject(pol, 'metadata_get', return_value=previous_meta)
self.patchobject(group, 'metadata_get', return_value=previous_meta)
ex = self.assertRaises(resource.NoActionRequired,
pol._check_scaling_allowed,
properties['cooldown'])
group._check_scaling_allowed,
60)
self.assertIn('due to scaling activity', six.text_type(ex))
def test_cooldown_not_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')
group = self.create_scaling_group(t, stack, 'my-group')
awhile_ago = timeutils.utcnow() - datetime.timedelta(seconds=100)
previous_meta = {
@ -220,54 +197,54 @@ class TestCooldownMixin(common.HeatTestCase):
},
'scaling_in_progress': False
}
self.patchobject(pol, 'metadata_get', return_value=previous_meta)
self.assertIsNone(pol._check_scaling_allowed(60))
self.patchobject(group, 'metadata_get', return_value=previous_meta)
self.assertIsNone(group._check_scaling_allowed(60))
def test_scaling_policy_cooldown_zero(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'my-policy')
group = self.create_scaling_group(t, stack, 'my-group')
now = timeutils.utcnow()
previous_meta = {'cooldown': {
now.isoformat(): 'change_in_capacity : 1'}}
self.patchobject(pol, 'metadata_get', return_value=previous_meta)
self.assertIsNone(pol._check_scaling_allowed(0))
self.patchobject(group, 'metadata_get', return_value=previous_meta)
self.assertIsNone(group._check_scaling_allowed(0))
def test_scaling_policy_cooldown_none(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'my-policy')
group = self.create_scaling_group(t, stack, 'my-group')
now = timeutils.utcnow()
previous_meta = {'cooldown': {
now.isoformat(): 'change_in_capacity : 1'}}
self.patchobject(pol, 'metadata_get', return_value=previous_meta)
self.assertIsNone(pol._check_scaling_allowed(None))
self.patchobject(group, 'metadata_get', return_value=previous_meta)
self.assertIsNone(group._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')
group = self.create_scaling_group(t, stack, 'my-group')
# no cooldown entry in the metadata
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(60))
self.patchobject(group, 'metadata_get', return_value=previous_meta)
self.assertIsNone(group._check_scaling_allowed(60))
def test_metadata_is_written(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'my-policy')
group = self.create_scaling_group(t, stack, 'my-group')
nowish = timeutils.utcnow()
reason = 'cool as'
meta_set = self.patchobject(pol, 'metadata_set')
meta_set = self.patchobject(group, 'metadata_set')
self.patchobject(timeutils, 'utcnow', return_value=nowish)
pol._finished_scaling(0, reason, size_changed=True)
group._finished_scaling(0, reason, size_changed=True)
meta_set.assert_called_once_with(
{'cooldown_end': {nowish.isoformat(): reason},
'scaling_in_progress': False})

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

@ -87,15 +87,9 @@ class TestAutoScalingPolicy(common.HeatTestCase):
group = stack['WebServerGroup']
self.patchobject(group, 'adjust',
side_effect=resource.NoActionRequired())
mock_fin_scaling = self.patchobject(up_policy, '_finished_scaling')
with mock.patch.object(up_policy,
'_check_scaling_allowed') as mock_isa:
self.assertRaises(resource.NoActionRequired,
up_policy.handle_signal)
mock_isa.assert_called_once_with(60)
mock_fin_scaling.assert_called_once_with(60,
'ChangeInCapacity : 1',
size_changed=False)
mock_fin_scaling = self.patchobject(group, '_finished_scaling')
self.assertRaises(resource.NoActionRequired, up_policy.handle_signal)
self.assertEqual(0, mock_fin_scaling.call_count)
def test_scaling_policy_adjust_size_changed(self):
t = template_format.parse(as_template)
@ -103,9 +97,10 @@ class TestAutoScalingPolicy(common.HeatTestCase):
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,
self.patchobject(group, 'resize')
self.patchobject(group, '_lb_reload')
mock_fin_scaling = self.patchobject(group, '_finished_scaling')
with mock.patch.object(group,
'_check_scaling_allowed') as mock_isa:
self.assertIsNone(up_policy.handle_signal())
mock_isa.assert_called_once_with(60)
@ -120,28 +115,27 @@ class TestAutoScalingPolicy(common.HeatTestCase):
group = stack['WebServerGroup']
test = {'current': 'alarm'}
with mock.patch.object(group, 'adjust',
side_effect=AssertionError) as dont_call:
with mock.patch.object(
pol, '_check_scaling_allowed',
side_effect=resource.NoActionRequired) as mock_isa:
self.assertRaises(resource.NoActionRequired,
pol.handle_signal, details=test)
mock_isa.assert_called_once_with(60)
self.assertEqual([], dont_call.call_args_list)
with mock.patch.object(
group, '_check_scaling_allowed',
side_effect=resource.NoActionRequired) as mock_isa:
self.assertRaises(resource.NoActionRequired,
pol.handle_signal, details=test)
mock_isa.assert_called_once_with(60)
def test_scaling_policy_cooldown_ok(self):
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'}
self.patchobject(group, '_finished_scaling')
self.patchobject(group, '_lb_reload')
mock_resize = self.patchobject(group, 'resize')
group = self.patchobject(pol.stack, 'resource_by_refid').return_value
group.name = 'fluffy'
with mock.patch.object(pol, '_check_scaling_allowed') as mock_isa:
with mock.patch.object(group, '_check_scaling_allowed') as mock_isa:
pol.handle_signal(details=test)
mock_isa.assert_called_once_with(60)
group.adjust.assert_called_once_with(1, 'ChangeInCapacity', None)
mock_resize.assert_called_once_with(1)
@mock.patch.object(aws_sp.AWSScalingPolicy, '_get_ec2_signed_url')
def test_scaling_policy_refid_signed_url(self, mock_get_ec2_url):
@ -173,42 +167,41 @@ class TestAutoScalingPolicy(common.HeatTestCase):
class TestCooldownMixin(common.HeatTestCase):
def create_scaling_policy(self, t, stack, resource_name):
def create_scaling_group(self, t, stack, resource_name):
stack.store()
rsrc = stack[resource_name]
self.assertIsNone(rsrc.validate())
scheduler.TaskRunner(rsrc.create)()
self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state)
rsrc.state_set('CREATE', 'COMPLETE')
return rsrc
def test_cooldown_is_in_progress_toosoon(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'WebServerScaleUpPolicy')
group = self.create_scaling_group(t, stack, 'WebServerGroup')
now = timeutils.utcnow()
previous_meta = {'cooldown': {
now.isoformat(): 'ChangeInCapacity : 1'}}
self.patchobject(pol, 'metadata_get', return_value=previous_meta)
self.patchobject(group, 'metadata_get', return_value=previous_meta)
ex = self.assertRaises(resource.NoActionRequired,
pol._check_scaling_allowed,
group._check_scaling_allowed,
60)
self.assertIn('due to cooldown', six.text_type(ex))
def test_cooldown_is_in_progress_scaling_unfinished(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'WebServerScaleUpPolicy')
group = self.create_scaling_group(t, stack, 'WebServerGroup')
previous_meta = {'scaling_in_progress': True}
self.patchobject(pol, 'metadata_get', return_value=previous_meta)
self.patchobject(group, 'metadata_get', return_value=previous_meta)
ex = self.assertRaises(resource.NoActionRequired,
pol._check_scaling_allowed, 60)
group._check_scaling_allowed, 60)
self.assertIn('due to scaling activity', six.text_type(ex))
def test_cooldown_not_in_progress(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'WebServerScaleUpPolicy')
group = self.create_scaling_group(t, stack, 'WebServerGroup')
awhile_ago = timeutils.utcnow() - datetime.timedelta(seconds=100)
previous_meta = {
@ -217,19 +210,19 @@ class TestCooldownMixin(common.HeatTestCase):
},
'scaling_in_progress': False
}
self.patchobject(pol, 'metadata_get', return_value=previous_meta)
self.assertIsNone(pol._check_scaling_allowed(60))
self.patchobject(group, 'metadata_get', return_value=previous_meta)
self.assertIsNone(group._check_scaling_allowed(60))
def test_scaling_policy_cooldown_zero(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'WebServerScaleUpPolicy')
group = self.create_scaling_group(t, stack, 'WebServerGroup')
now = timeutils.utcnow()
previous_meta = {now.isoformat(): 'ChangeInCapacity : 1'}
self.patchobject(pol, 'metadata_get', return_value=previous_meta)
self.assertIsNone(pol._check_scaling_allowed(0))
self.patchobject(group, 'metadata_get', return_value=previous_meta)
self.assertIsNone(group._check_scaling_allowed(0))
def test_scaling_policy_cooldown_none(self):
t = template_format.parse(as_template)
@ -240,23 +233,23 @@ class TestCooldownMixin(common.HeatTestCase):
del properties['Cooldown']
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'WebServerScaleUpPolicy')
group = self.create_scaling_group(t, stack, 'WebServerGroup')
now = timeutils.utcnow()
previous_meta = {now.isoformat(): 'ChangeInCapacity : 1'}
self.patchobject(pol, 'metadata_get', return_value=previous_meta)
self.assertIsNone(pol._check_scaling_allowed(None))
self.patchobject(group, 'metadata_get', return_value=previous_meta)
self.assertIsNone(group._check_scaling_allowed(None))
def test_metadata_is_written(self):
t = template_format.parse(as_template)
stack = utils.parse_stack(t, params=as_params)
pol = self.create_scaling_policy(t, stack, 'WebServerScaleUpPolicy')
group = self.create_scaling_group(t, stack, 'WebServerGroup')
nowish = timeutils.utcnow()
reason = 'cool as'
meta_set = self.patchobject(pol, 'metadata_set')
meta_set = self.patchobject(group, 'metadata_set')
self.patchobject(timeutils, 'utcnow', return_value=nowish)
pol._finished_scaling(0, reason)
group._finished_scaling(0, reason)
meta_set.assert_called_once_with(
{'cooldown_end': {nowish.isoformat(): reason},
'scaling_in_progress': False})

4
heat_integrationtests/common/test.py
View File

@ -682,13 +682,13 @@ class HeatIntegrationTest(testscenarios.WithScenarios,
time.sleep(build_interval)
def check_autoscale_complete(self, stack_id, expected_num, parent_stack,
policy):
group_name):
res_list = self.client.resources.list(stack_id)
all_res_complete = all(res.resource_status in ('UPDATE_COMPLETE',
'CREATE_COMPLETE')
for res in res_list)
all_res = len(res_list) == expected_num
if all_res and all_res_complete:
metadata = self.client.resources.metadata(parent_stack, policy)
metadata = self.client.resources.metadata(parent_stack, group_name)
return not metadata.get('scaling_in_progress')
return False

6
heat_integrationtests/functional/test_heat_autoscaling.py
View File

@ -120,7 +120,7 @@ outputs:
self.check_autoscale_complete,
asg.physical_resource_id,
expected_resources, stack_id,
'scale_up_policy'))
'random_group'))
def test_asg_scale_down_min_size(self):
stack_id = self.stack_create(template=self.template,
@ -142,7 +142,7 @@ outputs:
self.check_autoscale_complete,
asg.physical_resource_id,
expected_resources, stack_id,
'scale_down_policy'))
'random_group'))
def test_asg_cooldown(self):
cooldown_tmpl = self.template.replace('cooldown: 0',
@ -165,7 +165,7 @@ outputs:
self.check_autoscale_complete,
asg.physical_resource_id,
expected_resources, stack_id,
'scale_up_policy'))
'random_group'))
def test_path_attrs(self):
stack_id = self.stack_create(template=self.template)