openstack/nova
Code Issues Proposed changes

Keep attach_mode as top-level field in _translate_attachment_ref

Browse Source 
The attach_mode is a top-level field on the attachment record in Cinder.
There is an access_mode entry in the connection_info field which is used
by the volume driver in compute when making an attachment, but the
attach_mode field is read by the API during swap volume so we don't have
to dig into the connection_info details.

This changes _translate_attachment_ref to leave attach_mode on the
top-level attachment reference dict so the API does not need to look
into the connection_info data.

Change-Id: I1207687d1b00416482ac34ad3250e7121d9b4352
This commit is contained in:
Matt Riedemann 2018-06-11 15:43:08 -04:00 committed by Stephen Finucane
parent 5a1d159d14
commit c489ccab2d
5 changed files with 7 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
nova/compute/api.py
View File

@ -4411,10 +4411,7 @@ class API(base.Base):
# nova.volume.cinder code translates it and puts the
# attach_mode in the connection_info for some legacy
# reason...
if attachment_record.get(
'connection_info', {}).get(
# attachments are read/write by default
'attach_mode', 'rw') == 'rw':
if attachment_record['attach_mode'] == 'rw':
count += 1
except exception.VolumeAttachmentNotFound:
# attachments are read/write by default so count it

2
nova/tests/unit/api/openstack/compute/test_volumes.py
View File

@ -1010,7 +1010,7 @@ class SwapVolumeMultiattachTestCase(test.NoDBTestCase):
raise exception.VolumeNotFound(volume_id=volume_id)
def fake_attachment_get(_context, attachment_id):
return {'connection_info': {'attach_mode': 'rw'}}
return {'attach_mode': 'rw'}
ctxt = context.get_admin_context()
instance = fake_instance.fake_instance_obj(

2
nova/tests/unit/compute/test_compute_api.py
View File

@ -2879,7 +2879,7 @@ class _ComputeAPIUnitTestMixIn(object):
if attachment_id == uuids.attachment1:
raise exception.VolumeAttachmentNotFound(
attachment_id=attachment_id)
return {'connection_info': {'attach_mode': 'ro'}}
return {'attach_mode': 'ro'}
with mock.patch.object(self.compute_api.volume_api, 'attachment_get',
side_effect=fake_attachment_get) as mock_get:

4
nova/tests/unit/volume/test_cinder.py
View File

@ -408,8 +408,8 @@ class CinderApiTestCase(test.NoDBTestCase):
expected_attachment_ref = {
'id': uuids.attachment_id,
'volume_id': fake_attachment.volume_id,
'attach_mode': 'rw',
'connection_info': {
'attach_mode': 'rw',
'attached_at': fake_attachment.attached_at,
'data': {'foo': 'bar', 'target_lun': '1'},
'detached_at': None,
@ -438,8 +438,8 @@ class CinderApiTestCase(test.NoDBTestCase):
expected_attachment_ref = {
'id': uuids.attachment_id,
'volume_id': fake_attachment.volume_id,
'attach_mode': 'rw',
'connection_info': {
'attach_mode': 'rw',
'attached_at': fake_attachment.attached_at,
'data': {'foo': 'bar', 'target_lun': '1'},
'detached_at': None,

5
nova/volume/cinder.py
View File

@ -373,8 +373,6 @@ def _translate_attachment_ref(attachment_ref):
translated_con_info['data'] = connection_info_data
translated_con_info['status'] = attachment_ref.pop('status', None)
translated_con_info['instance'] = attachment_ref.pop('instance', None)
translated_con_info['attach_mode'] = attachment_ref.pop('attach_mode',
None)
translated_con_info['attached_at'] = attachment_ref.pop('attached_at',
None)
translated_con_info['detached_at'] = attachment_ref.pop('detached_at',
@ -382,7 +380,8 @@ def _translate_attachment_ref(attachment_ref):
# Now the catch all...
for k, v in attachment_ref.items():
if k != "id":
# Keep these as top-level fields on the attachment record.
if k not in ("id", "attach_mode"):
translated_con_info[k] = v
attachment_ref['connection_info'] = translated_con_info