From c0efaa1d46b762693f8fe3a09d0359ead3e097c4 Mon Sep 17 00:00:00 2001 From: wanghao Date: Mon, 15 Jan 2018 16:46:13 +0800 Subject: [PATCH] Transfer snapshots with volumes This feature changes Cinder to transfer snapshots with volumes at the same time by default. If user doesn't want to transfer snapshots, they could use a new optional argument '--no-snapshots' after microversion 3.55. And we also introduce the new V3 api 'v3/volume_transfers' to move this API out of contrib into Cinder V3 API. The cinderclient patch: https://review.openstack.org/#/c/577611/ Change-Id: If848d131e5edcdb77d0b3c2ca45a99c4d5e14d1e Implements: blueprint transfer-snps-with-vols --- api-ref/source/v3/parameters.yaml | 7 + .../volume-transfer-create-request.json | 3 +- api-ref/source/v3/vol-transfer-v3.inc | 268 ++++++++++++++++ cinder/api/contrib/volume_transfer.py | 3 +- cinder/api/microversions.py | 2 + cinder/api/openstack/api_version_request.py | 3 +- .../openstack/rest_api_version_history.rst | 4 + cinder/api/schemas/volume_transfer.py | 24 +- cinder/api/v3/router.py | 8 + cinder/api/v3/volume_transfer.py | 66 ++++ cinder/api/views/transfers.py | 15 +- cinder/db/api.py | 6 +- cinder/db/sqlalchemy/api.py | 64 +++- .../versions/123_add_transfer_no_snapshots.py | 21 ++ cinder/db/sqlalchemy/models.py | 1 + cinder/policies/volume_transfer.py | 24 ++ .../tests/unit/api/v3/test_volume_transfer.py | 292 ++++++++++++++++++ cinder/tests/unit/db/test_migrations.py | 4 + cinder/tests/unit/db/test_transfers.py | 36 +++ cinder/tests/unit/test_volume_transfer.py | 55 ++++ cinder/tests/unit/volume/test_rpcapi.py | 4 +- cinder/transfer/api.py | 80 ++++- cinder/volume/api.py | 6 +- cinder/volume/manager.py | 3 +- cinder/volume/rpcapi.py | 18 +- doc/source/cli/cli-manage-volumes.rst | 19 +- ...napshots-with-volume-a7763570a807c742.yaml | 6 + 27 files changed, 1014 insertions(+), 28 deletions(-) create mode 100644 api-ref/source/v3/vol-transfer-v3.inc create mode 100644 cinder/api/v3/volume_transfer.py create mode 100644 cinder/db/sqlalchemy/migrate_repo/versions/123_add_transfer_no_snapshots.py create mode 100644 cinder/tests/unit/api/v3/test_volume_transfer.py create mode 100644 releasenotes/notes/transfer-snapshots-with-volume-a7763570a807c742.yaml diff --git a/api-ref/source/v3/parameters.yaml b/api-ref/source/v3/parameters.yaml index a24b991b56c..40e26ca2d2d 100644 --- a/api-ref/source/v3/parameters.yaml +++ b/api-ref/source/v3/parameters.yaml @@ -1915,6 +1915,13 @@ new_type: in: body required: true type: string +no_snapshots: + description: | + Transfer volume without snapshots. + in: body + required: false + min_version: 3.55 + type: boolean object_count: description: | The number of objects in the backup. diff --git a/api-ref/source/v3/samples/volume-transfer-create-request.json b/api-ref/source/v3/samples/volume-transfer-create-request.json index f517b7498de..e7e29450e10 100644 --- a/api-ref/source/v3/samples/volume-transfer-create-request.json +++ b/api-ref/source/v3/samples/volume-transfer-create-request.json @@ -1,6 +1,7 @@ { "transfer": { "volume_id": "c86b9af4-151d-4ead-b62c-5fb967af0e37", - "name": "first volume" + "name": "first volume", + "no_snapshots": False, } } diff --git a/api-ref/source/v3/vol-transfer-v3.inc b/api-ref/source/v3/vol-transfer-v3.inc new file mode 100644 index 00000000000..db3ccb4237a --- /dev/null +++ b/api-ref/source/v3/vol-transfer-v3.inc @@ -0,0 +1,268 @@ +.. -*- rst -*- + +Volume transfer +=============== + +Transfers a volume from one user to another user. +This is the new transfer APIs with microversion 3.53. + + +Accept a volume transfer +~~~~~~~~~~~~~~~~~~~~~~~~ + +.. rest_method:: POST /v3/{project_id}/volume_transfers/{transfer_id}/accept + +Accepts a volume transfer. + +Response codes +-------------- + +.. rest_status_code:: success ../status.yaml + + - 202 + +.. rest_status_code:: error ../status.yaml + + - 400 + +.. rest_status_code:: error ../status.yaml + + - 413 + +Request +------- + +.. rest_parameters:: parameters.yaml + + - project_id: project_id_path + - transfer_id: transfer_id + - auth_key: auth_key + +Request Example +--------------- + +.. literalinclude:: ./samples/volume-transfer-accept-request.json + :language: javascript + +Response Parameters +------------------- + +.. rest_parameters:: parameters.yaml + + - transfer: transfer + - volume_id: volume_id + - id: id + - links: links + - name: name + +Response Example +---------------- + +.. literalinclude:: ./samples/volume-transfer-accept-response.json + :language: javascript + + +Create a volume transfer +~~~~~~~~~~~~~~~~~~~~~~~~ + +.. rest_method:: POST /v3/{project_id}/volume_transfers + +Creates a volume transfer. + +Response codes +-------------- + +.. rest_status_code:: success ../status.yaml + + - 202 + +.. rest_status_code:: error ../status.yaml + + - 400 + + +Request +------- + +.. rest_parameters:: parameters.yaml + + - project_id: project_id_path + - transfer: transfer + - name: name + - volume_id: volume_id + - no_snapshots: no_snapshots + +Request Example +--------------- + +.. literalinclude:: ./samples/volume-transfer-create-request.json + :language: javascript + + +Response Parameters +------------------- + +.. rest_parameters:: parameters.yaml + + - auth_key: auth_key + - links: links + - created_at: created_at + - volume_id: volume_id + - id: id + - name: name + +Response Example +---------------- + +.. literalinclude:: ./samples/volume-transfer-create-response.json + :language: javascript + + +List volume transfers for a project +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. rest_method:: GET /v3/{project_id}/volume_transfers + +Lists volume transfers. + +Response codes +-------------- + +.. rest_status_code:: success ../status.yaml + + - 200 + + +Request +------- + +.. rest_parameters:: parameters.yaml + + - project_id: project_id_path + - all_tenants: all-tenants + + +Response Parameters +------------------- + +.. rest_parameters:: parameters.yaml + + - volume_id: volume_id + - id: id + - links: links + - name: name + + +Response Example +---------------- + +.. literalinclude:: ./samples/volume-transfers-list-response.json + :language: javascript + + +Show volume transfer detail +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. rest_method:: GET /v3/{project_id}/volume_transfers/{transfer_id} + +Shows details for a volume transfer. + +Response codes +-------------- + +.. rest_status_code:: success ../status.yaml + + - 200 + + +Request +------- + +.. rest_parameters:: parameters.yaml + + - project_id: project_id_path + - transfer_id: transfer_id + + +Response Parameters +------------------- + +.. rest_parameters:: parameters.yaml + + - created_at: created_at + - volume_id: volume_id + - id: id + - links: links + - name: name + + +Response Example +---------------- + +.. literalinclude:: ./samples/volume-transfer-show-response.json + :language: javascript + + +Delete a volume transfer +~~~~~~~~~~~~~~~~~~~~~~~~ + +.. rest_method:: DELETE /v3/{project_id}/volume_transfers/{transfer_id} + +Deletes a volume transfer. + +Response codes +-------------- + +.. rest_status_code:: success ../status.yaml + + - 202 + + +Request +------- + +.. rest_parameters:: parameters.yaml + + - project_id: project_id_path + - transfer_id: transfer_id + + +List volume transfers and details +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. rest_method:: GET /v3/{project_id}/volume_transfers/detail + +Lists volume transfers, with details. + +Response codes +-------------- + +.. rest_status_code:: success ../status.yaml + + - 200 + +Request +------- + +.. rest_parameters:: parameters.yaml + + - project_id: project_id_path + - all_tenants: all-tenants + +Response Parameters +------------------- + +.. rest_parameters:: parameters.yaml + + - transfers: transfers + - created_at: created_at + - volume_id: volume_id + - id: id + - links: links + - name: name + +Response Example +---------------- + +.. literalinclude:: ./samples/volume-transfers-list-detailed-response.json + :language: javascript diff --git a/cinder/api/contrib/volume_transfer.py b/cinder/api/contrib/volume_transfer.py index 0ec1409164d..f795dbe2d52 100644 --- a/cinder/api/contrib/volume_transfer.py +++ b/cinder/api/contrib/volume_transfer.py @@ -93,7 +93,8 @@ class VolumeTransferController(wsgi.Controller): volume_id) try: - new_transfer = self.transfer_api.create(context, volume_id, name) + new_transfer = self.transfer_api.create(context, volume_id, name, + no_snapshots=False) # Not found exception will be handled at the wsgi level except exception.InvalidVolume as error: raise exc.HTTPBadRequest(explanation=error.msg) diff --git a/cinder/api/microversions.py b/cinder/api/microversions.py index 18866472fa7..b1922fa1be0 100644 --- a/cinder/api/microversions.py +++ b/cinder/api/microversions.py @@ -147,6 +147,8 @@ SUPPORT_VOLUME_SCHEMA_CHANGES = '3.53' ATTACHMENT_CREATE_MODE_ARG = '3.54' +TRANSFER_WITH_SNAPSHOTS = '3.55' + def get_mv_header(version): """Gets a formatted HTTP microversion header. diff --git a/cinder/api/openstack/api_version_request.py b/cinder/api/openstack/api_version_request.py index 027b554632a..a72111fb16f 100644 --- a/cinder/api/openstack/api_version_request.py +++ b/cinder/api/openstack/api_version_request.py @@ -126,6 +126,7 @@ REST_API_VERSION_HISTORY = """ parameter in the request body in order to update the volume. Also, additional parameters will not be allowed. * 3.54 - Add ``mode`` argument to attachment-create. + * 3.55 - Support transfer volume with snapshots """ # The minimum and maximum versions of the API supported @@ -133,7 +134,7 @@ REST_API_VERSION_HISTORY = """ # minimum version of the API supported. # Explicitly using /v2 endpoints will still work _MIN_API_VERSION = "3.0" -_MAX_API_VERSION = "3.54" +_MAX_API_VERSION = "3.55" _LEGACY_API_VERSION2 = "2.0" UPDATED = "2018-07-17T00:00:00Z" diff --git a/cinder/api/openstack/rest_api_version_history.rst b/cinder/api/openstack/rest_api_version_history.rst index 74681854b4c..4b27b3da8e0 100644 --- a/cinder/api/openstack/rest_api_version_history.rst +++ b/cinder/api/openstack/rest_api_version_history.rst @@ -437,3 +437,7 @@ volume APIs. 3.54 ---- Add ``mode`` argument to attachment-create. + +3.55 +---- +Support ability to transfer snapshots along with their parent volume. diff --git a/cinder/api/schemas/volume_transfer.py b/cinder/api/schemas/volume_transfer.py index 077c9f9a0c1..a247c429d2a 100644 --- a/cinder/api/schemas/volume_transfer.py +++ b/cinder/api/schemas/volume_transfer.py @@ -30,7 +30,7 @@ create = { 'name': {'oneOf': [{'type': 'string', 'format': "name_skip_leading_trailing_spaces"}, - {'type': 'null'}]} + {'type': 'null'}]}, }, 'required': ['volume_id'], 'additionalProperties': False, @@ -56,3 +56,25 @@ accept = { 'required': ['accept'], 'additionalProperties': False, } + + +create_v355 = { + 'type': 'object', + 'properties': { + 'transfer': { + 'type': 'object', + 'properties': { + 'volume_id': parameter_types.uuid, + 'name': {'oneOf': [{'type': 'string', + 'format': + "name_skip_leading_trailing_spaces"}, + {'type': 'null'}]}, + 'no_snapshots': parameter_types.boolean + }, + 'required': ['volume_id'], + 'additionalProperties': False, + }, + }, + 'required': ['transfer'], + 'additionalProperties': False, +} diff --git a/cinder/api/v3/router.py b/cinder/api/v3/router.py index f45e337688e..edff8c6b560 100644 --- a/cinder/api/v3/router.py +++ b/cinder/api/v3/router.py @@ -38,6 +38,7 @@ from cinder.api.v3 import snapshot_manage from cinder.api.v3 import snapshots from cinder.api.v3 import volume_manage from cinder.api.v3 import volume_metadata +from cinder.api.v3 import volume_transfer from cinder.api.v3 import volumes from cinder.api.v3 import workers from cinder.api import versions @@ -192,3 +193,10 @@ class APIRouter(cinder.api.openstack.APIRouter): ext_mgr) mapper.resource('resource_filter', 'resource_filters', controller=self.resources['resource_filters']) + + self.resources['volume_transfers'] = ( + volume_transfer.create_resource()) + mapper.resource("volume_transfer", "volume_transfers", + controller=self.resources['volume_transfers'], + collection={'detail': 'GET'}, + member={'accept': 'POST'}) diff --git a/cinder/api/v3/volume_transfer.py b/cinder/api/v3/volume_transfer.py new file mode 100644 index 00000000000..38d1311efef --- /dev/null +++ b/cinder/api/v3/volume_transfer.py @@ -0,0 +1,66 @@ +# Copyright 2018 FiberHome Telecommunication Technologies CO.,LTD +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_log import log as logging +from oslo_utils import strutils +from six.moves import http_client +from webob import exc + +from cinder.api.contrib import volume_transfer as volume_transfer_v2 +from cinder.api.openstack import wsgi +from cinder.api.schemas import volume_transfer +from cinder.api import validation +from cinder import exception + +LOG = logging.getLogger(__name__) + + +class VolumeTransferController(volume_transfer_v2.VolumeTransferController): + """The transfer API controller for the OpenStack API V3.""" + + @wsgi.response(http_client.ACCEPTED) + @validation.schema(volume_transfer.create, '3.0', '3.54') + @validation.schema(volume_transfer.create_v355, '3.55') + def create(self, req, body): + """Create a new volume transfer.""" + LOG.debug('Creating new volume transfer %s', body) + + context = req.environ['cinder.context'] + transfer = body['transfer'] + + volume_id = transfer['volume_id'] + + name = transfer.get('name', None) + if name is not None: + name = name.strip() + + no_snapshots = strutils.bool_from_string(transfer.get('no_snapshots', + False)) + + LOG.info("Creating transfer of volume %s", volume_id) + + try: + new_transfer = self.transfer_api.create(context, volume_id, name, + no_snapshots=no_snapshots) + # Not found exception will be handled at the wsgi level + except exception.Invalid as error: + raise exc.HTTPBadRequest(explanation=error.msg) + + transfer = self._view_builder.create(req, + dict(new_transfer)) + return transfer + + +def create_resource(): + return wsgi.Resource(VolumeTransferController()) diff --git a/cinder/api/views/transfers.py b/cinder/api/views/transfers.py index 167dffb7ebd..1e8bfdc9400 100644 --- a/cinder/api/views/transfers.py +++ b/cinder/api/views/transfers.py @@ -14,6 +14,7 @@ # under the License. from cinder.api import common +from cinder.api import microversions as mv class ViewBuilder(common.ViewBuilder): @@ -49,7 +50,7 @@ class ViewBuilder(common.ViewBuilder): def detail(self, request, transfer): """Detailed view of a single transfer.""" - return { + detail_body = { 'transfer': { 'id': transfer.get('id'), 'created_at': transfer.get('created_at'), @@ -58,10 +59,15 @@ class ViewBuilder(common.ViewBuilder): 'links': self._get_links(request, transfer['id']) } } + req_version = request.api_version_request + if req_version.matches(mv.TRANSFER_WITH_SNAPSHOTS): + detail_body['transfer'].update({'no_snapshots': + transfer.get('no_snapshots')}) + return detail_body def create(self, request, transfer): """Detailed view of a single transfer when created.""" - return { + create_body = { 'transfer': { 'id': transfer.get('id'), 'created_at': transfer.get('created_at'), @@ -71,6 +77,11 @@ class ViewBuilder(common.ViewBuilder): 'links': self._get_links(request, transfer['id']) } } + req_version = request.api_version_request + if req_version.matches(mv.TRANSFER_WITH_SNAPSHOTS): + create_body['transfer'].update({'no_snapshots': + transfer.get('no_snapshots')}) + return create_body def _list_view(self, func, request, transfers, origin_transfer_count): """Provide a view for a list of transfers.""" diff --git a/cinder/db/api.py b/cinder/db/api.py index 4d906675b84..f1d2f93d5db 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -1284,9 +1284,11 @@ def transfer_destroy(context, transfer_id): return IMPL.transfer_destroy(context, transfer_id) -def transfer_accept(context, transfer_id, user_id, project_id): +def transfer_accept(context, transfer_id, user_id, project_id, + no_snapshots=False): """Accept a volume transfer.""" - return IMPL.transfer_accept(context, transfer_id, user_id, project_id) + return IMPL.transfer_accept(context, transfer_id, user_id, project_id, + no_snapshots=no_snapshots) ################### diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index ad1493b4a38..74f082c5753 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -5417,7 +5417,8 @@ def transfer_get(context, transfer_id): def _translate_transfers(transfers): - fields = ('id', 'volume_id', 'display_name', 'created_at', 'deleted') + fields = ('id', 'volume_id', 'display_name', 'created_at', 'deleted', + 'no_snapshots') return [{k: transfer[k] for k in fields} for transfer in transfers] @@ -5491,8 +5492,37 @@ def transfer_destroy(context, transfer_id): return updated_values +def _roll_back_transferred_volume_and_snapshots(context, volume_id, + old_user_id, old_project_id, + transffered_snapshots): + expected = {'id': volume_id, 'status': 'available'} + update = {'status': 'awaiting-transfer', + 'user_id': old_user_id, + 'project_id': old_project_id, + 'updated_at': timeutils.utcnow()} + if not conditional_update(context, models.Volume, update, expected): + LOG.warning('Volume: %(volume_id)s is not in the expected available ' + 'status. Rolling it back.', {'volume_id': volume_id}) + return + + for snapshot_id in transffered_snapshots: + LOG.info('Beginning to roll back transferred snapshots: %s', + snapshot_id) + expected = {'id': snapshot_id, + 'status': 'available'} + update = {'user_id': old_user_id, + 'project_id': old_project_id, + 'updated_at': timeutils.utcnow()} + if not conditional_update(context, models.Snapshot, update, expected): + LOG.warning('Snapshot: %(snapshot_id)s is not in the expected ' + 'available state. Rolling it back.', + {'snapshot_id': snapshot_id}) + return + + @require_context -def transfer_accept(context, transfer_id, user_id, project_id): +def transfer_accept(context, transfer_id, user_id, project_id, + no_snapshots=False): session = get_session() with session.begin(): volume_id = _transfer_get(context, transfer_id, session)['volume_id'] @@ -5500,7 +5530,8 @@ def transfer_accept(context, transfer_id, user_id, project_id): 'status': 'awaiting-transfer'} update = {'status': 'available', 'user_id': user_id, - 'project_id': project_id} + 'project_id': project_id, + 'updated_at': timeutils.utcnow()} if not conditional_update(context, models.Volume, update, expected): msg = (_('Transfer %(transfer_id)s: Volume id %(volume_id)s ' 'expected in awaiting-transfer state.') @@ -5508,6 +5539,33 @@ def transfer_accept(context, transfer_id, user_id, project_id): LOG.error(msg) raise exception.InvalidVolume(reason=msg) + # Update snapshots for transfer snapshots with volume. + if not no_snapshots: + snapshots = snapshot_get_all_for_volume(context, volume_id) + transferred_snapshots = [] + for snapshot in snapshots: + LOG.info('Begin to transfer snapshot: %s', snapshot['id']) + old_user_id = snapshot['user_id'] + old_project_id = snapshot['project_id'] + expected = {'id': snapshot['id'], + 'status': 'available'} + update = {'user_id': user_id, + 'project_id': project_id, + 'updated_at': timeutils.utcnow()} + if not conditional_update(context, models.Snapshot, update, + expected): + msg = (_('Transfer %(transfer_id)s: Snapshot ' + '%(snapshot_id)s is not in the expected ' + 'available state.') + % {'transfer_id': transfer_id, + 'snapshot_id': snapshot['id']}) + LOG.warning(msg) + _roll_back_transferred_volume_and_snapshots( + context, volume_id, old_user_id, old_project_id, + transferred_snapshots) + raise exception.InvalidSnapshot(reason=msg) + transferred_snapshots.append(snapshot['id']) + (session.query(models.Transfer) .filter_by(id=transfer_id) .update({'deleted': True, diff --git a/cinder/db/sqlalchemy/migrate_repo/versions/123_add_transfer_no_snapshots.py b/cinder/db/sqlalchemy/migrate_repo/versions/123_add_transfer_no_snapshots.py new file mode 100644 index 00000000000..4cee7fe793b --- /dev/null +++ b/cinder/db/sqlalchemy/migrate_repo/versions/123_add_transfer_no_snapshots.py @@ -0,0 +1,21 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from sqlalchemy import Boolean, Column, MetaData, Table + + +def upgrade(migrate_engine): + """Add the no_snapshots column to the transfers table.""" + meta = MetaData(bind=migrate_engine) + transfers = Table('transfers', meta, autoload=True) + if not hasattr(transfers.c, 'no_snapshots'): + transfers.create_column(Column('no_snapshots', Boolean, default=False)) diff --git a/cinder/db/sqlalchemy/models.py b/cinder/db/sqlalchemy/models.py index f7543e79523..1c83add99bb 100644 --- a/cinder/db/sqlalchemy/models.py +++ b/cinder/db/sqlalchemy/models.py @@ -830,6 +830,7 @@ class Transfer(BASE, CinderBase): salt = Column(String(255)) crypt_hash = Column(String(255)) expires_at = Column(DateTime) + no_snapshots = Column(Boolean, default=False) volume = relationship(Volume, backref="transfer", foreign_keys=volume_id, primaryjoin='and_(' diff --git a/cinder/policies/volume_transfer.py b/cinder/policies/volume_transfer.py index 69971739d29..9975b985109 100644 --- a/cinder/policies/volume_transfer.py +++ b/cinder/policies/volume_transfer.py @@ -38,6 +38,14 @@ volume_transfer_policies = [ { 'method': 'GET', 'path': '/os-volume-transfer/detail' + }, + { + 'method': 'GET', + 'path': '/volume_transfers' + }, + { + 'method': 'GET', + 'path': '/volume_transfers/detail' } ]), policy.DocumentedRuleDefault( @@ -48,6 +56,10 @@ volume_transfer_policies = [ { 'method': 'POST', 'path': '/os-volume-transfer' + }, + { + 'method': 'POST', + 'path': '/volume_transfers' } ]), policy.DocumentedRuleDefault( @@ -58,6 +70,10 @@ volume_transfer_policies = [ { 'method': 'GET', 'path': '/os-volume-transfer/{transfer_id}' + }, + { + 'method': 'GET', + 'path': '/volume_transfers/{transfer_id}' } ]), policy.DocumentedRuleDefault( @@ -68,6 +84,10 @@ volume_transfer_policies = [ { 'method': 'POST', 'path': '/os-volume-transfer/{transfer_id}/accept' + }, + { + 'method': 'POST', + 'path': '/volume_transfers/{transfer_id}/accept' } ]), policy.DocumentedRuleDefault( @@ -78,6 +98,10 @@ volume_transfer_policies = [ { 'method': 'DELETE', 'path': '/os-volume-transfer/{transfer_id}' + }, + { + 'method': 'DELETE', + 'path': '/volume_transfers/{transfer_id}' } ]), ] diff --git a/cinder/tests/unit/api/v3/test_volume_transfer.py b/cinder/tests/unit/api/v3/test_volume_transfer.py new file mode 100644 index 00000000000..c73b37f4da6 --- /dev/null +++ b/cinder/tests/unit/api/v3/test_volume_transfer.py @@ -0,0 +1,292 @@ +# Copyright 2018 FiberHome Telecommunication Technologies CO.,LTD +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Tests for volume transfer code. +""" + +from oslo_serialization import jsonutils +from six.moves import http_client +import webob + +from cinder.api.contrib import volume_transfer +from cinder.api import microversions as mv +from cinder import context +from cinder import db +from cinder.objects import fields +from cinder import test +from cinder.tests.unit.api import fakes +from cinder.tests.unit import fake_constants as fake +import cinder.transfer + + +class VolumeTransferAPITestCase(test.TestCase): + """Test Case for transfers V3 API.""" + + def setUp(self): + super(VolumeTransferAPITestCase, self).setUp() + self.volume_transfer_api = cinder.transfer.API() + self.controller = volume_transfer.VolumeTransferController() + self.user_ctxt = context.RequestContext( + fake.USER_ID, fake.PROJECT_ID, auth_token=True, is_admin=True) + + def _create_transfer(self, volume_id=fake.VOLUME_ID, + display_name='test_transfer'): + """Create a transfer object.""" + return self.volume_transfer_api.create(context.get_admin_context(), + volume_id, + display_name) + + @staticmethod + def _create_volume(display_name='test_volume', + display_description='this is a test volume', + status='available', + size=1, + project_id=fake.PROJECT_ID, + attach_status=fields.VolumeAttachStatus.DETACHED): + """Create a volume object.""" + vol = {} + vol['host'] = 'fake_host' + vol['size'] = size + vol['user_id'] = fake.USER_ID + vol['project_id'] = project_id + vol['status'] = status + vol['display_name'] = display_name + vol['display_description'] = display_description + vol['attach_status'] = attach_status + vol['availability_zone'] = 'fake_zone' + return db.volume_create(context.get_admin_context(), vol)['id'] + + def test_show_transfer(self): + volume_id = self._create_volume(size=5) + transfer = self._create_transfer(volume_id) + req = webob.Request.blank('/v3/%s/volume_transfers/%s' % ( + fake.PROJECT_ID, transfer['id'])) + req.method = 'GET' + req.headers = mv.get_mv_header(mv.TRANSFER_WITH_SNAPSHOTS) + req.headers['Content-Type'] = 'application/json' + res = req.get_response(fakes.wsgi_app( + fake_auth_context=self.user_ctxt)) + res_dict = jsonutils.loads(res.body) + self.assertEqual(http_client.OK, res.status_int) + self.assertEqual('test_transfer', res_dict['transfer']['name']) + self.assertEqual(transfer['id'], res_dict['transfer']['id']) + self.assertEqual(volume_id, res_dict['transfer']['volume_id']) + + db.transfer_destroy(context.get_admin_context(), transfer['id']) + db.volume_destroy(context.get_admin_context(), volume_id) + + def test_list_transfers_json(self): + volume_id_1 = self._create_volume(size=5) + volume_id_2 = self._create_volume(size=5) + transfer1 = self._create_transfer(volume_id_1) + transfer2 = self._create_transfer(volume_id_2) + + req = webob.Request.blank('/v3/%s/volume_transfers' % + fake.PROJECT_ID) + req.method = 'GET' + req.headers = mv.get_mv_header(mv.TRANSFER_WITH_SNAPSHOTS) + req.headers['Content-Type'] = 'application/json' + res = req.get_response(fakes.wsgi_app( + fake_auth_context=self.user_ctxt)) + res_dict = jsonutils.loads(res.body) + + self.assertEqual(http_client.OK, res.status_int) + self.assertEqual(4, len(res_dict['transfers'][0])) + self.assertEqual(transfer1['id'], res_dict['transfers'][0]['id']) + self.assertEqual('test_transfer', res_dict['transfers'][0]['name']) + self.assertEqual(4, len(res_dict['transfers'][1])) + self.assertEqual('test_transfer', res_dict['transfers'][1]['name']) + + db.transfer_destroy(context.get_admin_context(), transfer2['id']) + db.transfer_destroy(context.get_admin_context(), transfer1['id']) + db.volume_destroy(context.get_admin_context(), volume_id_1) + db.volume_destroy(context.get_admin_context(), volume_id_2) + + def test_list_transfers_detail_json(self): + volume_id_1 = self._create_volume(size=5) + volume_id_2 = self._create_volume(size=5) + transfer1 = self._create_transfer(volume_id_1) + transfer2 = self._create_transfer(volume_id_2) + + req = webob.Request.blank('/v3/%s/volume_transfers/detail' % + fake.PROJECT_ID) + req.method = 'GET' + req.headers = mv.get_mv_header(mv.TRANSFER_WITH_SNAPSHOTS) + req.headers['Content-Type'] = 'application/json' + req.headers['Accept'] = 'application/json' + res = req.get_response(fakes.wsgi_app( + fake_auth_context=self.user_ctxt)) + res_dict = jsonutils.loads(res.body) + + self.assertEqual(http_client.OK, res.status_int) + self.assertEqual(6, len(res_dict['transfers'][0])) + self.assertEqual('test_transfer', + res_dict['transfers'][0]['name']) + self.assertEqual(transfer1['id'], res_dict['transfers'][0]['id']) + self.assertEqual(volume_id_1, res_dict['transfers'][0]['volume_id']) + + self.assertEqual(6, len(res_dict['transfers'][1])) + self.assertEqual('test_transfer', + res_dict['transfers'][1]['name']) + self.assertEqual(transfer2['id'], res_dict['transfers'][1]['id']) + self.assertEqual(volume_id_2, res_dict['transfers'][1]['volume_id']) + + db.transfer_destroy(context.get_admin_context(), transfer2['id']) + db.transfer_destroy(context.get_admin_context(), transfer1['id']) + db.volume_destroy(context.get_admin_context(), volume_id_2) + db.volume_destroy(context.get_admin_context(), volume_id_1) + + def test_list_transfers_detail_json_with_no_snapshots(self): + volume_id_1 = self._create_volume(size=5) + volume_id_2 = self._create_volume(size=5) + transfer1 = self._create_transfer(volume_id_1) + transfer2 = self._create_transfer(volume_id_2) + + req = webob.Request.blank('/v3/%s/volume_transfers/detail' % + fake.PROJECT_ID) + req.method = 'GET' + req.headers = mv.get_mv_header(mv.TRANSFER_WITH_SNAPSHOTS) + req.headers['Content-Type'] = 'application/json' + req.headers['Accept'] = 'application/json' + res = req.get_response(fakes.wsgi_app( + fake_auth_context=self.user_ctxt)) + res_dict = jsonutils.loads(res.body) + + self.assertEqual(http_client.OK, res.status_int) + self.assertEqual(6, len(res_dict['transfers'][0])) + self.assertEqual('test_transfer', + res_dict['transfers'][0]['name']) + self.assertEqual(transfer1['id'], res_dict['transfers'][0]['id']) + self.assertEqual(volume_id_1, res_dict['transfers'][0]['volume_id']) + self.assertEqual(False, res_dict['transfers'][0]['no_snapshots']) + + self.assertEqual(6, len(res_dict['transfers'][1])) + self.assertEqual('test_transfer', + res_dict['transfers'][1]['name']) + self.assertEqual(transfer2['id'], res_dict['transfers'][1]['id']) + self.assertEqual(volume_id_2, res_dict['transfers'][1]['volume_id']) + self.assertEqual(False, res_dict['transfers'][1]['no_snapshots']) + + db.transfer_destroy(context.get_admin_context(), transfer2['id']) + db.transfer_destroy(context.get_admin_context(), transfer1['id']) + db.volume_destroy(context.get_admin_context(), volume_id_2) + db.volume_destroy(context.get_admin_context(), volume_id_1) + + def test_create_transfer_json(self): + volume_id = self._create_volume(status='available', size=5) + body = {"transfer": {"name": "transfer1", + "volume_id": volume_id}} + + req = webob.Request.blank('/v3/%s/volume_transfers' % + fake.PROJECT_ID) + req.method = 'POST' + req.headers = mv.get_mv_header(mv.TRANSFER_WITH_SNAPSHOTS) + req.headers['Content-Type'] = 'application/json' + req.body = jsonutils.dump_as_bytes(body) + res = req.get_response(fakes.wsgi_app( + fake_auth_context=self.user_ctxt)) + + res_dict = jsonutils.loads(res.body) + + self.assertEqual(http_client.ACCEPTED, res.status_int) + self.assertIn('id', res_dict['transfer']) + self.assertIn('auth_key', res_dict['transfer']) + self.assertIn('created_at', res_dict['transfer']) + self.assertIn('name', res_dict['transfer']) + self.assertIn('volume_id', res_dict['transfer']) + + db.volume_destroy(context.get_admin_context(), volume_id) + + def test_create_transfer_with_no_snapshots(self): + volume_id = self._create_volume(status='available', size=5) + body = {"transfer": {"name": "transfer1", + "volume_id": volume_id, + 'no_snapshots': True}} + + req = webob.Request.blank('/v3/%s/volume_transfers' % + fake.PROJECT_ID) + req.method = 'POST' + req.headers = mv.get_mv_header(mv.TRANSFER_WITH_SNAPSHOTS) + req.headers['Content-Type'] = 'application/json' + req.body = jsonutils.dump_as_bytes(body) + res = req.get_response(fakes.wsgi_app( + fake_auth_context=self.user_ctxt)) + + res_dict = jsonutils.loads(res.body) + + self.assertEqual(http_client.ACCEPTED, res.status_int) + self.assertIn('id', res_dict['transfer']) + self.assertIn('auth_key', res_dict['transfer']) + self.assertIn('created_at', res_dict['transfer']) + self.assertIn('name', res_dict['transfer']) + self.assertIn('volume_id', res_dict['transfer']) + self.assertIn('no_snapshots', res_dict['transfer']) + + db.volume_destroy(context.get_admin_context(), volume_id) + + def test_delete_transfer_awaiting_transfer(self): + volume_id = self._create_volume() + transfer = self._create_transfer(volume_id) + req = webob.Request.blank('/v3/%s/volume_transfers/%s' % ( + fake.PROJECT_ID, transfer['id'])) + req.method = 'DELETE' + req.headers = mv.get_mv_header(mv.TRANSFER_WITH_SNAPSHOTS) + req.headers['Content-Type'] = 'application/json' + res = req.get_response(fakes.wsgi_app( + fake_auth_context=self.user_ctxt)) + + self.assertEqual(http_client.ACCEPTED, res.status_int) + + # verify transfer has been deleted + req = webob.Request.blank('/v3/%s/volume_transfers/%s' % ( + fake.PROJECT_ID, transfer['id'])) + req.method = 'GET' + req.headers['Content-Type'] = 'application/json' + res = req.get_response(fakes.wsgi_app( + fake_auth_context=self.user_ctxt)) + res_dict = jsonutils.loads(res.body) + + self.assertEqual(http_client.NOT_FOUND, res.status_int) + self.assertEqual(http_client.NOT_FOUND, + res_dict['itemNotFound']['code']) + self.assertEqual('Transfer %s could not be found.' % transfer['id'], + res_dict['itemNotFound']['message']) + self.assertEqual(db.volume_get(context.get_admin_context(), + volume_id)['status'], 'available') + + db.volume_destroy(context.get_admin_context(), volume_id) + + def test_accept_transfer_volume_id_specified_json(self): + volume_id = self._create_volume() + transfer = self._create_transfer(volume_id) + + svc = self.start_service('volume', host='fake_host') + body = {"accept": {"auth_key": transfer['auth_key']}} + req = webob.Request.blank('/v3/%s/volume_transfers/%s/accept' % ( + fake.PROJECT_ID, transfer['id'])) + req.method = 'POST' + req.headers = mv.get_mv_header(mv.TRANSFER_WITH_SNAPSHOTS) + req.headers['Content-Type'] = 'application/json' + req.body = jsonutils.dump_as_bytes(body) + res = req.get_response(fakes.wsgi_app( + fake_auth_context=self.user_ctxt)) + res_dict = jsonutils.loads(res.body) + + self.assertEqual(http_client.ACCEPTED, res.status_int) + self.assertEqual(transfer['id'], res_dict['transfer']['id']) + self.assertEqual(volume_id, res_dict['transfer']['volume_id']) + # cleanup + svc.stop() diff --git a/cinder/tests/unit/db/test_migrations.py b/cinder/tests/unit/db/test_migrations.py index 34482783cfd..c72262d0a21 100644 --- a/cinder/tests/unit/db/test_migrations.py +++ b/cinder/tests/unit/db/test_migrations.py @@ -405,6 +405,10 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin): volume_attachment = db_utils.get_table(engine, 'volume_attachment') self.assertIn('connector', volume_attachment.c) + def _check_123(self, engine, data): + volume_transfer = db_utils.get_table(engine, 'transfers') + self.assertIn('no_snapshots', volume_transfer.c) + def test_walk_versions(self): self.walk_versions(False, False) self.assert_each_foreign_key_is_part_of_an_index() diff --git a/cinder/tests/unit/db/test_transfers.py b/cinder/tests/unit/db/test_transfers.py index 64b811284ce..90e1daafed7 100644 --- a/cinder/tests/unit/db/test_transfers.py +++ b/cinder/tests/unit/db/test_transfers.py @@ -118,3 +118,39 @@ class TransfersTableTestCase(test.TestCase): db.transfer_destroy(nctxt.elevated(), xfer_id2) xfer = db.transfer_get_all(context.get_admin_context()) self.assertEqual(0, len(xfer), "Unexpected number of transfer records") + + def test_transfer_accept_with_snapshots(self): + volume_id = utils.create_volume(self.ctxt)['id'] + snapshot_id1 = utils.create_snapshot(self.ctxt, volume_id, + status='available')['id'] + snapshot_id2 = utils.create_snapshot(self.ctxt, volume_id, + status='available')['id'] + xfer_id = self._create_transfer(volume_id) + nctxt = context.RequestContext(user_id=fake.USER2_ID, + project_id=fake.PROJECT2_ID) + db.transfer_accept(nctxt.elevated(), xfer_id, fake.USER2_ID, + fake.PROJECT2_ID) + self.assertEqual(fake.PROJECT2_ID, + db.snapshot_get(nctxt, snapshot_id1)['project_id']) + self.assertEqual(fake.PROJECT2_ID, + db.snapshot_get(nctxt, snapshot_id2)['project_id']) + + def test_transfer_accept_with_snapshots_invalid_status(self): + volume_id = utils.create_volume(self.ctxt)['id'] + snapshot_id1 = utils.create_snapshot(self.ctxt, volume_id, + status='available')['id'] + snapshot_id2 = utils.create_snapshot(self.ctxt, volume_id)['id'] + xfer_id = self._create_transfer(volume_id) + nctxt = context.RequestContext(user_id=fake.USER2_ID, + project_id=fake.PROJECT2_ID) + self.assertRaises(exception.InvalidSnapshot, db.transfer_accept, + nctxt.elevated(), xfer_id, fake.USER2_ID, + fake.PROJECT2_ID) + self.assertEqual(fake.PROJECT_ID, + db.snapshot_get(self.ctxt, + snapshot_id1)['project_id']) + self.assertEqual(fake.PROJECT_ID, + db.snapshot_get(self.ctxt, + snapshot_id2)['project_id']) + self.assertEqual('awaiting-transfer', + db.volume_get(self.ctxt, volume_id)['status']) diff --git a/cinder/tests/unit/test_volume_transfer.py b/cinder/tests/unit/test_volume_transfer.py index b260cc687d1..977913f77a5 100644 --- a/cinder/tests/unit/test_volume_transfer.py +++ b/cinder/tests/unit/test_volume_transfer.py @@ -306,3 +306,58 @@ class VolumeTransferTestCase(test.TestCase): tx_api.get, self.ctxt, transfer['id']) + + @mock.patch('cinder.volume.utils.notify_about_volume_usage') + def test_transfer_accept_with_snapshots(self, mock_notify): + svc = self.start_service('volume', host='test_host') + self.addCleanup(svc.stop) + tx_api = transfer_api.API() + volume = utils.create_volume(self.ctxt, + volume_type_id=fake.VOLUME_TYPE_ID, + updated_at=self.updated_at) + utils.create_volume_type(self.ctxt.elevated(), + id=fake.VOLUME_TYPE_ID, name="test_type") + utils.create_snapshot(self.ctxt, volume.id, status='available') + transfer = tx_api.create(self.ctxt, volume.id, 'Description') + + # Get volume and snapshot quota before accept + self.ctxt.user_id = fake.USER2_ID + self.ctxt.project_id = fake.PROJECT2_ID + usages = db.quota_usage_get_all_by_project(self.ctxt, + self.ctxt.project_id) + self.assertEqual(0, usages.get('volumes', {}).get('in_use', 0)) + self.assertEqual(0, usages.get('snapshots', {}).get('in_use', 0)) + + tx_api.accept(self.ctxt, transfer['id'], transfer['auth_key']) + volume = objects.Volume.get_by_id(self.ctxt, volume.id) + self.assertEqual(fake.PROJECT2_ID, volume.project_id) + self.assertEqual(fake.USER2_ID, volume.user_id) + + calls = [mock.call(self.ctxt, mock.ANY, "transfer.accept.start"), + mock.call(self.ctxt, mock.ANY, "transfer.accept.end")] + mock_notify.assert_has_calls(calls) + # The notify_about_volume_usage is called twice at create(), + # and twice at accept(). + self.assertEqual(4, mock_notify.call_count) + + # Get volume and snapshot quota after accept + self.ctxt.user_id = fake.USER2_ID + self.ctxt.project_id = fake.PROJECT2_ID + usages = db.quota_usage_get_all_by_project(self.ctxt, + self.ctxt.project_id) + self.assertEqual(1, usages.get('volumes', {}).get('in_use', 0)) + self.assertEqual(1, usages.get('snapshots', {}).get('in_use', 0)) + + @mock.patch('cinder.volume.utils.notify_about_volume_usage') + def test_transfer_accept_with_snapshots_invalid(self, mock_notify): + svc = self.start_service('volume', host='test_host') + self.addCleanup(svc.stop) + tx_api = transfer_api.API() + volume = utils.create_volume(self.ctxt, + volume_type_id=fake.VOLUME_TYPE_ID, + updated_at=self.updated_at) + utils.create_volume_type(self.ctxt.elevated(), + id=fake.VOLUME_TYPE_ID, name="test_type") + utils.create_snapshot(self.ctxt, volume.id, status='deleting') + self.assertRaises(exception.InvalidSnapshot, + tx_api.create, self.ctxt, volume.id, 'Description') diff --git a/cinder/tests/unit/volume/test_rpcapi.py b/cinder/tests/unit/volume/test_rpcapi.py index b9115b2b03e..1f12d9b259f 100644 --- a/cinder/tests/unit/volume/test_rpcapi.py +++ b/cinder/tests/unit/volume/test_rpcapi.py @@ -274,8 +274,10 @@ class VolumeRPCAPITestCase(test.RPCAPITestCase): volume=self.fake_volume_obj, new_user=fake.USER_ID, new_project=fake.PROJECT_ID, + no_snapshots=True, expected_kwargs_diff={ - 'volume_id': self.fake_volume_obj.id}) + 'volume_id': self.fake_volume_obj.id}, + version='3.16') @ddt.data(None, 'mycluster') def test_extend_volume(self, cluster_name): diff --git a/cinder/transfer/api.py b/cinder/transfer/api.py index 213fd406471..91e95b881fa 100644 --- a/cinder/transfer/api.py +++ b/cinder/transfer/api.py @@ -113,7 +113,7 @@ class API(base.Base): auth_key = auth_key.encode('utf-8') return hmac.new(salt, auth_key, hashlib.sha1).hexdigest() - def create(self, context, volume_id, display_name): + def create(self, context, volume_id, display_name, no_snapshots=False): """Creates an entry in the transfers table.""" LOG.info("Generating transfer record for volume %s", volume_id) volume_ref = self.db.volume_get(context, volume_id) @@ -124,6 +124,18 @@ class API(base.Base): raise exception.InvalidVolume( reason=_("transferring encrypted volume is not supported")) + if not no_snapshots: + snapshots = self.db.snapshot_get_all_for_volume(context, volume_id) + for snapshot in snapshots: + if snapshot['status'] != "available": + msg = _("snapshot: %s status must be " + "available") % snapshot['id'] + raise exception.InvalidSnapshot(reason=msg) + if snapshot.get('encryption_key_id'): + msg = _("snapshot: %s encrypted snapshots cannot be " + "transferred") % snapshot['id'] + raise exception.InvalidSnapshot(reason=msg) + volume_utils.notify_about_volume_usage(context, volume_ref, "transfer.create.start") # The salt is just a short random string. @@ -136,7 +148,8 @@ class API(base.Base): 'display_name': display_name, 'salt': salt, 'crypt_hash': crypt_hash, - 'expires_at': None} + 'expires_at': None, + 'no_snapshots': no_snapshots} try: transfer = self.db.transfer_create(context, transfer_rec) @@ -149,7 +162,44 @@ class API(base.Base): 'volume_id': transfer['volume_id'], 'display_name': transfer['display_name'], 'auth_key': auth_key, - 'created_at': transfer['created_at']} + 'created_at': transfer['created_at'], + 'no_snapshots': transfer['no_snapshots']} + + def _handle_snapshot_quota(self, context, snapshots, volume_type_id, + donor_id): + snapshots_num = len(snapshots) + volume_sizes = 0 + if not CONF.no_snapshot_gb_quota: + for snapshot in snapshots: + volume_sizes += snapshot.volume_size + try: + reserve_opts = {'snapshots': snapshots_num, + 'gigabytes': volume_sizes} + QUOTAS.add_volume_type_opts(context, + reserve_opts, + volume_type_id) + reservations = QUOTAS.reserve(context, **reserve_opts) + except exception.OverQuota as e: + quota_utils.process_reserve_over_quota( + context, e, + resource='snapshots', + size=volume_sizes) + + try: + reserve_opts = {'snapshots': -snapshots_num, + 'gigabytes': -volume_sizes} + QUOTAS.add_volume_type_opts(context.elevated(), + reserve_opts, + volume_type_id) + donor_reservations = QUOTAS.reserve(context, + project_id=donor_id, + **reserve_opts) + except exception.OverQuota as e: + donor_reservations = None + LOG.exception("Failed to update volume providing snapshots quota:" + " Over quota.") + + return reservations, donor_reservations def accept(self, context, transfer_id, auth_key): """Accept a volume that has been offered for transfer.""" @@ -206,6 +256,15 @@ class API(base.Base): LOG.exception("Failed to update quota donating volume" " transfer id %s", transfer_id) + snap_res = None + snap_donor_res = None + if transfer['no_snapshots'] is False: + snapshots = objects.SnapshotList.get_all_for_volume( + context.elevated(), volume_id) + volume_type_id = vol_ref.volume_type_id + snap_res, snap_donor_res = self._handle_snapshot_quota( + context, snapshots, volume_type_id, vol_ref['project_id']) + volume_utils.notify_about_volume_usage(context, vol_ref, "transfer.accept.start") try: @@ -214,21 +273,32 @@ class API(base.Base): self.volume_api.accept_transfer(context, vol_ref, context.user_id, - context.project_id) + context.project_id, + transfer['no_snapshots']) self.db.transfer_accept(context.elevated(), transfer_id, context.user_id, - context.project_id) + context.project_id, + transfer['no_snapshots']) QUOTAS.commit(context, reservations) + if snap_res: + QUOTAS.commit(context, snap_res) if donor_reservations: QUOTAS.commit(context, donor_reservations, project_id=donor_id) + if snap_donor_res: + QUOTAS.commit(context, snap_donor_res, project_id=donor_id) LOG.info("Volume %s has been transferred.", volume_id) except Exception: with excutils.save_and_reraise_exception(): QUOTAS.rollback(context, reservations) + if snap_res: + QUOTAS.rollback(context, snap_res) if donor_reservations: QUOTAS.rollback(context, donor_reservations, project_id=donor_id) + if snap_donor_res: + QUOTAS.rollback(context, snap_donor_res, + project_id=donor_id) vol_ref = self.db.volume_get(context, volume_id) volume_utils.notify_about_volume_usage(context, vol_ref, diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 3d01ea8b055..0d1647934fb 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -815,7 +815,8 @@ class API(base.Base): resource=volume) self.unreserve_volume(context, volume) - def accept_transfer(self, context, volume, new_user, new_project): + def accept_transfer(self, context, volume, new_user, new_project, + no_snapshots=False): context.authorize(vol_transfer_policy.ACCEPT_POLICY, target_obj=volume) if volume['status'] == 'maintenance': @@ -826,7 +827,8 @@ class API(base.Base): results = self.volume_rpcapi.accept_transfer(context, volume, new_user, - new_project) + new_project, + no_snapshots=no_snapshots) LOG.info("Transfer volume completed successfully.", resource=volume) return results diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index d1105935f45..2c60733ed9c 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -1863,7 +1863,8 @@ class VolumeManager(manager.CleanableManager, LOG.info("Remove snapshot export completed successfully.", resource=snapshot) - def accept_transfer(self, context, volume_id, new_user, new_project): + def accept_transfer(self, context, volume_id, new_user, new_project, + no_snapshots=False): # NOTE(flaper87): Verify the driver is enabled # before going forward. The exception will be caught # and the volume status updated. diff --git a/cinder/volume/rpcapi.py b/cinder/volume/rpcapi.py index d263e9401fa..36f1afa1834 100644 --- a/cinder/volume/rpcapi.py +++ b/cinder/volume/rpcapi.py @@ -133,9 +133,10 @@ class VolumeAPI(rpc.RPCAPI): 3.14 - Adds enable_replication, disable_replication, failover_replication, and list_replication_targets. 3.15 - Add revert_to_snapshot method + 3.16 - Add no_snapshots to accept_transfer method """ - RPC_API_VERSION = '3.15' + RPC_API_VERSION = '3.16' RPC_DEFAULT_VERSION = '3.0' TOPIC = constants.VOLUME_TOPIC BINARY = constants.VOLUME_BINARY @@ -238,10 +239,17 @@ class VolumeAPI(rpc.RPCAPI): cctxt = self._get_cctxt(fanout=True) cctxt.cast(ctxt, 'publish_service_capabilities') - def accept_transfer(self, ctxt, volume, new_user, new_project): - cctxt = self._get_cctxt(volume.service_topic_queue) - return cctxt.call(ctxt, 'accept_transfer', volume_id=volume['id'], - new_user=new_user, new_project=new_project) + def accept_transfer(self, ctxt, volume, new_user, new_project, + no_snapshots=False): + msg_args = {'volume_id': volume['id'], + 'new_user': new_user, + 'new_project': new_project, + 'no_snapshots': no_snapshots + } + cctxt = self._get_cctxt(volume.service_topic_queue, ('3.16', '3.0')) + if not self.client.can_send_version('3.16'): + msg_args.pop('no_snapshots') + return cctxt.call(ctxt, 'accept_transfer', **msg_args) def extend_volume(self, ctxt, volume, new_size, reservations): cctxt = self._get_cctxt(volume.service_topic_queue) diff --git a/doc/source/cli/cli-manage-volumes.rst b/doc/source/cli/cli-manage-volumes.rst index 2d501c97151..fe766c57599 100644 --- a/doc/source/cli/cli-manage-volumes.rst +++ b/doc/source/cli/cli-manage-volumes.rst @@ -449,6 +449,14 @@ donor, or original owner, creates a transfer request and sends the created transfer ID and authorization key to the volume recipient. The volume recipient, or new owner, accepts the transfer by using the ID and key. +In Rocky, Cinder changes the API behavior for V2 and 3.x < 3.55, snapshots will +be transferred with volume by default. That means if the volume has some +snapshots, when a user transfers a volume from one owner to another, then those +snapshots will be transferred with the volume as well. After microversion 3.55, +Cinder supports the ability to transfer volume without snapshots. If users +don't want to transfer snapshots, they need to specify the new optional +argument `--no_snapshots`. + .. note:: The procedure for volume transfer is intended for projects (both the @@ -484,10 +492,15 @@ Create a volume transfer request .. code-block:: console - $ openstack volume transfer request create + $ openstack volume transfer request create [--no-snapshots] - - Name or ID of volume to transfer. +The arguments to be passed are: + +```` +Name or ID of volume to transfer. + +``--no-snapshots`` +Transfer the volume without snapshots. The volume must be in an ``available`` state or the request will be denied. If the transfer request is valid in the database (that is, it diff --git a/releasenotes/notes/transfer-snapshots-with-volume-a7763570a807c742.yaml b/releasenotes/notes/transfer-snapshots-with-volume-a7763570a807c742.yaml new file mode 100644 index 00000000000..0c723411add --- /dev/null +++ b/releasenotes/notes/transfer-snapshots-with-volume-a7763570a807c742.yaml @@ -0,0 +1,6 @@ +--- +features: + - Support transfer volume with snapshots by default in new V3 API + 'v3/volume_transfers'. After microverison 3.55, if users don't want to + transfer snapshots, they could use the new optional argument + `no_snapshots=True` in request body of new transfer creation API.