diff --git a/ironicclient/osc/plugin.py b/ironicclient/osc/plugin.py index f0b84e06c..a1dc94de6 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 f230755cd..b954d1a02 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 9f1f0b692..c968f7e56 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 a86f5fe58..a687d4797 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 000000000..34314159e --- /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.