Skip to content

Commit 8fce740

Browse files
AkrogAkrog
committed
Add cluster support in migration and manage
This patch adds support for API microversion 3.16, which allows us to pass --cluster optional argument to migration and manage operations. For this, a new type of CLI argument is added, the mutually exclusive arguments that can be used similarly to the utils.arg decorator, but with utils.exclusive_arg decorator. Implements: blueprint cinder-volume-active-active-support Change-Id: If004715b9887d2a0f9fc630b44d6e11a4a8b778d
1 parent 6c214ee commit 8fce740

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
@@ -490,6 +491,7 @@ def _find_actions(self, subparsers, actions_module, version,
490491
action_help = desc.strip().split('\n')[0]
491492
action_help += additional_msg
492493

494+
exclusive_args = getattr(callback, 'exclusive_args', {})
493495
arguments = getattr(callback, 'arguments', [])
494496

495497
subparser = subparsers.add_parser(
@@ -504,41 +506,59 @@ def _find_actions(self, subparsers, actions_module, version,
504506
help=argparse.SUPPRESS,)
505507

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

516+
def _add_subparser_args(self, subparser, arguments, version, do_help,
517+
input_args, command):
518+
# NOTE(ntpttr): We get a counter for each argument in this
519+
# command here because during the microversion check we only
520+
# want to raise an exception if no version of the argument
521+
# matches the current microversion. The exception will only
522+
# be raised after the last instance of a particular argument
523+
# fails the check.
524+
arg_counter = collections.defaultdict(int)
525+
for (args, kwargs) in arguments:
526+
arg_counter[args[0]] += 1
527+
528+
for (args, kwargs) in arguments:
529+
start_version = kwargs.get("start_version", None)
530+
start_version = api_versions.APIVersion(start_version)
531+
end_version = kwargs.get('end_version', None)
532+
end_version = api_versions.APIVersion(end_version)
533+
if do_help and (start_version or end_version):
534+
kwargs["help"] = kwargs.get("help", "") + (
535+
self._build_versioned_help_message(start_version,
536+
end_version))
537+
if not version.matches(start_version, end_version):
538+
if args[0] in input_args and command == input_args[0]:
539+
if arg_counter[args[0]] == 1:
540+
# This is the last version of this argument,
541+
# raise the exception.
542+
raise exc.UnsupportedAttribute(args[0],
543+
start_version, end_version)
544+
arg_counter[args[0]] -= 1
545+
continue
546+
kw = kwargs.copy()
547+
kw.pop("start_version", None)
548+
kw.pop("end_version", None)
549+
subparser.add_argument(*args, **kw)
550+
551+
def _add_subparser_exclusive_args(self, subparser, exclusive_args,
552+
version, do_help, input_args, command):
553+
for group_name, arguments in exclusive_args.items():
554+
if group_name == '__required__':
555+
continue
556+
required = exclusive_args['__required__'][group_name]
557+
exclusive_group = subparser.add_mutually_exclusive_group(
558+
required=required)
559+
self._add_subparser_args(exclusive_group, arguments,
560+
version, do_help, input_args, command)
561+
542562
def setup_debugging(self, debug):
543563
if not debug:
544564
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
@@ -1007,3 +1007,129 @@ def test_snapshot_list_with_userid(self, mock_print_list):
10071007
columns = ['ID', 'Volume ID', 'Status', 'Name', 'Size', 'User ID']
10081008
mock_print_list.assert_called_once_with(mock.ANY, columns,
10091009
sortby_index=0)
1010+
1011+
@mock.patch('cinderclient.v3.volumes.Volume.migrate_volume')
1012+
def test_migrate_volume_before_3_16(self, v3_migrate_mock):
1013+
self.run_command('--os-volume-api-version 3.15 '
1014+
'migrate 1234 fakehost')
1015+
1016+
v3_migrate_mock.assert_called_once_with(
1017+
'fakehost', False, False, None)
1018+
1019+
@mock.patch('cinderclient.v3.volumes.Volume.migrate_volume')
1020+
def test_migrate_volume_3_16(self, v3_migrate_mock):
1021+
self.run_command('--os-volume-api-version 3.16 '
1022+
'migrate 1234 fakehost')
1023+
self.assertEqual(4, len(v3_migrate_mock.call_args[0]))
1024+
1025+
def test_migrate_volume_with_cluster_before_3_16(self):
1026+
self.assertRaises(exceptions.UnsupportedAttribute,
1027+
self.run_command,
1028+
'--os-volume-api-version 3.15 '
1029+
'migrate 1234 fakehost --cluster fakecluster')
1030+
1031+
@mock.patch('cinderclient.shell.CinderClientArgumentParser.error')
1032+
def test_migrate_volume_mutual_exclusion(self, error_mock):
1033+
error_mock.side_effect = SystemExit
1034+
self.assertRaises(SystemExit,
1035+
self.run_command,
1036+
'--os-volume-api-version 3.16 '
1037+
'migrate 1234 fakehost --cluster fakecluster')
1038+
msg = 'argument --cluster: not allowed with argument <host>'
1039+
error_mock.assert_called_once_with(msg)
1040+
1041+
@mock.patch('cinderclient.shell.CinderClientArgumentParser.error')
1042+
def test_migrate_volume_missing_required(self, error_mock):
1043+
error_mock.side_effect = SystemExit
1044+
self.assertRaises(SystemExit,
1045+
self.run_command,
1046+
'--os-volume-api-version 3.16 '
1047+
'migrate 1234')
1048+
msg = 'one of the arguments <host> --cluster is required'
1049+
error_mock.assert_called_once_with(msg)
1050+
1051+
def test_migrate_volume_host(self):
1052+
self.run_command('--os-volume-api-version 3.16 '
1053+
'migrate 1234 fakehost')
1054+
expected = {'os-migrate_volume': {'force_host_copy': False,
1055+
'lock_volume': False,
1056+
'host': 'fakehost'}}
1057+
self.assert_called('POST', '/volumes/1234/action', body=expected)
1058+
1059+
def test_migrate_volume_cluster(self):
1060+
self.run_command('--os-volume-api-version 3.16 '
1061+
'migrate 1234 --cluster mycluster')
1062+
expected = {'os-migrate_volume': {'force_host_copy': False,
1063+
'lock_volume': False,
1064+
'cluster': 'mycluster'}}
1065+
self.assert_called('POST', '/volumes/1234/action', body=expected)
1066+
1067+
def test_migrate_volume_bool_force(self):
1068+
self.run_command('--os-volume-api-version 3.16 '
1069+
'migrate 1234 fakehost --force-host-copy '
1070+
'--lock-volume')
1071+
expected = {'os-migrate_volume': {'force_host_copy': True,
1072+
'lock_volume': True,
1073+
'host': 'fakehost'}}
1074+
self.assert_called('POST', '/volumes/1234/action', body=expected)
1075+
1076+
def test_migrate_volume_bool_force_false(self):
1077+
# Set both --force-host-copy and --lock-volume to False.
1078+
self.run_command('--os-volume-api-version 3.16 '
1079+
'migrate 1234 fakehost --force-host-copy=False '
1080+
'--lock-volume=False')
1081+
expected = {'os-migrate_volume': {'force_host_copy': 'False',
1082+
'lock_volume': 'False',
1083+
'host': 'fakehost'}}
1084+
self.assert_called('POST', '/volumes/1234/action', body=expected)
1085+
1086+
# Do not set the values to --force-host-copy and --lock-volume.
1087+
self.run_command('--os-volume-api-version 3.16 '
1088+
'migrate 1234 fakehost')
1089+
expected = {'os-migrate_volume': {'force_host_copy': False,
1090+
'lock_volume': False,
1091+
'host': 'fakehost'}}
1092+
self.assert_called('POST', '/volumes/1234/action',
1093+
body=expected)
1094+
1095+
@ddt.data({'bootable': False, 'by_id': False, 'cluster': None},
1096+
{'bootable': True, 'by_id': False, 'cluster': None},
1097+
{'bootable': False, 'by_id': True, 'cluster': None},
1098+
{'bootable': True, 'by_id': True, 'cluster': None},
1099+
{'bootable': True, 'by_id': True, 'cluster': 'clustername'})
1100+
@ddt.unpack
1101+
def test_volume_manage(self, bootable, by_id, cluster):
1102+
cmd = ('--os-volume-api-version 3.16 '
1103+
'manage host1 some_fake_name --name foo --description bar '
1104+
'--volume-type baz --availability-zone az '
1105+
'--metadata k1=v1 k2=v2')
1106+
if by_id:
1107+
cmd += ' --id-type source-id'
1108+
if bootable:
1109+
cmd += ' --bootable'
1110+
if cluster:
1111+
cmd += ' --cluster ' + cluster
1112+
1113+
self.run_command(cmd)
1114+
ref = 'source-id' if by_id else 'source-name'
1115+
expected = {'volume': {'host': 'host1',
1116+
'ref': {ref: 'some_fake_name'},
1117+
'name': 'foo',
1118+
'description': 'bar',
1119+
'volume_type': 'baz',
1120+
'availability_zone': 'az',
1121+
'metadata': {'k1': 'v1', 'k2': 'v2'},
1122+
'bootable': bootable}}
1123+
if cluster:
1124+
expected['cluster'] = cluster
1125+
self.assert_called_anytime('POST', '/os-volume-manage', body=expected)
1126+
1127+
def test_volume_manage_before_3_16(self):
1128+
"""Cluster optional argument was not acceptable."""
1129+
self.assertRaises(exceptions.UnsupportedAttribute,
1130+
self.run_command,
1131+
'manage host1 some_fake_name '
1132+
'--cluster clustername'
1133+
'--name foo --description bar --bootable '
1134+
'--volume-type baz --availability-zone az '
1135+
'--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)