Boot from volume without image supplied

This patch allows for booting instances without supplying an image if
there is block device mapping supplied. It makes changes to nova API
and
compute services to handle requests that do not have any image
supplied.

Also it makes rescue and rebuild work with instances started from
volume.

Finally the patch introduces tests to make sure the system acts as
expected, and in the process fixes and refactors some old tests to make
them test for cases this new functionality can introduce.

This patch is intended to be a proof of concept and a first step
towards
a more cleaner interface for booting from volumes, outlined in
https://etherpad.openstack.org/grizzly-boot-from-volumes.

This patch also introduces a slight modification of the nova API so I
am flagging it with DocImpact. The change is that if the os-volumes
extension is used ImageRef does not need to be supplied to the create
server API call provided there is block_device_mapping provided.

Also note that this is the first step towards introducing a 'volume'
parameter
for starting instances which will replace the somewhat unintuitive
block_device_mapping (they will still be used but not for the boot
device).

This patch is coupled with I5ba9b0f35a5084aa91eca260f46cac83b8b6591e
that provides changes to the nova client.

Implements: blueprint improve-boot-from-volume
Fixes bug #1008622

Change-Id: I530760cfaa5eb0cae590c7383e0840c6b3f896b9

2 files changed, 69 insertions, 39 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 42db85a66..07a3a3afe 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -317,8 +317,7 @@ class API(base.Base):
         self.network_api.validate_networks(context, requested_networks)
 
     @staticmethod
-    def _handle_kernel_and_ramdisk(context, kernel_id, ramdisk_id, image,
-                                   image_service):
+    def _handle_kernel_and_ramdisk(context, kernel_id, ramdisk_id, image):
         """Choose kernel and ramdisk appropriate for the instance.
 
         The kernel and ramdisk can be chosen in one of three ways:
@@ -330,11 +329,13 @@ class API(base.Base):
             3. Forced to None by using `null_kernel` FLAG.
         """
         # Inherit from image if not specified
+        image_properties = image.get('properties', {})
+
         if kernel_id is None:
-            kernel_id = image['properties'].get('kernel_id')
+            kernel_id = image_properties.get('kernel_id')
 
         if ramdisk_id is None:
-            ramdisk_id = image['properties'].get('ramdisk_id')
+            ramdisk_id = image_properties.get('ramdisk_id')
 
         # Force to None if using null_kernel
         if kernel_id == str(CONF.null_kernel):
@@ -343,9 +344,13 @@ class API(base.Base):
 
         # Verify kernel and ramdisk exist (fail-fast)
         if kernel_id is not None:
+            image_service, kernel_id = glance.get_remote_image_service(
+                context, kernel_id)
             image_service.show(context, kernel_id)
 
         if ramdisk_id is not None:
+            image_service, ramdisk_id = glance.get_remote_image_service(
+                context, ramdisk_id)
             image_service.show(context, ramdisk_id)
 
         return kernel_id, ramdisk_id
@@ -367,9 +372,11 @@ class API(base.Base):
 
     @staticmethod
     def _inherit_properties_from_image(image, auto_disk_config):
+        image_properties = image.get('properties', {})
+
         def prop(prop_, prop_type=None):
             """Return the value of an image property."""
-            value = image['properties'].get(prop_)
+            value = image_properties.get(prop_)
 
             if value is not None:
                 if prop_type == 'bool':
@@ -435,17 +442,23 @@ class API(base.Base):
             self._check_injected_file_quota(context, injected_files)
             self._check_requested_networks(context, requested_networks)
 
-            (image_service, image_id) = glance.get_remote_image_service(
-                    context, image_href)
-            image = image_service.show(context, image_id)
-            if image['status'] != 'active':
-                raise exception.ImageNotActive(image_id=image_id)
+            if image_href:
+                (image_service, image_id) = glance.get_remote_image_service(
+                        context, image_href)
+                image = image_service.show(context, image_id)
+                if image['status'] != 'active':
+                    raise exception.ImageNotActive(image_id=image_id)
+            else:
+                image = {}
 
             if instance_type['memory_mb'] < int(image.get('min_ram') or 0):
                 raise exception.InstanceTypeMemoryTooSmall()
             if instance_type['root_gb'] < int(image.get('min_disk') or 0):
                 raise exception.InstanceTypeDiskTooSmall()
 
+            kernel_id, ramdisk_id = self._handle_kernel_and_ramdisk(
+                    context, kernel_id, ramdisk_id, image)
+
             # Handle config_drive
             config_drive_id = None
             if config_drive and config_drive is not True:
@@ -454,10 +467,9 @@ class API(base.Base):
                 config_drive = None
 
                 # Ensure config_drive image exists
-                image_service.show(context, config_drive_id)
-
-            kernel_id, ramdisk_id = self._handle_kernel_and_ramdisk(
-                    context, kernel_id, ramdisk_id, image, image_service)
+                cd_image_service, config_drive_id = \
+                    glance.get_remote_image_service(context, config_drive_id)
+                cd_image_service.show(context, config_drive_id)
 
             if key_data is None and key_name:
                 key_pair = self.db.key_pair_get(context, context.user_id,
@@ -467,11 +479,8 @@ class API(base.Base):
             if reservation_id is None:
                 reservation_id = utils.generate_uid('r')
 
-            # grab the architecture from glance
-            architecture = image['properties'].get('architecture', 'Unknown')
-
             root_device_name = block_device.properties_root_device_name(
-                image['properties'])
+                image.get('properties', {}))
 
             availability_zone, forced_host = self._handle_availability_zone(
                     availability_zone)
@@ -505,7 +514,6 @@ class API(base.Base):
                 'access_ip_v6': access_ip_v6,
                 'availability_zone': availability_zone,
                 'root_device_name': root_device_name,
-                'architecture': architecture,
                 'progress': 0}
 
             if user_data:
@@ -663,12 +671,13 @@ class API(base.Base):
         # require elevated context?
         elevated = context.elevated()
         instance_uuid = instance['uuid']
-        mappings = image['properties'].get('mappings', [])
+        image_properties = image.get('properties', {})
+        mappings = image_properties.get('mappings', [])
         if mappings:
             self._update_image_block_device_mapping(elevated,
                     instance_type, instance_uuid, mappings)
 
-        image_bdm = image['properties'].get('block_device_mapping', [])
+        image_bdm = image_properties.get('block_device_mapping', [])
         for mapping in (image_bdm, block_device_mapping):
             if not mapping:
                 continue
@@ -678,9 +687,10 @@ class API(base.Base):
     def _populate_instance_shutdown_terminate(self, instance, image,
                                               block_device_mapping):
         """Populate instance shutdown_terminate information."""
+        image_properties = image.get('properties', {})
         if (block_device_mapping or
-            image['properties'].get('mappings') or
-            image['properties'].get('block_device_mapping')):
+            image_properties.get('mappings') or
+            image_properties.get('block_device_mapping')):
             instance['shutdown_terminate'] = False
 
     def _populate_instance_names(self, instance):
@@ -701,6 +711,7 @@ class API(base.Base):
     def _populate_instance_for_create(self, base_options, image,
             security_groups):
         """Build the beginning of a new instance."""
+        image_properties = image.get('properties', {})
 
         instance = base_options
         if not instance.get('uuid'):
@@ -716,13 +727,13 @@ class API(base.Base):
         # Store image properties so we can use them later
         # (for notifications, etc).  Only store what we can.
         instance.setdefault('system_metadata', {})
-        for key, value in image['properties'].iteritems():
+        for key, value in image_properties.iteritems():
             new_value = str(value)[:255]
             instance['system_metadata']['image_%s' % key] = new_value
 
         # Keep a record of the original base image that this
         # image's instance is derived from:
-        base_image_ref = image['properties'].get('base_image_ref')
+        base_image_ref = image_properties.get('base_image_ref')
         if not base_image_ref:
             # base image ref property not previously set through a snapshot.
             # default to using the image ref as the base:
@@ -1509,8 +1520,12 @@ class API(base.Base):
     def rebuild(self, context, instance, image_href, admin_password, **kwargs):
         """Rebuild the given instance with the provided attributes."""
 
-        orig_image_ref = instance['image_ref']
-        image = self._get_image(context, image_href)
+        if instance['image_ref']:
+            orig_image_ref = instance['image_ref']
+            image = self._get_image(context, image_href)
+        else:
+            orig_image_ref = ''
+            image = {}
 
         files_to_inject = kwargs.pop('files_to_inject', [])
         self._check_injected_file_quota(context, files_to_inject)
@@ -1524,11 +1539,14 @@ class API(base.Base):
         if instance_type['root_gb'] < int(image.get('min_disk') or 0):
             raise exception.InstanceTypeDiskTooSmall()
 
-        (image_service, image_id) = glance.get_remote_image_service(context,
-                                                                 image_href)
-        image = image_service.show(context, image_id)
+        if image_href:
+            (image_service, image_id) = glance.get_remote_image_service(
+                context, image_href)
+            image = image_service.show(context, image_id)
+        else:
+            image = {}
         kernel_id, ramdisk_id = self._handle_kernel_and_ramdisk(
-                context, None, None, image, image_service)
+                context, None, None, image)
 
         def _reset_image_metadata():
             """
@@ -1551,7 +1569,7 @@ class API(base.Base):
                 if key.startswith('image_'):
                     del sys_metadata[key]
             # Add the new ones
-            for key, value in image['properties'].iteritems():
+            for key, value in image.get('properties', {}).iteritems():
                 new_value = str(value)[:255]
                 sys_metadata['image_%s' % key] = new_value
             self.db.instance_system_metadata_update(context,
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 4baeab255..87b500689 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -585,7 +585,11 @@ class ComputeManager(manager.SchedulerDependentManager):
                 LOG.debug(_("No node specified, defaulting to %(node)s") %
                           locals())
 
-            extra_usage_info = {"image_name": image_meta['name']}
+            if image_meta:
+                extra_usage_info = {"image_name": image_meta['name']}
+            else:
+                extra_usage_info = {}
+
             self._start_building(context, instance)
             self._notify_about_instance_usage(
                     context, instance, "create.start",
@@ -766,7 +770,10 @@ class ComputeManager(manager.SchedulerDependentManager):
                image, but is accurate because it reflects the image's
                actual size.
         """
-        image_meta = _get_image_meta(context, instance['image_ref'])
+        if instance['image_ref']:
+            image_meta = _get_image_meta(context, instance['image_ref'])
+        else:  # Instance was started from volume - so no image ref
+            return {}
 
         try:
             size_bytes = image_meta['size']
@@ -1201,7 +1208,10 @@ class ComputeManager(manager.SchedulerDependentManager):
             LOG.audit(_("Rebuilding instance"), context=context,
                       instance=instance)
 
-            image_meta = _get_image_meta(context, image_ref)
+            if image_ref:
+                image_meta = _get_image_meta(context, image_ref)
+            else:
+                image_meta = {}
 
             # This instance.exists message should contain the original
             # image_ref, not the new one.  Since the DB has been updated
@@ -1213,7 +1223,7 @@ class ComputeManager(manager.SchedulerDependentManager):
                     extra_usage_info=extra_usage_info)
 
             # This message should contain the new image_ref
-            extra_usage_info = {'image_name': image_meta['name']}
+            extra_usage_info = {'image_name': image_meta.get('name', '')}
             self._notify_about_instance_usage(context, instance,
                     "rebuild.start", extra_usage_info=extra_usage_info)
 
@@ -1554,10 +1564,12 @@ class ComputeManager(manager.SchedulerDependentManager):
 
         network_info = self._get_instance_nw_info(context, instance)
 
-        # Boot the instance using the 'base' image instead of the user's
-        # current (possibly broken) image
         rescue_image_ref = self._get_rescue_image_ref(context, instance)
-        rescue_image_meta = _get_image_meta(context, rescue_image_ref)
+
+        if rescue_image_ref:
+            rescue_image_meta = _get_image_meta(context, rescue_image_ref)
+        else:
+            rescue_image_meta = {}
 
         with self._error_out_instance_on_exception(context, instance['uuid']):
             self.driver.rescue(context, instance,