Skip to content

Commit 709eac7

Browse files
author
Dean Troyer
committed
Fix volume transfers request commands
* Fix volume transfer request accept to actually not crash when trying to call Volume API. * Fix volume transfer request accept syntax to have only one positional argument, which is the ID of the resource in the command * Change the output column order in volume transfer request list to have ID followed by Name then the remaining columns. Closes-bug: 1633582 Change-Id: I5cc005f039d171cc70859f60e7fe649b09ead229
1 parent c3fee25 commit 709eac7

9 files changed

Lines changed: 261 additions & 74 deletions

File tree

‎doc/source/backwards-incompatible.rst‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,17 @@ this backwards incompatible change handling.
1616
Backwards Incompatible Changes
1717
==============================
1818

19+
.. Carry this section as comments until 4.0 release
20+
.. Release 4.0
21+
.. -----------
22+
23+
.. 1. Change ``volume transfer request accept`` to use new option ``--auth-key``
24+
.. rather than a second positional argument.
25+
26+
.. * As of: 4.0
27+
.. * Remove in: <5.0>
28+
.. * Commit: <tbd>
29+
1930
Release 3.0
2031
-----------
2132

‎doc/source/command-objects/volume-transfer-request.rst‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,19 @@ Accept volume transfer request
1313
.. code:: bash
1414
1515
openstack volume transfer request accept
16-
<transfer-request>
17-
<auth-key>
16+
--auth-key <key>
17+
<transfer-request-id>
1818
19-
.. _volume_transfer_request_accept:
20-
.. describe:: <transfer-request>
19+
.. option:: --auth-key <key>
2120

22-
Volume transfer request to accept (name or ID)
21+
Volume transfer request authentication key
22+
23+
.. _volume_transfer_request_accept:
24+
.. describe:: <transfer-request-id>
2325

24-
.. describe:: <auth-key>
26+
Volume transfer request to accept (ID only)
2527

26-
Authentication key of transfer request
28+
Non-admin users are only able to specify the transfer request by ID.
2729

2830
volume transfer request create
2931
------------------------------
@@ -65,7 +67,7 @@ Delete volume transfer request(s)
6567
volume transfer request list
6668
----------------------------
6769

68-
Lists all volume transfer requests.
70+
Lists all volume transfer requests
6971

7072
.. program:: volume transfer request list
7173
.. code:: bash
@@ -75,8 +77,7 @@ Lists all volume transfer requests.
7577
7678
.. option:: --all-projects
7779

78-
Shows detail for all projects. Admin only.
79-
(defaults to False)
80+
Include all projects (admin only)
8081

8182
volume transfer request show
8283
----------------------------

‎openstackclient/tests/functional/volume/v1/test_transfer_request.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
# License for the specific language governing permissions and limitations
1111
# under the License.
1212

13+
import json
1314
import uuid
1415

1516
from openstackclient.tests.functional.volume.v1 import common
@@ -67,11 +68,12 @@ def test_volume_transfer_request_accept(self):
6768
self.assertNotEqual('', auth_key)
6869

6970
# accept the volume transfer request
70-
opts = self.get_opts(self.FIELDS)
71-
raw_output = self.openstack(
72-
'volume transfer request accept ' + name +
73-
' ' + auth_key + opts)
74-
self.assertEqual(name + '\n', raw_output)
71+
json_output = json.loads(self.openstack(
72+
'volume transfer request accept -f json ' +
73+
name + ' ' +
74+
'--auth-key ' + auth_key
75+
))
76+
self.assertEqual(name, json_output.get('name'))
7577

7678
# the volume transfer will be removed by default after accepted
7779
# so just need to delete the volume here

‎openstackclient/tests/functional/volume/v2/test_transfer_request.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
# License for the specific language governing permissions and limitations
1111
# under the License.
1212

13+
import json
1314
import uuid
1415

1516
from openstackclient.tests.functional.volume.v2 import common
@@ -67,11 +68,12 @@ def test_volume_transfer_request_accept(self):
6768
self.assertNotEqual('', auth_key)
6869

6970
# accept the volume transfer request
70-
opts = self.get_opts(self.FIELDS)
71-
raw_output = self.openstack(
72-
'volume transfer request accept ' + name +
73-
' ' + auth_key + opts)
74-
self.assertEqual(name + '\n', raw_output)
71+
json_output = json.loads(self.openstack(
72+
'volume transfer request accept -f json ' +
73+
name + ' ' +
74+
'--auth-key ' + auth_key
75+
))
76+
self.assertEqual(name, json_output.get('name'))
7577

7678
# the volume transfer will be removed by default after accepted
7779
# so just need to delete the volume here

‎openstackclient/tests/unit/volume/v1/test_transfer_request.py‎

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,24 +64,62 @@ def setUp(self):
6464

6565
def test_transfer_accept(self):
6666
arglist = [
67+
'--auth-key', 'key_value',
6768
self.volume_transfer.id,
68-
'auth_key',
6969
]
7070
verifylist = [
7171
('transfer_request', self.volume_transfer.id),
72-
('auth_key', 'auth_key'),
72+
('auth_key', 'key_value'),
7373
]
7474
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
7575

7676
columns, data = self.cmd.take_action(parsed_args)
7777

7878
self.transfer_mock.get.assert_called_once_with(
79-
self.volume_transfer.id)
79+
self.volume_transfer.id,
80+
)
8081
self.transfer_mock.accept.assert_called_once_with(
81-
self.volume_transfer.id, 'auth_key')
82+
self.volume_transfer.id,
83+
'key_value',
84+
)
8285
self.assertEqual(self.columns, columns)
8386
self.assertEqual(self.data, data)
8487

88+
def test_transfer_accept_deprecated(self):
89+
arglist = [
90+
self.volume_transfer.id,
91+
'key_value',
92+
]
93+
verifylist = [
94+
('transfer_request', self.volume_transfer.id),
95+
('old_auth_key', 'key_value'),
96+
]
97+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
98+
99+
columns, data = self.cmd.take_action(parsed_args)
100+
101+
self.transfer_mock.accept.assert_called_once_with(
102+
self.volume_transfer.id,
103+
'key_value',
104+
)
105+
self.assertEqual(self.columns, columns)
106+
self.assertEqual(self.data, data)
107+
108+
def test_transfer_accept_no_option(self):
109+
arglist = [
110+
self.volume_transfer.id,
111+
]
112+
verifylist = [
113+
('transfer_request', self.volume_transfer.id),
114+
]
115+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
116+
117+
self.assertRaises(
118+
exceptions.CommandError,
119+
self.cmd.take_action,
120+
parsed_args,
121+
)
122+
85123

86124
class TestTransferCreate(TestTransfer):
87125

@@ -219,7 +257,7 @@ def test_delete_multiple_transfers_with_exception(self):
219257
self.fail('CommandError should be raised.')
220258
except exceptions.CommandError as e:
221259
self.assertEqual('1 of 2 volume transfer requests failed '
222-
'to delete.', str(e))
260+
'to delete', str(e))
223261

224262
find_mock.assert_any_call(
225263
self.transfer_mock, self.volume_transfers[0].id)
@@ -256,17 +294,17 @@ def test_transfer_list_without_argument(self):
256294

257295
expected_columns = [
258296
'ID',
259-
'Volume',
260297
'Name',
298+
'Volume',
261299
]
262300

263301
# confirming if all expected columns are present in the result.
264302
self.assertEqual(expected_columns, columns)
265303

266304
datalist = ((
267305
self.volume_transfers.id,
268-
self.volume_transfers.volume_id,
269306
self.volume_transfers.name,
307+
self.volume_transfers.volume_id,
270308
), )
271309

272310
# confirming if all expected values are present in the result.
@@ -295,17 +333,17 @@ def test_transfer_list_with_argument(self):
295333

296334
expected_columns = [
297335
'ID',
298-
'Volume',
299336
'Name',
337+
'Volume',
300338
]
301339

302340
# confirming if all expected columns are present in the result.
303341
self.assertEqual(expected_columns, columns)
304342

305343
datalist = ((
306344
self.volume_transfers.id,
307-
self.volume_transfers.volume_id,
308345
self.volume_transfers.name,
346+
self.volume_transfers.volume_id,
309347
), )
310348

311349
# confirming if all expected values are present in the result.

‎openstackclient/tests/unit/volume/v2/test_transfer_request.py‎

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,24 +64,62 @@ def setUp(self):
6464

6565
def test_transfer_accept(self):
6666
arglist = [
67+
'--auth-key', 'key_value',
6768
self.volume_transfer.id,
68-
'auth_key',
6969
]
7070
verifylist = [
7171
('transfer_request', self.volume_transfer.id),
72-
('auth_key', 'auth_key'),
72+
('auth_key', 'key_value'),
7373
]
7474
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
7575

7676
columns, data = self.cmd.take_action(parsed_args)
7777

7878
self.transfer_mock.get.assert_called_once_with(
79-
self.volume_transfer.id)
79+
self.volume_transfer.id,
80+
)
8081
self.transfer_mock.accept.assert_called_once_with(
81-
self.volume_transfer.id, 'auth_key')
82+
self.volume_transfer.id,
83+
'key_value',
84+
)
8285
self.assertEqual(self.columns, columns)
8386
self.assertEqual(self.data, data)
8487

88+
def test_transfer_accept_deprecated(self):
89+
arglist = [
90+
self.volume_transfer.id,
91+
'key_value',
92+
]
93+
verifylist = [
94+
('transfer_request', self.volume_transfer.id),
95+
('old_auth_key', 'key_value'),
96+
]
97+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
98+
99+
columns, data = self.cmd.take_action(parsed_args)
100+
101+
self.transfer_mock.accept.assert_called_once_with(
102+
self.volume_transfer.id,
103+
'key_value',
104+
)
105+
self.assertEqual(self.columns, columns)
106+
self.assertEqual(self.data, data)
107+
108+
def test_transfer_accept_no_option(self):
109+
arglist = [
110+
self.volume_transfer.id,
111+
]
112+
verifylist = [
113+
('transfer_request', self.volume_transfer.id),
114+
]
115+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
116+
117+
self.assertRaises(
118+
exceptions.CommandError,
119+
self.cmd.take_action,
120+
parsed_args,
121+
)
122+
85123

86124
class TestTransferCreate(TestTransfer):
87125

@@ -219,7 +257,7 @@ def test_delete_multiple_transfers_with_exception(self):
219257
self.fail('CommandError should be raised.')
220258
except exceptions.CommandError as e:
221259
self.assertEqual('1 of 2 volume transfer requests failed '
222-
'to delete.', str(e))
260+
'to delete', str(e))
223261

224262
find_mock.assert_any_call(
225263
self.transfer_mock, self.volume_transfers[0].id)
@@ -256,17 +294,17 @@ def test_transfer_list_without_argument(self):
256294

257295
expected_columns = [
258296
'ID',
259-
'Volume',
260297
'Name',
298+
'Volume',
261299
]
262300

263301
# confirming if all expected columns are present in the result.
264302
self.assertEqual(expected_columns, columns)
265303

266304
datalist = ((
267305
self.volume_transfers.id,
268-
self.volume_transfers.volume_id,
269306
self.volume_transfers.name,
307+
self.volume_transfers.volume_id,
270308
), )
271309

272310
# confirming if all expected values are present in the result.
@@ -295,17 +333,17 @@ def test_transfer_list_with_argument(self):
295333

296334
expected_columns = [
297335
'ID',
298-
'Volume',
299336
'Name',
337+
'Volume',
300338
]
301339

302340
# confirming if all expected columns are present in the result.
303341
self.assertEqual(expected_columns, columns)
304342

305343
datalist = ((
306344
self.volume_transfers.id,
307-
self.volume_transfers.volume_id,
308345
self.volume_transfers.name,
346+
self.volume_transfers.volume_id,
309347
), )
310348

311349
# confirming if all expected values are present in the result.

0 commit comments

Comments
 (0)