From c489ccab2dc962c554b60455dc22729d89535802 Mon Sep 17 00:00:00 2001
From: Matt Riedemann <mriedem.os@gmail.com>
Date: Mon, 11 Jun 2018 15:43:08 -0400
Subject: [PATCH] Keep attach_mode as top-level field in
 _translate_attachment_ref

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
---
 nova/compute/api.py                                   | 5 +----
 nova/tests/unit/api/openstack/compute/test_volumes.py | 2 +-
 nova/tests/unit/compute/test_compute_api.py           | 2 +-
 nova/tests/unit/volume/test_cinder.py                 | 4 ++--
 nova/volume/cinder.py                                 | 5 ++---
 5 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 882918a23836..784bcbc8236b 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -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
diff --git a/nova/tests/unit/api/openstack/compute/test_volumes.py b/nova/tests/unit/api/openstack/compute/test_volumes.py
index adb5e500de6b..c71778e484bd 100644
--- a/nova/tests/unit/api/openstack/compute/test_volumes.py
+++ b/nova/tests/unit/api/openstack/compute/test_volumes.py
@@ -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(
diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py
index e0c04d644ca9..49db91a0d435 100644
--- a/nova/tests/unit/compute/test_compute_api.py
+++ b/nova/tests/unit/compute/test_compute_api.py
@@ -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:
diff --git a/nova/tests/unit/volume/test_cinder.py b/nova/tests/unit/volume/test_cinder.py
index 1451de952f0d..605e83c227f3 100644
--- a/nova/tests/unit/volume/test_cinder.py
+++ b/nova/tests/unit/volume/test_cinder.py
@@ -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,
diff --git a/nova/volume/cinder.py b/nova/volume/cinder.py
index db6eae873b6f..addf58ec7719 100644
--- a/nova/volume/cinder.py
+++ b/nova/volume/cinder.py
@@ -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