Add API to allow update allocation name and extra field

Change-Id: I435e9734f2517b959f8e6124e54fc473c683b702
Story: 2005126
Task: 29796

doc/source/contributor/webapi-version-history.rst

@@ -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)
 --------------------
 

ironic/api/controllers/v1/allocation.py

@@ -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)

ironic/api/controllers/v1/utils.py

@@ -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

ironic/api/controllers/v1/versions.py

@@ -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)

ironic/common/policy.py

@@ -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 = [

ironic/common/release_mappings.py

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

ironic/tests/unit/api/controllers/v1/test_allocation.py

@@ -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()

releasenotes/notes/allow-allocation-update-94d862c3da454be2.yaml

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