From: Christopher Armstrong Date: Mon, 26 Aug 2013 17:52:53 +0000 (-0500) Subject: Don't delete failed instances in InstanceGroup X-Git-Tag: 2014.1~122 X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=fe498a2d4f97287cffe60a02d27a5e4b2665862d;p=openstack-build%2Fheat-build.git Don't delete failed instances in InstanceGroup This gets rid of the code to delete failed instances in InstanceGroup so that administrators can debug what happened if any instances fail to start. It also allows the resize operation to raise an error, so the InstanceGroup should be in a FAILED state after a failed resize operation. fixes bug 1214007 fixes bug 1215140 Change-Id: Id508195c847f09dd986e450f35876ff574ff7673 --- diff --git a/heat/engine/resources/autoscaling.py b/heat/engine/resources/autoscaling.py index 9ca41f59..d0e5e8ac 100644 --- a/heat/engine/resources/autoscaling.py +++ b/heat/engine/resources/autoscaling.py @@ -15,10 +15,10 @@ import copy -from heat.common import exception from heat.engine import resource from heat.engine import signal_responder +from heat.common import exception from heat.openstack.common import log as logging from heat.openstack.common import timeutils from heat.engine.properties import Properties @@ -116,14 +116,14 @@ class InstanceGroup(stack_resource.StackResource): def get_instance_names(self): """Get a list of resource names of the instances in this InstanceGroup. - Deleted resources will be ignored. + Failed resources will be ignored. """ return sorted(x.name for x in self.get_instances()) def get_instances(self): """Get a set of all the instance resources managed by this group.""" return [resource for resource in self.nested() - if resource.state[0] != resource.DELETE] + if resource.state[1] != resource.FAILED] def handle_create(self): """Create a nested stack and add the initial resources to it.""" @@ -137,14 +137,8 @@ class InstanceGroup(stack_resource.StackResource): If any instances failed to be created, delete them. """ - try: - done = super(InstanceGroup, self).check_create_complete(task) - except exception.Error as exc: - for resource in self.nested(): - if resource.state == ('CREATE', 'FAILED'): - resource.destroy() - raise - if done and len(self.get_instances()): + done = super(InstanceGroup, self).check_create_complete(task) + if done: self._lb_reload() return done @@ -217,13 +211,10 @@ class InstanceGroup(stack_resource.StackResource): new_template = self._create_template(new_capacity) try: self.update_with_template(new_template, {}) - except exception.Error as ex: - logger.error('Failed to resize instance group %s. Error: %s' % - (self.name, ex)) - for resource in self.nested(): - if resource.state == ('CREATE', 'FAILED'): - resource.destroy() - self._lb_reload() + finally: + # Reload the LB in any case, so it's only pointing at healthy + # nodes. + self._lb_reload() def _lb_reload(self): ''' diff --git a/heat/tests/test_autoscaling.py b/heat/tests/test_autoscaling.py index a56bb581..3f68a0bd 100644 --- a/heat/tests/test_autoscaling.py +++ b/heat/tests/test_autoscaling.py @@ -186,7 +186,7 @@ class AutoScalingTest(HeatTestCase): properties['MinSize'] = '0' properties['MaxSize'] = '0' stack = utils.parse_stack(t, params=self.params) - + self._stub_lb_reload(0) self.m.ReplayAll() rsrc = self.create_scaling_group(t, stack, 'WebServerGroup') self.assertEqual(None, rsrc.FnGetAtt("InstanceList")) @@ -780,7 +780,7 @@ class AutoScalingTest(HeatTestCase): self._stub_validate() self.m.ReplayAll() - rsrc.adjust(1) + self.assertRaises(Exception, rsrc.adjust, 1) self.assertEqual(['WebServerGroup-0'], rsrc.get_instance_names()) self.m.VerifyAll() diff --git a/heat/tests/test_instance_group.py b/heat/tests/test_instance_group.py index 9c4ac7eb..8e06468c 100644 --- a/heat/tests/test_instance_group.py +++ b/heat/tests/test_instance_group.py @@ -197,6 +197,75 @@ class InstanceGroupTest(HeatTestCase): rsrc.delete() self.m.VerifyAll() + def test_create_error(self): + """ + If a resource in an instance group fails to be created, the instance + group itself will fail and the broken inner resource will remain. + """ + t = template_format.parse(ig_template) + stack = utils.parse_stack(t) + + self.m.StubOutWithMock(parser.Stack, 'validate') + parser.Stack.validate() + self.m.StubOutWithMock(instance.Instance, 'handle_create') + instance.Instance.handle_create().AndRaise(Exception) + + self.m.ReplayAll() + conf = self.create_resource(t, stack, 'JobServerConfig') + self.assertRaises( + exception.ResourceFailure, + self.create_resource, t, stack, 'JobServerGroup') + + rsrc = stack.resources['JobServerGroup'] + self.assertEqual((rsrc.CREATE, rsrc.FAILED), rsrc.state) + + # The failed inner resource remains + child_resource = rsrc.nested().resources['JobServerGroup-0'] + self.assertEqual((child_resource.CREATE, child_resource.FAILED), + child_resource.state) + + self.m.VerifyAll() + + def test_update_error(self): + """ + If a resource in an instance group fails to be created during an + update, the instance group itself will fail and the broken inner + resource will remain. + """ + t = template_format.parse(ig_template) + stack = utils.parse_stack(t) + + self._stub_create(1) + self.m.ReplayAll() + conf = self.create_resource(t, stack, 'JobServerConfig') + rsrc = self.create_resource(t, stack, 'JobServerGroup') + + self.m.VerifyAll() + self.m.UnsetStubs() + + self.m.StubOutWithMock(parser.Stack, 'validate') + parser.Stack.validate() + self.m.StubOutWithMock(instance.Instance, 'handle_create') + instance.Instance.handle_create().AndRaise(Exception) + + self.m.ReplayAll() + + update_snippet = copy.deepcopy(rsrc.parsed_template()) + update_snippet['Properties']['Size'] = '2' + tmpl_diff = {'Properties': {'Size': '2'}} + prop_diff = {'Size': '2'} + self.assertRaises(exception.ResourceFailure, + rsrc.update, update_snippet) + + self.assertEqual((rsrc.UPDATE, rsrc.FAILED), rsrc.state) + + # The failed inner resource remains + child_resource = rsrc.nested().resources['JobServerGroup-1'] + self.assertEqual((child_resource.CREATE, child_resource.FAILED), + child_resource.state) + + self.m.VerifyAll() + def test_update_fail_badkey(self): t = template_format.parse(ig_template) properties = t['Resources']['JobServerGroup']['Properties']