Allow a custom new ID function for member_definitions()

Previously we always used a random short_id, and that is indeed what we
want for Autoscaling groups, but ResourceGroups generally use sequential
IDs, so pass a function that generates them instead of hard-coding it.

By happy accident, this makes the tests a bit nicer too.

Change-Id: I58ce67e4b58378f35d5690d653a113b03ee6e08e
Partially-Implements: blueprint scaling-group-common

heat/engine/resources/openstack/heat/instance_group.py

@@ -14,6 +14,7 @@
 from heat.common import environment_format
 from heat.common import grouputils
 from heat.common.i18n import _
+from heat.common import short_id
 from heat.common import timeutils as iso8601utils
 from heat.engine import attributes
 from heat.engine import environment
@@ -263,7 +264,8 @@ class InstanceGroup(stack_resource.StackResource):
         instance_definition = self._get_instance_definition()
         old_resources = self._get_instance_templates()
         definitions = template.member_definitions(
-            old_resources, instance_definition, num_instances, num_replace)
+            old_resources, instance_definition, num_instances, num_replace,
+            short_id.generate_id)
 
         child_env = environment.get_child_environment(
             self.stack.env,

heat/scaling/template.py

@@ -11,12 +11,12 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from heat.common import short_id
 from heat.engine import template
 
 
 def member_definitions(old_resources, new_definition,
-                       num_resources, num_new):
+                       num_resources, num_new,
+                       get_new_id):
     """
     Iterate over resource definitions for a scaling group
 
@@ -48,7 +48,7 @@ def member_definitions(old_resources, new_definition,
             else:
                 yield old_name, old_definition
         else:
-            yield short_id.generate_id(), new_definition
+            yield get_new_id(), new_definition
 
 
 def make_template(resource_definitions,

heat/tests/test_scaling_template.py

@@ -11,10 +11,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import functools
 import itertools
 
-from heat.common import short_id
 from heat.scaling import template
 from heat.tests import common
 
@@ -24,16 +22,15 @@ class ResourceTemplatesTest(common.HeatTestCase):
     def setUp(self):
         super(ResourceTemplatesTest, self).setUp()
         ids = ('stubbed-id-%s' % (i,) for i in itertools.count())
-        self.patchobject(
-            short_id, 'generate_id').side_effect = functools.partial(next,
-                                                                     ids)
+        self.next_id = lambda: next(ids)
 
     def test_create_template(self):
         """
         When creating a template from scratch, an empty list is accepted as
         the "old" resources and new resources are created up to num_resource.
         """
-        templates = template.member_definitions([], {'type': 'Foo'}, 2, 0)
+        templates = template.member_definitions([], {'type': 'Foo'}, 2, 0,
+                                                self.next_id)
         expected = [
             ('stubbed-id-0', {'type': 'Foo'}),
             ('stubbed-id-1', {'type': 'Foo'})]
@@ -48,7 +45,7 @@ class ResourceTemplatesTest(common.HeatTestCase):
             ('old-id-0', {'type': 'Foo'}),
             ('old-id-1', {'type': 'Foo'})]
         templates = template.member_definitions(old_resources, {'type': 'Bar'},
-                                                1, 2)
+                                                1, 2, self.next_id)
         expected = [('old-id-1', {'type': 'Bar'})]
         self.assertEqual(expected, list(templates))
 
@@ -61,7 +58,8 @@ class ResourceTemplatesTest(common.HeatTestCase):
             ('old-id-0', {'type': 'Foo'}),
             ('old-id-1', {'type': 'Foo'})]
         new_spec = {'type': 'Bar'}
-        templates = template.member_definitions(old_resources, new_spec, 2, 1)
+        templates = template.member_definitions(old_resources, new_spec, 2, 1,
+                                                self.next_id)
         expected = [
             ('old-id-0', {'type': 'Bar'}),
             ('old-id-1', {'type': 'Foo'})]
@@ -78,7 +76,8 @@ class ResourceTemplatesTest(common.HeatTestCase):
             ('old-id-0', spec),
             ('old-id-1', spec)]
         new_spec = {'type': 'Bar'}
-        templates = template.member_definitions(old_resources, new_spec, 4, 2)
+        templates = template.member_definitions(old_resources, new_spec, 4, 2,
+                                                self.next_id)
         expected = [
             ('old-id-0', spec),
             ('old-id-1', spec),
@@ -96,7 +95,8 @@ class ResourceTemplatesTest(common.HeatTestCase):
             ('old-id-0', {'type': 'Bar'}),
             ('old-id-1', {'type': 'Foo'})]
         new_spec = {'type': 'Bar'}
-        templates = template.member_definitions(old_resources, new_spec, 2, 1)
+        templates = template.member_definitions(old_resources, new_spec, 2, 1,
+                                                self.next_id)
         second_batch_expected = [
             ('old-id-0', {'type': 'Bar'}),
             ('old-id-1', {'type': 'Bar'})]