-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Add delete_active_pod cleanup mode and unit tests for process_pod_deletion #59160
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
Add delete_active_pod cleanup mode and unit tests for process_pod_deletion #59160
Conversation
|
@SameerMesiah97, I see you have taken condition isnt this true for all the running tasks? Also, can we implement a timebased mechanism for this? as some pods might be in pending state due to resource constaints for 1-2 mins and we dont want to delete them. |
In both of these 2 scenarios, only the pod(s) related to the tasks calling these functions will be affected.
The motivation behind this PR is to give users the option to delete zombie pods that may continue to be active after the task has been terminated. We could add a wait period before deletion for active pods but I am not sure if it belongs in this function as it is intended to facilitate clean ups rather than fetching existing pods when the task begins execution. Perhaps, this is out of scope? Thank you for the feedback. |
b4b9593 to
3d300d6
Compare
3d300d6 to
2f19864
Compare
|
This is just a soft reminder for review. I believe you are also able to review changes to Kubernetes Providers? I would be grateful if you could provide your comments. |
I do in general review in all sorts of PRs and this is usual. The both codeowners are added for convenience to be informed about new PRs. |
jscheffl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for me. Thanks for the addition!
|
Hey, sorry for jumping in just now, but DELETE_SUCCEEDED_POD should delete running and pending pods. I don’t think this is a backward feature, since it might lead to unnecessary resource consumption and infrastructure costs, which is why I opened the ticket. Introducing a new configuration makes this more confusing. |
…pache#59160) Co-authored-by: Sameer Mesiah <smesiah971@gmail.com>
Description
This PR introduces a new
on_finish_actionmode,delete_active_pod, which deletes pods that are in the 'Pending' or 'Running' state when the task finishes. This allows Airflow to clean up zombie or orphaned pods that never reach a terminal phase.Why introduce a new flag instead of changing
delete_succeeded_pod?The behavior of
delete_succeeded_podwas left unchanged to avoid breaking backward compatibility. Changing its semantics to also delete active pods could alter expectations for existing users who rely on the current behavior for debugging or operational workflows. Introducing a new explicit flag ensures clarity while preserving existing behavior.Tests
The deletion logic inside process_pod_deletion previously had no direct unit test coverage.
This PR adds a parameterized test that verifies pod deletion behavior across all supported modes ('delete_pod', 'delete_succeeded_pod', and the new 'delete_active_pod') and pod phases ('Pending', 'Running', 'Completed and Succeeded'). This ensures that the new flag behaves correctly and that existing behavior remains stable.
Existing tests have been updated where applicable to handle the 'delete_active_pod' cleanup mode.
Documentation
Docstring updated to reflect the new
delete_active_podoption.Related Issues
closes: #59083