From ea092b29880e71f0ba2d8e1eb93a9cf73edee2a2 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 8 Apr 2020 09:57:41 +0100 Subject: Make 'server list --config-drive' a boolean option Instead of passing through whatever the user provides and exposing this bug in the REST API, simply make the opt a boolean one in expectation of a day where the API issues have been resolved. This also introduces machinery necessary to use more of these types of opts in the future. Change-Id: I9033540ac65ac0ee7337f16bdd002060652092ea Signed-off-by: Stephen Finucane --- doc/source/cli/nova.rst | 13 +++++++----- novaclient/shell.py | 23 ++++++++++++++++------ novaclient/tests/unit/v2/test_shell.py | 21 ++++++++++---------- novaclient/v2/servers.py | 9 ++++----- novaclient/v2/shell.py | 19 +++++++++++------- .../add-filter-to-nova-list-831dcbb34420fb29.yaml | 1 + 6 files changed, 52 insertions(+), 34 deletions(-) diff --git a/doc/source/cli/nova.rst b/doc/source/cli/nova.rst index 48b1dbda..26c2ce2a 100644 --- a/doc/source/cli/nova.rst +++ b/doc/source/cli/nova.rst @@ -2224,7 +2224,7 @@ nova list [--fields ] [--minimal] [--sort [:]] [--marker ] [--limit ] [--availability-zone ] - [--key-name ] [--config-drive ] + [--key-name ] [--[no-]config-drive] [--progress ] [--vm-state ] [--task-state ] [--power-state ] [--changes-since ] @@ -2316,10 +2316,13 @@ present in the failure domain. Display servers based on their keypair name (Admin only until microversion 2.82). -``--config-drive `` - Display servers based on their config_drive value - The value must be a boolean. (Admin only until - microversion 2.82). +``--config-drive`` + Display servers that have a config drive attached. + (Admin only until microversion 2.82). + +``--no-config-drive`` + Display servers that do not have a config drive attached. + (Admin only until microversion 2.82). ``--progress `` Display servers based on their progress value diff --git a/novaclient/shell.py b/novaclient/shell.py index a5d1e937..22952b7c 100644 --- a/novaclient/shell.py +++ b/novaclient/shell.py @@ -442,6 +442,7 @@ class OpenStackComputeShell(object): action_help = desc.strip() arguments = getattr(callback, 'arguments', []) + groups = {} subparser = subparsers.add_parser( command, @@ -456,10 +457,14 @@ class OpenStackComputeShell(object): ) self.subcommands[command] = subparser for (args, kwargs) in arguments: - start_version = kwargs.get("start_version", None) + kwargs = kwargs.copy() + + start_version = kwargs.pop("start_version", None) + end_version = kwargs.pop("end_version", None) + group = kwargs.pop("group", None) + if start_version: start_version = api_versions.APIVersion(start_version) - end_version = kwargs.get("end_version", None) if end_version: end_version = api_versions.APIVersion(end_version) else: @@ -471,10 +476,16 @@ class OpenStackComputeShell(object): "end": end_version.get_string()}) if not version.matches(start_version, end_version): continue - kw = kwargs.copy() - kw.pop("start_version", None) - kw.pop("end_version", None) - subparser.add_argument(*args, **kw) + + if group: + if group not in groups: + groups[group] = ( + subparser.add_mutually_exclusive_group() + ) + kwargs['dest'] = kwargs.get('dest', group) + groups[group].add_argument(*args, **kwargs) + else: + subparser.add_argument(*args, **kwargs) subparser.set_defaults(func=callback) def setup_debugging(self, debug): diff --git a/novaclient/tests/unit/v2/test_shell.py b/novaclient/tests/unit/v2/test_shell.py index f00f583e..18badc0b 100644 --- a/novaclient/tests/unit/v2/test_shell.py +++ b/novaclient/tests/unit/v2/test_shell.py @@ -1846,18 +1846,17 @@ class ShellTest(utils.TestCase): self.run_command('list --key-name my_key') self.assert_called('GET', '/servers/detail?key_name=my_key') - def test_list_with_config_drive_passing_through_any_value(self): - self.run_command('list --config-drive True') + def test_list_with_config_drive(self): + self.run_command('list --config-drive') self.assert_called('GET', '/servers/detail?config_drive=True') - self.run_command('list --config-drive some-random-string') - self.assert_called( - 'GET', '/servers/detail?config_drive=some-random-string') - # This form is special for the test env to pass through an empty string - # as a parameter. The real CLI call would look like - # list --config drive '' - self.run_command(['list', '--config-drive', '']) - self.assert_called( - 'GET', '/servers/detail?config_drive=') + + def test_list_with_no_config_drive(self): + self.run_command('list --no-config-drive') + self.assert_called('GET', '/servers/detail?config_drive=False') + + def test_list_with_conflicting_config_drive(self): + self.assertRaises(SystemExit, self.run_command, + 'list --config-drive --no-config-drive') def test_list_with_progress(self): self.run_command('list --progress 100') diff --git a/novaclient/v2/servers.py b/novaclient/v2/servers.py index 42184345..7768ef7a 100644 --- a/novaclient/v2/servers.py +++ b/novaclient/v2/servers.py @@ -893,12 +893,11 @@ class ServerManager(base.BootingManagerWithFind): if isinstance(val, str): val = val.encode('utf-8') qparams[opt] = val - # NOTE(gibi): After we fixing bug 1871409 and cleaning up the API - # inconsistency around config_drive we can make the - # config_drive filter a boolean filter. But until that we - # simply pass through any value to the API even empty string. + # NOTE(gibi): The False value won't actually do anything until we + # fix bug 1871409 and clean up the API inconsistency, but we do it + # in preparation for that (hopefully backportable) fix if opt == 'config_drive' and val is not None: - qparams[opt] = val + qparams[opt] = str(val) detail = "" if detailed: diff --git a/novaclient/v2/shell.py b/novaclient/v2/shell.py index 8dafc164..ac6c5ea1 100644 --- a/novaclient/v2/shell.py +++ b/novaclient/v2/shell.py @@ -1544,16 +1544,21 @@ def _print_flavor(flavor): default=None, help=_('Display servers based on their keypair name (Admin only until ' 'microversion 2.82).')) -# NOTE(gibi): we can make this a real boolean filter after bug 1871409 is fixed -# and the REST API is cleaned up regarding the values of config_drive. Unit -# that we simply pass through any string from the user to the REST API. @utils.arg( '--config-drive', - dest='config_drive', - metavar='', + action='store_true', + group='config_drive', default=None, - help=_('Display servers based on their config_drive value (Admin only ' - 'until microversion 2.82). The value must be a boolean value.')) + help=_('Display servers that have a config drive attached. (Admin only ' + 'until microversion 2.82).')) +# 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 +@utils.arg( + '--no-config-drive', + action='store_false', + group='config_drive', + help=_('Display servers that do not have a config drive attached (Admin ' + 'only until microversion 2.82)')) @utils.arg( '--progress', dest='progress', diff --git a/releasenotes/notes/add-filter-to-nova-list-831dcbb34420fb29.yaml b/releasenotes/notes/add-filter-to-nova-list-831dcbb34420fb29.yaml index e91a6080..77acb757 100644 --- a/releasenotes/notes/add-filter-to-nova-list-831dcbb34420fb29.yaml +++ b/releasenotes/notes/add-filter-to-nova-list-831dcbb34420fb29.yaml @@ -6,6 +6,7 @@ * --availability-zone * --config-drive + * --no-config-drive * --key-name * --power-state * --task-state -- cgit v1.2.1