From 36aee019cdeec15d1b0076b30d37e610f80c8513 Mon Sep 17 00:00:00 2001 From: paulali Date: Wed, 19 May 2021 16:45:25 +0000 Subject: [PATCH] Add a --wait flag to share manage/unmanage This option enables "manage" and "unmanage" commands to wait until the action has completed. Partially-implements: bp add-wait-to-async-commands Closes-Bug: #1898309 Change-Id: I03b87f2f27b045e0a83c3a25cc482901bacffa15 --- manilaclient/tests/unit/v2/test_shell.py | 64 +++++++++++++++++-- manilaclient/v2/shell.py | 29 +++++++-- ...hare-manage-unmanage-d2060c61cc295bfd.yaml | 7 ++ 3 files changed, 89 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/bug-1898309-add-wait-to-share-manage-unmanage-d2060c61cc295bfd.yaml diff --git a/manilaclient/tests/unit/v2/test_shell.py b/manilaclient/tests/unit/v2/test_shell.py index b082cf35b..871863d46 100644 --- a/manilaclient/tests/unit/v2/test_shell.py +++ b/manilaclient/tests/unit/v2/test_shell.py @@ -691,7 +691,8 @@ class ShellTest(test_utils.TestCase): 'share_type': None, 'share_server_id': None, }}, - {'cmd_args': '--public', + {'cmd_args': '--public' + ' --wait', 'valid_params': { 'driver_options': {}, 'share_type': None, @@ -710,7 +711,8 @@ class ShellTest(test_utils.TestCase): 'version': '--os-share-api-version 2.8', }, {'cmd_args': '--driver_options opt1=opt1 opt2=opt2' - ' --share_type fake_share_type', + ' --share_type fake_share_type' + ' --wait', 'valid_params': { 'driver_options': {'opt1': 'opt1', 'opt2': 'opt2'}, 'share_type': 'fake_share_type', @@ -730,7 +732,8 @@ class ShellTest(test_utils.TestCase): }, {'cmd_args': '--driver_options opt1=opt1 opt2=opt2' ' --share_type fake_share_type' - ' --share_server_id fake_server', + ' --share_server_id fake_server' + ' --wait', 'valid_params': { 'driver_options': {'opt1': 'opt1', 'opt2': 'opt2'}, 'share_type': 'fake_share_type', @@ -740,6 +743,16 @@ class ShellTest(test_utils.TestCase): @ddt.unpack def test_manage(self, cmd_args, valid_params, is_public=False, version=None): + share_to_be_managed = shares.Share( + 'fake_share', { + 'id': 'fake' + } + ) + self.mock_object( + shell_v2, '_wait_for_resource_status', + mock.Mock(return_value=share_to_be_managed) + ) + if version is not None: self.run_command(version + ' manage fake_service fake_protocol ' @@ -761,8 +774,16 @@ class ShellTest(test_utils.TestCase): } } expected['share'].update(valid_params) + self.assert_called('POST', '/shares/manage', body=expected) + if '--wait' in cmd_args: + shell_v2._wait_for_resource_status.assert_called_once_with( + self.shell.cs, share_to_be_managed, resource_type='share', + expected_status='available') + else: + shell_v2._wait_for_resource_status.assert_not_called() + def test_manage_invalid_param_share_server_id(self): self.assertRaises( exceptions.CommandError, @@ -903,9 +924,40 @@ class ShellTest(test_utils.TestCase): expected = {'reset_status': {'status': status}} self.assert_called('POST', '/share-servers/1234/action', body=expected) - def test_unmanage(self): - self.run_command('unmanage 1234') - self.assert_called('POST', '/shares/1234/action') + @ddt.data('--wait', '') + def test_unmanage(self, wait_option): + version = api_versions.APIVersion('2.46') + api = mock.Mock(api_version=version) + manager = shares.ShareManager(api=api) + fake_share = shares.Share( + manager, { + 'id': 'xyzzyspoon', + 'api_version': version, + 'status': 'available', + } + ) + share_not_found_error = ("ERROR: No share with " + "a name or ID of '%s' exists.") + share_not_found_error = exceptions.CommandError( + share_not_found_error % (fake_share.id) + ) + self.mock_object( + shell_v2, '_find_share', + mock.Mock(side_effect=([fake_share, fake_share, fake_share, + share_not_found_error]))) + self.mock_object( + shares.ShareManager, 'get', + mock.Mock(return_value=fake_share)) + + self.run_command('unmanage %s xyzzyspoon' % wait_option) + + expected_get_share_calls = 4 if wait_option else 1 + shell_v2._find_share.assert_has_calls( + [mock.call(self.shell.cs, fake_share.id)] * + expected_get_share_calls + ) + uri = '/shares/%s/action' % fake_share.id + api.client.post.assert_called_once_with(uri, body={'unmanage': None}) def test_share_server_unmanage(self): self.run_command('share-server-unmanage 1234') diff --git a/manilaclient/v2/shell.py b/manilaclient/v2/shell.py index 4eb232e76..ede7b04f9 100644 --- a/manilaclient/v2/shell.py +++ b/manilaclient/v2/shell.py @@ -79,6 +79,8 @@ def _wait_for_resource_status(cs, "state.") deleted_message = ("%(resource_type)s %(resource)s has been successfully " "deleted.") + unmanaged_message = ("%(resource_type)s %(resource)s has been " + "successfully unmanaged.") message_payload = { 'resource_type': resource_type.capitalize(), 'resource': resource.id, @@ -94,10 +96,13 @@ def _wait_for_resource_status(cs, try: resource = find_resource[resource_type](cs, resource.id) except exceptions.CommandError as e: - if (re.search(not_found_regex, str(e), flags=re.IGNORECASE) - and 'deleted' in expected_status): - print(deleted_message % message_payload) - break + if (re.search(not_found_regex, str(e), flags=re.IGNORECASE)): + if 'deleted' in expected_status: + print(deleted_message % message_payload) + break + if 'unmanaged' in expected_status: + print(unmanaged_message % message_payload) + break else: raise e @@ -1523,6 +1528,10 @@ def do_share_export_location_show(cs, args): help="Share server associated with share when using a share type with " "'driver_handles_share_servers' extra_spec set to True. Available " "only for microversion >= 2.49. (Default=None)") +@cliutils.arg( + '--wait', + action='store_true', + help='Wait for share management') def do_manage(cs, args): """Manage share not handled by Manila (Admin only).""" driver_options = _extract_key_value_options(args, 'driver_options') @@ -1545,6 +1554,11 @@ def do_manage(cs, args): name=args.name, description=args.description, is_public=args.public) + if args.wait: + share = _wait_for_resource_status( + cs, share, resource_type='share', + expected_status='available' + ) _print_share(cs, share) @@ -1682,10 +1696,16 @@ def do_snapshot_manage(cs, args): 'share', metavar='', help='Name or ID of the share(s).') +@cliutils.arg( + '--wait', + action='store_true', + help='Wait for share unmanagement') def do_unmanage(cs, args): """Unmanage share (Admin only).""" share_ref = _find_share(cs, args.share) share_ref.unmanage() + if args.wait: + _wait_for_share_status(cs, share_ref, expected_status='unmanaged') @api_versions.wraps("2.49") @@ -1775,7 +1795,6 @@ def do_delete(cs, args): """Remove one or more shares.""" failure_count = 0 shares_to_delete = [] - for share in args.share: try: share_ref = _find_share(cs, share) diff --git a/releasenotes/notes/bug-1898309-add-wait-to-share-manage-unmanage-d2060c61cc295bfd.yaml b/releasenotes/notes/bug-1898309-add-wait-to-share-manage-unmanage-d2060c61cc295bfd.yaml new file mode 100644 index 000000000..5cb5dc6ab --- /dev/null +++ b/releasenotes/notes/bug-1898309-add-wait-to-share-manage-unmanage-d2060c61cc295bfd.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + The commands "manila manage" and "manila unmanage" + now accept an optional "--wait" flag that allows + users to let the client poll for the completion + of the operation.