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 9ca41f59398dce2b186d18f8ec4e3d8d98bfb8c1..d0e5e8ac9072de161e0c990059c89797a1384964 100644 (file)
--- 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 a56bb5816bad58e2e3cad9968bd6a036fe2b2e6d..3f68a0bd729f6d261015ae81af7f843f2c0dcba0 100644 (file)
--- 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 9c4ac7eb552b7909aa75c9b29b08e6339507928d..8e06468c1ce6ec58c35387f2b0b3f7c155b2bdfd 100644 (file)
--- 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']