Add "start_version" and "end_version" support to argparse

Now, "cinder help subcommand" can not show whether an argument
is supported for a specific microversion.
With this change, developers only need to add a start_version or
end_version in the utils.arg wrap, cinderclient will support
the microversion for that arguement.

    @utils.arg(
    '--foo',
    start_version='3.1')
    @utils.arg(
    '--bar',
    start_version='3.2',
    end_version='3.5')
    def do_some_action():
        ......

In previous example, an exception will be raised for such command:
   $ cinder --os-volume-api-version 3.6 --bar some-ation

And only "--foo" will show up for such help command:
   $ cinder --os-volume-api-version 3.1 help some-ation

Change-Id: I74137486992846bbf9fdff53c009851db2356eef
Partial-Bug: #1600567
Co-Authored-By: Nate Potter <nathaniel.potter@intel.com>

cinderclient/exceptions.py

@@ -28,6 +28,34 @@ class UnsupportedVersion(Exception):
     pass
 
 
+class UnsupportedAttribute(AttributeError):
+    """Indicates that the user is trying to transmit the argument to a method,
+    which is not supported by selected version.
+    """
+
+    def __init__(self, argument_name, start_version, end_version):
+        if not start_version.is_null() and not end_version.is_null():
+            self.message = (
+                "'%(name)s' argument is only allowed for microversions "
+                "%(start)s - %(end)s." % {"name": argument_name,
+                                          "start": start_version.get_string(),
+                                          "end": end_version.get_string()})
+        elif not start_version.is_null():
+            self.message = (
+                "'%(name)s' argument is only allowed since microversion "
+                "%(start)s." % {"name": argument_name,
+                                "start": start_version.get_string()})
+
+        elif not end_version.is_null():
+            self.message = (
+                "'%(name)s' argument is not allowed after microversion "
+                "%(end)s." % {"name": argument_name,
+                              "end": end_version.get_string()})
+
+    def __str__(self):
+        return self.message
+
+
 class InvalidAPIVersion(Exception):
     pass
 

cinderclient/shell.py

@@ -26,6 +26,7 @@ import logging
 import sys
 
 import requests
+import six
 
 from cinderclient import api_versions
 from cinderclient import client
@@ -56,6 +57,8 @@ DEFAULT_CINDER_ENDPOINT_TYPE = 'publicURL'
 V1_SHELL = 'cinderclient.v1.shell'
 V2_SHELL = 'cinderclient.v2.shell'
 V3_SHELL = 'cinderclient.v3.shell'
+HINT_HELP_MSG = (" [hint: use '--os-volume-api-version' flag to show help "
+                 "message for proper version]")
 
 logging.basicConfig()
 logger = logging.getLogger(__name__)
@@ -395,24 +398,26 @@ class OpenStackCinderShell(object):
         parser.set_defaults(insecure=utils.env('CINDERCLIENT_INSECURE',
                                                default=False))
 
-    def get_subcommand_parser(self, version):
+    def get_subcommand_parser(self, version, do_help=False, input_args=None):
         parser = self.get_base_parser()
 
         self.subcommands = {}
         subparsers = parser.add_subparsers(metavar='<subcommand>')
 
-        if version == '2':
+        if version.ver_major == 2:
             actions_module = importutils.import_module(V2_SHELL)
-        elif version == '3':
+        elif version.ver_major == 3:
             actions_module = importutils.import_module(V3_SHELL)
         else:
             actions_module = importutils.import_module(V1_SHELL)
 
-        self._find_actions(subparsers, actions_module)
-        self._find_actions(subparsers, self)
+        self._find_actions(subparsers, actions_module, version, do_help,
+                           input_args)
+        self._find_actions(subparsers, self, version, do_help, input_args)
 
         for extension in self.extensions:
-            self._find_actions(subparsers, extension.module)
+            self._find_actions(subparsers, extension.module, version, do_help,
+                               input_args)
 
         self._add_bash_completion_subparser(subparsers)
 
@@ -427,18 +432,53 @@ class OpenStackCinderShell(object):
         self.subcommands['bash_completion'] = subparser
         subparser.set_defaults(func=self.do_bash_completion)
 
-    def _find_actions(self, subparsers, actions_module):
+    def _build_versioned_help_message(self, start_version, end_version):
+        if not start_version.is_null() and not end_version.is_null():
+            msg = (_(" (Supported by API versions %(start)s - %(end)s)")
+                % {"start": start_version.get_string(),
+                   "end": end_version.get_string()})
+        elif not start_version.is_null():
+            msg = (_(" (Supported by API version %(start)s and later)")
+                % {"start": start_version.get_string()})
+        else:
+            msg = (_(" Supported until API version %(end)s)")
+                % {"end": end_version.get_string()})
+        return six.text_type(msg)
+
+    def _find_actions(self, subparsers, actions_module, version,
+                      do_help, input_args):
         for attr in (a for a in dir(actions_module) if a.startswith('do_')):
             # I prefer to be hyphen-separated instead of underscores.
             command = attr[3:].replace('_', '-')
             callback = getattr(actions_module, attr)
             desc = callback.__doc__ or ''
-            help = desc.strip().split('\n')[0]
+            action_help = desc.strip().split('\n')[0]
+            if hasattr(callback, "versioned"):
+                additional_msg = ""
+                subs = api_versions.get_substitutions(
+                    utils.get_function_name(callback))
+                if do_help:
+                    additional_msg = self._build_versioned_help_message(
+                        subs[0].start_version, subs[-1].end_version)
+                    if version.is_latest():
+                        additional_msg += HINT_HELP_MSG
+                subs = [versioned_method for versioned_method in subs
+                        if version.matches(versioned_method.start_version,
+                                           versioned_method.end_version)]
+                if not subs:
+                    # There is no proper versioned method.
+                    continue
+                # Use the "latest" substitution.
+                callback = subs[-1].func
+                desc = callback.__doc__ or desc
+                action_help = desc.strip().split('\n')[0]
+                action_help += additional_msg
+
             arguments = getattr(callback, 'arguments', [])
 
             subparser = subparsers.add_parser(
                 command,
-                help=help,
+                help=action_help,
                 description=desc,
                 add_help=False,
                 formatter_class=OpenStackHelpFormatter)
@@ -448,8 +488,40 @@ class OpenStackCinderShell(object):
                                    help=argparse.SUPPRESS,)
 
             self.subcommands[command] = subparser
+
+            # NOTE(ntpttr): We get a counter for each argument in this
+            # command here because during the microversion check we only
+            # want to raise an exception if no version of the argument
+            # matches the current microversion. The exception will only
+            # be raised after the last instance of a particular argument
+            # fails the check.
+            arg_counter = dict()
+            for (args, kwargs) in arguments:
+                arg_counter[args[0]] = arg_counter.get(args[0], 0) + 1
+
             for (args, kwargs) in arguments:
-                subparser.add_argument(*args, **kwargs)
+                start_version = kwargs.get("start_version", None)
+                start_version = api_versions.APIVersion(start_version)
+                end_version = kwargs.get('end_version', None)
+                end_version = api_versions.APIVersion(end_version)
+                if do_help and not (start_version.is_null()
+                                    and end_version.is_null()):
+                    kwargs["help"] = kwargs.get("help", "") + (
+                        self._build_versioned_help_message(start_version,
+                                                           end_version))
+                if not version.matches(start_version, end_version):
+                    if args[0] in input_args and command == input_args[0]:
+                        if arg_counter[args[0]] == 1:
+                            # This is the last version of this argument,
+                            # raise the exception.
+                            raise exc.UnsupportedAttribute(args[0],
+                                start_version, end_version)
+                        arg_counter[args[0]] -= 1
+                    continue
+                kw = kwargs.copy()
+                kw.pop("start_version", None)
+                kw.pop("end_version", None)
+                subparser.add_argument(*args, **kw)
             subparser.set_defaults(func=callback)
 
     def setup_debugging(self, debug):
@@ -502,6 +574,9 @@ class OpenStackCinderShell(object):
         api_version_input = True
         self.options = options
 
+        do_help = ('help' in argv) or (
+            '--help' in argv) or ('-h' in argv) or not argv
+
         if not options.os_volume_api_version:
             api_version = api_versions.get_api_version(
                 DEFAULT_MAJOR_OS_VOLUME_API_VERSION)
@@ -514,7 +589,8 @@ class OpenStackCinderShell(object):
         self.extensions = client.discover_extensions(major_version_string)
         self._run_extension_hooks('__pre_parse_args__')
 
-        subcommand_parser = self.get_subcommand_parser(major_version_string)
+        subcommand_parser = self.get_subcommand_parser(api_version,
+                                                       do_help, args)
         self.parser = subcommand_parser
 
         if options.help or not argv:

cinderclient/tests/unit/fake_actions_module.py

@@ -0,0 +1,65 @@
+# Copyright 2016 FUJITSU LIMITED
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from cinderclient import api_versions
+from cinderclient import utils
+
+
+@api_versions.wraps("3.0", "3.1")
+def do_fake_action():
+    """help message
+
+    This will not show up in help message
+    """
+    return "fake_action 3.0 to 3.1"
+
+
+@api_versions.wraps("3.2", "3.3")
+def do_fake_action():
+    return "fake_action 3.2 to 3.3"
+
+
+@api_versions.wraps("3.6")
+@utils.arg(
+    '--foo',
+    start_version='3.7')
+def do_another_fake_action():
+    return "another_fake_action"
+
+
+@utils.arg(
+    '--foo',
+    start_version='3.1',
+    end_version='3.2')
+@utils.arg(
+    '--bar',
+    help='bar help',
+    start_version='3.3',
+    end_version='3.4')
+def do_fake_action2():
+    return "fake_action2"
+
+
+@utils.arg(
+    '--foo',
+    help='first foo',
+    start_version='3.6',
+    end_version='3.7')
+@utils.arg(
+    '--foo',
+    help='second foo',
+    start_version='3.8')
+def do_fake_action3():
+    return "fake_action3"

cinderclient/tests/unit/test_shell.py

@@ -27,9 +27,11 @@ from six import moves
 from testtools import matchers
 
 import cinderclient
+from cinderclient import api_versions
 from cinderclient import exceptions
 from cinderclient import auth_plugin
 from cinderclient import shell
+from cinderclient.tests.unit import fake_actions_module
 from cinderclient.tests.unit.test_auth_plugins import mock_http_request
 from cinderclient.tests.unit.test_auth_plugins import requested_headers
 from cinderclient.tests.unit.fixture_data import keystone_client
@@ -304,3 +306,172 @@ class CinderClientArgumentParserTest(utils.TestCase):
                             help=argparse.SUPPRESS)
 
         self.assertRaises(SystemExit, parser.parse_args, ['--test'])
+
+
+class TestLoadVersionedActions(utils.TestCase):
+
+    def test_load_versioned_actions(self):
+        parser = cinderclient.shell.CinderClientArgumentParser()
+        subparsers = parser.add_subparsers(metavar='<subcommand>')
+        shell = cinderclient.shell.OpenStackCinderShell()
+        shell.subcommands = {}
+        shell._find_actions(subparsers, fake_actions_module,
+                            api_versions.APIVersion("3.0"), False, [])
+        self.assertIn('fake-action', shell.subcommands.keys())
+        self.assertEqual(
+            "fake_action 3.0 to 3.1",
+            shell.subcommands['fake-action'].get_default('func')())
+
+        shell.subcommands = {}
+        shell._find_actions(subparsers, fake_actions_module,
+                            api_versions.APIVersion("3.2"), False, [])
+        self.assertIn('fake-action', shell.subcommands.keys())
+        self.assertEqual(
+            "fake_action 3.2 to 3.3",
+            shell.subcommands['fake-action'].get_default('func')())
+
+        self.assertIn('fake-action2', shell.subcommands.keys())
+        self.assertEqual(
+            "fake_action2",
+            shell.subcommands['fake-action2'].get_default('func')())
+
+    def test_load_versioned_actions_not_in_version_range(self):
+        parser = cinderclient.shell.CinderClientArgumentParser()
+        subparsers = parser.add_subparsers(metavar='<subcommand>')
+        shell = cinderclient.shell.OpenStackCinderShell()
+        shell.subcommands = {}
+        shell._find_actions(subparsers, fake_actions_module,
+                            api_versions.APIVersion('3.10000'), False, [])
+        self.assertNotIn('fake-action', shell.subcommands.keys())
+        self.assertIn('fake-action2', shell.subcommands.keys())
+
+    def test_load_versioned_actions_unsupported_input(self):
+        parser = cinderclient.shell.CinderClientArgumentParser()
+        subparsers = parser.add_subparsers(metavar='<subcommand>')
+        shell = cinderclient.shell.OpenStackCinderShell()
+        shell.subcommands = {}
+        self.assertRaises(exceptions.UnsupportedAttribute,
+                          shell._find_actions, subparsers, fake_actions_module,
+                          api_versions.APIVersion('3.6'), False,
+                          ['another-fake-action', '--foo'])
+
+    def test_load_versioned_actions_with_help(self):
+        parser = cinderclient.shell.CinderClientArgumentParser()
+        subparsers = parser.add_subparsers(metavar='<subcommand>')
+        shell = cinderclient.shell.OpenStackCinderShell()
+        shell.subcommands = {}
+        with mock.patch.object(subparsers, 'add_parser') as mock_add_parser:
+            shell._find_actions(subparsers, fake_actions_module,
+                                api_versions.APIVersion("3.1"), True, [])
+            self.assertIn('fake-action', shell.subcommands.keys())
+            expected_help = ("help message (Supported by API versions "
+                             "%(start)s - %(end)s)") % {
+                'start': '3.0', 'end': '3.3'}
+            expected_desc = ("help message\n\n    "
+                            "This will not show up in help message\n    ")
+            mock_add_parser.assert_any_call(
+                'fake-action',
+                help=expected_help,
+                description=expected_desc,
+                add_help=False,
+                formatter_class=cinderclient.shell.OpenStackHelpFormatter)
+
+    def test_load_versioned_actions_with_help_on_latest(self):
+        parser = cinderclient.shell.CinderClientArgumentParser()
+        subparsers = parser.add_subparsers(metavar='<subcommand>')
+        shell = cinderclient.shell.OpenStackCinderShell()
+        shell.subcommands = {}
+        with mock.patch.object(subparsers, 'add_parser') as mock_add_parser:
+            shell._find_actions(subparsers, fake_actions_module,
+                                api_versions.APIVersion("3.latest"), True, [])
+            self.assertIn('another-fake-action', shell.subcommands.keys())
+            expected_help = (" (Supported by API versions %(start)s - "
+                             "%(end)s)%(hint)s") % {
+                'start': '3.6', 'end': '3.latest',
+                'hint': cinderclient.shell.HINT_HELP_MSG}
+            mock_add_parser.assert_any_call(
+                'another-fake-action',
+                help=expected_help,
+                description='',
+                add_help=False,
+                formatter_class=cinderclient.shell.OpenStackHelpFormatter)
+
+    @mock.patch.object(cinderclient.shell.CinderClientArgumentParser,
+                       'add_argument')
+    def test_load_versioned_actions_with_args(self, mock_add_arg):
+        parser = cinderclient.shell.CinderClientArgumentParser(add_help=False)
+        subparsers = parser.add_subparsers(metavar='<subcommand>')
+        shell = cinderclient.shell.OpenStackCinderShell()
+        shell.subcommands = {}
+        shell._find_actions(subparsers, fake_actions_module,
+                            api_versions.APIVersion("3.1"), False, [])
+        self.assertIn('fake-action2', shell.subcommands.keys())
+        mock_add_arg.assert_has_calls([
+            mock.call('-h', '--help', action='help', help='==SUPPRESS=='),
+            mock.call('--foo')])
+
+    @mock.patch.object(cinderclient.shell.CinderClientArgumentParser,
+                       'add_argument')
+    def test_load_versioned_actions_with_args2(self, mock_add_arg):
+        parser = cinderclient.shell.CinderClientArgumentParser(add_help=False)
+        subparsers = parser.add_subparsers(metavar='<subcommand>')
+        shell = cinderclient.shell.OpenStackCinderShell()
+        shell.subcommands = {}
+        shell._find_actions(subparsers, fake_actions_module,
+                            api_versions.APIVersion("3.4"), False, [])
+        self.assertIn('fake-action2', shell.subcommands.keys())
+        mock_add_arg.assert_has_calls([
+            mock.call('-h', '--help', action='help', help='==SUPPRESS=='),
+            mock.call('--bar', help="bar help")])
+
+    @mock.patch.object(cinderclient.shell.CinderClientArgumentParser,
+                       'add_argument')
+    def test_load_versioned_actions_with_args_not_in_version_range(
+            self, mock_add_arg):
+        parser = cinderclient.shell.CinderClientArgumentParser(add_help=False)
+        subparsers = parser.add_subparsers(metavar='<subcommand>')
+        shell = cinderclient.shell.OpenStackCinderShell()
+        shell.subcommands = {}
+        shell._find_actions(subparsers, fake_actions_module,
+                            api_versions.APIVersion("3.10000"), False, [])
+        self.assertIn('fake-action2', shell.subcommands.keys())
+        mock_add_arg.assert_has_calls([
+            mock.call('-h', '--help', action='help', help='==SUPPRESS==')])
+
+    @mock.patch.object(cinderclient.shell.CinderClientArgumentParser,
+                       'add_argument')
+    def test_load_versioned_actions_with_args_and_help(self, mock_add_arg):
+        parser = cinderclient.shell.CinderClientArgumentParser(add_help=False)
+        subparsers = parser.add_subparsers(metavar='<subcommand>')
+        shell = cinderclient.shell.OpenStackCinderShell()
+        shell.subcommands = {}
+        shell._find_actions(subparsers, fake_actions_module,
+                            api_versions.APIVersion("3.4"), True, [])
+        mock_add_arg.assert_has_calls([
+            mock.call('-h', '--help', action='help', help='==SUPPRESS=='),
+            mock.call('--bar',
+                      help="bar help (Supported by API versions"
+                           " 3.3 - 3.4)")])
+
+    @mock.patch.object(cinderclient.shell.CinderClientArgumentParser,
+                       'add_argument')
+    def test_load_actions_with_versioned_args(self, mock_add_arg):
+        parser = cinderclient.shell.CinderClientArgumentParser(add_help=False)
+        subparsers = parser.add_subparsers(metavar='<subcommand>')
+        shell = cinderclient.shell.OpenStackCinderShell()
+        shell.subcommands = {}
+        shell._find_actions(subparsers, fake_actions_module,
+                            api_versions.APIVersion("3.6"), False, [])
+        self.assertIn(mock.call('--foo', help="first foo"),
+                      mock_add_arg.call_args_list)
+        self.assertNotIn(mock.call('--foo', help="second foo"),
+                         mock_add_arg.call_args_list)
+
+        mock_add_arg.reset_mock()
+
+        shell._find_actions(subparsers, fake_actions_module,
+                            api_versions.APIVersion("3.9"), False, [])
+        self.assertNotIn(mock.call('--foo', help="first foo"),
+                         mock_add_arg.call_args_list)
+        self.assertIn(mock.call('--foo', help="second foo"),
+                      mock_add_arg.call_args_list)

cinderclient/tests/unit/v3/test_shell.py

@@ -129,7 +129,7 @@ class ShellTest(utils.TestCase):
                           '--os-volume-api-version 3.9 backup-update 1234')
 
     def test_backup_update_wrong_version(self):
-        self.assertRaises(exceptions.VersionNotFoundForAPIMethod,
+        self.assertRaises(SystemExit,
                           self.run_command,
                           '--os-volume-api-version 3.8 '
                           'backup-update --name new-name 1234')

cinderclient/v3/shell.py

@@ -1602,14 +1602,14 @@ def do_backup_reset_state(cs, args):
         raise exceptions.CommandError(msg)
 
 
+@utils.service_type('volumev3')
+@api_versions.wraps('3.9')
 @utils.arg('backup', metavar='<backup>',
            help='Name or ID of backup to rename.')
 @utils.arg('--name', nargs='?', metavar='<name>',
            help='New name for backup.')
 @utils.arg('--description', metavar='<description>',
            help='Backup description. Default=None.')
-@utils.service_type('volumev3')
-@api_versions.wraps('3.9')
 def do_backup_update(cs, args):
     """Renames a backup."""
     kwargs = {}