From e5e815bf09f2254cff24e6925b6f399af859332a Mon Sep 17 00:00:00 2001 From: Maari Tamm Date: Thu, 11 Feb 2021 18:41:13 +0000 Subject: [PATCH] [OSC] Add missing waiters We decided to start adding the optional '--wait' argument to OSC commands after we had some commands already implemented. This patch adds the missing waiters to commands that were originally implemented without, but should also include the option. Partially-implements: bp add-waiters Change-Id: Ib4f31747838d016da244b79d7d9be4aa8aff1a58 --- manilaclient/osc/v2/share_access_rules.py | 39 +++- manilaclient/osc/v2/share_snapshots.py | 56 +++++ manilaclient/tests/unit/osc/v2/fakes.py | 2 +- .../unit/osc/v2/test_share_access_rules.py | 103 +++++++++ .../tests/unit/osc/v2/test_share_snapshots.py | 199 +++++++++++++++++- 5 files changed, 393 insertions(+), 6 deletions(-) diff --git a/manilaclient/osc/v2/share_access_rules.py b/manilaclient/osc/v2/share_access_rules.py index 888fe71e3..bf72b08e2 100644 --- a/manilaclient/osc/v2/share_access_rules.py +++ b/manilaclient/osc/v2/share_access_rules.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +import logging + from osc_lib.cli import parseractions from osc_lib.command import command from osc_lib import exceptions @@ -33,6 +35,8 @@ ACCESS_RULE_ATTRIBUTES = [ 'properties' ] +LOG = logging.getLogger(__name__) + class ShareAccessAllow(command.ShowOne): """Create a new share access rule.""" @@ -75,6 +79,11 @@ class ShareAccessAllow(command.ShowOne): help=_('Share access level ("rw" and "ro" access levels ' 'are supported). Defaults to rw.') ) + parser.add_argument( + "--wait", + action='store_true', + help=_("Wait for share access rule creation.") + ) return parser def take_action(self, parsed_args): @@ -97,6 +106,17 @@ class ShareAccessAllow(command.ShowOne): access_level=parsed_args.access_level, metadata=properties ) + if parsed_args.wait: + if not oscutils.wait_for_status( + status_f=share_client.share_access_rules.get, + res_id=share_access_rule['id'], + status_field='state' + ): + LOG.error(_("ERROR: Share access rule is in error state.")) + + share_access_rule = oscutils.find_resource( + share_client.share_access_rules, + share_access_rule['id'])._info share_access_rule.update( { 'properties': utils.format_properties( @@ -128,6 +148,12 @@ class ShareAccessDeny(command.Command): metavar="", help=_('ID of the access rule to be deleted.') ) + parser.add_argument( + "--wait", + action='store_true', + default=False, + help=_("Wait for share access rule deletion") + ) return parser def take_action(self, parsed_args): @@ -135,12 +161,21 @@ class ShareAccessDeny(command.Command): share = apiutils.find_resource(share_client.shares, parsed_args.share) + error = None try: share.deny(parsed_args.id) + if parsed_args.wait: + if not oscutils.wait_for_delete( + manager=share_client.share_access_rules, + res_id=parsed_args.id): + error = _("Failed to delete share access rule with ID: %s" + % parsed_args.id) except Exception as e: - raise exceptions.CommandError( + error = e + if error: + raise exceptions.CommandError(_( "Failed to delete share access rule for share " - "'%s': %s" % (share, e)) + "'%s': %s" % (share, error))) class ListShareAccess(command.Lister): diff --git a/manilaclient/osc/v2/share_snapshots.py b/manilaclient/osc/v2/share_snapshots.py index 5d346586c..112827bd2 100644 --- a/manilaclient/osc/v2/share_snapshots.py +++ b/manilaclient/osc/v2/share_snapshots.py @@ -55,6 +55,12 @@ class CreateShareSnapshot(command.ShowOne): default=None, help=_("Add a description to the snapshot (Optional).") ) + parser.add_argument( + '--wait', + action='store_true', + default=False, + help=_('Wait for share snapshot creation') + ) return parser def take_action(self, parsed_args): @@ -69,6 +75,17 @@ class CreateShareSnapshot(command.ShowOne): name=parsed_args.name or None, description=parsed_args.description or None ) + if parsed_args.wait: + if not utils.wait_for_status( + status_f=share_client.share_snapshots.get, + res_id=share_snapshot.id, + success_status=['available'] + ): + LOG.error(_("ERROR: Share snapshot is in error state.")) + + share_snapshot = utils.find_resource( + share_client.share_snapshots, + share_snapshot.id) share_snapshot._info.pop('links', None) return self.dict2columns(share_snapshot._info) @@ -93,6 +110,12 @@ class DeleteShareSnapshot(command.Command): default=False, help=_("Delete the snapshot(s) ignoring the current state.") ) + parser.add_argument( + "--wait", + action='store_true', + default=False, + help=_("Wait for share snapshot deletion") + ) return parser def take_action(self, parsed_args): @@ -110,6 +133,11 @@ class DeleteShareSnapshot(command.Command): else: share_client.share_snapshots.delete( snapshot_obj) + if parsed_args.wait: + if not utils.wait_for_delete( + manager=share_client.share_snapshots, + res_id=snapshot_obj.id): + result += 1 except Exception as e: result += 1 LOG.error(_( @@ -466,6 +494,11 @@ class AdoptShareSnapshot(command.ShowOne): "Set driver options as key=value pairs." "(repeat option to set multiple key=value pairs)") ) + parser.add_argument( + "--wait", + action='store_true', + help=_("Wait until share snapshot is adopted") + ) return parser def take_action(self, parsed_args): @@ -481,6 +514,19 @@ class AdoptShareSnapshot(command.ShowOne): name=parsed_args.name, description=parsed_args.description ) + + if parsed_args.wait: + if not utils.wait_for_status( + status_f=share_client.share_snapshots.get, + res_id=snapshot.id, + success_status=['available'], + error_status=['manage_error', 'error'] + ): + LOG.error(_("ERROR: Share snapshot is in error state.")) + + snapshot = utils.find_resource(share_client.share_snapshots, + snapshot.id) + snapshot._info.pop('links', None) return self.dict2columns(snapshot._info) @@ -499,6 +545,11 @@ class AbandonShareSnapshot(command.Command): nargs='+', help=_("Name or ID of the snapshot(s) to be abandoned.") ) + parser.add_argument( + "--wait", + action='store_true', + help=_("Wait until share snapshot is abandoned") + ) return parser def take_action(self, parsed_args): @@ -511,6 +562,11 @@ class AbandonShareSnapshot(command.Command): snapshot) try: share_client.share_snapshots.unmanage(snapshot_obj) + if parsed_args.wait: + if not utils.wait_for_delete( + manager=share_client.share_snapshots, + res_id=snapshot_obj.id): + result += 1 except Exception as e: result += 1 LOG.error(_( diff --git a/manilaclient/tests/unit/osc/v2/fakes.py b/manilaclient/tests/unit/osc/v2/fakes.py index be443a6a1..69583113e 100644 --- a/manilaclient/tests/unit/osc/v2/fakes.py +++ b/manilaclient/tests/unit/osc/v2/fakes.py @@ -373,7 +373,7 @@ class FakeShareAccessRule(object): 'access_level': 'rw', 'access_to': 'demo', 'access_type': 'user', - 'state': 'queued_to_apply', + 'state': 'active', 'access_key': None, 'created_at': datetime.datetime.now().isoformat(), 'updated_at': None, diff --git a/manilaclient/tests/unit/osc/v2/test_share_access_rules.py b/manilaclient/tests/unit/osc/v2/test_share_access_rules.py index fee205f1c..916119a25 100644 --- a/manilaclient/tests/unit/osc/v2/test_share_access_rules.py +++ b/manilaclient/tests/unit/osc/v2/test_share_access_rules.py @@ -10,6 +10,9 @@ # License for the specific language governing permissions and limitations # under the License. # +from unittest import mock + +from osc_lib import exceptions from osc_lib import utils as oscutils from manilaclient import api_versions @@ -58,6 +61,7 @@ class TestShareAccessCreate(TestShareAccess): manila_fakes.FakeShareAccessRule.create_one_access_rule( attrs={"share_id": self.share.id})) self.share.allow.return_value = self.access_rule._info + self.access_rules_mock.get.return_value = self.access_rule # Get the command object to test self.cmd = osc_share_access_rules.ShareAccessAllow(self.app, None) @@ -135,6 +139,64 @@ class TestShareAccessCreate(TestShareAccess): self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns) self.assertCountEqual(self.access_rule._info.values(), data) + def test_share_access_create_wait(self): + arglist = [ + self.share.id, + 'user', + 'demo', + '--wait' + ] + verifylist = [ + ("share", self.share.id), + ("access_type", "user"), + ("access_to", "demo"), + ("wait", True) + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + self.shares_mock.get.assert_called_with(self.share.id) + self.share.allow.assert_called_with( + access_type="user", + access="demo", + access_level=None, + metadata={} + ) + self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns) + self.assertCountEqual(self.access_rule._info.values(), data) + + @mock.patch('manilaclient.osc.v2.share_access_rules.LOG') + def test_share_access_create_wait_error(self, mock_logger): + arglist = [ + self.share.id, + 'user', + 'demo', + '--wait' + ] + verifylist = [ + ("share", self.share.id), + ("access_type", "user"), + ("access_to", "demo"), + ("wait", True) + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + with mock.patch('osc_lib.utils.wait_for_status', return_value=False): + columns, data = self.cmd.take_action(parsed_args) + + self.shares_mock.get.assert_called_with(self.share.id) + self.share.allow.assert_called_with( + access_type="user", + access="demo", + access_level=None, + metadata={} + ) + + mock_logger.error.assert_called_with( + "ERROR: Share access rule is in error state.") + + self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns) + self.assertCountEqual(self.access_rule._info.values(), data) + class TestShareAccessDelete(TestShareAccess): @@ -166,6 +228,47 @@ class TestShareAccessDelete(TestShareAccess): self.share.deny.assert_called_with(self.access_rule.id) self.assertIsNone(result) + def test_share_access_delete_wait(self): + arglist = [ + self.share.id, + self.access_rule.id, + '--wait' + ] + verifylist = [ + ("share", self.share.id), + ("id", self.access_rule.id), + ('wait', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + with mock.patch('osc_lib.utils.wait_for_delete', return_value=True): + result = self.cmd.take_action(parsed_args) + + self.shares_mock.get.assert_called_with(self.share.id) + self.share.deny.assert_called_with(self.access_rule.id) + self.assertIsNone(result) + + def test_share_access_delete_wait_error(self): + arglist = [ + self.share.id, + self.access_rule.id, + '--wait' + ] + verifylist = [ + ("share", self.share.id), + ("id", self.access_rule.id), + ('wait', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + with mock.patch('osc_lib.utils.wait_for_delete', return_value=False): + self.assertRaises( + exceptions.CommandError, + self.cmd.take_action, + parsed_args + ) + class TestShareAccessList(TestShareAccess): diff --git a/manilaclient/tests/unit/osc/v2/test_share_snapshots.py b/manilaclient/tests/unit/osc/v2/test_share_snapshots.py index 40565344f..a19ecce95 100644 --- a/manilaclient/tests/unit/osc/v2/test_share_snapshots.py +++ b/manilaclient/tests/unit/osc/v2/test_share_snapshots.py @@ -71,7 +71,10 @@ class TestShareSnapshotCreate(TestShareSnapshot): self.shares_mock.get.return_value = self.share self.share_snapshot = ( - manila_fakes.FakeShareSnapshot.create_one_snapshot()) + manila_fakes.FakeShareSnapshot.create_one_snapshot( + attrs={'status': 'available'} + )) + self.snapshots_mock.get.return_value = self.share_snapshot self.snapshots_mock.create.return_value = self.share_snapshot self.cmd = osc_share_snapshots.CreateShareSnapshot(self.app, None) @@ -158,6 +161,63 @@ class TestShareSnapshotCreate(TestShareSnapshot): self.assertCountEqual(self.columns, columns) self.assertCountEqual(self.data, data) + def test_share_snapshot_create_wait(self): + arglist = [ + self.share.id, + '--wait' + ] + verifylist = [ + ('share', self.share.id), + ('wait', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.snapshots_mock.create.assert_called_with( + share=self.share, + force=False, + name=None, + description=None + ) + + self.snapshots_mock.get.assert_called_with( + self.share_snapshot.id) + self.assertCountEqual(self.columns, columns) + self.assertCountEqual(self.data, data) + + @mock.patch('manilaclient.osc.v2.share_snapshots.LOG') + def test_share_snapshot_create_wait_error(self, mock_logger): + arglist = [ + self.share.id, + '--wait' + ] + verifylist = [ + ('share', self.share.id), + ('wait', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + with mock.patch('osc_lib.utils.wait_for_status', return_value=False): + columns, data = self.cmd.take_action(parsed_args) + + self.snapshots_mock.create.assert_called_with( + share=self.share, + force=False, + name=None, + description=None + ) + + mock_logger.error.assert_called_with( + "ERROR: Share snapshot is in error state.") + + self.snapshots_mock.get.assert_called_with( + self.share_snapshot.id) + self.assertCountEqual(self.columns, columns) + self.assertCountEqual(self.data, data) + class TestShareSnapshotDelete(TestShareSnapshot): @@ -245,6 +305,42 @@ class TestShareSnapshotDelete(TestShareSnapshot): self.cmd.take_action, parsed_args) + def test_share_snapshot_delete_wait(self): + arglist = [ + self.share_snapshot.id, + '--wait' + ] + verifylist = [ + ('snapshot', [self.share_snapshot.id]), + ('wait', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + with mock.patch('osc_lib.utils.wait_for_delete', return_value=True): + result = self.cmd.take_action(parsed_args) + + self.snapshots_mock.delete.assert_called_with(self.share_snapshot) + self.assertIsNone(result) + + def test_share_snapshot_delete_wait_error(self): + arglist = [ + self.share_snapshot.id, + '--wait' + ] + verifylist = [ + ('snapshot', [self.share_snapshot.id]), + ('wait', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + with mock.patch('osc_lib.utils.wait_for_delete', return_value=False): + self.assertRaises( + exceptions.CommandError, + self.cmd.take_action, + parsed_args + ) + class TestShareSnapshotShow(TestShareSnapshot): @@ -645,8 +741,13 @@ class TestShareSnapshotAdopt(TestShareSnapshot): self.shares_mock.get.return_value = self.share self.share_snapshot = ( - manila_fakes.FakeShareSnapshot.create_one_snapshot()) + manila_fakes.FakeShareSnapshot.create_one_snapshot( + attrs={ + 'status': 'available' + } + )) + self.snapshots_mock.get.return_value = self.share_snapshot self.export_location = ( manila_fakes.FakeShareExportLocation.create_one_export_location()) @@ -748,6 +849,60 @@ class TestShareSnapshotAdopt(TestShareSnapshot): self.assertCountEqual(self.columns, columns) self.assertCountEqual(self.data, data) + def test_snapshot_adopt_wait(self): + arglist = [ + self.share.id, + self.export_location.fake_path, + '--wait' + ] + verifylist = [ + ('share', self.share.id), + ('provider_location', self.export_location.fake_path), + ('wait', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + self.snapshots_mock.get.assert_called_with(self.share_snapshot.id) + self.snapshots_mock.manage.assert_called_with( + share=self.share, + provider_location=self.export_location.fake_path, + driver_options={}, + name=None, + description=None + ) + + self.assertCountEqual(self.columns, columns) + self.assertCountEqual(self.data, data) + + def test_snapshot_adopt_wait_error(self): + arglist = [ + self.share.id, + self.export_location.fake_path, + '--wait' + ] + verifylist = [ + ('share', self.share.id), + ('provider_location', self.export_location.fake_path), + ('wait', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + with mock.patch('osc_lib.utils.wait_for_status', return_value=False): + columns, data = self.cmd.take_action(parsed_args) + self.snapshots_mock.get.assert_called_with(self.share_snapshot.id) + self.snapshots_mock.manage.assert_called_with( + share=self.share, + provider_location=self.export_location.fake_path, + driver_options={}, + name=None, + description=None + ) + self.assertCountEqual(self.columns, columns) + self.assertCountEqual(self.data, data) + class TestShareSnapshotAbandon(TestShareSnapshot): @@ -755,7 +910,9 @@ class TestShareSnapshotAbandon(TestShareSnapshot): super(TestShareSnapshotAbandon, self).setUp() self.share_snapshot = ( - manila_fakes.FakeShareSnapshot.create_one_snapshot()) + manila_fakes.FakeShareSnapshot.create_one_snapshot( + attrs={'status': 'available'} + )) self.snapshots_mock.get.return_value = self.share_snapshot @@ -802,6 +959,42 @@ class TestShareSnapshotAbandon(TestShareSnapshot): len(share_snapshots)) self.assertIsNone(result) + def test_share_snapshot_abandon_wait(self): + arglist = [ + self.share_snapshot.id, + '--wait' + ] + verifylist = [ + ('snapshot', [self.share_snapshot.id]), + ('wait', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + with mock.patch('osc_lib.utils.wait_for_delete', return_value=True): + result = self.cmd.take_action(parsed_args) + self.snapshots_mock.unmanage.assert_called_with( + self.share_snapshot) + self.assertIsNone(result) + + def test_share_snapshot_abandon_wait_error(self): + arglist = [ + self.share_snapshot.id, + '--wait' + ] + verifylist = [ + ('snapshot', [self.share_snapshot.id]), + ('wait', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + with mock.patch('osc_lib.utils.wait_for_delete', return_value=False): + self.assertRaises( + exceptions.CommandError, + self.cmd.take_action, + parsed_args) + class TestShareSnapshotAccessAllow(TestShareSnapshot):