Assign unique names with os-multiple-create.

This patch introduces a new configuration option,
multi_instance_display_name_template, which makes
it so that all instances are given a unique
display_name and hostname when requesting that
multiple instances get created at the same time.

Implements bp multi-boot-instance-naming

DocImpact

Change-Id: Id8e7dac1adadbb101dd1027807867ab13d3b7e80

6 files changed, 97 insertions, 11 deletions

diff --git a/nova/cells/scheduler.py b/nova/cells/scheduler.py
index 0b730290a..211bbb48f 100644
--- a/nova/cells/scheduler.py
+++ b/nova/cells/scheduler.py
@@ -55,7 +55,8 @@ class CellsScheduler(base.Base):
 
     def _create_instances_here(self, ctxt, request_spec):
         instance_values = request_spec['instance_properties']
-        for instance_uuid in request_spec['instance_uuids']:
+        num_instances = len(request_spec['instance_uuids'])
+        for i, instance_uuid in enumerate(request_spec['instance_uuids']):
             instance_values['uuid'] = instance_uuid
             instance = self.compute_api.create_db_entry_for_new_instance(
                     ctxt,
@@ -63,7 +64,9 @@ class CellsScheduler(base.Base):
                     request_spec['image'],
                     instance_values,
                     request_spec['security_group'],
-                    request_spec['block_device_mapping'])
+                    request_spec['block_device_mapping'],
+                    num_instances, i)
+
             self.msg_runner.instance_update_at_top(ctxt, instance)
 
     def _get_possible_cells(self):
diff --git a/nova/compute/api.py b/nova/compute/api.py
index a5d8ab6e8..7876dc333 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -3,7 +3,7 @@
 # Copyright 2010 United States Government as represented by the
 # Administrator of the National Aeronautics and Space Administration.
 # Copyright 2011 Piston Cloud Computing, Inc.
-# Copyright 2012 Red Hat, Inc.
+# Copyright 2012-2013 Red Hat, Inc.
 # All Rights Reserved.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -86,6 +86,16 @@ compute_opts = [
     cfg.StrOpt('security_group_api',
                default='nova.compute.api.SecurityGroupAPI',
                help='The full class name of the security API class'),
+    cfg.StrOpt('multi_instance_display_name_template',
+               default='%(name)s-%(uuid)s',
+               help='When creating multiple instances with a single request '
+                    'using the os-multiple-create API extension, this '
+                    'template will be used to build the display name for '
+                    'each instance. The benefit is that the instances '
+                    'end up with different hostnames. To restore legacy '
+                    'behavior of every instance having the same name, set '
+                    'this option to "%(name)s".  Valid keys for the '
+                    'template are: name, uuid, count.'),
 ]
 
 
@@ -419,6 +429,26 @@ class API(base.Base):
         options_from_image['auto_disk_config'] = auto_disk_config
         return options_from_image
 
+    def _apply_instance_name_template(self, context, instance, index):
+        params = {
+            'uuid': instance['uuid'],
+            'name': instance['display_name'],
+            'count': index + 1,
+        }
+        try:
+            new_name = (CONF.multi_instance_display_name_template %
+                        params)
+        except KeyError, TypeError:
+            LOG.exception(_('Failed to set instance name using '
+                            'multi_instance_display_name_template.'))
+            new_name = instance['display_name']
+        updates = {'display_name': new_name}
+        if not instance.get('hostname'):
+            updates['hostname'] = utils.sanitize_hostname(new_name)
+        instance = self.db.instance_update(context,
+                instance['uuid'], updates)
+        return instance
+
     def _validate_and_provision_instance(self, context, instance_type,
                                          image_href, kernel_id, ramdisk_id,
                                          min_count, max_count,
@@ -573,7 +603,8 @@ class API(base.Base):
                 options = base_options.copy()
                 instance = self.create_db_entry_for_new_instance(
                         context, instance_type, image, options,
-                        security_group, block_device_mapping)
+                        security_group, block_device_mapping, num_instances, i)
+
                 instances.append(instance)
                 instance_uuids.append(instance['uuid'])
                 self._validate_bdm(context, instance)
@@ -777,7 +808,7 @@ class API(base.Base):
             image_properties.get('block_device_mapping')):
             instance['shutdown_terminate'] = False
 
-    def _populate_instance_names(self, instance):
+    def _populate_instance_names(self, instance, num_instances):
         """Populate instance display_name and hostname."""
         display_name = instance.get('display_name')
         hostname = instance.get('hostname')
@@ -785,9 +816,17 @@ class API(base.Base):
         if display_name is None:
             display_name = self._default_display_name(instance['uuid'])
             instance['display_name'] = display_name
-        if hostname is None:
+
+        if hostname is None and num_instances == 1:
+            # NOTE(russellb) In the multi-instance case, we're going to
+            # overwrite the display_name using the
+            # multi_instance_display_name_template.  We need the default
+            # display_name set so that it can be used in the template, though.
+            # Only set the hostname here if we're only creating one instance.
+            # Otherwise, it will be built after the template based
+            # display_name.
             hostname = display_name
-        instance['hostname'] = utils.sanitize_hostname(hostname)
+            instance['hostname'] = utils.sanitize_hostname(hostname)
 
     def _default_display_name(self, instance_uuid):
         return "Server %s" % instance_uuid
@@ -837,7 +876,8 @@ class API(base.Base):
     #NOTE(bcwaldon): No policy check since this is only used by scheduler and
     # the compute api. That should probably be cleaned up, though.
     def create_db_entry_for_new_instance(self, context, instance_type, image,
-            base_options, security_group, block_device_mapping):
+            base_options, security_group, block_device_mapping, num_instances,
+            index):
         """Create an entry in the DB for this new instance,
         including any related table updates (such as security group,
         etc).
@@ -848,7 +888,7 @@ class API(base.Base):
         instance = self._populate_instance_for_create(base_options,
                 image, security_group)
 
-        self._populate_instance_names(instance)
+        self._populate_instance_names(instance, num_instances)
 
         self._populate_instance_shutdown_terminate(instance, image,
                                                    block_device_mapping)
@@ -859,6 +899,13 @@ class API(base.Base):
         self.security_group_api.ensure_default(context)
         instance = self.db.instance_create(context, instance)
 
+        if num_instances > 1:
+            # NOTE(russellb) We wait until this spot to handle
+            # multi_instance_display_name_template, because we need
+            # the UUID from the instance.
+            instance = self._apply_instance_name_template(context, instance,
+                                                          index)
+
         self._populate_instance_for_bdm(context, instance,
                 instance_type, image, block_device_mapping)
 
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 10441cd23..82619ffa5 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -1851,8 +1851,9 @@ def _instance_update(context, instance_uuid, values, copy_old_instance=False):
                 raise exception.UnexpectedTaskStateError(actual=actual_state,
                                                          expected=expected)
 
+        instance_hostname = instance_ref['hostname'] or ''
         if ("hostname" in values and
-            values["hostname"].lower() != instance_ref["hostname"].lower()):
+                values["hostname"].lower() != instance_hostname.lower()):
                 _validate_unique_server_name(context,
                                              session,
                                              values['hostname'])
diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py
index 4bfb1c1e3..e6ecd4c60 100644
--- a/nova/tests/api/openstack/compute/test_servers.py
+++ b/nova/tests/api/openstack/compute/test_servers.py
@@ -1740,6 +1740,11 @@ class ServersControllerCreateTest(test.TestCase):
             """
             return self.instance_cache_by_id[instance_id]
 
+        def instance_update(context, uuid, values):
+            instance = self.instance_cache_by_uuid[uuid]
+            instance.update(values)
+            return instance
+
         def rpc_call_wrapper(context, topic, msg, timeout=None):
             """Stub out the scheduler creating the instance entry."""
             if (topic == CONF.scheduler_topic and
@@ -1779,6 +1784,7 @@ class ServersControllerCreateTest(test.TestCase):
         self.stubs.Set(db, 'instance_system_metadata_update',
                 fake_method)
         self.stubs.Set(db, 'instance_get', instance_get)
+        self.stubs.Set(db, 'instance_update', instance_update)
         self.stubs.Set(rpc, 'cast', fake_method)
         self.stubs.Set(rpc, 'call', rpc_call_wrapper)
         self.stubs.Set(db, 'instance_update_and_get_original',
diff --git a/nova/tests/cells/test_cells_scheduler.py b/nova/tests/cells/test_cells_scheduler.py
index 15b2571b5..ecd51c9f7 100644
--- a/nova/tests/cells/test_cells_scheduler.py
+++ b/nova/tests/cells/test_cells_scheduler.py
@@ -78,7 +78,8 @@ class CellsSchedulerTestCase(test.TestCase):
         for instance_uuid in self.instance_uuids:
             instance = db.instance_get_by_uuid(self.ctxt, instance_uuid)
             self.assertEqual('meow', instance['hostname'])
-            self.assertEqual('moo', instance['display_name'])
+            self.assertEqual('moo-%s' % instance['uuid'],
+                             instance['display_name'])
             self.assertEqual('fake_image_ref', instance['image_ref'])
 
     def test_run_instance_selects_child_cell(self):
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index 12bd3cf19..eb7246cf8 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -5495,6 +5495,34 @@ class ComputeAPITestCase(BaseTestCase):
 
         db.instance_destroy(self.context, refs[0]['uuid'])
 
+    def test_multi_instance_display_name_template(self):
+        self.flags(multi_instance_display_name_template='%(name)s')
+        (refs, resv_id) = self.compute_api.create(self.context,
+                instance_types.get_default_instance_type(), None,
+                min_count=2, max_count=2, display_name='x')
+        self.assertEqual(refs[0]['display_name'], 'x')
+        self.assertEqual(refs[0]['hostname'], 'x')
+        self.assertEqual(refs[1]['display_name'], 'x')
+        self.assertEqual(refs[1]['hostname'], 'x')
+
+        self.flags(multi_instance_display_name_template='%(name)s-%(count)s')
+        (refs, resv_id) = self.compute_api.create(self.context,
+                instance_types.get_default_instance_type(), None,
+                min_count=2, max_count=2, display_name='x')
+        self.assertEqual(refs[0]['display_name'], 'x-1')
+        self.assertEqual(refs[0]['hostname'], 'x-1')
+        self.assertEqual(refs[1]['display_name'], 'x-2')
+        self.assertEqual(refs[1]['hostname'], 'x-2')
+
+        self.flags(multi_instance_display_name_template='%(name)s-%(uuid)s')
+        (refs, resv_id) = self.compute_api.create(self.context,
+                instance_types.get_default_instance_type(), None,
+                min_count=2, max_count=2, display_name='x')
+        self.assertEqual(refs[0]['display_name'], 'x-%s' % refs[0]['uuid'])
+        self.assertEqual(refs[0]['hostname'], 'x-%s' % refs[0]['uuid'])
+        self.assertEqual(refs[1]['display_name'], 'x-%s' % refs[1]['uuid'])
+        self.assertEqual(refs[1]['hostname'], 'x-%s' % refs[1]['uuid'])
+
     def test_instance_architecture(self):
         # Test the instance architecture.
         i_ref = self._create_fake_instance()