api: Introduce microversion 2.89 adjusting os-volume_attachments

This microversion adds attachment_id and bdm_uuid as stored in the
underlying bdm record while also removing the duplicate id field from
the responses of ``GET /servers/{server_id}/os-volume_attachments`` and
``GET /servers/{server_id}/os-volume_attachments/{volume_id}``.

To accomidate this within the _translate_attachment_summary_view helper
is folded into _translate_attachment_detail_view with the remaining
caller in the now deprecated os-volumes API now replaced with a static
dictionary.

Blueprint: add-attachmentid-to-responses-of-the-os-volume-attachments-api
Change-Id: I977c3fd9bbb1e1d42e6979222137e7366d2815e8

api-ref/source/os-volume-attachments.inc

@@ -34,21 +34,23 @@ Response
 .. rest_parameters:: parameters.yaml
 
   - volumeAttachments: volumeAttachments
-  - id: attachment_id_required
+  - id: volume_attachment_id_resp
   - serverId: server_id
   - volumeId: volumeId_resp
   - device: attachment_device_resp
   - tag: device_tag_bdm_attachment_resp
   - delete_on_termination: delete_on_termination_attachments_resp
+  - attachment_id: attachment_volume_id_resp
+  - bdm_uuid: attachment_bdm_id_resp
 
 **Example List volume attachments for an instance: JSON response**
 
 .. literalinclude:: ../../doc/api_samples/os-volumes/list-volume-attachments-resp.json
    :language: javascript
 
-**Example List tagged volume attachments for an instance (v2.79): JSON response**
+**Example List tagged volume attachments for an instance (v2.89): JSON response**
 
-.. literalinclude:: ../../doc/api_samples/os-volumes/v2.79/list-volume-attachments-resp.json
+.. literalinclude:: ../../doc/api_samples/os-volumes/v2.89/list-volume-attachments-resp.json
    :language: javascript
 
 Attach a volume to an instance
@@ -108,7 +110,7 @@ Response
 
   - volumeAttachment: volumeAttachment
   - device: device_resp
-  - id: attachment_id_required
+  - id: attachment_id_resp
   - serverId: server_id
   - volumeId: volumeId_resp
   - tag: device_tag_bdm_attachment_resp
@@ -154,21 +156,23 @@ Response
 .. rest_parameters:: parameters.yaml
 
   - volumeAttachment: volumeAttachment
-  - id: attachment_id_required
+  - id: volume_attachment_id_resp
   - serverId: server_id
   - volumeId: volumeId_resp
   - device: attachment_device_resp
   - tag: device_tag_bdm_attachment_resp
   - delete_on_termination: delete_on_termination_attachments_resp
+  - attachment_id: attachment_volume_id_resp
+  - bdm_uuid: attachment_bdm_id_resp
 
 **Example Show a detail of a volume attachment: JSON response**
 
 .. literalinclude:: ../../doc/api_samples/os-volumes/volume-attachment-detail-resp.json
    :language: javascript
 
-**Example Show a detail of a tagged volume attachment (v2.79): JSON response**
+**Example Show a detail of a tagged volume attachment (v2.89): JSON response**
 
-.. literalinclude:: ../../doc/api_samples/os-volumes/v2.79/volume-attachment-detail-resp.json
+.. literalinclude:: ../../doc/api_samples/os-volumes/v2.89/volume-attachment-detail-resp.json
    :language: javascript
 
 Update a volume attachment

api-ref/source/parameters.yaml

@@ -1776,6 +1776,13 @@ associate_host:
   in: body
   required: true
   type: string
+attachment_bdm_id_resp:
+  description: |
+    The UUID of the block device mapping record in Nova for the attachment.
+  in: body
+  required: false
+  type: string
+  min_version: 2.89
 attachment_device_put_req:
   description: |
     Name of the device in the attachment object, such as, ``/dev/vdb``.
@@ -1796,12 +1803,6 @@ attachment_id_put_req:
   required: false
   type: string
   min_version: 2.85
-attachment_id_required:
-  description: |
-    The UUID of the attachment.
-  in: body
-  required: true
-  type: string
 attachment_id_resp:
   description: |
     The UUID of the attachment.
@@ -1821,6 +1822,13 @@ attachment_server_id_resp:
   in: body
   required: false
   type: string
+attachment_volume_id_resp:
+  description: |
+    The UUID of the associated volume attachment in Cinder.
+  in: body
+  required: false
+  type: string
+  min_version: 2.89
 attachment_volumeId_resp:
   description: |
     The UUID of the attached volume.
@@ -7368,6 +7376,13 @@ volume:
   in: body
   required: true
   type: object
+volume_attachment_id_resp:
+  description: |
+    The volumeId of the attachment.
+  in: body
+  required: false
+  type: string
+  max_version: 2.88
 volume_id:
   description: |
     The source volume ID.

doc/api_samples/os-volumes/v2.89/attach-volume-to-server-req.json

@@ -0,0 +1,7 @@
+{
+    "volumeAttachment": {
+        "volumeId": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113",
+        "tag": "foo",
+        "delete_on_termination": true
+    }
+}

doc/api_samples/os-volumes/v2.89/attach-volume-to-server-resp.json

@@ -0,0 +1,10 @@
+{
+    "volumeAttachment": {
+        "delete_on_termination": true,
+        "device": "/dev/sdb",
+        "id": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113",
+        "serverId": "7ebed2ce-85b3-40b5-84ae-8cc725c37ed2",
+        "tag": "foo",
+        "volumeId": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113"
+    }
+}

doc/api_samples/os-volumes/v2.89/list-volume-attachments-resp.json

@@ -0,0 +1,22 @@
+{
+    "volumeAttachments": [
+        {
+            "attachment_id": "979ce4f8-033a-409d-85e6-6b5c0f6a6302",
+            "delete_on_termination": false,
+            "device": "/dev/sdc",
+            "serverId": "7696780b-3f53-4688-ab25-019bfcbbd806",
+            "tag": null,
+            "volumeId": "227cc671-f30b-4488-96fd-7d0bf13648d8",
+            "bdm_uuid": "c088db45-92b8-49e8-81e2-a1b77a144b3b"
+        },
+        {
+            "attachment_id": "c5684109-0311-4fca-9814-350e46ab7d2a",
+            "delete_on_termination": true,
+            "device": "/dev/sdb",
+            "serverId": "7696780b-3f53-4688-ab25-019bfcbbd806",
+            "tag": "foo",
+            "volumeId": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113",
+            "bdm_uuid": "1aa24536-6fb5-426c-8894-d627f39aa48b"
+        }
+    ]
+}

doc/api_samples/os-volumes/v2.89/update-volume-attachment-delete-flag-req.json

@@ -0,0 +1,10 @@
+{
+    "volumeAttachment": {
+        "volumeId": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113",
+        "id": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113",
+        "serverId": "fddf0901-8caf-42c9-b496-133c570b171b",
+        "device": "/dev/sdb",
+        "tag": "foo",
+        "delete_on_termination": true
+    }
+}

doc/api_samples/os-volumes/v2.89/volume-attachment-detail-resp.json

@@ -0,0 +1,11 @@
+{
+    "volumeAttachment": {
+        "attachment_id": "721a5c82-5ebc-4c6a-8339-3d33d8d027ed",
+        "delete_on_termination": true,
+        "device": "/dev/sdb",
+        "serverId": "7ebed2ce-85b3-40b5-84ae-8cc725c37ed2",
+        "tag": "foo",
+        "volumeId": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113",
+        "bdm_uuid": "c088db45-92b8-49e8-81e2-a1b77a144b3b"
+    }
+}

doc/api_samples/versions/v21-version-get-resp.json

@@ -19,7 +19,7 @@
             }
         ],
         "status": "CURRENT",
-        "version": "2.88",
+        "version": "2.89",
         "min_version": "2.1",
         "updated": "2013-07-23T11:33:21Z"
     }

doc/api_samples/versions/versions-get-resp.json

@@ -22,7 +22,7 @@
                 }
             ],
             "status": "CURRENT",
-            "version": "2.88",
+            "version": "2.89",
             "min_version": "2.1",
             "updated": "2013-07-23T11:33:21Z"
         }

nova/api/openstack/api_version_request.py

@@ -240,6 +240,10 @@ REST_API_VERSION_HISTORY = """REST API Version History:
              and ``/os-hypervisors/{hypervisor_id}`` APIs, and remove the
              ``/os-hypervisors/statistics`` and
              ``/os-hypervisors/{hypervisor_id}/uptime`` APIs entirely.
+    * 2.89 - Add ``attachment_id``, ``bdm_uuid`` and remove ``id`` from the
+             responses of ``GET  /servers/{server_id}/os-volume_attachments``
+             and ``GET /servers/{server_id}/os-volume_attachments/{volume_id}``
+
 """
 
 # The minimum and maximum versions of the API supported
@@ -248,7 +252,7 @@ REST_API_VERSION_HISTORY = """REST API Version History:
 # Note(cyeoh): This only applies for the v2.1 API once microversions
 # support is fully merged. It does not affect the V2 API.
 _MIN_API_VERSION = '2.1'
-_MAX_API_VERSION = '2.88'
+_MAX_API_VERSION = '2.89'
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 # Almost all proxy APIs which are related to network, images and baremetal

nova/api/openstack/compute/rest_api_version_history.rst

@@ -1177,3 +1177,14 @@ similar response to the ``GET /os-hypervisors/detail`` and ``GET
 /os-hypervisors/{hypervisor_id}`` APIs but with an additional ``uptime`` field,
 has been removed in favour of including this field in the primary ``GET
 /os-hypervisors/detail`` and ``GET /os-hypervisors/{hypervisor_id}`` APIs.
+
+.. _microversion 2.89:
+
+2.89 (Maximum in Xena)
+----------------------
+
+``attachment_id`` and ``bdm_uuid`` are now included in the responses for ``GET
+/servers/{server_id}/os-volume_attachments`` and ``GET
+/servers/{server_id}/os-volume_attachments/{volume_id}``. Additionally the
+``id`` field is dropped from the response as it duplicates the ``volumeId``
+field.

nova/api/openstack/compute/volumes.py

@@ -68,9 +68,18 @@ def _translate_volume_summary_view(context, vol):
         #                    }
         #                }
         attachment = list(vol['attachments'].items())[0]
-        d['attachments'] = [_translate_attachment_summary_view(vol['id'],
+        d['attachments'] = [
-            attachment[0],
+            {
-            attachment[1].get('mountpoint'))]
+                'id': vol['id'],
+                'volumeId': vol['id'],
+                'serverId': attachment[0],
+            }
+        ]
+
+        mountpoint = attachment[1].get('mountpoint')
+        if mountpoint:
+            d['attachments'][0]['device'] = mountpoint
+
     else:
         d['attachments'] = [{}]
 
@@ -215,8 +224,12 @@ class VolumeController(wsgi.Controller):
         return wsgi.ResponseObject(result, headers=dict(location=location))
 
 
-def _translate_attachment_detail_view(bdm, show_tag=False,
+def _translate_attachment_detail_view(
-                                      show_delete_on_termination=False):
+    bdm,
+    show_tag=False,
+    show_delete_on_termination=False,
+    show_attachment_id_bdm_uuid=False,
+):
     """Maps keys for attachment details view.
 
     :param bdm: BlockDeviceMapping object for an attached volume
@@ -225,10 +238,22 @@ def _translate_attachment_detail_view(bdm, show_tag=False,
     :param show_delete_on_termination: True if the "delete_on_termination"
         field should be in the response, False to exclude the
         "delete_on_termination" field from the response
+    :param show_attachment_id_bdm_uuid: True if the "attachment_id" and
+        "bdm_uuid" fields should be in the response. Also controls when the
+        "id" field is included.
     """
 
-    d = _translate_attachment_summary_view(
+    d = {}
-        bdm.volume_id, bdm.instance_uuid, bdm.device_name)
+
+    if not show_attachment_id_bdm_uuid:
+        d['id'] = bdm.volume_id
+
+    d['volumeId'] = bdm.volume_id
+
+    d['serverId'] = bdm.instance_uuid
+
+    if bdm.device_name:
+        d['device'] = bdm.device_name
 
     if show_tag:
         d['tag'] = bdm.tag
@@ -236,21 +261,9 @@ def _translate_attachment_detail_view(bdm, show_tag=False,
     if show_delete_on_termination:
         d['delete_on_termination'] = bdm.delete_on_termination
 
-    return d
+    if show_attachment_id_bdm_uuid:
-
+        d['attachment_id'] = bdm.attachment_id
-
+        d['bdm_uuid'] = bdm.uuid
-def _translate_attachment_summary_view(volume_id, instance_uuid, mountpoint):
-    """Maps keys for attachment summary view."""
-    d = {}
-
-    # NOTE(justinsb): We use the volume id as the id of the attachment object
-    d['id'] = volume_id
-
-    d['volumeId'] = volume_id
-
-    d['serverId'] = instance_uuid
-    if mountpoint:
-        d['device'] = mountpoint
 
     return d
 
@@ -299,11 +312,16 @@ class VolumeAttachmentController(wsgi.Controller):
         show_tag = api_version_request.is_supported(req, '2.70')
         show_delete_on_termination = api_version_request.is_supported(
             req, '2.79')
+        show_attachment_id_bdm_uuid = api_version_request.is_supported(
+            req, '2.89')
         for bdm in limited_list:
             if bdm.volume_id:
                 va = _translate_attachment_detail_view(
-                    bdm, show_tag=show_tag,
+                    bdm,
-                    show_delete_on_termination=show_delete_on_termination)
+                    show_tag=show_tag,
+                    show_delete_on_termination=show_delete_on_termination,
+                    show_attachment_id_bdm_uuid=show_attachment_id_bdm_uuid,
+                )
                 results.append(va)
 
         return {'volumeAttachments': results}
@@ -330,9 +348,16 @@ class VolumeAttachmentController(wsgi.Controller):
         show_tag = api_version_request.is_supported(req, '2.70')
         show_delete_on_termination = api_version_request.is_supported(
             req, '2.79')
-        return {'volumeAttachment': _translate_attachment_detail_view(
+        show_attachment_id_bdm_uuid = api_version_request.is_supported(
-            bdm, show_tag=show_tag,
+            req, '2.89')
-            show_delete_on_termination=show_delete_on_termination)}
+        return {
+            'volumeAttachment': _translate_attachment_detail_view(
+                bdm,
+                show_tag=show_tag,
+                show_delete_on_termination=show_delete_on_termination,
+                show_attachment_id_bdm_uuid=show_attachment_id_bdm_uuid,
+            )
+        }
 
     # TODO(mriedem): This API should return a 202 instead of a 200 response.
     @wsgi.expected_errors((400, 403, 404, 409))

nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.89/attach-volume-to-server-req.json.tpl

@@ -0,0 +1,7 @@
+{
+    "volumeAttachment": {
+        "volumeId": "%(volume_id)s",
+        "tag": "%(tag)s",
+        "delete_on_termination": true
+    }
+}

nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.89/attach-volume-to-server-resp.json.tpl

@@ -0,0 +1,10 @@
+{
+    "volumeAttachment": {
+        "device": "%(device)s",
+        "id": "%(volume_id)s",
+        "serverId": "%(uuid)s",
+        "tag": "%(tag)s",
+        "volumeId": "%(volume_id)s",
+        "delete_on_termination": true
+    }
+}

nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.89/list-volume-attachments-resp.json.tpl

@@ -0,0 +1,22 @@
+{
+    "volumeAttachments": [
+        {
+            "device": "%(device)s",
+            "serverId": "%(uuid)s",
+            "tag": "%(tag)s",
+            "volumeId": "%(volume_id)s",
+            "delete_on_termination": true,
+            "attachment_id": "%(uuid)s",
+            "bdm_uuid": "%(uuid)s"
+        },
+        {
+            "device": "%(text)s",
+            "serverId": "%(uuid)s",
+            "tag": null,
+            "volumeId": "%(volume_id2)s",
+            "delete_on_termination": false,
+            "attachment_id": "%(uuid)s",
+            "bdm_uuid": "%(uuid)s"
+        }
+    ]
+}

nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.89/update-volume-attachment-delete-flag-req.json.tpl

@@ -0,0 +1,10 @@
+{
+    "volumeAttachment": {
+        "volumeId": "%(volume_id)s",
+        "id": "%(volume_id)s",
+        "serverId": "%(server_id)s",
+        "device": "%(device)s",
+        "tag": "%(tag)s",
+        "delete_on_termination": true
+    }
+}

nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.89/update-volume-req.json.tpl

@@ -0,0 +1,5 @@
+{
+    "volumeAttachment": {
+        "volumeId": "%(new_volume_id)s"
+    }
+}

nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.89/volume-attachment-detail-resp.json.tpl

@@ -0,0 +1,11 @@
+{
+    "volumeAttachment": {
+        "device": "%(device)s",
+        "serverId": "%(uuid)s",
+        "tag": "%(tag)s",
+        "volumeId": "%(volume_id)s",
+        "delete_on_termination": true,
+        "attachment_id": "%(uuid)s",
+        "bdm_uuid": "%(uuid)s"
+    }
+}

nova/tests/functional/api_sample_tests/test_volumes.py

@@ -199,7 +199,7 @@ class VolumeAttachmentsSample(test_servers.ServersSampleBase):
 
     def setUp(self):
         super(VolumeAttachmentsSample, self).setUp()
-        self.useFixture(fixtures.CinderFixture(self))
+        self.cinder = self.useFixture(fixtures.CinderFixture(self))
         self.server_id = self._post_server()
 
     def _get_vol_attachment_subs(self, subs):
@@ -319,3 +319,11 @@ class UpdateVolumeAttachmentsSampleV285(VolumeAttachmentsSampleV279):
         self.assertEqual(1, len(attachments))
         self.assertEqual(self.server_id, attachments[0]['serverId'])
         self.assertTrue(attachments[0]['delete_on_termination'])
+
+
+class VolumeAttachmentsSampleV289(UpdateVolumeAttachmentsSampleV285):
+    """Microversion 2.89 adds the "attachment_id" parameter to the
+    response body of show and list.
+    """
+    microversion = '2.89'
+    scenarios = [('v2_89', {'api_major_version': 'v2.1'})]

nova/tests/unit/api/openstack/compute/test_volumes.py

@@ -101,16 +101,19 @@ def fake_compute_volume_snapshot_delete(self, context, volume_id, snapshot_id,
 def fake_bdm_get_by_volume_and_instance(cls, ctxt, volume_id, instance_uuid):
     if volume_id != FAKE_UUID_A:
         raise exception.VolumeBDMNotFound(volume_id=volume_id)
-    db_bdm = fake_block_device.FakeDbBlockDeviceDict(
+    db_bdm = fake_block_device.FakeDbBlockDeviceDict({
-        {'id': 1,
+        'id': 1,
-         'instance_uuid': instance_uuid,
+        'uuid': uuids.bdm,
-         'device_name': '/dev/fake0',
+        'instance_uuid': instance_uuid,
-         'delete_on_termination': 'False',
+        'device_name': '/dev/fake0',
-         'source_type': 'volume',
+        'delete_on_termination': 'False',
-         'destination_type': 'volume',
+        'source_type': 'volume',
-         'snapshot_id': None,
+        'destination_type': 'volume',
-         'volume_id': FAKE_UUID_A,
+        'snapshot_id': None,
-         'volume_size': 1})
+        'volume_id': FAKE_UUID_A,
+        'volume_size': 1,
+        'attachment_id': uuids.attachment_id
+    })
     return objects.BlockDeviceMapping._from_db_object(
         ctxt, objects.BlockDeviceMapping(), db_bdm)
 
@@ -1541,6 +1544,83 @@ class UpdateVolumeAttachTests(VolumeAttachTestsV279):
         self.assertIn('Additional properties are not allowed', str(ex))
 
 
+class VolumeAttachTestsV289(UpdateVolumeAttachTests):
+    microversion = '2.89'
+
+    def setUp(self):
+        super().setUp()
+        self.controller = volumes_v21.VolumeAttachmentController()
+        self.expected_show = {
+            'volumeAttachment': {
+                'device': '/dev/fake0',
+                'serverId': FAKE_UUID,
+                'volumeId': FAKE_UUID_A,
+                'tag': None,
+                'delete_on_termination': False,
+                'attachment_id': None,
+                'bdm_uuid': uuids.bdm,
+            }
+        }
+
+    def test_show_pre_v289(self):
+        req = self._get_req(body={}, microversion='2.88')
+        req.method = 'GET'
+        result = self.attachments.show(req, FAKE_UUID, FAKE_UUID_A)
+        self.assertIn('id', result['volumeAttachment'])
+        self.assertNotIn('bdm_uuid', result['volumeAttachment'])
+        self.assertNotIn('attachment_id', result['volumeAttachment'])
+
+    @mock.patch('nova.objects.BlockDeviceMappingList.get_by_instance_uuid')
+    def test_list(self, mock_get_bdms):
+        vol_bdm = objects.BlockDeviceMapping(
+            self.context,
+            id=1,
+            uuid=uuids.bdm,
+            instance_uuid=FAKE_UUID,
+            volume_id=FAKE_UUID_A,
+            source_type='volume',
+            destination_type='volume',
+            delete_on_termination=True,
+            connection_info=None,
+            tag='fake-tag',
+            device_name='/dev/fake0',
+            attachment_id=uuids.attachment_id)
+        bdms = objects.BlockDeviceMappingList(objects=[vol_bdm])
+        mock_get_bdms.return_value = bdms
+
+        req = fakes.HTTPRequest.blank(
+            '/v2/servers/id/os-volume_attachments',
+            version="2.88")
+        req.body = jsonutils.dump_as_bytes({})
+        req.method = 'GET'
+        req.headers['content-type'] = 'application/json'
+
+        result = self.attachments.index(req, FAKE_UUID)
+        self.assertIn('id', result['volumeAttachments'][0])
+        self.assertNotIn('attachment_id', result['volumeAttachments'][0])
+        self.assertNotIn('bdm_uuid', result['volumeAttachments'][0])
+
+        req = fakes.HTTPRequest.blank(
+            '/v2/servers/id/os-volume_attachments',
+            version="2.89")
+        req.body = jsonutils.dump_as_bytes({})
+        req.method = 'GET'
+        req.headers['content-type'] = 'application/json'
+
+        result = self.attachments.index(req, FAKE_UUID)
+        self.assertNotIn('id', result['volumeAttachments'][0])
+        self.assertIn('attachment_id', result['volumeAttachments'][0])
+        self.assertEqual(
+            uuids.attachment_id,
+            result['volumeAttachments'][0]['attachment_id']
+        )
+        self.assertIn('bdm_uuid', result['volumeAttachments'][0])
+        self.assertEqual(
+            uuids.bdm,
+            result['volumeAttachments'][0]['bdm_uuid']
+        )
+
+
 class SwapVolumeMultiattachTestCase(test.NoDBTestCase):
 
     @mock.patch('nova.api.openstack.common.get_instance')

releasenotes/notes/add-attachment-id-to-os-volume-attachments-a23818d5b11f15a1.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Microversion 2.89 has been introduced and will include the
+    ``attachment_id`` of a volume attachment, ``bdm_uuid`` of the block device
+    mapping record and removes the duplicate ``id`` from the responses for ``GET
+    /servers/{server_id}/os-volume_attachments`` and ``GET
+    /servers/{server_id}/os-volume_attachments/{volume_id}``.