Merge "Log warning when API version is not specified for the OSC plugin"

5 files changed, 82 insertions, 3 deletions

diff --git a/ironicclient/osc/plugin.py b/ironicclient/osc/plugin.py
index a1c88ed..01502f4 100644
--- a/ironicclient/osc/plugin.py
+++ b/ironicclient/osc/plugin.py
@@ -32,10 +32,23 @@ API_VERSIONS = {
     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."
+)
 
 
 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)
+
     baremetal_client_class = utils.get_client_class(
         API_NAME,
         instance._api_version[API_NAME],
@@ -80,6 +93,8 @@ def build_option_parser(parser):
 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
diff --git a/ironicclient/tests/functional/base.py b/ironicclient/tests/functional/base.py
index f230755..b954d1a 100644
--- a/ironicclient/tests/functional/base.py
+++ b/ironicclient/tests/functional/base.py
@@ -150,7 +150,7 @@ class FunctionalTestBase(base.ClientTestBase):
             return self.client.cmd_with_auth('ironic',
                                              action, flags, params)
 
-    def _ironic_osc(self, action, flags='', params=''):
+    def _ironic_osc(self, action, flags='', params='', merge_stderr=False):
         """Execute baremetal commands via OpenStack Client."""
         config = self._get_config()
         id_api_version = config.get('functional', 'os_identity_api_version')
@@ -164,7 +164,7 @@ class FunctionalTestBase(base.ClientTestBase):
                     'value': getattr(self, domain_attr)
                 }
         return self.client.cmd_with_auth(
-            'openstack', action, flags, params)
+            'openstack', action, flags, params, merge_stderr=merge_stderr)
 
     def ironic(self, action, flags='', params='', parse=True):
         """Return parsed list of dicts with basic item info.
diff --git a/ironicclient/tests/functional/osc/v1/test_baremetal_node_basic.py b/ironicclient/tests/functional/osc/v1/test_baremetal_node_basic.py
index 9f1f0b6..c968f7e 100644
--- a/ironicclient/tests/functional/osc/v1/test_baremetal_node_basic.py
+++ b/ironicclient/tests/functional/osc/v1/test_baremetal_node_basic.py
@@ -26,6 +26,26 @@ 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.
 
diff --git a/ironicclient/tests/unit/osc/test_plugin.py b/ironicclient/tests/unit/osc/test_plugin.py
index a86f5fe..a687d47 100644
--- a/ironicclient/tests/unit/osc/test_plugin.py
+++ b/ironicclient/tests/unit/osc/test_plugin.py
@@ -23,8 +23,10 @@ from ironicclient.v1 import client
 
 class MakeClientTest(testtools.TestCase):
 
+    @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):
+    def test_make_client(self, mock_client, mock_warning):
         instance = fakes.FakeClientManager()
         instance.get_endpoint_for_service_type = mock.Mock(
             return_value='endpoint')
@@ -33,10 +35,43 @@ class MakeClientTest(testtools.TestCase):
                                             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, '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,
+            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: '1.29')
+    @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):
+        instance = fakes.FakeClientManager()
+        instance.get_endpoint_for_service_type = mock.Mock(
+            return_value='endpoint')
+        plugin.make_client(instance)
+        self.assertFalse(mock_warning.called)
+
 
 class BuildOptionParserTest(testtools.TestCase):
 
@@ -63,6 +98,7 @@ class ReplaceLatestVersionTest(testtools.TestCase):
                                 namespace)
         self.assertEqual('1.%d' % plugin.LAST_KNOWN_API_VERSION,
                          namespace.os_baremetal_api_version)
+        self.assertTrue(plugin.OS_BAREMETAL_API_VERSION_SPECIFIED)
 
     def test___call___specific_version(self):
         parser = argparse.ArgumentParser()
@@ -71,3 +107,4 @@ 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)
diff --git a/releasenotes/notes/implicit-version-warning-d34b99727b50d519.yaml b/releasenotes/notes/implicit-version-warning-d34b99727b50d519.yaml
new file mode 100644
index 0000000..3431415
--- /dev/null
+++ b/releasenotes/notes/implicit-version-warning-d34b99727b50d519.yaml
@@ -0,0 +1,7 @@
+---
+deprecations:
+  - |
+    Currently, the default API version for the OSC plugin 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.