From 2d844d775a722bf15243214a3c406f37a35e23c4 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 11 Mar 2020 11:29:26 +0100 Subject: [PATCH] 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(, "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 | 9 +++++++++ openstack/baremetal/v1/node.py | 11 ++++++----- openstack/resource.py | 5 ++++- .../baremetal/test_baremetal_node.py | 19 +++++++++++++++++++ .../tests/unit/baremetal/v1/test_node.py | 7 +++++++ ...on-state-negotiation-0155b4d0e932054c.yaml | 12 ++++++++++++ 6 files changed, 57 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/provision-state-negotiation-0155b4d0e932054c.yaml diff --git a/openstack/baremetal/v1/_common.py b/openstack/baremetal/v1/_common.py index 4217c2909..8fcf7b579 100644 --- a/openstack/baremetal/v1/_common.py +++ b/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): diff --git a/openstack/baremetal/v1/node.py b/openstack/baremetal/v1/node.py index 868d6ad3f..7a515af8f 100644 --- a/openstack/baremetal/v1/node.py +++ b/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, diff --git a/openstack/resource.py b/openstack/resource.py index 6cefb5bd8..f48ef2331 100644 --- a/openstack/resource.py +++ b/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) diff --git a/openstack/tests/functional/baremetal/test_baremetal_node.py b/openstack/tests/functional/baremetal/test_baremetal_node.py index 5a8552de4..88e6f8011 100644 --- a/openstack/tests/functional/baremetal/test_baremetal_node.py +++ b/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) diff --git a/openstack/tests/unit/baremetal/v1/test_node.py b/openstack/tests/unit/baremetal/v1/test_node.py index a1a4dc04f..c2c1b5ab1 100644 --- a/openstack/tests/unit/baremetal/v1/test_node.py +++ b/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): diff --git a/releasenotes/notes/provision-state-negotiation-0155b4d0e932054c.yaml b/releasenotes/notes/provision-state-negotiation-0155b4d0e932054c.yaml new file mode 100644 index 000000000..3656cf9b3 --- /dev/null +++ b/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.