Fix max version handling for help output

stmcginnis · stmcginnis · commit bccbd510a778 · 2019-01-30T19:00:51.000-06:00
Commit fefe331 incorrectly set the default API version to the max the client knew about in order to get the full help output, including all microversioned commands. The original intent was to have help print out information for all versions, but still require the user to specify a version if they wanted to use any microversioned version of an API. The code accidentally made it so all commands would request the max version the client knew about. This meant an unspecified request would get the newer functionality rather than the default v3 functionality, and also meant the client could request a microversion higher than what the server knew about, resulting in an unexpected error being returned. To keep the originally intended functionality, but keep all help output, this only uses the max API version for the help command unless the user specifies otherwise. Closes-bug: #1813967 Change-Id: I20f6c5471ffefe5524a4d48c967e2e8db53233f1 Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
diff --git a/cinderclient/shell.py b/cinderclient/shell.py
@@ -46,6 +46,7 @@
 from cinderclient import utils
 
 
+DEFAULT_MAJOR_OS_VOLUME_API_VERSION = "3"
 DEFAULT_CINDER_ENDPOINT_TYPE = 'publicURL'
 V1_SHELL = 'cinderclient.v1.shell'
 V2_SHELL = 'cinderclient.v2.shell'
@@ -527,8 +528,10 @@ def main(self, argv):
             '--help' in argv) or ('-h' in argv) or not argv
 
         if not options.os_volume_api_version:
-            api_version = api_versions.get_api_version(
-                api_versions.MAX_VERSION)
+            use_version = DEFAULT_MAJOR_OS_VOLUME_API_VERSION
+            if do_help:
+                use_version = api_versions.MAX_VERSION
+            api_version = api_versions.get_api_version(use_version)
         else:
             api_version = api_versions.get_api_version(
                 options.os_volume_api_version)
diff --git a/cinderclient/tests/unit/test_shell.py b/cinderclient/tests/unit/test_shell.py
@@ -114,9 +114,11 @@ def test_help_unknown_command(self):
         self.assertRaises(exceptions.CommandError, self.shell, 'help foofoo')
 
     def test_help(self):
+        # Some expected help output, including microversioned commands
         required = [
             '.*?^usage: ',
             '.*?(?m)^\s+create\s+Creates a volume.',
+            '.*?(?m)^\s+summary\s+Get volumes summary.',
             '.*?(?m)^Run "cinder help SUBCOMMAND" for help on a subcommand.',
         ]
         help_text = self.shell('help')
@@ -134,6 +136,16 @@ def test_help_on_subcommand(self):
             self.assertThat(help_text,
                             matchers.MatchesRegex(r, re.DOTALL | re.MULTILINE))
 
+    def test_help_on_subcommand_mv(self):
+        required = [
+            '.*?^usage: cinder summary',
+            '.*?(?m)^Get volumes summary.',
+        ]
+        help_text = self.shell('help summary')
+        for r in required:
+            self.assertThat(help_text,
+                            matchers.MatchesRegex(r, re.DOTALL | re.MULTILINE))
+
     @ddt.data('backup-create --help', '--help backup-create')
     def test_dash_dash_help_on_subcommand(self, cmd):
         required = ['.*?^Creates a volume backup.']
