Allow API user to define list of versions

In cases where one may need to support multiple API
micro-versions, it makes sense to allow a user to submit
the list of versions their code can support, as long as
they have the visibility into that version.

Adds the ability to pass in a list to the
os_ironic_api_version value during client
initialization, and facilitate the negotiation
of the highest available version.

Change-Id: I0dfa3f7fe0a1e2aaf31d37c46b65cc6c064b5e86
Related-Bug: #1739440
Related-Bug: #1671145

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

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)

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'

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',

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.