From bbe47783b587c6da2ba1a86e332fbd37096005c4 Mon Sep 17 00:00:00 2001
From: Angus Salkeld <asalkeld@mirantis.com>
Date: Tue, 21 Oct 2014 10:48:28 +1000
Subject: [PATCH] Make sure that AutoScalingGroup depends on the launch config

It seems to be common error to do the following:
  LaunchConfigurationName: my_config
and not include a depends_on.

Largely because of the unfortunate property name.
So this patch tries to make sure the user has the
correct dependency in place.

Change-Id: I4f7d282d0af8681dc2c8cd265e16a27fe23a3bdb
Closes-bug: #1375777
---
 heat/engine/resources/autoscaling.py | 28 ++++++++++++++++++++++++++--
 heat/tests/test_instance_group.py    | 26 ++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/heat/engine/resources/autoscaling.py b/heat/engine/resources/autoscaling.py
index 176b6608a6..3d080ff459 100644
--- a/heat/engine/resources/autoscaling.py
+++ b/heat/engine/resources/autoscaling.py
@@ -116,7 +116,7 @@ class InstanceGroup(stack_resource.StackResource):
         ),
         LAUNCH_CONFIGURATION_NAME: properties.Schema(
             properties.Schema.STRING,
-            _('Name of LaunchConfiguration resource.'),
+            _('The reference to a LaunchConfiguration resource.'),
             required=True,
             update_allowed=True
         ),
@@ -192,6 +192,28 @@ class InstanceGroup(stack_resource.StackResource):
                 if iso8601utils.parse_isoduration(pause_time) > 3600:
                     raise ValueError('Maximum PauseTime is 1 hour.')
 
+    def validate_launchconfig(self):
+        # It seems to be a common error to not have a dependancy on the
+        # launchconfiguration. This can happen if the the actual resource
+        # name is used instead of {get_resource: launch_conf} and no
+        # depends_on is used.
+
+        conf_refid = self.properties.get(self.LAUNCH_CONFIGURATION_NAME)
+        if conf_refid:
+            conf = self.stack.resource_by_refid(conf_refid)
+            if conf is None:
+                raise ValueError(_('%(lc)s (%(ref)s)'
+                                   ' reference can not be found.')
+                                 % dict(lc=self.LAUNCH_CONFIGURATION_NAME,
+                                        ref=conf_refid))
+            if self.name not in conf.required_by():
+                raise ValueError(_('%(lc)s (%(ref)s)'
+                                   ' requires a reference to the'
+                                   ' configuration not just the name of the'
+                                   ' resource.') % dict(
+                                       lc=self.LAUNCH_CONFIGURATION_NAME,
+                                       ref=conf_refid))
+
     def get_instance_names(self):
         """Get a list of resource names of the instances in this InstanceGroup.
 
@@ -221,6 +243,7 @@ class InstanceGroup(stack_resource.StackResource):
 
     def handle_create(self):
         """Create a nested stack and add the initial resources to it."""
+        self.validate_launchconfig()
         num_instances = self.properties[self.SIZE]
         initial_template = self._create_template(num_instances)
         return self.create_with_template(initial_template, self._environment())
@@ -484,7 +507,7 @@ class AutoScalingGroup(InstanceGroup, cooldown.CooldownMixin):
         ),
         LAUNCH_CONFIGURATION_NAME: properties.Schema(
             properties.Schema.STRING,
-            _('Name of LaunchConfiguration resource.'),
+            _('The reference to a LaunchConfiguration resource.'),
             required=True,
             update_allowed=True
         ),
@@ -574,6 +597,7 @@ class AutoScalingGroup(InstanceGroup, cooldown.CooldownMixin):
     }
 
     def handle_create(self):
+        self.validate_launchconfig()
         return self.create_with_template(self.child_template(),
                                          self._environment())
 
diff --git a/heat/tests/test_instance_group.py b/heat/tests/test_instance_group.py
index f6976d7762..21ced2ec30 100644
--- a/heat/tests/test_instance_group.py
+++ b/heat/tests/test_instance_group.py
@@ -12,6 +12,7 @@
 #    under the License.
 
 import copy
+import six
 
 import mock
 
@@ -364,6 +365,31 @@ class InstanceGroupTest(common.HeatTestCase):
 
         self.m.VerifyAll()
 
+    def test_validate_launch_conf(self):
+        t = template_format.parse(ig_template)
+        properties = t['Resources']['JobServerGroup']['Properties']
+        properties['LaunchConfigurationName'] = 'urg_i_cant_spell'
+        stack = utils.parse_stack(t)
+
+        rsrc = stack['JobServerGroup']
+        creator = scheduler.TaskRunner(rsrc.create)
+        error = self.assertRaises(exception.ResourceFailure, creator)
+
+        self.assertIn('(urg_i_cant_spell) reference can not be found.',
+                      six.text_type(error))
+
+    def test_validate_launch_conf_no_ref(self):
+        t = template_format.parse(ig_template)
+        properties = t['Resources']['JobServerGroup']['Properties']
+        properties['LaunchConfigurationName'] = 'JobServerConfig'
+        stack = utils.parse_stack(t)
+
+        rsrc = stack['JobServerGroup']
+        creator = scheduler.TaskRunner(rsrc.create)
+        error = self.assertRaises(exception.ResourceFailure, creator)
+        self.assertIn('(JobServerConfig) requires a reference to the',
+                      six.text_type(error))
+
 
 class TestChildTemplate(common.HeatTestCase):
     def setUp(self):