From 202f65bd83819c5306e7abb0e2a449adcffb8057 Mon Sep 17 00:00:00 2001 From: tspyderboy Date: Fri, 22 Mar 2024 00:39:46 +0530 Subject: [PATCH] "--wait" option added for snapshot create/revert/delete commands 1) Creating a snapshot: $ manila snapshot-create share1 --name testshare1snapshot --wait 2) Deleting a snapshot: $ manila snapshot-delete testshare1snapshot --wait 3) Reverting a share to its snapshot: $ manila revert-to-snapshot testshare1snapshot --wait Closes-Bug: #1898307 Change-Id: Ib248e48c53c4c6c5886f2693e3bc26a0848fff3a --- manilaclient/tests/unit/v2/test_shell.py | 70 +++++++++++++++++++ manilaclient/v2/shell.py | 26 +++++++ ...rt-delete-a-snapshot-20271b8ebb60ade5.yaml | 6 ++ 3 files changed, 102 insertions(+) create mode 100644 releasenotes/notes/bug-1898307-add-wait-to-create-revert-delete-a-snapshot-20271b8ebb60ade5.yaml diff --git a/manilaclient/tests/unit/v2/test_shell.py b/manilaclient/tests/unit/v2/test_shell.py index 33a03480f..7e563c432 100644 --- a/manilaclient/tests/unit/v2/test_shell.py +++ b/manilaclient/tests/unit/v2/test_shell.py @@ -1054,6 +1054,7 @@ class ShellTest(test_utils.TestCase): self.shell.cs, expected_snapshot, 'deleted', resource_type='snapshot') + @mock.patch.object(shell_v2, '_wait_for_share_status', mock.Mock()) def test_revert_to_snapshot(self): fake_share_snapshot = type( @@ -1066,6 +1067,25 @@ class ShellTest(test_utils.TestCase): self.assert_called('POST', '/shares/1234/action', body={'revert': {'snapshot_id': '5678'}}) + # _wait_for_share_status should not be trigerred + self.assertEqual(0, shell_v2._wait_for_share_status.call_count) + + @mock.patch.object(shell_v2, '_wait_for_share_status', mock.Mock()) + def test_revert_to_snapshot_with_wait(self): + + fake_share_snapshot = type( + 'FakeShareSnapshot', (object,), {'id': '5678', 'share_id': '1234'}) + self.mock_object( + shell_v2, '_find_share_snapshot', + mock.Mock(return_value=fake_share_snapshot)) + + self.run_command('revert-to-snapshot 5678 --wait') + + self.assert_called('POST', '/shares/1234/action', + body={'revert': {'snapshot_id': '5678'}}) + # _wait_for_share_status should be trigerred once + shell_v2._wait_for_share_status.assert_called_once_with( + self.shell.cs, mock.ANY) def test_delete(self): self.run_command('delete 1234') @@ -3693,6 +3713,39 @@ class ShellTest(test_utils.TestCase): 'DELETE', '/share-networks/%s' % sn.id, clear_callstack=False) + @mock.patch.object(shell_v2, '_find_share', mock.Mock()) + @mock.patch.object(shell_v2, '_wait_for_snapshot_status', mock.Mock()) + def test_snapshot_create(self): + share_to_create_snapshot = shares.Share('fake_share', {'id': '1234'}) + shell_v2._find_share.return_value = share_to_create_snapshot + + self.run_command( + 'snapshot-create fake_share --name testshare1snapshot') + + shell_v2._find_share.assert_has_calls([ + mock.call(self.shell.cs, 'fake_share')]) + self.assert_called_anytime( + 'POST', '/snapshots', clear_callstack=False) + # _wait_for_snapshot_status should not be trigerred + self.assertEqual(0, shell_v2._wait_for_snapshot_status.call_count) + + @mock.patch.object(shell_v2, '_find_share', mock.Mock()) + @mock.patch.object(shell_v2, '_wait_for_snapshot_status', mock.Mock()) + def test_snapshot_create_with_wait(self): + share_to_create_snapshot = shares.Share('fake_share', {'id': '1234'}) + shell_v2._find_share.return_value = share_to_create_snapshot + + self.run_command( + 'snapshot-create fake_share --name testshare1snapshot --wait') + + shell_v2._find_share.assert_has_calls([ + mock.call(self.shell.cs, 'fake_share')]) + self.assert_called_anytime( + 'POST', '/snapshots', clear_callstack=False) + # _wait_for_snapshot_status should be trigerred once + shell_v2._wait_for_snapshot_status.assert_called_once_with( + self.shell.cs, mock.ANY, expected_status='available') + @ddt.data(('fake_snapshot1', ), ('fake_snapshot1', 'fake_snapshot2')) def test_snapshot_delete(self, snapshot_ids): fake_snapshots = [ @@ -3713,6 +3766,23 @@ class ShellTest(test_utils.TestCase): 'DELETE', '/snapshots/%s' % snapshot.id, clear_callstack=False) + @mock.patch.object(shell_v2, '_find_share_snapshot', mock.Mock()) + @mock.patch.object(shell_v2, '_wait_for_snapshot_status', mock.Mock()) + def test_snapshot_delete_with_wait(self): + fake_snapshot = share_snapshots.ShareSnapshot( + 'fake', {'id': 'fake_snapshot1'}, True) + shell_v2._find_share_snapshot.return_value = fake_snapshot + + self.run_command('snapshot-delete fake_snapshot1 --wait') + + shell_v2._find_share_snapshot.assert_has_calls([ + mock.call(self.shell.cs, 'fake_snapshot1')]) + self.assert_called_anytime( + 'DELETE', '/snapshots/fake_snapshot1', clear_callstack=False) + # _wait_for_resource_status should be trigerred once + shell_v2._wait_for_snapshot_status.assert_called_once_with( + self.shell.cs, 'fake_snapshot1', expected_status='deleted') + @ddt.data(('snapshot_xyz', ), ('snapshot_abc', 'snapshot_xyz')) def test_snapshot_force_delete_wait(self, snapshots_to_delete): fake_manager = mock.Mock() diff --git a/manilaclient/v2/shell.py b/manilaclient/v2/shell.py index d00ab0b75..54ca59a4b 100644 --- a/manilaclient/v2/shell.py +++ b/manilaclient/v2/shell.py @@ -1817,11 +1817,18 @@ def do_snapshot_unmanage(cs, args): metavar='', help='Name or ID of the snapshot to restore. The snapshot must be the ' 'most recent one known to manila.') +@cliutils.arg( + '--wait', + action='store_true', + default=False, + help='Wait for share to be reverted from snapshot.') def do_revert_to_snapshot(cs, args): """Revert a share to the specified snapshot.""" snapshot = _find_share_snapshot(cs, args.snapshot) share = _find_share(cs, snapshot.share_id) share.revert_to_snapshot(snapshot) + if args.wait: + _wait_for_share_status(cs, share) @cliutils.arg( @@ -3061,6 +3068,11 @@ def do_snapshot_instance_export_location_show(cs, args): metavar='', default=None, help='Optional snapshot description. (Default=None)') +@cliutils.arg( + '--wait', + action='store_true', + default=False, + help='Wait for snapshot to be created.') def do_snapshot_create(cs, args): """Add a new snapshot.""" share = _find_share(cs, args.share) @@ -3068,6 +3080,12 @@ def do_snapshot_create(cs, args): args.force, args.name, args.description) + if args.wait: + try: + _wait_for_snapshot_status(cs, snapshot, + expected_status='available') + except exceptions.CommandError as e: + print(e, file=sys.stderr) _print_share_snapshot(cs, snapshot) @@ -3143,6 +3161,11 @@ def do_snapshot_rename(cs, args): metavar='', nargs='+', help='Name or ID of the snapshot(s) to delete.') +@cliutils.arg( + '--wait', + action='store_true', + default=False, + help='Wait for snapshot to be deleted') def do_snapshot_delete(cs, args): """Remove one or more snapshots.""" failure_count = 0 @@ -3152,6 +3175,9 @@ def do_snapshot_delete(cs, args): snapshot_ref = _find_share_snapshot( cs, snapshot) cs.share_snapshots.delete(snapshot_ref) + if args.wait: + _wait_for_snapshot_status(cs, snapshot, + expected_status='deleted') except Exception as e: failure_count += 1 print("Delete for snapshot %s failed: %s" % ( diff --git a/releasenotes/notes/bug-1898307-add-wait-to-create-revert-delete-a-snapshot-20271b8ebb60ade5.yaml b/releasenotes/notes/bug-1898307-add-wait-to-create-revert-delete-a-snapshot-20271b8ebb60ade5.yaml new file mode 100644 index 000000000..df2557f68 --- /dev/null +++ b/releasenotes/notes/bug-1898307-add-wait-to-create-revert-delete-a-snapshot-20271b8ebb60ade5.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + The commands "snapshot-create", "snapshot-delete" and "revert-to-snapshot" + now accept an optional "--wait" option that allows users to let the + client poll for the completion of the operation. \ No newline at end of file