Make run_instance only support instance uuids.

Related to blueprint internal-uuids.

This patchset also attempts a major overhaul of run_instance so that the
code is cleaner and easier to understand (no more global-style
variables!)

Change-Id: I2289f3c253c6246ea51395b2dcfccee2256a2813

1 files changed, 151 insertions, 160 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 0cbddca3e..71ba69c92 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -151,9 +151,6 @@ class ComputeManager(manager.SchedulerDependentManager):
 
     def _instance_update(self, context, instance_id, **kwargs):
         """Update an instance in the database using kwargs as value."""
-        if utils.is_uuid_like(instance_id):
-            instance = self.db.instance_get_by_uuid(context, instance_id)
-            instance_id = instance['id']
         return self.db.instance_update(context, instance_id, kwargs)
 
     def init_host(self):
@@ -320,182 +317,176 @@ class ComputeManager(manager.SchedulerDependentManager):
 
         return (swap, ephemerals, block_device_mapping)
 
-    def _run_instance(self, context, instance_uuid, **kwargs):
+    def _run_instance(self, context, instance_uuid,
+                      requested_networks=None,
+                      injected_files=[],
+                      admin_pass=None,
+                      **kwargs):
         """Launch a new instance with specified options."""
-        def _check_image_size(image_meta):
-            """Ensure image is smaller than the maximum size allowed by the
-            instance_type.
+        context = context.elevated()
+        try:
+            instance = self.db.instance_get_by_uuid(context, instance_uuid)
+            self._check_instance_not_already_created(context, instance)
+            image_meta = self._check_image_size(context, instance)
+            self._start_building(context, instance)
+            network_info = self._allocate_network(context, instance,
+                                                  requested_networks)
+            try:
+                block_device_info = self._prep_block_device(context, instance)
+                instance = self._spawn(context, instance, image_meta,
+                                       network_info, block_device_info,
+                                       injected_files, admin_pass)
+            except:
+                self._deallocate_network(context, instance)
+                raise
+            self._notify_about_instance_usage(instance)
+        except exception.InstanceNotFound:
+            LOG.exception(_("Instance %s not found.") % instance_uuid)
+            return  # assuming the instance was already deleted
+        except Exception:
+            with utils.save_and_reraise_exception():
+                self._instance_update(context, instance_uuid,
+                                      vm_state=vm_states.ERROR)
 
-            The image stored in Glance is potentially compressed, so we use two
-            checks to ensure that the size isn't exceeded:
+    def _check_instance_not_already_created(self, context, instance):
+        """Ensure an instance with the same name is not already present."""
+        if instance['name'] in self.driver.list_instances():
+            raise exception.Error(_("Instance has already been created"))
 
-                1) This one - checks compressed size, this a quick check to
-                   eliminate any images which are obviously too large
+    def _check_image_size(self, context, instance):
+        """Ensure image is smaller than the maximum size allowed by the
+        instance_type.
 
-                2) Check uncompressed size in nova.virt.xenapi.vm_utils. This
-                   is a slower check since it requires uncompressing the entire
-                   image, but is accurate because it reflects the image's
-                   actual size.
-            """
+        The image stored in Glance is potentially compressed, so we use two
+        checks to ensure that the size isn't exceeded:
 
-            try:
-                size_bytes = image_meta['size']
-            except KeyError:
-                # Size is not a required field in the image service (yet), so
-                # we are unable to rely on it being there even though it's in
-                # glance.
-
-                # TODO(jk0): Should size be required in the image service?
-                return
-
-            instance_type_id = instance['instance_type_id']
-            instance_type = instance_types.get_instance_type(instance_type_id)
-            allowed_size_gb = instance_type['local_gb']
-
-            # NOTE(jk0): Since libvirt uses local_gb as a secondary drive, we
-            # need to handle potential situations where local_gb is 0. This is
-            # the default for m1.tiny.
-            if allowed_size_gb == 0:
-                return
-
-            allowed_size_bytes = allowed_size_gb * 1024 * 1024 * 1024
-
-            image_id = image_meta['id']
-            LOG.debug(_("image_id=%(image_id)s, image_size_bytes="
-                        "%(size_bytes)d, allowed_size_bytes="
-                        "%(allowed_size_bytes)d") % locals())
-
-            if size_bytes > allowed_size_bytes:
-                LOG.info(_("Image '%(image_id)s' size %(size_bytes)d exceeded"
-                           " instance_type allowed size "
-                           "%(allowed_size_bytes)d")
-                           % locals())
-                raise exception.ImageTooLarge()
-
-        def _make_network_info():
-            if FLAGS.stub_network:
-                # TODO(tr3buchet) not really sure how this should be handled.
-                # virt requires network_info to be passed in but stub_network
-                # is enabled. Setting to [] for now will cause virt to skip
-                # all vif creation and network injection, maybe this is correct
-                network_info = []
-            else:
-                # NOTE(vish): This could be a cast because we don't do
-                # anything with the address currently, but I'm leaving it as a
-                # call to ensure that network setup completes.  We will
-                # eventually also need to save the address here.
-                network_info = self.network_api.allocate_for_instance(context,
-                                    instance, vpn=is_vpn,
-                                    requested_networks=requested_networks)
-                LOG.debug(_("instance network_info: |%s|"), network_info)
-            return network_info
-
-        def _make_block_device_info():
-            (swap, ephemerals,
-             block_device_mapping) = self._setup_block_device_mapping(
-                context, instance)
-            block_device_info = {
-                'root_device_name': instance['root_device_name'],
-                'swap': swap,
-                'ephemerals': ephemerals,
-                'block_device_mapping': block_device_mapping}
-            return block_device_info
+            1) This one - checks compressed size, this a quick check to
+               eliminate any images which are obviously too large
 
-        def _deallocate_network():
-            if not FLAGS.stub_network:
-                LOG.debug(_("deallocating network for instance: %s"),
-                          instance['id'])
-                self.network_api.deallocate_for_instance(context,
-                                    instance)
+            2) Check uncompressed size in nova.virt.xenapi.vm_utils. This
+               is a slower check since it requires uncompressing the entire
+               image, but is accurate because it reflects the image's
+               actual size.
+        """
+        image_meta = _get_image_meta(context, instance['image_ref'])
 
-        def _cleanup():
-            with utils.save_and_reraise_exception():
-                self._instance_update(context,
-                                      instance_id,
-                                      vm_state=vm_states.ERROR)
-                if network_info is not None:
-                    _deallocate_network()
+        try:
+            size_bytes = image_meta['size']
+        except KeyError:
+            # Size is not a required field in the image service (yet), so
+            # we are unable to rely on it being there even though it's in
+            # glance.
 
-        def _error_message(instance_uuid, message):
-            return _("Instance '%(instance_uuid)s' "
-                     "failed %(message)s.") % locals()
+            # TODO(jk0): Should size be required in the image service?
+            return
 
-        context = context.elevated()
-        if utils.is_uuid_like(instance_uuid):
-            instance = self.db.instance_get_by_uuid(context, instance_uuid)
-            instance_id = instance['id']
-        else:
-            instance_id = instance_uuid
-            instance = self.db.instance_get(context, instance_id)
-            instance_uuid = instance['uuid']
+        instance_type_id = instance['instance_type_id']
+        instance_type = instance_types.get_instance_type(instance_type_id)
+        allowed_size_gb = instance_type['local_gb']
+
+        # NOTE(jk0): Since libvirt uses local_gb as a secondary drive, we
+        # need to handle potential situations where local_gb is 0. This is
+        # the default for m1.tiny.
+        if allowed_size_gb == 0:
+            return
 
-        requested_networks = kwargs.get('requested_networks', None)
+        allowed_size_bytes = allowed_size_gb * 1024 * 1024 * 1024
 
-        if instance['name'] in self.driver.list_instances():
-            raise exception.Error(_("Instance has already been created"))
+        image_id = image_meta['id']
+        LOG.debug(_("image_id=%(image_id)s, image_size_bytes="
+                    "%(size_bytes)d, allowed_size_bytes="
+                    "%(allowed_size_bytes)d") % locals())
 
-        image_meta = _get_image_meta(context, instance['image_ref'])
+        if size_bytes > allowed_size_bytes:
+            LOG.info(_("Image '%(image_id)s' size %(size_bytes)d exceeded"
+                       " instance_type allowed size "
+                       "%(allowed_size_bytes)d")
+                       % locals())
+            raise exception.ImageTooLarge()
 
-        _check_image_size(image_meta)
+        return image_meta
 
-        LOG.audit(_("instance %s: starting..."), instance_uuid,
+    def _start_building(self, context, instance):
+        """Save the host and launched_on fields and log appropriately."""
+        LOG.audit(_("instance %s: starting..."), instance['uuid'],
                   context=context)
-        updates = {}
-        updates['host'] = self.host
-        updates['launched_on'] = self.host
-        updates['vm_state'] = vm_states.BUILDING
-        updates['task_state'] = task_states.NETWORKING
-        instance = self.db.instance_update(context, instance_id, updates)
-        instance['injected_files'] = kwargs.get('injected_files', [])
-        instance['admin_pass'] = kwargs.get('admin_password', None)
+        self._instance_update(context, instance['uuid'],
+                              host=self.host, launched_on=self.host,
+                              vm_state=vm_states.BUILDING,
+                              task_state=None)
 
+    def _allocate_network(self, context, instance, requested_networks):
+        """Allocate networks for an instance and return the network info"""
+        if FLAGS.stub_network:
+            msg = _("Skipping network allocation for instance %s")
+            LOG.debug(msg % instance['uuid'])
+            return []
+        self._instance_update(context, instance['uuid'],
+                              vm_state=vm_states.BUILDING,
+                              task_state=task_states.NETWORKING)
         is_vpn = instance['image_ref'] == str(FLAGS.vpn_image_id)
         try:
-            network_info = None
-            with utils.logging_error(_error_message(instance_id,
-                                                    "network setup")):
-                network_info = _make_network_info()
+            network_info = self.network_api.allocate_for_instance(
+                                context, instance, vpn=is_vpn,
+                                requested_networks=requested_networks)
+        except:
+            msg = _("Instance %s failed network setup")
+            LOG.exception(msg % instance['uuid'])
+            raise
+        LOG.debug(_("instance network_info: |%s|"), network_info)
+        return network_info
 
-            self._instance_update(context,
-                                  instance_uuid,
-                                  vm_state=vm_states.BUILDING,
-                                  task_state=task_states.BLOCK_DEVICE_MAPPING)
-            with utils.logging_error(_error_message(instance_uuid,
-                                                    "block device setup")):
-                block_device_info = _make_block_device_info()
+    def _prep_block_device(self, context, instance):
+        """Set up the block device for an instance with error logging"""
+        self._instance_update(context, instance['uuid'],
+                              vm_state=vm_states.BUILDING,
+                              task_state=task_states.BLOCK_DEVICE_MAPPING)
+        try:
+            mapping = self._setup_block_device_mapping(context, instance)
+            swap, ephemerals, block_device_mapping = mapping
+        except:
+            msg = _("Instance %s failed block device setup")
+            LOG.exception(msg % instance['uuid'])
+            raise
+        return {'root_device_name': instance['root_device_name'],
+                'swap': swap,
+                'ephemerals': ephemerals,
+                'block_device_mapping': block_device_mapping}
 
-            self._instance_update(context,
-                                  instance_uuid,
-                                  vm_state=vm_states.BUILDING,
-                                  task_state=task_states.SPAWNING)
-
-            # TODO(vish) check to make sure the availability zone matches
-            with utils.logging_error(_error_message(instance_uuid,
-                                                    "failed to spawn")):
-                self.driver.spawn(context, instance, image_meta,
-                                  network_info, block_device_info)
-
-            current_power_state = self._get_power_state(context, instance)
-            instance = self._instance_update(context,
-                                             instance_uuid,
-                                             power_state=current_power_state,
-                                             vm_state=vm_states.ACTIVE,
-                                             task_state=None,
-                                             launched_at=utils.utcnow())
-
-            usage_info = utils.usage_from_instance(instance)
-            notifier.notify('compute.%s' % self.host,
-                            'compute.instance.create',
-                            notifier.INFO, usage_info)
+    def _spawn(self, context, instance, image_meta, network_info,
+               block_device_info, injected_files, admin_pass):
+        """Spawn an instance with error logging and update its power state"""
+        self._instance_update(context, instance['uuid'],
+                              vm_state=vm_states.BUILDING,
+                              task_state=task_states.SPAWNING)
+        instance['injected_files'] = injected_files
+        instance['admin_pass'] = admin_pass
+        try:
+            self.driver.spawn(context, instance, image_meta,
+                              network_info, block_device_info)
+        except:
+            msg = _("Instance %s failed to spawn")
+            LOG.exception(msg % instance['uuid'])
+            raise
 
-        except exception.InstanceNotFound:
-            # FIXME(wwolf): We are just ignoring InstanceNotFound
-            # exceptions here in case the instance was immediately
-            # deleted before it actually got created.  This should
-            # be fixed once we have no-db-messaging
-            pass
-        except Exception:
-            _cleanup()
+        current_power_state = self._get_power_state(context, instance)
+        return self._instance_update(context, instance['uuid'],
+                                     power_state=current_power_state,
+                                     vm_state=vm_states.ACTIVE,
+                                     task_state=None,
+                                     launched_at=utils.utcnow())
+
+    def _notify_about_instance_usage(self, instance):
+        usage_info = utils.usage_from_instance(instance)
+        notifier.notify('compute.%s' % self.host,
+                        'compute.instance.create',
+                        notifier.INFO, usage_info)
+
+    def _deallocate_network(self, context, instance):
+        if not FLAGS.stub_network:
+            msg = _("deallocating network for instance: %s")
+            LOG.debug(msg % instance['uuid'])
+            self.network_api.deallocate_for_instance(context, instance)
 
     def _get_instance_volume_bdms(self, context, instance_id):
         bdms = self.db.block_device_mapping_get_all_by_instance(context,
@@ -515,8 +506,8 @@ class ComputeManager(manager.SchedulerDependentManager):
         return {'block_device_mapping': block_device_mapping}
 
     @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
-    def run_instance(self, context, instance_id, **kwargs):
-        self._run_instance(context, instance_id, **kwargs)
+    def run_instance(self, context, instance_uuid, **kwargs):
+        self._run_instance(context, instance_uuid, **kwargs)
 
     @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
     @checks_instance_lock_uuid