From 789e65bee3156d861c813f290e6f87153aaea3b3 Mon Sep 17 00:00:00 2001 From: tspyderboy Date: Tue, 26 Mar 2024 18:12:46 +0530 Subject: [PATCH] Add "--wait" option for creating/promoting/deleting a share replicas 1) Creating a share replica $ manila share-replica-create share1 --az manila-zone-1 --wait (The CLI does not wait for "replica_state" to become "in_sync" - it only waits until the replica reaches an "available" status) 2) Promoting a share replica $ manila share-replica-promote 9b6b909b-3790-4a65-a89d-f9437496f171 --wait 3) Deleting a share replica $ manila share-replica-delete 9b6b909b-3790-4a65-a89d-f9437496f171 --wait Closes-Bug: #1898310 Change-Id: If269c708c894756c0223e3bfa173670bcc6ef763 --- manilaclient/osc/v2/share_replicas.py | 15 +++++ .../tests/unit/osc/v2/test_share_replicas.py | 28 +++++++++- manilaclient/tests/unit/v2/test_shell.py | 55 +++++++++++++++++++ manilaclient/v2/shell.py | 29 ++++++++++ ...delete-share-replica-35f17e4a51b8f62e.yaml | 6 ++ 5 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/bug-1898310-add-wait-to-create-promote-delete-share-replica-35f17e4a51b8f62e.yaml diff --git a/manilaclient/osc/v2/share_replicas.py b/manilaclient/osc/v2/share_replicas.py index 522501722..5b0d01e73 100644 --- a/manilaclient/osc/v2/share_replicas.py +++ b/manilaclient/osc/v2/share_replicas.py @@ -356,6 +356,12 @@ class PromoteShareReplica(command.Command): help=_('Quiesce wait time in seconds. Available for ' 'microversion >= 2.75') ) + parser.add_argument( + '--wait', + action='store_true', + default=False, + help=_('Wait for share replica promotion') + ) return parser def take_action(self, parsed_args): @@ -377,6 +383,15 @@ class PromoteShareReplica(command.Command): try: share_client.share_replicas.promote(*args) + if parsed_args.wait: + if not osc_utils.wait_for_status( + status_f=share_client.share_replicas.get, + res_id=replica.id, + success_status=['active'], + status_field='replica_state' + ): + LOG.error(_("ERROR: Share replica is in error state.")) + except Exception as e: raise exceptions.CommandError(_( "Failed to promote replica to 'active': %s" % (e))) diff --git a/manilaclient/tests/unit/osc/v2/test_share_replicas.py b/manilaclient/tests/unit/osc/v2/test_share_replicas.py index 1b250dc94..7da4b2c4f 100644 --- a/manilaclient/tests/unit/osc/v2/test_share_replicas.py +++ b/manilaclient/tests/unit/osc/v2/test_share_replicas.py @@ -617,7 +617,10 @@ class TestShareReplicaPromote(TestShareReplica): super(TestShareReplicaPromote, self).setUp() self.share_replica = ( - manila_fakes.FakeShareReplica.create_one_replica() + manila_fakes.FakeShareReplica.create_one_replica( + attrs={ + 'status': 'available'} + ) ) self.replicas_mock.get.return_value = self.share_replica @@ -630,6 +633,7 @@ class TestShareReplicaPromote(TestShareReplica): ] verifylist = [ ('replica', self.share_replica.id), + ('wait', False) ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -658,6 +662,28 @@ class TestShareReplicaPromote(TestShareReplica): wait_time) self.assertIsNone(result) + @mock.patch.object(osc_share_replicas.osc_utils, 'wait_for_status', + mock.Mock()) + def test_share_replica_promote_wait(self): + arglist = [ + self.share_replica.id, + '--wait' + ] + verifylist = [ + ('replica', self.share_replica.id), + ('wait', True) + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + self.replicas_mock.promote.assert_called_with( + self.share_replica) + self.assertIsNone(result) + osc_share_replicas.osc_utils.wait_for_status.assert_called_once_with( + status_f=self.replicas_mock.get, res_id=self.share_replica.id, + success_status=['active'], status_field='replica_state') + def test_share_replica_promote_exception(self): arglist = [ self.share_replica.id, diff --git a/manilaclient/tests/unit/v2/test_shell.py b/manilaclient/tests/unit/v2/test_shell.py index 33a03480f..108012e40 100644 --- a/manilaclient/tests/unit/v2/test_shell.py +++ b/manilaclient/tests/unit/v2/test_shell.py @@ -3400,6 +3400,7 @@ class ShellTest(test_utils.TestCase): @ddt.data(True, False) @mock.patch.object(shell_v2, '_find_share_replica', mock.Mock()) + @mock.patch.object(shell_v2, '_wait_for_resource_status', mock.Mock()) def test_share_replica_delete_force(self, force): fake_replica = type('FakeShareReplica', (object,), {'id': '1234'}) @@ -3413,6 +3414,23 @@ class ShellTest(test_utils.TestCase): body={'force_delete': None}) else: self.assert_called('DELETE', '/share-replicas/1234') + self.assertEqual(0, shell_v2._wait_for_resource_status.call_count) + + @mock.patch.object(shell_v2, '_find_share_replica', mock.Mock()) + @mock.patch.object(shell_v2, '_wait_for_resource_status', mock.Mock()) + def test_share_replica_delete_with_wait(self): + + fake_replica = type('FakeShareReplica', (object,), {'id': '1234'}) + shell_v2._find_share_replica.return_value = fake_replica + + self.run_command('share-replica-delete fake-replica --wait') + + self.assert_called('DELETE', '/share-replicas/1234') + + # _wait_for_resource_status should be triggered once + shell_v2._wait_for_resource_status.assert_called_once_with( + self.shell.cs, mock.ANY, resource_type='share_replica', + expected_status='deleted') @ddt.data([1, 0], [1, 1], [2, 0], [2, 1], [2, 2]) @ddt.unpack @@ -3484,6 +3502,7 @@ class ShellTest(test_utils.TestCase): 'fake-share-id --availability-zone fake-az', ) @mock.patch.object(shell_v2, '_find_share', mock.Mock()) + @mock.patch.object(shell_v2, '_wait_for_resource_status', mock.Mock()) def test_share_replica_create(self, data): fshare = type('FakeShare', (object,), {'id': 'fake-share-id'}) @@ -3495,6 +3514,26 @@ class ShellTest(test_utils.TestCase): shell_v2._find_share.assert_called_with(mock.ANY, fshare.id) self.assert_called('POST', '/share-replicas') + 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_share_replica_create_with_wait(self): + + fshare = type('FakeShare', (object,), {'id': 'fake-share-id'}) + shell_v2._find_share.return_value = fshare + + cmd = ('share-replica-create fake-share-id ' + '--availability-zone fake-az --wait') + + self.run_command(cmd) + + shell_v2._find_share.assert_called_with(mock.ANY, fshare.id) + self.assert_called('POST', '/share-replicas') + # _wait_for_resource_status should be triggered once + shell_v2._wait_for_resource_status.assert_called_once_with( + self.shell.cs, mock.ANY, resource_type='share_replica', + expected_status='available') def test_share_replica_show(self): @@ -3503,6 +3542,7 @@ class ShellTest(test_utils.TestCase): self.assert_called_anytime('GET', '/share-replicas/5678') @mock.patch.object(shell_v2, '_find_share_replica', mock.Mock()) + @mock.patch.object(shell_v2, '_wait_for_resource_status', mock.Mock()) def test_share_replica_promote_quiesce_wait_time(self): fake_replica = type('FakeShareReplica', (object,), {'id': '1234'}) shell_v2._find_share_replica.return_value = fake_replica @@ -3512,6 +3552,21 @@ class ShellTest(test_utils.TestCase): self.assert_called( 'POST', '/share-replicas/1234/action', body={'promote': {'quiesce_wait_time': '5'}}) + self.assertEqual(0, shell_v2._wait_for_resource_status.call_count) + + @mock.patch.object(shell_v2, '_find_share_replica', mock.Mock()) + @mock.patch.object(shell_v2, '_wait_for_resource_status', mock.Mock()) + def test_share_replica_promote_with_wait_(self): + fake_replica = type('FakeShareReplica', (object,), {'id': '1234'}) + shell_v2._find_share_replica.return_value = fake_replica + cmd = ('share-replica-promote ' + fake_replica.id + ' --wait') + self.run_command(cmd) + self.assert_called( + 'POST', '/share-replicas/1234/action') + # _wait_for_resource_status should be triggered once + shell_v2._wait_for_resource_status.assert_called_once_with( + self.shell.cs, mock.ANY, resource_type='share_replica', + expected_status='active', status_attr='replica_state') @ddt.data('promote', 'resync') @mock.patch.object(shell_v2, '_find_share_replica', mock.Mock()) diff --git a/manilaclient/v2/shell.py b/manilaclient/v2/shell.py index d00ab0b75..84d0ba57a 100644 --- a/manilaclient/v2/shell.py +++ b/manilaclient/v2/shell.py @@ -6416,6 +6416,11 @@ def do_share_replica_create(cs, args): # noqa action='single_alias', help='Optional network info ID or name. ' 'Available only for microversion >= 2.72') +@cliutils.arg( + '--wait', + action='store_true', + default=False, + help='Wait for share replica to be created') def do_share_replica_create(cs, args): # noqa """Create a share replica.""" share = _find_share(cs, args.share) @@ -6444,6 +6449,10 @@ def do_share_replica_create(cs, args): # noqa body['share_network'] = share_network replica = cs.share_replicas.create(**body) + if args.wait: + _wait_for_resource_status( + cs, replica, resource_type='share_replica', + expected_status='available') _print_share_replica(cs, replica) @@ -6485,6 +6494,11 @@ def do_share_replica_show(cs, args): # noqa help='Attempt to force deletion of a replica on its backend. Using ' 'this option will purge the replica from Manila even if it ' 'is not cleaned up on the backend. Defaults to False.') +@cliutils.arg( + '--wait', + action='store_true', + default=False, + help='Wait for share replica to be deleted') @api_versions.wraps("2.11") def do_share_replica_delete(cs, args): """Remove one or more share replicas.""" @@ -6497,6 +6511,10 @@ def do_share_replica_delete(cs, args): try: replica_ref = _find_share_replica(cs, replica) cs.share_replicas.delete(replica_ref, **kwargs) + if args.wait: + _wait_for_resource_status( + cs, replica_ref, resource_type='share_replica', + expected_status='deleted') except Exception as e: failure_count += 1 print("Delete for share replica %s failed: %s" % (replica, e), @@ -6528,6 +6546,11 @@ def do_share_replica_promote(cs, args): default=None, help='Quiesce wait time in seconds. Available for ' 'microversion >= 2.75') +@cliutils.arg( + '--wait', + action='store_true', + default=False, + help='Wait for share replica to be promoted') @api_versions.wraps("2.75") # noqa def do_share_replica_promote(cs, args): # noqa """Promote specified replica to 'active' replica_state.""" @@ -6537,6 +6560,12 @@ def do_share_replica_promote(cs, args): # noqa if args.quiesce_wait_time: quiesce_wait_time = args.quiesce_wait_time cs.share_replicas.promote(replica, quiesce_wait_time) + if args.wait: + _wait_for_resource_status( + cs, replica, + resource_type='share_replica', + expected_status='active', + status_attr='replica_state') @api_versions.wraps("2.47") diff --git a/releasenotes/notes/bug-1898310-add-wait-to-create-promote-delete-share-replica-35f17e4a51b8f62e.yaml b/releasenotes/notes/bug-1898310-add-wait-to-create-promote-delete-share-replica-35f17e4a51b8f62e.yaml new file mode 100644 index 000000000..e41d4609f --- /dev/null +++ b/releasenotes/notes/bug-1898310-add-wait-to-create-promote-delete-share-replica-35f17e4a51b8f62e.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + The commands "share-replica-create", "share-replica-promote", and + "share-replica-delete" 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