diff --git a/ironicclient/osc/plugin.py b/ironicclient/osc/plugin.py index e1f9764e5..2a49b4bbb 100644 --- a/ironicclient/osc/plugin.py +++ b/ironicclient/osc/plugin.py @@ -27,6 +27,7 @@ LOG = logging.getLogger(__name__) 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' for i in range(1, LAST_KNOWN_API_VERSION + 1) @@ -75,9 +76,7 @@ def build_option_parser(parser): parser.add_argument( '--os-baremetal-api-version', metavar='', - default=utils.env( - 'OS_BAREMETAL_API_VERSION', - default=http.DEFAULT_VER), + default=_get_environment_version(http.DEFAULT_VER), choices=sorted( API_VERSIONS, key=lambda k: [int(x) for x in k.split('.')]) + ['latest'], @@ -92,14 +91,20 @@ def build_option_parser(parser): return parser +def _get_environment_version(default): + env_value = utils.env('OS_BAREMETAL_API_VERSION') + if not env_value: + return default + if env_value == 'latest': + env_value = LATEST_VERSION + return env_value + + class ReplaceLatestVersion(argparse.Action): """Replaces `latest` keyword by last known version.""" def __call__(self, parser, namespace, values, option_string=None): global OS_BAREMETAL_API_VERSION_SPECIFIED OS_BAREMETAL_API_VERSION_SPECIFIED = True - latest = values == 'latest' - if latest: - values = '1.%d' % LAST_KNOWN_API_VERSION - LOG.debug("Replacing 'latest' API version with the " - "latest known version '%s'", values) + if values == 'latest': + values = LATEST_VERSION setattr(namespace, self.dest, values) diff --git a/ironicclient/tests/unit/osc/test_plugin.py b/ironicclient/tests/unit/osc/test_plugin.py index a687d4797..3dcf4d696 100644 --- a/ironicclient/tests/unit/osc/test_plugin.py +++ b/ironicclient/tests/unit/osc/test_plugin.py @@ -23,6 +23,7 @@ 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_VERSION_SPECIFIED', new=True) @mock.patch.object(plugin.LOG, 'warning', autospec=True) @mock.patch.object(client, 'Client', autospec=True) @@ -40,6 +41,7 @@ class MakeClientTest(testtools.TestCase): '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_VERSION_SPECIFIED', new=False) @mock.patch.object(plugin.LOG, 'warning', autospec=True) @mock.patch.object(client, 'Client', autospec=True) @@ -75,6 +77,7 @@ class MakeClientTest(testtools.TestCase): class BuildOptionParserTest(testtools.TestCase): + @mock.patch.object(plugin.utils, 'env', lambda x: None) @mock.patch.object(argparse.ArgumentParser, 'add_argument') def test_build_option_parser(self, mock_add_argument): parser = argparse.ArgumentParser() @@ -87,19 +90,48 @@ class BuildOptionParserTest(testtools.TestCase): choices=version_list, default=http.DEFAULT_VER, help=mock.ANY, metavar='') + @mock.patch.object(plugin.utils, 'env', lambda x: "latest") + @mock.patch.object(argparse.ArgumentParser, 'add_argument') + def test_build_option_parser_env_latest(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( + '--os-baremetal-api-version', action=plugin.ReplaceLatestVersion, + choices=version_list, default=plugin.LATEST_VERSION, help=mock.ANY, + metavar='') + + @mock.patch.object(plugin.utils, 'env') + 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) + class ReplaceLatestVersionTest(testtools.TestCase): + @mock.patch.object(plugin.utils, 'env', lambda x: None) def test___call___latest(self): parser = argparse.ArgumentParser() plugin.build_option_parser(parser) namespace = argparse.Namespace() parser.parse_known_args(['--os-baremetal-api-version', 'latest'], namespace) - self.assertEqual('1.%d' % plugin.LAST_KNOWN_API_VERSION, + self.assertEqual(plugin.LATEST_VERSION, namespace.os_baremetal_api_version) self.assertTrue(plugin.OS_BAREMETAL_API_VERSION_SPECIFIED) + @mock.patch.object(plugin.utils, 'env', lambda x: None) def test___call___specific_version(self): parser = argparse.ArgumentParser() plugin.build_option_parser(parser) diff --git a/releasenotes/notes/bug-1712935-allow-os_baremetal_api_version_env_var_to_be_latest-28c8eed24f389673.yaml b/releasenotes/notes/bug-1712935-allow-os_baremetal_api_version_env_var_to_be_latest-28c8eed24f389673.yaml new file mode 100644 index 000000000..ce93b9975 --- /dev/null +++ b/releasenotes/notes/bug-1712935-allow-os_baremetal_api_version_env_var_to_be_latest-28c8eed24f389673.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fix issue where doing ``export OS_BAREMETAL_API_VERSION=latest`` would + cause the ``openstack baremetal`` commands to fail. See `bug 1712935 + `_.