openstack/python-ironicclient
Code Issues Proposed changes

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

Browse Source 
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
This commit is contained in:
Dmitry Tantsur 2017-07-18 17:54:23 +02:00
parent 137233b217
commit cdae0fb045
5 changed files with 61 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ironicclient/shell.py
View File

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

7
ironicclient/tests/functional/base.py
View File

@ -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."""

8
ironicclient/tests/functional/test_help_msg.py
View File

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

47
ironicclient/tests/unit/test_shell.py
View File

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

7
releasenotes/notes/implicit-version-warning-old-cli-fe34d423ae63544a.yaml Normal file
View File

@ -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.