Fix update issues with autoscaling group
- Fix update stuck in IN_PROGRESS if issued within the cooldown period after CREATE_COMPLETE. - Fix update stuck in IN_PROGRESS if issued when scaling is happening. Change-Id: I31074783f525ea811056e8df7ff46545e58075b5 Closes-Bug: #1559132
This commit is contained in:
parent
6c25e8302d
commit
6bd7352dc4
|
@ -223,7 +223,7 @@ class AutoScalingGroup(instgrp.InstanceGroup, cooldown.CooldownMixin):
|
|||
return conf, props
|
||||
|
||||
def check_create_complete(self, task):
|
||||
"""Invoke the cooldown after creation succeeds."""
|
||||
"""Update cooldown timestamp after create succeeds."""
|
||||
done = super(AutoScalingGroup, self).check_create_complete(task)
|
||||
if done:
|
||||
self._finished_scaling(
|
||||
|
@ -231,6 +231,26 @@ class AutoScalingGroup(instgrp.InstanceGroup, cooldown.CooldownMixin):
|
|||
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)
|
||||
if done:
|
||||
self._finished_scaling(
|
||||
"%s : %s" % (sc_util.CFN_EXACT_CAPACITY,
|
||||
grouputils.get_size(self)))
|
||||
return done
|
||||
|
||||
def _get_new_capacity(self, capacity,
|
||||
adjustment,
|
||||
adjustment_type=sc_util.CFN_EXACT_CAPACITY,
|
||||
min_adjustment_step=None):
|
||||
lower = self.properties[self.MIN_SIZE]
|
||||
upper = self.properties[self.MAX_SIZE]
|
||||
return sc_util.calculate_new_capacity(capacity, adjustment,
|
||||
adjustment_type,
|
||||
min_adjustment_step,
|
||||
lower, upper)
|
||||
|
||||
def handle_update(self, json_snippet, tmpl_diff, prop_diff):
|
||||
"""Updates self.properties, if Properties has changed.
|
||||
|
||||
|
@ -250,36 +270,28 @@ class AutoScalingGroup(instgrp.InstanceGroup, cooldown.CooldownMixin):
|
|||
# Replace instances first if launch configuration has changed
|
||||
self._try_rolling_update(prop_diff)
|
||||
|
||||
if self.properties[self.DESIRED_CAPACITY] is not None:
|
||||
self.adjust(self.properties[self.DESIRED_CAPACITY],
|
||||
adjustment_type=sc_util.CFN_EXACT_CAPACITY)
|
||||
else:
|
||||
current_capacity = grouputils.get_size(self)
|
||||
self.adjust(current_capacity,
|
||||
adjustment_type=sc_util.CFN_EXACT_CAPACITY)
|
||||
# Update will happen irrespective of whether auto-scaling
|
||||
# is in progress or not.
|
||||
capacity = grouputils.get_size(self)
|
||||
desired_capacity = self.properties[self.DESIRED_CAPACITY] or capacity
|
||||
new_capacity = self._get_new_capacity(capacity, desired_capacity)
|
||||
self.resize(new_capacity)
|
||||
|
||||
def adjust(self, adjustment,
|
||||
adjustment_type=sc_util.CFN_CHANGE_IN_CAPACITY,
|
||||
min_adjustment_step=None, signal=False):
|
||||
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]})
|
||||
if signal:
|
||||
raise exception.NoActionRequired()
|
||||
else:
|
||||
return
|
||||
raise exception.NoActionRequired()
|
||||
|
||||
capacity = grouputils.get_size(self)
|
||||
lower = self.properties[self.MIN_SIZE]
|
||||
upper = self.properties[self.MAX_SIZE]
|
||||
|
||||
new_capacity = sc_util.calculate_new_capacity(capacity, adjustment,
|
||||
adjustment_type,
|
||||
min_adjustment_step,
|
||||
lower, upper)
|
||||
new_capacity = self._get_new_capacity(capacity, adjustment,
|
||||
adjustment_type,
|
||||
min_adjustment_step)
|
||||
|
||||
changed_size = new_capacity != capacity
|
||||
# send a notification before, on-error and on-success.
|
||||
|
|
|
@ -186,8 +186,7 @@ class AutoScalingPolicy(signal_responder.SignalResponder,
|
|||
changed_size = group.adjust(
|
||||
self.properties[self.SCALING_ADJUSTMENT],
|
||||
self.properties[self.ADJUSTMENT_TYPE],
|
||||
self.properties[self.MIN_ADJUSTMENT_STEP],
|
||||
signal=True)
|
||||
self.properties[self.MIN_ADJUSTMENT_STEP])
|
||||
finally:
|
||||
self._finished_scaling("%s : %s" % (
|
||||
self.properties[self.ADJUSTMENT_TYPE],
|
||||
|
|
|
@ -114,19 +114,13 @@ 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')
|
||||
with mock.patch.object(self.group, '_is_scaling_allowed',
|
||||
return_value=False):
|
||||
self.group.adjust(1)
|
||||
self.patchobject(self.group, '_is_scaling_allowed',
|
||||
return_value=False)
|
||||
self.assertRaises(exception.NoActionRequired,
|
||||
self.group.adjust, 1)
|
||||
self.assertEqual([], dont_call.call_args_list)
|
||||
|
||||
def test_scaling_policy_cooldown_toosoon_with_signal(self):
|
||||
with mock.patch.object(self.group, '_is_scaling_allowed',
|
||||
return_value=False):
|
||||
self.assertRaises(exception.NoActionRequired, self.group.adjust, 1,
|
||||
signal=True)
|
||||
|
||||
def test_scaling_same_capacity(self):
|
||||
"""Alway resize even if the capacity is the same."""
|
||||
self.patchobject(grouputils, 'get_size', return_value=3)
|
||||
|
@ -337,9 +331,11 @@ class TestGroupCrud(common.HeatTestCase):
|
|||
|
||||
def test_handle_update_desired_cap(self):
|
||||
self.group._try_rolling_update = mock.Mock(return_value=None)
|
||||
self.group.adjust = mock.Mock(return_value=None)
|
||||
self.group.resize = mock.Mock(return_value=None)
|
||||
|
||||
props = {'desired_capacity': 4}
|
||||
props = {'desired_capacity': 4,
|
||||
'min_size': 0,
|
||||
'max_size': 6}
|
||||
defn = rsrc_defn.ResourceDefinition(
|
||||
'nopayload',
|
||||
'OS::Heat::AutoScalingGroup',
|
||||
|
@ -347,17 +343,17 @@ class TestGroupCrud(common.HeatTestCase):
|
|||
|
||||
self.group.handle_update(defn, None, props)
|
||||
|
||||
self.group.adjust.assert_called_once_with(
|
||||
4, adjustment_type='ExactCapacity')
|
||||
self.group.resize.assert_called_once_with(4)
|
||||
self.group._try_rolling_update.assert_called_once_with(props)
|
||||
|
||||
def test_handle_update_desired_nocap(self):
|
||||
self.group._try_rolling_update = mock.Mock(return_value=None)
|
||||
self.group.adjust = mock.Mock(return_value=None)
|
||||
self.group.resize = mock.Mock(return_value=None)
|
||||
get_size = self.patchobject(grouputils, 'get_size')
|
||||
get_size.return_value = 6
|
||||
|
||||
props = {'Tags': []}
|
||||
props = {'min_size': 0,
|
||||
'max_size': 6}
|
||||
defn = rsrc_defn.ResourceDefinition(
|
||||
'nopayload',
|
||||
'OS::Heat::AutoScalingGroup',
|
||||
|
@ -365,14 +361,13 @@ class TestGroupCrud(common.HeatTestCase):
|
|||
|
||||
self.group.handle_update(defn, None, props)
|
||||
|
||||
self.group.adjust.assert_called_once_with(
|
||||
6, adjustment_type='ExactCapacity')
|
||||
self.group.resize.assert_called_once_with(6)
|
||||
self.group._try_rolling_update.assert_called_once_with(props)
|
||||
|
||||
def test_update_in_failed(self):
|
||||
self.group.state_set('CREATE', 'FAILED')
|
||||
# to update the failed asg
|
||||
self.group.adjust = mock.Mock(return_value=None)
|
||||
self.group.resize = mock.Mock(return_value=None)
|
||||
|
||||
new_defn = rsrc_defn.ResourceDefinition(
|
||||
'asg', 'OS::Heat::AutoScalingGroup',
|
||||
|
@ -387,8 +382,7 @@ class TestGroupCrud(common.HeatTestCase):
|
|||
'Foo': 'hello'}}})
|
||||
|
||||
self.group.handle_update(new_defn, None, None)
|
||||
self.group.adjust.assert_called_once_with(
|
||||
2, adjustment_type='ExactCapacity')
|
||||
self.group.resize.assert_called_once_with(2)
|
||||
|
||||
|
||||
class HeatScalingGroupAttrTest(common.HeatTestCase):
|
||||
|
@ -590,7 +584,7 @@ class RollingUpdatePolicyDiffTest(common.HeatTestCase):
|
|||
current_grp.type(),
|
||||
properties=updated_grp.t['Properties'])
|
||||
current_grp._try_rolling_update = mock.MagicMock()
|
||||
current_grp.adjust = mock.MagicMock()
|
||||
current_grp.resize = mock.MagicMock()
|
||||
current_grp.handle_update(update_snippet, tmpl_diff, None)
|
||||
if updated_policy is None:
|
||||
self.assertIsNone(
|
||||
|
|
|
@ -115,8 +115,7 @@ class TestAutoScalingPolicy(common.HeatTestCase):
|
|||
return_value=True) as mock_isa:
|
||||
pol.handle_signal(details=test)
|
||||
mock_isa.assert_called_once_with()
|
||||
group.adjust.assert_called_once_with(1, 'change_in_capacity', None,
|
||||
signal=True)
|
||||
group.adjust.assert_called_once_with(1, 'change_in_capacity', None)
|
||||
|
||||
def test_scaling_policy_refid(self):
|
||||
t = template_format.parse(as_template)
|
||||
|
|
|
@ -301,19 +301,13 @@ 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')
|
||||
with mock.patch.object(self.group, '_is_scaling_allowed',
|
||||
return_value=False):
|
||||
self.group.adjust(1)
|
||||
self.patchobject(self.group, '_is_scaling_allowed',
|
||||
return_value=False)
|
||||
self.assertRaises(exception.NoActionRequired,
|
||||
self.group.adjust, 1)
|
||||
self.assertEqual([], dont_call.call_args_list)
|
||||
|
||||
def test_scaling_policy_cooldown_toosoon_with_signal(self):
|
||||
with mock.patch.object(self.group, '_is_scaling_allowed',
|
||||
return_value=False):
|
||||
self.assertRaises(exception.NoActionRequired, self.group.adjust, 1,
|
||||
signal=True)
|
||||
|
||||
def test_scaling_same_capacity(self):
|
||||
"""Alway resize even if the capacity is the same."""
|
||||
self.patchobject(grouputils, 'get_size', return_value=3)
|
||||
|
@ -520,36 +514,35 @@ class TestGroupCrud(common.HeatTestCase):
|
|||
|
||||
def test_handle_update_desired_cap(self):
|
||||
self.group._try_rolling_update = mock.Mock(return_value=None)
|
||||
self.group.adjust = mock.Mock(return_value=None)
|
||||
|
||||
props = {'DesiredCapacity': 4}
|
||||
self.group.resize = mock.Mock(return_value=None)
|
||||
props = {'DesiredCapacity': 4,
|
||||
'MinSize': 0,
|
||||
'MaxSize': 6}
|
||||
self.group._get_new_capacity = mock.Mock(return_value=4)
|
||||
defn = rsrc_defn.ResourceDefinition(
|
||||
'nopayload',
|
||||
'AWS::AutoScaling::AutoScalingGroup',
|
||||
props)
|
||||
|
||||
self.group.handle_update(defn, None, props)
|
||||
|
||||
self.group.adjust.assert_called_once_with(
|
||||
4, adjustment_type='ExactCapacity')
|
||||
self.group.resize.assert_called_once_with(4)
|
||||
self.group._try_rolling_update.assert_called_once_with(props)
|
||||
|
||||
def test_handle_update_desired_nocap(self):
|
||||
self.group._try_rolling_update = mock.Mock(return_value=None)
|
||||
self.group.adjust = mock.Mock(return_value=None)
|
||||
self.group.resize = mock.Mock(return_value=None)
|
||||
get_size = self.patchobject(grouputils, 'get_size')
|
||||
get_size.return_value = 6
|
||||
get_size.return_value = 4
|
||||
|
||||
props = {'Tags': []}
|
||||
props = {'MinSize': 0,
|
||||
'MaxSize': 6}
|
||||
defn = rsrc_defn.ResourceDefinition(
|
||||
'nopayload',
|
||||
'AWS::AutoScaling::AutoScalingGroup',
|
||||
props)
|
||||
|
||||
self.group.handle_update(defn, None, props)
|
||||
|
||||
self.group.adjust.assert_called_once_with(
|
||||
6, adjustment_type='ExactCapacity')
|
||||
self.group.resize.assert_called_once_with(4)
|
||||
self.group._try_rolling_update.assert_called_once_with(props)
|
||||
|
||||
def test_conf_properties_vpc_zone(self):
|
||||
|
@ -576,7 +569,7 @@ class TestGroupCrud(common.HeatTestCase):
|
|||
def test_update_in_failed(self):
|
||||
self.group.state_set('CREATE', 'FAILED')
|
||||
# to update the failed asg
|
||||
self.group.adjust = mock.Mock(return_value=None)
|
||||
self.group.resize = mock.Mock(return_value=None)
|
||||
|
||||
new_defn = rsrc_defn.ResourceDefinition(
|
||||
'asg', 'AWS::AutoScaling::AutoScalingGroup',
|
||||
|
@ -587,8 +580,7 @@ class TestGroupCrud(common.HeatTestCase):
|
|||
'DesiredCapacity': 2})
|
||||
|
||||
self.group.handle_update(new_defn, None, None)
|
||||
self.group.adjust.assert_called_once_with(
|
||||
2, adjustment_type='ExactCapacity')
|
||||
self.group.resize.assert_called_once_with(2)
|
||||
|
||||
|
||||
def asg_tmpl_with_bad_updt_policy():
|
||||
|
@ -727,7 +719,7 @@ class RollingUpdatePolicyDiffTest(common.HeatTestCase):
|
|||
properties=updated_grp.t['Properties'],
|
||||
update_policy=updated_policy)
|
||||
current_grp._try_rolling_update = mock.MagicMock()
|
||||
current_grp.adjust = mock.MagicMock()
|
||||
current_grp.resize = mock.MagicMock()
|
||||
current_grp.handle_update(update_snippet, tmpl_diff, None)
|
||||
if updated_policy is None:
|
||||
self.assertEqual({}, current_grp.update_policy.data)
|
||||
|
|
|
@ -121,8 +121,7 @@ class TestAutoScalingPolicy(common.HeatTestCase):
|
|||
return_value=True) as mock_isa:
|
||||
pol.handle_signal(details=test)
|
||||
mock_isa.assert_called_once_with()
|
||||
group.adjust.assert_called_once_with(1, 'ChangeInCapacity', None,
|
||||
signal=True)
|
||||
group.adjust.assert_called_once_with(1, 'ChangeInCapacity', None)
|
||||
|
||||
@mock.patch.object(aws_sp.AWSScalingPolicy, '_get_ec2_signed_url')
|
||||
def test_scaling_policy_refid_signed_url(self, mock_get_ec2_url):
|
||||
|
|
Loading…
Reference in New Issue