[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

@@ -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="<id>",
             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):

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(_(

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,

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):
 

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):