From 100fd90401bd05893f9b4d6552dd3793971f37b8 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 18 Jul 2018 10:10:11 +0200 Subject: [PATCH] Support for microversions in baremetal resources The baremetal resources are updated with a _max_microversion reflecting the currently supported set of fields. Doing that would make create() call create nodes in the "enroll" state, which would be a breaking change. Thus, the create() call is updated to handle provision_state being set to enroll, manageable or available, with available being the default. Change-Id: If46e339070514bcd34bc3ba336f0cfb5556a5cea --- openstack/baremetal/v1/_common.py | 6 ++ openstack/baremetal/v1/node.py | 67 +++++++++++++ openstack/baremetal/v1/port.py | 5 +- openstack/baremetal/v1/port_group.py | 3 + openstack/exceptions.py | 4 + openstack/resource.py | 50 +++++++++- .../tests/unit/baremetal/v1/test_node.py | 97 +++++++++++++++++++ openstack/tests/unit/test_resource.py | 42 ++++++++ 8 files changed, 270 insertions(+), 4 deletions(-) diff --git a/openstack/baremetal/v1/_common.py b/openstack/baremetal/v1/_common.py index 16b5506ae..ffa1ee668 100644 --- a/openstack/baremetal/v1/_common.py +++ b/openstack/baremetal/v1/_common.py @@ -46,3 +46,9 @@ EXPECTED_STATES = { 'rescue': 'rescue', } """Mapping of provisioning actions to expected stable states.""" + +STATE_VERSIONS = { + 'enroll': '1.11', + 'manageable': '1.4', +} +"""API versions when certain states were introduced.""" diff --git a/openstack/baremetal/v1/node.py b/openstack/baremetal/v1/node.py index a57574411..428d8ab39 100644 --- a/openstack/baremetal/v1/node.py +++ b/openstack/baremetal/v1/node.py @@ -41,6 +41,9 @@ class Node(resource.Resource): is_maintenance='maintenance', ) + # Full port groups support introduced in 1.24 + _max_microversion = '1.24' + # Properties #: The UUID of the chassis associated wit this node. Can be empty or None. chassis_id = resource.Body("chassis_uuid") @@ -120,6 +123,70 @@ class Node(resource.Resource): #: Timestamp at which the node was last updated. updated_at = resource.Body("updated_at") + def create(self, session, *args, **kwargs): + """Create a remote resource based on this instance. + + 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. + + Note that Bare Metal API 1.4 is required for ``manageable`` and + 1.11 is required for ``enroll``. + + :param session: The session to use for making this request. + :type session: :class:`~keystoneauth1.adapter.Adapter` + + :return: This :class:`Resource` instance. + :raises: ValueError if the Node's ``provision_state`` is not one of + ``None``, ``enroll``, ``manageable`` or ``available``. + :raises: :exc:`~openstack.exceptions.NotSupported` if + the ``provision_state`` cannot be reached with any API version + 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 + else: + self._assert_microversion_for( + session, 'create', expected_version, + error_message="Cannot create a node with initial provision " + "state %s" % expected_provision_state) + + # Ironic cannot set provision_state itself, so marking it as unchanged + self._body.clean(only={'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) + + 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'): + self.set_provision_state(session, 'manage', wait=True) + + return self + def set_provision_state(self, session, target, config_drive=None, clean_steps=None, rescue_password=None, wait=False, timeout=None): diff --git a/openstack/baremetal/v1/port.py b/openstack/baremetal/v1/port.py index cca5d14aa..99879de35 100644 --- a/openstack/baremetal/v1/port.py +++ b/openstack/baremetal/v1/port.py @@ -32,6 +32,9 @@ class Port(resource.Resource): 'fields' ) + # Port group ID introduced in 1.24 + _max_microversion = '1.24' + #: The physical hardware address of the network port, typically the #: hardware MAC address. address = resource.Body('address') @@ -56,7 +59,7 @@ class Port(resource.Resource): #: The UUID of node this port belongs to node_id = resource.Body('node_uuid') #: The UUID of PortGroup this port belongs to. Added in API microversion - #: 1.23. + #: 1.24. port_group_id = resource.Body('portgroup_uuid') #: Timestamp at which the port was last updated. updated_at = resource.Body('updated_at') diff --git a/openstack/baremetal/v1/port_group.py b/openstack/baremetal/v1/port_group.py index d1d44e7d4..d8fda6429 100644 --- a/openstack/baremetal/v1/port_group.py +++ b/openstack/baremetal/v1/port_group.py @@ -32,6 +32,9 @@ class PortGroup(resource.Resource): 'node', 'address', 'fields', ) + # Port groups introduced in 1.23 + _max_microversion = '1.23' + #: The physical hardware address of the portgroup, typically the hardware #: MAC address. Added in API microversion 1.23. address = resource.Body('address') diff --git a/openstack/exceptions.py b/openstack/exceptions.py index 619c0b491..8d5ca7dcf 100644 --- a/openstack/exceptions.py +++ b/openstack/exceptions.py @@ -220,3 +220,7 @@ class ArgumentDeprecationWarning(Warning): class ConfigException(SDKException): """Something went wrong with parsing your OpenStack Config.""" + + +class NotSupported(SDKException): + """Request cannot be performed by any supported API version.""" diff --git a/openstack/resource.py b/openstack/resource.py index cc8371049..efa04e4a6 100644 --- a/openstack/resource.py +++ b/openstack/resource.py @@ -35,6 +35,7 @@ import collections import itertools from keystoneauth1 import adapter +from keystoneauth1 import discover import munch from requests import structures @@ -200,9 +201,16 @@ class _ComponentManager(collections.MutableMapping): return dict((key, self.attributes.get(key, None)) for key in self._dirty) - def clean(self): - """Signal that the resource no longer has modified attributes""" - self._dirty = set() + def clean(self, only=None): + """Signal that the resource no longer has modified attributes. + + :param only: an optional set of attributes to no longer consider + changed + """ + if only: + self._dirty = self._dirty - set(only) + else: + self._dirty = set() class _Request(object): @@ -774,6 +782,42 @@ class Resource(object): return self._get_microversion_for_list(session) + def _assert_microversion_for(self, session, action, expected, + error_message=None): + """Enforce that the microversion for action satisfies the requirement. + + :param session: :class`keystoneauth1.adapter.Adapter` + :param action: One of "get", "update", "create", "delete". + :param expected: Expected microversion. + :param error_message: Optional error message with details. Will be + prepended to the message generated here. + :return: resulting microversion as string. + :raises: :exc:`~openstack.exceptions.NotSupported` if the version + used for the action is lower than the expected one. + """ + def _raise(message): + if error_message: + error_message.rstrip('.') + message = '%s. %s' % (error_message, message) + + raise exceptions.NotSupported(message) + + actual = self._get_microversion_for(session, action) + 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) + + return actual + def create(self, session, prepend_key=True): """Create a remote resource based on this instance. diff --git a/openstack/tests/unit/baremetal/v1/test_node.py b/openstack/tests/unit/baremetal/v1/test_node.py index 5866ea256..3ae7da564 100644 --- a/openstack/tests/unit/baremetal/v1/test_node.py +++ b/openstack/tests/unit/baremetal/v1/test_node.py @@ -274,3 +274,100 @@ class TestNodeSetProvisionState(base.TestCase): def test_no_arguments(self): self.node.set_provision_state(self.session, 'manage') + + +@mock.patch.object(node.Node, '_translate_response', mock.Mock()) +@mock.patch.object(node.Node, '_get_session', lambda self, x: x) +@mock.patch.object(node.Node, 'set_provision_state', autospec=True) +class TestNodeCreate(base.TestCase): + + def setUp(self): + super(TestNodeCreate, self).setUp() + self.new_state = None + self.session = mock.Mock(spec=adapter.Adapter) + self.session.default_microversion = '1.1' + self.node = node.Node(driver=FAKE['driver']) + + def _change_state(*args, **kwargs): + self.node.provision_state = self.new_state + + self.session.post.side_effect = _change_state + + def test_available_old_version(self, mock_prov): + 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) + 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) + mock_prov.assert_has_calls([ + mock.call(self.node, self.session, 'manage', wait=True), + mock.call(self.node, self.session, 'provide', wait=True) + ]) + + def test_no_enroll_in_old_version(self, mock_prov): + self.node.provision_state = 'enroll' + self.assertRaises(exceptions.NotSupported, + self.node.create, self.session) + self.assertFalse(self.session.post.called) + self.assertFalse(mock_prov.called) + + def test_enroll_new_version(self, mock_prov): + self.session.default_microversion = '1.11' + self.node.provision_state = 'enroll' + self.new_state = 'enroll' + + 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) + self.assertFalse(mock_prov.called) + + def test_no_manageable_in_old_version(self, mock_prov): + self.node.provision_state = 'manageable' + self.assertRaises(exceptions.NotSupported, + self.node.create, self.session) + self.assertFalse(self.session.post.called) + self.assertFalse(mock_prov.called) + + def test_manageable_old_version(self, mock_prov): + self.session.default_microversion = '1.4' + self.node.provision_state = 'manageable' + self.new_state = 'available' + + 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) + mock_prov.assert_called_once_with(self.node, self.session, 'manage', + wait=True) + + def test_manageable_new_version(self, mock_prov): + self.session.default_microversion = '1.11' + self.node.provision_state = 'manageable' + self.new_state = 'enroll' + + 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) + mock_prov.assert_called_once_with(self.node, self.session, 'manage', + wait=True) diff --git a/openstack/tests/unit/test_resource.py b/openstack/tests/unit/test_resource.py index be5e18c26..df5f70eb5 100644 --- a/openstack/tests/unit/test_resource.py +++ b/openstack/tests/unit/test_resource.py @@ -2106,3 +2106,45 @@ class TestWaitForDelete(base.TestCase): exceptions.ResourceTimeout, resource.wait_for_delete, "session", res, 0.1, 0.3) + + +@mock.patch.object(resource.Resource, '_get_microversion_for', autospec=True) +class TestAssertMicroversionFor(base.TestCase): + session = mock.Mock() + res = resource.Resource() + + def test_compatible(self, mock_get_ver): + mock_get_ver.return_value = '1.42' + + self.assertEqual( + '1.42', + self.res._assert_microversion_for(self.session, 'get', '1.6')) + mock_get_ver.assert_called_once_with(self.res, self.session, 'get') + + def test_incompatible(self, mock_get_ver): + mock_get_ver.return_value = '1.1' + + self.assertRaisesRegex(exceptions.NotSupported, + '1.6 is required, but 1.1 will be used', + self.res._assert_microversion_for, + self.session, 'get', '1.6') + mock_get_ver.assert_called_once_with(self.res, self.session, 'get') + + def test_custom_message(self, mock_get_ver): + mock_get_ver.return_value = '1.1' + + self.assertRaisesRegex(exceptions.NotSupported, + 'boom.*1.6 is required, but 1.1 will be used', + self.res._assert_microversion_for, + self.session, 'get', '1.6', + error_message='boom') + mock_get_ver.assert_called_once_with(self.res, self.session, 'get') + + def test_none(self, mock_get_ver): + mock_get_ver.return_value = None + + self.assertRaisesRegex(exceptions.NotSupported, + '1.6 is required, but the default version', + self.res._assert_microversion_for, + self.session, 'get', '1.6') + mock_get_ver.assert_called_once_with(self.res, self.session, 'get')