diff --git a/api-ref/source/os-volume-attachments.inc b/api-ref/source/os-volume-attachments.inc index 3deb96d05a42..5ca5432b71ed 100644 --- a/api-ref/source/os-volume-attachments.inc +++ b/api-ref/source/os-volume-attachments.inc @@ -177,19 +177,25 @@ Update a volume attachment. .. note:: This action only valid when the server is in ACTIVE, PAUSED and RESIZED state, or a conflict(409) error will be returned. -.. warning:: This API is typically meant to only be used as part of a larger - orchestrated volume migration operation initiated in the block - storage service via the ``os-retype`` or ``os-migrate_volume`` - volume actions. Direct usage of this API is not recommended and - may result in needing to hard reboot the server to update details - within the guest such as block storage serial IDs. Furthermore, - this API is only implemented by `certain compute drivers`_. +.. warning:: When updating volumeId, this API is typically meant to + only be used as part of a larger orchestrated volume + migration operation initiated in the block storage + service via the ``os-retype`` or ``os-migrate_volume`` + volume actions. Direct usage of this API to update + volumeId is not recommended and may result in needing to + hard reboot the server to update details within the guest + such as block storage serial IDs. Furthermore, updating + volumeId via this API is only implemented by `certain + compute drivers`_. .. _certain compute drivers: https://docs.openstack.org/nova/latest/user/support-matrix.html#operation_swap_volume -Policy defaults enable only users with the administrative role to perform -this operation. Cloud providers can change these permissions through the -``policy.json`` file. +Policy default role is 'rule:system_admin_or_owner', its scope is +[system, project], which allow project members or system admins to +change the fields of an attached volume of a server. Policy defaults +enable only users with the administrative role to change ``volumeId`` +via this operation. Cloud providers can change these permissions +through the ``policy.json`` file. Updating, or what is commonly referred to as "swapping", volume attachments with volumes that have more than one read/write attachment, is not supported. @@ -207,10 +213,19 @@ Request - volume_id: volume_id_swap_src - volumeAttachment: volumeAttachment_put - volumeId: volumeId_swap + - delete_on_termination: delete_on_termination_put_req + - device: attachment_device_put_req + - serverId: attachment_server_id_put_req + - tag: device_tag_bdm_attachment_put_req + - id: attachment_id_put_req -**Example Update a volume attachment: JSON request** +.. note:: Other than ``volumeId``, as of v2.85 only + ``delete_on_termination`` may be changed from the current + value. -.. literalinclude:: ../../doc/api_samples/os-volumes/update-volume-req.json +**Example Update a volume attachment (v2.85): JSON request** + +.. literalinclude:: ../../doc/api_samples/os-volumes/v2.85/update-volume-attachment-delete-flag-req.json :language: javascript Response diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 4dff3fcf2b73..263835d84848 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -1776,12 +1776,26 @@ associate_host: in: body required: true type: string +attachment_device_put_req: + description: | + Name of the device in the attachment object, such as, ``/dev/vdb``. + in: body + required: false + type: string + min_version: 2.85 attachment_device_resp: description: | Name of the device in the attachment object, such as, ``/dev/vdb``. in: body required: false type: string +attachment_id_put_req: + description: | + The UUID of the attachment. + in: body + required: false + type: string + min_version: 2.85 attachment_id_required: description: | The UUID of the attachment. @@ -1794,6 +1808,13 @@ attachment_id_resp: in: body required: false type: string +attachment_server_id_put_req: + description: | + The UUID of the server. + in: body + required: false + type: string + min_version: 2.85 attachment_server_id_resp: description: | The UUID of the server. @@ -2294,6 +2315,14 @@ delete_on_termination_attachments_resp: required: true type: boolean min_version: 2.79 +delete_on_termination_put_req: + description: | + A flag indicating if the attached volume will be deleted when the server is + deleted. + in: body + required: false + type: boolean + min_version: 2.85 deleted: description: | A boolean indicates whether this aggregate is deleted or not, if it has @@ -2384,6 +2413,13 @@ device_tag_bdm_attachment: required: false type: string min_version: 2.49 +device_tag_bdm_attachment_put_req: + description: | + The device tag applied to the volume block device or ``null``. + in: body + required: true + type: string + min_version: 2.85 device_tag_bdm_attachment_resp: description: | The device tag applied to the volume block device or ``null``. @@ -7370,7 +7406,8 @@ volumeAttachment_post: volumeAttachment_put: description: | A dictionary representation of a volume attachment containing the field - ``volumeId`` which is the UUID of the replacement volume. + ``volumeId`` which is the UUID of the replacement volume, and other fields + to update in the attachment. in: body required: true type: object diff --git a/doc/api_samples/os-volumes/v2.85/attach-volume-to-server-req.json b/doc/api_samples/os-volumes/v2.85/attach-volume-to-server-req.json new file mode 100644 index 000000000000..b4429e12e968 --- /dev/null +++ b/doc/api_samples/os-volumes/v2.85/attach-volume-to-server-req.json @@ -0,0 +1,7 @@ +{ + "volumeAttachment": { + "volumeId": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113", + "tag": "foo", + "delete_on_termination": true + } +} \ No newline at end of file diff --git a/doc/api_samples/os-volumes/v2.85/attach-volume-to-server-resp.json b/doc/api_samples/os-volumes/v2.85/attach-volume-to-server-resp.json new file mode 100644 index 000000000000..3a60cdc0d095 --- /dev/null +++ b/doc/api_samples/os-volumes/v2.85/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": "09b3b9d1-b8c5-48e1-841d-62c3ef967a88", + "tag": "foo", + "volumeId": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113" + } +} \ No newline at end of file diff --git a/doc/api_samples/os-volumes/v2.85/list-volume-attachments-resp.json b/doc/api_samples/os-volumes/v2.85/list-volume-attachments-resp.json new file mode 100644 index 000000000000..ffe7c0baf1eb --- /dev/null +++ b/doc/api_samples/os-volumes/v2.85/list-volume-attachments-resp.json @@ -0,0 +1,20 @@ +{ + "volumeAttachments": [ + { + "delete_on_termination": false, + "device": "/dev/sdc", + "id": "227cc671-f30b-4488-96fd-7d0bf13648d8", + "serverId": "d5e4ae35-ac0e-4311-a8c5-0ee863e951d9", + "tag": null, + "volumeId": "227cc671-f30b-4488-96fd-7d0bf13648d8" + }, + { + "delete_on_termination": true, + "device": "/dev/sdb", + "id": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113", + "serverId": "d5e4ae35-ac0e-4311-a8c5-0ee863e951d9", + "tag": "foo", + "volumeId": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113" + } + ] +} \ No newline at end of file diff --git a/doc/api_samples/os-volumes/v2.85/update-volume-attachment-delete-flag-req.json b/doc/api_samples/os-volumes/v2.85/update-volume-attachment-delete-flag-req.json new file mode 100644 index 000000000000..30105458e7cd --- /dev/null +++ b/doc/api_samples/os-volumes/v2.85/update-volume-attachment-delete-flag-req.json @@ -0,0 +1,6 @@ +{ + "volumeAttachment": { + "volumeId": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113", + "delete_on_termination": true + } +} diff --git a/doc/api_samples/os-volumes/v2.85/update-volume-req.json b/doc/api_samples/os-volumes/v2.85/update-volume-req.json new file mode 100644 index 000000000000..e5ad47aa3cce --- /dev/null +++ b/doc/api_samples/os-volumes/v2.85/update-volume-req.json @@ -0,0 +1,5 @@ +{ + "volumeAttachment": { + "volumeId": "227cc671-f30b-4488-96fd-7d0bf13648d8" + } +} \ No newline at end of file diff --git a/doc/api_samples/os-volumes/v2.85/volume-attachment-detail-resp.json b/doc/api_samples/os-volumes/v2.85/volume-attachment-detail-resp.json new file mode 100644 index 000000000000..4a54243c2b1c --- /dev/null +++ b/doc/api_samples/os-volumes/v2.85/volume-attachment-detail-resp.json @@ -0,0 +1,10 @@ +{ + "volumeAttachment": { + "delete_on_termination": true, + "device": "/dev/sdb", + "id": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113", + "serverId": "2aad99d3-7aa4-41e9-b4e6-3f960b115d68", + "tag": "foo", + "volumeId": "a07f71dc-8151-4e7d-a0cc-cd24a3f11113" + } +} \ No newline at end of file diff --git a/doc/api_samples/versions/v21-version-get-resp.json b/doc/api_samples/versions/v21-version-get-resp.json index 5e5c77561fbc..2d1760aa312f 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.84", + "version": "2.85", "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 8eb238e31930..d0a29d7ff181 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.84", + "version": "2.85", "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 3d2f0549ee88..b6e354e32caf 100644 --- a/nova/api/openstack/api_version_request.py +++ b/nova/api/openstack/api_version_request.py @@ -227,6 +227,10 @@ REST_API_VERSION_HISTORY = """REST API Version History: * 2.83 - Allow more filter parameters for ``GET /servers/detail`` and ``GET /servers`` for non-admin. * 2.84 - Adds ``details`` field to instance action events. + * 2.85 - Add support for + ``PUT /servers/{server_id}/os-volume_attachments/{volume_id}`` + which supports specifying the ``delete_on_termination`` field in + the request body to change the attached volume's flag. """ # The minimum and maximum versions of the API supported @@ -235,7 +239,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.84" +_MAX_API_VERSION = "2.85" 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 28c567e652c2..cb51613d6a6a 100644 --- a/nova/api/openstack/compute/rest_api_version_history.rst +++ b/nova/api/openstack/compute/rest_api_version_history.rst @@ -1113,3 +1113,10 @@ The ``GET /servers/{server_id}/os-instance-actions/{request_id}`` API returns a ``details`` parameter for each failed event with a fault message, similar to the server ``fault.message`` parameter in ``GET /servers/{server_id}`` for a server with status ``ERROR``. + +2.85 +---- + +Adds the ability to specify ``delete_on_termination`` in the +``PUT /servers/{server_id}/os-volume_attachments/{volume_id}`` API, which +allows changing the behavior of volume deletion on instance deletion. diff --git a/nova/api/openstack/compute/schemas/volumes.py b/nova/api/openstack/compute/schemas/volumes.py index 2848f7f2ebb9..6ac52354c1b3 100644 --- a/nova/api/openstack/compute/schemas/volumes.py +++ b/nova/api/openstack/compute/schemas/volumes.py @@ -74,7 +74,7 @@ create_volume_attachment = { # NOTE: The validation pattern from match_device() in # nova/block_device.py. 'pattern': '(^/dev/x{0,1}[a-z]{0,1}d{0,1})([a-z]+)[0-9]*$' - } + }, }, 'required': ['volumeId'], 'additionalProperties': False, @@ -95,6 +95,35 @@ update_volume_attachment = copy.deepcopy(create_volume_attachment) del update_volume_attachment['properties']['volumeAttachment'][ 'properties']['device'] +# NOTE(brinzhang): Allow attachment_id, serverId, device, tag, and +# delete_on_termination to be specified for RESTfulness, even though +# we will not allow updating all of them. +update_volume_attachment_v285 = { + 'type': 'object', + 'properties': { + 'volumeAttachment': { + 'type': 'object', + 'properties': { + 'volumeId': parameter_types.volume_id, + 'device': { + 'type': ['string', 'null'], + # NOTE: The validation pattern from match_device() in + # nova/block_device.py. + 'pattern': '(^/dev/x{0,1}[a-z]{0,1}d{0,1})([a-z]+)[0-9]*$' + }, + 'tag': parameter_types.tag, + 'delete_on_termination': parameter_types.boolean, + 'serverId': parameter_types.server_id, + 'id': parameter_types.attachment_id + }, + 'required': ['volumeId'], + 'additionalProperties': False, + }, + }, + 'required': ['volumeAttachment'], + 'additionalProperties': False, +} + index_query = { 'type': 'object', 'properties': { diff --git a/nova/api/openstack/compute/volumes.py b/nova/api/openstack/compute/volumes.py index 3fa49356a397..6129fa2d5477 100644 --- a/nova/api/openstack/compute/volumes.py +++ b/nova/api/openstack/compute/volumes.py @@ -391,15 +391,8 @@ class VolumeAttachmentController(wsgi.Controller): attachment['delete_on_termination'] = delete_on_termination return {'volumeAttachment': attachment} - @wsgi.response(202) - @wsgi.expected_errors((400, 404, 409)) - @validation.schema(volumes_schema.update_volume_attachment) - def update(self, req, server_id, id, body): + def _update_volume_swap(self, req, instance, id, body): context = req.environ['nova.context'] - instance = common.get_instance(self.compute_api, context, server_id) - context.can(va_policies.POLICY_ROOT % 'update', - target={'project_id': instance.project_id}) - old_volume_id = id try: old_volume = self.volume_api.get(context, old_volume_id) @@ -431,7 +424,67 @@ class VolumeAttachmentController(wsgi.Controller): raise exc.HTTPConflict(explanation=e.format_message()) except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, - 'swap_volume', server_id) + 'swap_volume', instance.uuid) + + def _update_volume_regular(self, req, instance, id, body): + context = req.environ['nova.context'] + att = body['volumeAttachment'] + # NOTE(danms): We may be doing an update of regular parameters in + # the midst of a swap operation, so to find the original BDM, we need + # to use the old volume ID, which is the one in the path. + volume_id = id + + try: + bdm = objects.BlockDeviceMapping.get_by_volume_and_instance( + context, volume_id, instance.uuid) + + # NOTE(danms): The attachment id is just the (current) volume id + if 'id' in att and att['id'] != volume_id: + raise exc.HTTPBadRequest(explanation='The id property is ' + 'not mutable') + if 'serverId' in att and att['serverId'] != instance.uuid: + raise exc.HTTPBadRequest(explanation='The serverId property ' + 'is not mutable') + if 'device' in att and att['device'] != bdm.device_name: + raise exc.HTTPBadRequest(explanation='The device property is ' + 'not mutable') + if 'tag' in att and att['tag'] != bdm.tag: + raise exc.HTTPBadRequest(explanation='The tag property is ' + 'not mutable') + if 'delete_on_termination' in att: + bdm.delete_on_termination = att['delete_on_termination'] + bdm.save() + except exception.VolumeBDMNotFound as e: + raise exc.HTTPNotFound(explanation=e.format_message()) + + @wsgi.response(202) + @wsgi.expected_errors((400, 404, 409)) + @validation.schema(volumes_schema.update_volume_attachment, '2.0', '2.84') + @validation.schema(volumes_schema.update_volume_attachment_v285, + min_version='2.85') + def update(self, req, server_id, id, body): + context = req.environ['nova.context'] + instance = common.get_instance(self.compute_api, context, server_id) + # TODO(danms): For now, use the existing admin-only policy for update. + # Later, split off the swap_volume permission and check the correct + # policy based on what is being asked by the client. + context.can(va_policies.POLICY_ROOT % 'update', + target={'project_id': instance.project_id}) + + attachment = body['volumeAttachment'] + volume_id = attachment['volumeId'] + only_swap = not api_version_request.is_supported(req, '2.85') + if only_swap: + # NOTE(danms): Original behavior is always call swap on PUT + # FIXME(danms): Check the swap volume policy here + self._update_volume_swap(req, instance, id, body) + else: + # NOTE(danms): New behavior is update any supported attachment + # properties first, and then call swap if volumeId differs + # FIXME(danms): Check the volume attachment update policy here + self._update_volume_regular(req, instance, id, body) + if id != volume_id: + self._update_volume_swap(req, instance, id, body) @wsgi.response(202) @wsgi.expected_errors((400, 403, 404, 409)) diff --git a/nova/api/validation/parameter_types.py b/nova/api/validation/parameter_types.py index caeb2d7eb055..144a1f64cfe3 100644 --- a/nova/api/validation/parameter_types.py +++ b/nova/api/validation/parameter_types.py @@ -336,6 +336,11 @@ volume_id = { } +attachment_id = { + 'type': 'string', 'format': 'uuid' +} + + volume_type = { 'type': ['string', 'null'], 'minLength': 0, 'maxLength': 255 } diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/attach-volume-to-server-req.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/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.85/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.85/attach-volume-to-server-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/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.85/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.85/list-volume-attachments-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/list-volume-attachments-resp.json.tpl new file mode 100644 index 000000000000..700cf5e759f2 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/list-volume-attachments-resp.json.tpl @@ -0,0 +1,20 @@ +{ + "volumeAttachments": [ + { + "device": "%(device)s", + "id": "%(volume_id)s", + "serverId": "%(uuid)s", + "tag": "%(tag)s", + "volumeId": "%(volume_id)s", + "delete_on_termination": true + }, + { + "device": "%(text)s", + "id": "%(volume_id2)s", + "serverId": "%(uuid)s", + "tag": null, + "volumeId": "%(volume_id2)s", + "delete_on_termination": false + } + ] +} diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/update-volume-attachment-delete-flag-req.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/update-volume-attachment-delete-flag-req.json.tpl new file mode 100644 index 000000000000..2aba36133c5e --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/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 + } +} diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/update-volume-req.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/update-volume-req.json.tpl new file mode 100644 index 000000000000..85c1244b1438 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/update-volume-req.json.tpl @@ -0,0 +1,5 @@ +{ + "volumeAttachment": { + "volumeId": "%(new_volume_id)s" + } +} diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/volume-attachment-detail-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/volume-attachment-detail-resp.json.tpl new file mode 100644 index 000000000000..34eacc94d233 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-volumes/v2.85/volume-attachment-detail-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/test_volumes.py b/nova/tests/functional/api_sample_tests/test_volumes.py index 0b5827a0a885..4c48a3e164ee 100644 --- a/nova/tests/functional/api_sample_tests/test_volumes.py +++ b/nova/tests/functional/api_sample_tests/test_volumes.py @@ -287,3 +287,33 @@ class VolumeAttachmentsSampleV279(VolumeAttachmentsSampleV270): """ microversion = '2.79' scenarios = [('v2_79', {'api_major_version': 'v2.1'})] + + +class UpdateVolumeAttachmentsSampleV285(VolumeAttachmentsSampleV279): + """Microversion 2.85 adds the ``PUT + /servers/{server_id}/os-volume_attachments/{volume_id}`` + support for specifying ``delete_on_termination`` field in the request + body to re-config the attached volume whether to delete when the instance + is deleted. + """ + microversion = '2.85' + scenarios = [('v2_85', {'api_major_version': 'v2.1'})] + + def test_volume_attachment_update(self): + subs = self.test_attach_volume_to_server() + attached_volume_id = subs['volume_id'] + subs['server_id'] = self.server_id + response = self._do_put('servers/%s/os-volume_attachments/%s' + % (self.server_id, attached_volume_id), + 'update-volume-attachment-delete-flag-req', + subs) + self.assertEqual(202, response.status_code) + self.assertEqual('', response.text) + + # Make sure the attached volume was changed + attachments = self.api.api_get( + '/servers/%s/os-volume_attachments' % self.server_id).body[ + 'volumeAttachments'] + self.assertEqual(1, len(attachments)) + self.assertEqual(self.server_id, attachments[0]['serverId']) + self.assertTrue(attachments[0]['delete_on_termination']) diff --git a/nova/tests/unit/api/openstack/compute/test_volumes.py b/nova/tests/unit/api/openstack/compute/test_volumes.py index c8f5a0beec95..0b04f23f96c5 100644 --- a/nova/tests/unit/api/openstack/compute/test_volumes.py +++ b/nova/tests/unit/api/openstack/compute/test_volumes.py @@ -39,6 +39,7 @@ import nova.conf from nova import context from nova import exception from nova import objects +from nova.objects import block_device as block_device_obj from nova import test from nova.tests.unit.api.openstack import fakes from nova.tests.unit import fake_block_device @@ -731,8 +732,9 @@ class VolumeAttachTestsV21(test.NoDBTestCase): side_effect=exception.InstanceIsLocked( instance_uuid=uuids.instance)) def test_swap_volume_for_locked_server(self, mock_swap_volume): - self.assertRaises(webob.exc.HTTPConflict, self._test_swap, - self.attachments) + with mock.patch.object(self.attachments, '_update_volume_regular'): + self.assertRaises(webob.exc.HTTPConflict, self._test_swap, + self.attachments) mock_swap_volume.assert_called_once_with( self.req.environ['nova.context'], test.MatchType(objects.Instance), {'attach_status': 'attached', @@ -771,8 +773,9 @@ class VolumeAttachTestsV21(test.NoDBTestCase): mock_get.side_effect = [ None, exception.VolumeNotFound(volume_id=FAKE_UUID_C)] body = {'volumeAttachment': {'volumeId': FAKE_UUID_C}} - self.assertRaises(exc.HTTPBadRequest, self._test_swap, - self.attachments, body=body) + with mock.patch.object(self.attachments, '_update_volume_regular'): + self.assertRaises(exc.HTTPBadRequest, self._test_swap, + self.attachments, body=body) mock_get.assert_has_calls([ mock.call(self.req.environ['nova.context'], FAKE_UUID_A), mock.call(self.req.environ['nova.context'], FAKE_UUID_C)]) @@ -796,17 +799,30 @@ class VolumeAttachTestsV21(test.NoDBTestCase): @mock.patch.object(compute_api.API, 'swap_volume', side_effect=exception.VolumeBDMNotFound( volume_id=FAKE_UUID_B)) - def test_swap_volume_for_bdm_not_found(self, mock_swap_volume): + @mock.patch.object(objects.BlockDeviceMapping, + 'get_by_volume_and_instance', + side_effect=exception.VolumeBDMNotFound( + volume_id=FAKE_UUID_A)) + def test_swap_volume_for_bdm_not_found(self, mock_bdm, mock_swap_volume): self.assertRaises(webob.exc.HTTPNotFound, self._test_swap, self.attachments) - mock_swap_volume.assert_called_once_with( - self.req.environ['nova.context'], test.MatchType(objects.Instance), - {'attach_status': 'attached', - 'status': 'in-use', - 'id': FAKE_UUID_A}, - {'attach_status': 'detached', - 'status': 'available', - 'id': FAKE_UUID_B}) + if mock_bdm.called: + # New path includes regular PUT procedure + mock_bdm.assert_called_once_with(self.req.environ['nova.context'], + FAKE_UUID_A, uuids.instance) + mock_swap_volume.assert_not_called() + else: + # Old path is pure swap-volume + mock_bdm.assert_not_called() + mock_swap_volume.assert_called_once_with( + self.req.environ['nova.context'], + test.MatchType(objects.Instance), + {'attach_status': 'attached', + 'status': 'in-use', + 'id': FAKE_UUID_A}, + {'attach_status': 'detached', + 'status': 'available', + 'id': FAKE_UUID_B}) def _test_list_with_invalid_filter(self, url): req = self._build_request(url) @@ -1149,6 +1165,296 @@ class VolumeAttachTestsV279(VolumeAttachTestsV2_75): self.assertNotIn('delete_on_termination', result['volumeAttachments']) +class UpdateVolumeAttachTests(VolumeAttachTestsV279): + microversion = '2.85' + + @mock.patch.object(objects.BlockDeviceMapping, + 'get_by_volume_and_instance') + @mock.patch.object(block_device_obj.BlockDeviceMapping, 'save') + def test_swap_volume(self, mock_save_bdm, mock_get_bdm): + vol_bdm = objects.BlockDeviceMapping( + self.context, + id=1, + instance_uuid=FAKE_UUID, + volume_id=FAKE_UUID_A, + source_type='volume', + destination_type='volume', + delete_on_termination=False, + connection_info=None, + tag='fake-tag', + device_name='/dev/fake0', + attachment_id=uuids.attachment_id) + mock_get_bdm.return_value = vol_bdm + # On the newer microversion, this test will try to look up the + # BDM to check for update of other fields. + super(UpdateVolumeAttachTests, self).test_swap_volume() + + def test_swap_volume_with_extra_arg(self): + # NOTE(danms): Override this from parent because now device + # is checked for unchanged-ness. + body = {'volumeAttachment': {'volumeId': FAKE_UUID_A, + 'device': '/dev/fake0', + 'notathing': 'foo'}} + + self.assertRaises(self.validation_error, + self._test_swap, + self.attachments, + body=body) + + @mock.patch.object(compute_api.API, 'swap_volume') + @mock.patch.object(objects.BlockDeviceMapping, + 'get_by_volume_and_instance') + @mock.patch.object(block_device_obj.BlockDeviceMapping, 'save') + def test_update_volume(self, mock_bdm_save, + mock_get_vol_and_inst, mock_swap): + vol_bdm = objects.BlockDeviceMapping( + self.context, + id=1, + instance_uuid=FAKE_UUID, + volume_id=FAKE_UUID_A, + source_type='volume', + destination_type='volume', + delete_on_termination=False, + connection_info=None, + tag='fake-tag', + device_name='/dev/fake0', + attachment_id=uuids.attachment_id) + mock_get_vol_and_inst.return_value = vol_bdm + + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_A, + 'tag': 'fake-tag', + 'delete_on_termination': True, + 'device': '/dev/fake0', + }} + self.attachments.update(self.req, FAKE_UUID, + FAKE_UUID_A, body=body) + mock_swap.assert_not_called() + mock_bdm_save.assert_called_once() + self.assertTrue(vol_bdm['delete_on_termination']) + + @mock.patch.object(compute_api.API, 'swap_volume') + @mock.patch.object(objects.BlockDeviceMapping, + 'get_by_volume_and_instance') + @mock.patch.object(block_device_obj.BlockDeviceMapping, 'save') + def test_update_volume_swap(self, mock_bdm_save, + mock_get_vol_and_inst, mock_swap): + vol_bdm = objects.BlockDeviceMapping( + self.context, + id=1, + instance_uuid=FAKE_UUID, + volume_id=FAKE_UUID_A, + source_type='volume', + destination_type='volume', + delete_on_termination=False, + connection_info=None, + tag='fake-tag', + device_name='/dev/fake0', + attachment_id=uuids.attachment_id) + mock_get_vol_and_inst.return_value = vol_bdm + + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_B, + 'tag': 'fake-tag', + 'delete_on_termination': True, + }} + self.attachments.update(self.req, FAKE_UUID, + FAKE_UUID_A, body=body) + mock_bdm_save.assert_called_once() + self.assertTrue(vol_bdm['delete_on_termination']) + # Swap volume is tested elsewhere, just make sure that we did + # attempt to call it in addition to updating the BDM + self.assertTrue(mock_swap.called) + + @mock.patch.object(compute_api.API, 'swap_volume') + @mock.patch.object(objects.BlockDeviceMapping, + 'get_by_volume_and_instance') + @mock.patch.object(block_device_obj.BlockDeviceMapping, 'save') + def test_update_volume_swap_only_old_microversion( + self, mock_bdm_save, mock_get_vol_and_inst, mock_swap): + vol_bdm = objects.BlockDeviceMapping( + self.context, + id=1, + instance_uuid=FAKE_UUID, + volume_id=FAKE_UUID_A, + source_type='volume', + destination_type='volume', + delete_on_termination=False, + connection_info=None, + tag='fake-tag', + device_name='/dev/fake0', + attachment_id=uuids.attachment_id) + mock_get_vol_and_inst.return_value = vol_bdm + + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_B, + }} + req = self._get_req(body, microversion='2.84') + self.attachments.update(req, FAKE_UUID, + FAKE_UUID_A, body=body) + mock_swap.assert_called_once() + mock_bdm_save.assert_not_called() + + @mock.patch.object(objects.BlockDeviceMapping, + 'get_by_volume_and_instance', + side_effect=exception.VolumeBDMNotFound( + volume_id=FAKE_UUID_A)) + def test_update_volume_with_invalid_volume_id(self, mock_mr): + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_A, + 'delete_on_termination': True, + }} + self.assertRaises(exc.HTTPNotFound, + self.attachments.update, + self.req, FAKE_UUID, + FAKE_UUID_A, body=body) + + @mock.patch.object(objects.BlockDeviceMapping, + 'get_by_volume_and_instance') + def test_update_volume_with_changed_attachment_id(self, + mock_get_vol_and_inst): + vol_bdm = objects.BlockDeviceMapping( + self.context, + id=1, + instance_uuid=FAKE_UUID, + volume_id=FAKE_UUID_A, + source_type='volume', + destination_type='volume', + delete_on_termination=False, + connection_info=None, + tag='fake-tag', + device_name='/dev/fake0', + attachment_id=uuids.attachment_id) + mock_get_vol_and_inst.return_value = vol_bdm + + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_A, + 'id': uuids.attachment_id2, + }} + self.assertRaises(exc.HTTPBadRequest, + self.attachments.update, + self.req, FAKE_UUID, + FAKE_UUID_A, body=body) + + @mock.patch.object(objects.BlockDeviceMapping, + 'get_by_volume_and_instance') + def test_update_volume_with_changed_serverId(self, + mock_get_vol_and_inst): + vol_bdm = objects.BlockDeviceMapping( + self.context, + id=1, + instance_uuid=FAKE_UUID, + volume_id=FAKE_UUID_A, + source_type='volume', + destination_type='volume', + delete_on_termination=False, + connection_info=None, + tag='fake-tag', + device_name='/dev/fake0', + attachment_id=uuids.attachment_id) + mock_get_vol_and_inst.return_value = vol_bdm + + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_A, + 'serverId': uuids.server_id, + }} + self.assertRaises(exc.HTTPBadRequest, + self.attachments.update, + self.req, FAKE_UUID, + FAKE_UUID_A, body=body) + + @mock.patch.object(objects.BlockDeviceMapping, + 'get_by_volume_and_instance') + def test_update_volume_with_changed_device(self, mock_get_vol_and_inst): + vol_bdm = objects.BlockDeviceMapping( + self.context, + id=1, + instance_uuid=FAKE_UUID, + volume_id=FAKE_UUID_A, + source_type='volume', + destination_type='volume', + delete_on_termination=False, + connection_info=None, + tag='fake-tag', + device_name='/dev/fake0', + attachment_id=uuids.attachment_id) + mock_get_vol_and_inst.return_value = vol_bdm + + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_A, + 'device': '/dev/sdz', + }} + self.assertRaises(exc.HTTPBadRequest, + self.attachments.update, + self.req, FAKE_UUID, + FAKE_UUID_A, body=body) + + def test_update_volume_with_device_name_old_microversion(self): + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_A, + 'device': '/dev/fake0', + }} + req = self._get_req(body, microversion='2.84') + ex = self.assertRaises(exception.ValidationError, + self.attachments.update, + req, FAKE_UUID, + FAKE_UUID_A, body=body) + self.assertIn('Additional properties are not allowed', + six.text_type(ex)) + + @mock.patch.object(objects.BlockDeviceMapping, + 'get_by_volume_and_instance') + def test_update_volume_with_changed_tag(self, mock_get_vol_and_inst): + vol_bdm = objects.BlockDeviceMapping( + self.context, + id=1, + instance_uuid=FAKE_UUID, + volume_id=FAKE_UUID_A, + source_type='volume', + destination_type='volume', + delete_on_termination=False, + connection_info=None, + tag='fake-tag', + device_name='/dev/fake0', + attachment_id=uuids.attachment_id) + mock_get_vol_and_inst.return_value = vol_bdm + + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_A, + 'tag': 'icanhaznewtag', + }} + self.assertRaises(exc.HTTPBadRequest, + self.attachments.update, + self.req, FAKE_UUID, + FAKE_UUID_A, body=body) + + def test_update_volume_with_tag_old_microversion(self): + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_A, + 'tag': 'fake-tag', + }} + req = self._get_req(body, microversion='2.84') + ex = self.assertRaises(exception.ValidationError, + self.attachments.update, + req, FAKE_UUID, + FAKE_UUID_A, body=body) + self.assertIn('Additional properties are not allowed', + six.text_type(ex)) + + def test_update_volume_with_delete_flag_old_microversion(self): + body = {'volumeAttachment': { + 'volumeId': FAKE_UUID_A, + 'delete_on_termination': True, + }} + req = self._get_req(body, microversion='2.84') + ex = self.assertRaises(exception.ValidationError, + self.attachments.update, + req, FAKE_UUID, + FAKE_UUID_A, body=body) + self.assertIn('Additional properties are not allowed', + six.text_type(ex)) + + class SwapVolumeMultiattachTestCase(test.NoDBTestCase): @mock.patch('nova.api.openstack.common.get_instance') diff --git a/releasenotes/notes/bp-destroy-instance-with-datavolume-4c71b12e005832b0.yaml b/releasenotes/notes/bp-destroy-instance-with-datavolume-4c71b12e005832b0.yaml new file mode 100644 index 000000000000..8abb0210d236 --- /dev/null +++ b/releasenotes/notes/bp-destroy-instance-with-datavolume-4c71b12e005832b0.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + With microversion 2.85 add new API + ``PUT /servers/{server_id}/os-volume_attachments/{volume_id}`` which + support for specifying ``delete_on_termination`` field in the request + body to re-config the attached volume whether to delete when the instance + is deleted.