Add "--wait" option for force-deleting a share/snapshot/share-instance

Closes-Bug: #1898317
Change-Id: I04265d6bdf84038670a3188a99cd2d639ad62cfe

manilaclient/tests/unit/v2/test_shell.py

@@ -512,6 +512,38 @@ class ShellTest(test_utils.TestCase):
             self.run_command('share-instance-force-delete 1234')
             manager_mock.force_delete.assert_called_once_with(share_instance)
 
+    @ddt.data(('share_instance_xyz', ), ('share_instance_abc',
+                                         'share_instance_xyz'))
+    def test_share_instance_force_delete_wait(self, instances_to_delete):
+        fake_manager = mock.Mock()
+        fake_instances = [
+            share_instances.ShareInstance(fake_manager, {'id': '1234'})
+            for instance in instances_to_delete
+        ]
+        instance_not_found_error = ("Delete for instance %s failed: No "
+                                    "instance with a name or "
+                                    "ID of '%s' exists.")
+        instances_are_not_found_errors = [
+            exceptions.CommandError(
+                instance_not_found_error % (instance, instance))
+            for instance in instances_to_delete
+        ]
+        self.mock_object(
+            shell_v2, '_find_share_instance',
+            mock.Mock(side_effect=(
+                fake_instances + instances_are_not_found_errors)))
+        self.run_command(
+            'share-instance-force-delete %s --wait' % ' '.join(
+                instances_to_delete))
+        shell_v2._find_share_instance.assert_has_calls([
+            mock.call(self.shell.cs, instance) for instance in
+            instances_to_delete
+        ])
+        fake_manager.force_delete.assert_has_calls([
+            mock.call(instance) for instance in fake_instances])
+        self.assertEqual(len(instances_to_delete),
+                         fake_manager.force_delete.call_count)
+
     def test_type_show_details(self):
         self.run_command('type-show 1234')
         self.assert_called_anytime('GET', '/types/1234')
@@ -956,6 +988,31 @@ class ShellTest(test_utils.TestCase):
             uri = '/shares/%s' % share.id
             self.assert_called_anytime('DELETE', uri, clear_callstack=False)
 
+    @ddt.data(('share_xyz', ), ('share_abc', 'share_xyz'))
+    def test_force_delete_wait(self, shares_to_delete):
+        fake_manager = mock.Mock()
+        fake_shares = [
+            shares.Share(fake_manager, {'id': '1234'})
+            for share in shares_to_delete
+        ]
+        share_not_found_error = ("Delete for share %s failed: No share with "
+                                 "a name or ID of '%s' exists.")
+        shares_are_not_found_errors = [
+            exceptions.CommandError(share_not_found_error % (share, share))
+            for share in shares_to_delete
+        ]
+        self.mock_object(
+            shell_v2, '_find_share',
+            mock.Mock(side_effect=(fake_shares + shares_are_not_found_errors)))
+        self.run_command('force-delete %s --wait' % ' '.join(shares_to_delete))
+        shell_v2._find_share.assert_has_calls([
+            mock.call(self.shell.cs, share) for share in shares_to_delete
+        ])
+        fake_manager.force_delete.assert_has_calls([
+            mock.call(share) for share in fake_shares])
+        self.assertEqual(len(shares_to_delete),
+                         fake_manager.force_delete.call_count)
+
     def test_list_snapshots(self):
         self.run_command('snapshot-list')
         self.assert_called('GET', '/snapshots/detail')
@@ -3353,26 +3410,36 @@ class ShellTest(test_utils.TestCase):
                 'DELETE', '/snapshots/%s' % snapshot.id,
                 clear_callstack=False)
 
-    @ddt.data(('1234', ), ('1234', '5678'))
+    @ddt.data(('snapshot_xyz', ), ('snapshot_abc', 'snapshot_xyz'))
-    def test_snapshot_force_delete(self, snapshot_ids):
+    def test_snapshot_force_delete_wait(self, snapshots_to_delete):
+        fake_manager = mock.Mock()
         fake_snapshots = [
-            share_snapshots.ShareSnapshot('fake', {'id': snapshot_id}, True)
+            share_snapshots.ShareSnapshot(fake_manager, {'id': '1234'})
-            for snapshot_id in snapshot_ids
+            for snapshot in snapshots_to_delete
+        ]
+        snapshot_not_found_error = ("Delete for snapshot %s failed: No "
+                                    "snapshot with a name or "
+                                    "ID of '%s' exists.")
+        snapshots_are_not_found_errors = [
+            exceptions.CommandError(
+                snapshot_not_found_error % (snapshot, snapshot))
+            for snapshot in snapshots_to_delete
         ]
         self.mock_object(
             shell_v2, '_find_share_snapshot',
-            mock.Mock(side_effect=fake_snapshots))
+            mock.Mock(side_effect=(
-
+                fake_snapshots + snapshots_are_not_found_errors)))
-        self.run_command('snapshot-force-delete %s' % ' '.join(snapshot_ids))
+        self.run_command(
-
+            'snapshot-force-delete %s --wait' % ' '.join(
+                snapshots_to_delete))
         shell_v2._find_share_snapshot.assert_has_calls([
-            mock.call(self.shell.cs, s_id) for s_id in snapshot_ids
+            mock.call(self.shell.cs, snapshot) for snapshot in
+            snapshots_to_delete
         ])
-        for snapshot in fake_snapshots:
+        fake_manager.force_delete.assert_has_calls([
-            self.assert_called_anytime(
+            mock.call(snapshot) for snapshot in fake_snapshots])
-                'POST', '/snapshots/%s/action' % snapshot.id,
+        self.assertEqual(len(snapshots_to_delete),
-                {'force_delete': None},
+                         fake_manager.force_delete.call_count)
-                clear_callstack=False)
 
     @ddt.data(('fake_type1', ), ('fake_type1', 'fake_type2'))
     def test_share_type_delete(self, type_ids):

manilaclient/v2/shell.py

@@ -55,6 +55,7 @@ def _wait_for_resource_status(cs,
         'share_replica': _find_share_replica,
         'share_group': _find_share_group,
         'share_group_snapshot': _find_share_group_snapshot,
+        'share_instance': _find_share_instance,
     }
 
     print_resource = {
@@ -63,6 +64,7 @@ def _wait_for_resource_status(cs,
         'share_replica': _print_share_replica,
         'share_group': _print_share_group,
         'share_group_snapshot': _print_share_group_snapshot,
+        'share_instance': _print_share_instance,
     }
 
     expected_status = expected_status or ('available', )
@@ -80,7 +82,7 @@ def _wait_for_resource_status(cs,
         'resource_type': resource_type.capitalize(),
         'resource': resource.id,
     }
-    not_found_regex = "no %s .* exists" % resource_type
+    not_found_regex = "no .* exists"
     while True:
         if time_elapsed > poll_timeout:
             print_resource[resource_type](cs, resource)
@@ -1686,12 +1688,20 @@ def do_delete(cs, args):
     metavar='<share>',
     nargs='+',
     help='Name or ID of the share(s) to force delete.')
+@cliutils.arg(
+    '--wait',
+    action='store_true',
+    help='Wait for share to delete')
+@cliutils.service_type('sharev2')
 def do_force_delete(cs, args):
     """Attempt force-delete of share, regardless of state (Admin only)."""
     failure_count = 0
+    shares_to_delete = []
     for share in args.share:
         try:
-            _find_share(cs, share).force_delete()
+            share_ref = _find_share(cs, share)
+            shares_to_delete.append(share_ref)
+            share_ref.force_delete()
         except Exception as e:
             failure_count += 1
             print("Delete for share %s failed: %s" % (share, e),
@@ -1699,6 +1709,12 @@ def do_force_delete(cs, args):
     if failure_count == len(args.share):
         raise exceptions.CommandError("Unable to force delete any of "
                                       "specified shares.")
+    if args.wait:
+        for share in shares_to_delete:
+            try:
+                _wait_for_share_status(cs, share, expected_status='deleted')
+            except exceptions.CommandError as e:
+                print(e, file=sys.stderr)
 
 
 @api_versions.wraps("1.0", "2.8")
@@ -2358,12 +2374,20 @@ def do_share_instance_show(cs, args):  # noqa
     nargs='+',
     help='Name or ID of the instance(s) to force delete.')
 @api_versions.wraps("2.3")
+@cliutils.arg(
+    '--wait',
+    action='store_true',
+    help='Wait for share instance deletion')
+@cliutils.service_type('sharev2')
 def do_share_instance_force_delete(cs, args):
     """Force-delete the share instance, regardless of state (Admin only)."""
     failure_count = 0
+    instances_to_delete = []
     for instance in args.instance:
         try:
-            _find_share_instance(cs, instance).force_delete()
+            instance_ref = _find_share_instance(cs, instance)
+            instances_to_delete.append(instance_ref)
+            instance_ref.force_delete()
         except Exception as e:
             failure_count += 1
             print("Delete for share instance %s failed: %s" % (instance, e),
@@ -2371,6 +2395,14 @@ def do_share_instance_force_delete(cs, args):
     if failure_count == len(args.instance):
         raise exceptions.CommandError("Unable to force delete any of "
                                       "specified share instances.")
+    if args.wait:
+        for instance in instances_to_delete:
+            try:
+                _wait_for_resource_status(
+                    cs, instance, resource_type='share_instance',
+                    expected_status='deleted')
+            except exceptions.CommandError as e:
+                print(e, file=sys.stderr)
 
 
 @cliutils.arg(
@@ -2814,18 +2846,25 @@ def do_snapshot_delete(cs, args):
     metavar='<snapshot>',
     nargs='+',
     help='Name or ID of the snapshot(s) to force delete.')
+@cliutils.arg(
+    '--wait',
+    action='store_true',
+    help='Wait for snapshot to delete')
+@cliutils.service_type('sharev2')
 def do_snapshot_force_delete(cs, args):
     """Attempt force-deletion of one or more snapshots.
 
     Regardless of the state (Admin only).
     """
     failure_count = 0
+    snapshots_to_delete = []
 
     for snapshot in args.snapshot:
         try:
             snapshot_ref = _find_share_snapshot(
                 cs, snapshot)
-            cs.share_snapshots.force_delete(snapshot_ref)
+            snapshots_to_delete.append(snapshot_ref)
+            snapshot_ref.force_delete()
         except Exception as e:
             failure_count += 1
             print("Delete for snapshot %s failed: %s" % (
@@ -2834,6 +2873,14 @@ def do_snapshot_force_delete(cs, args):
     if failure_count == len(args.snapshot):
         raise exceptions.CommandError("Unable to force delete any of the "
                                       "specified snapshots.")
+    if args.wait:
+        for snapshot in snapshots_to_delete:
+            try:
+                _wait_for_resource_status(
+                    cs, snapshot, resource_type='snapshot',
+                    expected_status='deleted')
+            except exceptions.CommandError as e:
+                print(e, file=sys.stderr)
 
 
 @cliutils.arg(

releasenotes/notes/bug-1898317-add-wait-option-for-force-deleting-share-snapshot-share-instance-fb2531b6033f0ae5.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    The commands "manila force-delete", "manila snapshot-force-delete"
+    and "manila share-instance-force-delete" now accept an optional
+    "--wait" that allows administrator users to let the client poll for the
+    completion of the operation.
+