"--wait" option added for managing/unmanaging a share snapshot

Implements: Add "--wait" to asynchronous operation commands
Closes-Bug: #1898312
Change-Id: I354ca855352228911f809769ce89d06af8dd9f61

manilaclient/tests/unit/v2/fakes.py

@@ -250,7 +250,8 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
         return (200, {}, shares)
 
     def get_snapshots_1234(self, **kw):
-        snapshot = {'snapshot': {'id': 1234, 'name': 'sharename'}}
+        snapshot = {'snapshot': {'id': 1234, 'name': 'sharename',
+                                 'status': 'available'}}
         return (200, {}, snapshot)
 
     def get_share_servers(self, **kw):

manilaclient/tests/unit/v2/test_shell.py

@@ -981,13 +981,15 @@ class ShellTest(test_utils.TestCase):
               )
     @ddt.unpack
     @mock.patch.object(shell_v2, '_find_share', mock.Mock())
+    @mock.patch.object(shell_v2, '_wait_for_resource_status', mock.Mock())
     def test_snapshot_manage(self, cmd_args, valid_params):
-        shell_v2._find_share.return_value = 'fake_share'
+        share_containing_snapshot = shares.Share('fake_share', {'id': '1234'})
+        shell_v2._find_share.return_value = share_containing_snapshot
         self.run_command('snapshot-manage fake_share fake_provider_location '
                          + cmd_args)
         expected = {
             'snapshot': {
-                'share_id': 'fake_share',
+                'share_id': '1234',
                 'provider_location': 'fake_provider_location',
                 'name': None,
                 'description': None,
@@ -995,11 +997,62 @@ class ShellTest(test_utils.TestCase):
         }
         expected['snapshot'].update(valid_params)
         self.assert_called('POST', '/snapshots/manage', body=expected)
+        # _wait_for_resource_status should not be triggered
+        self.assertEqual(0, shell_v2._wait_for_resource_status.call_count)
 
+    @mock.patch.object(shell_v2, '_find_share', mock.Mock())
+    @mock.patch.object(shell_v2, '_wait_for_resource_status', mock.Mock())
+    def test_snapshot_manage_with_wait(self):
+        share_containing_snapshot = shares.Share('fake_share', {'id': '1234'})
+        shell_v2._find_share.return_value = share_containing_snapshot
+        cmd_args = '--wait --driver_options opt1=opt1 opt2=opt2'
+        self.run_command('snapshot-manage fake_share fake_provider_location '
+                         + cmd_args)
+        expected = {
+            'snapshot': {
+                'share_id': '1234',
+                'provider_location': 'fake_provider_location',
+                'name': None,
+                'description': None,
+            }
+        }
+        valid_params = {'driver_options': {'opt1': 'opt1', 'opt2': 'opt2'}}
+        expected['snapshot'].update(valid_params)
+        self.assert_called('POST', '/snapshots/manage', body=expected)
+
+        shell_v2._find_share.assert_has_calls(
+            [mock.call(self.shell.cs, 'fake_share')])
+        self.assertEqual(1, shell_v2._find_share.call_count)
+        # _wait_for_resource_status should be triggered once
+        shell_v2._wait_for_resource_status.assert_called_once_with(
+            self.shell.cs, mock.ANY, 'available', resource_type='snapshot')
+
+    @mock.patch.object(shell_v2, '_find_share', mock.Mock())
+    @mock.patch.object(shell_v2, '_wait_for_resource_status', mock.Mock())
     def test_snapshot_unmanage(self):
+        share_containing_snapshot = shares.Share('fake_share', {'id': '1234'})
+        shell_v2._find_share.return_value = share_containing_snapshot
         self.run_command('snapshot-unmanage 1234')
+
         self.assert_called('POST', '/snapshots/1234/action',
                            body={'unmanage': None})
+        self.assertEqual(0, shell_v2._wait_for_resource_status.call_count)
+
+    @mock.patch.object(shell_v2, '_find_share', mock.Mock())
+    @mock.patch.object(shell_v2, '_wait_for_resource_status', mock.Mock())
+    def test_snapshot_unmanage_with_wait(self):
+        share_containing_snapshot = shares.Share('fake_share', {'id': '1234'})
+        shell_v2._find_share.return_value = share_containing_snapshot
+        self.run_command('snapshot-unmanage 1234 --wait')
+
+        self.assert_called('POST', '/snapshots/1234/action',
+                           body={'unmanage': None})
+        expected_snapshot = shell_v2._find_share_snapshot(
+            self.shell.cs, '1234')
+        # _wait_for_resource_status should be trigerred once
+        shell_v2._wait_for_resource_status.assert_called_once_with(
+            self.shell.cs, expected_snapshot, 'deleted',
+            resource_type='snapshot')
 
     def test_revert_to_snapshot(self):
 

manilaclient/v2/shell.py

@@ -326,6 +326,11 @@ def _print_share_group_snapshot_members(cs, share_group_snapshot):
     cliutils.print_dict(info.get('members', {}))
 
 
+def _wait_for_snapshot_status(cs, snapshot, expected_status='available'):
+    return _wait_for_resource_status(
+        cs, snapshot, expected_status, resource_type='snapshot')
+
+
 def _find_share_snapshot(cs, snapshot):
     """Get a snapshot by ID."""
     return apiclient_utils.find_resource(cs.share_snapshots, snapshot)
@@ -1704,10 +1709,14 @@ def do_share_server_reset_state(cs, args):
     action='single_alias',
     help='Optional driver options as key=value pairs (Default=None).',
     default=None)
+@cliutils.arg(
+    '--wait',
+    action='store_true',
+    default=False,
+    help='Wait for share snapshot to be managed')
 def do_snapshot_manage(cs, args):
     """Manage share snapshot not handled by Manila (Admin only)."""
     share_ref = _find_share(cs, args.share)
-
     driver_options = _extract_key_value_options(args, 'driver_options')
 
     share_snapshot = cs.share_snapshots.manage(
@@ -1716,6 +1725,12 @@ def do_snapshot_manage(cs, args):
         name=args.name, description=args.description
     )
 
+    if args.wait:
+        try:
+            _wait_for_snapshot_status(cs, share_snapshot,
+                                      expected_status='available')
+        except exceptions.CommandError as e:
+            print(e, file=sys.stderr)
     _print_share_snapshot(cs, share_snapshot)
 
 
@@ -1771,6 +1786,11 @@ def do_share_server_unmanage(cs, args):
     metavar='<snapshot>',
     nargs='+',
     help='Name or ID of the snapshot(s).')
+@cliutils.arg(
+    '--wait',
+    action='store_true',
+    default=False,
+    help='Wait for share snapshot to be unmanaged')
 def do_snapshot_unmanage(cs, args):
     """Unmanage one or more share snapshots (Admin only)."""
     failure_count = 0
@@ -1778,6 +1798,9 @@ def do_snapshot_unmanage(cs, args):
         try:
             snapshot_ref = _find_share_snapshot(cs, snapshot)
             snapshot_ref.unmanage_snapshot()
+            if args.wait:
+                _wait_for_snapshot_status(cs, snapshot_ref,
+                                          expected_status='deleted')
         except Exception as e:
             failure_count += 1
             print("Unmanage for share snapshot %s failed: %s" % (snapshot, e),

releasenotes/notes/bug-913201-add-wait-to-manage-unmanage-snapshot-9cc017a15733120f.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    The commands "snapshot-manage" and "snapshot-unmanage" now accept
+    an optional "--wait" option that allows users to let the client
+    poll for the completion of the operation.