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, 185 insertions, 76 deletions

diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py
index e7f4d08a0..d8c388865 100644
--- a/nova/tests/api/openstack/compute/test_servers.py
+++ b/nova/tests/api/openstack/compute/test_servers.py
@@ -2006,9 +2006,11 @@ class ServersControllerCreateTest(test.TestCase):
         fakes.stub_out_key_pair_funcs(self.stubs, have_key_pair=False)
         self._test_create_instance()
 
-    def _test_create_extra(self, params):
+    def _test_create_extra(self, params, no_image=False):
         image_uuid = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77'
         server = dict(name='server_test', imageRef=image_uuid, flavorRef=2)
+        if no_image:
+            server.pop('imageRef', None)
         server.update(params)
         body = dict(server=server)
         req = fakes.HTTPRequest.blank('/v2/fake/servers')
@@ -2109,6 +2111,40 @@ class ServersControllerCreateTest(test.TestCase):
         self.stubs.Set(compute_api.API, 'create', create)
         self._test_create_extra(params)
 
+    def test_create_instance_with_volumes_enabled_no_image(self):
+        """
+        Test that the create will fail if there is no image
+        and no bdms supplied in the request
+        """
+        self.ext_mgr.extensions = {'os-volumes': 'fake'}
+        old_create = compute_api.API.create
+
+        def create(*args, **kwargs):
+            self.assertNotIn('imageRef', kwargs)
+            return old_create(*args, **kwargs)
+
+        self.stubs.Set(compute_api.API, 'create', create)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self._test_create_extra, {}, no_image=True)
+
+    def test_create_instance_with_volumes_enabled_and_bdms_no_image(self):
+        """
+        Test that the create works if there is no image supplied but
+        os-volumes extension is enabled and bdms are supplied
+        """
+        self.ext_mgr.extensions = {'os-volumes': 'fake'}
+        bdm = [{'device_name': 'foo'}]
+        params = {'block_device_mapping': bdm}
+        old_create = compute_api.API.create
+
+        def create(*args, **kwargs):
+            self.assertEqual(kwargs['block_device_mapping'], bdm)
+            self.assertNotIn('imageRef', kwargs)
+            return old_create(*args, **kwargs)
+
+        self.stubs.Set(compute_api.API, 'create', create)
+        self._test_create_extra(params, no_image=True)
+
     def test_create_instance_with_volumes_disabled(self):
         bdm = [{'device_name': 'foo'}]
         params = {'block_device_mapping': bdm}
@@ -3801,6 +3837,11 @@ class ServersViewBuilderTest(test.TestCase):
         output = self.view_builder.show(self.request, self.instance)
         self.assertThat(output, matchers.DictMatches(expected_server))
 
+    def test_build_server_no_image(self):
+        self.instance["image_ref"] = ""
+        output = self.view_builder.show(self.request, self.instance)
+        self.assertEqual(output['server']['image'], "")
+
     def test_build_server_detail_with_fault(self):
         self.instance['vm_state'] = vm_states.ERROR
         self.instance['fault'] = {
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index c4be414da..f99dc5281 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -125,12 +125,15 @@ class BaseTestCase(test.TestCase):
         test_notifier.NOTIFICATIONS = []
 
         def fake_show(meh, context, id):
-            return {'id': id, 'min_disk': None, 'min_ram': None,
-                    'name': 'fake_name',
-                    'status': 'active',
-                    'properties': {'kernel_id': 'fake_kernel_id',
-                                   'ramdisk_id': 'fake_ramdisk_id',
-                                   'something_else': 'meow'}}
+            if id:
+                return {'id': id, 'min_disk': None, 'min_ram': None,
+                        'name': 'fake_name',
+                        'status': 'active',
+                        'properties': {'kernel_id': 'fake_kernel_id',
+                                       'ramdisk_id': 'fake_ramdisk_id',
+                                       'something_else': 'meow'}}
+            else:
+                raise exception.ImageNotFound(image_id=id)
 
         fake_image.stub_out_image_service(self.stubs)
         self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
@@ -521,6 +524,14 @@ class ComputeTestCase(BaseTestCase):
 
         self.assertEqual(NODENAME, instance['node'])
 
+    def test_create_instance_no_image(self):
+        """Create instance with no image provided"""
+        params = {'image_ref': ''}
+        instance = self._create_fake_instance(params)
+        self.compute.run_instance(self.context, instance=instance)
+        self._assert_state({'vm_state': vm_states.ACTIVE,
+                            'task_state': None})
+
     def test_default_access_ip(self):
         self.flags(default_access_ip_network_name='test1')
         fake_network.unset_stub_network_methods(self.stubs)
@@ -682,6 +693,21 @@ class ComputeTestCase(BaseTestCase):
                                                            instance['uuid'])
         self.assertEqual(len(bdms), 0)
 
+    def test_run_terminate_no_image(self):
+        """
+        Make sure instance started without image (from volume)
+        can be termintad without issues
+        """
+        params = {'image_ref': ''}
+        instance = self._create_fake_instance(params)
+        self.compute.run_instance(self.context, instance=instance)
+        self._assert_state({'vm_state': vm_states.ACTIVE,
+                            'task_state': None})
+
+        self.compute.terminate_instance(self.context, instance=instance)
+        instances = db.instance_get_all(self.context)
+        self.assertEqual(len(instances), 0)
+
     def test_terminate_no_network(self):
         # This is as reported in LP bug 1008875
         instance = jsonutils.to_primitive(self._create_fake_instance())
@@ -775,6 +801,18 @@ class ComputeTestCase(BaseTestCase):
         self.compute.start_instance(self.context, instance=instance)
         self.compute.terminate_instance(self.context, instance=instance)
 
+    def test_stop_start_no_image(self):
+        params = {'image_ref': ''}
+        instance = self._create_fake_instance(params)
+        self.compute.run_instance(self.context, instance=instance)
+        db.instance_update(self.context, instance['uuid'],
+                           {"task_state": task_states.POWERING_OFF})
+        self.compute.stop_instance(self.context, instance=instance)
+        db.instance_update(self.context, instance['uuid'],
+                           {"task_state": task_states.POWERING_ON})
+        self.compute.start_instance(self.context, instance=instance)
+        self.compute.terminate_instance(self.context, instance=instance)
+
     def test_rescue(self):
         """Ensure instance can be rescued and unrescued"""
 
@@ -808,6 +846,18 @@ class ComputeTestCase(BaseTestCase):
 
         self.compute.terminate_instance(self.context, instance=instance)
 
+    def test_rescue_no_image(self):
+        params = {'image_ref': ''}
+        instance = self._create_fake_instance(params)
+        instance_uuid = instance['uuid']
+        self.compute.run_instance(self.context, instance=instance)
+        db.instance_update(self.context, instance_uuid,
+                           {"task_state": task_states.RESCUING})
+        self.compute.rescue_instance(self.context, instance=instance)
+        db.instance_update(self.context, instance_uuid,
+                           {"task_state": task_states.UNRESCUING})
+        self.compute.unrescue_instance(self.context, instance=instance)
+
     def test_power_on(self):
         """Ensure instance can be powered on"""
 
@@ -904,6 +954,21 @@ class ComputeTestCase(BaseTestCase):
                                       bdms=[])
         self.compute.terminate_instance(self.context, instance=instance)
 
+    def test_rebuild_no_image(self):
+        """Ensure instance can be rebuilt when started with no image"""
+        params = {'image_ref': ''}
+        instance = self._create_fake_instance(params)
+        sys_metadata = db.instance_system_metadata_get(self.context,
+                        instance['uuid'])
+        self.compute.run_instance(self.context, instance=instance)
+        db.instance_update(self.context, instance['uuid'],
+                           {"task_state": task_states.REBUILDING})
+        self.compute.rebuild_instance(self.context, instance,
+                                      '', '', injected_files=[],
+                                      new_pass="new_password",
+                                      orig_sys_metadata=sys_metadata)
+        self.compute.terminate_instance(self.context, instance=instance)
+
     def test_rebuild_launch_time(self):
         """Ensure instance can be rebuilt"""
         old_time = datetime.datetime(2012, 4, 1)
@@ -1190,6 +1255,16 @@ class ComputeTestCase(BaseTestCase):
         self.compute.snapshot_instance(self.context, name, instance=instance)
         self.compute.terminate_instance(self.context, instance=instance)
 
+    def test_snapshot_no_image(self):
+        params = {'image_ref': ''}
+        name = "myfakesnapshot"
+        instance = self._create_fake_instance(params)
+        self.compute.run_instance(self.context, instance=instance)
+        db.instance_update(self.context, instance['uuid'],
+                           {"task_state": task_states.IMAGE_SNAPSHOT})
+        self.compute.snapshot_instance(self.context, name, instance=instance)
+        self.compute.terminate_instance(self.context, instance=instance)
+
     def test_snapshot_fails(self):
         """Ensure task_state is set to None if snapshot fails"""
         def fake_snapshot(*args, **kwargs):
@@ -2920,6 +2995,14 @@ class ComputeAPITestCase(BaseTestCase):
                            'ramdisk_id': 'fake_ramdisk_id'},
         }
 
+        def fake_show(obj, context, image_id):
+            if image_id:
+                return self.fake_image
+            else:
+                raise exception.ImageNotFound(image_id=image_id)
+
+        self.fake_show = fake_show
+
     def _run_instance(self, params=None):
         instance = jsonutils.to_primitive(self._create_fake_instance(params))
         instance_uuid = instance['uuid']
@@ -2935,19 +3018,17 @@ class ComputeAPITestCase(BaseTestCase):
         inst_type = instance_types.get_default_instance_type()
         inst_type['memory_mb'] = 1
 
-        def fake_show(*args):
-            img = copy.copy(self.fake_image)
-            img['min_ram'] = 2
-            return img
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        self.fake_image['min_ram'] = 2
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         self.assertRaises(exception.InstanceTypeMemoryTooSmall,
-            self.compute_api.create, self.context, inst_type, None)
+            self.compute_api.create, self.context,
+            inst_type, self.fake_image['id'])
 
         # Now increase the inst_type memory and make sure all is fine.
         inst_type['memory_mb'] = 2
         (refs, resv_id) = self.compute_api.create(self.context,
-                inst_type, None)
+                inst_type, self.fake_image['id'])
         db.instance_destroy(self.context, refs[0]['uuid'])
 
     def test_create_with_too_little_disk(self):
@@ -2956,19 +3037,17 @@ class ComputeAPITestCase(BaseTestCase):
         inst_type = instance_types.get_default_instance_type()
         inst_type['root_gb'] = 1
 
-        def fake_show(*args):
-            img = copy.copy(self.fake_image)
-            img['min_disk'] = 2
-            return img
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        self.fake_image['min_disk'] = 2
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         self.assertRaises(exception.InstanceTypeDiskTooSmall,
-            self.compute_api.create, self.context, inst_type, None)
+            self.compute_api.create, self.context,
+            inst_type, self.fake_image['id'])
 
         # Now increase the inst_type disk space and make sure all is fine.
         inst_type['root_gb'] = 2
         (refs, resv_id) = self.compute_api.create(self.context,
-                inst_type, None)
+                inst_type, self.fake_image['id'])
         db.instance_destroy(self.context, refs[0]['uuid'])
 
     def test_create_just_enough_ram_and_disk(self):
@@ -2978,16 +3057,13 @@ class ComputeAPITestCase(BaseTestCase):
         inst_type['root_gb'] = 2
         inst_type['memory_mb'] = 2
 
-        def fake_show(*args):
-            img = copy.copy(self.fake_image)
-            img['min_ram'] = 2
-            img['min_disk'] = 2
-            img['name'] = 'fake_name'
-            return img
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        self.fake_image['min_ram'] = 2
+        self.fake_image['min_disk'] = 2
+        self.fake_image['name'] = 'fake_name'
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         (refs, resv_id) = self.compute_api.create(self.context,
-                inst_type, None)
+                inst_type, self.fake_image['id'])
         db.instance_destroy(self.context, refs[0]['uuid'])
 
     def test_create_with_no_ram_and_disk_reqs(self):
@@ -2997,12 +3073,10 @@ class ComputeAPITestCase(BaseTestCase):
         inst_type['root_gb'] = 1
         inst_type['memory_mb'] = 1
 
-        def fake_show(*args):
-            return copy.copy(self.fake_image)
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         (refs, resv_id) = self.compute_api.create(self.context,
-                inst_type, None)
+                inst_type, self.fake_image['id'])
         db.instance_destroy(self.context, refs[0]['uuid'])
 
     def test_create_instance_defaults_display_name(self):
@@ -3010,7 +3084,8 @@ class ComputeAPITestCase(BaseTestCase):
         cases = [dict(), dict(display_name=None)]
         for instance in cases:
             (ref, resv_id) = self.compute_api.create(self.context,
-                instance_types.get_default_instance_type(), None, **instance)
+                instance_types.get_default_instance_type(),
+                'fake-image-uuid', **instance)
             try:
                 self.assertNotEqual(ref[0]['display_name'], None)
             finally:
@@ -3021,7 +3096,7 @@ class ComputeAPITestCase(BaseTestCase):
         (ref, resv_id) = self.compute_api.create(
                 self.context,
                 instance_type=instance_types.get_default_instance_type(),
-                image_href=None)
+                image_href='fake-image-uuid')
         try:
             sys_metadata = db.instance_system_metadata_get(self.context,
                     ref[0]['uuid'])
@@ -3071,46 +3146,37 @@ class ComputeAPITestCase(BaseTestCase):
 
         inst_type = instance_types.get_default_instance_type()
 
-        def fake_show(*args):
-            img = copy.copy(self.fake_image)
-            img['min_ram'] = 2
-            return img
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        self.fake_image['min_ram'] = 2
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         self.assertRaises(exception.InstanceUserDataTooLarge,
-            self.compute_api.create, self.context, inst_type, None,
-                          user_data=('1' * 65536))
+            self.compute_api.create, self.context, inst_type,
+            self.fake_image['id'], user_data=('1' * 65536))
 
     def test_create_with_malformed_user_data(self):
         """Test an instance type with malformed user data."""
 
         inst_type = instance_types.get_default_instance_type()
 
-        def fake_show(*args):
-            img = copy.copy(self.fake_image)
-            img['min_ram'] = 2
-            return img
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        self.fake_image['min_ram'] = 2
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         self.assertRaises(exception.InstanceUserDataMalformed,
-            self.compute_api.create, self.context, inst_type, None,
-                          user_data='banana')
+            self.compute_api.create, self.context, inst_type,
+            self.fake_image['id'], user_data='banana')
 
     def test_create_with_base64_user_data(self):
         """Test an instance type with ok much user data."""
 
         inst_type = instance_types.get_default_instance_type()
 
-        def fake_show(*args):
-            img = copy.copy(self.fake_image)
-            img['min_ram'] = 2
-            return img
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        self.fake_image['min_ram'] = 2
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         # NOTE(mikal): a string of length 48510 encodes to 65532 characters of
         # base64
         (refs, resv_id) = self.compute_api.create(
-            self.context, inst_type, None,
+            self.context, inst_type, self.fake_image['id'],
             user_data=base64.encodestring('1' * 48510))
         db.instance_destroy(self.context, refs[0]['uuid'])
 
@@ -3564,6 +3630,17 @@ class ComputeAPITestCase(BaseTestCase):
                 'preserved': 'preserve this!'})
         db.instance_destroy(self.context, instance['uuid'])
 
+    def test_rebuild_no_image(self):
+        instance = jsonutils.to_primitive(
+            self._create_fake_instance(params={'image_ref': ''}))
+        instance_uuid = instance['uuid']
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
+        self.compute.run_instance(self.context, instance=instance)
+        self.compute_api.rebuild(self.context, instance, '', 'new_password')
+
+        instance = db.instance_get_by_uuid(self.context, instance_uuid)
+        self.assertEqual(instance['task_state'], task_states.REBUILDING)
+
     def _stub_out_reboot(self, device_name):
         def fake_reboot_instance(rpcapi, context, instance,
                                  block_device_info,
@@ -3752,11 +3829,8 @@ class ComputeAPITestCase(BaseTestCase):
         and min_disk set to that of the original instances flavor.
         """
 
-        def fake_show(*args):
-            img = copy.copy(self.fake_image)
-            img['disk_format'] = 'vhd'
-            return img
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        self.fake_image['disk_format'] = 'vhd'
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         instance = self._create_fake_instance()
         inst_params = {'root_gb': 2, 'memory_mb': 256}
@@ -3784,13 +3858,10 @@ class ComputeAPITestCase(BaseTestCase):
         image had a disk format of vhd.
         """
 
-        def fake_show(*args):
-            img = copy.copy(self.fake_image)
-            img['disk_format'] = 'raw'
-            img['min_ram'] = 512
-            img['min_disk'] = 1
-            return img
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        self.fake_image['disk_format'] = 'raw'
+        self.fake_image['min_ram'] = 512
+        self.fake_image['min_disk'] = 1
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         instance = self._create_fake_instance()
 
@@ -3814,12 +3885,9 @@ class ComputeAPITestCase(BaseTestCase):
         Do not show an attribute that the orig img did not have.
         """
 
-        def fake_show(*args):
-            img = copy.copy(self.fake_image)
-            img['disk_format'] = 'raw'
-            img['min_disk'] = 1
-            return img
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        self.fake_image['disk_format'] = 'raw'
+        self.fake_image['min_disk'] = 1
+        self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         instance = self._create_fake_instance()
 
@@ -5654,7 +5722,7 @@ class DisabledInstanceTypesTestCase(BaseTestCase):
 
     def test_can_rebuild_instance_from_visible_instance_type(self):
         instance = self._create_fake_instance()
-        image_href = None
+        image_href = 'fake-image-id'
         admin_password = 'blah'
 
         instance['instance_type']['disabled'] = True
@@ -5670,7 +5738,7 @@ class DisabledInstanceTypesTestCase(BaseTestCase):
         when the slice is on disabled type already.
         """
         instance = self._create_fake_instance()
-        image_href = None
+        image_href = 'fake-image-id'
         admin_password = 'blah'
 
         instance['instance_type']['disabled'] = True
@@ -6068,4 +6136,4 @@ class ComputeInactiveImageTestCase(BaseTestCase):
         inst_type = instance_types.get_instance_type_by_name('m1.tiny')
         self.assertRaises(exception.ImageNotActive,
                           self.compute_api.create,
-                          self.context, inst_type, None)
+                          self.context, inst_type, 'fake-image-uuid')