openstack
/
python-ironicclient
Code Issues Proposed changes

Switch the deprecated "ironic" CLI to "latest" API version by default 

The functional tests were updated to account for the initial state changed
to "enroll" and for new fields appearing in "show" and "update" responses.

Closes-Bug: #1671145
Change-Id: Ida18541fbbc8064868cac0accb6919de08e9f795
This commit is contained in:
Dmitry Tantsur 2017-10-25 15:31:05 +02:00
parent ac5b86a6d5
commit 28560398fa
7 changed files with 183 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
ironicclient/shell.py
View File

@ -38,14 +38,8 @@ from ironicclient.common import utils
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."
)
LAST_KNOWN_API_VERSION = 34
LATEST_VERSION = '1.{}'.format(LAST_KNOWN_API_VERSION)
class IronicShell(object):
@ -161,12 +155,10 @@ class IronicShell(object):
parser.add_argument('--ironic-api-version',
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 %s. Starting '
'with the Queens release this will '
'default to "latest".') % http.DEFAULT_VER)
default="latest"),
help=_('Accepts 1.x (where "x" is microversion), '
'1 or "latest". Defaults to '
'env[IRONIC_API_VERSION] or "latest".'))
parser.add_argument('--ironic_api_version',
help=argparse.SUPPRESS)
@ -300,35 +292,31 @@ class IronicShell(object):
print(' '.join(commands | options))
def _check_version(self, api_version):
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'
"""Validate the supplied API (micro)version.
:param api_version: API version as a string ("1", "1.x" or "latest")
:returns: tuple (major version, version string)
"""
if api_version in ('1', 'latest'):
return (1, LATEST_VERSION)
else:
try:
versions = tuple(int(i) for i in api_version.split('.'))
except ValueError:
versions = ()
if len(versions) == 1:
# Default value of ironic_api_version is '1'.
# If user not specify the value of api version, not passing
# headers at all.
os_ironic_api_version = None
elif len(versions) == 2:
os_ironic_api_version = api_version
# In the case of '1.0'
if versions[1] == 0:
os_ironic_api_version = None
else:
if not versions or len(versions) > 2:
msg = _("The requested API version %(ver)s is an unexpected "
"format. Acceptable formats are 'X', 'X.Y', or the "
"literal string '%(latest)s'."
) % {'ver': api_version, 'latest': 'latest'}
"literal string 'latest'."
) % {'ver': api_version}
raise exc.CommandError(msg)
if versions == (1, 0):
os_ironic_api_version = None
else:
os_ironic_api_version = api_version
api_major_version = versions[0]
return (api_major_version, os_ironic_api_version)
@ -422,6 +410,11 @@ class IronicShell(object):
kwargs[key] = getattr(args, key)
kwargs['os_ironic_api_version'] = os_ironic_api_version
client = ironicclient.client.get_client(api_major_version, **kwargs)
if options.ironic_api_version in ('1', 'latest'):
# Allow negotiating a lower version, if the latest version
# supported by the client is higher than the latest version
# supported by the server.
client.http_client.api_version_select_state = 'default'
try:
args.func(client, args)

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

@ -216,7 +216,9 @@ class FunctionalTestBase(base.ClientTestBase):
if utils.get_object(node_list, node_id):
node_show = self.show_node(node_id)
if node_show['provision_state'] != 'available':
if node_show['provision_state'] not in ('available',
'manageable',
'enroll'):
self.ironic('node-set-provision-state',
params='{0} deleted'.format(node_id))
if node_show['power_state'] not in ('None', 'off'):

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

@ -67,11 +67,3 @@ 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)

12
ironicclient/tests/functional/test_json_response.py
View File

@ -48,10 +48,10 @@ class TestNodeJsonResponse(base.FunctionalTestBase):
"uuid": {"type": "string"},
"console_enabled": {"type": "boolean"},
"target_provision_state": {"type": ["string", "null"]},
"raid_config": {"type": "string"},
"raid_config": {"type": "object"},
"provision_updated_at": {"type": ["string", "null"]},
"maintenance": {"type": "boolean"},
"target_raid_config": {"type": "string"},
"target_raid_config": {"type": "object"},
"inspection_started_at": {"type": ["string", "null"]},
"inspection_finished_at": {"type": ["string", "null"]},
"power_state": {"type": ["string", "null"]},
@ -65,8 +65,12 @@ class TestNodeJsonResponse(base.FunctionalTestBase):
"driver_internal_info": {"type": "object"},
"chassis_uuid": {"type": ["string", "null"]},
"instance_info": {"type": "object"}
}
}
},
"patternProperties": {
".*_interface$": {"type": ["string", "null"]}
},
"additionalProperties": True
}
def setUp(self):
super(TestNodeJsonResponse, self).setUp()

15
ironicclient/tests/functional/test_node.py
View File

@ -161,18 +161,21 @@ class NodeSanityTestIronicClient(base.FunctionalTestBase):
"""Test steps:
1) create node
2) check that provision state is 'available'
2) check that provision state is 'enroll'
3) set new provision state to the node
4) check that provision state has been updated successfully
"""
node_show = self.show_node(self.node['uuid'])
self.assertEqual('available', node_show['provision_state'])
self.assertEqual('enroll', node_show['provision_state'])
self.set_node_provision_state(self.node['uuid'], 'active')
node_show = self.show_node(self.node['uuid'])
self.assertEqual('active', node_show['provision_state'])
for verb, target in [('manage', 'manageable'),
('provide', 'available'),
('active', 'active'),
('deleted', 'available')]:
self.set_node_provision_state(self.node['uuid'], verb)
node_show = self.show_node(self.node['uuid'])
self.assertEqual(target, node_show['provision_state'])
def test_node_validate(self):
"""Test steps:

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

@ -174,7 +174,8 @@ class ShellTest(utils.BaseTestCase):
'os_cert': None, 'os_key': None,
'max_retries': http.DEFAULT_MAX_RETRIES,
'retry_interval': http.DEFAULT_RETRY_INTERVAL,
'os_ironic_api_version': None, 'timeout': 600, 'insecure': False
'os_ironic_api_version': ironic_shell.LATEST_VERSION,
'timeout': 600, 'insecure': False
}
mock_client.assert_called_once_with(1, **expected_kwargs)
# Make sure we are actually prompted.
@ -203,7 +204,8 @@ class ShellTest(utils.BaseTestCase):
'os_endpoint_type': '', 'os_cacert': None, 'os_cert': None,
'os_key': None, 'max_retries': http.DEFAULT_MAX_RETRIES,
'retry_interval': http.DEFAULT_RETRY_INTERVAL,
'os_ironic_api_version': None, 'timeout': 600, 'insecure': False
'os_ironic_api_version': ironic_shell.LATEST_VERSION,
'timeout': 600, 'insecure': False
}
mock_client.assert_called_once_with(1, **expected_kwargs)
self.assertFalse(mock_getpass.called)
@ -254,17 +256,118 @@ class ShellTest(utils.BaseTestCase):
err = self.shell('--ironic-api-version latest help')[1]
self.assertIn('The "ironic" CLI is deprecated', err)
self.assertRaises(exc.CommandError,
self.shell, '--ironic-api-version 1.2.1 help')
err = self.shell('--ironic-api-version 1 help')[1]
self.assertIn('The "ironic" CLI is deprecated', err)
def test_invalid_ironic_api_version(self):
self.assertRaises(exceptions.UnsupportedVersion,
self.shell, '--ironic-api-version 0.8 help')
self.assertRaises(exc.CommandError,
self.shell, '--ironic-api-version 1.2.1 help')
def test_warning_on_no_version(self):
err = self.shell('help')[1]
self.assertIn('You are using the default API version', err)
self.assertIn('The "ironic" CLI is deprecated', err)
@mock.patch.object(client, 'get_client', autospec=True,
side_effect=keystone_exc.ConnectFailure)
def test_api_version_in_env(self, mock_client):
env = dict(IRONIC_API_VERSION='1.10', **FAKE_ENV)
self.make_env(environ_dict=env)
# We will get a ConnectFailure because there is no keystone.
self.assertRaises(keystone_exc.ConnectFailure,
self.shell, 'node-list')
expected_kwargs = {
'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'],
'os_tenant_id': '', 'os_tenant_name': '',
'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '',
'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'],
'os_auth_token': '', 'os_project_id': '',
'os_project_name': FAKE_ENV['OS_PROJECT_NAME'],
'os_project_domain_id': '',
'os_project_domain_name': '', 'os_region_name': '',
'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None,
'os_cert': None, 'os_key': None,
'max_retries': http.DEFAULT_MAX_RETRIES,
'retry_interval': http.DEFAULT_RETRY_INTERVAL,
'os_ironic_api_version': '1.10',
'timeout': 600, 'insecure': False
}
mock_client.assert_called_once_with(1, **expected_kwargs)
@mock.patch.object(client, 'get_client', autospec=True,
side_effect=keystone_exc.ConnectFailure)
def test_api_version_v1_in_env(self, mock_client):
env = dict(IRONIC_API_VERSION='1', **FAKE_ENV)
self.make_env(environ_dict=env)
# We will get a ConnectFailure because there is no keystone.
self.assertRaises(keystone_exc.ConnectFailure,
self.shell, 'node-list')
expected_kwargs = {
'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'],
'os_tenant_id': '', 'os_tenant_name': '',
'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '',
'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'],
'os_auth_token': '', 'os_project_id': '',
'os_project_name': FAKE_ENV['OS_PROJECT_NAME'],
'os_project_domain_id': '',
'os_project_domain_name': '', 'os_region_name': '',
'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None,
'os_cert': None, 'os_key': None,
'max_retries': http.DEFAULT_MAX_RETRIES,
'retry_interval': http.DEFAULT_RETRY_INTERVAL,
'os_ironic_api_version': ironic_shell.LATEST_VERSION,
'timeout': 600, 'insecure': False
}
mock_client.assert_called_once_with(1, **expected_kwargs)
@mock.patch.object(client, 'get_client', autospec=True,
side_effect=keystone_exc.ConnectFailure)
def test_api_version_in_args(self, mock_client):
env = dict(IRONIC_API_VERSION='1.10', **FAKE_ENV)
self.make_env(environ_dict=env)
# We will get a ConnectFailure because there is no keystone.
self.assertRaises(keystone_exc.ConnectFailure,
self.shell, '--ironic-api-version 1.11 node-list')
expected_kwargs = {
'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'],
'os_tenant_id': '', 'os_tenant_name': '',
'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '',
'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'],
'os_auth_token': '', 'os_project_id': '',
'os_project_name': FAKE_ENV['OS_PROJECT_NAME'],
'os_project_domain_id': '',
'os_project_domain_name': '', 'os_region_name': '',
'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None,
'os_cert': None, 'os_key': None,
'max_retries': http.DEFAULT_MAX_RETRIES,
'retry_interval': http.DEFAULT_RETRY_INTERVAL,
'os_ironic_api_version': '1.11',
'timeout': 600, 'insecure': False
}
mock_client.assert_called_once_with(1, **expected_kwargs)
@mock.patch.object(client, 'get_client', autospec=True,
side_effect=keystone_exc.ConnectFailure)
def test_api_version_v1_in_args(self, mock_client):
env = dict(IRONIC_API_VERSION='1.10', **FAKE_ENV)
self.make_env(environ_dict=env)
# We will get a ConnectFailure because there is no keystone.
self.assertRaises(keystone_exc.ConnectFailure,
self.shell, '--ironic-api-version 1 node-list')
expected_kwargs = {
'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'],
'os_tenant_id': '', 'os_tenant_name': '',
'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '',
'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'],
'os_auth_token': '', 'os_project_id': '',
'os_project_name': FAKE_ENV['OS_PROJECT_NAME'],
'os_project_domain_id': '',
'os_project_domain_name': '', 'os_region_name': '',
'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None,
'os_cert': None, 'os_key': None,
'max_retries': http.DEFAULT_MAX_RETRIES,
'retry_interval': http.DEFAULT_RETRY_INTERVAL,
'os_ironic_api_version': ironic_shell.LATEST_VERSION,
'timeout': 600, 'insecure': False
}
mock_client.assert_called_once_with(1, **expected_kwargs)
class TestCase(testtools.TestCase):

25
releasenotes/notes/ironic-cli-version-a5cdec73d585444d.yaml Normal file
View File

@ -0,0 +1,25 @@
---
upgrade:
- |
The default API version for the ``ironic`` command is now "latest", which
is the maximum version understood by both the client and the server.
This change makes the CLI automatically pull in new features and changes
(including potentially breaking), when talking to new servers.
Scripts that rely on some specific API behavior should set the
``IRONIC_API_VERSION`` environment variable or use the
``--ironic-api-version`` CLI argument.
.. note:: This change does not affect the Python API.
features:
- |
The ``ironic`` command now supports the specification of API version ``1``.
The actual version used will be the maximum 1.x version understood by both
the client and the server. Thus, it is currently identical to the
``latest`` value.
fixes:
- |
Users of the ``ironic`` command no longer have to specify an explicit
API version to use the latest features. The default API version is now
"latest", which is the maximum version understood by both the client
and the server.