diff --git a/cinderclient/tests/unit/v2/test_shell.py b/cinderclient/tests/unit/v2/test_shell.py index 4cb3f63bf..3a88691be 100644 --- a/cinderclient/tests/unit/v2/test_shell.py +++ b/cinderclient/tests/unit/v2/test_shell.py @@ -276,7 +276,8 @@ class ShellTest(utils.TestCase): with mock.patch('cinderclient.utils.print_list') as mock_print: self.run_command(cmd) mock_print.assert_called_once_with( - mock.ANY, mock.ANY, sortby_index=None) + mock.ANY, mock.ANY, exclude_unavailable=True, + sortby_index=None) def test_list_reorder_without_sort(self): # sortby_index is 0 without sort information @@ -284,7 +285,8 @@ class ShellTest(utils.TestCase): with mock.patch('cinderclient.utils.print_list') as mock_print: self.run_command(cmd) mock_print.assert_called_once_with( - mock.ANY, mock.ANY, sortby_index=0) + mock.ANY, mock.ANY, exclude_unavailable=True, + sortby_index=0) def test_list_availability_zone(self): self.run_command('availability-zone-list') @@ -710,17 +712,38 @@ class ShellTest(utils.TestCase): self.assert_called_anytime('GET', '/types/1') def test_migrate_volume(self): - self.run_command('migrate 1234 fakehost --force-host-copy=True') + self.run_command('migrate 1234 fakehost --force-host-copy=True ' + '--lock-volume=True') expected = {'os-migrate_volume': {'force_host_copy': 'True', + 'lock_volume': 'True', 'host': 'fakehost'}} self.assert_called('POST', '/volumes/1234/action', body=expected) def test_migrate_volume_bool_force(self): - self.run_command('migrate 1234 fakehost --force-host-copy') + self.run_command('migrate 1234 fakehost --force-host-copy ' + '--lock-volume') expected = {'os-migrate_volume': {'force_host_copy': True, + 'lock_volume': True, 'host': 'fakehost'}} self.assert_called('POST', '/volumes/1234/action', body=expected) + def test_migrate_volume_bool_force_false(self): + # Set both --force-host-copy and --lock-volume to False. + self.run_command('migrate 1234 fakehost --force-host-copy=False ' + '--lock-volume=False') + expected = {'os-migrate_volume': {'force_host_copy': 'False', + 'lock_volume': 'False', + 'host': 'fakehost'}} + self.assert_called('POST', '/volumes/1234/action', body=expected) + + # Do not set the values to --force-host-copy and --lock-volume. + self.run_command('migrate 1234 fakehost') + expected = {'os-migrate_volume': {'force_host_copy': False, + 'lock_volume': False, + 'host': 'fakehost'}} + self.assert_called('POST', '/volumes/1234/action', + body=expected) + def test_snapshot_metadata_set(self): self.run_command('snapshot-metadata 1234 set key1=val1 key2=val2') self.assert_called('POST', '/snapshots/1234/metadata', diff --git a/cinderclient/tests/unit/v2/test_volumes.py b/cinderclient/tests/unit/v2/test_volumes.py index b56d908da..08b9678fa 100644 --- a/cinderclient/tests/unit/v2/test_volumes.py +++ b/cinderclient/tests/unit/v2/test_volumes.py @@ -166,8 +166,19 @@ class VolumesTest(utils.TestCase): def test_migrate(self): v = cs.volumes.get('1234') - cs.volumes.migrate_volume(v, 'dest', False) - cs.assert_called('POST', '/volumes/1234/action') + cs.volumes.migrate_volume(v, 'dest', False, False) + cs.assert_called('POST', '/volumes/1234/action', + {'os-migrate_volume': {'host': 'dest', + 'force_host_copy': False, + 'lock_volume': False}}) + + def test_migrate_with_lock_volume(self): + v = cs.volumes.get('1234') + cs.volumes.migrate_volume(v, 'dest', False, True) + cs.assert_called('POST', '/volumes/1234/action', + {'os-migrate_volume': {'host': 'dest', + 'force_host_copy': False, + 'lock_volume': True}}) def test_metadata_update_all(self): cs.volumes.update_all_metadata(1234, {'k1': 'v1'}) diff --git a/cinderclient/utils.py b/cinderclient/utils.py index 89478bd61..13baf11e0 100644 --- a/cinderclient/utils.py +++ b/cinderclient/utils.py @@ -110,11 +110,14 @@ def _print(pt, order): print(strutils.safe_encode(pt.get_string(sortby=order))) -def print_list(objs, fields, formatters=None, sortby_index=0): +def print_list(objs, fields, exclude_unavailable=False, formatters=None, + sortby_index=0): '''Prints a list of objects. @param objs: Objects to print @param fields: Fields on each object to be printed + @param exclude_unavailable: Boolean to decide if unavailable fields are + removed @param formatters: Custom field formatters @param sortby_index: Results sorted against the key in the fields list at this index; if None then the object order is not @@ -122,12 +125,14 @@ def print_list(objs, fields, formatters=None, sortby_index=0): ''' formatters = formatters or {} mixed_case_fields = ['serverId'] - pt = prettytable.PrettyTable([f for f in fields], caching=False) - pt.aligns = ['l' for f in fields] + removed_fields = [] + rows = [] for o in objs: row = [] for field in fields: + if field in removed_fields: + continue if field in formatters: row.append(formatters[field](o)) else: @@ -138,12 +143,24 @@ def print_list(objs, fields, formatters=None, sortby_index=0): if type(o) == dict and field in o: data = o[field] else: - data = getattr(o, field_name, '') + if not hasattr(o, field_name) and exclude_unavailable: + removed_fields.append(field) + continue + else: + data = getattr(o, field_name, '') if data is None: data = '-' if isinstance(data, six.string_types) and "\r" in data: data = data.replace("\r", " ") row.append(data) + rows.append(row) + + for f in removed_fields: + fields.remove(f) + + pt = prettytable.PrettyTable((f for f in fields), caching=False) + pt.aligns = ['l' for f in fields] + for row in rows: pt.add_row(row) if sortby_index is None: diff --git a/cinderclient/v2/shell.py b/cinderclient/v2/shell.py index f041be848..c2ac8bfc9 100644 --- a/cinderclient/v2/shell.py +++ b/cinderclient/v2/shell.py @@ -160,6 +160,11 @@ def _extract_metadata(args): metavar='', default=None, help='Filters results by a status. Default=None.') +@utils.arg('--migration_status', + metavar='', + default=None, + help='Filters results by a migration status. Default=None. ' + 'Admin only.') @utils.arg('--metadata', type=str, nargs='*', @@ -212,6 +217,7 @@ def do_list(cs, args): 'project_id': args.tenant, 'name': args.name, 'status': args.status, + 'migration_status': args.migration_status, 'metadata': _extract_metadata(args) if args.metadata else None, } @@ -233,18 +239,19 @@ def do_list(cs, args): setattr(vol, 'attached_to', ','.join(map(str, servers))) if all_tenants: - key_list = ['ID', 'Tenant ID', 'Status', 'Name', + key_list = ['ID', 'Tenant ID', 'Status', 'Migration Status', 'Name', 'Size', 'Volume Type', 'Bootable', 'Multiattach', 'Attached to'] else: - key_list = ['ID', 'Status', 'Name', + key_list = ['ID', 'Status', 'Migration Status', 'Name', 'Size', 'Volume Type', 'Bootable', 'Multiattach', 'Attached to'] if args.sort_key or args.sort_dir or args.sort: sortby_index = None else: sortby_index = 0 - utils.print_list(volumes, key_list, sortby_index=sortby_index) + utils.print_list(volumes, key_list, exclude_unavailable=True, + sortby_index=sortby_index) @utils.arg('volume', @@ -452,7 +459,8 @@ def do_force_delete(cs, args): @utils.arg('--state', metavar='', default='available', help=('The state to assign to the volume. Valid values are ' '"available", "error", "creating", "deleting", "in-use", ' - '"attaching", "detaching" and "error_deleting". ' + '"attaching", "detaching", "error_deleting" and ' + '"maintenance". ' 'NOTE: This command simply changes the state of the ' 'Volume in the DataBase with no regard to actual status, ' 'exercise caution when using. Default=available.')) @@ -1153,11 +1161,31 @@ def do_upload_to_image(cs, args): help='Enables or disables generic host-based ' 'force-migration, which bypasses driver ' 'optimizations. Default=False.') +@utils.arg('--lock-volume', metavar='', + choices=['True', 'False'], + required=False, + const=True, + nargs='?', + default=False, + help='Enables or disables the termination of volume migration ' + 'caused by other commands. This option applies to the ' + 'available volume. True means it locks the volume ' + 'state and does not allow the migration to be aborted. The ' + 'volume status will be in maintenance during the ' + 'migration. False means it allows the volume migration ' + 'to be aborted. The volume status is still in the original ' + 'status. Default=False.') @utils.service_type('volumev2') def do_migrate(cs, args): """Migrates volume to a new host.""" volume = utils.find_volume(cs, args.volume) - volume.migrate_volume(args.host, args.force_host_copy) + try: + volume.migrate_volume(args.host, args.force_host_copy, + args.lock_volume) + print("Request to migrate volume %s has been accepted." % (volume)) + except Exception as e: + print("Migration for volume %s failed: %s." % (volume, + six.text_type(e))) @utils.arg('volume', metavar='', diff --git a/cinderclient/v2/volumes.py b/cinderclient/v2/volumes.py index 3fdf9868d..76bf293a6 100644 --- a/cinderclient/v2/volumes.py +++ b/cinderclient/v2/volumes.py @@ -139,9 +139,9 @@ class Volume(base.Resource): """ self.manager.extend(self, new_size) - def migrate_volume(self, host, force_host_copy): + def migrate_volume(self, host, force_host_copy, lock_volume): """Migrate the volume to a new host.""" - self.manager.migrate_volume(self, host, force_host_copy) + self.manager.migrate_volume(self, host, force_host_copy, lock_volume) def retype(self, volume_type, policy): """Change a volume's type.""" @@ -554,16 +554,19 @@ class VolumeManager(base.ManagerWithFind): """ return self._get("/volumes/%s/encryption" % volume_id)._info - def migrate_volume(self, volume, host, force_host_copy): + def migrate_volume(self, volume, host, force_host_copy, lock_volume): """Migrate volume to new host. :param volume: The :class:`Volume` to migrate :param host: The destination host :param force_host_copy: Skip driver optimizations + :param lock_volume: Lock the volume and guarantee the migration + to finish """ return self._action('os-migrate_volume', volume, - {'host': host, 'force_host_copy': force_host_copy}) + {'host': host, 'force_host_copy': force_host_copy, + 'lock_volume': lock_volume}) def migrate_volume_completion(self, old_volume, new_volume, error): """Complete the migration from the old volume to the temp new one.