diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index b4b7ae6ce..08a269986 100644 --- a/shade/openstackcloud.py +++ b/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): diff --git a/shade/tests/functional/test_compute.py b/shade/tests/functional/test_compute.py index 637261f9e..1c62dcc59 100644 --- a/shade/tests/functional/test_compute.py +++ b/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' - server = self.cloud.create_server(name=server_name, + 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, wait=True) - self.assertEquals(server['name'], server_name) - self.assertEquals(server['image']['id'], self.image.id) - self.assertEquals(server['flavor']['id'], self.flavor.id) - self.assertTrue(self.cloud.delete_server(server_name, wait=True)) + self.assertEqual(self.server_name, server['name']) + self.assertEqual(self.image.id, server['image']['id']) + self.assertEqual(self.flavor.id, server['flavor']['id']) + 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)) diff --git a/shade/tests/functional/test_volume.py b/shade/tests/functional/test_volume.py index eff06041f..11b4f4ece 100644 --- a/shade/tests/functional/test_volume.py +++ b/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.assertEqual([snapshot], self.cloud.list_volume_snapshots()) + self.assertIn(volume, self.cloud.list_volumes()) + 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)