From 460229c6099719dec0d027f798f9c751b8ec7e44 Mon Sep 17 00:00:00 2001 From: Sean McGinnis Date: Tue, 31 Jul 2018 13:59:32 -0500 Subject: [PATCH] Allow volume-transfer creation < 3.55 microversion Handling for the new `no_snapshots` option was incorrectly handling microversion evaluation that would prevent anything less than the new microversion from working. This changes the check to only handle the changed argument for 3.55 and later. Change-Id: If96889ccde6044706e6a5dcd83fde3c20fe1c1fd Closes-bug: #1784703 Signed-off-by: Sean McGinnis --- cinderclient/tests/unit/v3/test_shell.py | 7 +++ .../tests/unit/v3/test_volume_transfers.py | 24 ++++---- cinderclient/v3/volume_transfers.py | 56 +------------------ .../volume-transfer-bug-23c760efb9f98a4d.yaml | 8 +++ 4 files changed, 32 insertions(+), 63 deletions(-) create mode 100644 releasenotes/notes/volume-transfer-bug-23c760efb9f98a4d.yaml diff --git a/cinderclient/tests/unit/v3/test_shell.py b/cinderclient/tests/unit/v3/test_shell.py index e06dedc13..c9f776059 100644 --- a/cinderclient/tests/unit/v3/test_shell.py +++ b/cinderclient/tests/unit/v3/test_shell.py @@ -1334,6 +1334,13 @@ class ShellTest(utils.TestCase): self.assert_called('POST', '/workers/cleanup', body=expected) def test_create_transfer(self): + self.run_command('transfer-create 1234') + expected = {'transfer': {'volume_id': 1234, + 'name': None, + }} + self.assert_called('POST', '/volume-transfers', body=expected) + + def test_create_transfer_no_snaps(self): self.run_command('--os-volume-api-version 3.55 transfer-create ' '--no-snapshots 1234') expected = {'transfer': {'volume_id': 1234, diff --git a/cinderclient/tests/unit/v3/test_volume_transfers.py b/cinderclient/tests/unit/v3/test_volume_transfers.py index 4eab0b22b..7fc79afe3 100644 --- a/cinderclient/tests/unit/v3/test_volume_transfers.py +++ b/cinderclient/tests/unit/v3/test_volume_transfers.py @@ -13,25 +13,29 @@ # License for the specific language governing permissions and limitations # under the License. +from cinderclient import api_versions from cinderclient.tests.unit import utils from cinderclient.tests.unit.v3 import fakes +# Create calls need the right version of faked client +v3cs = fakes.FakeClient(api_versions.APIVersion('3.55')) +# Other calls fall back to default behavior cs = fakes.FakeClient() class VolumeTransfersTest(utils.TestCase): def test_create(self): - vol = cs.transfers.create('1234') - cs.assert_called('POST', '/volume-transfers', + vol = v3cs.transfers.create('1234') + v3cs.assert_called('POST', '/volume-transfers', body={'transfer': {'volume_id': '1234', 'name': None, 'no_snapshots': False}}) self._assert_request_id(vol) def test_create_without_snapshots(self): - vol = cs.transfers.create('1234', no_snapshots=True) - cs.assert_called('POST', '/volume-transfers', + vol = v3cs.transfers.create('1234', no_snapshots=True) + v3cs.assert_called('POST', '/volume-transfers', body={'transfer': {'volume_id': '1234', 'name': None, 'no_snapshots': True}}) self._assert_request_id(vol) @@ -39,29 +43,29 @@ class VolumeTransfersTest(utils.TestCase): def test_get(self): transfer_id = '5678' vol = cs.transfers.get(transfer_id) - cs.assert_called('GET', '/volume-transfers/%s' % transfer_id) + cs.assert_called('GET', '/os-volume-transfer/%s' % transfer_id) self._assert_request_id(vol) def test_list(self): lst = cs.transfers.list() - cs.assert_called('GET', '/volume-transfers/detail') + cs.assert_called('GET', '/os-volume-transfer/detail') self._assert_request_id(lst) def test_delete(self): b = cs.transfers.list()[0] vol = b.delete() - cs.assert_called('DELETE', '/volume-transfers/5678') + cs.assert_called('DELETE', '/os-volume-transfer/5678') self._assert_request_id(vol) vol = cs.transfers.delete('5678') self._assert_request_id(vol) - cs.assert_called('DELETE', '/volume-transfers/5678') + cs.assert_called('DELETE', '/os-volume-transfer/5678') vol = cs.transfers.delete(b) - cs.assert_called('DELETE', '/volume-transfers/5678') + cs.assert_called('DELETE', '/os-volume-transfer/5678') self._assert_request_id(vol) def test_accept(self): transfer_id = '5678' auth_key = '12345' vol = cs.transfers.accept(transfer_id, auth_key) - cs.assert_called('POST', '/volume-transfers/%s/accept' % transfer_id) + cs.assert_called('POST', '/os-volume-transfer/%s/accept' % transfer_id) self._assert_request_id(vol) diff --git a/cinderclient/v3/volume_transfers.py b/cinderclient/v3/volume_transfers.py index 4e80d20a5..aa3308256 100644 --- a/cinderclient/v3/volume_transfers.py +++ b/cinderclient/v3/volume_transfers.py @@ -17,17 +17,10 @@ Volume transfer interface (v3 extension). """ -from cinderclient import api_versions -from cinderclient import base -from cinderclient import utils from cinderclient.v2 import volume_transfers -VolumeTransfer = volume_transfers.VolumeTransfer - - class VolumeTransferManager(volume_transfers.VolumeTransferManager): - @api_versions.wraps("3.55") def create(self, volume_id, name=None, no_snapshots=False): """Creates a volume transfer. @@ -37,50 +30,7 @@ class VolumeTransferManager(volume_transfers.VolumeTransferManager): :rtype: :class:`VolumeTransfer` """ body = {'transfer': {'volume_id': volume_id, - 'name': name, - 'no_snapshots': no_snapshots}} + 'name': name}} + if self.api_version.matches('3.55'): + body['transfer']['no_snapshots'] = no_snapshots return self._create('/volume-transfers', body, 'transfer') - - @api_versions.wraps("3.55") - def accept(self, transfer_id, auth_key): - """Accept a volume transfer. - - :param transfer_id: The ID of the transfer to accept. - :param auth_key: The auth_key of the transfer. - :rtype: :class:`VolumeTransfer` - """ - body = {'accept': {'auth_key': auth_key}} - return self._create('/volume-transfers/%s/accept' % transfer_id, - body, 'transfer') - - @api_versions.wraps("3.55") - def get(self, transfer_id): - """Show details of a volume transfer. - - :param transfer_id: The ID of the volume transfer to display. - :rtype: :class:`VolumeTransfer` - """ - return self._get("/volume-transfers/%s" % transfer_id, "transfer") - - @api_versions.wraps("3.55") - def list(self, detailed=True, search_opts=None): - """Get a list of all volume transfer. - - :rtype: list of :class:`VolumeTransfer` - """ - query_string = utils.build_query_param(search_opts) - - detail = "" - if detailed: - detail = "/detail" - - return self._list("/volume-transfers%s%s" % (detail, query_string), - "transfers") - - @api_versions.wraps("3.55") - def delete(self, transfer_id): - """Delete a volume transfer. - - :param transfer_id: The :class:`VolumeTransfer` to delete. - """ - return self._delete("/volume-transfers/%s" % base.getid(transfer_id)) diff --git a/releasenotes/notes/volume-transfer-bug-23c760efb9f98a4d.yaml b/releasenotes/notes/volume-transfer-bug-23c760efb9f98a4d.yaml new file mode 100644 index 000000000..a22dd669c --- /dev/null +++ b/releasenotes/notes/volume-transfer-bug-23c760efb9f98a4d.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + An issue was discovered with the way API microversions were handled for the + new volume-transfer with snapshot handling with microversion 3.55. This + release includes a fix to keep backwards compatibility with earlier + releases. See `bug #1784703 + `_ for more details.