openstack/ironic
Code Issues Proposed changes

Merge "Add API to allow update allocation name and extra field"

Browse Source
This commit is contained in:
Zuul 2019-05-20 09:26:37 +00:00 committed by Gerrit Code Review
commit 566801418c
8 changed files with 261 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
doc/source/contributor/webapi-version-history.rst
View File

@ -2,6 +2,14 @@
REST API Version History
========================
1.57 (master)
-------------
Added the following new endpoint for allocation:
* ``PATCH /v1/allocations/<allocation_ident>`` that allows updating ``name``
and ``extra`` fields for an existing allocation.
1.56 (Stein, 12.1.0)
--------------------

60
ironic/api/controllers/v1/allocation.py
View File

@ -196,6 +196,11 @@ class AllocationCollection(collection.Collection):
return sample
class AllocationPatchType(types.JsonPatchType):
_api_base = Allocation
class AllocationsController(pecan.rest.RestController):
"""REST controller for allocations."""
@ -377,6 +382,61 @@ class AllocationsController(pecan.rest.RestController):
new_allocation.uuid)
return Allocation.convert_with_links(new_allocation)
def _validate_patch(self, patch):
allowed_fields = ['name', 'extra']
for p in patch:
path = p['path'].split('/')[1]
if path not in allowed_fields:
msg = _("Cannot update %s in an allocation. Only 'name' and "
"'extra' are allowed to be updated.")
raise exception.Invalid(msg % p['path'])
@METRICS.timer('AllocationsController.patch')
@wsme.validate(types.uuid, [AllocationPatchType])
@expose.expose(Allocation, types.uuid_or_name, body=[AllocationPatchType])
def patch(self, allocation_ident, patch):
"""Update an existing allocation.
:param allocation_ident: UUID or logical name of an allocation.
:param patch: a json PATCH document to apply to this allocation.
"""
if not api_utils.allow_allocation_update():
raise webob_exc.HTTPMethodNotAllowed(_(
"The API version does not allow updating allocations"))
context = pecan.request.context
cdict = context.to_policy_values()
policy.authorize('baremetal:allocation:update', cdict, cdict)
self._validate_patch(patch)
names = api_utils.get_patch_values(patch, '/name')
for name in names:
if len(name) and not api_utils.is_valid_logical_name(name):
msg = _("Cannot update allocation with invalid name "
"'%(name)s'") % {'name': name}
raise exception.Invalid(msg)
rpc_allocation = api_utils.get_rpc_allocation_with_suffix(
allocation_ident)
allocation_dict = rpc_allocation.as_dict()
allocation = Allocation(**api_utils.apply_jsonpatch(allocation_dict,
patch))
# Update only the fields that have changed
for field in objects.Allocation.fields:
try:
patch_val = getattr(allocation, field)
except AttributeError:
# Ignore fields that aren't exposed in the API
continue
if patch_val == wtypes.Unset:
patch_val = None
if rpc_allocation[field] != patch_val:
rpc_allocation[field] = patch_val
notify.emit_start_notification(context, rpc_allocation, 'update')
with notify.handle_error_notification(context,
rpc_allocation, 'update'):
rpc_allocation.save()
notify.emit_end_notification(context, rpc_allocation, 'update')
return Allocation.convert_with_links(rpc_allocation)
@METRICS.timer('AllocationsController.delete')
@expose.expose(None, types.uuid_or_name,
status_code=http_client.NO_CONTENT)

8
ironic/api/controllers/v1/utils.py
View File

@ -1161,3 +1161,11 @@ def allow_build_configdrive():
Version 1.56 of the API added support for building configdrive.
"""
return pecan.request.version.minor >= versions.MINOR_56_BUILD_CONFIGDRIVE
def allow_allocation_update():
"""Check if updating an existing allocation is allowed or not.
Version 1.57 of the API added support for updating an allocation.
"""
return pecan.request.version.minor >= versions.MINOR_57_ALLOCATION_UPDATE

4
ironic/api/controllers/v1/versions.py
View File

@ -94,6 +94,7 @@ BASE_VERSION = 1
# v1.54: Add events support.
# v1.55: Add deploy templates API.
# v1.56: Add support for building configdrives.
# v1.57: Add support for updating an exisiting allocation.
MINOR_0_JUNO = 0
MINOR_1_INITIAL_VERSION = 1
@ -152,6 +153,7 @@ MINOR_53_PORT_SMARTNIC = 53
MINOR_54_EVENTS = 54
MINOR_55_DEPLOY_TEMPLATES = 55
MINOR_56_BUILD_CONFIGDRIVE = 56
MINOR_57_ALLOCATION_UPDATE = 57
# When adding another version, update:
# - MINOR_MAX_VERSION
@ -159,7 +161,7 @@ MINOR_56_BUILD_CONFIGDRIVE = 56
# explanation of what changed in the new version
# - common/release_mappings.py, RELEASE_MAPPING['master']['api']
MINOR_MAX_VERSION = MINOR_56_BUILD_CONFIGDRIVE
MINOR_MAX_VERSION = MINOR_57_ALLOCATION_UPDATE
# String representations of the minor and maximum versions
_MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)

5
ironic/common/policy.py
View File

@ -423,6 +423,11 @@ allocation_policies = [
'Delete Allocation records',
[{'path': '/allocations/{allocation_id}', 'method': 'DELETE'},
{'path': '/nodes/{node_ident}/allocation', 'method': 'DELETE'}]),
policy.DocumentedRuleDefault(
'baremetal:allocation:update',
'rule:is_admin',
'Change name and extra fields of an allocation',
[{'path': '/allocations/{allocation_id}', 'method': 'PATCH'}]),
]
event_policies = [

2
ironic/common/release_mappings.py
View File

@ -163,7 +163,7 @@ RELEASE_MAPPING = {
}
},
'master': {
'api': '1.56',
'api': '1.57',
'rpc': '1.48',
'objects': {
'Allocation': ['1.0'],

172
ironic/tests/unit/api/controllers/v1/test_allocation.py
View File

@ -386,10 +386,180 @@ class TestPatch(test_api_base.BaseApiTest):
'value': 'bar',
'op': 'add'}],
expect_errors=True,
headers=self.headers)
headers={api_base.Version.string: '1.56'})
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int)
def test_update_not_found(self):
uuid = uuidutils.generate_uuid()
response = self.patch_json('/allocations/%s' % uuid,
[{'path': '/name', 'value': 'b',
'op': 'replace'}],
expect_errors=True,
headers=self.headers)
self.assertEqual(http_client.NOT_FOUND, response.status_int)
self.assertEqual('application/json', response.content_type)
self.assertTrue(response.json['error_message'])
def test_add(self):
response = self.patch_json('/allocations/%s' % self.allocation.uuid,
[{'path': '/extra/foo', 'value': 'bar',
'op': 'add'}], headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_int)
def test_add_non_existent(self):
response = self.patch_json('/allocations/%s' % self.allocation.uuid,
[{'path': '/foo', 'value': 'bar',
'op': 'add'}],
expect_errors=True,
headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
self.assertTrue(response.json['error_message'])
def test_add_multi(self):
response = self.patch_json('/allocations/%s' % self.allocation.uuid,
[{'path': '/extra/foo1', 'value': 'bar1',
'op': 'add'},
{'path': '/extra/foo2', 'value': 'bar2',
'op': 'add'}], headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
result = self.get_json('/allocations/%s' % self.allocation.uuid,
headers=self.headers)
expected = {"foo1": "bar1", "foo2": "bar2"}
self.assertEqual(expected, result['extra'])
def test_replace_invalid_name(self):
response = self.patch_json('/allocations/%s' % self.allocation.uuid,
[{'path': '/name', 'value': '[test]',
'op': 'replace'}],
expect_errors=True,
headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
self.assertTrue(response.json['error_message'])
@mock.patch.object(notification_utils, '_emit_api_notification')
@mock.patch.object(timeutils, 'utcnow')
def test_replace_singular(self, mock_utcnow, mock_notify):
test_time = datetime.datetime(2000, 1, 1, 0, 0)
mock_utcnow.return_value = test_time
response = self.patch_json('/allocations/%s' % self.allocation.uuid,
[{'path': '/name',
'value': 'test', 'op': 'replace'}],
headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
result = self.get_json('/allocations/%s' % self.allocation.uuid,
headers=self.headers)
self.assertEqual('test', result['name'])
return_updated_at = timeutils.parse_isotime(
result['updated_at']).replace(tzinfo=None)
self.assertEqual(test_time, return_updated_at)
mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update',
obj_fields.NotificationLevel.INFO,
obj_fields.NotificationStatus.START),
mock.call(mock.ANY, mock.ANY, 'update',
obj_fields.NotificationLevel.INFO,
obj_fields.NotificationStatus.END)])
@mock.patch.object(notification_utils, '_emit_api_notification')
@mock.patch.object(objects.Allocation, 'save')
def test_update_error(self, mock_save, mock_notify):
mock_save.side_effect = Exception()
allocation = obj_utils.create_test_allocation(self.context)
self.patch_json('/allocations/%s' % allocation.uuid, [{'path': '/name',
'value': 'new', 'op': 'replace'}],
expect_errors=True, headers=self.headers)
mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update',
obj_fields.NotificationLevel.INFO,
obj_fields.NotificationStatus.START),
mock.call(mock.ANY, mock.ANY, 'update',
obj_fields.NotificationLevel.ERROR,
obj_fields.NotificationStatus.ERROR)])
def test_replace_multi(self):
extra = {"foo1": "bar1", "foo2": "bar2", "foo3": "bar3"}
allocation = obj_utils.create_test_allocation(
self.context, extra=extra, uuid=uuidutils.generate_uuid())
new_value = 'new value'
response = self.patch_json('/allocations/%s' % allocation.uuid,
[{'path': '/extra/foo2',
'value': new_value, 'op': 'replace'}],
headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
result = self.get_json('/allocations/%s' % allocation.uuid,
headers=self.headers)
extra["foo2"] = new_value
self.assertEqual(extra, result['extra'])
def test_remove_uuid(self):
response = self.patch_json('/allocations/%s' % self.allocation.uuid,
[{'path': '/uuid', 'op': 'remove'}],
expect_errors=True,
headers=self.headers)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
self.assertEqual('application/json', response.content_type)
self.assertTrue(response.json['error_message'])
def test_remove_singular(self):
allocation = obj_utils.create_test_allocation(
self.context, extra={'a': 'b'}, uuid=uuidutils.generate_uuid())
response = self.patch_json('/allocations/%s' % allocation.uuid,
[{'path': '/extra/a', 'op': 'remove'}],
headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
result = self.get_json('/allocations/%s' % allocation.uuid,
headers=self.headers)
self.assertEqual(result['extra'], {})
# Assert nothing else was changed
self.assertEqual(allocation.uuid, result['uuid'])
def test_remove_multi(self):
extra = {"foo1": "bar1", "foo2": "bar2", "foo3": "bar3"}
allocation = obj_utils.create_test_allocation(
self.context, extra=extra, uuid=uuidutils.generate_uuid())
# Removing one item from the collection
response = self.patch_json('/allocations/%s' % allocation.uuid,
[{'path': '/extra/foo2', 'op': 'remove'}],
headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
result = self.get_json('/allocations/%s' % allocation.uuid,
headers=self.headers)
extra.pop("foo2")
self.assertEqual(extra, result['extra'])
# Removing the collection
response = self.patch_json('/allocations/%s' % allocation.uuid,
[{'path': '/extra', 'op': 'remove'}],
headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
result = self.get_json('/allocations/%s' % allocation.uuid,
headers=self.headers)
self.assertEqual({}, result['extra'])
# Assert nothing else was changed
self.assertEqual(allocation.uuid, result['uuid'])
def test_remove_non_existent_property_fail(self):
response = self.patch_json(
'/allocations/%s' % self.allocation.uuid,
[{'path': '/extra/non-existent', 'op': 'remove'}],
expect_errors=True, headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.BAD_REQUEST, response.status_code)
self.assertTrue(response.json['error_message'])
def _create_locally(_api, _ctx, allocation, _topic):
allocation.create()

5
releasenotes/notes/allow-allocation-update-94d862c3da454be2.yaml Normal file
View File

@ -0,0 +1,5 @@
---
features:
- |
Adds REST API endpoint for updating an existing allocation. Only ``name``
and ``extra`` fields are allowed to be updated.