Merge "compute: Add missing options for 'server list'"

2 files changed, 345 insertions, 12 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 6cb364fe..c49c1815 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -1346,19 +1346,19 @@ class DeleteServer(command.Command):
                     raise SystemExit
 
 
+def percent_type(x):
+    x = int(x)
+    if not 0 < x <= 100:
+        raise argparse.ArgumentTypeError("Must be between 0 and 100")
+    return x
+
+
 class ListServer(command.Lister):
     _description = _("List servers")
 
     def get_parser(self, prog_name):
         parser = super(ListServer, self).get_parser(prog_name)
         parser.add_argument(
-            '--availability-zone',
-            metavar='<availability-zone>',
-            help=_('Only return instances that match the availability zone. '
-                   'Note that this option will be ignored for non-admin users '
-                   'when using ``--os-compute-api-version`` prior to 2.83.'),
-        )
-        parser.add_argument(
             '--reservation-id',
             metavar='<reservation-id>',
             help=_('Only return instances that match the reservation'),
@@ -1387,10 +1387,35 @@ class ListServer(command.Lister):
             metavar='<server-name>',
             help=_('Regular expression to match instance name (admin only)'),
         )
+        # taken from 'task_and_vm_state_from_status' function in nova
+        # the API sadly reports these in upper case and while it would be
+        # wonderful to plaster over this ugliness client-side, there are
+        # already users in the wild doing this in upper case that we need to
+        # support
         parser.add_argument(
             '--status',
             metavar='<status>',
-            # FIXME(dhellmann): Add choices?
+            choices=(
+                'ACTIVE',
+                'BUILD',
+                'DELETED',
+                'ERROR',
+                'HARD_REBOOT',
+                'MIGRATING',
+                'PASSWORD',
+                'PAUSED',
+                'REBOOT',
+                'REBUILD',
+                'RESCUE',
+                'RESIZE',
+                'REVERT_RESIZE',
+                'SHELVED',
+                'SHELVED_OFFLOADED',
+                'SHUTOFF',
+                'SOFT_DELETED',
+                'SUSPENDED',
+                'VERIFY_RESIZE'
+            ),
             help=_('Search by server status'),
         )
         parser.add_argument(
@@ -1423,7 +1448,10 @@ class ListServer(command.Lister):
         parser.add_argument(
             '--user',
             metavar='<user>',
-            help=_('Search by user (admin only) (name or ID)'),
+            help=_(
+                'Search by user (name or ID) '
+                '(admin only before microversion 2.83)'
+            ),
         )
         identity_common.add_user_domain_option_to_parser(parser)
         parser.add_argument(
@@ -1433,6 +1461,146 @@ class ListServer(command.Lister):
             help=_('Only display deleted servers (admin only)'),
         )
         parser.add_argument(
+            '--availability-zone',
+            default=None,
+            help=_(
+                'Search by availability zone '
+                '(admin only before microversion 2.83)'
+            ),
+        )
+        parser.add_argument(
+            '--key-name',
+            help=_(
+                'Search by keypair name '
+                '(admin only before microversion 2.83)'
+            ),
+        )
+        config_drive_group = parser.add_mutually_exclusive_group()
+        config_drive_group.add_argument(
+            '--config-drive',
+            action='store_true',
+            dest='has_config_drive',
+            default=None,
+            help=_(
+                'Only display servers with a config drive attached '
+                '(admin only before microversion 2.83)'
+            ),
+        )
+        # NOTE(gibi): this won't actually do anything until bug 1871409 is
+        # fixed and the REST API is cleaned up regarding the values of
+        # config_drive
+        config_drive_group.add_argument(
+            '--no-config-drive',
+            action='store_false',
+            dest='has_config_drive',
+            help=_(
+                'Only display servers without a config drive attached '
+                '(admin only before microversion 2.83)'
+            ),
+        )
+        parser.add_argument(
+            '--progress',
+            type=percent_type,
+            default=None,
+            help=_(
+                'Search by progress value (%%) '
+                '(admin only before microversion 2.83)'
+            ),
+        )
+        parser.add_argument(
+            '--vm-state',
+            metavar='<state>',
+            # taken from 'InstanceState' object field in nova
+            choices=(
+                'active',
+                'building',
+                'deleted',
+                'error',
+                'paused',
+                'stopped',
+                'suspended',
+                'rescued',
+                'resized',
+                'shelved',
+                'shelved_offloaded',
+                'soft-delete',
+            ),
+            help=_(
+                'Search by vm_state value '
+                '(admin only before microversion 2.83)'
+            ),
+        )
+        parser.add_argument(
+            '--task-state',
+            metavar='<state>',
+            # taken from 'InstanceTaskState' object field in nova
+            choices=(
+                'block_device_mapping',
+                'deleting',
+                'image_backup',
+                'image_pending_upload',
+                'image_snapshot',
+                'image_snapshot_pending',
+                'image_uploading',
+                'migrating',
+                'networking',
+                'pausing',
+                'powering-off',
+                'powering-on',
+                'rebooting',
+                'reboot_pending',
+                'reboot_started',
+                'reboot_pending_hard',
+                'reboot_started_hard',
+                'rebooting_hard',
+                'rebuilding',
+                'rebuild_block_device_mapping',
+                'rebuild_spawning',
+                'rescuing',
+                'resize_confirming',
+                'resize_finish',
+                'resize_migrated',
+                'resize_migrating',
+                'resize_prep',
+                'resize_reverting',
+                'restoring',
+                'resuming',
+                'scheduling',
+                'shelving',
+                'shelving_image_pending_upload',
+                'shelving_image_uploading',
+                'shelving_offloading',
+                'soft-deleting',
+                'spawning',
+                'suspending',
+                'updating_password',
+                'unpausing',
+                'unrescuing',
+                'unshelving',
+            ),
+            help=_(
+                'Search by task_state value '
+                '(admin only before microversion 2.83)'
+            ),
+        )
+        parser.add_argument(
+            '--power-state',
+            metavar='<state>',
+            # taken from 'InstancePowerState' object field in nova
+            choices=(
+                'pending',
+                'running',
+                'paused',
+                'shutdown',
+                'crashed',
+                'suspended',
+            ),
+            help=_(
+                'Search by power_state value '
+                '(admin only before microversion 2.83)'
+            ),
+        )
+        parser.add_argument(
             '--long',
             action='store_true',
             default=False,
@@ -1584,7 +1752,6 @@ class ListServer(command.Lister):
                                                ignore_missing=False).id
 
         search_opts = {
-            'availability_zone': parsed_args.availability_zone,
             'reservation_id': parsed_args.reservation_id,
             'ip': parsed_args.ip,
             'ip6': parsed_args.ip6,
@@ -1602,6 +1769,36 @@ class ListServer(command.Lister):
             'changes-since': parsed_args.changes_since,
         }
 
+        if parsed_args.availability_zone:
+            search_opts['availability_zone'] = parsed_args.availability_zone
+
+        if parsed_args.key_name:
+            search_opts['key_name'] = parsed_args.key_name
+
+        if parsed_args.has_config_drive is not None:
+            search_opts['config_drive'] = parsed_args.has_config_drive
+
+        if parsed_args.progress is not None:
+            search_opts['progress'] = str(parsed_args.progress)
+
+        if parsed_args.vm_state:
+            search_opts['vm_state'] = parsed_args.vm_state
+
+        if parsed_args.task_state:
+            search_opts['task_state'] = parsed_args.task_state
+
+        if parsed_args.power_state:
+            # taken from 'InstancePowerState' object field in nova
+            power_state = {
+                'pending': 0,
+                'running': 1,
+                'paused': 3,
+                'shutdown': 4,
+                'crashed': 6,
+                'suspended': 7,
+            }[parsed_args.power_state]
+            search_opts['power_state'] = power_state
+
         if parsed_args.tags:
             if compute_client.api_version < api_versions.APIVersion('2.26'):
                 msg = _(
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 99f7eede..f046925a 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -2986,7 +2986,6 @@ class TestServerList(TestServer):
         super(TestServerList, self).setUp()
 
         self.search_opts = {
-            'availability_zone': None,
             'reservation_id': None,
             'ip': None,
             'ip6': None,
@@ -3431,7 +3430,7 @@ class TestServerList(TestServer):
             'Invalid time value'
         )
 
-    def test_server_with_changes_before_older_version(self):
+    def test_server_with_changes_before_pre_v266(self):
         self.app.client_manager.compute.api_version = (
             api_versions.APIVersion('2.65'))
 
@@ -3587,6 +3586,143 @@ class TestServerList(TestServer):
             '--os-compute-api-version 2.26 or greater is required',
             str(ex))
 
+    def test_server_list_with_availability_zone(self):
+        arglist = [
+            '--availability-zone', 'test-az',
+        ]
+        verifylist = [
+            ('availability_zone', 'test-az'),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.search_opts['availability_zone'] = 'test-az'
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
+    def test_server_list_with_key_name(self):
+        arglist = [
+            '--key-name', 'test-key',
+        ]
+        verifylist = [
+            ('key_name', 'test-key'),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.search_opts['key_name'] = 'test-key'
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
+    def test_server_list_with_config_drive(self):
+        arglist = [
+            '--config-drive',
+        ]
+        verifylist = [
+            ('has_config_drive', True),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.search_opts['config_drive'] = True
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
+    def test_server_list_with_no_config_drive(self):
+        arglist = [
+            '--no-config-drive',
+        ]
+        verifylist = [
+            ('has_config_drive', False),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.search_opts['config_drive'] = False
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
+    def test_server_list_with_progress(self):
+        arglist = [
+            '--progress', '100',
+        ]
+        verifylist = [
+            ('progress', 100),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.search_opts['progress'] = '100'
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
+    def test_server_list_with_progress_invalid(self):
+        arglist = [
+            '--progress', '101',
+        ]
+
+        self.assertRaises(
+            utils.ParserException,
+            self.check_parser, self.cmd, arglist, verify_args=[])
+
+    def test_server_list_with_vm_state(self):
+        arglist = [
+            '--vm-state', 'active',
+        ]
+        verifylist = [
+            ('vm_state', 'active'),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.search_opts['vm_state'] = 'active'
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
+    def test_server_list_with_task_state(self):
+        arglist = [
+            '--task-state', 'deleting',
+        ]
+        verifylist = [
+            ('task_state', 'deleting'),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.search_opts['task_state'] = 'deleting'
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
+    def test_server_list_with_power_state(self):
+        arglist = [
+            '--power-state', 'running',
+        ]
+        verifylist = [
+            ('power_state', 'running'),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.search_opts['power_state'] = 1
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
 
 class TestServerLock(TestServer):