Merge "Log warning when API version is not specified for the ironic tool"

5 files changed, 61 insertions, 24 deletions

diff --git a/ironicclient/shell.py b/ironicclient/shell.py
index 4a67365..5ecd0d2 100644
--- a/ironicclient/shell.py
+++ b/ironicclient/shell.py
@@ -39,6 +39,13 @@ from ironicclient import exc
 
 
 LATEST_API_VERSION = ('1', 'latest')
+MISSING_VERSION_WARNING = (
+    "You are using the default API version of the 'ironic' command "
+    "This is currently API version %s. In the future, the default will be "
+    "the latest API version understood by both API and CLI. You can preserve "
+    "the current behavior by passing the --ironic-api-version argument with "
+    "the desired version or using the IRONIC_API_VERSION environment variable."
+)
 
 
 class IronicShell(object):
@@ -153,8 +160,8 @@ class IronicShell(object):
                             help=argparse.SUPPRESS)
 
         parser.add_argument('--ironic-api-version',
-                            default=cliutils.env(
-                                'IRONIC_API_VERSION', default='1'),
+                            default=cliutils.env('IRONIC_API_VERSION',
+                                                 default=None),
                             help=_('Accepts 1.x (where "x" is microversion) '
                                    'or "latest", Defaults to '
                                    'env[IRONIC_API_VERSION] or 1'))
@@ -294,6 +301,11 @@ class IronicShell(object):
         if api_version == 'latest':
             return LATEST_API_VERSION
         else:
+            if api_version is None:
+                print(MISSING_VERSION_WARNING % http.DEFAULT_VER,
+                      file=sys.stderr)
+                api_version = '1'
+
             try:
                 versions = tuple(int(i) for i in api_version.split('.'))
             except ValueError:
diff --git a/ironicclient/tests/functional/base.py b/ironicclient/tests/functional/base.py
index b954d1a..a0ea61b 100644
--- a/ironicclient/tests/functional/base.py
+++ b/ironicclient/tests/functional/base.py
@@ -126,7 +126,7 @@ class FunctionalTestBase(base.ClientTestBase):
         return base.execute(cmd, action, flags, params,
                             cli_dir=self.client.cli_dir)
 
-    def _ironic(self, action, flags='', params=''):
+    def _ironic(self, action, flags='', params='', merge_stderr=False):
         """Execute ironic command for the given action.
 
         :param action: the cli command to run using Ironic
@@ -135,6 +135,8 @@ class FunctionalTestBase(base.ClientTestBase):
         :type flags: string
         :param params: any optional positional args to use
         :type params: string
+        :param merge_stderr: whether to merge stderr into the result
+        :type merge_stderr: bool
         """
         flags += ' --os-endpoint-type publicURL'
         if hasattr(self, 'os_auth_token'):
@@ -148,7 +150,8 @@ class FunctionalTestBase(base.ClientTestBase):
                         'value': getattr(self, domain_attr)
                     }
             return self.client.cmd_with_auth('ironic',
-                                             action, flags, params)
+                                             action, flags, params,
+                                             merge_stderr=merge_stderr)
 
     def _ironic_osc(self, action, flags='', params='', merge_stderr=False):
         """Execute baremetal commands via OpenStack Client."""
diff --git a/ironicclient/tests/functional/test_help_msg.py b/ironicclient/tests/functional/test_help_msg.py
index 39bde41..07b094b 100644
--- a/ironicclient/tests/functional/test_help_msg.py
+++ b/ironicclient/tests/functional/test_help_msg.py
@@ -67,3 +67,11 @@ class IronicClientHelp(base.FunctionalTestBase):
         self.assertIn(caption, output)
         for string in subcommands:
             self.assertIn(string, output)
+
+    def test_warning_on_api_version(self):
+        result = self._ironic('help', merge_stderr=True)
+        self.assertIn('You are using the default API version', result)
+
+        result = self._ironic('help', flags='--ironic-api-version 1.9',
+                              merge_stderr=True)
+        self.assertNotIn('You are using the default API version', result)
diff --git a/ironicclient/tests/unit/test_shell.py b/ironicclient/tests/unit/test_shell.py
index d80b356..77a2f19 100644
--- a/ironicclient/tests/unit/test_shell.py
+++ b/ironicclient/tests/unit/test_shell.py
@@ -74,19 +74,18 @@ class ShellTest(utils.BaseTestCase):
         super(ShellTest, self).setUp()
 
     def shell(self, argstr):
-        orig = sys.stdout
-        try:
-            sys.stdout = six.StringIO()
-            _shell = ironic_shell.IronicShell()
-            _shell.main(argstr.split())
-        except SystemExit:
-            exc_type, exc_value, exc_traceback = sys.exc_info()
-            self.assertEqual(0, exc_value.code)
-        finally:
-            out = sys.stdout.getvalue()
-            sys.stdout.close()
-            sys.stdout = orig
-        return out
+        with mock.patch.object(sys, 'stdout', six.StringIO()):
+            with mock.patch.object(sys, 'stderr', six.StringIO()):
+                try:
+                    _shell = ironic_shell.IronicShell()
+                    _shell.main(argstr.split())
+                except SystemExit:
+                    exc_type, exc_value, exc_traceback = sys.exc_info()
+                    self.assertEqual(0, exc_value.code)
+                finally:
+                    out = sys.stdout.getvalue()
+                    err = sys.stderr.getvalue()
+                return out, err
 
     def test_help_unknown_command(self):
         self.assertRaises(exc.CommandError, self.shell, 'help foofoo')
@@ -99,7 +98,7 @@ class ShellTest(utils.BaseTestCase):
             'for help on a specific command',
         ]
         for argstr in ['--help', 'help']:
-            help_text = self.shell(argstr)
+            help_text = self.shell(argstr)[0]
             for r in required:
                 self.assertThat(help_text,
                                 matchers.MatchesRegex(r,
@@ -114,7 +113,7 @@ class ShellTest(utils.BaseTestCase):
             'help chassis-show',
         ]
         for argstr in argstrings:
-            help_text = self.shell(argstr)
+            help_text = self.shell(argstr)[0]
             for r in required:
                 self.assertThat(help_text,
                                 matchers.MatchesRegex(r, self.re_options))
@@ -129,7 +128,7 @@ class ShellTest(utils.BaseTestCase):
             'help node-create',
         ]
         for argstr in argstrings:
-            help_text = self.shell(argstr)
+            help_text = self.shell(argstr)[0]
             for r in required:
                 self.assertThat(help_text,
                                 matchers.MatchesRegex(r, self.re_options))
@@ -144,7 +143,7 @@ class ShellTest(utils.BaseTestCase):
             'help port-create',
         ]
         for argstr in argstrings:
-            help_text = self.shell(argstr)
+            help_text = self.shell(argstr)[0]
             for r in required:
                 self.assertThat(help_text,
                                 matchers.MatchesRegex(r, self.re_options))
@@ -236,7 +235,7 @@ class ShellTest(utils.BaseTestCase):
             self.fail('CommandError not raised')
 
     def test_bash_completion(self):
-        stdout = self.shell('bash-completion')
+        stdout = self.shell('bash-completion')[0]
         # just check we have some output
         required = [
             '.*--driver_info',
@@ -249,8 +248,12 @@ class ShellTest(utils.BaseTestCase):
                             matchers.MatchesRegex(r, self.re_options))
 
     def test_ironic_api_version(self):
-        self.shell('--ironic-api-version 1.2 help')
-        self.shell('--ironic-api-version latest help')
+        err = self.shell('--ironic-api-version 1.2 help')[1]
+        self.assertFalse(err)
+
+        err = self.shell('--ironic-api-version latest help')[1]
+        self.assertFalse(err)
+
         self.assertRaises(exc.CommandError,
                           self.shell, '--ironic-api-version 1.2.1 help')
 
@@ -258,6 +261,10 @@ class ShellTest(utils.BaseTestCase):
         self.assertRaises(exceptions.UnsupportedVersion,
                           self.shell, '--ironic-api-version 0.8 help')
 
+    def test_warning_on_no_version(self):
+        err = self.shell('help')[1]
+        self.assertIn('You are using the default API version', err)
+
 
 class TestCase(testtools.TestCase):
 
diff --git a/releasenotes/notes/implicit-version-warning-old-cli-fe34d423ae63544a.yaml b/releasenotes/notes/implicit-version-warning-old-cli-fe34d423ae63544a.yaml
new file mode 100644
index 0000000..29967f4
--- /dev/null
+++ b/releasenotes/notes/implicit-version-warning-old-cli-fe34d423ae63544a.yaml
@@ -0,0 +1,7 @@
+---
+deprecations:
+  - |
+    Currently, the default API version for the ``ironic`` tool is fixed to be
+    1.9.  In the Queens release, it will be changed to the latest version
+    understood by both the client and the server. In this release a warning is
+    logged, if no explicit version is provided.