Respect default microversion in the microversion negotiation

When the client explicitly chooses the desired microversion (openstack
--os-compute-api-version=2.5 console url show test-server) we need to
respect this choice. If some feature require particular microversion -
ensure it is <= default mv and supported by the server, otherwise do nothing.

Change-Id: Ib94f5c9212d00945f378f035563f78fd61d21fa3

openstack/tests/unit/baremetal/v1/test_node.py

@@ -18,6 +18,7 @@ from openstack.baremetal.v1 import _common
 from openstack.baremetal.v1 import node
 from openstack import exceptions
 from openstack import resource
+from openstack import utils
 from openstack.tests.unit import base
 
 # NOTE: Sample data from api-ref doc
@@ -731,6 +732,7 @@ class TestNodeSetBootDevice(base.TestCase):
             retriable_status_codes=_common.RETRIABLE_STATUS_CODES)
 
 
+@mock.patch.object(utils, 'pick_microversion', lambda session, v: v)
 @mock.patch.object(node.Node, 'fetch', lambda self, session: self)
 @mock.patch.object(exceptions, 'raise_from_response', mock.Mock())
 class TestNodeTraits(base.TestCase):

openstack/tests/unit/compute/v2/test_server.py

@@ -447,6 +447,7 @@ class TestServer(base.TestCase):
                                        min_microversion='2.1',
                                        max_microversion='2.56')
         self.sess.get_endpoint_data.return_value = self.endpoint_data
+        self.sess.default_microversion = None
 
         image_id = sot.create_image(self.sess, name, metadata)
 
@@ -474,6 +475,7 @@ class TestServer(base.TestCase):
                                        min_microversion='2.1',
                                        max_microversion='2.56')
         self.sess.get_endpoint_data.return_value = self.endpoint_data
+        self.sess.default_microversion = None
 
         self.assertIsNone(self.resp.body, sot.create_image(self.sess, name))
 
@@ -900,6 +902,7 @@ class TestServer(base.TestCase):
             min_microversion = '2.1'
             max_microversion = '2.25'
         self.sess.get_endpoint_data.return_value = FakeEndpointData()
+        self.sess.default_microversion = None
 
         res = sot.live_migrate(
             self.sess, host='HOST2', force=True, block_migration=False)
@@ -924,6 +927,7 @@ class TestServer(base.TestCase):
             min_microversion = '2.1'
             max_microversion = '2.25'
         self.sess.get_endpoint_data.return_value = FakeEndpointData()
+        self.sess.default_microversion = None
 
         res = sot.live_migrate(
             self.sess, host='HOST2', force=True, block_migration=None)
@@ -948,6 +952,7 @@ class TestServer(base.TestCase):
             min_microversion = '2.1'
             max_microversion = '2.30'
         self.sess.get_endpoint_data.return_value = FakeEndpointData()
+        self.sess.default_microversion = None
 
         res = sot.live_migrate(
             self.sess, host='HOST2', force=False, block_migration=False)
@@ -972,6 +977,7 @@ class TestServer(base.TestCase):
             min_microversion = '2.1'
             max_microversion = '2.30'
         self.sess.get_endpoint_data.return_value = FakeEndpointData()
+        self.sess.default_microversion = None
 
         res = sot.live_migrate(
             self.sess, host='HOST2', force=True, block_migration=None)

openstack/tests/unit/test_utils.py

@@ -133,6 +133,67 @@ class Test_urljoin(base.TestCase):
         self.assertEqual(result, u"http://www.example.com/ascii/extra_chars-™")
 
 
+class TestSupportsMicroversion(base.TestCase):
+    def setUp(self):
+        super(TestSupportsMicroversion, self).setUp()
+        self.adapter = mock.Mock(spec=['get_endpoint_data'])
+        self.endpoint_data = mock.Mock(spec=['min_microversion',
+                                             'max_microversion'],
+                                       min_microversion='1.1',
+                                       max_microversion='1.99')
+        self.adapter.get_endpoint_data.return_value = self.endpoint_data
+
+    def test_requested_supported_no_default(self):
+        self.adapter.default_microversion = None
+        self.assertTrue(
+            utils.supports_microversion(self.adapter, '1.2'))
+
+    def test_requested_not_supported_no_default(self):
+        self.adapter.default_microversion = None
+        self.assertFalse(
+            utils.supports_microversion(self.adapter, '2.2'))
+
+    def test_requested_not_supported_no_default_exception(self):
+        self.adapter.default_microversion = None
+        self.assertRaises(
+            exceptions.SDKException,
+            utils.supports_microversion,
+            self.adapter,
+            '2.2',
+            True)
+
+    def test_requested_supported_higher_default(self):
+        self.adapter.default_microversion = '1.8'
+        self.assertTrue(
+            utils.supports_microversion(self.adapter, '1.6'))
+
+    def test_requested_supported_equal_default(self):
+        self.adapter.default_microversion = '1.8'
+        self.assertTrue(
+            utils.supports_microversion(self.adapter, '1.8'))
+
+    def test_requested_supported_lower_default(self):
+        self.adapter.default_microversion = '1.2'
+        self.assertFalse(
+            utils.supports_microversion(self.adapter, '1.8'))
+
+    def test_requested_supported_lower_default_exception(self):
+        self.adapter.default_microversion = '1.2'
+        self.assertRaises(
+            exceptions.SDKException,
+            utils.supports_microversion,
+            self.adapter,
+            '1.8',
+            True)
+
+    @mock.patch('openstack.utils.supports_microversion')
+    def test_require_microversion(self, sm_mock):
+        utils.require_microversion(self.adapter, '1.2')
+        sm_mock.assert_called_with(self.adapter,
+                                   '1.2',
+                                   raise_exception=True)
+
+
 class TestMaximumSupportedMicroversion(base.TestCase):
     def setUp(self):
         super(TestMaximumSupportedMicroversion, self).setUp()

openstack/utils.py

@@ -94,18 +94,23 @@ def get_string_format_keys(fmt_string, old_style=True):
         return keys
 
 
-def supports_microversion(adapter, microversion):
+def supports_microversion(adapter, microversion, raise_exception=False):
     """Determine if the given adapter supports the given microversion.
 
-    Checks the min and max microversion asserted by the service and checks
+    Checks the min and max microversion asserted by the service and checks to
-    to make sure that ``min <= microversion <= max``.
+    make sure that ``min <= microversion <= max``. Current default microversion
+    is taken into consideration if set and verifies that ``microversion <=
+    default``.
 
-    :param adapter:
+    :param adapter: :class:`~keystoneauth1.adapter.Adapter` instance.
-        :class:`~keystoneauth1.adapter.Adapter` instance.
+    :param str microversion: String containing the desired microversion.
-    :param str microversion:
+    :param bool raise_exception: Raise exception when requested microversion
-        String containing the desired microversion.
+        is not supported be the server side or is higher than the current
+        default microversion.
     :returns: True if the service supports the microversion.
     :rtype: bool
+    :raises: :class:`~openstack.exceptions.SDKException` when requested
+        microversion is not supported.
     """
 
     endpoint_data = adapter.get_endpoint_data()
@@ -115,10 +120,41 @@ def supports_microversion(adapter, microversion):
                 endpoint_data.min_microversion,
                 endpoint_data.max_microversion,
                 microversion)):
+        if adapter.default_microversion is not None:
+            # If default_microversion is set - evaluate
+            # whether it match the expectation
+            candidate = discover.normalize_version_number(
+                adapter.default_microversion)
+            required = discover.normalize_version_number(microversion)
+            supports = discover.version_match(required, candidate)
+            if raise_exception and not supports:
+                raise exceptions.SDKException(
+                    'Required microversion {ver} is higher than currently '
+                    'selected {curr}'.format(
+                        ver=microversion,
+                        curr=adapter.default_microversion)
+                )
+            return supports
         return True
+    if raise_exception:
+        raise exceptions.SDKException(
+            'Required microversion {ver} is not supported '
+            'by the server side'.format(ver=microversion)
+        )
     return False
 
 
+def require_microversion(adapter, required):
+    """Require microversion.
+
+    :param adapter: :class:`~keystoneauth1.adapter.Adapter` instance.
+    :param str microversion: String containing the desired microversion.
+    :raises: :class:`~openstack.exceptions.SDKException` when requested
+        microversion is not supported
+    """
+    supports_microversion(adapter, required, raise_exception=True)
+
+
 def pick_microversion(session, required):
     """Get a new microversion if it is higher than session's default.
 
@@ -145,6 +181,10 @@ def pick_microversion(session, required):
                         else required)
 
     if required is not None:
+        if not supports_microversion(session, required):
+            raise exceptions.SDKException(
+                'Requested microversion is not supported by the server side '
+                'or the default microversion is too low')
         return discover.version_to_string(required)
 
 

releasenotes/notes/fix-microversion-354dc70deb2b2f0b.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Modify microversion handling. Microversion chosen by the client/user is
+    respected in the microversion  negotiation. For features, requiring
+    particular microversion, it would be ensured it is supported by the server
+    side and required microversion is <= chosen microversion, otherwise call
+    will be rejected.