Skip to content

Commit d17ae43

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "Add cluster support in migration and manage"
2 parents 1f61e3c + 8fce740 commit d17ae43

9 files changed

Lines changed: 454 additions & 48 deletions

File tree

‎cinderclient/shell.py‎

Lines changed: 53 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from __future__ import print_function
2121

2222
import argparse
23+
import collections
2324
import getpass
2425
import logging
2526
import sys
@@ -492,6 +493,7 @@ def _find_actions(self, subparsers, actions_module, version,
492493
action_help = desc.strip().split('\n')[0]
493494
action_help += additional_msg
494495

496+
exclusive_args = getattr(callback, 'exclusive_args', {})
495497
arguments = getattr(callback, 'arguments', [])
496498

497499
subparser = subparsers.add_parser(
@@ -506,41 +508,59 @@ def _find_actions(self, subparsers, actions_module, version,
506508
help=argparse.SUPPRESS,)
507509

508510
self.subcommands[command] = subparser
509-
510-
# NOTE(ntpttr): We get a counter for each argument in this
511-
# command here because during the microversion check we only
512-
# want to raise an exception if no version of the argument
513-
# matches the current microversion. The exception will only
514-
# be raised after the last instance of a particular argument
515-
# fails the check.
516-
arg_counter = dict()
517-
for (args, kwargs) in arguments:
518-
arg_counter[args[0]] = arg_counter.get(args[0], 0) + 1
519-
520-
for (args, kwargs) in arguments:
521-
start_version = kwargs.get("start_version", None)
522-
start_version = api_versions.APIVersion(start_version)
523-
end_version = kwargs.get('end_version', None)
524-
end_version = api_versions.APIVersion(end_version)
525-
if do_help and (start_version or end_version):
526-
kwargs["help"] = kwargs.get("help", "") + (
527-
self._build_versioned_help_message(start_version,
528-
end_version))
529-
if not version.matches(start_version, end_version):
530-
if args[0] in input_args and command == input_args[0]:
531-
if arg_counter[args[0]] == 1:
532-
# This is the last version of this argument,
533-
# raise the exception.
534-
raise exc.UnsupportedAttribute(args[0],
535-
start_version, end_version)
536-
arg_counter[args[0]] -= 1
537-
continue
538-
kw = kwargs.copy()
539-
kw.pop("start_version", None)
540-
kw.pop("end_version", None)
541-
subparser.add_argument(*args, **kw)
511+
self._add_subparser_args(subparser, arguments, version, do_help,
512+
input_args, command)
513+
self._add_subparser_exclusive_args(subparser, exclusive_args,
514+
version, do_help, input_args,
515+
command)
542516
subparser.set_defaults(func=callback)
543517

518+
def _add_subparser_args(self, subparser, arguments, version, do_help,
519+
input_args, command):
520+
# NOTE(ntpttr): We get a counter for each argument in this
521+
# command here because during the microversion check we only
522+
# want to raise an exception if no version of the argument
523+
# matches the current microversion. The exception will only
524+
# be raised after the last instance of a particular argument
525+
# fails the check.
526+
arg_counter = collections.defaultdict(int)
527+
for (args, kwargs) in arguments:
528+
arg_counter[args[0]] += 1
529+
530+
for (args, kwargs) in arguments:
531+
start_version = kwargs.get("start_version", None)
532+
start_version = api_versions.APIVersion(start_version)
533+
end_version = kwargs.get('end_version', None)
534+
end_version = api_versions.APIVersion(end_version)
535+
if do_help and (start_version or end_version):
536+
kwargs["help"] = kwargs.get("help", "") + (
537+
self._build_versioned_help_message(start_version,
538+
end_version))
539+
if not version.matches(start_version, end_version):
540+
if args[0] in input_args and command == input_args[0]:
541+
if arg_counter[args[0]] == 1:
542+
# This is the last version of this argument,
543+
# raise the exception.
544+
raise exc.UnsupportedAttribute(args[0],
545+
start_version, end_version)
546+
arg_counter[args[0]] -= 1
547+
continue
548+
kw = kwargs.copy()
549+
kw.pop("start_version", None)
550+
kw.pop("end_version", None)
551+
subparser.add_argument(*args, **kw)
552+
553+
def _add_subparser_exclusive_args(self, subparser, exclusive_args,
554+
version, do_help, input_args, command):
555+
for group_name, arguments in exclusive_args.items():
556+
if group_name == '__required__':
557+
continue
558+
required = exclusive_args['__required__'][group_name]
559+
exclusive_group = subparser.add_mutually_exclusive_group(
560+
required=required)
561+
self._add_subparser_args(exclusive_group, arguments,
562+
version, do_help, input_args, command)
563+
544564
def setup_debugging(self, debug):
545565
if not debug:
546566
return

‎cinderclient/tests/unit/v2/fakes.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,15 @@ def post_volumes_1234_action(self, body, **kw):
543543
raise AssertionError("Unexpected action: %s" % action)
544544
return (resp, {}, _body)
545545

546+
def get_volumes_fake(self, **kw):
547+
r = {'volume': self.get_volumes_detail(id='fake')[2]['volumes'][0]}
548+
return (200, {}, r)
549+
550+
def post_volumes_fake_action(self, body, **kw):
551+
_body = None
552+
resp = 202
553+
return (resp, {}, _body)
554+
546555
def post_volumes_5678_action(self, body, **kw):
547556
return self.post_volumes_1234_action(body, **kw)
548557

‎cinderclient/tests/unit/v2/test_volumes.py‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@
1515
# License for the specific language governing permissions and limitations
1616
# under the License.
1717

18+
from cinderclient import api_versions
1819
from cinderclient.tests.unit import utils
1920
from cinderclient.tests.unit.v2 import fakes
2021
from cinderclient.v2.volumes import Volume
2122

2223
cs = fakes.FakeClient()
24+
cs3 = fakes.FakeClient(api_versions.APIVersion('3.15'))
2325

2426

2527
class VolumesTest(utils.TestCase):
@@ -211,23 +213,23 @@ def test_get_encryption_metadata(self):
211213
self._assert_request_id(vol)
212214

213215
def test_migrate(self):
214-
v = cs.volumes.get('1234')
216+
v = cs3.volumes.get('1234')
215217
self._assert_request_id(v)
216-
vol = cs.volumes.migrate_volume(v, 'dest', False, False)
217-
cs.assert_called('POST', '/volumes/1234/action',
218-
{'os-migrate_volume': {'host': 'dest',
219-
'force_host_copy': False,
218+
vol = cs3.volumes.migrate_volume(v, 'dest', False, False)
219+
cs3.assert_called('POST', '/volumes/1234/action',
220+
{'os-migrate_volume': {'host': 'dest',
221+
'force_host_copy': False,
220222
'lock_volume': False}})
221223
self._assert_request_id(vol)
222224

223225
def test_migrate_with_lock_volume(self):
224-
v = cs.volumes.get('1234')
226+
v = cs3.volumes.get('1234')
225227
self._assert_request_id(v)
226-
vol = cs.volumes.migrate_volume(v, 'dest', False, True)
227-
cs.assert_called('POST', '/volumes/1234/action',
228-
{'os-migrate_volume': {'host': 'dest',
229-
'force_host_copy': False,
230-
'lock_volume': True}})
228+
vol = cs3.volumes.migrate_volume(v, 'dest', False, True)
229+
cs3.assert_called('POST', '/volumes/1234/action',
230+
{'os-migrate_volume': {'host': 'dest',
231+
'force_host_copy': False,
232+
'lock_volume': True}})
231233
self._assert_request_id(vol)
232234

233235
def test_metadata_update_all(self):
@@ -260,19 +262,19 @@ def test_set_bootable(self):
260262
self._assert_request_id(vol)
261263

262264
def test_volume_manage(self):
263-
vol = cs.volumes.manage('host1', {'k': 'v'})
265+
vol = cs3.volumes.manage('host1', {'k': 'v'})
264266
expected = {'host': 'host1', 'name': None, 'availability_zone': None,
265267
'description': None, 'metadata': None, 'ref': {'k': 'v'},
266268
'volume_type': None, 'bootable': False}
267-
cs.assert_called('POST', '/os-volume-manage', {'volume': expected})
269+
cs3.assert_called('POST', '/os-volume-manage', {'volume': expected})
268270
self._assert_request_id(vol)
269271

270272
def test_volume_manage_bootable(self):
271-
vol = cs.volumes.manage('host1', {'k': 'v'}, bootable=True)
273+
vol = cs3.volumes.manage('host1', {'k': 'v'}, bootable=True)
272274
expected = {'host': 'host1', 'name': None, 'availability_zone': None,
273275
'description': None, 'metadata': None, 'ref': {'k': 'v'},
274276
'volume_type': None, 'bootable': True}
275-
cs.assert_called('POST', '/os-volume-manage', {'volume': expected})
277+
cs3.assert_called('POST', '/os-volume-manage', {'volume': expected})
276278
self._assert_request_id(vol)
277279

278280
def test_volume_list_manageable(self):

‎cinderclient/tests/unit/v3/test_shell.py‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,3 +1013,129 @@ def test_snapshot_list_with_userid(self, mock_print_list):
10131013
columns = ['ID', 'Volume ID', 'Status', 'Name', 'Size', 'User ID']
10141014
mock_print_list.assert_called_once_with(mock.ANY, columns,
10151015
sortby_index=0)
1016+
1017+
@mock.patch('cinderclient.v3.volumes.Volume.migrate_volume')
1018+
def test_migrate_volume_before_3_16(self, v3_migrate_mock):
1019+
self.run_command('--os-volume-api-version 3.15 '
1020+
'migrate 1234 fakehost')
1021+
1022+
v3_migrate_mock.assert_called_once_with(
1023+
'fakehost', False, False, None)
1024+
1025+
@mock.patch('cinderclient.v3.volumes.Volume.migrate_volume')
1026+
def test_migrate_volume_3_16(self, v3_migrate_mock):
1027+
self.run_command('--os-volume-api-version 3.16 '
1028+
'migrate 1234 fakehost')
1029+
self.assertEqual(4, len(v3_migrate_mock.call_args[0]))
1030+
1031+
def test_migrate_volume_with_cluster_before_3_16(self):
1032+
self.assertRaises(exceptions.UnsupportedAttribute,
1033+
self.run_command,
1034+
'--os-volume-api-version 3.15 '
1035+
'migrate 1234 fakehost --cluster fakecluster')
1036+
1037+
@mock.patch('cinderclient.shell.CinderClientArgumentParser.error')
1038+
def test_migrate_volume_mutual_exclusion(self, error_mock):
1039+
error_mock.side_effect = SystemExit
1040+
self.assertRaises(SystemExit,
1041+
self.run_command,
1042+
'--os-volume-api-version 3.16 '
1043+
'migrate 1234 fakehost --cluster fakecluster')
1044+
msg = 'argument --cluster: not allowed with argument <host>'
1045+
error_mock.assert_called_once_with(msg)
1046+
1047+
@mock.patch('cinderclient.shell.CinderClientArgumentParser.error')
1048+
def test_migrate_volume_missing_required(self, error_mock):
1049+
error_mock.side_effect = SystemExit
1050+
self.assertRaises(SystemExit,
1051+
self.run_command,
1052+
'--os-volume-api-version 3.16 '
1053+
'migrate 1234')
1054+
msg = 'one of the arguments <host> --cluster is required'
1055+
error_mock.assert_called_once_with(msg)
1056+
1057+
def test_migrate_volume_host(self):
1058+
self.run_command('--os-volume-api-version 3.16 '
1059+
'migrate 1234 fakehost')
1060+
expected = {'os-migrate_volume': {'force_host_copy': False,
1061+
'lock_volume': False,
1062+
'host': 'fakehost'}}
1063+
self.assert_called('POST', '/volumes/1234/action', body=expected)
1064+
1065+
def test_migrate_volume_cluster(self):
1066+
self.run_command('--os-volume-api-version 3.16 '
1067+
'migrate 1234 --cluster mycluster')
1068+
expected = {'os-migrate_volume': {'force_host_copy': False,
1069+
'lock_volume': False,
1070+
'cluster': 'mycluster'}}
1071+
self.assert_called('POST', '/volumes/1234/action', body=expected)
1072+
1073+
def test_migrate_volume_bool_force(self):
1074+
self.run_command('--os-volume-api-version 3.16 '
1075+
'migrate 1234 fakehost --force-host-copy '
1076+
'--lock-volume')
1077+
expected = {'os-migrate_volume': {'force_host_copy': True,
1078+
'lock_volume': True,
1079+
'host': 'fakehost'}}
1080+
self.assert_called('POST', '/volumes/1234/action', body=expected)
1081+
1082+
def test_migrate_volume_bool_force_false(self):
1083+
# Set both --force-host-copy and --lock-volume to False.
1084+
self.run_command('--os-volume-api-version 3.16 '
1085+
'migrate 1234 fakehost --force-host-copy=False '
1086+
'--lock-volume=False')
1087+
expected = {'os-migrate_volume': {'force_host_copy': 'False',
1088+
'lock_volume': 'False',
1089+
'host': 'fakehost'}}
1090+
self.assert_called('POST', '/volumes/1234/action', body=expected)
1091+
1092+
# Do not set the values to --force-host-copy and --lock-volume.
1093+
self.run_command('--os-volume-api-version 3.16 '
1094+
'migrate 1234 fakehost')
1095+
expected = {'os-migrate_volume': {'force_host_copy': False,
1096+
'lock_volume': False,
1097+
'host': 'fakehost'}}
1098+
self.assert_called('POST', '/volumes/1234/action',
1099+
body=expected)
1100+
1101+
@ddt.data({'bootable': False, 'by_id': False, 'cluster': None},
1102+
{'bootable': True, 'by_id': False, 'cluster': None},
1103+
{'bootable': False, 'by_id': True, 'cluster': None},
1104+
{'bootable': True, 'by_id': True, 'cluster': None},
1105+
{'bootable': True, 'by_id': True, 'cluster': 'clustername'})
1106+
@ddt.unpack
1107+
def test_volume_manage(self, bootable, by_id, cluster):
1108+
cmd = ('--os-volume-api-version 3.16 '
1109+
'manage host1 some_fake_name --name foo --description bar '
1110+
'--volume-type baz --availability-zone az '
1111+
'--metadata k1=v1 k2=v2')
1112+
if by_id:
1113+
cmd += ' --id-type source-id'
1114+
if bootable:
1115+
cmd += ' --bootable'
1116+
if cluster:
1117+
cmd += ' --cluster ' + cluster
1118+
1119+
self.run_command(cmd)
1120+
ref = 'source-id' if by_id else 'source-name'
1121+
expected = {'volume': {'host': 'host1',
1122+
'ref': {ref: 'some_fake_name'},
1123+
'name': 'foo',
1124+
'description': 'bar',
1125+
'volume_type': 'baz',
1126+
'availability_zone': 'az',
1127+
'metadata': {'k1': 'v1', 'k2': 'v2'},
1128+
'bootable': bootable}}
1129+
if cluster:
1130+
expected['cluster'] = cluster
1131+
self.assert_called_anytime('POST', '/os-volume-manage', body=expected)
1132+
1133+
def test_volume_manage_before_3_16(self):
1134+
"""Cluster optional argument was not acceptable."""
1135+
self.assertRaises(exceptions.UnsupportedAttribute,
1136+
self.run_command,
1137+
'manage host1 some_fake_name '
1138+
'--cluster clustername'
1139+
'--name foo --description bar --bootable '
1140+
'--volume-type baz --availability-zone az '
1141+
'--metadata k1=v1 k2=v2')

‎cinderclient/tests/unit/v3/test_volumes.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from six.moves.urllib import parse
2828

2929
cs = fakes.FakeClient()
30+
cs3 = fakes.FakeClient(api_versions.APIVersion('3.16'))
3031

3132

3233
@ddt.ddt
@@ -145,3 +146,34 @@ def test_get_pools_filter_by_name(self, detail):
145146
request_url = '/scheduler-stats/get_pools?detail=True&name=pool1'
146147
cs.assert_called('GET', request_url)
147148
self._assert_request_id(vol)
149+
150+
def test_migrate_host(self):
151+
v = cs3.volumes.get('1234')
152+
self._assert_request_id(v)
153+
vol = cs3.volumes.migrate_volume(v, 'host_dest', False, False)
154+
cs3.assert_called('POST', '/volumes/1234/action',
155+
{'os-migrate_volume': {'host': 'host_dest',
156+
'force_host_copy': False,
157+
'lock_volume': False}})
158+
self._assert_request_id(vol)
159+
160+
def test_migrate_with_lock_volume(self):
161+
v = cs3.volumes.get('1234')
162+
self._assert_request_id(v)
163+
vol = cs3.volumes.migrate_volume(v, 'dest', False, True)
164+
cs3.assert_called('POST', '/volumes/1234/action',
165+
{'os-migrate_volume': {'host': 'dest',
166+
'force_host_copy': False,
167+
'lock_volume': True}})
168+
self._assert_request_id(vol)
169+
170+
def test_migrate_cluster(self):
171+
v = cs3.volumes.get('fake')
172+
self._assert_request_id(v)
173+
vol = cs3.volumes.migrate_volume(v, 'host_dest', False, False,
174+
'cluster_dest')
175+
cs3.assert_called('POST', '/volumes/fake/action',
176+
{'os-migrate_volume': {'cluster': 'cluster_dest',
177+
'force_host_copy': False,
178+
'lock_volume': False}})
179+
self._assert_request_id(vol)

0 commit comments

Comments
 (0)