From c9bfb763c2e67e299f650c78f6b10597ff01a8b1 Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Thu, 28 Aug 2025 16:16:01 +0100 Subject: [PATCH] Allow status_message updates for actions in SKIPPED state Fixed action status_message update restrictions to allow updates when action is already in SKIPPED state. Previously, users could only update the status_message when initially transitioning to SKIPPED state. Changes include: - Modified validation logic to allow status_message updates for SKIPPED actions - Changed exception type from PatchError to Conflict for better semantics - Added comprehensive test coverage for the new behavior - Updated API documentation and samples - Added release note documenting the fix This enables administrators to fix typos, provide more detailed explanations, or expand on reasons in action status messages after the action has been skipped. Generated-By: claude-code Closes-Bug: #2121601 Change-Id: I64def708389a8ecd32080fba1638a4499ead349d Signed-off-by: Sean Mooney --- api-ref/source/parameters.yaml | 3 ++ .../action-update-status-message-request.json | 7 +++ ...action-update-status-message-response.json | 29 +++++++++++ api-ref/source/watcher-api-v1-actions.inc | 49 +++++++++++++++++++ ...tes-in-skipped-state-a8b4c5d7e9f2g3h1.yaml | 11 +++++ .../controllers/rest_api_version_history.rst | 5 +- watcher/api/controllers/v1/action.py | 13 +++-- watcher/tests/api/v1/test_actions.py | 34 +++++++++++-- 8 files changed, 141 insertions(+), 10 deletions(-) create mode 100644 api-ref/source/samples/action-update-status-message-request.json create mode 100644 api-ref/source/samples/action-update-status-message-response.json create mode 100644 releasenotes/notes/bug-2121601-allow-status-message-updates-in-skipped-state-a8b4c5d7e9f2g3h1.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index eff542ccd..520291ad8 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -192,6 +192,9 @@ action_state: action_status_message: description: | Message with additional information about the Action state. + This field can be set when transitioning an action to SKIPPED state, + or updated for actions that are already in SKIPPED state to provide + more detailed explanations, fix typos, or expand on initial reasons. in: body required: false type: string diff --git a/api-ref/source/samples/action-update-status-message-request.json b/api-ref/source/samples/action-update-status-message-request.json new file mode 100644 index 000000000..62da3126e --- /dev/null +++ b/api-ref/source/samples/action-update-status-message-request.json @@ -0,0 +1,7 @@ +[ + { + "op": "replace", + "value": "Action skipped due to scheduled maintenance window", + "path": "/status_message" + } +] \ No newline at end of file diff --git a/api-ref/source/samples/action-update-status-message-response.json b/api-ref/source/samples/action-update-status-message-response.json new file mode 100644 index 000000000..ed96a6334 --- /dev/null +++ b/api-ref/source/samples/action-update-status-message-response.json @@ -0,0 +1,29 @@ +{ + "state": "SKIPPED", + "description": "Migrate instance to another compute node", + "parents": [ + "b4529294-1de6-4302-b57a-9b5d5dc363c6" + ], + "links": [ + { + "rel": "self", + "href": "http://controller:9322/v1/actions/54acc7a0-91b0-46ea-a5f7-4ae2b9df0b0a" + }, + { + "rel": "bookmark", + "href": "http://controller:9322/actions/54acc7a0-91b0-46ea-a5f7-4ae2b9df0b0a" + } + ], + "action_plan_uuid": "4cbc4ede-0d25-481b-b86e-998dbbd4f8bf", + "uuid": "54acc7a0-91b0-46ea-a5f7-4ae2b9df0b0a", + "deleted_at": null, + "updated_at": "2018-04-10T12:20:15.123456+00:00", + "input_parameters": { + "migration_type": "live", + "destination_node": "compute-2", + "resource_id": "a1b2c3d4-e5f6-7890-1234-567890abcdef" + }, + "action_type": "migrate", + "created_at": "2018-04-10T11:59:12.725147+00:00", + "status_message": "Action skipped by user. Reason: Action skipped due to scheduled maintenance window" +} \ No newline at end of file diff --git a/api-ref/source/watcher-api-v1-actions.inc b/api-ref/source/watcher-api-v1-actions.inc index 0968bb73f..a9f51e3c7 100644 --- a/api-ref/source/watcher-api-v1-actions.inc +++ b/api-ref/source/watcher-api-v1-actions.inc @@ -210,4 +210,53 @@ Response **Example JSON representation of a skipped Action:** .. literalinclude:: samples/action-skip-response.json + :language: javascript + +Update Action Status Message +============================ + +.. rest_method:: PATCH /v1/actions/{action_ident} + +Updates the status_message of an Action that is already in SKIPPED state. + +.. note:: + The status_message field can only be updated for Actions that are currently + in SKIPPED state. This allows administrators to fix typos, provide more + detailed explanations, or expand on reasons that were initially omitted. + This operation requires API microversion 1.5 or later. + +Normal response codes: 200 + +Error codes: 400,404,403,409 + +Request +------- + +.. rest_parameters:: parameters.yaml + + - action_ident: action_ident + +**Example status_message update request for a SKIPPED action:** + +.. literalinclude:: samples/action-update-status-message-request.json + :language: javascript + +Response +-------- + +.. rest_parameters:: parameters.yaml + + - uuid: uuid + - action_type: action_type + - state: action_state + - action_plan_uuid: action_action_plan_uuid + - parents: action_parents + - description: action_description + - input_parameters: action_input_parameters + - links: links + - status_message: action_status_message + +**Example JSON representation of an Action with updated status_message:** + +.. literalinclude:: samples/action-update-status-message-response.json :language: javascript \ No newline at end of file diff --git a/releasenotes/notes/bug-2121601-allow-status-message-updates-in-skipped-state-a8b4c5d7e9f2g3h1.yaml b/releasenotes/notes/bug-2121601-allow-status-message-updates-in-skipped-state-a8b4c5d7e9f2g3h1.yaml new file mode 100644 index 000000000..024b4125a --- /dev/null +++ b/releasenotes/notes/bug-2121601-allow-status-message-updates-in-skipped-state-a8b4c5d7e9f2g3h1.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Fixed action status_message update restrictions to allow updates when + action is in SKIPPED state. Previously, users could only update the + status_message when initially changing the action state to SKIPPED. + Now users can update the status_message field at any time while the + action remains in SKIPPED state, enabling them to fix typos, provide + more detailed explanations, or expand on reasons that were initially + omitted. For more details, see the bug report: + https://bugs.launchpad.net/watcher/+bug/2121601 \ No newline at end of file diff --git a/watcher/api/controllers/rest_api_version_history.rst b/watcher/api/controllers/rest_api_version_history.rst index 0ab78a9ac..783b2468f 100644 --- a/watcher/api/controllers/rest_api_version_history.rst +++ b/watcher/api/controllers/rest_api_version_history.rst @@ -44,4 +44,7 @@ with ``event`` type. --- Added support for SKIPPED actions status via PATCH support for Actions API. This feature also introduces the ``status_message`` field to audits, actions -and action plans. +and action plans. The ``status_message`` field can be set when transitioning +an action to SKIPPED state, and can also be updated for actions that are +already in SKIPPED state, allowing administrators to fix typos, provide more +detailed explanations, or expand on reasons that were initially omitted. diff --git a/watcher/api/controllers/v1/action.py b/watcher/api/controllers/v1/action.py index c33e58ae9..8893ad790 100644 --- a/watcher/api/controllers/v1/action.py +++ b/watcher/api/controllers/v1/action.py @@ -439,15 +439,18 @@ class ActionsController(rest.RestController): ap_state=action_plan.state)) status_message = _("Action skipped by user.") - # status_message update only allowed with status update + # status_message update only allowed with status update or when + # already SKIPPED # NOTE(dviroel): status_message is an exposed field. if action.status_message != action_to_update.status_message: - if action.state == action_to_update.state: + if (action.state == action_to_update.state and + action_to_update.state != objects.action.State.SKIPPED): error_message = _( - "status_message update only allowed with state change") - raise exception.PatchError( + "status_message update only allowed when action state " + "is SKIPPED") + raise exception.Conflict( patch=patch, - reason=error_message) + message=error_message) else: status_message = (_("%(status_message)s Reason: %(reason)s") % dict(status_message=status_message, diff --git a/watcher/tests/api/v1/test_actions.py b/watcher/tests/api/v1/test_actions.py index 5e5884511..10bc1e7b2 100644 --- a/watcher/tests/api/v1/test_actions.py +++ b/watcher/tests/api/v1/test_actions.py @@ -656,17 +656,17 @@ class TestPatchAction(api_base.FunctionalTest): response.json['error_message']) def test_patch_action_status_message_not_allowed(self): - """Test that status_message cannot be patched directly""" + """Test status_message cannot be patched directly when not SKIPPED""" response = self.patch_json( '/actions/%s' % self.action.uuid, [{'path': '/status_message', 'value': 'test message', 'op': 'replace'}], headers={'OpenStack-API-Version': 'infra-optim 1.5'}, expect_errors=True) - self.assertEqual(HTTPStatus.BAD_REQUEST, response.status_int) + self.assertEqual(HTTPStatus.CONFLICT, response.status_int) self.assertEqual('application/json', response.content_type) - self.assertIn("status_message update only allowed with state change", - response.json['error_message']) + self.assertIn("status_message update only allowed when action state " + "is SKIPPED", response.json['error_message']) self.assertIsNone(self.action.status_message) def test_patch_action_one_allowed_one_not_allowed(self): @@ -685,6 +685,32 @@ class TestPatchAction(api_base.FunctionalTest): "can not be updated", response.json['error_message']) self.assertIsNone(self.action.status_message) + def test_patch_action_status_message_allowed_when_skipped(self): + """Test that status_message can be updated when action is SKIPPED""" + # First transition to SKIPPED state + new_state = objects.action.State.SKIPPED + response = self.patch_json( + '/actions/%s' % self.action.uuid, + [{'path': '/state', 'value': new_state, 'op': 'replace'}, + {'path': '/status_message', 'value': 'initial message', + 'op': 'replace'}], + headers={'OpenStack-API-Version': 'infra-optim 1.5'}) + self.assertEqual(HTTPStatus.OK, response.status_int) + self.assertEqual(new_state, response.json['state']) + + # Now update status_message while in SKIPPED state + response = self.patch_json( + '/actions/%s' % self.action.uuid, + [{'path': '/status_message', 'value': 'updated message', + 'op': 'replace'}], + headers={'OpenStack-API-Version': 'infra-optim 1.5'}) + self.assertEqual(HTTPStatus.OK, response.status_int) + self.assertEqual('application/json', response.content_type) + self.assertEqual(new_state, response.json['state']) + self.assertEqual( + 'Action skipped by user. Reason: updated message', + response.json['status_message']) + class TestActionPolicyEnforcement(api_base.FunctionalTest):