From e5b47543cf2693f8d79df304ac3dc56a8f54089b Mon Sep 17 00:00:00 2001 From: zhangbailin Date: Sat, 27 Jul 2019 18:16:36 +0800 Subject: [PATCH] Add delete_on_termination to volume-attach API Add the 'delete_on_termination' field to the volume attach API to support configuring whether to delete the data volume when the instance is destroyed. To avoid upgrade impact issues with older computes, the 'delete_on_termination' field is set in the API rather than when the BDM is created in the compute service. Implements: blueprint support-delete-on-termination-in-server-attach-volume Change-Id: I55731b1822a4e32909665a2872d80895cb5b12f7 --- api-ref/source/os-volume-attachments.inc | 14 +++ api-ref/source/parameters.yaml | 24 +++-- .../v2.79/attach-volume-to-server-req.json | 7 ++ .../v2.79/attach-volume-to-server-resp.json | 10 ++ .../v2.79/list-volume-attachments-resp.json | 20 ++++ .../os-volumes/v2.79/update-volume-req.json | 5 + .../v2.79/volume-attachment-detail-resp.json | 10 ++ .../versions/v21-version-get-resp.json | 2 +- .../versions/versions-get-resp.json | 2 +- nova/api/openstack/api_version_request.py | 8 +- .../compute/rest_api_version_history.rst | 15 +++ nova/api/openstack/compute/schemas/volumes.py | 4 + nova/api/openstack/compute/volumes.py | 31 +++++- nova/compute/api.py | 28 +++-- .../attach-volume-to-server-req.json.tpl | 7 ++ .../attach-volume-to-server-resp.json.tpl | 10 ++ .../list-volume-attachments-resp.json.tpl | 20 ++++ .../v2.79/update-volume-req.json.tpl | 5 + .../volume-attachment-detail-resp.json.tpl | 10 ++ .../api_sample_tests/test_volumes.py | 20 +++- .../api/openstack/compute/test_volumes.py | 102 +++++++++++++++++- nova/tests/unit/compute/test_compute.py | 4 +- nova/tests/unit/compute/test_compute_api.py | 5 +- ...server-attach-volume-5d08b4e97fdd24f9.yaml | 14 +++ 24 files changed, 348 insertions(+), 29 deletions(-) create mode 100644 doc/api_samples/os-volumes/v2.79/attach-volume-to-server-req.json create mode 100644 doc/api_samples/os-volumes/v2.79/attach-volume-to-server-resp.json create mode 100644 doc/api_samples/os-volumes/v2.79/list-volume-attachments-resp.json create mode 100644 doc/api_samples/os-volumes/v2.79/update-volume-req.json create mode 100644 doc/api_samples/os-volumes/v2.79/volume-attachment-detail-resp.json create mode 100644 nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/attach-volume-to-server-req.json.tpl create mode 100644 nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/attach-volume-to-server-resp.json.tpl create mode 100644 nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/list-volume-attachments-resp.json.tpl create mode 100644 nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/update-volume-req.json.tpl create mode 100644 nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/volume-attachment-detail-resp.json.tpl create mode 100644 releasenotes/notes/bp-support-delete-on-termination-in-server-attach-volume-5d08b4e97fdd24f9.yaml diff --git a/api-ref/source/os-volume-attachments.inc b/api-ref/source/os-volume-attachments.inc index 3304c88cc066..68db210c2bd4 100644 --- a/api-ref/source/os-volume-attachments.inc +++ b/api-ref/source/os-volume-attachments.inc @@ -39,6 +39,7 @@ Response - serverId: server_id - volumeId: volumeId_resp - tag: device_tag_bdm_attachment_resp + - delete_on_termination: delete_on_termination_attachments_resp **Example List volume attachments for an instance: JSON response** @@ -79,6 +80,7 @@ Request - volumeId: volumeId - device: device - tag: device_tag_bdm_attachment + - delete_on_termination: delete_on_termination_attachments_req **Example Attach a volume to an instance: JSON request** @@ -90,6 +92,11 @@ Request .. literalinclude:: ../../doc/api_samples/os-volumes/v2.49/attach-volume-to-server-req.json :language: javascript +**Example Attach a volume to an instance with "delete_on_termination" (v2.79): JSON request** + +.. literalinclude:: ../../doc/api_samples/os-volumes/v2.79/attach-volume-to-server-req.json + :language: javascript + Response -------- @@ -101,6 +108,7 @@ Response - serverId: server_id - volumeId: volumeId_resp - tag: device_tag_bdm_attachment_resp + - delete_on_termination: delete_on_termination_attachments_resp **Example Attach a volume to an instance: JSON response** @@ -112,6 +120,11 @@ Response .. literalinclude:: ../../doc/api_samples/os-volumes/v2.70/attach-volume-to-server-resp.json :language: javascript +**Example Attach a volume with "delete_on_termination" (v2.79): JSON response** + +.. literalinclude:: ../../doc/api_samples/os-volumes/v2.79/attach-volume-to-server-resp.json + :language: javascript + Show a detail of a volume attachment ==================================== @@ -142,6 +155,7 @@ Response - serverId: server_id - volumeId: volumeId_resp - tag: device_tag_bdm_attachment_resp + - delete_on_termination: delete_on_termination_attachments_resp **Example Show a detail of a volume attachment: JSON response** diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 150691f890c8..46dddd243653 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -2248,6 +2248,22 @@ delete_on_termination: in: body required: false type: boolean +delete_on_termination_attachments_req: + description: | + To delete the attached volume when the server is destroyed, specify ``true``. + Otherwise, specify ``false``. Default: ``false`` + in: body + required: false + type: boolean + min_version: 2.79 +delete_on_termination_attachments_resp: + description: | + A flag indicating if the attached volume will be deleted when the server is + deleted. + in: body + required: true + type: boolean + min_version: 2.79 deleted: description: | A boolean indicates whether this aggregate is deleted or not, if it has @@ -5148,9 +5164,7 @@ os-extended-volumes:volumes_attached: os-extended-volumes:volumes_attached.delete_on_termination: description: | A flag indicating if the attached volume will be deleted - when the server is deleted. By default this is False and - can only be set when creating a volume while creating a - server, which is commonly referred to as boot from volume. + when the server is deleted. By default this is False. in: body required: true type: boolean @@ -5158,9 +5172,7 @@ os-extended-volumes:volumes_attached.delete_on_termination: os-extended-volumes:volumes_attached.delete_on_termination_update_rebuild: description: | A flag indicating if the attached volume will be deleted - when the server is deleted. By default this is False and - can only be set when creating a volume while creating a - server, which is commonly referred to as boot from volume. + when the server is deleted. By default this is False. in: body required: true type: boolean diff --git a/doc/api_samples/os-volumes/v2.79/attach-volume-to-server-req.json b/doc/api_samples/os-volumes/v2.79/attach-volume-to-server-req.json new file mode 100644 index 000000000000..a6087418690f --- /dev/null +++ b/doc/api_samples/os-volumes/v2.79/attach-volume-to-server-req.json @@ -0,0 +1,7 @@ +{ + "volumeAttachment": { + "volumeId": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "tag": "foo", + "delete_on_termination": true + } +} diff --git a/doc/api_samples/os-volumes/v2.79/attach-volume-to-server-resp.json b/doc/api_samples/os-volumes/v2.79/attach-volume-to-server-resp.json new file mode 100644 index 000000000000..b761bc316886 --- /dev/null +++ b/doc/api_samples/os-volumes/v2.79/attach-volume-to-server-resp.json @@ -0,0 +1,10 @@ +{ + "volumeAttachment": { + "device": "/dev/vdd", + "id": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "serverId": "521b9e49-4855-4a9a-8474-759a86c1b12d", + "tag": "foo", + "volumeId": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "delete_on_termination": true + } +} diff --git a/doc/api_samples/os-volumes/v2.79/list-volume-attachments-resp.json b/doc/api_samples/os-volumes/v2.79/list-volume-attachments-resp.json new file mode 100644 index 000000000000..a6faf7e2f721 --- /dev/null +++ b/doc/api_samples/os-volumes/v2.79/list-volume-attachments-resp.json @@ -0,0 +1,20 @@ +{ + "volumeAttachments": [ + { + "device": "/dev/sdd", + "id": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "serverId": "fb6077e6-c10d-4e81-87fa-cb0f8c103051", + "tag": "foo", + "volumeId": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "delete_on_termination": false + }, + { + "device": "/dev/sdc", + "id": "a26887c6-c47b-4654-abb5-dfadf7d3f804", + "serverId": "fb6077e6-c10d-4e81-87fa-cb0f8c103051", + "tag": null, + "volumeId": "a26887c6-c47b-4654-abb5-dfadf7d3f804", + "delete_on_termination": false + } + ] +} diff --git a/doc/api_samples/os-volumes/v2.79/update-volume-req.json b/doc/api_samples/os-volumes/v2.79/update-volume-req.json new file mode 100644 index 000000000000..d95f96981aa4 --- /dev/null +++ b/doc/api_samples/os-volumes/v2.79/update-volume-req.json @@ -0,0 +1,5 @@ +{ + "volumeAttachment": { + "volumeId": "a26887c6-c47b-4654-abb5-dfadf7d3f805" + } +} diff --git a/doc/api_samples/os-volumes/v2.79/volume-attachment-detail-resp.json b/doc/api_samples/os-volumes/v2.79/volume-attachment-detail-resp.json new file mode 100644 index 000000000000..172aafc67217 --- /dev/null +++ b/doc/api_samples/os-volumes/v2.79/volume-attachment-detail-resp.json @@ -0,0 +1,10 @@ +{ + "volumeAttachment": { + "device": "/dev/sdd", + "id": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "serverId": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "tag": "foo", + "volumeId": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "delete_on_termination": false + } +} diff --git a/doc/api_samples/versions/v21-version-get-resp.json b/doc/api_samples/versions/v21-version-get-resp.json index 8ca415c77045..8712419940b9 100644 --- a/doc/api_samples/versions/v21-version-get-resp.json +++ b/doc/api_samples/versions/v21-version-get-resp.json @@ -19,7 +19,7 @@ } ], "status": "CURRENT", - "version": "2.78", + "version": "2.79", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/doc/api_samples/versions/versions-get-resp.json b/doc/api_samples/versions/versions-get-resp.json index a475df645c20..59e804c62999 100644 --- a/doc/api_samples/versions/versions-get-resp.json +++ b/doc/api_samples/versions/versions-get-resp.json @@ -22,7 +22,7 @@ } ], "status": "CURRENT", - "version": "2.78", + "version": "2.79", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/nova/api/openstack/api_version_request.py b/nova/api/openstack/api_version_request.py index c37c098c729c..a6982660ae90 100644 --- a/nova/api/openstack/api_version_request.py +++ b/nova/api/openstack/api_version_request.py @@ -205,6 +205,12 @@ REST_API_VERSION_HISTORY = """REST API Version History: shelved offload server. * 2.78 - Adds new API ``GET /servers/{server_id}/topology`` which shows NUMA topology of a given server. + * 2.79 - Adds support for specifying ``delete_on_termination`` field in the + request body to + ``POST /servers/{server_id}/os-volume_attachments`` and exposes + this via the response from + ``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 @@ -213,7 +219,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.78" +_MAX_API_VERSION = "2.79" DEFAULT_API_VERSION = _MIN_API_VERSION # Almost all proxy APIs which are related to network, images and baremetal diff --git a/nova/api/openstack/compute/rest_api_version_history.rst b/nova/api/openstack/compute/rest_api_version_history.rst index 2044f5762baf..4247ba0bcf7b 100644 --- a/nova/api/openstack/compute/rest_api_version_history.rst +++ b/nova/api/openstack/compute/rest_api_version_history.rst @@ -1011,3 +1011,18 @@ The default behavior is configurable using two new policies: * ``compute:server:topology:index`` * ``compute:server:topology:host:index`` + +2.79 +---- + +API microversion 2.79 adds support for specifying the ``delete_on_termination`` +field in the request body when attaching a volume to a server, to support +configuring whether to delete the data volume when the server is destroyed. +Also, ``delete_on_termination`` is added to the GET responses when showing +attached volumes. + +The affected APIs are as follows: + +* ``POST /servers/{server_id}/os-volume_attachments`` +* ``GET /servers/{server_id}/os-volume_attachments`` +* ``GET /servers/{server_id}/os-volume_attachments/{volume_id}`` diff --git a/nova/api/openstack/compute/schemas/volumes.py b/nova/api/openstack/compute/schemas/volumes.py index 9a6a3d9b3628..2848f7f2ebb9 100644 --- a/nova/api/openstack/compute/schemas/volumes.py +++ b/nova/api/openstack/compute/schemas/volumes.py @@ -87,6 +87,10 @@ create_volume_attachment_v249 = copy.deepcopy(create_volume_attachment) create_volume_attachment_v249['properties']['volumeAttachment'][ 'properties']['tag'] = parameter_types.tag +create_volume_attachment_v279 = copy.deepcopy(create_volume_attachment_v249) +create_volume_attachment_v279['properties']['volumeAttachment'][ + 'properties']['delete_on_termination'] = parameter_types.boolean + update_volume_attachment = copy.deepcopy(create_volume_attachment) del update_volume_attachment['properties']['volumeAttachment'][ 'properties']['device'] diff --git a/nova/api/openstack/compute/volumes.py b/nova/api/openstack/compute/volumes.py index d593787bcb07..42cac905e07e 100644 --- a/nova/api/openstack/compute/volumes.py +++ b/nova/api/openstack/compute/volumes.py @@ -207,12 +207,16 @@ 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(bdm, show_tag=False, + show_delete_on_termination=False): """Maps keys for attachment details view. :param bdm: BlockDeviceMapping object for an attached volume :param show_tag: True if the "tag" field should be in the response, False to exclude the "tag" field from the response + :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 """ d = _translate_attachment_summary_view( @@ -221,6 +225,9 @@ def _translate_attachment_detail_view(bdm, show_tag=False): if show_tag: d['tag'] = bdm.tag + if show_delete_on_termination: + d['delete_on_termination'] = bdm.delete_on_termination + return d @@ -282,9 +289,13 @@ class VolumeAttachmentController(wsgi.Controller): results = [] show_tag = api_version_request.is_supported(req, '2.70') + show_delete_on_termination = api_version_request.is_supported( + req, '2.79') for bdm in limited_list: if bdm.volume_id: - va = _translate_attachment_detail_view(bdm, show_tag=show_tag) + va = _translate_attachment_detail_view( + bdm, show_tag=show_tag, + show_delete_on_termination=show_delete_on_termination) results.append(va) return {'volumeAttachments': results} @@ -308,13 +319,18 @@ class VolumeAttachmentController(wsgi.Controller): raise exc.HTTPNotFound(explanation=msg) 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( - bdm, show_tag=show_tag)} + bdm, show_tag=show_tag, + show_delete_on_termination=show_delete_on_termination)} # TODO(mriedem): This API should return a 202 instead of a 200 response. @wsgi.expected_errors((400, 403, 404, 409)) @validation.schema(volumes_schema.create_volume_attachment, '2.0', '2.48') - @validation.schema(volumes_schema.create_volume_attachment_v249, '2.49') + @validation.schema(volumes_schema.create_volume_attachment_v249, '2.49', + '2.78') + @validation.schema(volumes_schema.create_volume_attachment_v279, '2.79') def create(self, req, server_id, body): """Attach a volume to an instance.""" context = req.environ['nova.context'] @@ -323,6 +339,8 @@ class VolumeAttachmentController(wsgi.Controller): volume_id = body['volumeAttachment']['volumeId'] device = body['volumeAttachment'].get('device') tag = body['volumeAttachment'].get('tag') + delete_on_termination = body['volumeAttachment'].get( + 'delete_on_termination', False) instance = common.get_instance(self.compute_api, context, server_id) @@ -335,7 +353,8 @@ class VolumeAttachmentController(wsgi.Controller): supports_multiattach = common.supports_multiattach_volume(req) device = self.compute_api.attach_volume( context, instance, volume_id, device, tag=tag, - supports_multiattach=supports_multiattach) + supports_multiattach=supports_multiattach, + delete_on_termination=delete_on_termination) except exception.VolumeNotFound as e: raise exc.HTTPNotFound(explanation=e.format_message()) except (exception.InstanceIsLocked, @@ -367,6 +386,8 @@ class VolumeAttachmentController(wsgi.Controller): attachment['device'] = device if api_version_request.is_supported(req, '2.70'): attachment['tag'] = tag + if api_version_request.is_supported(req, '2.79'): + attachment['delete_on_termination'] = delete_on_termination return {'volumeAttachment': attachment} @wsgi.response(202) diff --git a/nova/compute/api.py b/nova/compute/api.py index 1dd6b20be026..db5287e10094 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3977,7 +3977,7 @@ class API(base.Base): def _create_volume_bdm(self, context, instance, device, volume, disk_bus, device_type, is_local_creation=False, - tag=None): + tag=None, delete_on_termination=False): volume_id = volume['id'] if is_local_creation: # when the creation is done locally we can't specify the device @@ -3996,7 +3996,8 @@ class API(base.Base): instance_uuid=instance.uuid, boot_index=None, volume_id=volume_id, device_name=None, guest_format=None, - disk_bus=disk_bus, device_type=device_type) + disk_bus=disk_bus, device_type=device_type, + delete_on_termination=delete_on_termination) volume_bdm.create() else: # NOTE(vish): This is done on the compute host because we want @@ -4008,6 +4009,8 @@ class API(base.Base): context, instance, device, volume_id, disk_bus=disk_bus, device_type=device_type, tag=tag, multiattach=volume['multiattach']) + volume_bdm.delete_on_termination = delete_on_termination + volume_bdm.save() return volume_bdm def _check_volume_already_attached_to_instance(self, context, instance, @@ -4056,7 +4059,8 @@ class API(base.Base): # override it. def _attach_volume(self, context, instance, volume, device, disk_bus, device_type, tag=None, - supports_multiattach=False): + supports_multiattach=False, + delete_on_termination=False): """Attach an existing volume to an existing instance. This method is separated to make it possible for cells version @@ -4064,7 +4068,8 @@ class API(base.Base): """ volume_bdm = self._create_volume_bdm( context, instance, device, volume, disk_bus=disk_bus, - device_type=device_type, tag=tag) + device_type=device_type, tag=tag, + delete_on_termination=delete_on_termination) try: self._check_attach_and_reserve_volume(context, volume, instance, volume_bdm, @@ -4079,7 +4084,8 @@ class API(base.Base): return volume_bdm.device_name def _attach_volume_shelved_offloaded(self, context, instance, volume, - device, disk_bus, device_type): + device, disk_bus, device_type, + delete_on_termination): """Attach an existing volume to an instance in shelved offloaded state. Attaching a volume for an instance in shelved offloaded state requires @@ -4109,7 +4115,8 @@ class API(base.Base): volume_bdm = self._create_volume_bdm( context, instance, device, volume, disk_bus=disk_bus, - device_type=device_type, is_local_creation=True) + device_type=device_type, is_local_creation=True, + delete_on_termination=delete_on_termination) try: self._check_attach_and_reserve_volume(context, volume, instance, volume_bdm) @@ -4131,7 +4138,8 @@ class API(base.Base): vm_states.SHELVED_OFFLOADED]) def attach_volume(self, context, instance, volume_id, device=None, disk_bus=None, device_type=None, tag=None, - supports_multiattach=False): + supports_multiattach=False, + delete_on_termination=False): """Attach an existing volume to an existing instance.""" # NOTE(vish): Fail fast if the device is not going to pass. This # will need to be removed along with the test if we @@ -4172,11 +4180,13 @@ class API(base.Base): volume, device, disk_bus, - device_type) + device_type, + delete_on_termination) return self._attach_volume(context, instance, volume, device, disk_bus, device_type, tag=tag, - supports_multiattach=supports_multiattach) + supports_multiattach=supports_multiattach, + delete_on_termination=delete_on_termination) # TODO(stephenfin): Fold this back in now that cells v1 no longer needs to # override it. diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/attach-volume-to-server-req.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/attach-volume-to-server-req.json.tpl new file mode 100644 index 000000000000..92c13861bb6f --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/attach-volume-to-server-req.json.tpl @@ -0,0 +1,7 @@ +{ + "volumeAttachment": { + "volumeId": "%(volume_id)s", + "tag": "%(tag)s", + "delete_on_termination": true + } +} diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/attach-volume-to-server-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/attach-volume-to-server-resp.json.tpl new file mode 100644 index 000000000000..34eacc94d233 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/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 + } +} diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/list-volume-attachments-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/list-volume-attachments-resp.json.tpl new file mode 100644 index 000000000000..6782aed70e78 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/list-volume-attachments-resp.json.tpl @@ -0,0 +1,20 @@ +{ + "volumeAttachments": [ + { + "device": "/dev/sdd", + "id": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "serverId": "%(uuid)s", + "tag": "%(tag)s", + "volumeId": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "delete_on_termination": false + }, + { + "device": "/dev/sdc", + "id": "a26887c6-c47b-4654-abb5-dfadf7d3f804", + "serverId": "%(uuid)s", + "tag": null, + "volumeId": "a26887c6-c47b-4654-abb5-dfadf7d3f804", + "delete_on_termination": false + } + ] +} diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/update-volume-req.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/update-volume-req.json.tpl new file mode 100644 index 000000000000..41411472ab09 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/update-volume-req.json.tpl @@ -0,0 +1,5 @@ +{ + "volumeAttachment": { + "volumeId": "%(volume_id)s" + } +} diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/volume-attachment-detail-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/volume-attachment-detail-resp.json.tpl new file mode 100644 index 000000000000..7f4ea81819ac --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.79/volume-attachment-detail-resp.json.tpl @@ -0,0 +1,10 @@ +{ + "volumeAttachment": { + "device": "/dev/sdd", + "id": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "serverId": "%(uuid)s", + "tag": "%(tag)s", + "volumeId": "a26887c6-c47b-4654-abb5-dfadf7d3f803", + "delete_on_termination": false + } +} diff --git a/nova/tests/functional/api_sample_tests/test_volumes.py b/nova/tests/functional/api_sample_tests/test_volumes.py index 353660cb6af0..f3582cd4bbac 100644 --- a/nova/tests/functional/api_sample_tests/test_volumes.py +++ b/nova/tests/functional/api_sample_tests/test_volumes.py @@ -209,6 +209,11 @@ class VolumeAttachmentsSample(test_servers.ServersSampleBase): """ return {} + # TODO(mriedem): There is really no good reason we should have to stub + # so much of this DB and compute service code when we can just use the + # CinderFixture. The stubs make these tests very brittle and potentially + # false regarding how the API/compute service interaction works. + def _stub_db_bdms_get_all_by_instance(self, server_id): def fake_bdms_get_all_by_instance(context, instance_uuid, @@ -240,7 +245,7 @@ class VolumeAttachmentsSample(test_servers.ServersSampleBase): {'id': 1, 'volume_id': self.OLD_VOLUME_ID, 'instance_uuid': instance_uuid, 'source_type': 'volume', 'destination_type': 'volume', 'device_name': '/dev/sdd', - 'tag': tag}) + 'tag': tag, 'delete_on_termination': False}) ) def _stub_compute_api_get(self): @@ -264,9 +269,14 @@ class VolumeAttachmentsSample(test_servers.ServersSampleBase): device_name = '/dev/vdd' bdm = objects.BlockDeviceMapping() bdm['device_name'] = device_name + bdm['delete_on_termination'] = True self.stub_out( 'nova.compute.manager.ComputeManager.reserve_block_device_name', lambda *a, **k: bdm) + # 2.79+ will save the delete_on_termination value on the BDM after + # reserve_block_device_name "creates" the BDM. + self.stub_out('nova.objects.BlockDeviceMapping.save', + lambda *a, **k: None) self.stub_out( 'nova.compute.manager.ComputeManager.attach_volume', lambda *a, **k: None) @@ -366,3 +376,11 @@ class VolumeAttachmentsSampleV270(VolumeAttachmentsSampleV249): self.OLD_VOLUME_ID: 'foo', self.NEW_VOLUME_ID: None } + + +class VolumeAttachmentsSampleV279(VolumeAttachmentsSampleV270): + """Microversion 2.79 adds the "delete_on_termination" parameter to the + request and response body. + """ + microversion = '2.79' + scenarios = [('v2_79', {'api_major_version': 'v2.1'})] diff --git a/nova/tests/unit/api/openstack/compute/test_volumes.py b/nova/tests/unit/api/openstack/compute/test_volumes.py index 34eb45cefbbd..9377befa4549 100644 --- a/nova/tests/unit/api/openstack/compute/test_volumes.py +++ b/nova/tests/unit/api/openstack/compute/test_volumes.py @@ -572,7 +572,8 @@ class VolumeAttachTestsV21(test.NoDBTestCase): self.stub_out('nova.compute.api.API.attach_volume', lambda self, context, instance, volume_id, device, tag=None, - supports_multiattach=False: None) + supports_multiattach=False, + delete_on_termination=False: None) body = {'volumeAttachment': {'volumeId': FAKE_UUID_A, 'device': '/dev/fake'}} result = self.attachments.create(self.req, FAKE_UUID, body=body) @@ -649,7 +650,8 @@ class VolumeAttachTestsV21(test.NoDBTestCase): mock_attach_volume.assert_called_once_with( self.req.environ['nova.context'], test.MatchType(objects.Instance), FAKE_UUID_A, '/dev/fake', - supports_multiattach=supports_multiattach, tag=None) + supports_multiattach=supports_multiattach, + delete_on_termination=False, tag=None) def test_attach_volume_bad_id(self): self.stub_out('nova.compute.api.API.attach_volume', @@ -993,6 +995,102 @@ class VolumeAttachTestsV2_75(VolumeAttachTestsV21): req, FAKE_UUID) +class VolumeAttachTestsV279(VolumeAttachTestsV2_75): + microversion = '2.79' + + def setUp(self): + super(VolumeAttachTestsV279, self).setUp() + self.controller = volumes_v21.VolumeAttachmentController() + self.expected_show = {'volumeAttachment': + {'device': '/dev/fake0', + 'serverId': FAKE_UUID, + 'id': FAKE_UUID_A, + 'volumeId': FAKE_UUID_A, + 'tag': None, + 'delete_on_termination': False + }} + + def _get_req(self, body, microversion=None): + req = fakes.HTTPRequest.blank( + '/v2/servers/id/os-volume_attachments/uuid', + version=microversion or self.microversion) + req.body = jsonutils.dump_as_bytes(body) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + return req + + def test_create_volume_attach_pre_v279(self): + """Tests the case that the user tries to attach a volume with + delete_on_termination field, but before using microversion 2.79. + """ + body = {'volumeAttachment': {'volumeId': FAKE_UUID_A, + 'delete_on_termination': False}} + req = self._get_req(body, microversion='2.78') + ex = self.assertRaises(exception.ValidationError, + self.controller.create, + req, FAKE_UUID, body=body) + self.assertIn("Additional properties are not allowed", + six.text_type(ex)) + + @mock.patch('nova.compute.api.API.attach_volume', return_value=None) + def test_attach_volume_pre_v279(self, mock_attach_volume): + """Before microversion 2.79, attach a volume will not contain + 'delete_on_termination' field in the response. + """ + body = {'volumeAttachment': {'volumeId': FAKE_UUID_A}} + req = self._get_req(body, microversion='2.78') + result = self.attachments.create(req, FAKE_UUID, body=body) + self.assertNotIn('delete_on_termination', result['volumeAttachment']) + mock_attach_volume.assert_called_once_with( + req.environ['nova.context'], test.MatchType(objects.Instance), + FAKE_UUID_A, None, tag=None, supports_multiattach=True, + delete_on_termination=False) + + @mock.patch('nova.compute.api.API.attach_volume', return_value=None) + def test_attach_volume_with_delete_on_termination_default_value( + self, mock_attach_volume): + """Test attach a volume doesn't specify 'delete_on_termination' in + the request, you will be get it's default value in the response. + The delete_on_termination's default value is 'False'. + """ + body = {'volumeAttachment': {'volumeId': FAKE_UUID_A}} + req = self._get_req(body) + result = self.attachments.create(req, FAKE_UUID, body=body) + self.assertFalse(result['volumeAttachment']['delete_on_termination']) + mock_attach_volume.assert_called_once_with( + req.environ['nova.context'], test.MatchType(objects.Instance), + FAKE_UUID_A, None, tag=None, supports_multiattach=True, + delete_on_termination=False) + + def test_create_volume_attach_invalid_delete_on_termination_empty(self): + body = {'volumeAttachment': {'volumeId': FAKE_UUID_A, + 'delete_on_termination': None}} + req = self._get_req(body) + ex = self.assertRaises(exception.ValidationError, + self.controller.create, + req, FAKE_UUID, body=body) + self.assertIn("Invalid input for field/attribute " + "delete_on_termination.", six.text_type(ex)) + + def test_create_volume_attach_invalid_delete_on_termination_value(self): + """"Test the case that the user tries to set the delete_on_termination + value not in the boolean or string-boolean check, the valid boolean + value are: + + [True, 'True', 'TRUE', 'true', '1', 'ON', 'On', 'on', 'YES', 'Yes', + 'yes', False, 'False', 'FALSE', 'false', '0', 'OFF', 'Off', 'off', + 'NO', 'No', 'no'] + """ + body = {'volumeAttachment': {'volumeId': FAKE_UUID_A, + 'delete_on_termination': 'foo'}} + req = self._get_req(body) + ex = self.assertRaises(exception.ValidationError, + self.controller.create, + req, FAKE_UUID, body=body) + self.assertIn("Invalid input for field/attribute " + "delete_on_termination.", six.text_type(ex)) + + class SwapVolumeMultiattachTestCase(test.NoDBTestCase): @mock.patch('nova.api.openstack.common.get_instance') diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 95b36938e735..27910b8661c8 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -10682,7 +10682,7 @@ class ComputeAPITestCase(BaseTestCase): volume = {'id': uuids.volume} self.compute_api._attach_volume_shelved_offloaded( self.context, instance, volume, - '/dev/vdb', 'ide', 'cdrom') + '/dev/vdb', 'ide', 'cdrom', False) mock_attach_and_reserve.assert_called_once_with(self.context, volume, instance, @@ -10722,7 +10722,7 @@ class ComputeAPITestCase(BaseTestCase): volume = {'id': uuids.volume} self.compute_api._attach_volume_shelved_offloaded( self.context, instance, volume, - '/dev/vdb', 'ide', 'cdrom') + '/dev/vdb', 'ide', 'cdrom', False) mock_attach_and_reserve.assert_called_once_with(self.context, volume, instance, diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 7972a989ec0f..93a75ce49e9a 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -361,8 +361,10 @@ class _ComputeAPIUnitTestMixIn(object): self._test_specified_ip_and_multiple_instances_helper( requested_networks) + @mock.patch('nova.objects.BlockDeviceMapping.save') @mock.patch.object(compute_rpcapi.ComputeAPI, 'reserve_block_device_name') - def test_create_volume_bdm_call_reserve_dev_name(self, mock_reserve): + def test_create_volume_bdm_call_reserve_dev_name(self, mock_reserve, + mock_bdm_save): bdm = objects.BlockDeviceMapping( **fake_block_device.FakeDbBlockDeviceDict( { @@ -384,6 +386,7 @@ class _ComputeAPIUnitTestMixIn(object): None) self.assertTrue(mock_reserve.called) self.assertEqual(result, bdm) + mock_bdm_save.assert_called_once_with() @mock.patch.object(objects.BlockDeviceMapping, 'create') def test_create_volume_bdm_local_creation(self, bdm_create): diff --git a/releasenotes/notes/bp-support-delete-on-termination-in-server-attach-volume-5d08b4e97fdd24f9.yaml b/releasenotes/notes/bp-support-delete-on-termination-in-server-attach-volume-5d08b4e97fdd24f9.yaml new file mode 100644 index 000000000000..2ef1e11b0e5e --- /dev/null +++ b/releasenotes/notes/bp-support-delete-on-termination-in-server-attach-volume-5d08b4e97fdd24f9.yaml @@ -0,0 +1,14 @@ +--- +features: + - | + Microversion 2.79 adds support for specifying the ``delete_on_termination`` + field in the request body when attaching a volume to a server, to support + configuring whether to delete the data volume when the server is destroyed. + Also, ``delete_on_termination`` is added to the GET responses when showing + attached volumes. + + The affected APIs are as follows: + + * ``POST /servers/{server_id}/os-volume_attachments`` + * ``GET /servers/{server_id}/os-volume_attachments`` + * ``GET /servers/{server_id}/os-volume_attachments/{volume_id}``