From 743c841cc90acef5b738d94d9e8c4d19b82e855e Mon Sep 17 00:00:00 2001 From: Haytham Abuelfutuh Date: Tue, 11 Apr 2023 12:59:52 -0700 Subject: [PATCH 1/2] Enrich TerminateWorkflow error to tell propeller the execution already terminated Signed-off-by: Haytham Abuelfutuh --- pkg/manager/impl/execution_manager.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/manager/impl/execution_manager.go b/pkg/manager/impl/execution_manager.go index 2e33fb758..b92242e45 100644 --- a/pkg/manager/impl/execution_manager.go +++ b/pkg/manager/impl/execution_manager.go @@ -1644,8 +1644,9 @@ func (m *ExecutionManager) TerminateExecution( logger.Infof(ctx, "couldn't find execution [%+v] to save termination cause", request.Id) return nil, err } + if common.IsExecutionTerminal(core.WorkflowExecution_Phase(core.WorkflowExecution_Phase_value[executionModel.Phase])) { - return nil, errors.NewFlyteAdminError(codes.PermissionDenied, "Cannot abort an already terminate workflow execution") + return nil, errors.NewAlreadyInTerminalStateError(ctx, "Cannot abort an already terminate workflow execution", executionModel.Phase) } err = transformers.SetExecutionAborting(&executionModel, request.Cause, getUser(ctx)) @@ -1653,6 +1654,7 @@ func (m *ExecutionManager) TerminateExecution( logger.Debugf(ctx, "failed to add abort metadata for execution [%+v] with err: %v", request.Id, err) return nil, err } + err = m.db.ExecutionRepo().Update(ctx, executionModel) if err != nil { logger.Debugf(ctx, "failed to save abort cause for terminated execution: %+v with err: %v", request.Id, err) From bd45b17832632f60571a5865cb71740c8b95f85e Mon Sep 17 00:00:00 2001 From: Haytham Abuelfutuh Date: Tue, 11 Apr 2023 13:07:11 -0700 Subject: [PATCH 2/2] Fix unit test Signed-off-by: Haytham Abuelfutuh --- pkg/manager/impl/execution_manager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/manager/impl/execution_manager_test.go b/pkg/manager/impl/execution_manager_test.go index 2ccd7b7c8..2fe11bf1a 100644 --- a/pkg/manager/impl/execution_manager_test.go +++ b/pkg/manager/impl/execution_manager_test.go @@ -3231,7 +3231,7 @@ func TestTerminateExecution_AlreadyTerminated(t *testing.T) { assert.Nil(t, resp) s, ok := status.FromError(err) assert.True(t, ok) - assert.Equal(t, codes.PermissionDenied, s.Code()) + assert.Equal(t, codes.FailedPrecondition, s.Code()) } func TestGetExecutionData(t *testing.T) {