openstack/openstacksdk
Code Issues Proposed changes

Merge "Make PATCH a first class operation and support it for baremetal"

Browse Source
This commit is contained in:
Zuul 2019-03-28 17:54:14 +00:00 committed by Gerrit Code Review
commit d2d3de5881
15 changed files with 304 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/source/user/proxies/baremetal.rst
View File

@ -18,6 +18,7 @@ Node Operations
.. automethod:: openstack.baremetal.v1._proxy.Proxy.create_node
.. automethod:: openstack.baremetal.v1._proxy.Proxy.update_node
.. automethod:: openstack.baremetal.v1._proxy.Proxy.patch_node
.. automethod:: openstack.baremetal.v1._proxy.Proxy.delete_node
.. automethod:: openstack.baremetal.v1._proxy.Proxy.get_node
.. automethod:: openstack.baremetal.v1._proxy.Proxy.find_node
@ -36,6 +37,7 @@ Port Operations
.. automethod:: openstack.baremetal.v1._proxy.Proxy.create_port
.. automethod:: openstack.baremetal.v1._proxy.Proxy.update_port
.. automethod:: openstack.baremetal.v1._proxy.Proxy.patch_port
.. automethod:: openstack.baremetal.v1._proxy.Proxy.delete_port
.. automethod:: openstack.baremetal.v1._proxy.Proxy.get_port
.. automethod:: openstack.baremetal.v1._proxy.Proxy.find_port
@ -47,6 +49,7 @@ Port Group Operations
.. automethod:: openstack.baremetal.v1._proxy.Proxy.create_port_group
.. automethod:: openstack.baremetal.v1._proxy.Proxy.update_port_group
.. automethod:: openstack.baremetal.v1._proxy.Proxy.patch_port_group
.. automethod:: openstack.baremetal.v1._proxy.Proxy.delete_port_group
.. automethod:: openstack.baremetal.v1._proxy.Proxy.get_port_group
.. automethod:: openstack.baremetal.v1._proxy.Proxy.find_port_group
@ -65,6 +68,7 @@ Chassis Operations
.. automethod:: openstack.baremetal.v1._proxy.Proxy.create_chassis
.. automethod:: openstack.baremetal.v1._proxy.Proxy.update_chassis
.. automethod:: openstack.baremetal.v1._proxy.Proxy.patch_chassis
.. automethod:: openstack.baremetal.v1._proxy.Proxy.delete_chassis
.. automethod:: openstack.baremetal.v1._proxy.Proxy.get_chassis
.. automethod:: openstack.baremetal.v1._proxy.Proxy.find_chassis

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

@ -114,6 +114,18 @@ class Proxy(proxy.Proxy):
"""
return self._update(_chassis.Chassis, chassis, **attrs)
def patch_chassis(self, chassis, patch):
"""Apply a JSON patch to the chassis.
:param chassis: The value can be the ID of a chassis or a
:class:`~openstack.baremetal.v1.chassis.Chassis` instance.
:param patch: JSON patch to apply.
:returns: The updated chassis.
:rtype: :class:`~openstack.baremetal.v1.chassis.Chassis`
"""
return self._get_resource(_chassis.Chassis, chassis).patch(self, patch)
def delete_chassis(self, chassis, ignore_missing=True):
"""Delete a chassis.
@ -246,8 +258,8 @@ class Proxy(proxy.Proxy):
def update_node(self, node, retry_on_conflict=True, **attrs):
"""Update a node.
:param chassis: Either the name or the ID of a node or an instance
of :class:`~openstack.baremetal.v1.node.Node`.
:param node: The value can be the name or ID of a node or a
:class:`~openstack.baremetal.v1.node.Node` instance.
:param bool retry_on_conflict: Whether to retry HTTP CONFLICT error.
Most of the time it can be retried, since it is caused by the node
being locked. However, when setting ``instance_id``, this is
@ -261,6 +273,23 @@ class Proxy(proxy.Proxy):
res = self._get_resource(_node.Node, node, **attrs)
return res.commit(self, retry_on_conflict=retry_on_conflict)
def patch_node(self, node, patch, retry_on_conflict=True):
"""Apply a JSON patch to the node.
:param node: The value can be the name or ID of a node or a
:class:`~openstack.baremetal.v1.node.Node` instance.
:param patch: JSON patch to apply.
:param bool retry_on_conflict: Whether to retry HTTP CONFLICT error.
Most of the time it can be retried, since it is caused by the node
being locked. However, when setting ``instance_id``, this is
a normal code and should not be retried.
:returns: The updated node.
:rtype: :class:`~openstack.baremetal.v1.node.Node`
"""
res = self._get_resource(_node.Node, node)
return res.patch(self, patch, retry_on_conflict=retry_on_conflict)
def set_node_provision_state(self, node, target, config_drive=None,
clean_steps=None, rescue_password=None,
wait=False, timeout=None):
@ -527,7 +556,7 @@ class Proxy(proxy.Proxy):
def update_port(self, port, **attrs):
"""Update a port.
:param chassis: Either the ID of a port or an instance
:param port: Either the ID of a port or an instance
of :class:`~openstack.baremetal.v1.port.Port`.
:param dict attrs: The attributes to update on the port represented
by the ``port`` parameter.
@ -537,6 +566,18 @@ class Proxy(proxy.Proxy):
"""
return self._update(_port.Port, port, **attrs)
def patch_port(self, port, patch):
"""Apply a JSON patch to the port.
:param port: The value can be the ID of a port or a
:class:`~openstack.baremetal.v1.port.Port` instance.
:param patch: JSON patch to apply.
:returns: The updated port.
:rtype: :class:`~openstack.baremetal.v1.port.Port`
"""
return self._get_resource(_port.Port, port).patch(self, patch)
def delete_port(self, port, ignore_missing=True):
"""Delete a port.
@ -638,7 +679,7 @@ class Proxy(proxy.Proxy):
def update_port_group(self, port_group, **attrs):
"""Update a port group.
:param chassis: Either the name or the ID of a port group or
:param port_group: Either the name or the ID of a port group or
an instance of
:class:`~openstack.baremetal.v1.port_group.PortGroup`.
:param dict attrs: The attributes to update on the port group
@ -649,6 +690,19 @@ class Proxy(proxy.Proxy):
"""
return self._update(_portgroup.PortGroup, port_group, **attrs)
def patch_port_group(self, port_group, patch):
"""Apply a JSON patch to the port_group.
:param port_group: The value can be the ID of a port group or a
:class:`~openstack.baremetal.v1.port_group.PortGroup` instance.
:param patch: JSON patch to apply.
:returns: The updated port group.
:rtype: :class:`~openstack.baremetal.v1.port_group.PortGroup`
"""
res = self._get_resource(_portgroup.PortGroup, port_group)
return res.patch(self, patch)
def delete_port_group(self, port_group, ignore_missing=True):
"""Delete a port group.

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

@ -28,6 +28,7 @@ class Chassis(_common.ListMixin, resource.Resource):
allow_commit = True
allow_delete = True
allow_list = True
allow_patch = True
commit_method = 'PATCH'
commit_jsonpatch = True

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

@ -45,6 +45,7 @@ class Node(_common.ListMixin, resource.Resource):
allow_commit = True
allow_delete = True
allow_list = True
allow_patch = True
commit_method = 'PATCH'
commit_jsonpatch = True

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

@ -25,6 +25,7 @@ class Port(_common.ListMixin, resource.Resource):
allow_commit = True
allow_delete = True
allow_list = True
allow_patch = True
commit_method = 'PATCH'
commit_jsonpatch = True

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

@ -25,6 +25,7 @@ class PortGroup(_common.ListMixin, resource.Resource):
allow_commit = True
allow_delete = True
allow_list = True
allow_patch = True
commit_method = 'PATCH'
commit_jsonpatch = True

34
openstack/cloud/_baremetal.py
View File

@ -434,17 +434,8 @@ class BaremetalCloudMixin(_normalize.Normalizer):
:returns: ``munch.Munch`` representing the newly updated node.
"""
node = self.baremetal.get_node(name_or_id)
microversion = node._get_microversion_for(self._baremetal_client,
'commit')
msg = ("Error updating machine via patch operation on node "
"{node}".format(node=name_or_id))
url = '/nodes/{node_id}'.format(node_id=node.id)
return self._normalize_machine(
self._baremetal_client.patch(url,
json=patch,
microversion=microversion,
error_message=msg))
self.baremetal.patch_node(name_or_id, patch))
def update_machine(self, name_or_id, **attrs):
"""Update a machine with new configuration information
@ -687,22 +678,17 @@ class BaremetalCloudMixin(_normalize.Normalizer):
uuid, 'deleted', wait=wait, timeout=timeout)
def set_node_instance_info(self, uuid, patch):
msg = ("Error updating machine via patch operation on node "
"{node}".format(node=uuid))
url = '/nodes/{node_id}'.format(node_id=uuid)
return self._baremetal_client.patch(url,
json=patch,
error_message=msg)
warnings.warn("The set_node_instance_info call is deprecated, "
"use patch_machine or update_machine instead",
DeprecationWarning)
return self.patch_machine(uuid, patch)
def purge_node_instance_info(self, uuid):
patch = []
patch.append({'op': 'remove', 'path': '/instance_info'})
msg = ("Error updating machine via patch operation on node "
"{node}".format(node=uuid))
url = '/nodes/{node_id}'.format(node_id=uuid)
return self._baremetal_client.patch(url,
json=patch,
error_message=msg)
warnings.warn("The purge_node_instance_info call is deprecated, "
"use patch_machine or update_machine instead",
DeprecationWarning)
return self.patch_machine(uuid,
dict(path='/instance_info', op='remove'))
def wait_for_baremetal_node_lock(self, node, timeout=30):
"""Wait for a baremetal node to have no lock.

121
openstack/resource.py
View File

@ -387,6 +387,8 @@ class Resource(dict):
allow_list = False
#: Allow head operation for this resource.
allow_head = False
#: Allow patch operation for this resource.
allow_patch = False
# TODO(mordred) Unused - here for transition with OSC. Remove once
# OSC no longer checks for allow_get
@ -455,7 +457,7 @@ class Resource(dict):
self._computed = _ComponentManager(
attributes=computed,
synchronized=_synchronized)
if self.commit_jsonpatch:
if self.commit_jsonpatch or self.allow_patch:
# We need the original body to compare against
if _synchronized:
self._original_body = self._body.attributes.copy()
@ -698,7 +700,7 @@ class Resource(dict):
def _clean_body_attrs(self, attrs):
"""Mark the attributes as up-to-date."""
self._body.clean(only=attrs)
if self.commit_jsonpatch:
if self.commit_jsonpatch or self.allow_patch:
for attr in attrs:
if attr in self._body:
self._original_body[attr] = self._body[attr]
@ -976,7 +978,7 @@ class Resource(dict):
body = self._consume_body_attrs(body)
self._body.attributes.update(body)
self._body.clean()
if self.commit_jsonpatch:
if self.commit_jsonpatch or self.allow_patch:
# We need the original body to compare against
self._original_body = body.copy()
@ -1037,11 +1039,11 @@ class Resource(dict):
Subclasses can override this method if more complex logic is needed.
:param session: :class`keystoneauth1.adapter.Adapter`
:param action: One of "fetch", "commit", "create", "delete". Unused in
the base implementation.
:param action: One of "fetch", "commit", "create", "delete", "patch".
Unused in the base implementation.
:return: microversion as string or ``None``
"""
if action not in ('fetch', 'commit', 'create', 'delete'):
if action not in ('fetch', 'commit', 'create', 'delete', 'patch'):
raise ValueError('Invalid action: %s' % action)
return self._get_microversion_for_list(session)
@ -1234,6 +1236,14 @@ class Resource(dict):
request = self._prepare_request(prepend_key=prepend_key,
base_path=base_path,
**kwargs)
microversion = self._get_microversion_for(session, 'commit')
return self._commit(session, request, self.commit_method, microversion,
has_body=has_body,
retry_on_conflict=retry_on_conflict)
def _commit(self, session, request, method, microversion, has_body=True,
retry_on_conflict=None):
session = self._get_session(session)
kwargs = {}
@ -1245,28 +1255,95 @@ class Resource(dict):
# overriding it via an explicit retry_on_conflict=False.
kwargs['retriable_status_codes'] = retriable_status_codes - {409}
microversion = self._get_microversion_for(session, 'commit')
if self.commit_method == 'PATCH':
response = session.patch(
request.url, json=request.body, headers=request.headers,
microversion=microversion, **kwargs)
elif self.commit_method == 'POST':
response = session.post(
request.url, json=request.body, headers=request.headers,
microversion=microversion, **kwargs)
elif self.commit_method == 'PUT':
response = session.put(
request.url, json=request.body, headers=request.headers,
microversion=microversion, **kwargs)
else:
try:
call = getattr(session, method.lower())
except AttributeError:
raise exceptions.ResourceFailure(
msg="Invalid commit method: %s" % self.commit_method)
msg="Invalid commit method: %s" % method)
response = call(request.url, json=request.body,
headers=request.headers, microversion=microversion,
**kwargs)
self.microversion = microversion
self._translate_response(response, has_body=has_body)
return self
def _convert_patch(self, patch):
if not isinstance(patch, list):
patch = [patch]
converted = []
for item in patch:
try:
path = item['path']
parts = path.lstrip('/').split('/', 1)
field = parts[0]
except (KeyError, IndexError):
raise ValueError("Malformed or missing path in %s" % item)
try:
component = getattr(self.__class__, field)
except AttributeError:
server_field = field
else:
server_field = component.name
if len(parts) > 1:
new_path = '/%s/%s' % (server_field, parts[1])
else:
new_path = '/%s' % server_field
converted.append(dict(item, path=new_path))
return converted
def patch(self, session, patch=None, prepend_key=True, has_body=True,
retry_on_conflict=None, base_path=None):
"""Patch the remote resource.
Allows modifying the resource by providing a list of JSON patches to
apply to it. The patches can use both the original (server-side) and
SDK field names.
:param session: The session to use for making this request.
:type session: :class:`~keystoneauth1.adapter.Adapter`
:param patch: Additional JSON patch as a list or one patch item.
If provided, it is applied on top of any changes to the
current resource.
:param prepend_key: A boolean indicating whether the resource_key
should be prepended in a resource update request.
Default to True.
:param bool retry_on_conflict: Whether to enable retries on HTTP
CONFLICT (409). Value of ``None`` leaves
the `Adapter` defaults.
:param str base_path: Base part of the URI for modifying resources, if
different from
:data:`~openstack.resource.Resource.base_path`.
:return: This :class:`Resource` instance.
:raises: :exc:`~openstack.exceptions.MethodNotSupported` if
:data:`Resource.allow_patch` is not set to ``True``.
"""
# The id cannot be dirty for an commit
self._body._dirty.discard("id")
# Only try to update if we actually have anything to commit.
if not patch and not self.requires_commit:
return self
if not self.allow_patch:
raise exceptions.MethodNotSupported(self, "patch")
request = self._prepare_request(prepend_key=prepend_key,
base_path=base_path, patch=True)
microversion = self._get_microversion_for(session, 'patch')
if patch:
request.body += self._convert_patch(patch)
return self._commit(session, request, 'PATCH', microversion,
has_body=has_body,
retry_on_conflict=retry_on_conflict)
def delete(self, session, error_message=None):
"""Delete the remote resource based on this instance.

10
openstack/tests/functional/baremetal/test_baremetal_chassis.py
View File

@ -37,6 +37,16 @@ class TestBareMetalChassis(base.BaseBaremetalTest):
chassis = self.conn.baremetal.get_chassis(chassis.id)
self.assertEqual({'answer': 42}, chassis.extra)
def test_chassis_patch(self):
chassis = self.create_chassis()
chassis = self.conn.baremetal.patch_chassis(
chassis, dict(path='/extra/answer', op='add', value=42))
self.assertEqual({'answer': 42}, chassis.extra)
chassis = self.conn.baremetal.get_chassis(chassis.id)
self.assertEqual({'answer': 42}, chassis.extra)
def test_chassis_negative_non_existing(self):
uuid = "5c9dcd04-2073-49bc-9618-99ae634d8971"
self.assertRaises(exceptions.ResourceNotFound,

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

@ -86,6 +86,35 @@ class TestBareMetalNode(base.BaseBaremetalTest):
node = self.conn.baremetal.get_node('node-name')
self.assertIsNone(node.instance_id)
def test_node_patch(self):
node = self.create_node(name='node-name', extra={'foo': 'bar'})
node.name = 'new-name'
instance_uuid = str(uuid.uuid4())
node = self.conn.baremetal.patch_node(
node,
[dict(path='/instance_id', op='replace', value=instance_uuid),
dict(path='/extra/answer', op='add', value=42)])
self.assertEqual('new-name', node.name)
self.assertEqual({'foo': 'bar', 'answer': 42}, node.extra)
self.assertEqual(instance_uuid, node.instance_id)
node = self.conn.baremetal.get_node('new-name')
self.assertEqual('new-name', node.name)
self.assertEqual({'foo': 'bar', 'answer': 42}, node.extra)
self.assertEqual(instance_uuid, node.instance_id)
node = self.conn.baremetal.patch_node(
node,
[dict(path='/instance_id', op='remove'),
dict(path='/extra/answer', op='remove')])
self.assertIsNone(node.instance_id)
self.assertNotIn('answer', node.extra)
node = self.conn.baremetal.get_node('new-name')
self.assertIsNone(node.instance_id)
self.assertNotIn('answer', node.extra)
def test_node_list_update_delete(self):
self.create_node(name='node-name', extra={'foo': 'bar'})
node = next(n for n in

13
openstack/tests/functional/baremetal/test_baremetal_port.py
View File

@ -77,6 +77,19 @@ class TestBareMetalPort(base.BaseBaremetalTest):
self.assertEqual('66:55:44:33:22:11', port.address)
self.assertEqual({'answer': 42}, port.extra)
def test_port_patch(self):
port = self.create_port(address='11:22:33:44:55:66')
port.address = '66:55:44:33:22:11'
port = self.conn.baremetal.patch_port(
port, dict(path='/extra/answer', op='add', value=42))
self.assertEqual('66:55:44:33:22:11', port.address)
self.assertEqual({'answer': 42}, port.extra)
port = self.conn.baremetal.get_port(port.id)
self.assertEqual('66:55:44:33:22:11', port.address)
self.assertEqual({'answer': 42}, port.extra)
def test_port_negative_non_existing(self):
uuid = "5c9dcd04-2073-49bc-9618-99ae634d8971"
self.assertRaises(exceptions.ResourceNotFound,

10
openstack/tests/functional/baremetal/test_baremetal_port_group.py
View File

@ -72,6 +72,16 @@ class TestBareMetalPortGroup(base.BaseBaremetalTest):
port_group = self.conn.baremetal.get_port_group(port_group.id)
self.assertEqual({'answer': 42}, port_group.extra)
def test_port_group_patch(self):
port_group = self.create_port_group()
port_group = self.conn.baremetal.patch_port_group(
port_group, dict(path='/extra/answer', op='add', value=42))
self.assertEqual({'answer': 42}, port_group.extra)
port_group = self.conn.baremetal.get_port_group(port_group.id)
self.assertEqual({'answer': 42}, port_group.extra)
def test_port_group_negative_non_existing(self):
uuid = "5c9dcd04-2073-49bc-9618-99ae634d8971"
self.assertRaises(exceptions.ResourceNotFound,

5
openstack/tests/unit/cloud/test_baremetal_node.py
View File

@ -206,11 +206,6 @@ class TestBaremetalNode(base.IronicTestCase):
'path': '/instance_info'}]
self.fake_baremetal_node['instance_info'] = {}
self.register_uris([
dict(method='GET',
uri=self.get_mock_url(
resource='nodes',
append=[self.fake_baremetal_node['uuid']]),
json=self.fake_baremetal_node),
dict(method='PATCH',
uri=self.get_mock_url(
resource='nodes',

54
openstack/tests/unit/test_resource.py
View File

@ -1336,6 +1336,60 @@ class TestResourceActions(base.TestCase):
self.session.put.assert_not_called()
def test_patch_with_sdk_names(self):
class Test(resource.Resource):
allow_patch = True
id = resource.Body('id')
attr = resource.Body('attr')
nested = resource.Body('renamed')
other = resource.Body('other')
test_patch = [{'path': '/attr', 'op': 'replace', 'value': 'new'},
{'path': '/nested/dog', 'op': 'remove'},
{'path': '/nested/cat', 'op': 'add', 'value': 'meow'}]
expected = [{'path': '/attr', 'op': 'replace', 'value': 'new'},
{'path': '/renamed/dog', 'op': 'remove'},
{'path': '/renamed/cat', 'op': 'add', 'value': 'meow'}]
sot = Test.existing(id=1, attr=42, nested={'dog': 'bark'})
sot.patch(self.session, test_patch)
self.session.patch.assert_called_once_with(
'/1', json=expected, headers=mock.ANY, microversion=None)
def test_patch_with_server_names(self):
class Test(resource.Resource):
allow_patch = True
id = resource.Body('id')
attr = resource.Body('attr')
nested = resource.Body('renamed')
other = resource.Body('other')
test_patch = [{'path': '/attr', 'op': 'replace', 'value': 'new'},
{'path': '/renamed/dog', 'op': 'remove'},
{'path': '/renamed/cat', 'op': 'add', 'value': 'meow'}]
sot = Test.existing(id=1, attr=42, nested={'dog': 'bark'})
sot.patch(self.session, test_patch)
self.session.patch.assert_called_once_with(
'/1', json=test_patch, headers=mock.ANY, microversion=None)
def test_patch_with_changed_fields(self):
class Test(resource.Resource):
allow_patch = True
attr = resource.Body('attr')
nested = resource.Body('renamed')
other = resource.Body('other')
sot = Test.existing(id=1, attr=42, nested={'dog': 'bark'})
sot.attr = 'new'
sot.patch(self.session, {'path': '/renamed/dog', 'op': 'remove'})
expected = [{'path': '/attr', 'op': 'replace', 'value': 'new'},
{'path': '/renamed/dog', 'op': 'remove'}]
self.session.patch.assert_called_once_with(
'/1', json=expected, headers=mock.ANY, microversion=None)
def test_delete(self):
result = self.sot.delete(self.session)

13
releasenotes/notes/baremetal-patch-feebd96b1b92f3b9.yaml Normal file
View File

@ -0,0 +1,13 @@
---
features:
- |
Adds support for changing bare metal resources by providing a JSON patch.
Adds the following calls to the bare metal proxy: ``patch_node``,
``patch_port``, ``patch_port_group`` and ``patch_chassis``.
deprecations:
- |
The ``set_node_instance_info`` call is deprecated, use ``patch_machine``
with the same arguments instead.
- |
The ``purge_node_instance_info`` call is deprecated, use ``patch_machine``
or ``update_machine`` instead.