openstack
/
nova
Code Issues Proposed changes

Remove compute API get_instance_bdms method 

This method is just an unnecessary level of indirecton, now that we have
BDM objects that give us a nice API on top of db calls, so we can remove
it,

Part of blueprint: icehouse-objects
Part of blueprint: clean-up-legacy-block-device-mapping

Change-Id: I02226cfb67955b604d6bed513211ef46fb65dc69
This commit is contained in:
Nikola Dipanov 2014-02-06 17:35:19 +01:00
parent 55b54ac9e6
commit 4390d190b5
11 changed files with 119 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
nova/api/openstack/compute/contrib/extended_volumes.py
View File

@ -18,6 +18,7 @@ from nova.api.openstack import extensions
from nova.api.openstack import wsgi
from nova.api.openstack import xmlutil
from nova import compute
from nova.objects import block_device as block_device_obj
authorize = extensions.soft_extension_authorizer('compute', 'extended_volumes')
@ -28,8 +29,9 @@ class ExtendedVolumesController(wsgi.Controller):
self.compute_api = compute.API()
def _extend_server(self, context, server, instance):
bdms = self.compute_api.get_instance_bdms(context, instance)
volume_ids = [bdm['volume_id'] for bdm in bdms if bdm['volume_id']]
bdms = block_device_obj.BlockDeviceMappingList.get_by_instance_uuid(
context, instance['uuid'])
volume_ids = [bdm.volume_id for bdm in bdms if bdm.volume_id]
key = "%s:volumes_attached" % Extended_volumes.alias
server[key] = [{'id': volume_id} for volume_id in volume_ids]

29
nova/api/openstack/compute/contrib/volumes.py
View File

@ -24,6 +24,7 @@ from nova.api.openstack import wsgi
from nova.api.openstack import xmlutil
from nova import compute
from nova import exception
from nova.objects import block_device as block_device_obj
from nova.openstack.common.gettextutils import _
from nova.openstack.common import log as logging
from nova.openstack.common import strutils
@ -354,7 +355,8 @@ class VolumeAttachmentController(wsgi.Controller):
except exception.NotFound:
raise exc.HTTPNotFound()
bdms = self.compute_api.get_instance_bdms(context, instance)
bdms = block_device_obj.BlockDeviceMappingList.get_by_instance_uuid(
context, instance['uuid'])
if not bdms:
LOG.debug(_("Instance %s is not attached."), server_id)
@ -363,8 +365,8 @@ class VolumeAttachmentController(wsgi.Controller):
assigned_mountpoint = None
for bdm in bdms:
if bdm['volume_id'] == volume_id:
assigned_mountpoint = bdm['device_name']
if bdm.volume_id == volume_id:
assigned_mountpoint = bdm.device_name
break
if assigned_mountpoint is None:
@ -459,11 +461,12 @@ class VolumeAttachmentController(wsgi.Controller):
except exception.NotFound:
raise exc.HTTPNotFound()
bdms = self.compute_api.get_instance_bdms(context, instance)
bdms = block_device_obj.BlockDeviceMappingList.get_by_instance_uuid(
context, instance.uuid)
found = False
try:
for bdm in bdms:
if bdm['volume_id'] != old_volume_id:
if bdm.volume_id != old_volume_id:
continue
try:
self.compute_api.swap_volume(context, instance, old_volume,
@ -501,7 +504,8 @@ class VolumeAttachmentController(wsgi.Controller):
volume = self.volume_api.get(context, volume_id)
bdms = self.compute_api.get_instance_bdms(context, instance)
bdms = block_device_obj.BlockDeviceMappingList.get_by_instance_uuid(
context, instance['uuid'])
if not bdms:
LOG.debug(_("Instance %s is not attached."), server_id)
raise exc.HTTPNotFound()
@ -509,7 +513,7 @@ class VolumeAttachmentController(wsgi.Controller):
found = False
try:
for bdm in bdms:
if bdm['volume_id'] != volume_id:
if bdm.volume_id != volume_id:
continue
try:
self.compute_api.detach_volume(context, instance, volume)
@ -540,15 +544,16 @@ class VolumeAttachmentController(wsgi.Controller):
except exception.NotFound:
raise exc.HTTPNotFound()
bdms = self.compute_api.get_instance_bdms(context, instance)
bdms = block_device_obj.BlockDeviceMappingList.get_by_instance_uuid(
context, instance['uuid'])
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']))
if bdm.volume_id:
results.append(entity_maker(bdm.volume_id,
bdm.instance_uuid,
bdm.device_name))
return {'volumeAttachments': results}

17
nova/api/openstack/compute/plugins/v3/extended_volumes.py
View File

@ -23,6 +23,7 @@ from nova.api.openstack import wsgi
from nova.api import validation
from nova import compute
from nova import exception
from nova.objects import block_device as block_device_obj
from nova.openstack.common.gettextutils import _
from nova.openstack.common import log as logging
from nova import volume
@ -45,7 +46,8 @@ class ExtendedVolumesController(wsgi.Controller):
self.volume_api = volume.API()
def _extend_server(self, context, server, instance):
bdms = self.compute_api.get_instance_bdms(context, instance)
bdms = block_device_obj.BlockDeviceMappingList.get_by_instance_uuid(
context, instance['uuid'])
volume_ids = [bdm['volume_id'] for bdm in bdms if bdm['volume_id']]
key = "%s:volumes_attached" % ExtendedVolumes.alias
server[key] = [{'id': volume_id} for volume_id in volume_ids]
@ -68,11 +70,12 @@ class ExtendedVolumesController(wsgi.Controller):
instance = common.get_instance(self.compute_api, context, id,
want_objects=True)
bdms = self.compute_api.get_instance_bdms(context, instance)
bdms = block_device_obj.BlockDeviceMappingList.get_by_instance_uuid(
context, instance.uuid)
found = False
try:
for bdm in bdms:
if bdm['volume_id'] != old_volume_id:
if bdm.volume_id != old_volume_id:
continue
try:
self.compute_api.swap_volume(context, instance, old_volume,
@ -174,13 +177,15 @@ class ExtendedVolumesController(wsgi.Controller):
{"volume_id": volume_id,
"server_id": id,
"context": context})
instance = common.get_instance(self.compute_api, context, server_id)
instance = common.get_instance(self.compute_api, context, server_id,
want_objects=True)
try:
volume = self.volume_api.get(context, volume_id)
except exception.VolumeNotFound as e:
raise exc.HTTPNotFound(explanation=e.format_message())
bdms = self.compute_api.get_instance_bdms(context, instance)
bdms = block_device_obj.BlockDeviceMappingList.get_by_instance_uuid(
context, instance.uuid)
if not bdms:
msg = _("Volume %(volume_id)s is not attached to the "
"instance %(server_id)s") % {'server_id': server_id,
@ -189,7 +194,7 @@ class ExtendedVolumesController(wsgi.Controller):
raise exc.HTTPNotFound(explanation=msg)
for bdm in bdms:
if bdm['volume_id'] != volume_id:
if bdm.volume_id != volume_id:
continue
try:
self.compute_api.detach_volume(context, instance, volume)

8
nova/compute/api.py
View File

@ -2939,14 +2939,6 @@ class API(base.Base):
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, legacy=True):
"""Get all bdm tables for specified instance."""
bdms = self.db.block_device_mapping_get_all_by_instance(context,
instance['uuid'])
if legacy:
return block_device.legacy_mapping(bdms)
return bdms
def is_volume_backed_instance(self, context, instance, bdms=None):
if not instance['image_ref']:
return True

14
nova/tests/api/openstack/compute/contrib/test_extended_volumes.py
View File

@ -23,6 +23,7 @@ from nova.objects import instance as instance_obj
from nova.openstack.common import jsonutils
from nova import test
from nova.tests.api.openstack import fakes
from nova.tests import fake_block_device
from nova.tests import fake_instance
UUID1 = '00000000-0000-0000-0000-000000000001'
@ -43,8 +44,13 @@ def fake_compute_get_all(*args, **kwargs):
db_list, fields)
def fake_compute_get_instance_bdms(*args, **kwargs):
return [{'volume_id': UUID1}, {'volume_id': UUID2}]
def fake_bdms_get_all_by_instance(*args, **kwargs):
return [fake_block_device.FakeDbBlockDeviceDict(
{'volume_id': UUID1, 'source_type': 'volume',
'destination_type': 'volume', 'id': 1}),
fake_block_device.FakeDbBlockDeviceDict(
{'volume_id': UUID2, 'source_type': 'volume',
'destination_type': 'volume', 'id': 2})]
class ExtendedVolumesTest(test.TestCase):
@ -56,8 +62,8 @@ class ExtendedVolumesTest(test.TestCase):
fakes.stub_out_nw_api(self.stubs)
self.stubs.Set(compute.api.API, 'get', fake_compute_get)
self.stubs.Set(compute.api.API, 'get_all', fake_compute_get_all)
self.stubs.Set(compute.api.API, 'get_instance_bdms',
fake_compute_get_instance_bdms)
self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
fake_bdms_get_all_by_instance)
self.flags(
osapi_compute_extension=[
'nova.api.openstack.compute.contrib.select_extensions'],

30
nova/tests/api/openstack/compute/contrib/test_volumes.py
View File

@ -28,11 +28,14 @@ from nova.api.openstack import extensions
from nova.compute import api as compute_api
from nova.compute import flavors
from nova import context
from nova import db
from nova import exception
from nova.openstack.common import jsonutils
from nova.openstack.common import timeutils
from nova import test
from nova.tests.api.openstack import fakes
from nova.tests import fake_block_device
from nova.tests import fake_instance
from nova.volume import cinder
CONF = cfg.CONF
@ -48,7 +51,7 @@ IMAGE_UUID = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77'
def fake_get_instance(self, context, instance_id, want_objects=False):
return {'uuid': instance_id}
return fake_instance.fake_instance_obj(context, **{'uuid': instance_id})
def fake_get_volume(self, context, id):
@ -92,23 +95,27 @@ def fake_compute_volume_snapshot_create(self, context, volume_id,
pass
def fake_get_instance_bdms(self, context, instance):
return [{'id': 1,
'instance_uuid': instance['uuid'],
def fake_bdms_get_all_by_instance(context, instance_uuid):
return [fake_block_device.FakeDbBlockDeviceDict(
{'id': 1,
'instance_uuid': instance_uuid,
'device_name': '/dev/fake0',
'delete_on_termination': 'False',
'virtual_name': 'MyNamesVirtual',
'source_type': 'volume',
'destination_type': 'volume',
'snapshot_id': None,
'volume_id': FAKE_UUID_A,
'volume_size': 1},
'volume_size': 1}),
fake_block_device.FakeDbBlockDeviceDict(
{'id': 2,
'instance_uuid': instance['uuid'],
'instance_uuid': instance_uuid,
'device_name': '/dev/fake1',
'delete_on_termination': 'False',
'virtual_name': 'MyNamesVirtual',
'source_type': 'volume',
'destination_type': 'volume',
'snapshot_id': None,
'volume_id': FAKE_UUID_B,
'volume_size': 1}]
'volume_size': 1})]
def fake_volume_actions_to_locked_server(self, context, instance, volume):
@ -318,9 +325,8 @@ class VolumeApiTest(test.TestCase):
class VolumeAttachTests(test.TestCase):
def setUp(self):
super(VolumeAttachTests, self).setUp()
self.stubs.Set(compute_api.API,
'get_instance_bdms',
fake_get_instance_bdms)
self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
fake_bdms_get_all_by_instance)
self.stubs.Set(compute_api.API, 'get', fake_get_instance)
self.stubs.Set(cinder.API, 'get', fake_get_volume)
self.context = context.get_admin_context()

14
nova/tests/api/openstack/compute/plugins/v3/test_extended_volumes.py
View File

@ -24,6 +24,7 @@ from nova.objects import instance as instance_obj
from nova.openstack.common import jsonutils
from nova import test
from nova.tests.api.openstack import fakes
from nova.tests import fake_block_device
from nova.tests import fake_instance
from nova import volume
@ -49,8 +50,13 @@ def fake_compute_get_all(*args, **kwargs):
db_list, fields)
def fake_compute_get_instance_bdms(*args, **kwargs):
return [{'volume_id': UUID1}, {'volume_id': UUID2}]
def fake_bdms_get_all_by_instance(*args, **kwargs):
return [fake_block_device.FakeDbBlockDeviceDict(
{'volume_id': UUID1, 'source_type': 'volume',
'destination_type': 'volume', 'id': 1}),
fake_block_device.FakeDbBlockDeviceDict(
{'volume_id': UUID2, 'source_type': 'volume',
'destination_type': 'volume', 'id': 2})]
def fake_attach_volume(self, context, instance, volume_id,
@ -129,8 +135,8 @@ class ExtendedVolumesTest(test.TestCase):
fakes.stub_out_nw_api(self.stubs)
self.stubs.Set(compute.api.API, 'get', fake_compute_get)
self.stubs.Set(compute.api.API, 'get_all', fake_compute_get_all)
self.stubs.Set(compute.api.API, 'get_instance_bdms',
fake_compute_get_instance_bdms)
self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
fake_bdms_get_all_by_instance)
self.stubs.Set(volume.cinder.API, 'get', fake_volume_get)
self.stubs.Set(compute.api.API, 'detach_volume', fake_detach_volume)
self.stubs.Set(compute.api.API, 'attach_volume', fake_attach_volume)

9
nova/tests/api/openstack/fakes.py
View File

@ -42,6 +42,7 @@ from nova.network import api as network_api
from nova.openstack.common import jsonutils
from nova.openstack.common import timeutils
from nova import quota
from nova.tests import fake_block_device
from nova.tests import fake_network
from nova.tests.glance import stubs as glance_stubs
from nova.tests.objects import test_keypair
@ -709,8 +710,12 @@ def stub_snapshot_get_all(self, context):
def stub_bdm_get_all_by_instance(context, instance_uuid):
return [{'source_type': 'volume', 'volume_id': 'volume_id1'},
{'source_type': 'volume', 'volume_id': 'volume_id2'}]
return [fake_block_device.FakeDbBlockDeviceDict(
{'id': 1, 'source_type': 'volume', 'destination_type': 'volume',
'volume_id': 'volume_id1', 'instance_uuid': instance_uuid}),
fake_block_device.FakeDbBlockDeviceDict(
{'id': 2, 'source_type': 'volume', 'destination_type': 'volume',
'volume_id': 'volume_id2', 'instance_uuid': instance_uuid})]
def fake_get_available_languages(domain):

33
nova/tests/compute/test_compute.py
View File

@ -9095,39 +9095,6 @@ class ComputeAPITestCase(BaseTestCase):
self.assertEqual(1, len(migrations))
self.assertEqual(migrations[0].id, migration['id'])
def _setup_get_instance_bdm_mox(self):
new_bdm = object()
self.mox.StubOutWithMock(self.compute_api.db,
'block_device_mapping_get_all_by_instance')
self.compute_api.db.\
block_device_mapping_get_all_by_instance(
mox.IgnoreArg(), mox.IgnoreArg()).AndReturn(new_bdm)
return new_bdm
def test_get_instance_bdms_legacy(self):
expected = self._setup_get_instance_bdm_mox()
self.mox.ReplayAll()
instance = {'uuid': 'fake-instance'}
self.assertEqual(expected,
self.compute_api.get_instance_bdms({},
instance, legacy=False))
def test_get_instance_bdms_default(self):
new_bdm = self._setup_get_instance_bdm_mox()
expected = legacy_bdm = object()
self.mox.StubOutWithMock(block_device, 'legacy_mapping')
block_device.legacy_mapping(new_bdm).AndReturn(legacy_bdm)
self.mox.ReplayAll()
instance = {'uuid': 'fake-instance'}
self.assertEqual(expected,
self.compute_api.get_instance_bdms({}, instance))
def fake_rpc_method(context, method, **kwargs):
pass

38
nova/tests/integrated/test_api_samples.py
View File

@ -55,6 +55,8 @@ from nova.tests.api.openstack.compute.contrib import test_fping
from nova.tests.api.openstack.compute.contrib import test_networks
from nova.tests.api.openstack.compute.contrib import test_services
from nova.tests.api.openstack import fakes
from nova.tests import fake_block_device
from nova.tests import fake_instance
from nova.tests import fake_instance_actions
from nova.tests import fake_network
from nova.tests import fake_network_cache_model
@ -3793,27 +3795,33 @@ class AssistedVolumeSnapshotsXmlTest(AssistedVolumeSnapshotsJsonTest):
class VolumeAttachmentsSampleBase(ServersSampleBase):
def _stub_compute_api_get_instance_bdms(self, server_id):
def _stub_db_bdms_get_all_by_instance(self, server_id):
def fake_compute_api_get_instance_bdms(self, context, instance):
def fake_bdms_get_all_by_instance(context, instance_uui):
bdms = [
{'volume_id': 'a26887c6-c47b-4654-abb5-dfadf7d3f803',
'instance_uuid': server_id,
'device_name': '/dev/sdd'},
{'volume_id': 'a26887c6-c47b-4654-abb5-dfadf7d3f804',
'instance_uuid': server_id,
'device_name': '/dev/sdc'}
fake_block_device.FakeDbBlockDeviceDict(
{'id': 1, 'volume_id': 'a26887c6-c47b-4654-abb5-dfadf7d3f803',
'instance_uuid': server_id, 'source_type': 'volume',
'destination_type': 'volume', 'device_name': '/dev/sdd'}),
fake_block_device.FakeDbBlockDeviceDict(
{'id': 2, 'volume_id': 'a26887c6-c47b-4654-abb5-dfadf7d3f804',
'instance_uuid': server_id, 'source_type': 'volume',
'destination_type': 'volume', 'device_name': '/dev/sdc'})
]
return bdms
self.stubs.Set(compute_api.API, "get_instance_bdms",
fake_compute_api_get_instance_bdms)
self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
fake_bdms_get_all_by_instance)
def _stub_compute_api_get(self):
def fake_compute_api_get(self, context, instance_id,
want_objects=False):
return {'uuid': instance_id}
if want_objects:
return fake_instance.fake_instance_obj(
context, **{'uuid': instance_id})
else:
return {'uuid': instance_id}
self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
@ -3853,7 +3861,7 @@ class VolumeAttachmentsSampleJsonTest(VolumeAttachmentsSampleBase):
def test_list_volume_attachments(self):
server_id = self._post_server()
self._stub_compute_api_get_instance_bdms(server_id)
self._stub_db_bdms_get_all_by_instance(server_id)
response = self._do_get('servers/%s/os-volume_attachments'
% server_id)
@ -3864,7 +3872,7 @@ class VolumeAttachmentsSampleJsonTest(VolumeAttachmentsSampleBase):
def test_volume_attachment_detail(self):
server_id = self._post_server()
attach_id = "a26887c6-c47b-4654-abb5-dfadf7d3f803"
self._stub_compute_api_get_instance_bdms(server_id)
self._stub_db_bdms_get_all_by_instance(server_id)
self._stub_compute_api_get()
response = self._do_get('servers/%s/os-volume_attachments/%s'
% (server_id, attach_id))
@ -3875,7 +3883,7 @@ class VolumeAttachmentsSampleJsonTest(VolumeAttachmentsSampleBase):
def test_volume_attachment_delete(self):
server_id = self._post_server()
attach_id = "a26887c6-c47b-4654-abb5-dfadf7d3f803"
self._stub_compute_api_get_instance_bdms(server_id)
self._stub_db_bdms_get_all_by_instance(server_id)
self._stub_compute_api_get()
self.stubs.Set(cinder.API, 'get', fakes.stub_volume_get)
self.stubs.Set(compute_api.API, 'detach_volume', lambda *a, **k: None)
@ -3902,7 +3910,7 @@ class VolumeAttachUpdateSampleJsonTest(VolumeAttachmentsSampleBase):
}
server_id = self._post_server()
attach_id = 'a26887c6-c47b-4654-abb5-dfadf7d3f803'
self._stub_compute_api_get_instance_bdms(server_id)
self._stub_db_bdms_get_all_by_instance(server_id)
self._stub_compute_api_get()
self.stubs.Set(cinder.API, 'get', fakes.stub_volume_get)
self.stubs.Set(compute_api.API, 'swap_volume', lambda *a, **k: None)

30
nova/tests/integrated/v3/test_extended_volumes.py
View File

@ -19,6 +19,8 @@ from nova import context
from nova import db
from nova.objects import block_device as block_device_obj
from nova.tests.api.openstack import fakes
from nova.tests import fake_block_device
from nova.tests import fake_instance
from nova.tests.integrated.v3 import test_servers
from nova.volume import cinder
@ -28,24 +30,30 @@ class ExtendedVolumesSampleJsonTests(test_servers.ServersSampleBase):
def _stub_compute_api_get_instance_bdms(self, server_id):
def fake_compute_api_get_instance_bdms(self, context, instance):
def fake_bdms_get_all_by_instance(context, instance_uui):
bdms = [
{'volume_id': 'a26887c6-c47b-4654-abb5-dfadf7d3f803',
'instance_uuid': server_id,
'device_name': '/dev/sdd'},
{'volume_id': 'a26887c6-c47b-4654-abb5-dfadf7d3f804',
'instance_uuid': server_id,
'device_name': '/dev/sdc'}
fake_block_device.FakeDbBlockDeviceDict(
{'id': 1, 'volume_id': 'a26887c6-c47b-4654-abb5-dfadf7d3f803',
'instance_uuid': server_id, 'source_type': 'volume',
'destination_type': 'volume', 'device_name': '/dev/sdd'}),
fake_block_device.FakeDbBlockDeviceDict(
{'id': 2, 'volume_id': 'a26887c6-c47b-4654-abb5-dfadf7d3f804',
'instance_uuid': server_id, 'source_type': 'volume',
'destination_type': 'volume', 'device_name': '/dev/sdc'})
]
return bdms
self.stubs.Set(compute_api.API, "get_instance_bdms",
fake_compute_api_get_instance_bdms)
self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
fake_bdms_get_all_by_instance)
def _stub_compute_api_get(self):
def fake_compute_api_get(self, context, instance_id, **kwargs):
return {'uuid': instance_id}
want_objects = kwargs.get('want_objects')
if want_objects:
return fake_instance.fake_instance_obj(
context, **{'uuid': instance_id})
else:
return {'uuid': instance_id}
self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)