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 <stephenfin@redhat.com>

doc/source/cli/nova.rst

@@ -2224,7 +2224,7 @@ nova list
                     [--fields <fields>] [--minimal]
                     [--sort <key>[:<direction>]] [--marker <marker>]
                     [--limit <limit>] [--availability-zone <availability_zone>]
-                    [--key-name <key_name>] [--config-drive <config_drive>]
+                    [--key-name <key_name>] [--[no-]config-drive]
                     [--progress <progress>] [--vm-state <vm_state>]
                     [--task-state <task_state>] [--power-state <power_state>]
                     [--changes-since <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 <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 <progress>``
   Display servers based on their progress value

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

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

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:

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='<config_drive>',
+    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',

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