Merge "Volume status management for volume migration"

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',

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.volumes.migrate_volume(v, 'dest', False, False)
-        cs.assert_called('POST', '/volumes/1234/action')
+        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'})

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)
+    removed_fields = []
-    pt.aligns = ['l' for f in 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:

cinderclient/v2/shell.py

@@ -160,6 +160,11 @@ def _extract_metadata(args):
            metavar='<status>',
            default=None,
            help='Filters results by a status. Default=None.')
+@utils.arg('--migration_status',
+           metavar='<migration_status>',
+           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='<state>', 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='<True|False>',
+           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='<volume>',

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.