Allow OS_BAREMETAL_API_VERSION=latest to work

Allow OS_BAREMETAL_API_VERSION=latest to work. If
OS_BAREMETAL_API_VERSION=latest then convert to the latest version and
set that as the 'default' value for the os_baremetal_api_version
value.

Remove the LOG.debug() message about setting to latest as the message
never appears in the logfile. As tested with:
  $ openstack --debug --log-file log.txt baremetal node list --os-baremetal-api-version latest

Also it was set to always log the message.

Change-Id: Idc3de8ae55e8267342633668a06d18b45c9e5f0a
Closes-Bug: #1712935
(cherry picked from commit 9d8afb38e8d6a6c61f446c93206d10b4bc35cce2)

3 files changed, 52 insertions, 9 deletions

diff --git a/ironicclient/osc/plugin.py b/ironicclient/osc/plugin.py
index e1f9764..2a49b4b 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='<baremetal-api-version>',
-        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 a687d47..3dcf4d6 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='<baremetal-api-version>')
 
+    @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='<baremetal-api-version>')
+
+    @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 0000000..ce93b99
--- /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
+    <https://bugs.launchpad.net/ironic/+bug/1712935>`_.