Set the default API version of OSC CLI to "latest"

Now the default of OSC is the negotiated maximum version understood
by both the server and the client. The value of "1" is now equivalent
to "latest" as well.

This change also cleans up unit tests for the OSC plugin.

Change-Id: I489fee937a356b523eb35379dce3631195132fe5
Closes-Bug: #1671145
This commit is contained in:
Dmitry Tantsur 2017-10-18 11:50:29 +02:00
parent bc4403ff05
commit c534b9465d
6 changed files with 148 additions and 139 deletions

View File

@ -19,41 +19,28 @@
import argparse
import logging
from ironicclient.common import http
from osc_lib import utils
LOG = logging.getLogger(__name__)
CLIENT_CLASS = 'ironicclient.v1.client.Client'
API_VERSION_OPTION = 'os_baremetal_api_version'
API_NAME = 'baremetal'
LAST_KNOWN_API_VERSION = 34
LATEST_VERSION = "1.{}".format(LAST_KNOWN_API_VERSION)
API_VERSIONS = {
'1.%d' % i: 'ironicclient.v1.client.Client'
'1.%d' % i: CLIENT_CLASS
for i in range(1, LAST_KNOWN_API_VERSION + 1)
}
API_VERSIONS['1'] = API_VERSIONS[http.DEFAULT_VER]
OS_BAREMETAL_API_VERSION_SPECIFIED = False
MISSING_VERSION_WARNING = (
"You are using the default API version of the OpenStack CLI baremetal "
"(ironic) plugin. 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 "
"--os-baremetal-api-version argument with the desired version or using "
"the OS_BAREMETAL_API_VERSION environment variable."
)
API_VERSIONS['1'] = CLIENT_CLASS
# NOTE(dtantsur): flag to indicate that the requested version was "latest".
# Due to how OSC works we cannot just add "latest" to the list of supported
# versions - it breaks the major version detection.
OS_BAREMETAL_API_LATEST = False
OS_BAREMETAL_API_LATEST = True
def make_client(instance):
"""Returns a baremetal service client."""
if (not OS_BAREMETAL_API_VERSION_SPECIFIED and not
utils.env('OS_BAREMETAL_API_VERSION')):
LOG.warning(MISSING_VERSION_WARNING, http.DEFAULT_VER)
requested_api_version = instance._api_version[API_NAME]
baremetal_client_class = utils.get_client_class(
@ -65,11 +52,19 @@ def make_client(instance):
requested_api_version if not OS_BAREMETAL_API_LATEST
else "latest")
if requested_api_version == '1':
# NOTE(dtantsur): '1' means 'the latest v1 API version'. Since we don't
# have other major versions, it's identical to 'latest'.
requested_api_version = LATEST_VERSION
allow_api_version_downgrade = True
else:
allow_api_version_downgrade = OS_BAREMETAL_API_LATEST
client = baremetal_client_class(
os_ironic_api_version=requested_api_version,
# NOTE(dtantsur): enable re-negotiation of the latest version, if CLI
# latest is too high for the server we're talking to.
allow_api_version_downgrade=OS_BAREMETAL_API_LATEST,
allow_api_version_downgrade=allow_api_version_downgrade,
session=instance.session,
region_name=instance._region_name,
# NOTE(vdrok): This will be set as endpoint_override, and the Client
@ -87,17 +82,14 @@ def build_option_parser(parser):
parser.add_argument(
'--os-baremetal-api-version',
metavar='<baremetal-api-version>',
default=_get_environment_version(http.DEFAULT_VER),
default=_get_environment_version("latest"),
choices=sorted(
API_VERSIONS,
key=lambda k: [int(x) for x in k.split('.')]) + ['latest'],
action=ReplaceLatestVersion,
help='Baremetal API version, default=' +
http.DEFAULT_VER +
' (Env: OS_BAREMETAL_API_VERSION). '
'Use "latest" for the latest known API version. '
'The default value will change to "latest" in the Queens '
'release.',
help='Bare metal API version, default="latest" (the maximum version '
'supported by both the client and the server). '
'(Env: OS_BAREMETAL_API_VERSION)',
)
return parser
@ -109,7 +101,8 @@ def _get_environment_version(default):
env_value = default
if env_value == 'latest':
env_value = LATEST_VERSION
OS_BAREMETAL_API_LATEST = True
else:
OS_BAREMETAL_API_LATEST = False
return env_value
@ -120,12 +113,16 @@ class ReplaceLatestVersion(argparse.Action):
breaks the major version detection (OSC tries to load configuration options
from setuptools entrypoint openstack.baremetal.vlatest). This action
replaces "latest" with the latest known version, and sets the global
OS_BAREMETAL_API_LATEST flag to True.
OS_BAREMETAL_API_LATEST flag appropriately.
"""
def __call__(self, parser, namespace, values, option_string=None):
global OS_BAREMETAL_API_VERSION_SPECIFIED, OS_BAREMETAL_API_LATEST
OS_BAREMETAL_API_VERSION_SPECIFIED = True
global OS_BAREMETAL_API_LATEST
if values == 'latest':
values = LATEST_VERSION
# The default value of "True" may have been overriden due to
# non-empty OS_BAREMETAL_API_VERSION env variable. If a user
# explicitly requests "latest", we need to correct it.
OS_BAREMETAL_API_LATEST = True
else:
OS_BAREMETAL_API_LATEST = False
setattr(namespace, self.dest, values)

View File

@ -27,26 +27,6 @@ class BaremetalNodeTests(base.TestCase):
super(BaremetalNodeTests, self).setUp()
self.node = self.node_create()
def test_warning_version_not_specified(self):
"""Test API version warning is printed when API version unspecified.
A warning will appear for any invocation of the baremetal OSC plugin
without --os-baremetal-api-version specified. It's tested with a simple
node list command here.
"""
output = self.openstack('baremetal node list', merge_stderr=True)
self.assertIn('the default will be the latest API version', output)
def test_no_warning_version_specified(self):
"""Test API version warning is not printed when API version specified.
This warning should not appear when a user specifies the ironic API
version to use.
"""
output = self.openstack('baremetal --os-baremetal-api-version=1.9 node'
' list', merge_stderr=True)
self.assertNotIn('the default will be the latest API version', output)
def test_create_name_uuid(self):
"""Check baremetal node create command with name and UUID.
@ -64,10 +44,25 @@ class BaremetalNodeTests(base.TestCase):
self.assertEqual(node_info['name'], name)
self.assertEqual(node_info['driver'], 'fake')
self.assertEqual(node_info['maintenance'], False)
self.assertEqual(node_info['provision_state'], 'enroll')
node_list = self.node_list()
self.assertIn(uuid, [x['UUID'] for x in node_list])
self.assertIn(name, [x['Name'] for x in node_list])
def test_create_old_api_version(self):
"""Check baremetal node create command with name and UUID.
Test steps:
1) Create baremetal node in setUp.
2) Create one more baremetal node explicitly with old API version
3) Check that node successfully created.
"""
node_info = self.node_create(
params='--os-baremetal-api-version 1.5')
self.assertEqual(node_info['driver'], 'fake')
self.assertEqual(node_info['maintenance'], False)
self.assertEqual(node_info['provision_state'], 'available')
@ddt.data('name', 'uuid')
def test_delete(self, key):
"""Check baremetal node delete command with name/UUID argument.

View File

@ -22,20 +22,38 @@ class ProvisionStateTests(base.TestCase):
super(ProvisionStateTests, self).setUp()
self.node = self.node_create()
def test_deploy_rebuild_undeploy(self):
def test_deploy_rebuild_undeploy_manage(self):
"""Deploy, rebuild and undeploy node.
Test steps:
1) Create baremetal node in setUp.
2) Check initial "available" provision state.
3) Set baremetal node "deploy" provision state.
4) Check baremetal node provision_state field value is "active".
5) Set baremetal node "rebuild" provision state.
6) Check baremetal node provision_state field value is "active".
7) Set baremetal node "undeploy" provision state.
8) Check baremetal node provision_state field value is "available".
2) Check initial "enroll" provision state.
3) Set baremetal node "manage" provision state.
4) Check baremetal node provision_state field value is "manageable".
5) Set baremetal node "provide" provision state.
6) Check baremetal node provision_state field value is "available".
7) Set baremetal node "deploy" provision state.
8) Check baremetal node provision_state field value is "active".
9) Set baremetal node "rebuild" provision state.
10) Check baremetal node provision_state field value is "active".
11) Set baremetal node "undeploy" provision state.
12) Check baremetal node provision_state field value is "available".
13) Set baremetal node "manage" provision state.
14) Check baremetal node provision_state field value is "manageable".
15) Set baremetal node "provide" provision state.
16) Check baremetal node provision_state field value is "available".
"""
show_prop = self.node_show(self.node['uuid'], ["provision_state"])
self.assertEqual("enroll", show_prop["provision_state"])
# manage
self.openstack('baremetal node manage {0}'.format(self.node['uuid']))
show_prop = self.node_show(self.node['uuid'], ["provision_state"])
self.assertEqual("manageable", show_prop["provision_state"])
# provide
self.openstack('baremetal node provide {0}'.format(self.node['uuid']))
show_prop = self.node_show(self.node['uuid'], ["provision_state"])
self.assertEqual("available", show_prop["provision_state"])
# deploy
@ -55,21 +73,6 @@ class ProvisionStateTests(base.TestCase):
show_prop = self.node_show(self.node['uuid'], ["provision_state"])
self.assertEqual("available", show_prop["provision_state"])
def test_manage_provide(self):
"""Manage and provide node back.
Steps:
1) Create baremetal node in setUp.
2) Check initial "available" provision state.
3) Set baremetal node "manage" provision state.
4) Check baremetal node provision_state field value is "manageable".
5) Set baremetal node "provide" provision state.
6) Check baremetal node provision_state field value is "available".
"""
show_prop = self.node_show(self.node['uuid'], ["provision_state"])
self.assertEqual("available", show_prop["provision_state"])
# manage
self.openstack('baremetal node manage {0}'.format(self.node['uuid']))
show_prop = self.node_show(self.node['uuid'], ["provision_state"])

View File

@ -19,6 +19,7 @@ import sys
AUTH_TOKEN = "foobar"
AUTH_URL = "http://0.0.0.0"
API_VERSION = '1.6'
class FakeApp(object):
@ -38,7 +39,7 @@ class FakeClientManager(object):
self.interface = 'public'
self._region_name = 'RegionOne'
self.session = 'fake session'
self._api_version = {'baremetal': '1.6'}
self._api_version = {'baremetal': API_VERSION}
class FakeResource(object):

View File

@ -15,7 +15,6 @@ import argparse
import mock
import testtools
from ironicclient.common import http
from ironicclient.osc import plugin
from ironicclient.tests.unit.osc import fakes
from ironicclient.v1 import client
@ -23,87 +22,67 @@ from ironicclient.v1 import client
class MakeClientTest(testtools.TestCase):
@mock.patch.object(plugin.utils, 'env', lambda x: None)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_LATEST', new=False)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_VERSION_SPECIFIED', new=True)
@mock.patch.object(plugin.LOG, 'warning', autospec=True)
@mock.patch.object(client, 'Client', autospec=True)
def test_make_client(self, mock_client, mock_warning):
def test_make_client_explicit_version(self, mock_client):
instance = fakes.FakeClientManager()
instance.get_endpoint_for_service_type = mock.Mock(
return_value='endpoint')
plugin.make_client(instance)
mock_client.assert_called_once_with(os_ironic_api_version='1.6',
allow_api_version_downgrade=False,
session=instance.session,
region_name=instance._region_name,
endpoint='endpoint')
self.assertFalse(mock_warning.called)
instance.get_endpoint_for_service_type.assert_called_once_with(
'baremetal', region_name=instance._region_name,
interface=instance.interface)
@mock.patch.object(plugin.utils, 'env', lambda x: None)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_LATEST', new=False)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_VERSION_SPECIFIED', new=False)
@mock.patch.object(plugin.LOG, 'warning', autospec=True)
@mock.patch.object(client, 'Client', autospec=True)
def test_make_client_log_warning_no_version_specified(self, mock_client,
mock_warning):
instance = fakes.FakeClientManager()
instance.get_endpoint_for_service_type = mock.Mock(
return_value='endpoint')
instance._api_version = {'baremetal': http.DEFAULT_VER}
plugin.make_client(instance)
mock_client.assert_called_once_with(
os_ironic_api_version=http.DEFAULT_VER,
os_ironic_api_version=fakes.API_VERSION,
allow_api_version_downgrade=False,
session=instance.session,
region_name=instance._region_name,
endpoint='endpoint')
self.assertTrue(mock_warning.called)
instance.get_endpoint_for_service_type.assert_called_once_with(
'baremetal', region_name=instance._region_name,
interface=instance.interface)
@mock.patch.object(plugin.utils, 'env', lambda x: None)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_LATEST', new=True)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_VERSION_SPECIFIED', new=True)
@mock.patch.object(plugin.LOG, 'warning', autospec=True)
@mock.patch.object(client, 'Client', autospec=True)
def test_make_client_latest(self, mock_client, mock_warning):
def test_make_client_latest(self, mock_client):
instance = fakes.FakeClientManager()
instance.get_endpoint_for_service_type = mock.Mock(
return_value='endpoint')
instance._api_version = {'baremetal': plugin.LATEST_VERSION}
plugin.make_client(instance)
mock_client.assert_called_once_with(os_ironic_api_version='1.6',
allow_api_version_downgrade=True,
session=instance.session,
region_name=instance._region_name,
endpoint='endpoint')
self.assertFalse(mock_warning.called)
mock_client.assert_called_once_with(
# NOTE(dtantsur): "latest" is changed to an actual version before
# make_client is called.
os_ironic_api_version=plugin.LATEST_VERSION,
allow_api_version_downgrade=True,
session=instance.session,
region_name=instance._region_name,
endpoint='endpoint')
instance.get_endpoint_for_service_type.assert_called_once_with(
'baremetal', region_name=instance._region_name,
interface=instance.interface)
@mock.patch.object(plugin.utils, 'env', lambda x: '1.29')
@mock.patch.object(plugin, 'OS_BAREMETAL_API_LATEST', new=False)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_VERSION_SPECIFIED', new=False)
@mock.patch.object(plugin.LOG, 'warning', autospec=True)
@mock.patch.object(client, 'Client', autospec=True)
def test_make_client_version_from_env_no_warning(self, mock_client,
mock_warning):
def test_make_client_v1(self, mock_client):
instance = fakes.FakeClientManager()
instance.get_endpoint_for_service_type = mock.Mock(
return_value='endpoint')
instance._api_version = {'baremetal': '1'}
plugin.make_client(instance)
self.assertFalse(mock_warning.called)
mock_client.assert_called_once_with(
os_ironic_api_version=plugin.LATEST_VERSION,
allow_api_version_downgrade=True,
session=instance.session,
region_name=instance._region_name,
endpoint='endpoint')
instance.get_endpoint_for_service_type.assert_called_once_with(
'baremetal', region_name=instance._region_name,
interface=instance.interface)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_LATEST', new=True)
@mock.patch.object(argparse.ArgumentParser, 'add_argument', autospec=True)
class BuildOptionParserTest(testtools.TestCase):
@mock.patch.object(plugin.utils, 'env', lambda x: None)
@mock.patch.object(argparse.ArgumentParser, 'add_argument', autospec=True)
def test_build_option_parser(self, mock_add_argument):
parser = argparse.ArgumentParser()
mock_add_argument.reset_mock()
@ -113,11 +92,11 @@ class BuildOptionParserTest(testtools.TestCase):
mock_add_argument.assert_called_once_with(
mock.ANY, '--os-baremetal-api-version',
action=plugin.ReplaceLatestVersion, choices=version_list,
default=http.DEFAULT_VER, help=mock.ANY,
default=plugin.LATEST_VERSION, help=mock.ANY,
metavar='<baremetal-api-version>')
self.assertTrue(plugin.OS_BAREMETAL_API_LATEST)
@mock.patch.object(plugin.utils, 'env', lambda x: "latest")
@mock.patch.object(argparse.ArgumentParser, 'add_argument', autospec=True)
def test_build_option_parser_env_latest(self, mock_add_argument):
parser = argparse.ArgumentParser()
mock_add_argument.reset_mock()
@ -129,26 +108,27 @@ class BuildOptionParserTest(testtools.TestCase):
action=plugin.ReplaceLatestVersion, choices=version_list,
default=plugin.LATEST_VERSION, help=mock.ANY,
metavar='<baremetal-api-version>')
self.assertTrue(plugin.OS_BAREMETAL_API_LATEST)
@mock.patch.object(plugin.utils, 'env', autospec=True)
def test__get_environment_version(self, mock_utils_env):
mock_utils_env.return_value = 'latest'
result = plugin._get_environment_version(None)
self.assertEqual(plugin.LATEST_VERSION, result)
mock_utils_env.return_value = None
result = plugin._get_environment_version('1.22')
self.assertEqual("1.22", result)
mock_utils_env.return_value = "1.23"
result = plugin._get_environment_version('1.9')
self.assertEqual("1.23", result)
@mock.patch.object(plugin.utils, 'env', lambda x: "1.1")
def test_build_option_parser_env(self, mock_add_argument):
parser = argparse.ArgumentParser()
mock_add_argument.reset_mock()
plugin.build_option_parser(parser)
version_list = ['1'] + ['1.%d' % i for i in range(
1, plugin.LAST_KNOWN_API_VERSION + 1)] + ['latest']
mock_add_argument.assert_called_once_with(
mock.ANY, '--os-baremetal-api-version',
action=plugin.ReplaceLatestVersion, choices=version_list,
default='1.1', help=mock.ANY,
metavar='<baremetal-api-version>')
self.assertFalse(plugin.OS_BAREMETAL_API_LATEST)
@mock.patch.object(plugin.utils, 'env', lambda x: None)
class ReplaceLatestVersionTest(testtools.TestCase):
@mock.patch.object(plugin, 'OS_BAREMETAL_API_LATEST', new=False)
@mock.patch.object(plugin.utils, 'env', lambda x: None)
def test___call___latest(self):
parser = argparse.ArgumentParser()
plugin.build_option_parser(parser)
@ -157,11 +137,9 @@ class ReplaceLatestVersionTest(testtools.TestCase):
namespace)
self.assertEqual(plugin.LATEST_VERSION,
namespace.os_baremetal_api_version)
self.assertTrue(plugin.OS_BAREMETAL_API_VERSION_SPECIFIED)
self.assertTrue(plugin.OS_BAREMETAL_API_LATEST)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_LATEST', new=False)
@mock.patch.object(plugin.utils, 'env', lambda x: None)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_LATEST', new=True)
def test___call___specific_version(self):
parser = argparse.ArgumentParser()
plugin.build_option_parser(parser)
@ -169,5 +147,14 @@ class ReplaceLatestVersionTest(testtools.TestCase):
parser.parse_known_args(['--os-baremetal-api-version', '1.4'],
namespace)
self.assertEqual('1.4', namespace.os_baremetal_api_version)
self.assertTrue(plugin.OS_BAREMETAL_API_VERSION_SPECIFIED)
self.assertFalse(plugin.OS_BAREMETAL_API_LATEST)
@mock.patch.object(plugin, 'OS_BAREMETAL_API_LATEST', new=True)
def test___call___default(self):
parser = argparse.ArgumentParser()
plugin.build_option_parser(parser)
namespace = argparse.Namespace()
parser.parse_known_args([], namespace)
self.assertEqual(plugin.LATEST_VERSION,
namespace.os_baremetal_api_version)
self.assertTrue(plugin.OS_BAREMETAL_API_LATEST)

View File

@ -0,0 +1,26 @@
---
upgrade:
- |
The default API version for the bare metal OSC client (``openstack
baremetal`` commands) 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
``OS_BAREMETAL_API_VERSION`` environment variable or use the
``--os-baremetal-api-version`` CLI argument.
.. note:: This change does not affect the Python API.
features:
- |
The bare metal OSC client (``openstack baremetal`` commands) 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 ``openstack baremetal`` commands 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.