Fix microversion negotiation in some bare metal node call

Using utils.pick_microversion means that the result may be None,
which is likely lower than a version negotiated for the resource.

For example, when calling set_node_provision_state(<node name>, "provide"),
it is determined that "provide" does not require a non-default microversion,
so None is used, breaking using node name.

This change switches set_node_provision_state, set_node_power_state and
patch_node to _assert_microversion_for that takes into account the
microversion negotiated for the resource.

Change-Id: Ia81d8a39ca1c8407c689e7d128ace82071b52a01

openstack/baremetal/v1/_common.py

@@ -58,6 +58,15 @@ STATE_VERSIONS = {
 VIF_VERSION = '1.28'
 """API version in which the VIF operations were introduced."""
 
+CONFIG_DRIVE_REBUILD_VERSION = '1.35'
+"""API version in which rebuild accepts a configdrive."""
+
+RESET_INTERFACES_VERSION = '1.45'
+"""API version in which the reset_interfaces parameter was introduced."""
+
+CONFIG_DRIVE_DICT_VERSION = '1.56'
+"""API version in which configdrive can be a dictionary."""
+
 
 class ListMixin(object):
 

openstack/baremetal/v1/node.py

@@ -346,11 +346,11 @@ class Node(_common.ListMixin, resource.Resource):
         if config_drive:
             # Some config drive actions require a higher version.
             if isinstance(config_drive, dict):
-                version = '1.56'
+                version = _common.CONFIG_DRIVE_DICT_VERSION
             elif target == 'rebuild':
-                version = '1.35'
+                version = _common.CONFIG_DRIVE_REBUILD_VERSION
 
-        version = utils.pick_microversion(session, version)
+        version = self._assert_microversion_for(session, 'commit', version)
 
         body = {'target': target}
         if config_drive:
@@ -532,7 +532,7 @@ class Node(_common.ListMixin, resource.Resource):
         else:
             version = None
 
-        version = utils.pick_microversion(session, version)
+        version = self._assert_microversion_for(session, 'commit', version)
 
         # TODO(dtantsur): server timeout support
         body = {'target': target}
@@ -854,7 +854,8 @@ class Node(_common.ListMixin, resource.Resource):
                 raise exceptions.MethodNotSupported(self, "patch")
 
             session = self._get_session(session)
-            microversion = utils.pick_microversion(session, '1.45')
+            microversion = self._assert_microversion_for(
+                session, 'commit', _common.RESET_INTERFACES_VERSION)
             params = [('reset_interfaces', reset_interfaces)]
 
             request = self._prepare_request(requires_id=True,

openstack/resource.py

@@ -1219,7 +1219,10 @@ class Resource(dict):
             raise exceptions.NotSupported(message)
 
         actual = self._get_microversion_for(session, action)
-        if actual is None:
+
+        if expected is None:
+            return actual
+        elif actual is None:
             message = ("API version %s is required, but the default "
                        "version will be used.") % expected
             _raise(message)

openstack/tests/functional/baremetal/test_baremetal_node.py

@@ -10,6 +10,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import random
 import uuid
 
 from openstack import exceptions
@@ -154,6 +155,24 @@ class TestBareMetalNode(base.BaseBaremetalTest):
                                                      wait=True)
         self.assertEqual(node.provision_state, 'available')
 
+    def test_node_create_in_enroll_provide_by_name(self):
+        name = 'node-%d' % random.randint(0, 1000)
+        node = self.create_node(provision_state='enroll', name=name)
+        self.node_id = node.id
+
+        self.assertEqual(node.driver, 'fake-hardware')
+        self.assertEqual(node.provision_state, 'enroll')
+        self.assertIsNone(node.power_state)
+        self.assertFalse(node.is_maintenance)
+
+        node = self.conn.baremetal.set_node_provision_state(name, 'manage',
+                                                            wait=True)
+        self.assertEqual(node.provision_state, 'manageable')
+
+        node = self.conn.baremetal.set_node_provision_state(name, 'provide',
+                                                            wait=True)
+        self.assertEqual(node.provision_state, 'available')
+
     def test_node_power_state(self):
         node = self.create_node()
         self.assertIsNone(node.power_state)

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

@@ -229,6 +229,11 @@ class TestNodeWaitForProvisionState(base.TestCase):
                           abort_on_failed_state=False)
 
 
+def _fake_assert(self, session, action, expected, error_message=None):
+    return expected
+
+
+@mock.patch.object(node.Node, '_assert_microversion_for', _fake_assert)
 @mock.patch.object(node.Node, 'fetch', lambda self, session: self)
 @mock.patch.object(exceptions, 'raise_from_response', mock.Mock())
 class TestNodeSetProvisionState(base.TestCase):
@@ -558,6 +563,7 @@ class TestNodeWaitForReservation(base.TestCase):
         mock_fetch.assert_called_with(self.node, self.session)
 
 
+@mock.patch.object(node.Node, '_assert_microversion_for', _fake_assert)
 @mock.patch.object(exceptions, 'raise_from_response', mock.Mock())
 class TestNodeSetPowerState(base.TestCase):
 
@@ -769,6 +775,7 @@ class TestNodeTraits(base.TestCase):
             retriable_status_codes=_common.RETRIABLE_STATUS_CODES)
 
 
+@mock.patch.object(node.Node, '_assert_microversion_for', _fake_assert)
 @mock.patch.object(resource.Resource, 'patch', autospec=True)
 class TestNodePatch(base.TestCase):
 

releasenotes/notes/provision-state-negotiation-0155b4d0e932054c.yaml

@@ -0,0 +1,12 @@
+---
+fixes:
+  - |
+    Fixes API version negotiation in the following bare metal node calls:
+
+    * ``set_node_provision_state``
+    * ``set_node_power_state``
+    * ``patch_node``
+
+    Previously an unexpectingly low version could be negotiated, breaking
+    certain features, for example calling the ``provide`` provisioning action
+    with a node name.