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

At the Pike PTG, an issue was brought up regarding the use of an old API
version in the ironic tool [0]. It was suggested that we begin printing
a warning whenever the default API version is used and later default
to using the latest API version when --ironic-api-version is unspecified.

This patch adds this warning.

[0] https://etherpad.openstack.org/p/ironic-pike-ptg-operations L30

Change-Id: I80d553e4d3b007d8312931019037f495425b5ea5
Partial-Bug: #1671145

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.