Only allow for deleting completed executions

A force parameter was added in case deleting a running execution is
needed but that will spew all kinds of errors afterwards
The default is force = False

Change-Id: I19d822800a1ee056682daeaa7ddf0f400392525d
Closes-bug: 1598135
Depends-On: Id935265fcbe0a3072ba2d693edf54b892173fbdf
This commit is contained in:
Adriano Petrich 2018-04-12 08:03:50 +01:00 committed by Dougal Matthews
parent bfb6548027
commit af84fa9181
3 changed files with 65 additions and 4 deletions

View File

@ -268,16 +268,29 @@ class ExecutionsController(rest.RestController):
return resources.Execution.from_dict(result)
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(None, wtypes.text, status_code=204)
def delete(self, id):
@wsme_pecan.wsexpose(None, wtypes.text, bool, status_code=204)
def delete(self, id, force=False):
"""Delete the specified Execution.
:param id: UUID of execution to delete.
:param force: Optional. Force the deletion of unfinished executions.
Default: false. While the api is backward compatible
the behaviour is not the same. The new default is the
safer option
"""
acl.enforce('executions:delete', context.ctx())
LOG.debug("Delete execution [id=%s]", id)
if not force:
wf_ex = db_api.get_workflow_execution(id)
if not states.is_completed(wf_ex.state):
raise exc.NotAllowedException(
"Only completed executions can be deleted."
" Execution {} is in {} state".format(id, wf_ex.state)
)
return rest_utils.rest_retry_on_db_error(
db_api.delete_workflow_execution
)(id)

View File

@ -144,6 +144,14 @@ MOCK_EMPTY = mock.MagicMock(return_value=[])
MOCK_NOT_FOUND = mock.MagicMock(side_effect=exc.DBEntityNotFoundError())
MOCK_ACTION_EXC = mock.MagicMock(side_effect=exc.ActionException())
ERROR_WF_EX = copy.deepcopy(WF_EX)
ERROR_WF_EX['state'] = states.ERROR
MOCK_ERROR_WF_EX = mock.MagicMock(return_value=ERROR_WF_EX)
SUCCESS_WF_EX = copy.deepcopy(WF_EX)
SUCCESS_WF_EX['state'] = states.SUCCESS
MOCK_SUCCESS_WF_EX = mock.MagicMock(return_value=SUCCESS_WF_EX)
@mock.patch.object(rpc_base, '_IMPL_CLIENT', mock.Mock())
class TestExecutionsController(base.APITest):
@ -693,8 +701,39 @@ class TestExecutionsController(base.APITest):
self.assertIn('Bad response: 400', context.args[0])
@mock.patch.object(db_api, 'delete_workflow_execution', MOCK_DELETE)
def test_delete(self):
@mock.patch.object(
db_api,
'get_workflow_execution',
MOCK_WF_EX
)
def test_delete_unfished_execution(self):
resp = self.app.delete('/v2/executions/123', expect_errors=True)
self.assertEqual(403, resp.status_int)
self.assertIn(
"Only completed executions can be deleted. "
"Execution 123 is in RUNNING state",
resp.body.decode()
)
@mock.patch.object(db_api,
'get_workflow_execution',
MOCK_ERROR_WF_EX)
@mock.patch.object(db_api,
'delete_workflow_execution',
MOCK_DELETE)
def test_delete_error_exec(self):
resp = self.app.delete('/v2/executions/123')
self.assertEqual(204, resp.status_int)
@mock.patch.object(db_api,
'get_workflow_execution',
MOCK_SUCCESS_WF_EX)
@mock.patch.object(db_api,
'delete_workflow_execution',
MOCK_DELETE)
def test_delete_success_exec(self):
resp = self.app.delete('/v2/executions/123')
self.assertEqual(204, resp.status_int)

View File

@ -0,0 +1,9 @@
---
features:
- Use of the parameter force to forcefully delete executions. Note using this parameter
on unfinished executions might cause a cascade of errors.
issues:
- Deleting unfinished executions might cause a cascade of errors, so the standard behaviour
has been changed to delete only safe to delete executions and a new parameter force
was added to forceful delete ignoring the state the execution is in.