From d0112a801a20c810d92dfcdf1f8fb71df3bd1d26 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 18 Nov 2020 15:11:59 +0000 Subject: [PATCH] compute: Add missing options for 'server list' This accepts a large number of options that we weren't exposing. Add the following options: '--availability-zone', '--key-name', '--config-drive' and '--no-config-drive', '--progress', '--vm-state', '--task-state' and '--power-state'. In addition, refine the 'openstack server list --status' parameter to restrict users to the actual choices supported by the server. Change-Id: Ieeb1f22df7092e66a411b6a36eafb3e16efc2fc2 Signed-off-by: Stephen Finucane --- openstackclient/compute/v2/server.py | 217 +++++++++++++++++- .../tests/unit/compute/v2/test_server.py | 140 ++++++++++- ...ing-server-list-opts-c41e97e86ff1e1ca.yaml | 16 ++ 3 files changed, 361 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/add-missing-server-list-opts-c41e97e86ff1e1ca.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index ba0243ef3e..2d6a4b1849 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -1344,18 +1344,18 @@ 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='', - 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='', @@ -1385,10 +1385,35 @@ class ListServer(command.Lister): metavar='', 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='', - # 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( @@ -1421,7 +1446,10 @@ class ListServer(command.Lister): parser.add_argument( '--user', metavar='', - 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( @@ -1430,6 +1458,146 @@ class ListServer(command.Lister): default=False, 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='', + # 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='', + # 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='', + # 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', @@ -1582,7 +1750,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, @@ -1600,6 +1767,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 efc105e565..0eeb2cec23 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): diff --git a/releasenotes/notes/add-missing-server-list-opts-c41e97e86ff1e1ca.yaml b/releasenotes/notes/add-missing-server-list-opts-c41e97e86ff1e1ca.yaml new file mode 100644 index 0000000000..f536eb9ea1 --- /dev/null +++ b/releasenotes/notes/add-missing-server-list-opts-c41e97e86ff1e1ca.yaml @@ -0,0 +1,16 @@ +--- +features: + - | + Add a number of additional options to the ``server list`` command: + + - ``--availability-zone`` + - ``--key-name`` + - ``--config-drive``, ``--no-config-drive`` + - ``--progress`` + - ``--vm-state`` + - ``--task-state`` + - ``--power-state`` +upgrade: + - | + The ``openstack server list --status`` parameter will now validate the + requested status.