Add functional tests for boot from volume

Test both that we can boot from volume from a pre-existing image
with and without auto-termination. Also that manually making the volume
from the image and then booting from it also works.

Also, add a cache invalidation that testing showed to be an issue.

Co-Authored-By: David Shrewsbury <shrewsbury.dave@gmail.com>

Change-Id: Ia0f5801c17edacf8cce67c7594657e7ae41e0f18

shade/openstackcloud.py

@@ -3087,7 +3087,9 @@ class OpenStackCloud(object):
             kwargs['image'] = None
         elif boot_from_volume:
 
-            if not hasattr(image, 'id'):
+            if hasattr(image, 'id'):
+                image_obj = image
+            else:
                 image_obj = self.get_image(image)
             if not image_obj:
                 raise OpenStackCloudException(
@@ -3095,14 +3097,12 @@ class OpenStackCloud(object):
                     ' {cloud}:{region}'.format(
                         image=image,
                         cloud=self.name, region=self.region_name))
-            else:
-                image = image_obj
 
             block_mapping = {
                 'boot_index': '0',
                 'delete_on_termination': terminate_volume,
                 'destination_type': 'volume',
-                'uuid': image['id'],
+                'uuid': image_obj['id'],
                 'source_type': 'image',
                 'volume_size': volume_size,
             }
@@ -3124,6 +3124,8 @@ class OpenStackCloud(object):
                 'source_type': 'volume',
             }
             kwargs['block_device_mapping_v2'].append(block_mapping)
+        if boot_volume or boot_from_volume or volumes:
+            self.list_volumes.invalidate(self)
         return kwargs
 
     @_utils.valid_kwargs(
@@ -3419,6 +3421,13 @@ class OpenStackCloud(object):
             except Exception as e:
                 raise OpenStackCloudException(
                     "Error in deleting server: {0}".format(e))
+
+        # If the server has volume attachments, or if it has booted
+        # from volume, deleting it will change volume state
+        if (not server['image'] or not server['image']['id']
+                or self.get_volumes(server)):
+            self.list_volumes.invalidate(self)
+
         return True
 
     def list_containers(self, full_listing=True):

shade/tests/functional/test_compute.py

@@ -34,18 +34,36 @@ class TestCompute(base.TestCase):
         self.image = pick_image(self.cloud.list_images())
         if self.image is None:
             self.assertFalse('no sensible image available')
-        self.server_prefix = self.getUniqueString('server')
+        self.server_name = self.getUniqueString()
+
+    def _cleanup_servers_and_volumes(self, server_name):
+        """Delete the named server and any attached volumes.
+
+        Adding separate cleanup calls for servers and volumes can be tricky
+        since they need to be done in the proper order. And sometimes deleting
+        a server can start the process of deleting a volume if it is booted
+        from that volume. This encapsulates that logic.
+        """
+        server = self.cloud.get_server(server_name)
+        if not server:
+            return
+        volumes = self.cloud.get_volumes(server)
+        self.cloud.delete_server(server.name, wait=True)
+        for volume in volumes:
+            if volume.status != 'deleting':
+                self.cloud.delete_volume(volume.id, wait=True)
 
     def test_create_and_delete_server(self):
-        server_name = self.server_prefix + '_create_and_delete'
+        self.addCleanup(self._cleanup_servers_and_volumes, self.server_name)
-        server = self.cloud.create_server(name=server_name,
+        server = self.cloud.create_server(name=self.server_name,
                                           image=self.image,
                                           flavor=self.flavor,
                                           wait=True)
-        self.assertEquals(server['name'], server_name)
+        self.assertEqual(self.server_name, server['name'])
-        self.assertEquals(server['image']['id'], self.image.id)
+        self.assertEqual(self.image.id, server['image']['id'])
-        self.assertEquals(server['flavor']['id'], self.flavor.id)
+        self.assertEqual(self.flavor.id, server['flavor']['id'])
-        self.assertTrue(self.cloud.delete_server(server_name, wait=True))
+        self.assertTrue(self.cloud.delete_server(self.server_name, wait=True))
+        self.assertIsNone(self.cloud.get_server(self.server_name))
 
     def test_get_image_id(self):
         self.assertEqual(
@@ -58,3 +76,109 @@ class TestCompute(base.TestCase):
             self.image.name, self.cloud.get_image_name(self.image.id))
         self.assertEqual(
             self.image.name, self.cloud.get_image_name(self.image.name))
+
+    def _assert_volume_attach(self, server, volume_id=None):
+        self.assertEqual(self.server_name, server['name'])
+        self.assertEqual('', server['image'])
+        self.assertEqual(self.flavor.id, server['flavor']['id'])
+        volumes = self.cloud.get_volumes(server)
+        self.assertEqual(1, len(volumes))
+        volume = volumes[0]
+        if volume_id:
+            self.assertEqual(volume_id, volume['id'])
+        else:
+            volume_id = volume['id']
+        self.assertEqual(1, len(volume['attachments']), 1)
+        self.assertEqual(server['id'], volume['attachments'][0]['server_id'])
+        return volume_id
+
+    def test_create_boot_from_volume_image(self):
+        if not self.cloud.has_service('volume'):
+            self.skipTest('volume service not supported by cloud')
+        self.addCleanup(self._cleanup_servers_and_volumes, self.server_name)
+        server = self.cloud.create_server(
+            name=self.server_name,
+            image=self.image,
+            flavor=self.flavor,
+            boot_from_volume=True,
+            volume_size=1,
+            wait=True)
+        volume_id = self._assert_volume_attach(server)
+        volume = self.cloud.get_volume(volume_id)
+        self.assertIsNotNone(volume)
+        self.assertEqual(volume['name'], volume['display_name'])
+        self.assertEqual(True, volume['bootable'])
+        self.assertEqual(server['id'], volume['attachments'][0]['server_id'])
+        self.assertTrue(self.cloud.delete_server(server.id, wait=True))
+        self.assertTrue(self.cloud.delete_volume(volume.id, wait=True))
+        self.assertIsNone(self.cloud.get_server(server.id))
+        self.assertIsNone(self.cloud.get_volume(volume.id))
+
+    def test_create_terminate_volume_image(self):
+        if not self.cloud.has_service('volume'):
+            self.skipTest('volume service not supported by cloud')
+        self.addCleanup(self._cleanup_servers_and_volumes, self.server_name)
+        server = self.cloud.create_server(
+            name=self.server_name,
+            image=self.image,
+            flavor=self.flavor,
+            boot_from_volume=True,
+            terminate_volume=True,
+            volume_size=1,
+            wait=True)
+        volume_id = self._assert_volume_attach(server)
+        self.assertTrue(self.cloud.delete_server(self.server_name, wait=True))
+        volume = self.cloud.get_volume(volume_id)
+        # We can either get None (if the volume delete was quick), or a volume
+        # that is in the process of being deleted.
+        if volume:
+            self.assertEquals('deleting', volume.status)
+        self.assertIsNone(self.cloud.get_server(self.server_name))
+
+    def test_create_boot_from_volume_preexisting(self):
+        if not self.cloud.has_service('volume'):
+            self.skipTest('volume service not supported by cloud')
+        self.addCleanup(self._cleanup_servers_and_volumes, self.server_name)
+        volume = self.cloud.create_volume(
+            size=1, name=self.server_name, image=self.image, wait=True)
+        server = self.cloud.create_server(
+            name=self.server_name,
+            image=None,
+            flavor=self.flavor,
+            boot_volume=volume,
+            volume_size=1,
+            wait=True)
+        volume_id = self._assert_volume_attach(server, volume_id=volume['id'])
+        self.assertTrue(self.cloud.delete_server(self.server_name, wait=True))
+        self.addCleanup(self.cloud.delete_volume, volume_id)
+        volume = self.cloud.get_volume(volume_id)
+        self.assertIsNotNone(volume)
+        self.assertEqual(volume['name'], volume['display_name'])
+        self.assertEqual(True, volume['bootable'])
+        self.assertEqual([], volume['attachments'])
+        self.assertTrue(self.cloud.delete_volume(volume_id))
+        self.assertIsNone(self.cloud.get_server(self.server_name))
+        self.assertIsNone(self.cloud.get_volume(volume_id))
+
+    def test_create_boot_from_volume_preexisting_terminate(self):
+        if not self.cloud.has_service('volume'):
+            self.skipTest('volume service not supported by cloud')
+        self.addCleanup(self._cleanup_servers_and_volumes, self.server_name)
+        volume = self.cloud.create_volume(
+            size=1, name=self.server_name, image=self.image, wait=True)
+        server = self.cloud.create_server(
+            name=self.server_name,
+            image=None,
+            flavor=self.flavor,
+            boot_volume=volume,
+            terminate_volume=True,
+            volume_size=1,
+            wait=True)
+        volume_id = self._assert_volume_attach(server, volume_id=volume['id'])
+        self.assertTrue(self.cloud.delete_server(self.server_name, wait=True))
+        volume = self.cloud.get_volume(volume_id)
+        # We can either get None (if the volume delete was quick), or a volume
+        # that is in the process of being deleted.
+        if volume:
+            self.assertEquals('deleting', volume.status)
+        self.assertIsNone(self.cloud.get_server(self.server_name))

shade/tests/functional/test_volume.py

@@ -45,8 +45,8 @@ class TestVolume(base.TestCase):
             display_name=snapshot_name
         )
 
-        self.assertEqual([volume], self.cloud.list_volumes())
+        self.assertIn(volume, self.cloud.list_volumes())
-        self.assertEqual([snapshot], self.cloud.list_volume_snapshots())
+        self.assertIn(snapshot, self.cloud.list_volume_snapshots())
         self.assertEqual(snapshot,
                          self.cloud.get_volume_snapshot(
                              snapshot['display_name']))
@@ -54,7 +54,7 @@ class TestVolume(base.TestCase):
                          self.cloud.get_volume_snapshot_by_id(snapshot['id']))
 
         self.cloud.delete_volume_snapshot(snapshot_name, wait=True)
-        self.cloud.delete_volume(volume_name)
+        self.cloud.delete_volume(volume_name, wait=True)
 
     def cleanup(self, volume_name, snapshot_name):
         volume = self.cloud.get_volume(volume_name)