Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent pod from being deleted before workflow taskresult complete. Fixes: #12993 #13051

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

shuangkun
Copy link
Member

@shuangkun shuangkun commented May 14, 2024

add a finalizer , prevent pod from being deleted before workflow taskresult complete.

Fixes #12993

Motivation

Modifications

Verification

…. Fixes:argoproj#12993

Signed-off-by: shuangkun <tsk2013uestc@163.com>
@shuangkun shuangkun marked this pull request as draft May 14, 2024 11:41
Signed-off-by: shuangkun <tsk2013uestc@163.com>
Signed-off-by: shuangkun <tsk2013uestc@163.com>
Signed-off-by: shuangkun <tsk2013uestc@163.com>
Signed-off-by: shuangkun <tsk2013uestc@163.com>
Signed-off-by: shuangkun <tsk2013uestc@163.com>
@shuangkun shuangkun marked this pull request as ready for review May 17, 2024 03:54
@shuangkun shuangkun changed the title fix: prevent workflow taskresult from being deleted before completion. Fixes: #12993 fix: prevent pod from being deleted before workflow taskresult complete. Fixes: #12993 May 17, 2024
}
err = we.RemoveFinalizer(ctx, common.FinalizerTaskResultStatus)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The executor removes the new /taskresult finalizer here after the WorkflowTaskResult has been updated. If the executor is interrupted before this point is reached, will the finalizer ever be removed? As far as I understand, the controller does not currently do anything with it.

If the controller were to remove the finalizer, it would also need to make sure that the associated WorkflowTaskResult is left in an appropriate state, as the executor is not guaranteed to have done so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workflow processing fails to complete due to invalid WorkflowTaskResult from interrupted pod
2 participants