Adapt to nova api 2.89 and skip some cinder calls
the 2.89 api version stopped returning 'id' field for volume_attachments. Use 'volumeId' instead, and skip resolving the volume name as none of the code does use volume names, everything only ever needs volume id, so we can safely skip some API requests. Related-Bug: #2091658 Change-Id: I5031fa4686fad4de5eb9151ca86f51eb5a2c1d76
This commit is contained in:
@@ -185,15 +185,12 @@ class API(base.Base):
|
|||||||
|
|
||||||
@translate_server_exception
|
@translate_server_exception
|
||||||
def instance_volumes_list(self, context, instance_id):
|
def instance_volumes_list(self, context, instance_id):
|
||||||
from manila.volume import cinder
|
|
||||||
|
|
||||||
volumes = novaclient(context).volumes.get_server_volumes(instance_id)
|
volumes = novaclient(context).volumes.get_server_volumes(instance_id)
|
||||||
|
|
||||||
for volume in volumes:
|
# NOTE(pas-ha): Nova API 2.89 dropped 'id' field of the volume object,
|
||||||
volume_data = cinder.cinderclient(context).volumes.get(volume.id)
|
# so we use 'volumeId' field that is present in all API versions.
|
||||||
volume.name = volume_data.name
|
return [volume.volumeId for volume in volumes]
|
||||||
|
|
||||||
return volumes
|
|
||||||
|
|
||||||
@translate_server_exception
|
@translate_server_exception
|
||||||
def server_update(self, context, instance_id, name):
|
def server_update(self, context, instance_id, name):
|
||||||
|
|||||||
@@ -417,9 +417,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||||||
"generic_driver_attach_detach_%s" % instance_id, external=True)
|
"generic_driver_attach_detach_%s" % instance_id, external=True)
|
||||||
def do_attach(volume):
|
def do_attach(volume):
|
||||||
if volume['status'] == 'in-use':
|
if volume['status'] == 'in-use':
|
||||||
attached_volumes = [vol.id for vol in
|
attached_volumes = self.compute_api.instance_volumes_list(
|
||||||
self.compute_api.instance_volumes_list(
|
self.admin_context, instance_id)
|
||||||
self.admin_context, instance_id)]
|
|
||||||
if volume['id'] in attached_volumes:
|
if volume['id'] in attached_volumes:
|
||||||
return volume
|
return volume
|
||||||
else:
|
else:
|
||||||
@@ -525,9 +524,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||||||
@utils.synchronized(
|
@utils.synchronized(
|
||||||
"generic_driver_attach_detach_%s" % instance_id, external=True)
|
"generic_driver_attach_detach_%s" % instance_id, external=True)
|
||||||
def do_detach():
|
def do_detach():
|
||||||
attached_volumes = [vol.id for vol in
|
attached_volumes = self.compute_api.instance_volumes_list(
|
||||||
self.compute_api.instance_volumes_list(
|
self.admin_context, instance_id)
|
||||||
self.admin_context, instance_id)]
|
|
||||||
try:
|
try:
|
||||||
volume = self._get_volume(context, share['id'])
|
volume = self._get_volume(context, share['id'])
|
||||||
except exception.VolumeNotFound:
|
except exception.VolumeNotFound:
|
||||||
@@ -970,10 +968,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||||||
share_volume = get_volume()
|
share_volume = get_volume()
|
||||||
|
|
||||||
if share_volume:
|
if share_volume:
|
||||||
instance_volumes = self.compute_api.instance_volumes_list(
|
attached_volumes = self.compute_api.instance_volumes_list(
|
||||||
self.admin_context, server_details['instance_id'])
|
self.admin_context, server_details['instance_id'])
|
||||||
|
|
||||||
attached_volumes = [vol.id for vol in instance_volumes]
|
|
||||||
LOG.debug('Manage: attached volumes = %s', attached_volumes)
|
LOG.debug('Manage: attached volumes = %s', attached_volumes)
|
||||||
|
|
||||||
if share_volume['id'] not in attached_volumes:
|
if share_volume['id'] not in attached_volumes:
|
||||||
|
|||||||
@@ -24,13 +24,12 @@ from manila import context
|
|||||||
from manila import exception
|
from manila import exception
|
||||||
from manila import test
|
from manila import test
|
||||||
from manila.tests import utils as test_utils
|
from manila.tests import utils as test_utils
|
||||||
from manila.volume import cinder
|
|
||||||
|
|
||||||
|
|
||||||
class Volume(object):
|
class Volume(object):
|
||||||
def __init__(self, volume_id):
|
def __init__(self, volume_id):
|
||||||
self.id = volume_id
|
self.id = volume_id
|
||||||
self.name = volume_id
|
self.volumeId = volume_id
|
||||||
|
|
||||||
|
|
||||||
class Network(object):
|
class Network(object):
|
||||||
@@ -272,13 +271,8 @@ class NovaApiTestCase(test.TestCase):
|
|||||||
self.mock_object(
|
self.mock_object(
|
||||||
self.novaclient.volumes, 'get_server_volumes',
|
self.novaclient.volumes, 'get_server_volumes',
|
||||||
mock.Mock(return_value=[Volume('id1'), Volume('id2')]))
|
mock.Mock(return_value=[Volume('id1'), Volume('id2')]))
|
||||||
self.cinderclient = self.novaclient
|
|
||||||
self.mock_object(cinder, 'cinderclient',
|
|
||||||
mock.Mock(return_value=self.novaclient))
|
|
||||||
result = self.api.instance_volumes_list(self.ctx, 'instance_id')
|
result = self.api.instance_volumes_list(self.ctx, 'instance_id')
|
||||||
self.assertEqual(2, len(result))
|
self.assertEqual(['id1', 'id2'], result)
|
||||||
self.assertEqual('id1', result[0].id)
|
|
||||||
self.assertEqual('id2', result[1].id)
|
|
||||||
|
|
||||||
def test_server_update(self):
|
def test_server_update(self):
|
||||||
self.mock_object(self.novaclient.servers, 'update')
|
self.mock_object(self.novaclient.servers, 'update')
|
||||||
|
|||||||
@@ -581,7 +581,7 @@ class GenericShareDriverTestCase(test.TestCase):
|
|||||||
fake_server = fake_compute.FakeServer()
|
fake_server = fake_compute.FakeServer()
|
||||||
attached_volume = fake_volume.FakeVolume(status='in-use')
|
attached_volume = fake_volume.FakeVolume(status='in-use')
|
||||||
self.mock_object(self._driver.compute_api, 'instance_volumes_list',
|
self.mock_object(self._driver.compute_api, 'instance_volumes_list',
|
||||||
mock.Mock(return_value=[attached_volume]))
|
mock.Mock(return_value=[attached_volume.id]))
|
||||||
|
|
||||||
result = self._driver._attach_volume(self._context, self.share,
|
result = self._driver._attach_volume(self._context, self.share,
|
||||||
fake_server, attached_volume)
|
fake_server, attached_volume)
|
||||||
@@ -751,7 +751,7 @@ class GenericShareDriverTestCase(test.TestCase):
|
|||||||
self.mock_object(self._driver, '_get_volume',
|
self.mock_object(self._driver, '_get_volume',
|
||||||
mock.Mock(return_value=attached_volume))
|
mock.Mock(return_value=attached_volume))
|
||||||
self.mock_object(self._driver.compute_api, 'instance_volumes_list',
|
self.mock_object(self._driver.compute_api, 'instance_volumes_list',
|
||||||
mock.Mock(return_value=[attached_volume]))
|
mock.Mock(return_value=[attached_volume.id]))
|
||||||
self.mock_object(self._driver.compute_api, 'instance_volume_detach')
|
self.mock_object(self._driver.compute_api, 'instance_volume_detach')
|
||||||
self.mock_object(self._driver.volume_api, 'get',
|
self.mock_object(self._driver.volume_api, 'get',
|
||||||
mock.Mock(return_value=available_volume))
|
mock.Mock(return_value=available_volume))
|
||||||
@@ -1511,10 +1511,8 @@ class GenericShareDriverTestCase(test.TestCase):
|
|||||||
self.mock_object(self._driver.volume_api, 'get',
|
self.mock_object(self._driver.volume_api, 'get',
|
||||||
mock.Mock(return_value=volume))
|
mock.Mock(return_value=volume))
|
||||||
self._driver.volume_api.update = mock.Mock()
|
self._driver.volume_api.update = mock.Mock()
|
||||||
fake_volume = mock.Mock()
|
|
||||||
fake_volume.id = 'fake'
|
|
||||||
self.mock_object(self._driver.compute_api, 'instance_volumes_list',
|
self.mock_object(self._driver.compute_api, 'instance_volumes_list',
|
||||||
mock.Mock(return_value=[fake_volume]))
|
mock.Mock(return_value=['fake']))
|
||||||
self.mock_object(
|
self.mock_object(
|
||||||
self._driver._helpers[share['share_proto']],
|
self._driver._helpers[share['share_proto']],
|
||||||
'get_exports_for_share',
|
'get_exports_for_share',
|
||||||
|
|||||||
Reference in New Issue
Block a user