Merge "get instance details results in volumes key error"

5 files changed, 179 insertions, 42 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 797cf35bf..b077d55aa 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -850,11 +850,16 @@ class CloudController(object):
     def _format_volume(self, context, volume):
         instance_ec2_id = None
         instance_data = None
-        if volume.get('instance', None):
-            instance_id = volume['instance']['id']
+
+        if volume.get('instance_uuid', None):
+            instance_uuid = volume['instance_uuid']
+            instance = db.instance_get_by_uuid(context.elevated(),
+                    instance_uuid)
+
+            instance_id = instance['id']
             instance_ec2_id = ec2utils.id_to_ec2_id(instance_id)
             instance_data = '%s[%s]' % (instance_ec2_id,
-                                        volume['instance']['host'])
+                                        instance['host'])
         v = {}
         v['volumeId'] = ec2utils.id_to_ec2_vol_id(volume['id'])
         v['status'] = volume['status']
diff --git a/nova/api/openstack/compute/contrib/volumes.py b/nova/api/openstack/compute/contrib/volumes.py
index e3357cf2b..a51ba9229 100644
--- a/nova/api/openstack/compute/contrib/volumes.py
+++ b/nova/api/openstack/compute/contrib/volumes.py
@@ -57,7 +57,9 @@ def _translate_volume_summary_view(context, vol):
     d['createdAt'] = vol['created_at']
 
     if vol['attach_status'] == 'attached':
-        d['attachments'] = [_translate_attachment_detail_view(context, vol)]
+        d['attachments'] = [_translate_attachment_detail_view(vol['id'],
+            vol['instance_uuid'],
+            vol['mountpoint'])]
     else:
         d['attachments'] = [{}]
 
@@ -222,30 +224,29 @@ class VolumeController(object):
         return {'volume': retval}
 
 
-def _translate_attachment_detail_view(_context, vol):
+def _translate_attachment_detail_view(volume_id, instance_uuid, mountpoint):
     """Maps keys for attachment details view."""
 
-    d = _translate_attachment_summary_view(_context, vol)
+    d = _translate_attachment_summary_view(volume_id,
+            instance_uuid,
+            mountpoint)
 
     # No additional data / lookups at the moment
-
     return d
 
 
-def _translate_attachment_summary_view(_context, vol):
+def _translate_attachment_summary_view(volume_id, instance_uuid, mountpoint):
     """Maps keys for attachment summary view."""
     d = {}
 
-    volume_id = vol['id']
-
     # NOTE(justinsb): We use the volume id as the id of the attachment object
     d['id'] = volume_id
 
     d['volumeId'] = volume_id
-    if vol.get('instance'):
-        d['serverId'] = vol['instance']['uuid']
-    if vol.get('mountpoint'):
-        d['device'] = vol['mountpoint']
+
+    d['serverId'] = instance_uuid
+    if mountpoint:
+        d['device'] = mountpoint
 
     return d
 
@@ -284,7 +285,6 @@ class VolumeAttachmentController(object):
 
     def __init__(self):
         self.compute_api = compute.API()
-        self.volume_api = volume.API()
         super(VolumeAttachmentController, self).__init__()
 
     @wsgi.serializers(xml=VolumeAttachmentsTemplate)
@@ -301,19 +301,31 @@ class VolumeAttachmentController(object):
 
         volume_id = id
         try:
-            vol = self.volume_api.get(context, volume_id)
+            instance = self.compute_api.get(context, server_id)
         except exception.NotFound:
-            LOG.debug("volume_id not found")
             raise exc.HTTPNotFound()
 
-        instance = vol['instance']
-        if instance is None or str(instance['uuid']) != server_id:
-            LOG.debug("Instance not found (server_id=%(server_id)s)",
-                      {'server_id': server_id}, instance=instance)
+        bdms = self.compute_api.get_instance_bdms(context, instance)
+
+        if not bdms:
+            LOG.debug(_("Instance %s is not attached."), server_id)
             raise exc.HTTPNotFound()
 
-        return {'volumeAttachment': _translate_attachment_detail_view(context,
-                                                                      vol)}
+        assigned_mountpoint = None
+
+        for bdm in bdms:
+            if bdm['volume_id'] == volume_id:
+                assigned_mountpoint = bdm['device_name']
+                break
+
+        if assigned_mountpoint is None:
+            LOG.debug("volume_id not found")
+            raise exc.HTTPNotFound()
+
+        return {'volumeAttachment': _translate_attachment_detail_view(
+            volume_id,
+            instance['uuid'],
+            assigned_mountpoint)}
 
     @wsgi.serializers(xml=VolumeAttachmentTemplate)
     def create(self, req, server_id, body):
@@ -367,19 +379,28 @@ class VolumeAttachmentController(object):
         LOG.audit(_("Detach volume %s"), volume_id, context=context)
 
         try:
-            vol = self.volume_api.get(context, volume_id)
+            instance = self.compute_api.get(context, server_id)
         except exception.NotFound:
             raise exc.HTTPNotFound()
 
-        instance_uuid = vol['instance_uuid']
-        if instance_uuid is None or instance_uuid != server_id:
-            LOG.debug(_("Instance %s is not attached."), instance_uuid)
+        bdms = self.compute_api.get_instance_bdms(context, instance)
+
+        if not bdms:
+            LOG.debug(_("Instance %s is not attached."), server_id)
             raise exc.HTTPNotFound()
 
-        self.compute_api.detach_volume(context,
-                                       volume_id=volume_id)
+        found = False
+        for bdm in bdms:
+            if bdm['volume_id'] == volume_id:
+                self.compute_api.detach_volume(context,
+                    volume_id=volume_id)
+                found = True
+                break
 
-        return webob.Response(status_int=202)
+        if not found:
+            raise exc.HTTPNotFound()
+        else:
+            return webob.Response(status_int=202)
 
     def _items(self, req, server_id, entity_maker):
         """Returns a list of attachments, transformed through entity_maker."""
@@ -391,10 +412,17 @@ class VolumeAttachmentController(object):
         except exception.NotFound:
             raise exc.HTTPNotFound()
 
-        volumes = instance['volumes']
-        limited_list = common.limited(volumes, req)
-        res = [entity_maker(context, vol) for vol in limited_list]
-        return {'volumeAttachments': res}
+        bdms = self.compute_api.get_instance_bdms(context, instance)
+        limited_list = common.limited(bdms, req)
+        results = []
+
+        for bdm in limited_list:
+            if bdm['volume_id']:
+                results.append(entity_maker(bdm['volume_id'],
+                        bdm['instance_uuid'],
+                        bdm['device_name']))
+
+        return {'volumeAttachments': results}
 
 
 class BootFromVolumeController(servers.Controller):
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 7ede3c8ec..82bcff3f1 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1786,6 +1786,11 @@ class API(BaseAPI):
         uuids = [instance['uuid'] for instance in instances]
         return self.db.instance_fault_get_by_instance_uuids(context, uuids)
 
+    def get_instance_bdms(self, context, instance):
+        """Get all bdm tables for specified instance."""
+        return self.db.block_device_mapping_get_all_by_instance(context,
+                instance['uuid'])
+
 
 class HostAPI(BaseAPI):
     """Sub-set of the Compute Manager API for managing host operations."""
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 8a446091b..c07f2c9de 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -531,15 +531,9 @@ class BlockDeviceMapping(BASE, NovaBase):
     # for ephemeral device
     virtual_name = Column(String(255), nullable=True)
 
-    # for snapshot or volume
-    snapshot_id = Column(String(36), ForeignKey('snapshots.id'))
-    # outer join
-    snapshot = relationship(Snapshot,
-                            foreign_keys=snapshot_id)
+    snapshot_id = Column(String(36))
 
-    volume_id = Column(String(36), ForeignKey('volumes.id'), nullable=True)
-    volume = relationship(Volume,
-                          foreign_keys=volume_id)
+    volume_id = Column(String(36), nullable=True)
     volume_size = Column(Integer, nullable=True)
 
     # for no device to suppress devices.
diff --git a/nova/tests/api/openstack/compute/contrib/test_volumes.py b/nova/tests/api/openstack/compute/contrib/test_volumes.py
index 9cc914e79..92382edc7 100644
--- a/nova/tests/api/openstack/compute/contrib/test_volumes.py
+++ b/nova/tests/api/openstack/compute/contrib/test_volumes.py
@@ -28,11 +28,17 @@ from nova import flags
 from nova import test
 from nova.tests.api.openstack import fakes
 from nova import volume
+from webob import exc
 
 
 FLAGS = flags.FLAGS
 
 FAKE_UUID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
+FAKE_UUID_A = '00000000-aaaa-aaaa-aaaa-000000000000'
+FAKE_UUID_B = 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb'
+FAKE_UUID_C = 'cccccccc-cccc-cccc-cccc-cccccccccccc'
+FAKE_UUID_D = 'dddddddd-dddd-dddd-dddd-dddddddddddd'
+
 IMAGE_UUID = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77'
 
 
@@ -58,6 +64,37 @@ def fake_compute_api_create(cls, context, instance_type, image_href, **kwargs):
              }], resv_id)
 
 
+def fake_get_instance(self, context, instance_id):
+    return({'uuid': instance_id})
+
+
+def fake_attach_volume(self, context, instance, volume_id, device):
+    return()
+
+
+def fake_detach_volume(self, context, volume_id):
+    return()
+
+
+def fake_get_instance_bdms(self, context, instance):
+    return([{'id': 1,
+             'instance_uuid': instance['uuid'],
+             'device_name': '/dev/fake0',
+             'delete_on_termination': 'False',
+             'virtual_name': 'MyNamesVirtual',
+             'snapshot_id': None,
+             'volume_id': FAKE_UUID_A,
+             'volume_size': 1},
+            {'id': 2,
+             'instance_uuid':instance['uuid'],
+             'device_name': '/dev/fake1',
+             'delete_on_termination': 'False',
+             'virtual_name': 'MyNamesVirtual',
+             'snapshot_id': None,
+             'volume_id': FAKE_UUID_B,
+             'volume_size': 1}])
+
+
 class BootFromVolumeTest(test.TestCase):
 
     def setUp(self):
@@ -185,6 +222,74 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(resp.status_int, 404)
 
 
+class VolumeAttachTests(test.TestCase):
+    def setUp(self):
+        super(VolumeAttachTests, self).setUp()
+        self.stubs.Set(nova.compute.API,
+                       'get_instance_bdms',
+                       fake_get_instance_bdms)
+        self.stubs.Set(nova.compute.API, 'get', fake_get_instance)
+        self.context = context.get_admin_context()
+        self.expected_show = {'volumeAttachment':
+            {'device': '/dev/fake0',
+             'serverId': FAKE_UUID,
+             'id': FAKE_UUID_A,
+             'volumeId': FAKE_UUID_A
+            }}
+
+    def test_show(self):
+        attachments = volumes.VolumeAttachmentController()
+        req = webob.Request.blank('/v2/fake/os-volumes/show')
+        req.method = 'POST'
+        req.body = json.dumps({})
+        req.headers['content-type'] = 'application/json'
+        req.environ['nova.context'] = self.context
+
+        result = attachments.show(req, FAKE_UUID, FAKE_UUID_A)
+        self.assertEqual(self.expected_show, result)
+
+    def test_delete(self):
+        self.stubs.Set(nova.compute.API, 'detach_volume', fake_detach_volume)
+        attachments = volumes.VolumeAttachmentController()
+        req = webob.Request.blank('/v2/fake/os-volumes/delete')
+        req.method = 'POST'
+        req.body = json.dumps({})
+        req.headers['content-type'] = 'application/json'
+        req.environ['nova.context'] = self.context
+
+        result = attachments.delete(req, FAKE_UUID, FAKE_UUID_A)
+        self.assertEqual('202 Accepted', result.status)
+
+    def test_delete_vol_not_found(self):
+        self.stubs.Set(nova.compute.API, 'detach_volume', fake_detach_volume)
+        attachments = volumes.VolumeAttachmentController()
+        req = webob.Request.blank('/v2/fake/os-volumes/delete')
+        req.method = 'POST'
+        req.body = json.dumps({})
+        req.headers['content-type'] = 'application/json'
+        req.environ['nova.context'] = self.context
+
+        self.assertRaises(exc.HTTPNotFound,
+                          attachments.delete,
+                          req,
+                          FAKE_UUID,
+                          FAKE_UUID_C)
+
+    def test_attach_volume(self):
+        self.stubs.Set(nova.compute.API, 'attach_volume', fake_attach_volume)
+        attachments = volumes.VolumeAttachmentController()
+        body = {'volumeAttachment': {'volumeId': FAKE_UUID_A,
+                                    'device': '/dev/fake'}}
+        req = webob.Request.blank('/v2/fake/os-volumes/attach')
+        req.method = 'POST'
+        req.body = json.dumps({})
+        req.headers['content-type'] = 'application/json'
+        req.environ['nova.context'] = self.context
+        result = attachments.create(req, FAKE_UUID, body)
+        self.assertEqual(result['volumeAttachment']['id'],
+            '00000000-aaaa-aaaa-aaaa-000000000000')
+
+
 class VolumeSerializerTest(test.TestCase):
     def _verify_volume_attachment(self, attach, tree):
         for attr in ('id', 'volumeId', 'serverId', 'device'):