diff options
author | Zuul <zuul@review.openstack.org> | 2018-01-24 20:20:15 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2018-01-24 20:20:15 +0000 |
commit | 4b5d6fe2e2332ac8d9fc910173adfa005c25c10e (patch) | |
tree | 330e0212e1971c97a8a59744287b528a9aacfc10 | |
parent | 2214728a03d2fef43c96db36ce1cc6ade5c237cf (diff) | |
parent | 22ab93e8d6af21ef5946cb2515c381332ed59b04 (diff) | |
download | python-ironicclient-4b5d6fe2e2332ac8d9fc910173adfa005c25c10e.tar.gz |
Merge "Allow API user to define list of versions"
-rw-r--r-- | ironicclient/client.py | 3 | ||||
-rw-r--r-- | ironicclient/common/http.py | 52 | ||||
-rw-r--r-- | ironicclient/tests/unit/common/test_http.py | 95 | ||||
-rw-r--r-- | ironicclient/tests/unit/test_client.py | 14 | ||||
-rw-r--r-- | releasenotes/notes/allow-client-to-request-list-of-versions-88f019cad76e6464.yaml | 7 |
5 files changed, 163 insertions, 8 deletions
diff --git a/ironicclient/client.py b/ironicclient/client.py index 1499c67..0c4b111 100644 --- a/ironicclient/client.py +++ b/ironicclient/client.py @@ -58,7 +58,8 @@ def get_client(api_version, os_auth_token=None, ironic_url=None, :param cert_file: path to cert file, deprecated in favour of os_cert :param os_key: path to key file :param key_file: path to key file, deprecated in favour of os_key - :param os_ironic_api_version: ironic API version to use + :param os_ironic_api_version: ironic API version to use or a list of + available API versions to attempt to negotiate. :param max_retries: Maximum number of retries in case of conflict error :param retry_interval: Amount of time (in seconds) between retries in case of conflict error diff --git a/ironicclient/common/http.py b/ironicclient/common/http.py index b549da7..e56bef0 100644 --- a/ironicclient/common/http.py +++ b/ironicclient/common/http.py @@ -101,6 +101,7 @@ class VersionNegotiationMixin(object): """ def _query_server(conn): if (self.os_ironic_api_version and + not isinstance(self.os_ironic_api_version, list) and self.os_ironic_api_version != 'latest'): base_version = ("/v%s" % str(self.os_ironic_api_version).split('.')[0]) @@ -140,7 +141,8 @@ class VersionNegotiationMixin(object): # TODO(TheJulia): We should break this method into several parts, # such as a sanity check/error method. if (self.api_version_select_state == 'user' and - self.os_ironic_api_version != 'latest'): + self.os_ironic_api_version != 'latest' and + not isinstance(self.os_ironic_api_version, list)): raise exc.UnsupportedVersion(textwrap.fill( _("Requested API version %(req)s is not supported by the " "server, client, or the requested operation is not " @@ -157,11 +159,45 @@ class VersionNegotiationMixin(object): % {'req': self.os_ironic_api_version, 'min': min_ver, 'max': max_ver})) - if self.os_ironic_api_version == 'latest': - negotiated_ver = max_ver + if isinstance(self.os_ironic_api_version, six.string_types): + if self.os_ironic_api_version == 'latest': + negotiated_ver = max_ver + else: + negotiated_ver = str( + min(StrictVersion(self.os_ironic_api_version), + StrictVersion(max_ver))) + + elif isinstance(self.os_ironic_api_version, list): + if 'latest' in self.os_ironic_api_version: + raise ValueError(textwrap.fill( + _("The 'latest' API version can not be requested " + "in a list of versions. Please explicitly request " + "'latest' or request only versios between " + "%(min)s to %(max)s") + % {'min': min_ver, 'max': max_ver})) + + versions = [] + for version in self.os_ironic_api_version: + if min_ver <= StrictVersion(version) <= max_ver: + versions.append(StrictVersion(version)) + if versions: + negotiated_ver = str(max(versions)) + else: + raise exc.UnsupportedVersion(textwrap.fill( + _("Requested API version specified and the requested " + "operation was not supported by the client's " + "requested API version %(req)s. Supported " + "version range is: %(min)s to %(max)s") + % {'req': self.os_ironic_api_version, + 'min': min_ver, 'max': max_ver})) + else: - negotiated_ver = str(min(StrictVersion(self.os_ironic_api_version), - StrictVersion(max_ver))) + raise ValueError(textwrap.fill( + _("Requested API version %(req)s type is unsupported. " + "Valid types are Strings such as '1.1', 'latest' " + "or a list of string values representing API versions.") + % {'req': self.os_ironic_api_version})) + if StrictVersion(negotiated_ver) < StrictVersion(min_ver): negotiated_ver = min_ver # server handles microversions, but doesn't support @@ -337,7 +373,8 @@ class HTTPClient(VersionNegotiationMixin): # the self.negotiate_version() call if negotiation occurs. if (self.os_ironic_api_version and self.api_version_select_state == 'user' and - self.os_ironic_api_version == 'latest'): + (self.os_ironic_api_version == 'latest' or + isinstance(self.os_ironic_api_version, list))): self.negotiate_version(self.session, None) # Copy the kwargs so we can reuse the original in case of redirects @@ -552,7 +589,8 @@ class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter): # the self.negotiate_version() call if negotiation occurs. if (self.os_ironic_api_version and self.api_version_select_state == 'user' and - self.os_ironic_api_version == 'latest'): + (self.os_ironic_api_version == 'latest' or + isinstance(self.os_ironic_api_version, list))): self.negotiate_version(self.session, None) kwargs.setdefault('user_agent', USER_AGENT) diff --git a/ironicclient/tests/unit/common/test_http.py b/ironicclient/tests/unit/common/test_http.py index fffc9db..de6e331 100644 --- a/ironicclient/tests/unit/common/test_http.py +++ b/ironicclient/tests/unit/common/test_http.py @@ -223,6 +223,101 @@ class VersionNegotiationMixinTest(utils.BaseTestCase): self.assertEqual(2, mock_pvh.call_count) self.assertEqual(1, mock_save_data.call_count) + @mock.patch.object(filecache, 'save_data', autospec=True) + @mock.patch.object(http.VersionNegotiationMixin, '_make_simple_request', + autospec=True) + @mock.patch.object(http.VersionNegotiationMixin, '_parse_version_headers', + autospec=True) + def test_negotiate_version_server_user_list( + self, mock_pvh, mock_msr, mock_save_data): + # have to retry with simple get + mock_pvh.side_effect = iter([(None, None), ('1.1', '1.26')]) + mock_conn = mock.MagicMock() + self.test_object.api_version_select_state = 'user' + self.test_object.os_ironic_api_version = ['1.1', '1.6', '1.25', + '1.26', '1.26.1', '1.27', + '1.30'] + result = self.test_object.negotiate_version(mock_conn, self.response) + self.assertEqual('1.26', result) + self.assertEqual('negotiated', + self.test_object.api_version_select_state) + self.assertEqual('1.26', + self.test_object.os_ironic_api_version) + + self.assertTrue(mock_msr.called) + self.assertEqual(2, mock_pvh.call_count) + self.assertEqual(1, mock_save_data.call_count) + + @mock.patch.object(filecache, 'save_data', autospec=True) + @mock.patch.object(http.VersionNegotiationMixin, '_make_simple_request', + autospec=True) + @mock.patch.object(http.VersionNegotiationMixin, '_parse_version_headers', + autospec=True) + def test_negotiate_version_server_user_list_fails_nomatch( + self, mock_pvh, mock_msr, mock_save_data): + # have to retry with simple get + mock_pvh.side_effect = iter([(None, None), ('1.2', '1.26')]) + mock_conn = mock.MagicMock() + self.test_object.api_version_select_state = 'user' + self.test_object.os_ironic_api_version = ['1.39', '1.1'] + self.assertRaises( + exc.UnsupportedVersion, + self.test_object.negotiate_version, + mock_conn, self.response) + self.assertEqual('user', + self.test_object.api_version_select_state) + self.assertEqual(['1.39', '1.1'], + self.test_object.os_ironic_api_version) + self.assertEqual(2, mock_pvh.call_count) + self.assertEqual(0, mock_save_data.call_count) + + @mock.patch.object(filecache, 'save_data', autospec=True) + @mock.patch.object(http.VersionNegotiationMixin, '_make_simple_request', + autospec=True) + @mock.patch.object(http.VersionNegotiationMixin, '_parse_version_headers', + autospec=True) + def test_negotiate_version_server_user_list_single_value( + self, mock_pvh, mock_msr, mock_save_data): + # have to retry with simple get + mock_pvh.side_effect = iter([(None, None), ('1.1', '1.26')]) + mock_conn = mock.MagicMock() + self.test_object.api_version_select_state = 'user' + # NOTE(TheJulia): Lets test this value explicitly because the + # minor number is actually the same. + self.test_object.os_ironic_api_version = ['1.01'] + result = self.test_object.negotiate_version(mock_conn, None) + self.assertEqual('1.1', result) + self.assertEqual('negotiated', + self.test_object.api_version_select_state) + self.assertEqual('1.1', + self.test_object.os_ironic_api_version) + self.assertTrue(mock_msr.called) + self.assertEqual(2, mock_pvh.call_count) + self.assertEqual(1, mock_save_data.call_count) + + @mock.patch.object(filecache, 'save_data', autospec=True) + @mock.patch.object(http.VersionNegotiationMixin, '_make_simple_request', + autospec=True) + @mock.patch.object(http.VersionNegotiationMixin, '_parse_version_headers', + autospec=True) + def test_negotiate_version_server_user_list_fails_latest( + self, mock_pvh, mock_msr, mock_save_data): + # have to retry with simple get + mock_pvh.side_effect = iter([(None, None), ('1.1', '1.2')]) + mock_conn = mock.MagicMock() + self.test_object.api_version_select_state = 'user' + self.test_object.os_ironic_api_version = ['1.01', 'latest'] + self.assertRaises( + ValueError, + self.test_object.negotiate_version, + mock_conn, self.response) + self.assertEqual('user', + self.test_object.api_version_select_state) + self.assertEqual(['1.01', 'latest'], + self.test_object.os_ironic_api_version) + self.assertEqual(2, mock_pvh.call_count) + self.assertEqual(0, mock_save_data.call_count) + def test_get_server(self): host = 'ironic-host' port = '6385' diff --git a/ironicclient/tests/unit/test_client.py b/ironicclient/tests/unit/test_client.py index 336a87c..17b51d5 100644 --- a/ironicclient/tests/unit/test_client.py +++ b/ironicclient/tests/unit/test_client.py @@ -58,6 +58,9 @@ class ClientTest(utils.BaseTestCase): interface=kwargs.get('os_endpoint_type') or 'publicURL', region_name=kwargs.get('os_region_name')) if 'os_ironic_api_version' in kwargs: + # NOTE(TheJulia): This does not test the negotiation logic + # as a request must be triggered in order for any verison + # negotiation actions to occur. self.assertEqual(0, mock_retrieve_data.call_count) self.assertEqual(kwargs['os_ironic_api_version'], client.current_api_version) @@ -135,6 +138,17 @@ class ClientTest(utils.BaseTestCase): } self._test_get_client(**kwargs) + def test_get_client_with_api_version_list(self): + kwargs = { + 'os_project_name': 'PROJECT_NAME', + 'os_username': 'USERNAME', + 'os_password': 'PASSWORD', + 'os_auth_url': 'http://localhost:35357/v2.0', + 'os_auth_token': '', + 'os_ironic_api_version': ['1.1', '1.99'], + } + self._test_get_client(**kwargs) + def test_get_client_with_api_version_numeric(self): kwargs = { 'os_project_name': 'PROJECT_NAME', diff --git a/releasenotes/notes/allow-client-to-request-list-of-versions-88f019cad76e6464.yaml b/releasenotes/notes/allow-client-to-request-list-of-versions-88f019cad76e6464.yaml new file mode 100644 index 0000000..447b9c8 --- /dev/null +++ b/releasenotes/notes/allow-client-to-request-list-of-versions-88f019cad76e6464.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + The ``os_ironic_api_version`` parameter now accepts a list of REST + API micro-versions to attempt to negotiate with the remote server. + The highest available microversion in the list will be negotiated + for the remaining lifetime of the client session. |