openstack/openstacksdk
Code Issues Proposed changes

baremetal: rework node creation to be closer to the backend

Browse Source 
The proxy layer has been simplified to stay closer to the wire.
It no longer runs automated cleaning. The initial state of available
is implemented via using an old API version. The default state is
enroll (like in Ironic).

Change-Id: I2894c82d847f8a3dc6bdae1913cb72a1ca4b764b
This commit is contained in:
Dmitry Tantsur 2022-07-11 15:35:37 +02:00
parent 915da1e576
commit 6e1b5e0ac1
7 changed files with 95 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
openstack/baremetal/v1/_common.py
View File

@ -59,6 +59,7 @@ EXPECTED_POWER_STATES = {
"""Mapping of target power states to expected power states."""
STATE_VERSIONS = {
'available': '1.1',
'enroll': '1.11',
'manageable': '1.4',
}

3
openstack/baremetal/v1/_proxy.py
View File

@ -276,6 +276,9 @@ class Proxy(proxy.Proxy):
def create_node(self, **attrs):
"""Create a new node from attributes.
See :meth:`~openstack.baremetal.v1.node.Node.create` for an explanation
of the initial provision state.
:param dict attrs: Keyword arguments that will be used to create a
:class:`~openstack.baremetal.v1.node.Node`.

70
openstack/baremetal/v1/node.py
View File

@ -257,12 +257,21 @@ class Node(_common.ListMixin, resource.Resource):
The overridden version is capable of handling the populated
``provision_state`` field of one of three values: ``enroll``,
``manageable`` or ``available``. The default is currently
``available``, since it's the only state supported by all API versions.
``manageable`` or ``available``. If not provided, the server default
is used (``enroll`` in newer versions of Ironic).
This call does not cause a node to go through automated cleaning.
If you need it, use ``provision_state=manageable`` followed by
a call to :meth:`set_provision_state`.
Note that Bare Metal API 1.4 is required for ``manageable`` and
1.11 is required for ``enroll``.
.. warning::
Using ``provision_state=available`` is only possible with API
versions 1.1 to 1.10 and thus is incompatible with setting any
fields that appeared after 1.11.
:param session: The session to use for making this request.
:type session: :class:`~keystoneauth1.adapter.Adapter`
@ -274,44 +283,41 @@ class Node(_common.ListMixin, resource.Resource):
supported by the server.
"""
expected_provision_state = self.provision_state
if expected_provision_state is None:
expected_provision_state = 'available'
if expected_provision_state not in ('enroll',
'manageable',
'available'):
raise ValueError(
"Node's provision_state must be one of 'enroll', "
"'manageable' or 'available' for creation, got %s" %
expected_provision_state)
session = self._get_session(session)
# Verify that the requested provision state is reachable with the API
# version we are going to use.
try:
expected_version = _common.STATE_VERSIONS[expected_provision_state]
except KeyError:
pass
if expected_provision_state is not None:
# Verify that the requested provision state is reachable with
# the API version we are going to use.
try:
microversion = _common.STATE_VERSIONS[
expected_provision_state]
except KeyError:
raise ValueError(
"Node's provision_state must be one of %s for creation, "
"got %s" % (', '.join(_common.STATE_VERSIONS),
expected_provision_state))
else:
error_message = ("Cannot create a node with initial provision "
"state %s" % expected_provision_state)
# Nodes cannot be created as available using new API versions
maximum = ('1.10' if expected_provision_state == 'available'
else None)
microversion = self._assert_microversion_for(
session, 'create', microversion, maximum=maximum,
error_message=error_message,
)
else:
self._assert_microversion_for(
session, 'create', expected_version,
error_message="Cannot create a node with initial provision "
"state %s" % expected_provision_state)
microversion = None # use the base negotiation
# Ironic cannot set provision_state itself, so marking it as unchanged
self._clean_body_attrs({'provision_state'})
super(Node, self).create(session, *args, **kwargs)
if (self.provision_state == 'enroll'
and expected_provision_state != 'enroll'):
self.set_provision_state(session, 'manage', wait=True)
super(Node, self).create(session, *args, microversion=microversion,
**kwargs)
if (self.provision_state == 'manageable'
and expected_provision_state == 'available'):
self.set_provision_state(session, 'provide', wait=True)
if (self.provision_state == 'available'
and expected_provision_state == 'manageable'):
if (expected_provision_state == 'manageable'
and self.provision_state != 'manageable'):
# Manageable is not reachable directly
self.set_provision_state(session, 'manage', wait=True)
return self

30
openstack/resource.py
View File

@ -1342,6 +1342,7 @@ class Resource(dict):
action,
expected,
error_message=None,
maximum=None,
):
"""Enforce that the microversion for action satisfies the requirement.
@ -1350,6 +1351,7 @@ class Resource(dict):
:param expected: Expected microversion.
:param error_message: Optional error message with details. Will be
prepended to the message generated here.
:param maximum: Maximum microversion.
:return: resulting microversion as string.
:raises: :exc:`~openstack.exceptions.NotSupported` if the version
used for the action is lower than the expected one.
@ -1364,23 +1366,29 @@ class Resource(dict):
actual = self._get_microversion(session, action=action)
if expected is None:
return actual
elif actual is None:
if actual is None:
message = (
"API version %s is required, but the default "
"version will be used."
) % expected
_raise(message)
actual_n = discover.normalize_version_number(actual)
expected_n = discover.normalize_version_number(expected)
if actual_n < expected_n:
message = (
"API version %(expected)s is required, but %(actual)s "
"will be used."
) % {'expected': expected, 'actual': actual}
_raise(message)
if expected is not None:
expected_n = discover.normalize_version_number(expected)
if actual_n < expected_n:
message = (
"API version %(expected)s is required, but %(actual)s "
"will be used."
) % {'expected': expected, 'actual': actual}
_raise(message)
if maximum is not None:
maximum_n = discover.normalize_version_number(maximum)
# Assume that if a service supports higher versions, it also
# supports lower ones. Breaks for services that remove old API
# versions (which is not something they should do).
if actual_n > maximum_n:
return maximum
return actual

36
openstack/tests/functional/baremetal/test_baremetal_node.py
View File

@ -22,7 +22,7 @@ class TestBareMetalNode(base.BaseBaremetalTest):
node = self.create_node(name='node-name')
self.assertEqual(node.name, 'node-name')
self.assertEqual(node.driver, 'fake-hardware')
self.assertEqual(node.provision_state, 'available')
self.assertEqual(node.provision_state, 'enroll')
self.assertFalse(node.is_maintenance)
# NOTE(dtantsur): get_node and find_node only differ in handing missing
@ -50,6 +50,16 @@ class TestBareMetalNode(base.BaseBaremetalTest):
self.assertRaises(exceptions.ResourceNotFound,
self.conn.baremetal.get_node, self.node_id)
def test_node_create_in_available(self):
node = self.create_node(name='node-name', provision_state='available')
self.assertEqual(node.name, 'node-name')
self.assertEqual(node.driver, 'fake-hardware')
self.assertEqual(node.provision_state, 'available')
self.conn.baremetal.delete_node(node, ignore_missing=False)
self.assertRaises(exceptions.ResourceNotFound,
self.conn.baremetal.get_node, self.node_id)
def test_node_update(self):
node = self.create_node(name='node-name', extra={'foo': 'bar'})
node.name = 'new-name'
@ -128,7 +138,7 @@ class TestBareMetalNode(base.BaseBaremetalTest):
self.create_node(name='node-name', extra={'foo': 'bar'})
node = next(n for n in
self.conn.baremetal.nodes(details=True,
provision_state='available',
provision_state='enroll',
is_maintenance=False,
associated=False)
if n.name == 'node-name')
@ -139,7 +149,7 @@ class TestBareMetalNode(base.BaseBaremetalTest):
self.conn.baremetal.delete_node(node, ignore_missing=False)
def test_node_create_in_enroll_provide(self):
node = self.create_node(provision_state='enroll')
node = self.create_node()
self.node_id = node.id
self.assertEqual(node.driver, 'fake-hardware')
@ -157,7 +167,7 @@ class TestBareMetalNode(base.BaseBaremetalTest):
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)
node = self.create_node(name=name)
self.node_id = node.id
self.assertEqual(node.driver, 'fake-hardware')
@ -297,16 +307,6 @@ class TestNodeRetired(base.BaseBaremetalTest):
node = self.create_node()
# Set retired when node state available should fail!
self.assertRaises(
exceptions.ConflictException,
self.conn.baremetal.update_node, node, is_retired=True)
# Set node state to manageable
self.conn.baremetal.set_node_provision_state(node, 'manage',
wait=True)
self.assertEqual(node.provision_state, 'manageable')
# Set retired without reason
node = self.conn.baremetal.update_node(node, is_retired=True)
self.assertTrue(node.is_retired)
@ -348,6 +348,14 @@ class TestNodeRetired(base.BaseBaremetalTest):
self.assertTrue(node.is_retired)
self.assertEqual(reason, node.retired_reason)
def test_retired_in_available(self):
node = self.create_node(provision_state='available')
# Set retired when node state available should fail!
self.assertRaises(
exceptions.ConflictException,
self.conn.baremetal.update_node, node, is_retired=True)
class TestBareMetalNodeFields(base.BaseBaremetalTest):

13
openstack/tests/unit/baremetal/v1/test_node.py
View File

@ -347,6 +347,7 @@ class TestNodeCreate(base.TestCase):
self.session.post.side_effect = _change_state
def test_available_old_version(self, mock_prov):
self.node.provision_state = 'available'
result = self.node.create(self.session)
self.assertIs(result, self.node)
self.session.post.assert_called_once_with(
@ -356,24 +357,16 @@ class TestNodeCreate(base.TestCase):
self.assertFalse(mock_prov.called)
def test_available_new_version(self, mock_prov):
def _change_state(*args, **kwargs):
self.node.provision_state = 'manageable'
self.session.default_microversion = '1.11'
self.node.provision_state = 'available'
self.new_state = 'enroll'
mock_prov.side_effect = _change_state
result = self.node.create(self.session)
self.assertIs(result, self.node)
self.session.post.assert_called_once_with(
mock.ANY, json={'driver': FAKE['driver']},
headers=mock.ANY, microversion=self.session.default_microversion,
headers=mock.ANY, microversion='1.10',
params={})
mock_prov.assert_has_calls([
mock.call(self.node, self.session, 'manage', wait=True),
mock.call(self.node, self.session, 'provide', wait=True)
])
mock_prov.assert_not_called()
def test_no_enroll_in_old_version(self, mock_prov):
self.node.provision_state = 'enroll'

9
releasenotes/notes/node-create-027ea99193f344ef.yaml Normal file
View File

@ -0,0 +1,9 @@
---
upgrade:
- |
Changes the baremetal ``create_node`` call to be closer to how Ironic
behaves. If no provision state is requested, the default state of the
current microversion is used (which usually means ``enroll``).
If the ``available`` state is requested, the node does not go through
cleaning (it won't work without creating ports), an old API version is
used to achieve this provision state.