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

Bugfix for cleanup-pods command #20417

Merged
merged 1 commit into from
Dec 27, 2021
Merged

Conversation

humit0
Copy link
Contributor

@humit0 humit0 commented Dec 20, 2021

When getting cleanup pods list, it requires to have execution_date label.
But from the code, it does not set execution_date label when trigger by scheduler.

So I remove execution_date label when get pod list.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@boring-cyborg boring-cyborg bot added area:CLI provider:cncf-kubernetes Kubernetes provider related issues labels Dec 20, 2021
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

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

We can't just remove execution_date date alone as it will otherwise remove tasks from other execution_date or run_ids too.

We add a run_id or an execution_date to labels, see:

if date:
annotations['execution_date'] = date.isoformat()
labels['execution_date'] = datetime_to_label_safe_datestring(date)
if run_id:
annotations['run_id'] = run_id
labels['run_id'] = make_safe_label_value(run_id)

@humit0
Copy link
Contributor Author

humit0 commented Dec 21, 2021

@kaxil From original code, it removes all success, evicted, and (failed & restartPolicy: Never) pods, not filter pod by execution_date or run_id.
So I think execution_date part is unnecessary.

@kaxil
Copy link
Member

kaxil commented Dec 21, 2021

@kaxil From original code, it removes all success, evicted, and (failed & restartPolicy: Never) pods, not filter pod by execution_date or run_id. So I think execution_date part is unnecessary.

I will take another look later today or tomorrow

@kaxil kaxil merged commit 3b709cc into apache:main Dec 27, 2021
@humit0 humit0 deleted the fix-cleanup-pods-command branch January 8, 2022 05:58
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:CLI provider:cncf-kubernetes Kubernetes provider related issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants